feat: allow rewards to be paid in any currency

[gregdhill]

Signed-off-by: Gregory Hill [email protected]

An oversight required for #826 to pay rewards in all currencies, otherwise we would get undefined results when distributing currencies that were not KINT/INTR or kBTC/iBTC. I have not updated the staking pallet since this is only required for farming rewards, Vaults are unaffected.

I wanted to simply iterate over the currencies in RewardPerToken but there was no way to do that just for the first key / prefix so the complexity for collecting and de-duplicating that list would be too great. Instead I have added a new storage value for all the reward currencies.

See also: paritytech/substrate#13162

[sander2]

I wanted to simply iterate over the currencies in RewardPerToken but there was no way to do that just for the first key / prefix

Can't you use https://github.com/paritytech/substrate/blob/f7e53fe1a8c0e15604cc30177c312241624831bf/frame/support/src/storage/types/double_map.rs#L319 ?

[sander2]

Hmm vault rewards iteration will remain unchanged, but I'm a little bit worried that this may result in a lot of unnecessary iteration in for amm farming rewards. It's not unthinkable that each pool will have some reward for each of the currencies in the pool, so it adds up.. Maybe we should have the rewards api functions take an explicit vec of reward currencies? Or perhaps a config trait item that gets the reward currencies for the given item

[gregdhill]

Hmm vault rewards iteration will remain unchanged, but I'm a little bit worried that this may result in a lot of unnecessary iteration in for amm farming rewards. It's not unthinkable that each pool will have some reward for each of the currencies in the pool, so it adds up.. Maybe we should have the rewards api functions take an explicit vec of reward currencies? Or perhaps a config trait item that gets the reward currencies for the given item

In practice I don't think it will be so bad but we should definitely parameterize the weights here at some point. The point of this approach is that you would not need to do a runtime upgrade every time we add a rewards currency, besides this would only add an additional loop when we start using a rewards currency which is required regardless.

[sander2]

besides this would only add an additional loop when we start using a rewards currency which is required regardless.

A loop is necessary, but the loop doesn't necessarily need to be over all currencies, right? If LpToken(DOT, IBTC) has rewards in DOT and IBTC, and LpToken(INTR, MOVR) has rewards in INTR and MOVR, then when you operate on the LpToken(DOT, IBTC) token, there is no need to iterate over INTR and MOVR

In practice I don't think it will be so bad

Maybe you're right, idk. That depends on how many pools with how many rewards we'll launch, and on what period we use for reward distribution. It's a lot more scalable when you don't iterate over all currencies but maybe it's premature optimization..

[gregdhill]

A loop is necessary, but the loop doesn't necessarily need to be over all currencies, right? If LpToken(DOT, IBTC) has rewards in DOT and IBTC, and LpToken(INTR, MOVR) has rewards in INTR and MOVR, then when you operate on the LpToken(DOT, IBTC) token, there is no need to iterate over INTR and MOVR

Ah yeah that is true, maybe this RewardCurrencies storage value can instead be a map over each pool id? Although that does increase the storage cost vs the computational complexity.