epoch-rewards: remove assertion inside distribute#629
epoch-rewards: remove assertion inside distribute#629mjain-jump wants to merge 1 commit intoanza-xyz:masterfrom
Conversation
|
If this PR represents a change to the sysvar layout, please open a follow-up PR to update the JavaScript client |
buffalojoec
left a comment
buffalojoec
left a comment
There was a problem hiding this comment.
Can't we do debug_assert! instead?
| impl EpochRewards { | ||
| pub fn distribute(&mut self, amount: u64) { | ||
| let new_distributed_rewards = self.distributed_rewards.saturating_add(amount); | ||
| assert!(new_distributed_rewards <= self.total_rewards); |
There was a problem hiding this comment.
@joncinque is this one of those "if there's ever an issue with the lamport accounting, halt the chain as a last resort rather than create an LoF scenario"?
There was a problem hiding this comment.
We were discussing this briefly -- there's only one callsite for this function, so we could hoist the checks into agave to make sure or have this return an error, but I think that would just move the panic up higher into the runtime eventually.
Would that work for the fuzzer @mjain-jump ?
3 participants
When fuzzing FD + Agave at the block level, ensuring that this assertion doesn't trigger is incredibly difficult. Within the fuzzer itself, it involves parsing through all vote / stake accounts / delegations, recomputing partitions for epoch rewards, calculating the total distributed amount, and parsing the epoch rewards sysvar to make sure the
total_rewardsfield is properly bounded. It is a lot easier to simply remove this assertion, or handling it in a more graceful manner.