apps icon indicating copy to clipboard operation
apps copied to clipboard

Brace for the new reward logic in nomination pools

Open kianenigma opened this issue 2 years ago • 5 comments

As described in https://github.com/paritytech/substrate/pull/11669, the logic for how the rewards are computed is changing in a non-compatible way. This needs some catering for, until all chains are updated with the new logic.

kianenigma avatar Jun 15 '22 18:06 kianenigma

This is about to be baked into the 9.26 release pretty soon FYI https://polkadiff.parity.io/

kianenigma avatar Jul 25 '22 16:07 kianenigma

Thank you. May have to disable it as detected until support is in.

jacogr avatar Jul 25 '22 16:07 jacogr

FYI, even though you said it won't be handy, I am adding this: https://github.com/paritytech/substrate/pull/11831

The transaction to claim rewards is exactly the same as before. We could also add a endpoint that returns the pool's total pending reward.

You can re-implement the logic on your side, but RPC sounds much more sustainable to me.

kianenigma avatar Jul 25 '22 20:07 kianenigma

This will probably make it to 9.27

kianenigma avatar Jul 25 '22 20:07 kianenigma

If you say this will help you, I will see if we can add the RPC in 9.26 as well.

kianenigma avatar Jul 25 '22 20:07 kianenigma

A ping on this, as this will be soon enabled on Kusama and not all functions work (see Westend.).

Pools will also be on Polkadot hopefully in the next 4-8 weeks, which makes https://github.com/polkadot-js/apps/issues/7808 and https://github.com/polkadot-js/apps/issues/7902 pretty important too.

kianenigma avatar Aug 17 '22 18:08 kianenigma

fwiw, here's an example of how to compute the pending rewards in JS (using overflow prone vanilla number type):

https://github.com/substrate-portfolio/polkadot-portfolio/pull/16/files#diff-17782fa0982182d57be83b90b00797e4ef1da85220c6780e23e310f272389aecR225

Once https://github.com/paritytech/substrate/pull/12095 is merged (9.29), you can use this RPC.

kianenigma avatar Sep 01 '22 11:09 kianenigma

The UI currently does retrieve rewards using the runtime call, but as suggested above (after this fix in API 9.3.1 which seems to have pointed to pre-merge PR status), however it is not quite working on Westend for me at least (everything is always 0 at this point, even with unclaimed pool rewards).

Usage here - https://github.com/polkadot-js/apps/blob/16d1be3c16ee98f8e9f9bf745dede07b557c3f9e/packages/page-staking/src/Actions/Pool/useAccountInfo.ts#L23-L31

jacogr avatar Sep 04 '22 10:09 jacogr

If you want to support it now (which I encourage you do), you should use something along the lines of the link above: https://github.com/substrate-portfolio/polkadot-portfolio/pull/16/files#diff-17782fa0982182d57be83b90b00797e4ef1da85220c6780e23e310f272389aecR225

Once 9.29 is enacted, we will have the runtime API working again.

kianenigma avatar Sep 06 '22 07:09 kianenigma

Thanks for confirming. my observations

I will close this then since the UI does support the correct logic to cater with rewards. Obviously because of an oversight this is not quite working as of yet (it happens) however it seems the runtime call bug has been addressed - once that is in, the UI will "just work, no adjustments" since the runtime is now properly treated as "the source of truth".

PS: Putting in code to work around bugs and then removing it is not quite maintainable with multiple runtimes. And even with the link, I do have my own implementation that was reverse-engineered, however maintenance-wise this is a dead-end and it it not going in. I would just suggest getting the fixed into the networks as a priority, starting with Westend.

(Other codebases may obviously have differing opinions on this and be willing to pay the long-term maintenance price of "either this or that since we cannot trust the runtime", I just believe the cause needed to be fixed instead of plastered over.)

Thanks for the pointers and especially the runtime interfaces which is "the right way to go(tm)", this would be great when fully enabled and runtime, no-bug functional on all relay chains.

jacogr avatar Sep 06 '22 12:09 jacogr

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-bot avatar Sep 13 '22 14:09 polkadot-js-bot