Remove concept of Domain in pallet-rewards

[wischli]

Which part of the code is the issue addressing?

• Addressing a bug
• Refactoring
• Feature

Description

• Remove concept of Domain from pallet-rewards
• Add documentation to force a unique instance of pallet-rewards per reward system, e.g. pallet-block-rewards and pallet-liquidity-rewards

Research/based on

#1198 (comment)

How will this affect the code base

• Reduces complexity
• Improves readability

What are forseen obstacles or hurdles to overcome?

• Investigate potential currency collisions using the same pallet-rewards
• We need to enforce separate instances of pallet-rewards in the future if we add more consumers

[lemunozm]

@mikiquantum WDYT about this? If you agree, I'll go with this first before adding rewards to altair/centrifuge runtime to avoid migrations later.

[mikiquantum]

So is the general idea that the rewards pallet doesnt know anything about the domains is dealing with underneath and since it the access to its storage is merely internal, delegating this identification to the block-rewards & liquidity-rewards pallets, by having a Config trait implemented on each defining its Domain ?
Then we can use the same instance of the pallet_rewards for every parent pallet that uses the same mechanisms.

I think this change makes sense to me.

[lemunozm]

Then we can use the same instance of the pallet_rewards for every parent pallet that uses the same mechanisms.

Yes, and this is what we want to avoid. It's true that domains remove the CurrencyId collision when two pallets use the same instance of pallet-rewards, but we still need to differentiate between groups. So we would need to add a domain to a group.

What I propose is to use different instances of pallet-rewards even if each pallet uses the same mechanisms:

PROS:

• We do not need to deal with Domains which is not trivial to understand well.
• We do not need to implement a Domain for groups that makes it complex

CONS:

• We need to create several instances of pallet-rewards in case we have 2 pallets that use the exact mechanism. But I do not think it's really a problem. The storage used should be almost the same.

[mikiquantum]

If we always create one instance of pallet-rewards for each parent pallet (block or liquidity) then we have all this abstraction for nothing. If we are going to simplify this, I would then try to make pallet_liquidity & pallet_block as self contained as possible. Meaning, all storage definition within that pallet, since we will only allow a pallet_rewards per consumer pallet, I do not think it makes sense to have two pallets and two different storage indexes.

[lemunozm]

I would then try to make pallet_liquidity & pallet_block as self contained as possible. Meaning, all storage definition within that pallet

And how can we achieve this without code duplication if we do not extract it into a pallet? I miss some way to store a pallet into a pallet in Substrate.

[mikiquantum]

Rust inheritance would be nice in this case 😄 . We could use a macro that spits out the storage needed for the reward pallet to be included in the consumer pallets, but that does come with readability issues.

[lemunozm]

I've already tried that for the mock-builder, and I couldn't. The procedural macro of Substrate [frame_support::pallet] on top of the module will be processed first, and when it encounters the declarative macro, it complains. 😞