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

Payout displays eras already paid #10677

Closed
2 of 7 tasks
Juanma0x opened this issue Jun 17, 2024 · 10 comments · Fixed by #11035
Closed
2 of 7 tasks

Payout displays eras already paid #10677

Juanma0x opened this issue Jun 17, 2024 · 10 comments · Fixed by #11035
Assignees
Labels
Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability. P1 - High Essential for progress, blocks other tasks. Must be completed soon to maintain project flow.

Comments

@Juanma0x
Copy link

  • I'm submitting a ...

    • Bug report
    • Feature request
    • Support request
    • Other
  • What is the current behavior and expected behavior?

Payout should only display unpaid, pending staking rewards.

  • What is the motivation for changing the behavior?

Avoid allowing users to issue extrinsics for reward claims that will fail because the rewards have already been claimed.

  • Please tell us about your environment:

https://polkadot.js.org/apps/#/staking/payout

  • Version:
    Parity Polkadot v1.13.0
    api v11.1.1
    apps v0.138.2-21

  • Environment:

    • Node.js
    • Browser
    • Other (limited support for other environments)
  • Example:
    Random staker: 15rkXKWsDfwPreMDyTyt3riPGkeyj4fvfGeHhkorhqU7MsWQ

Polkadot-JS UI
image

Subscan
image

@TarikGul
Copy link
Member

So the issue here is with RANDOM_STAKER? It might have to do with this comment here, and if I had to take a guess might be resolved once we pass 84 eras since the breaking changes with staking.

@Juanma0x
Copy link
Author

So the issue here is with RANDOM_STAKER? [...]

I observed the same issue for any nominator. The RANDOM_STAKER is just that, a random nominator I used as an example. You can check the issue using any other nominator.

[...] if I had to take a guess might be resolved once we pass 84 eras since the breaking changes with staking.

I hope it's solved, as you commented. There are quite a few failed transactions (alreadyClaimed) because of it:

@Leouarz
Copy link

Leouarz commented Aug 8, 2024

It's the same for us at avail, and there's not legacy reward since the chain started after the staking breaking change.

@dcolley
Copy link

dcolley commented Sep 14, 2024

This is still a problem on polkadot:
image

@TarikGul
Copy link
Member

Yes, this is definitely still an issue and since the big breaking changes in staking this is one of the last nits I haven't been able to pin point. I don't have the bandwidth to solve this currently I have assigned it as an open issue that should be tackled in the team. Hopefully it gets picked up!

@jonasW3F
Copy link

I also just ran into that issue. It would be nice if it could be fixed, because people might spend a lot of fees claiming rewards that are already claimed (there is no error message that the rewards were already claimed).

@Ank4n
Copy link
Contributor

Ank4n commented Oct 14, 2024

I understand that showing unclaimed eras in a backward compatible does not scale well.

Context: Previously, we had a single vec of claimed rewards, but with rewards now being paginated, we could not longer store claimed rewards in bounded vector anymore. Currently, they are stored under different keys for each era, which means the UI must now make up to 84 queries instead of just 1 to accurately determine which eras have unclaimed rewards.

Could we instead update UI to display this differently? For example: we could add a button that scans for the last 10 eras at a time?

Additionally, at minimum, before issuing the payout extrinsic, we should check that the era still needs to be paid out to avoid failed fees.

Please refer the runtime api pending_rewards (1, 2).

@TarikGul TarikGul self-assigned this Oct 14, 2024
@TarikGul
Copy link
Member

If I remember correctly, the biggest issue remains where all the data is coupled and aggregated in the api using the api-derive package - sent to the UI and filtered through a stack of Components.

Could we instead update UI to display this differently? For example: we could add a button that scans for the last 10 eras at a time?

This could be fine but the above issue still remains, and things need to be heavily decoupled (which I am not sure how it will affect the rest of the functionality).

Im assigning myself to this as a second task for me to get done. For now I am just going to try and track down the bug, and fix it with what we have. IMO it will be a bit easier, and less of a headache.

@TarikGul TarikGul moved this from Issues (Bugs) to In Progress in Polkadot-js general project board Oct 14, 2024
@TarikGul
Copy link
Member

Finally have this fixed locally, but it requires a release in the api, which eases the pain in fixing this - so it will be released upstream into apps next week!

@TarikGul TarikGul added P1 - High Essential for progress, blocks other tasks. Must be completed soon to maintain project flow. Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability. labels Oct 30, 2024
@TarikGul TarikGul moved this from In Progress to P1 - High in Polkadot-js general project board Oct 30, 2024
@polkadot-js-bot
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Nov 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability. P1 - High Essential for progress, blocks other tasks. Must be completed soon to maintain project flow.
Development

Successfully merging a pull request may close this issue.

7 participants