rewards_per_chain should not depend on chains without collators #1412
+1,346
−137
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WASM runtime size check:Compared to target branchdancebox runtime: 1900 KB (+4 KB) flashbox runtime: 1128 KB (no changes) ✅ dancelight runtime: 2668 KB (+12 KB) 🚨 starlight runtime: 2580 KB (+12 KB) 🚨 container chain template simple runtime: 1472 KB (no changes) ✅ container chain template frontier runtime: 1832 KB (no changes) ✅ |
Contributor
Coverage Report@@ Coverage Diff @@
## master tomasz-inflation-only-chains-with-collators +/- ##
===============================================================================
+ Coverage 71.03% 71.31% +0.28%
Files 550 550
+ Lines 75341 76090 +749
===============================================================================
+ Hits 53516 54262 +746
+ Misses 21825 21828 +3
|
tmpolaczyk
added
A8-mergeoncegreen
…ly-chains-with-collators
…ly-chains-with-collators
…ly-chains-with-collators
Agusrodri
reviewed
Dec 9, 2025
Agusrodri
reviewed
Dec 9, 2025
|
|
||
| let all_rewards = RewardsPortion::get() * summary.inflation; | ||
| // rewards are shared between the 2 paras | ||
| let orchestrator_rewards = all_rewards / 2; |
Contributor
There was a problem hiding this comment.
This is actually the collator reward and not the orchestrator one right? The name confused me a bit initially.
| const expectedOrchestratorReward = chainRewards / 3n; | ||
| const reward = await filterRewardFromOrchestrator(events, account); | ||
| expect(reward).to.equal(expectedOrchestratorReward); | ||
| expect(reward).to.equal(expectedOrchestratorReward + 1n); |
Contributor
There was a problem hiding this comment.
Why this +1? Is it related to rounding errors?
Contributor
Author
There was a problem hiding this comment.
I wish I knew, because if I put +1 starlight tests fail, and if I don't then dancelight tests fail 😂
Agusrodri
reviewed
Dec 9, 2025
| await polkadotJs.query.system.account(DANCELIGHT_BOND) | ||
| ).data.free.toBigInt(); | ||
|
|
||
| expectedAmountParachainBond += (issuance * 3n) / 10n + dust + 1n; |
Contributor
There was a problem hiding this comment.
Same question here for the +1
…ly-chains-with-collators
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have a small bug in pallet_inflation_rewards, where we calculate the reward per chain based on the total number of registered chains, regardless of whether they have collators or not. This case doesn't happen in any live runtime because we have enough collators for all chains (for now).
In the case of some chains without collators, there is a portion of the inflation that always goes to the parachain bond. This PR fixes this, increasing the rewards_per_chain in that case. It doesn't affect inflation, the total inflated value is still the same. (except in the edge case where we have 0 collators, because now we won't mint anything).