Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: ProjectToken.RevenueSplitLeft mapping #324

Conversation

zeeshanakram3
Copy link
Contributor

This fix makes it possible to unstake participant tokens after the current revenue share is finalized and the new revenue share is started

@WRadoslaw
Copy link
Contributor

Here is a whole function for a context:

export async function processRevenueSplitLeftEvent({
  overlay,
  event: {
    asV1000: [tokenId, memberId, unstakedAmount],
  },
}: EventHandlerContext<'ProjectToken.RevenueSplitLeft'>) {
  const account = await getTokenAccountByMemberByTokenOrFail(overlay, memberId, tokenId)
  account.stakedAmount -= unstakedAmount
  const token = await overlay.getRepository(CreatorToken).getByIdOrFail(tokenId.toString())
  if (token.currentRevenueShareId) {
    // TODO: refactor this as should be true all the times, might be a good idea to panic
    const revenueShare = await overlay
      .getRepository(RevenueShare)
      .getByIdOrFail(token.currentRevenueShareId)
    revenueShare.participantsNum -= 1
    const qRevenueShareParticipation = (
      await overlay
        .getRepository(RevenueShareParticipation)
        .getManyByRelation('accountId', account.id)
    ).find((participation) => participation.recovered === false)
    if (qRevenueShareParticipation) {
      qRevenueShareParticipation.recovered = true
    }
  }
}

I see two problems:

  1. It is possible for a user to unstake tokens/leave revenue share after the creator finalized it. So currentRevenueShareId will be empty at this point (or populated with the next one).
  2. I wonder why we decrease the number of participants, this user participated in the revenue share, leave split event is required to get his tokens back. Plus Atlas uses participantsNum like a counter for a stakers field. To avoid fetching the whole list just to display a number on the revenue share history.
  3. In the light of the possibility of having unrecovered RevenueShareParticipation most of the logic inside the if statement is flawed. Part of it is on the runtime, since now we cannot bind this event to the particular revenue share, just to the token that can have multiple revenue shares. The only thing that comes to my mind is that we could rely on the staked amount and pray that it will change between revenue shares.

@zeeshanakram3
Copy link
Contributor Author

Thanks for pointing out the issues

since now we cannot bind this event to the particular revenue share, just to the token that can have multiple revenue shares. The only thing that comes to my mind is that we could rely on the staked amount and pray that it will change between revenue shares.

Even if the token has multiple revenue shares, the User can only participate in at most one revenue split (this is enforced by runtime), so we can actually so we can actually get the RevenueShareParticipation record for given RevenueSplitLeft using the following filter condition

 const revenueShareParticipation = (
    await overlay
      .getRepository(RevenueShareParticipation)
      .getManyByRelation('accountId', account.id)
  ).find((participation) => participation.recovered === false)

commit: 12bbbe2

Copy link
Contributor

@WRadoslaw WRadoslaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zeeshanakram3 zeeshanakram3 merged commit ba75a4b into Joystream:master Apr 2, 2024
7 checks passed
malchililj added a commit to malchililj/orion that referenced this pull request Sep 3, 2024
* fix: unstake  tokens atfer current revenue share finalized and new revenue share issued

* [offchainState] fix: transform raw json objects to jsonb properties

* fix: ProjectToken.RevenueSplitLeft mapping

* [CRT] dont decrease revenue share participant numbers

* fix: getCumulativeHistoricalShareAllocation custom resolver

* bump package version & update CHANGELOG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants