From c3b42a4554541cf7dbb1c6fb96d52bb7039447c6 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Fri, 6 Dec 2024 10:39:44 +0700 Subject: [PATCH 1/2] fix: check pubkey or validator index known to a state --- .../src/block/processConsolidationRequest.ts | 9 +++++++-- .../src/epoch/processPendingDeposits.ts | 6 +++--- packages/state-transition/src/util/electra.ts | 17 +++++++++++++++++ 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/packages/state-transition/src/block/processConsolidationRequest.ts b/packages/state-transition/src/block/processConsolidationRequest.ts index d0650135d0c6..1db5a6f9a59a 100644 --- a/packages/state-transition/src/block/processConsolidationRequest.ts +++ b/packages/state-transition/src/block/processConsolidationRequest.ts @@ -3,7 +3,7 @@ import {electra, ssz} from "@lodestar/types"; import {CachedBeaconStateElectra} from "../types.js"; import {hasEth1WithdrawalCredential} from "../util/capella.js"; -import {hasExecutionWithdrawalCredential, switchToCompoundingValidator} from "../util/electra.js"; +import {hasExecutionWithdrawalCredential, isPubkeyKnown, switchToCompoundingValidator} from "../util/electra.js"; import {computeConsolidationEpochAndUpdateChurn} from "../util/epoch.js"; import {getConsolidationChurnLimit, isActiveValidator} from "../util/validator.js"; @@ -13,6 +13,10 @@ export function processConsolidationRequest( consolidationRequest: electra.ConsolidationRequest ): void { const {sourcePubkey, targetPubkey, sourceAddress} = consolidationRequest; + if (!isPubkeyKnown(state, sourcePubkey) || !isPubkeyKnown(state, targetPubkey)) { + return; + } + const sourceIndex = state.epochCtx.getValidatorIndex(sourcePubkey); const targetIndex = state.epochCtx.getValidatorIndex(targetPubkey); @@ -95,8 +99,9 @@ function isValidSwitchToCompoundRequest( const sourceIndex = state.epochCtx.getValidatorIndex(sourcePubkey); const targetIndex = state.epochCtx.getValidatorIndex(targetPubkey); + // since we share pubkey2index, validatorIndex maybe known by other epoch transition but we don't have that validator in this state // Verify pubkey exists - if (sourceIndex === null) { + if (sourceIndex === null || sourceIndex >= state.validators.length) { return false; } diff --git a/packages/state-transition/src/epoch/processPendingDeposits.ts b/packages/state-transition/src/epoch/processPendingDeposits.ts index d925aa1cc741..441d5601a380 100644 --- a/packages/state-transition/src/epoch/processPendingDeposits.ts +++ b/packages/state-transition/src/epoch/processPendingDeposits.ts @@ -3,7 +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 {hasCompoundingWithdrawalCredential, isValidatorKnown} from "../util/electra.js"; import {computeStartSlotAtEpoch} from "../util/epoch.js"; import {getActivationExitChurnLimit} from "../util/validator.js"; @@ -51,7 +51,7 @@ export function processPendingDeposits(state: CachedBeaconStateElectra, cache: E let isValidatorWithdrawn = false; const validatorIndex = state.epochCtx.getValidatorIndex(deposit.pubkey); - if (validatorIndex !== null) { + if (isValidatorKnown(state, validatorIndex)) { const validator = state.validators.getReadonly(validatorIndex); isValidatorExited = validator.exitEpoch < FAR_FUTURE_EPOCH; isValidatorWithdrawn = validator.withdrawableEpoch < nextEpoch; @@ -103,7 +103,7 @@ function applyPendingDeposit( const {pubkey, withdrawalCredentials, amount, signature} = deposit; const cachedBalances = cache.balances; - if (validatorIndex === null) { + if (!isValidatorKnown(state, validatorIndex)) { // 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); diff --git a/packages/state-transition/src/util/electra.ts b/packages/state-transition/src/util/electra.ts index f5b899eadcab..a9736dc8161e 100644 --- a/packages/state-transition/src/util/electra.ts +++ b/packages/state-transition/src/util/electra.ts @@ -45,3 +45,20 @@ export function queueExcessActiveBalance(state: CachedBeaconStateElectra, index: state.pendingDeposits.push(pendingDeposit); } } + +/** + * Since we share pubkey2index, pubkey maybe added by other epoch transition but we don't have that validator in this state + */ +export function isPubkeyKnown(state: CachedBeaconStateElectra, pubkey: Uint8Array): boolean { + return isValidatorKnown(state, state.epochCtx.getValidatorIndex(pubkey)); +} + +/** + * Since we share pubkey2index, validatorIndex maybe not null but we don't have that validator in this state + */ +export function isValidatorKnown( + state: CachedBeaconStateElectra, + index: ValidatorIndex | null +): index is ValidatorIndex { + return index !== null && index < state.validators.length; +} From e91c9ec89471ab64e742dea0002b7cb459ce5d91 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Fri, 6 Dec 2024 10:54:35 +0700 Subject: [PATCH 2/2] chore: add more comments --- .../state-transition/src/block/processConsolidationRequest.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/state-transition/src/block/processConsolidationRequest.ts b/packages/state-transition/src/block/processConsolidationRequest.ts index 1db5a6f9a59a..1a1d83eee0be 100644 --- a/packages/state-transition/src/block/processConsolidationRequest.ts +++ b/packages/state-transition/src/block/processConsolidationRequest.ts @@ -99,9 +99,9 @@ function isValidSwitchToCompoundRequest( const sourceIndex = state.epochCtx.getValidatorIndex(sourcePubkey); const targetIndex = state.epochCtx.getValidatorIndex(targetPubkey); - // since we share pubkey2index, validatorIndex maybe known by other epoch transition but we don't have that validator in this state // Verify pubkey exists - if (sourceIndex === null || sourceIndex >= state.validators.length) { + if (sourceIndex === null) { + // this check is mainly to make the compiler happy, pubkey is checked by the consumer already return false; }