From 808b37bdfd3b744d1f0313ef22dbb754f2799aea Mon Sep 17 00:00:00 2001 From: NC <17676176+ensi321@users.noreply.github.com> Date: Wed, 2 Oct 2024 12:35:56 -0700 Subject: [PATCH 1/7] New compound switching logic --- .../src/block/processConsolidationRequest.ts | 85 ++++++++++++++++--- .../src/block/processDeposit.ts | 14 +-- .../src/cache/epochTransitionCache.ts | 3 +- .../src/epoch/processPendingConsolidations.ts | 7 -- packages/state-transition/src/util/electra.ts | 16 ++-- 5 files changed, 89 insertions(+), 36 deletions(-) diff --git a/packages/state-transition/src/block/processConsolidationRequest.ts b/packages/state-transition/src/block/processConsolidationRequest.ts index 691ecd5eca0b..177797132c58 100644 --- a/packages/state-transition/src/block/processConsolidationRequest.ts +++ b/packages/state-transition/src/block/processConsolidationRequest.ts @@ -1,33 +1,41 @@ import {electra, ssz} from "@lodestar/types"; import {FAR_FUTURE_EPOCH, MIN_ACTIVATION_BALANCE, PENDING_CONSOLIDATIONS_LIMIT} from "@lodestar/params"; +import {toHex} from "@lodestar/utils"; import {CachedBeaconStateElectra} from "../types.js"; import {getConsolidationChurnLimit, isActiveValidator} from "../util/validator.js"; -import {hasExecutionWithdrawalCredential} from "../util/electra.js"; +import {hasExecutionWithdrawalCredential, switchToCompoundingValidator} from "../util/electra.js"; import {computeConsolidationEpochAndUpdateChurn} from "../util/epoch.js"; +import {hasEth1WithdrawalCredential} from "../util/capella.js"; +// TODO Electra: Clean up necessary as there is a lot of overlap with isValidSwitchToCompoundRequest export function processConsolidationRequest( state: CachedBeaconStateElectra, consolidationRequest: electra.ConsolidationRequest ): void { - // If the pending consolidations queue is full, consolidation requests are ignored - if (state.pendingConsolidations.length >= PENDING_CONSOLIDATIONS_LIMIT) { + const {sourcePubkey, targetPubkey, sourceAddress} = consolidationRequest; + const sourceIndex = state.epochCtx.getValidatorIndex(sourcePubkey); + const targetIndex = state.epochCtx.getValidatorIndex(targetPubkey); + + if (sourceIndex === null || targetIndex === null) { return; } - // If there is too little available consolidation churn limit, consolidation requests are ignored - if (getConsolidationChurnLimit(state.epochCtx) <= MIN_ACTIVATION_BALANCE) { + if (isValidSwitchToCompoundRequest(state, consolidationRequest)) { + switchToCompoundingValidator(state, sourceIndex); + // Early return since we have already switched validator to compounding return; } - const {sourcePubkey, targetPubkey} = consolidationRequest; - const sourceIndex = state.epochCtx.getValidatorIndex(sourcePubkey); - const targetIndex = state.epochCtx.getValidatorIndex(targetPubkey); - - if (sourceIndex === null || targetIndex === null) { + // If the pending consolidations queue is full, consolidation requests are ignored + if (state.pendingConsolidations.length >= PENDING_CONSOLIDATIONS_LIMIT) { return; } + // If there is too little available consolidation churn limit, consolidation requests are ignored + if (getConsolidationChurnLimit(state.epochCtx) <= MIN_ACTIVATION_BALANCE) { + return; + } // Verify that source != target, so a consolidation cannot be used as an exit. if (sourceIndex === targetIndex) { return; @@ -35,8 +43,9 @@ export function processConsolidationRequest( const sourceValidator = state.validators.get(sourceIndex); const targetValidator = state.validators.getReadonly(targetIndex); - const sourceWithdrawalAddress = sourceValidator.withdrawalCredentials.subarray(12); const currentEpoch = state.epochCtx.epoch; + const sourceWithdrawalAddressStr = toHex(sourceValidator.withdrawalCredentials.subarray(12)); + const sourceAddressStr = toHex(sourceAddress); // Verify withdrawal credentials if ( @@ -46,7 +55,7 @@ export function processConsolidationRequest( return; } - if (Buffer.compare(sourceWithdrawalAddress, consolidationRequest.sourceAddress) !== 0) { + if (sourceWithdrawalAddressStr !== sourceAddressStr) { return; } @@ -70,4 +79,56 @@ export function processConsolidationRequest( targetIndex, }); state.pendingConsolidations.push(pendingConsolidation); + + // Churn any target excess active balance of target and raise its max + if (hasEth1WithdrawalCredential(targetValidator.withdrawalCredentials)) { + switchToCompoundingValidator(state, targetIndex); + } +} + +/** + * Determine if we should set consolidation target validator to compounding credential + */ +function isValidSwitchToCompoundRequest( + state: CachedBeaconStateElectra, + consolidationRequest: electra.ConsolidationRequest +): boolean { + const {sourcePubkey, targetPubkey, sourceAddress} = consolidationRequest; + const sourceIndex = state.epochCtx.getValidatorIndex(sourcePubkey); + const targetIndex = state.epochCtx.getValidatorIndex(targetPubkey); + + // Verify pubkey exists + if (sourceIndex === null) { + return false; + } + + // Switch to compounding requires source and target be equal + if (sourceIndex !== targetIndex) { + return false; + } + + const sourceValidator = state.validators.getReadonly(sourceIndex); + const addressStr = toHex(sourceValidator.withdrawalCredentials.subarray(12)); + const sourceAddressStr = toHex(sourceAddress); + // Verify request has been authorized + if (addressStr !== sourceAddressStr) { + return false; + } + + // Verify source withdrawal credentials + if (!hasEth1WithdrawalCredential(sourceValidator.withdrawalCredentials)) { + return false; + } + + // Verify the source is active + if (!isActiveValidator(sourceValidator, state.epochCtx.epoch)) { + return false; + } + + // Verify exit for source has not been initiated + if (sourceValidator.exitEpoch !== FAR_FUTURE_EPOCH) { + return false; + } + + return true; } diff --git a/packages/state-transition/src/block/processDeposit.ts b/packages/state-transition/src/block/processDeposit.ts index ee75dff0dfd1..1e0f420d6f60 100644 --- a/packages/state-transition/src/block/processDeposit.ts +++ b/packages/state-transition/src/block/processDeposit.ts @@ -81,13 +81,13 @@ export function applyDeposit( }); stateElectra.pendingBalanceDeposits.push(pendingBalanceDeposit); - if ( - hasCompoundingWithdrawalCredential(withdrawalCredentials) && - hasEth1WithdrawalCredential(validators.getReadonly(cachedIndex).withdrawalCredentials) && - isValidDepositSignature(config, pubkey, withdrawalCredentials, amount, deposit.signature) - ) { - switchToCompoundingValidator(stateElectra, cachedIndex); - } + // if ( + // hasCompoundingWithdrawalCredential(withdrawalCredentials) && + // hasEth1WithdrawalCredential(validators.getReadonly(cachedIndex).withdrawalCredentials) && + // isValidDepositSignature(config, pubkey, withdrawalCredentials, amount, deposit.signature) + // ) { + // switchToCompoundingValidator(stateElectra, cachedIndex); + // } } } } diff --git a/packages/state-transition/src/cache/epochTransitionCache.ts b/packages/state-transition/src/cache/epochTransitionCache.ts index 27b781e8a6a1..1f52e2170d02 100644 --- a/packages/state-transition/src/cache/epochTransitionCache.ts +++ b/packages/state-transition/src/cache/epochTransitionCache.ts @@ -141,7 +141,8 @@ export interface EpochTransitionCache { /** * This is for electra only - * Validators that're switched to compounding during processPendingConsolidations(), not available in beforeProcessEpoch() + * Previously this was used to improve performance in processEffectiveBalanceUpdates() to keep track of validators + * switched to compounding during epoch processing. Switching is now moved to block processing. */ newCompoundingValidators?: Set; diff --git a/packages/state-transition/src/epoch/processPendingConsolidations.ts b/packages/state-transition/src/epoch/processPendingConsolidations.ts index 28178a509bba..0c0d61fd78a9 100644 --- a/packages/state-transition/src/epoch/processPendingConsolidations.ts +++ b/packages/state-transition/src/epoch/processPendingConsolidations.ts @@ -1,8 +1,6 @@ -import {ValidatorIndex} from "@lodestar/types"; import {CachedBeaconStateElectra, EpochTransitionCache} from "../types.js"; import {decreaseBalance, increaseBalance} from "../util/balance.js"; import {getActiveBalance} from "../util/validator.js"; -import {switchToCompoundingValidator} from "../util/electra.js"; /** * Starting from Electra: @@ -22,7 +20,6 @@ export function processPendingConsolidations(state: CachedBeaconStateElectra, ca let nextPendingConsolidation = 0; const validators = state.validators; const cachedBalances = cache.balances; - const newCompoundingValidators = new Set(); for (const pendingConsolidation of state.pendingConsolidations.getAllReadonly()) { const {sourceIndex, targetIndex} = pendingConsolidation; @@ -36,9 +33,6 @@ export function processPendingConsolidations(state: CachedBeaconStateElectra, ca if (sourceValidator.withdrawableEpoch > nextEpoch) { break; } - // Churn any target excess active balance of target and raise its max - switchToCompoundingValidator(state, targetIndex); - newCompoundingValidators.add(targetIndex); // Move active balance to target. Excess balance is withdrawable. const activeBalance = getActiveBalance(state, sourceIndex); decreaseBalance(state, sourceIndex, activeBalance); @@ -51,6 +45,5 @@ export function processPendingConsolidations(state: CachedBeaconStateElectra, ca nextPendingConsolidation++; } - cache.newCompoundingValidators = newCompoundingValidators; state.pendingConsolidations = state.pendingConsolidations.sliceFrom(nextPendingConsolidation); } diff --git a/packages/state-transition/src/util/electra.ts b/packages/state-transition/src/util/electra.ts index ac34da6407de..ffa09a96bd3e 100644 --- a/packages/state-transition/src/util/electra.ts +++ b/packages/state-transition/src/util/electra.ts @@ -16,15 +16,13 @@ export function hasExecutionWithdrawalCredential(withdrawalCredentials: Uint8Arr export function switchToCompoundingValidator(state: CachedBeaconStateElectra, index: ValidatorIndex): void { const validator = state.validators.get(index); - if (hasEth1WithdrawalCredential(validator.withdrawalCredentials)) { - // directly modifying the byte leads to ssz missing the modification resulting into - // wrong root compute, although slicing can be avoided but anyway this is not going - // to be a hot path so its better to clean slice and avoid side effects - const newWithdrawalCredentials = validator.withdrawalCredentials.slice(); - newWithdrawalCredentials[0] = COMPOUNDING_WITHDRAWAL_PREFIX; - validator.withdrawalCredentials = newWithdrawalCredentials; - queueExcessActiveBalance(state, index); - } + // directly modifying the byte leads to ssz missing the modification resulting into + // wrong root compute, although slicing can be avoided but anyway this is not going + // to be a hot path so its better to clean slice and avoid side effects + const newWithdrawalCredentials = validator.withdrawalCredentials.slice(); + newWithdrawalCredentials[0] = COMPOUNDING_WITHDRAWAL_PREFIX; + validator.withdrawalCredentials = newWithdrawalCredentials; + queueExcessActiveBalance(state, index); } export function queueExcessActiveBalance(state: CachedBeaconStateElectra, index: ValidatorIndex): void { From 9673c6c5428357af9fcdcca4099e027deb3f07bb Mon Sep 17 00:00:00 2001 From: NC <17676176+ensi321@users.noreply.github.com> Date: Wed, 2 Oct 2024 12:51:10 -0700 Subject: [PATCH 2/7] clean up --- .../src/block/processDeposit.ts | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/packages/state-transition/src/block/processDeposit.ts b/packages/state-transition/src/block/processDeposit.ts index 1e0f420d6f60..44a31ea4b88c 100644 --- a/packages/state-transition/src/block/processDeposit.ts +++ b/packages/state-transition/src/block/processDeposit.ts @@ -15,14 +15,7 @@ import {DepositData} from "@lodestar/types/lib/phase0/types.js"; import {DepositRequest} from "@lodestar/types/lib/electra/types.js"; import {BeaconConfig} from "@lodestar/config"; import {ZERO_HASH} from "../constants/index.js"; -import { - computeDomain, - computeSigningRoot, - hasCompoundingWithdrawalCredential, - hasEth1WithdrawalCredential, - increaseBalance, - switchToCompoundingValidator, -} from "../util/index.js"; +import {computeDomain, computeSigningRoot, increaseBalance} from "../util/index.js"; import {CachedBeaconStateAllForks, CachedBeaconStateAltair, CachedBeaconStateElectra} from "../types.js"; /** @@ -80,14 +73,6 @@ export function applyDeposit( amount: BigInt(amount), }); stateElectra.pendingBalanceDeposits.push(pendingBalanceDeposit); - - // if ( - // hasCompoundingWithdrawalCredential(withdrawalCredentials) && - // hasEth1WithdrawalCredential(validators.getReadonly(cachedIndex).withdrawalCredentials) && - // isValidDepositSignature(config, pubkey, withdrawalCredentials, amount, deposit.signature) - // ) { - // switchToCompoundingValidator(stateElectra, cachedIndex); - // } } } } From 65f06af1077d1d7e63198c7e0c636b50876c91c6 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Thu, 3 Oct 2024 11:02:25 +0700 Subject: [PATCH 3/7] fix: remove newCompoundingValidators in EpochTransitionCache --- .../state-transition/src/cache/epochTransitionCache.ts | 9 --------- .../src/epoch/processEffectiveBalanceUpdates.ts | 7 +++---- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/packages/state-transition/src/cache/epochTransitionCache.ts b/packages/state-transition/src/cache/epochTransitionCache.ts index 1f52e2170d02..2a35317fbc93 100644 --- a/packages/state-transition/src/cache/epochTransitionCache.ts +++ b/packages/state-transition/src/cache/epochTransitionCache.ts @@ -139,13 +139,6 @@ export interface EpochTransitionCache { */ validators: phase0.Validator[]; - /** - * This is for electra only - * Previously this was used to improve performance in processEffectiveBalanceUpdates() to keep track of validators - * switched to compounding during epoch processing. Switching is now moved to block processing. - */ - newCompoundingValidators?: Set; - /** * balances array will be populated by processRewardsAndPenalties() and consumed by processEffectiveBalanceUpdates(). * processRewardsAndPenalties() already has a regular Javascript array of balances. @@ -519,8 +512,6 @@ export function beforeProcessEpoch( inclusionDelays, flags, validators, - // will be assigned in processPendingConsolidations() - newCompoundingValidators: undefined, // Will be assigned in processRewardsAndPenalties() balances: undefined, }; diff --git a/packages/state-transition/src/epoch/processEffectiveBalanceUpdates.ts b/packages/state-transition/src/epoch/processEffectiveBalanceUpdates.ts index 0ea4b49dddf4..fdaea2f83163 100644 --- a/packages/state-transition/src/epoch/processEffectiveBalanceUpdates.ts +++ b/packages/state-transition/src/epoch/processEffectiveBalanceUpdates.ts @@ -46,7 +46,6 @@ export function processEffectiveBalanceUpdates( // so it's recycled here for performance. const balances = cache.balances ?? state.balances.getAll(); const currentEpochValidators = cache.validators; - const newCompoundingValidators = cache.newCompoundingValidators ?? new Set(); let numUpdate = 0; for (let i = 0, len = balances.length; i < len; i++) { @@ -61,9 +60,9 @@ export function processEffectiveBalanceUpdates( effectiveBalanceLimit = MAX_EFFECTIVE_BALANCE; } else { // from electra, effectiveBalanceLimit is per validator - const isCompoundingValidator = - hasCompoundingWithdrawalCredential(currentEpochValidators[i].withdrawalCredentials) || - newCompoundingValidators.has(i); + const isCompoundingValidator = hasCompoundingWithdrawalCredential( + currentEpochValidators[i].withdrawalCredentials + ); effectiveBalanceLimit = isCompoundingValidator ? MAX_EFFECTIVE_BALANCE_ELECTRA : MIN_ACTIVATION_BALANCE; } From 37b1b7bee0e881ec602dc7b39a081f41164b485f Mon Sep 17 00:00:00 2001 From: NC <17676176+ensi321@users.noreply.github.com> Date: Mon, 7 Oct 2024 09:15:53 -0700 Subject: [PATCH 4/7] Bump spec version --- packages/beacon-node/test/spec/specTestVersioning.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/beacon-node/test/spec/specTestVersioning.ts b/packages/beacon-node/test/spec/specTestVersioning.ts index d4bc192f6cad..bef42fd1cdfa 100644 --- a/packages/beacon-node/test/spec/specTestVersioning.ts +++ b/packages/beacon-node/test/spec/specTestVersioning.ts @@ -15,7 +15,7 @@ import {DownloadTestsOptions} from "@lodestar/spec-test-util/downloadTests"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); export const ethereumConsensusSpecsTests: DownloadTestsOptions = { - specVersion: "v1.5.0-alpha.6", + specVersion: "v1.5.0-alpha.7", // Target directory is the host package root: 'packages/*/spec-tests' outputDir: path.join(__dirname, "../../spec-tests"), specTestsRepoUrl: "https://github.com/ethereum/consensus-spec-tests", From 7e0957e1e6e94cbc14af4c88517fe1d62d92d956 Mon Sep 17 00:00:00 2001 From: NC <17676176+ensi321@users.noreply.github.com> Date: Thu, 10 Oct 2024 15:53:49 -0700 Subject: [PATCH 5/7] Address comment --- .../src/block/processConsolidationRequest.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/state-transition/src/block/processConsolidationRequest.ts b/packages/state-transition/src/block/processConsolidationRequest.ts index 177797132c58..3b58a614e804 100644 --- a/packages/state-transition/src/block/processConsolidationRequest.ts +++ b/packages/state-transition/src/block/processConsolidationRequest.ts @@ -43,9 +43,8 @@ export function processConsolidationRequest( const sourceValidator = state.validators.get(sourceIndex); const targetValidator = state.validators.getReadonly(targetIndex); + const sourceWithdrawalAddress = sourceValidator.withdrawalCredentials.subarray(12); const currentEpoch = state.epochCtx.epoch; - const sourceWithdrawalAddressStr = toHex(sourceValidator.withdrawalCredentials.subarray(12)); - const sourceAddressStr = toHex(sourceAddress); // Verify withdrawal credentials if ( @@ -55,7 +54,7 @@ export function processConsolidationRequest( return; } - if (sourceWithdrawalAddressStr !== sourceAddressStr) { + if (Buffer.compare(sourceWithdrawalAddress, sourceAddress) !== 0) { return; } From 522e8c9931e42d610bd256d8f0aed00ab2d5f29d Mon Sep 17 00:00:00 2001 From: NC <17676176+ensi321@users.noreply.github.com> Date: Thu, 10 Oct 2024 17:12:23 -0700 Subject: [PATCH 6/7] address comment --- .../src/block/processConsolidationRequest.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/state-transition/src/block/processConsolidationRequest.ts b/packages/state-transition/src/block/processConsolidationRequest.ts index 3b58a614e804..c14612579c58 100644 --- a/packages/state-transition/src/block/processConsolidationRequest.ts +++ b/packages/state-transition/src/block/processConsolidationRequest.ts @@ -1,7 +1,6 @@ import {electra, ssz} from "@lodestar/types"; import {FAR_FUTURE_EPOCH, MIN_ACTIVATION_BALANCE, PENDING_CONSOLIDATIONS_LIMIT} from "@lodestar/params"; -import {toHex} from "@lodestar/utils"; import {CachedBeaconStateElectra} from "../types.js"; import {getConsolidationChurnLimit, isActiveValidator} from "../util/validator.js"; import {hasExecutionWithdrawalCredential, switchToCompoundingValidator} from "../util/electra.js"; @@ -107,10 +106,9 @@ function isValidSwitchToCompoundRequest( } const sourceValidator = state.validators.getReadonly(sourceIndex); - const addressStr = toHex(sourceValidator.withdrawalCredentials.subarray(12)); - const sourceAddressStr = toHex(sourceAddress); + const sourceWithdrawalAddress = sourceValidator.withdrawalCredentials.subarray(12); // Verify request has been authorized - if (addressStr !== sourceAddressStr) { + if (Buffer.compare(sourceWithdrawalAddress, sourceAddress) !== 0) { return false; } From 12a5914f56f1b5269be5415dc49ba322385aa031 Mon Sep 17 00:00:00 2001 From: NC <17676176+ensi321@users.noreply.github.com> Date: Fri, 11 Oct 2024 13:01:11 -0700 Subject: [PATCH 7/7] Lint --- packages/state-transition/src/block/processDeposit.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/state-transition/src/block/processDeposit.ts b/packages/state-transition/src/block/processDeposit.ts index 00fe33cbd328..f900cdc39bad 100644 --- a/packages/state-transition/src/block/processDeposit.ts +++ b/packages/state-transition/src/block/processDeposit.ts @@ -68,7 +68,8 @@ export function applyDeposit( } } else { // increase balance by deposit amount right away pre-electra - increaseBalance(state, cachedIndex, amount); } + increaseBalance(state, cachedIndex, amount); + } } else { const stateElectra = state as CachedBeaconStateElectra; const pendingDeposit = ssz.electra.PendingDeposit.toViewDU({