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: duplicate payouts in staking-payouts endpoint #1439

Merged
merged 1 commit into from
May 9, 2024

Conversation

Imod7
Copy link
Contributor

@Imod7 Imod7 commented May 9, 2024

Description

Closes #1438

This PR fixes duplicate payouts found in the accounts/<address>/staking-payouts endpoint when querying some specific cases of nominators.

Root cause

In certain rare cases (eras 6xx), a single nominator may have multiple nominations for the same validator and at the same era, each with a different stake amount.

How Sidecar handles this case

In Sidecar we were not expecting to have this case. We were expecting one stake of a nominator per validator, not multiple. Consequently :

  • When we print payouts we iterate through a list that has now 2 entries of the same validator instead of one. That is why we get twice the same validator in the response.
  • And then, for these two entries, the payout calculations are based on the first stake that we find and not the second (because we were not expecting to have a second actually). That is why we see the same result. So that is not correct.

Suggested Fix

The suggested solution includes adding an index with which we can check the right stake amount that we need to retrieve every time and then return its corresponding payout.

Response (old code)

For some particular cases of nominators (whose addresses we refrain to disclose)
[http://127.0.0.1:8080/accounts/<nominatorAddress>/staking-payouts?at=1879836&depth=1&era=661&unclaimedOnly=false

We would have duplicate payouts in the response as shown below:

{
  "at": {
    "height": "1879836",
    "hash": "0xcb5d9775f65ad9dd2aec3270a1943f988382de26ca10652e0ed8435624f69d6e"
  },
  "erasPayouts": [
    {
      "era": "664",
      "totalEraRewardPoints": "68960",
      "totalEraPayout": "448038005779728",
      "payouts": [
        {
          "validatorId": "<validatorAddress>",
          "nominatorStakingPayout": "9070...4309",
          ...
          "nominatorExposure": "7197...17645"
        },
        {
          "validatorId": "<validatorAddress>",
          "nominatorStakingPayout": "9070...4309",
          ...
          "nominatorExposure": "7197...17645"
        },

      ]
    }
  ]
}

Response (new code)

For the same cases of nominators
[http://127.0.0.1:8080/accounts/<nominatorAddress>/staking-payouts?at=1879836&depth=1&era=661&unclaimedOnly=false

We now have multiple payouts in the response as shown below:

{
  "at": {
    "height": "1879836",
    "hash": "0xcb5d9775f65ad9dd2aec3270a1943f988382de26ca10652e0ed8435624f69d6e"
  },
  "erasPayouts": [
    {
      "era": "664",
      "totalEraRewardPoints": "68960",
      "totalEraPayout": "448038005779728",
      "payouts": [
        {
          "validatorId": "<validatorAddress>",
          "nominatorStakingPayout": "9070...4309",
          ...
          "nominatorExposure": "7197...17645"
        },
        {
          "validatorId": "<validatorAddress>",
          "nominatorStakingPayout": "15...07",
          ...
          "nominatorExposure": "16...551"
        },

      ]
    }
  ]
}

but with different nominatorExposure and nominatorStakingPayout in each case. Each payout was calculated based on the stake of each nomination (of that nominator to that validator and at that era).

Testing

  • Tested all the cases provided by the internal team in Parity and for those specific eras and it works as expected
  • Tested payouts with no multiple nominations to make sure they are not affected

Credits & Thanks

  • to @ruiparitydata for bringing this to our attention. This lead to this Sidecar fix but also a possible fix in the Staking pallet (related issue)
  • to @Ank4n for advising on the most accurate solution among the two options we considered for this issue here
  • to my teammate @bee344 for helping debugging, figuring out the root cause of this issue, discussing on the possible solutions and our next steps.

@Imod7 Imod7 requested a review from a team as a code owner May 9, 2024 16:05
Copy link
Member

@TarikGul TarikGul left a comment

Choose a reason for hiding this comment

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

lgtm 🙏

Copy link
Contributor

@bee344 bee344 left a comment

Choose a reason for hiding this comment

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

Great job!

@Imod7 Imod7 merged commit b751ca4 into master May 9, 2024
19 checks passed
@Imod7 Imod7 deleted the domi-fix-duplicates branch May 9, 2024 21:53
This pull request was closed.
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.

/accounts/{account-id}/staking-payoutsreturning duplicated payouts for some nominators
3 participants