CRT: Revenue splits and AMM may be active at the same time, causing unexpected states

[Lezek123]

As I understand in the current runtime implementation of CRT:

• Revenue split and AMM can be active at the same time (nothing in either activate_amm or issue_revenue_split seems to prevent this)
• Users can buy / sell CRT on AMM during an active revenue split (token.ensure_can_modify_supply() is not called inside buy_on_amm / sell_on_amm, even though both those functions affect the token supply)
• The dividend paid to rev. split participants is calculated based on total supply and paid immediately on participate_in_split call

This means that it's possible to claim over 100% of holders' share of a revenue split, ie.:

1. Member A has 100 CRT which is 100% of the current total supply
2. Member A stakes all the tokens on the current revenue split, claiming 100% of holders' share (revenue_split.allocation)
3. Member B buys 100 CRT on AMM and stakes them on the same split
4. Member B claims 50% of revenue_split.allocation JOY, even though 100% of token.revenue_split.allocation has already been claimed
5. Members A and B together have claimed 150% of token.revenue_split.allocation (the amount they can claim is only limited by the balance of CRT module account)

[bedeho]

Hmm, if this turns out to be correct, what is best fix? blocking AMM perhaps? seems most orderly anyway. please verify @ignazio-bovo , if this turns out to be accurate, we need to have @KubaMikolajczyk review designs and tweak.

[ignazio-bovo]

I have written a test here: https://github.com/ignazio-bovo/joystream/commit/4c792880b0f0396cea450b8211a56907a6f6ca13 and in this scenario the split participation will throw an error saying that the treasury balance is too low.

[bedeho]

and in this scenario the split participation will throw an error saying that the treasury balance is too low.

Ok, thank you for getting on top of this so soon, but should we keep this behaviour? seems more orderly to actually block AMM during splits?

[ignazio-bovo]

I agree that is why I have opened a PR with the fix already. See #4784

[kdembler]

Wow we still shipped this bug 🤦 Fixed in #5127