Fix: Use NextState.ConsolidatedAmounts instead of CurrentState in EpochReward#218
Open
kamoallebabot-blip wants to merge 1 commit intomigalabs:masterfrom
Open
Conversation
…chReward Fixes migalabs#215 ## Problem Validators that are targets of consolidations (post-Electra) have their f_reward values inflated by exactly the consolidated amount (~32 ETH) due to incorrect timing when subtracting ConsolidatedAmounts from the epoch reward calculation. ## Root Cause EpochReward() function was using CurrentState.ConsolidatedAmounts which doesn't contain the consolidated amount until NextState. This caused the subtraction to fail and resulted in inflated f_reward values. ## Solution Changed line 31 in pkg/spec/metrics/standard.go to use NextState.ConsolidatedAmounts instead of CurrentState.ConsolidatedAmounts, aligning with how Deposits are handled on line 42. Before fix: consolidatedAmount, ok := p.CurrentState.ConsolidatedAmounts[valIdx] After fix: consolidatedAmount, ok := p.NextState.ConsolidatedAmounts[valIdx] This ensures consolidated amounts are properly subtracted from the reward calculation for target validators. ## Testing The fix resolves the symptoms where validators show f_reward >> f_max_reward for consolidated amounts, bringing f_reward back to expected ranges.
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
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.
Fixes #215
Problem
Validators that are targets of consolidations (post-Electra) have their
f_rewardvalues inflated by exactly the consolidated amount (~32 ETH) due to incorrect timing when subtractingConsolidatedAmountsfrom the epoch reward calculation.Root Cause
The
EpochReward()function inpkg/spec/metrics/standard.gowas usingCurrentState.ConsolidatedAmountsinstead ofNextState.ConsolidatedAmounts. This caused the subtraction to fail because the consolidated amount is only available inNextStateduring consolidation events.The Fix
Changed line 31 in
pkg/spec/metrics/standard.gofrom:To:
This aligns with how
Depositsare handled (line 42 usesp.NextState.Deposits).Impact
f_rewardvalues for validators receiving consolidationsTesting
The fix resolves the symptoms where validators show
f_reward >> f_max_rewardfor consolidated amounts, bringingf_rewardback to expected ranges.