BlockRewards: Fix how it handle ED for Staking currency.

[lemunozm]

Description

I guess this comes from the previous upgrade when balance/and currencies force ExistentialDeposit usage.

Issue 1: We're using an ED denominated in CFG for the StakingCurrency. This is not critical if both currencies use the same denomination, but we should use the correct ED for Staking.

Issue 2: Because Staking currencies were not in the asset registry. From orml_tokens perspective, the ED is 0 while from block-rewards perspective it's 1 * MICRO_CFG. I guess this is not a problem for us right now, but it should be fixed to avoid future problems.

[lemunozm]

Not sure if possible, but I guess staking currency should have an ED of 0 to avoid all this issues (and migrations each time we use a new reward system). Also for accounting, because now we have in centrifuge a total issuance of: 13,000,002,000,000,000,000 and I would say 2,000,000,000,000 is ED

[lemunozm]

Here, we were using the CFG existential deposit instead of the one from StakeCurrencyId

[lemunozm]

Here we were assuming the ED for StakingCurrency was registered in the orml asset registry or, if not, it was 0. But the value was always T::ExistentialDeposit::get() which maps to 1 * MICRO_CFG.

I maintain the same ED to avoid issues counting the current staking currency issued, but maybe it should ideally contain another value

[lemunozm]

Not sure if possible, but I guess staking currency should have an ED of 0 to avoid all these issues

From some tests I'm doing, it seems possible. I'm not sure exactly what the problem we had was.

[codecov]

Codecov Report

Attention: Patch coverage is 50.00000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 48.53%. Comparing base (79c5800) to head (60c7b28).

Files	Patch %	Lines
pallets/block-rewards/src/lib.rs	66.66%	1 Missing ⚠️
runtime/common/src/lib.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1802   +/-   ##
=======================================
  Coverage   48.53%   48.53%           
=======================================
  Files         167      167           
  Lines       13311    13314    +3     
=======================================
+ Hits         6460     6462    +2     
- Misses       6851     6852    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[wischli]

Not sure if possible, but I guess staking currency should have an ED of 0 to avoid all these issues

From some tests I'm doing, it seems possible. I'm not sure exactly what the problem we had was.

AFAIR the Polkadot v0.9.43 update introduced a required ED for all currencies. Beforehand, we did not require ED for any currency but native. Unfortunately, I don't remember whether relying on unit tests was sufficient here or whether the StakingCurrency ED requirement was a result from testing on a local chain. I think though, some unit tests indicated a failure with ED set to 0 because some balance assertions after distributing block rewards were failing.

[lemunozm]

Mmm... IIRC, when we "discovered" that ED requirement, we didn't have yet CurrencyED type and we were using the balance ExistentialDeposit for all currencies. Which we should't. Maybe for that we encountered that we needed ED != 0.

With some minor modifications, I've got all integration tests passing with ED = 0 for staking. Maybe it deserves after we merge this check if we have any issues in dev/demo if we use ED = 0 for staking.

[lemunozm]

Can I have your approval on this @wischli @mustermeiszer @cdamian ?

[wischli]

Reapproving