From cc13e0b7e0aa97f353f817776f7fae22286b6c50 Mon Sep 17 00:00:00 2001 From: tersec Date: Fri, 8 Sep 2023 16:42:18 +0000 Subject: [PATCH] restore full test coverage for process_rewards_and_penalties (#5407) * restore full test coverage for process_rewards_and_penalties * adjust ncli_db to use new iterator --- beacon_chain/spec/state_transition_epoch.nim | 120 +++--------------- ncli/ncli_common.nim | 56 ++++---- .../altair/test_fixture_rewards.nim | 30 +++-- .../bellatrix/test_fixture_rewards.nim | 30 +++-- .../capella/test_fixture_rewards.nim | 30 +++-- .../deneb/test_fixture_rewards.nim | 30 +++-- 6 files changed, 119 insertions(+), 177 deletions(-) diff --git a/beacon_chain/spec/state_transition_epoch.nim b/beacon_chain/spec/state_transition_epoch.nim index d430b0b10..8b9e595c8 100644 --- a/beacon_chain/spec/state_transition_epoch.nim +++ b/beacon_chain/spec/state_transition_epoch.nim @@ -665,63 +665,17 @@ func get_active_increments*( info: altair.EpochInfo | bellatrix.BeaconState): Gwei = info.balances.current_epoch div EFFECTIVE_BALANCE_INCREMENT -# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.1/specs/altair/beacon-chain.md#get_flag_index_deltas -# This version mainly exists for the rewards and state transition epoch -# consensus-spec tests, which check rewards calculations for individual -# TimelyFlags. -iterator get_flag_index_deltas*( - state: altair.BeaconState | bellatrix.BeaconState | capella.BeaconState | - deneb.BeaconState, - flag_index: TimelyFlag, base_reward_per_increment: Gwei, - info: var altair.EpochInfo, finality_delay: uint64): - (ValidatorIndex, RewardDelta) = - ## Return the deltas for a given ``flag_index`` by scanning through the - ## participation flags. - let - previous_epoch = get_previous_epoch(state) - weight = PARTICIPATION_FLAG_WEIGHTS[flag_index] - unslashed_participating_increments = get_unslashed_participating_increment( - info, flag_index) - active_increments = get_active_increments(info) - - for vidx in state.validators.vindices: - if not is_eligible_validator(info.validators[vidx]): - continue - - let base_reward = get_base_reward_increment(state, vidx, base_reward_per_increment) - yield - if is_unslashed_participating_index( - state, flag_index, previous_epoch, vidx): - - let pflag = case flag_index - of TIMELY_SOURCE_FLAG_INDEX: ParticipationFlag.timelySourceAttester - of TIMELY_TARGET_FLAG_INDEX: ParticipationFlag.timelyTargetAttester - of TIMELY_HEAD_FLAG_INDEX: ParticipationFlag.timelyHeadAttester - - info.validators[vidx].flags.incl pflag - - (vidx, RewardDelta( - rewards: get_flag_index_reward( - state, base_reward, active_increments, - unslashed_participating_increments, weight, finality_delay), - penalties: 0.Gwei)) - elif flag_index != TIMELY_HEAD_FLAG_INDEX: - (vidx, RewardDelta( - rewards: 0.Gwei, - penalties: base_reward * weight div WEIGHT_DENOMINATOR)) - else: - (vidx, RewardDelta(rewards: 0.Gwei, penalties: 0.Gwei)) - # https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.1/specs/altair/beacon-chain.md#get_flag_index_deltas # https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.1/specs/altair/beacon-chain.md#modified-get_inactivity_penalty_deltas # https://github.com/ethereum/consensus-specs/blob/v1.3.0/specs/bellatrix/beacon-chain.md#modified-get_inactivity_penalty_deltas # Combines get_flag_index_deltas() and get_inactivity_penalty_deltas() -func update_flag_and_inactivity_deltas( +iterator get_flag_and_inactivity_deltas*( cfg: RuntimeConfig, state: altair.BeaconState | bellatrix.BeaconState | capella.BeaconState | deneb.BeaconState, base_reward_per_increment: Gwei, info: var altair.EpochInfo, - finality_delay: uint64) = + finality_delay: uint64): + (ValidatorIndex, Gwei, Gwei, Gwei, Gwei, Gwei, Gwei) = ## Return the deltas for a given ``flag_index`` by scanning through the ## participation flags. # @@ -793,60 +747,13 @@ func update_flag_and_inactivity_deltas( let penalty_numerator = state.validators[vidx].effective_balance * state.inactivity_scores[vidx] penalty_numerator div penalty_denominator - info.validators[vidx].delta.rewards += - reward(TIMELY_SOURCE_FLAG_INDEX) + reward(TIMELY_TARGET_FLAG_INDEX) + - reward(TIMELY_HEAD_FLAG_INDEX) - info.validators[vidx].delta.penalties += - penalty(TIMELY_SOURCE_FLAG_INDEX) + penalty(TIMELY_TARGET_FLAG_INDEX) + - inactivity_penalty - -# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.1/specs/altair/beacon-chain.md#modified-get_inactivity_penalty_deltas -# Exists for consensus-spec tests which probe for specific flag calculations. -iterator get_inactivity_penalty_deltas*( - cfg: RuntimeConfig, state: altair.BeaconState, info: altair.EpochInfo): - (ValidatorIndex, Gwei) = - ## Return the inactivity penalty deltas by considering timely target - ## participation flags and inactivity scores. - let - penalty_denominator = - cfg.INACTIVITY_SCORE_BIAS * INACTIVITY_PENALTY_QUOTIENT_ALTAIR - previous_epoch = get_previous_epoch(state) - - for vidx in state.validators.vindices: - if not is_eligible_validator(info.validators[vidx]): - continue - - if not is_unslashed_participating_index( - state, TIMELY_TARGET_FLAG_INDEX, previous_epoch, vidx): - let - penalty_numerator = state.validators[vidx].effective_balance * - state.inactivity_scores[vidx] - yield (vidx, Gwei(penalty_numerator div penalty_denominator)) - -# https://github.com/ethereum/consensus-specs/blob/v1.3.0/specs/bellatrix/beacon-chain.md#modified-get_inactivity_penalty_deltas -# Exists for consensus-spec tests which probe for specific flag calculations. -iterator get_inactivity_penalty_deltas*( - cfg: RuntimeConfig, - state: bellatrix.BeaconState | capella.BeaconState | deneb.BeaconState, - info: altair.EpochInfo): (ValidatorIndex, Gwei) = - ## Return the inactivity penalty deltas by considering timely target - ## participation flags and inactivity scores. - let - # [Modified in Bellatrix] - penalty_denominator = - cfg.INACTIVITY_SCORE_BIAS * INACTIVITY_PENALTY_QUOTIENT_BELLATRIX - previous_epoch = get_previous_epoch(state) - - for vidx in state.validators.vindices: - if not is_eligible_validator(info.validators[vidx]): - continue - - if not is_unslashed_participating_index( - state, TIMELY_TARGET_FLAG_INDEX, previous_epoch, vidx): - let - penalty_numerator = state.validators[vidx].effective_balance * - state.inactivity_scores[vidx] - yield (vidx, Gwei(penalty_numerator div penalty_denominator)) + # Yielding these as a structure with identifiable names rather than + # multiple-return-value style creates spurious nimZeroMem calls. + yield + (vidx, reward(TIMELY_SOURCE_FLAG_INDEX), + reward(TIMELY_TARGET_FLAG_INDEX), reward(TIMELY_HEAD_FLAG_INDEX), + penalty(TIMELY_SOURCE_FLAG_INDEX), penalty(TIMELY_TARGET_FLAG_INDEX), + inactivity_penalty) # https://github.com/ethereum/consensus-specs/blob/v1.4.0-alpha.3/specs/phase0/beacon-chain.md#rewards-and-penalties-1 func process_rewards_and_penalties*( @@ -887,8 +794,11 @@ func process_rewards_and_penalties*( finality_delay = get_finality_delay(state) doAssert state.validators.len() == info.validators.len() - update_flag_and_inactivity_deltas( - cfg, state, base_reward_per_increment, info, finality_delay) + for validator_index, reward0, reward1, reward2, penalty0, penalty1, penalty2 in + get_flag_and_inactivity_deltas( + cfg, state, base_reward_per_increment, info, finality_delay): + info.validators[validator_index].delta.rewards += reward0 + reward1 + reward2 + info.validators[validator_index].delta.penalties += penalty0 + penalty1 + penalty2 # Here almost all balances are updated (assuming most validators are active) - # clearing the cache becomes a bottleneck if done item by item because of the diff --git a/ncli/ncli_common.nim b/ncli/ncli_common.nim index 5f3c65862..eaee6a036 100644 --- a/ncli/ncli_common.nim +++ b/ncli/ncli_common.nim @@ -288,37 +288,37 @@ proc collectEpochRewardsAndPenalties*( total_active_balance) finality_delay = get_finality_delay(state) - for flag_index in TimelyFlag: - for validator_index, delta in get_flag_index_deltas( - state, flag_index, base_reward_per_increment, info, finality_delay): - template rp: untyped = rewardsAndPenalties[validator_index] + for validator_index, reward_source, reward_target, reward_head, + penalty_source, penalty_target, penalty_inactivity in + get_flag_and_inactivity_deltas( + cfg, state, base_reward_per_increment, info, finality_delay): + template rp: untyped = rewardsAndPenalties[validator_index] - let - base_reward = get_base_reward_increment( - state, validator_index, base_reward_per_increment) - active_increments = get_active_increments(info) - unslashed_participating_increment = - get_unslashed_participating_increment(info, flag_index) - max_flag_index_reward = get_flag_index_reward( - state, base_reward, active_increments, - unslashed_participating_increment, - PARTICIPATION_FLAG_WEIGHTS[flag_index], - finality_delay) + let + base_reward = get_base_reward_increment( + state, validator_index, base_reward_per_increment) + active_increments = get_active_increments(info) - case flag_index - of TIMELY_SOURCE_FLAG_INDEX: - rp.source_outcome = delta.getOutcome - rp.max_source_reward = max_flag_index_reward - of TIMELY_TARGET_FLAG_INDEX: - rp.target_outcome = delta.getOutcome - rp.max_target_reward = max_flag_index_reward - of TIMELY_HEAD_FLAG_INDEX: - rp.head_outcome = delta.getOutcome - rp.max_head_reward = max_flag_index_reward + template unslashed_participating_increment(flag_index: untyped): untyped = + get_unslashed_participating_increment(info, flag_index) + template max_flag_index_reward(flag_index: untyped): untyped = + get_flag_index_reward( + state, base_reward, active_increments, + unslashed_participating_increment(flag_index), + PARTICIPATION_FLAG_WEIGHTS[flag_index], finality_delay) - for validator_index, penalty in get_inactivity_penalty_deltas( - cfg, state, info): - rewardsAndPenalties[validator_index].inactivity_penalty += penalty + rp.source_outcome = reward_source.int64 - penalty_source.int64 + rp.max_source_reward = + max_flag_index_reward(TimelyFlag.TIMELY_SOURCE_FLAG_INDEX) + rp.target_outcome = reward_target.int64 - penalty_target.int64 + rp.max_target_reward = + max_flag_index_reward(TimelyFlag.TIMELY_TARGET_FLAG_INDEX) + rp.head_outcome = reward_head.int64 + rp.max_head_reward = + max_flag_index_reward(TimelyFlag.TIMELY_HEAD_FLAG_INDEX) + + rewardsAndPenalties[validator_index].inactivity_penalty += + penalty_inactivity rewardsAndPenalties.collectSlashings(state, info.balances.current_epoch) diff --git a/tests/consensus_spec/altair/test_fixture_rewards.nim b/tests/consensus_spec/altair/test_fixture_rewards.nim index b9e464534..effb69c1a 100644 --- a/tests/consensus_spec/altair/test_fixture_rewards.nim +++ b/tests/consensus_spec/altair/test_fixture_rewards.nim @@ -56,17 +56,25 @@ proc runTest(rewardsDir, identifier: string) = let finality_delay = get_finality_delay(state[]) - for flag_index in TimelyFlag: - for validator_index, delta in get_flag_index_deltas( - state[], flag_index, base_reward_per_increment, info, finality_delay): - if not is_eligible_validator(info.validators[validator_index]): - continue - flagDeltas2[flag_index].rewards[validator_index] = delta.rewards - flagDeltas2[flag_index].penalties[validator_index] = delta.penalties - - for validator_index, delta in get_inactivity_penalty_deltas( - defaultRuntimeConfig, state[], info): - inactivityPenaltyDeltas2.penalties[validator_index] = delta + for validator_index, reward0, reward1, reward2, penalty0, penalty1, penalty2 + in get_flag_and_inactivity_deltas( + defaultRuntimeConfig, state[], base_reward_per_increment, info, + finality_delay): + if not is_eligible_validator(info.validators[validator_index]): + continue + flagDeltas2[TimelyFlag.TIMELY_SOURCE_FLAG_INDEX].rewards[validator_index] = + reward0 + flagDeltas2[TimelyFlag.TIMELY_TARGET_FLAG_INDEX].rewards[validator_index] = + reward1 + flagDeltas2[TimelyFlag.TIMELY_HEAD_FLAG_INDEX].rewards[validator_index] = + reward2 + flagDeltas2[TimelyFlag.TIMELY_SOURCE_FLAG_INDEX].penalties[validator_index] = + penalty0 + flagDeltas2[TimelyFlag.TIMELY_TARGET_FLAG_INDEX].penalties[validator_index] = + penalty1 + flagDeltas2[TimelyFlag.TIMELY_HEAD_FLAG_INDEX].penalties[validator_index] = + 0 + inactivityPenaltyDeltas2.penalties[validator_index] = penalty2 check: flagDeltas == flagDeltas2 diff --git a/tests/consensus_spec/bellatrix/test_fixture_rewards.nim b/tests/consensus_spec/bellatrix/test_fixture_rewards.nim index c25ebe073..f08660024 100644 --- a/tests/consensus_spec/bellatrix/test_fixture_rewards.nim +++ b/tests/consensus_spec/bellatrix/test_fixture_rewards.nim @@ -56,17 +56,25 @@ proc runTest(rewardsDir, identifier: string) = let finality_delay = get_finality_delay(state[]) - for flag_index in TimelyFlag: - for validator_index, delta in get_flag_index_deltas( - state[], flag_index, base_reward_per_increment, info, finality_delay): - if not is_eligible_validator(info.validators[validator_index]): - continue - flagDeltas2[flag_index].rewards[validator_index] = delta.rewards - flagDeltas2[flag_index].penalties[validator_index] = delta.penalties - - for validator_index, delta in get_inactivity_penalty_deltas( - defaultRuntimeConfig, state[], info): - inactivityPenaltyDeltas2.penalties[validator_index] = delta + for validator_index, reward0, reward1, reward2, penalty0, penalty1, penalty2 + in get_flag_and_inactivity_deltas( + defaultRuntimeConfig, state[], base_reward_per_increment, info, + finality_delay): + if not is_eligible_validator(info.validators[validator_index]): + continue + flagDeltas2[TimelyFlag.TIMELY_SOURCE_FLAG_INDEX].rewards[validator_index] = + reward0 + flagDeltas2[TimelyFlag.TIMELY_TARGET_FLAG_INDEX].rewards[validator_index] = + reward1 + flagDeltas2[TimelyFlag.TIMELY_HEAD_FLAG_INDEX].rewards[validator_index] = + reward2 + flagDeltas2[TimelyFlag.TIMELY_SOURCE_FLAG_INDEX].penalties[validator_index] = + penalty0 + flagDeltas2[TimelyFlag.TIMELY_TARGET_FLAG_INDEX].penalties[validator_index] = + penalty1 + flagDeltas2[TimelyFlag.TIMELY_HEAD_FLAG_INDEX].penalties[validator_index] = + 0 + inactivityPenaltyDeltas2.penalties[validator_index] = penalty2 check: flagDeltas == flagDeltas2 diff --git a/tests/consensus_spec/capella/test_fixture_rewards.nim b/tests/consensus_spec/capella/test_fixture_rewards.nim index b02d8770c..c673a4cdf 100644 --- a/tests/consensus_spec/capella/test_fixture_rewards.nim +++ b/tests/consensus_spec/capella/test_fixture_rewards.nim @@ -56,17 +56,25 @@ proc runTest(rewardsDir, identifier: string) = let finality_delay = get_finality_delay(state[]) - for flag_index in TimelyFlag: - for validator_index, delta in get_flag_index_deltas( - state[], flag_index, base_reward_per_increment, info, finality_delay): - if not is_eligible_validator(info.validators[validator_index]): - continue - flagDeltas2[flag_index].rewards[validator_index] = delta.rewards - flagDeltas2[flag_index].penalties[validator_index] = delta.penalties - - for validator_index, delta in get_inactivity_penalty_deltas( - defaultRuntimeConfig, state[], info): - inactivityPenaltyDeltas2.penalties[validator_index] = delta + for validator_index, reward0, reward1, reward2, penalty0, penalty1, penalty2 + in get_flag_and_inactivity_deltas( + defaultRuntimeConfig, state[], base_reward_per_increment, info, + finality_delay): + if not is_eligible_validator(info.validators[validator_index]): + continue + flagDeltas2[TimelyFlag.TIMELY_SOURCE_FLAG_INDEX].rewards[validator_index] = + reward0 + flagDeltas2[TimelyFlag.TIMELY_TARGET_FLAG_INDEX].rewards[validator_index] = + reward1 + flagDeltas2[TimelyFlag.TIMELY_HEAD_FLAG_INDEX].rewards[validator_index] = + reward2 + flagDeltas2[TimelyFlag.TIMELY_SOURCE_FLAG_INDEX].penalties[validator_index] = + penalty0 + flagDeltas2[TimelyFlag.TIMELY_TARGET_FLAG_INDEX].penalties[validator_index] = + penalty1 + flagDeltas2[TimelyFlag.TIMELY_HEAD_FLAG_INDEX].penalties[validator_index] = + 0 + inactivityPenaltyDeltas2.penalties[validator_index] = penalty2 check: flagDeltas == flagDeltas2 diff --git a/tests/consensus_spec/deneb/test_fixture_rewards.nim b/tests/consensus_spec/deneb/test_fixture_rewards.nim index fbbfc8c38..bbc4f25c2 100644 --- a/tests/consensus_spec/deneb/test_fixture_rewards.nim +++ b/tests/consensus_spec/deneb/test_fixture_rewards.nim @@ -56,17 +56,25 @@ proc runTest(rewardsDir, identifier: string) = let finality_delay = get_finality_delay(state[]) - for flag_index in TimelyFlag: - for validator_index, delta in get_flag_index_deltas( - state[], flag_index, base_reward_per_increment, info, finality_delay): - if not is_eligible_validator(info.validators[validator_index]): - continue - flagDeltas2[flag_index].rewards[validator_index] = delta.rewards - flagDeltas2[flag_index].penalties[validator_index] = delta.penalties - - for validator_index, delta in get_inactivity_penalty_deltas( - defaultRuntimeConfig, state[], info): - inactivityPenaltyDeltas2.penalties[validator_index] = delta + for validator_index, reward0, reward1, reward2, penalty0, penalty1, penalty2 + in get_flag_and_inactivity_deltas( + defaultRuntimeConfig, state[], base_reward_per_increment, info, + finality_delay): + if not is_eligible_validator(info.validators[validator_index]): + continue + flagDeltas2[TimelyFlag.TIMELY_SOURCE_FLAG_INDEX].rewards[validator_index] = + reward0 + flagDeltas2[TimelyFlag.TIMELY_TARGET_FLAG_INDEX].rewards[validator_index] = + reward1 + flagDeltas2[TimelyFlag.TIMELY_HEAD_FLAG_INDEX].rewards[validator_index] = + reward2 + flagDeltas2[TimelyFlag.TIMELY_SOURCE_FLAG_INDEX].penalties[validator_index] = + penalty0 + flagDeltas2[TimelyFlag.TIMELY_TARGET_FLAG_INDEX].penalties[validator_index] = + penalty1 + flagDeltas2[TimelyFlag.TIMELY_HEAD_FLAG_INDEX].penalties[validator_index] = + 0 + inactivityPenaltyDeltas2.penalties[validator_index] = penalty2 check: flagDeltas == flagDeltas2