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: claimed in staking payout endpoint #1579

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

Imod7
Copy link
Contributor

@Imod7 Imod7 commented Jan 21, 2025

Description

New PR that is replacing the following quite old & outdated PRs :

All changes in the current PR should be aligned with the changes merged & final logic implemented in the #1445 PR of the staking-info endpoint. In the 1445 PR, two very useful guides were added to explain the finalized logic for the field claimed:

  1. the hackMD: "Staking PR - claimed field - Guide".
  2. the README: "STAKING_IMPLEMENTATION_DETAILS.md".

which can be helpful here too.

Changes

  • Renamed fetchCommissionAndLedger to fetchCommissionLedgerAndClaimed because it now also returns claimedRewards.
  • The logic for claimedRewards is based on this PR for the staking-info endpoint.
  • The claimed field returned from staking-payouts is no longer a boolean value; It now has different statuses.
  • A status called undefined was added which means that with the current information, we cannot determine if the rewards of the era were claimed, unclaimed or partially claimed.
    • an example is the case when the only available info is lastReward and it is null, then claimed is set to undefined
  • With the old code, if none of the checks/properties/calls could define the claimedfield, we would continue/skip that era and do not return any info about that era. Now we don't, we just set the claimed field as undefined.
  • Some of the logic was moved from function deriveEraPayouts to fetchCommissionAndLedger.
  • The case era < 518 && chain is Kusama is still working as expected. The era in this case is "automatically" considered as claimed.

Testing

  • Tested cases claimed and unclaimed
  • Tested for era < 518 in Kusama chain
  • Tested a case where lastReward is available and it is null
  • Updated the existing tests
  • I did not test a case with a partially claimed era

Todos

  • Minor adjustment for query param unclaimedOnly to work with the new statuses.
  • Review all the latest changes from fix: claimed in staking info endpoint #1445 and make sure that is all implemented in this endpoint
  • Test with validator and nominator accounts.
  • Update docs.

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.

1 participant