Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: sync cached isCompoundingValidatorArr at epoch transition #7247

Merged
merged 2 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions packages/state-transition/src/cache/epochTransitionCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ import {Epoch, RootHex, ValidatorIndex, phase0} from "@lodestar/types";
import {intDiv, toRootHex} from "@lodestar/utils";

import {processPendingAttestations} from "../epoch/processPendingAttestations.js";
import {CachedBeaconStateAllForks, CachedBeaconStateAltair, CachedBeaconStatePhase0} from "../index.js";
import {
CachedBeaconStateAllForks,
CachedBeaconStateAltair,
CachedBeaconStatePhase0,
hasCompoundingWithdrawalCredential,
} from "../index.js";
import {computeBaseRewardPerIncrement} from "../util/altair.js";
import {
FLAG_CURR_HEAD_ATTESTER,
Expand Down Expand Up @@ -133,11 +138,7 @@ export interface EpochTransitionCache {

flags: number[];

/**
* Validators in the current epoch, should use it for read-only value instead of accessing state.validators directly.
* Note that during epoch processing, validators could be updated so need to use it with care.
*/
validators: phase0.Validator[];
isCompoundingValidatorArr: boolean[];

/**
* balances array will be populated by processRewardsAndPenalties() and consumed by processEffectiveBalanceUpdates().
Expand Down Expand Up @@ -209,6 +210,8 @@ const inclusionDelays = new Array<number>();
/** WARNING: reused, never gc'd */
const flags = new Array<number>();
/** WARNING: reused, never gc'd */
const isCompoundingValidatorArr = new Array<boolean>();
/** WARNING: reused, never gc'd */
const nextEpochShufflingActiveValidatorIndices = new Array<number>();

export function beforeProcessEpoch(
Expand Down Expand Up @@ -262,6 +265,10 @@ export function beforeProcessEpoch(
// TODO: optimize by combining the two loops
// likely will require splitting into phase0 and post-phase0 versions

if (forkSeq >= ForkSeq.electra) {
isCompoundingValidatorArr.length = validatorCount;
}

// Clone before being mutated in processEffectiveBalanceUpdates
epochCtx.beforeEpochTransition();

Expand Down Expand Up @@ -298,6 +305,10 @@ export function beforeProcessEpoch(

flags[i] = flag;

if (forkSeq >= ForkSeq.electra) {
isCompoundingValidatorArr[i] = hasCompoundingWithdrawalCredential(validator.withdrawalCredentials);
}

if (isActiveCurr) {
totalActiveStakeByIncrement += effectiveBalancesByIncrements[i];
} else {
Expand Down Expand Up @@ -511,7 +522,7 @@ export function beforeProcessEpoch(
proposerIndices,
inclusionDelays,
flags,
validators,
isCompoundingValidatorArr,
// Will be assigned in processRewardsAndPenalties()
balances: undefined,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ import {
HYSTERESIS_QUOTIENT,
HYSTERESIS_UPWARD_MULTIPLIER,
MAX_EFFECTIVE_BALANCE,
MAX_EFFECTIVE_BALANCE_ELECTRA,
MIN_ACTIVATION_BALANCE,
TIMELY_TARGET_FLAG_INDEX,
} from "@lodestar/params";
import {BeaconStateAltair, CachedBeaconStateAllForks, EpochTransitionCache} from "../types.js";
import {getMaxEffectiveBalance} from "../util/validator.js";

/** Same to https://github.com/ethereum/eth2.0-specs/blob/v1.1.0-alpha.5/specs/altair/beacon-chain.md#has_flag */
const TIMELY_TARGET = 1 << TIMELY_TARGET_FLAG_INDEX;
Expand Down Expand Up @@ -43,7 +44,7 @@ export function processEffectiveBalanceUpdates(
// and updated in processPendingDeposits() and processPendingConsolidations()
// so it's recycled here for performance.
const balances = cache.balances ?? state.balances.getAll();
const currentEpochValidators = cache.validators;
const isCompoundingValidatorArr = cache.isCompoundingValidatorArr;

let numUpdate = 0;
for (let i = 0, len = balances.length; i < len; i++) {
Expand All @@ -58,7 +59,7 @@ export function processEffectiveBalanceUpdates(
effectiveBalanceLimit = MAX_EFFECTIVE_BALANCE;
} else {
// from electra, effectiveBalanceLimit is per validator
effectiveBalanceLimit = getMaxEffectiveBalance(currentEpochValidators[i].withdrawalCredentials);
effectiveBalanceLimit = isCompoundingValidatorArr[i] ? MAX_EFFECTIVE_BALANCE_ELECTRA : MIN_ACTIVATION_BALANCE;
}

if (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {PendingDeposit} from "@lodestar/types/lib/electra/types.js";
import {addValidatorToRegistry, isValidDepositSignature} from "../block/processDeposit.js";
import {CachedBeaconStateElectra, EpochTransitionCache} from "../types.js";
import {increaseBalance} from "../util/balance.js";
import {hasCompoundingWithdrawalCredential} from "../util/electra.js";
import {computeStartSlotAtEpoch} from "../util/epoch.js";
import {getActivationExitChurnLimit} from "../util/validator.js";

Expand Down Expand Up @@ -106,6 +107,8 @@ function applyPendingDeposit(
// Verify the deposit signature (proof of possession) which is not checked by the deposit contract
if (isValidDepositSignature(state.config, pubkey, withdrawalCredentials, amount, signature)) {
addValidatorToRegistry(ForkSeq.electra, state, pubkey, withdrawalCredentials, amount);
const newValidatorIndex = state.validators.length - 1;
cache.isCompoundingValidatorArr[newValidatorIndex] = hasCompoundingWithdrawalCredential(withdrawalCredentials);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the main fix

}
} else {
// Increase balance
Expand Down
Loading