Fix: Era Info voting stake discrepancy when performing unstake/move operations#1476
Fix: Era Info voting stake discrepancy when performing unstake/move operations#1476ipapandinas merged 4 commits intomasterfrom
Conversation
Dinonard
left a comment
There was a problem hiding this comment.
Please also do the test with chopsticks.
You can replay the block on which the issue first appeared, but with the runtime that you build from your PR.
| // expected to be non-zero in case we're past the voting sub-period | ||
| if !post_era_info.total_staked_amount().is_zero() { | ||
| // not fully correct, but good enough for unstake tests | ||
| assert!( | ||
| post_era_info.current_stake_amount.voting >= post_era_info.next_stake_amount.voting |
There was a problem hiding this comment.
I've added the 2nd comment assuming that for move operation it might be untrue. If it's not, the comment can be removed, or at least the wording can be changed.
There was a problem hiding this comment.
Small adjustment here, the voting stake must be strictly equal for current/next era for unstake operations.
| assert_eq!( | ||
| post_era_info.total_staked_amount(), | ||
| pre_era_info.total_staked_amount() - amount, | ||
| pre_era_info.total_staked_amount().saturating_sub(amount), |
There was a problem hiding this comment.
it's better to keep the old -, because if underflow happens, the test will crash.
| contract_stake.stake(stake_amount, era_1, period); | ||
| let total_stake_amount = stake_amount.total(); | ||
|
|
||
| // 1st scenario - unstake some amount from Voting stake don't affect B&E stake |
There was a problem hiding this comment.
Did you intend to add more scenarios here?
Asking just because of the comment.
There was a problem hiding this comment.
Small leftover. No additional scenarios are planned here. I have modified the comment.
|
@Dinonard - I've tested both unstake and move operations with Chopsticks, everything works as expected now |
Minimum allowed line rate is |
That's good! Just to make sure we're on the same page, I meant to say to re-run the old blocks using the new runtime. |
Pull Request Summary
This PR fixes a bug that caused discrepancies in the voting stake in
EraInfo, particularly during unstake or move operations.Context
Previously, the
EraInfo::unstake_amountmethod incorrectly aggregated the unstaked voting and build_and_earn stakes using thetotalmethod, and subtracted the combined amount starting with build_and_earn. While this logic was acceptable for the current era (due to delayed staking eligibility after a move), it was incorrect for the next era leading to an inflated voting stake.Fixes
EraInfo::unstake_amountwith a newsubtract_stakemethod that subtracts voting and build_and_earn balances independently.ContractStaketo ensure consistency across smart contract staking records.SingularStakingInfounstake flow. This is now performed explicitly during a move operation (before applying theinner_stake) and no longer handled during unstake operations.The
era_info_stakes_remain_syncedtest was implemented to reproduce the bug.Check list
try-statetests