From de93ae97b8d78c2945bb3525f1995bd920e3caec Mon Sep 17 00:00:00 2001 From: Shane Bammel Date: Tue, 4 Apr 2023 18:48:37 -0500 Subject: [PATCH] Fix overflow in reward calculations Overflow occurs when using updated issuance parameters. --- beacon-chain/core/altair/epoch_precompute.go | 12 ++++--- beacon-chain/core/altair/reward.go | 9 +++-- .../core/epoch/precompute/reward_penalty.go | 34 ++++++++++++------- 3 files changed, 35 insertions(+), 20 deletions(-) diff --git a/beacon-chain/core/altair/epoch_precompute.go b/beacon-chain/core/altair/epoch_precompute.go index 1f1d91c46..1c2f21029 100644 --- a/beacon-chain/core/altair/epoch_precompute.go +++ b/beacon-chain/core/altair/epoch_precompute.go @@ -268,10 +268,12 @@ func AttestationsDelta(beaconState state.BeaconState, bal *precompute.Balance, v cfg := params.BeaconConfig() prevEpoch := time.PrevEpoch(beaconState) finalizedEpoch := beaconState.FinalizedCheckpointEpoch() - increment := cfg.EffectiveBalanceIncrement - factor := cfg.BaseRewardFactor - activeCurrentEpochSqrt := new(big.Int).Sqrt(bal.ActiveCurrentEpoch).Uint64() - baseRewardMultiplier := increment * factor / activeCurrentEpochSqrt + + // baseRewardMultiplier = increment * factor / activeCurrentEpochSqrt + baseRewardMultiplier := new(big.Int).SetUint64(cfg.EffectiveBalanceIncrement) + baseRewardMultiplier.Mul(baseRewardMultiplier, new(big.Int).SetUint64(cfg.BaseRewardFactor)) + baseRewardMultiplier.Div(baseRewardMultiplier, new(big.Int).Sqrt(bal.ActiveCurrentEpoch)) + leak := helpers.IsInInactivityLeak(prevEpoch, finalizedEpoch) // Modified in Altair and Bellatrix. @@ -283,7 +285,7 @@ func AttestationsDelta(beaconState state.BeaconState, bal *precompute.Balance, v inactivityDenominator := bias * inactivityPenaltyQuotient for i, v := range vals { - attDeltas[i], err = attestationDelta(bal, v, baseRewardMultiplier, inactivityDenominator, leak) + attDeltas[i], err = attestationDelta(bal, v, baseRewardMultiplier.Uint64(), inactivityDenominator, leak) if err != nil { return nil, err } diff --git a/beacon-chain/core/altair/reward.go b/beacon-chain/core/altair/reward.go index 5e75e5711..68dff146a 100644 --- a/beacon-chain/core/altair/reward.go +++ b/beacon-chain/core/altair/reward.go @@ -59,6 +59,11 @@ func BaseRewardPerIncrement(activeBalance *big.Int) (uint64, error) { return 0, errors.New("active balance can't be 0") } cfg := params.BeaconConfig() - activeBalanceSqrt := new(big.Int).Sqrt(activeBalance).Uint64() - return cfg.EffectiveBalanceIncrement * cfg.BaseRewardFactor / activeBalanceSqrt, nil + + // baseRewardsPerIncrement = EffectiveBalanceIncrement * BaseRewardFactor / sqrt(activeBalance) + baseRewardsPerIncrement := new(big.Int).SetUint64(cfg.EffectiveBalanceIncrement) + baseRewardsPerIncrement.Mul(baseRewardsPerIncrement, new(big.Int).SetUint64(cfg.BaseRewardFactor)) + baseRewardsPerIncrement.Div(baseRewardsPerIncrement, new(big.Int).Sqrt(activeBalance)) + + return baseRewardsPerIncrement.Uint64(), nil } diff --git a/beacon-chain/core/epoch/precompute/reward_penalty.go b/beacon-chain/core/epoch/precompute/reward_penalty.go index 075201529..3c0f447dc 100644 --- a/beacon-chain/core/epoch/precompute/reward_penalty.go +++ b/beacon-chain/core/epoch/precompute/reward_penalty.go @@ -74,22 +74,27 @@ func AttestationsDelta(state state.ReadOnlyBeaconState, pBal *Balance, vp []*Val prevEpoch := time.PrevEpoch(state) finalizedEpoch := state.FinalizedCheckpointEpoch() - sqrtActiveCurrentEpoch := new(big.Int).Sqrt(pBal.ActiveCurrentEpoch).Uint64() + sqrtActiveCurrentEpoch := new(big.Int).Sqrt(pBal.ActiveCurrentEpoch) for i, v := range vp { rewards[i], penalties[i] = attestationDelta(pBal, sqrtActiveCurrentEpoch, v, prevEpoch, finalizedEpoch) } return rewards, penalties, nil } -func attestationDelta(pBal *Balance, sqrtActiveCurrentEpoch uint64, v *Validator, prevEpoch, finalizedEpoch primitives.Epoch) (uint64, uint64) { +func attestationDelta(pBal *Balance, sqrtActiveCurrentEpoch *big.Int, v *Validator, prevEpoch, finalizedEpoch primitives.Epoch) (uint64, uint64) { if !EligibleForRewards(v) || pBal.ActiveCurrentEpoch.Cmp(big.NewInt(0)) != 1 { return 0, 0 } baseRewardsPerEpoch := params.BeaconConfig().BaseRewardsPerEpoch effectiveBalanceIncrement := new(big.Int).SetUint64(params.BeaconConfig().EffectiveBalanceIncrement) - vb := v.CurrentEpochEffectiveBalance - br := vb * params.BeaconConfig().BaseRewardFactor / sqrtActiveCurrentEpoch / baseRewardsPerEpoch + vb := new(big.Int).SetUint64(v.CurrentEpochEffectiveBalance) + + // br := vb * params.BeaconConfig().BaseRewardFactor / sqrtActiveCurrentEpoch / baseRewardsPerEpoch + baseReward := new(big.Int).Mul(vb, new(big.Int).SetUint64(params.BeaconConfig().BaseRewardFactor)) + baseReward.Div(baseReward, sqrtActiveCurrentEpoch) + br := baseReward.Uint64() / baseRewardsPerEpoch + r, p := uint64(0), uint64(0) currentEpochBalance := new(big.Int).Div(pBal.ActiveCurrentEpoch, effectiveBalanceIncrement).Uint64() @@ -149,7 +154,7 @@ func attestationDelta(pBal *Balance, sqrtActiveCurrentEpoch uint64, v *Validator // `index not in get_unslashed_attesting_indices(state, matching_target_attestations)` if !v.IsPrevEpochTargetAttester || v.IsSlashed { finalityDelay := helpers.FinalityDelay(prevEpoch, finalizedEpoch) - p += vb * uint64(finalityDelay) / params.BeaconConfig().InactivityPenaltyQuotient + p += vb.Uint64() * uint64(finalityDelay) / params.BeaconConfig().InactivityPenaltyQuotient } } return r, p @@ -162,14 +167,14 @@ func ProposersDelta(state state.ReadOnlyBeaconState, pBal *Balance, vp []*Valida rewards := make([]uint64, numofVals) totalBalance := pBal.ActiveCurrentEpoch - balanceSqrt := new(big.Int).Sqrt(totalBalance).Uint64() + balanceSqrt := new(big.Int).Sqrt(totalBalance) // Balance square root cannot be 0, this prevents division by 0. - if balanceSqrt == 0 { - balanceSqrt = 1 + if balanceSqrt.Cmp(big.NewInt(0)) == 0 { + balanceSqrt = balanceSqrt.SetUint64(1) } - baseRewardFactor := params.BeaconConfig().BaseRewardFactor - baseRewardsPerEpoch := params.BeaconConfig().BaseRewardsPerEpoch + baseRewardFactor := new(big.Int).SetUint64(params.BeaconConfig().BaseRewardFactor) + baseRewardsPerEpoch := new(big.Int).SetUint64(params.BeaconConfig().BaseRewardsPerEpoch) proposerRewardQuotient := params.BeaconConfig().ProposerRewardQuotient for _, v := range vp { if uint64(v.ProposerIndex) >= uint64(len(rewards)) { @@ -178,9 +183,12 @@ func ProposersDelta(state state.ReadOnlyBeaconState, pBal *Balance, vp []*Valida } // Only apply inclusion rewards to proposer only if the attested hasn't been slashed. if v.IsPrevEpochAttester && !v.IsSlashed { - vBalance := v.CurrentEpochEffectiveBalance - baseReward := vBalance * baseRewardFactor / balanceSqrt / baseRewardsPerEpoch - proposerReward := baseReward / proposerRewardQuotient + // baseReward := vBalance * baseRewardFactor / balanceSqrt / baseRewardsPerEpoch + baseReward := new(big.Int).SetUint64(v.CurrentEpochEffectiveBalance) + baseReward.Mul(baseReward, baseRewardFactor) + baseReward.Div(baseReward, balanceSqrt) + baseReward.Div(baseReward, baseRewardsPerEpoch) + proposerReward := baseReward.Uint64() / proposerRewardQuotient rewards[v.ProposerIndex] += proposerReward } }