Feat: Simplify Emissions

[msgmaxim]

Pull Request

Closes: PRO-854

Checklist

Please conduct a thorough self-review before opening the PR.

• I am confident that the code works.
• I have updated documentation where appropriate.

Summary

As discussed in PRO-854, compounding effect is negligible in this context and we can simplify how rewards are computed:

block_rewards = total_issuance * annual_emission / blocks_in_a_year

I have also added a test (rewards_calculation_compounding) that confirms that even if we compound every block (looks like we do every 150 blocks instead), we still get very close to the emission target.

For sanity, I also checked that the actual per-block rewards on localnet before and after the change weren't too different.

I had to move SECONDS_PER_BLOCK out of runtime constants so that I can refer to it from a pallet, and it is now in cf-primitives. Not sure if there is a better place for it, but at least @kylezs thought cf-primitives was reasonable.

[linear]

PRO-854 Simplify Emissions calculation

maxim pointed out that we can simplify how we calculate the emissions rate.

We can simply use a linear approximation of the per-block inflation %

The advantage of doing this is that it's easier to interpret - users can easily look at the numbers an be confident that everything is correct.

It's a simple and low-risk change.

[codecov]

Codecov Report

Merging #4043 (91d82cb) into main (1fb950c) will decrease coverage by 0%.
Report is 4 commits behind head on main.
The diff coverage is 82%.

@@          Coverage Diff           @@
##            main   #4043    +/-   ##
======================================
- Coverage     72%     72%    -0%     
======================================
  Files        368     369     +1     
  Lines      58380   58597   +217     
  Branches   58380   58597   +217     
======================================
+ Hits       42301   42378    +77     
- Misses     13984   14118   +134     
- Partials    2095    2101     +6     

Files Changed	Coverage Δ
state-chain/cf-integration-tests/src/genesis.rs	77% <ø> (ø)
state-chain/node/src/chain_spec.rs	1% <0%> (ø)
state-chain/pallets/cf-emissions/src/mock.rs	84% <ø> (-<1%)	⬇️
state-chain/primitives/src/lib.rs	67% <ø> (ø)
state-chain/runtime/src/lib.rs	39% <ø> (ø)
...ate-chain/cf-integration-tests/src/mock_runtime.rs	100% <100%> (ø)
state-chain/pallets/cf-emissions/src/lib.rs	82% <100%> (+2%)	⬆️
state-chain/pallets/cf-emissions/src/tests.rs	99% <100%> (+<1%)	⬆️

... and 19 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ramizhasan111]

I disagree with this change (see my comment in the linked linear issue)

[ramizhasan111]

Also, i see where the confusion regarding the annual emission rate being 0.1% comes from. I see that the current rate in chainspec is set at 0.1%. I guess dan changed it to that for testing ? The actual mainnet emission rate is actually around the order of 10%. We should keep it to 10% in the main branch to avoid confusion and change it to 0.1% for testing only in release branches of test networks @dandanlen .

[msgmaxim]

I disagree with this change (see my comment in the linked linear issue)

Responded in the linear issue.

[dandanlen]

I've replied in Linear too. Sorry for the confusion everyone.

I think we should close this, although clearly some clarifying comments in the emissions code might help avoid future confusion. @msgmaxim if there is anything you think would be appropriate to add/change, feel free to propose another PR.

[msgmaxim]

I've replied in Linear too. Sorry for the confusion everyone.

I think we should close this, although clearly some clarifying comments in the emissions code might help avoid future confusion. @msgmaxim if there is anything you think would be appropriate to add/change, feel free to propose another PR.

Perhaps it is still valuable to have a test to show what the effective annual inflation will be if we use the current approach (which I added here). It is still not clear to me that we shouldn't take fee burning into account. Let me keep this open for a few more days (maybe until our meeting on Wednesday).

[msgmaxim]

As discussed, closing in favour of the existing approach. I extracted the test into #4057, as it still seems useful.