Check pending deposits before applying builder deposits

beacon-chain/core/gloas/deposit_request.go

@@ -68,37 +68,27 @@ func processDepositRequests(ctx context.Context, beaconState state.BeaconState,
 //	    )
 //	</spec>
 func processDepositRequest(beaconState state.BeaconState, request *enginev1.DepositRequest) error {
-	var err error
-	defer func() {
-		if err == nil {
-			builderDepositsProcessedTotal.Inc()
-		}
-	}()
-
 	if request == nil {
-		err = errors.New("nil deposit request")
-		return err
+		return errors.New("nil deposit request")
 	}
 
-	var applied bool
-	applied, err = applyBuilderDepositRequest(beaconState, request)
+	applied, err := applyBuilderDepositRequest(beaconState, request)
 	if err != nil {
-		err = errors.Wrap(err, "could not apply builder deposit")
-		return err
+		return errors.Wrap(err, "could not apply builder deposit")
 	}
 	if applied {
+		builderDepositsProcessedTotal.Inc()
 		return nil
 	}
 
-	if err = beaconState.AppendPendingDeposit(&ethpb.PendingDeposit{
+	if err := beaconState.AppendPendingDeposit(&ethpb.PendingDeposit{
 		PublicKey:             request.Pubkey,
 		WithdrawalCredentials: request.WithdrawalCredentials,
 		Amount:                request.Amount,
 		Signature:             request.Signature,
 		Slot:                  beaconState.Slot(),
 	}); err != nil {
-		err = errors.Wrap(err, "could not append deposit request")
-		return err
+		return errors.Wrap(err, "could not append deposit request")
 	}
 	return nil
 }
@@ -132,20 +122,28 @@ func applyBuilderDepositRequest(beaconState state.BeaconState, request *enginev1
 	}
 
 	pubkey := bytesutil.ToBytes48(request.Pubkey)
-	_, isValidator := beaconState.ValidatorIndexByPubkey(pubkey)
 	idx, isBuilder := beaconState.BuilderIndexByPubkey(pubkey)
-	isBuilderPrefix := helpers.IsBuilderWithdrawalCredential(request.WithdrawalCredentials)
-	if !isBuilder && (!isBuilderPrefix || isValidator) {
-		return false, nil
-	}
-
 	if isBuilder {
 		if err := beaconState.IncreaseBuilderBalance(idx, request.Amount); err != nil {
 			return false, err
 		}
 		return true, nil
 	}
 
+	isBuilderPrefix := helpers.IsBuilderWithdrawalCredential(request.WithdrawalCredentials)
+	_, isValidator := beaconState.ValidatorIndexByPubkey(pubkey)
+	if !isBuilderPrefix || isValidator {
+		return false, nil
+	}
+
+	isPending, err := beaconState.IsPendingValidator(request.Pubkey)
+	if err != nil {
+		return false, err
+	}
+	if isPending {
+		return false, nil
+	}
+
 	if err := applyDepositForNewBuilder(
 		beaconState,
 		request.Pubkey,

beacon-chain/core/gloas/deposit_request_test.go

@@ -91,6 +91,33 @@ func TestProcessDepositRequest_ExistingBuilderIncreasesBalance(t *testing.T) {
 	require.Equal(t, 0, len(pending))
 }
 
+func TestProcessDepositRequest_BuilderDepositWithExistingPendingDepositStaysPending(t *testing.T) {
+	sk, err := bls.RandKey()
+	require.NoError(t, err)
+
+	validatorCred := validatorWithdrawalCredentials()
+	builderCred := builderWithdrawalCredentials()
+	existingPending := stateTesting.GeneratePendingDeposit(t, sk, 1234, validatorCred, 0)
+	req := depositRequestFromPending(stateTesting.GeneratePendingDeposit(t, sk, 200, builderCred, 1), 9)
+
+	st := newGloasState(t, nil, nil)
+	require.NoError(t, st.SetPendingDeposits([]*ethpb.PendingDeposit{existingPending}))
+
+	err = processDepositRequest(st, req)
+	require.NoError(t, err)
+
+	_, ok := st.BuilderIndexByPubkey(toBytes48(req.Pubkey))
+	require.Equal(t, false, ok)
+
+	pending, err := st.PendingDeposits()
+	require.NoError(t, err)
+	require.Equal(t, 2, len(pending))
+	require.DeepEqual(t, existingPending.PublicKey, pending[0].PublicKey)
+	require.DeepEqual(t, req.Pubkey, pending[1].PublicKey)
+	require.DeepEqual(t, req.WithdrawalCredentials, pending[1].WithdrawalCredentials)
+	require.Equal(t, req.Amount, pending[1].Amount)
+}
+
 func TestApplyDepositForBuilder_InvalidSignatureIgnoresDeposit(t *testing.T) {
 	sk, err := bls.RandKey()
 	require.NoError(t, err)

beacon-chain/state/interfaces.go

@@ -238,6 +238,7 @@ type ReadOnlyDeposits interface {
 	DepositBalanceToConsume() (primitives.Gwei, error)
 	DepositRequestsStartIndex() (uint64, error)
 	PendingDeposits() ([]*ethpb.PendingDeposit, error)
+	IsPendingValidator(pubkey []byte) (bool, error)
 }
 
 type ReadOnlyConsolidations interface {

beacon-chain/state/state-native/getters_deposits.go

@@ -1,6 +1,10 @@
 package state_native
 
 import (
+	"bytes"
+	"fmt"
+
+	"github.com/OffchainLabs/prysm/v7/beacon-chain/core/helpers"
 	"github.com/OffchainLabs/prysm/v7/consensus-types/primitives"
 	ethpb "github.com/OffchainLabs/prysm/v7/proto/prysm/v1alpha1"
 	"github.com/OffchainLabs/prysm/v7/runtime/version"
@@ -30,6 +34,59 @@ func (b *BeaconState) PendingDeposits() ([]*ethpb.PendingDeposit, error) {
 	return b.pendingDepositsVal(), nil
 }
 
+// IsPendingValidator checks whether a pending deposit with a valid signature exists for the
+// given pubkey. This method requires access to the RLock on the state and only applies in
+// electra or later.
+//
+// <spec fn="is_pending_validator" fork="gloas" hash="f3f06b56">
+// def is_pending_validator(state: BeaconState, pubkey: BLSPubkey) -> bool:
+//
+//	"""
+//	Check if a pending deposit with a valid signature is in the queue for the given pubkey.
+//	"""
+//	for pending_deposit in state.pending_deposits:
+//	    if pending_deposit.pubkey != pubkey:
+//	        continue
+//	    if is_valid_deposit_signature(
+//	        pending_deposit.pubkey,
+//	        pending_deposit.withdrawal_credentials,
+//	        pending_deposit.amount,
+//	        pending_deposit.signature,
+//	    ):
+//	        return True
+//	return False
+//
+// </spec>
+func (b *BeaconState) IsPendingValidator(pubkey []byte) (bool, error) {
+	if b.version < version.Electra {
+		return false, errNotSupported("IsPendingValidator", b.version)
+	}
+	b.lock.RLock()
+	defer b.lock.RUnlock()
+	for _, deposit := range b.pendingDeposits {
+		if deposit == nil {
+			continue
+		}
+		if !bytes.Equal(deposit.PublicKey, pubkey) {
+			continue
+		}
+		valid, err := helpers.IsValidDepositSignature(&ethpb.Deposit_Data{
+			PublicKey:             deposit.PublicKey,
+			WithdrawalCredentials: deposit.WithdrawalCredentials,
+			Amount:                deposit.Amount,
+			Signature:             deposit.Signature,
+		})
+		if err != nil {
+			log.WithField("pubkey", fmt.Sprintf("%x", deposit.PublicKey)).WithError(err).Warn("Could not verify pending deposit signature")
+			continue
+		}
+		if valid {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
 func (b *BeaconState) pendingDepositsVal() []*ethpb.PendingDeposit {
 	if b.pendingDeposits == nil {
 		return nil

beacon-chain/state/state-native/getters_deposits_test.go

@@ -4,7 +4,9 @@ import (
 	"testing"
 
 	state_native "github.com/OffchainLabs/prysm/v7/beacon-chain/state/state-native"
+	stateTesting "github.com/OffchainLabs/prysm/v7/beacon-chain/state/testing"
 	"github.com/OffchainLabs/prysm/v7/consensus-types/primitives"
+	"github.com/OffchainLabs/prysm/v7/crypto/bls"
 	eth "github.com/OffchainLabs/prysm/v7/proto/prysm/v1alpha1"
 	"github.com/OffchainLabs/prysm/v7/testing/require"
 )
@@ -66,3 +68,66 @@ func TestPendingDeposits(t *testing.T) {
 	_, err = s.DepositBalanceToConsume()
 	require.ErrorContains(t, "not supported", err)
 }
+
+func TestIsPendingValidator(t *testing.T) {
+	sk, err := bls.RandKey()
+	require.NoError(t, err)
+	validDeposit := stateTesting.GeneratePendingDeposit(t, sk, 1000, [32]byte{0x01}, 0)
+
+	t.Run("valid signature returns true", func(t *testing.T) {
+		s, err := state_native.InitializeFromProtoElectra(&eth.BeaconStateElectra{
+			PendingDeposits: []*eth.PendingDeposit{validDeposit},
+		})
+		require.NoError(t, err)
+
+		ok, err := s.IsPendingValidator(validDeposit.PublicKey)
+		require.NoError(t, err)
+		require.Equal(t, true, ok)
+	})
+
+	t.Run("invalid signature returns false", func(t *testing.T) {
+		invalidDeposit := &eth.PendingDeposit{
+			PublicKey:             validDeposit.PublicKey,
+			WithdrawalCredentials: validDeposit.WithdrawalCredentials,
+			Amount:                validDeposit.Amount,
+			Signature:             make([]byte, 96), // invalid empty signature
+		}
+		s, err := state_native.InitializeFromProtoElectra(&eth.BeaconStateElectra{
+			PendingDeposits: []*eth.PendingDeposit{invalidDeposit},
+		})
+		require.NoError(t, err)
+
+		ok, err := s.IsPendingValidator(validDeposit.PublicKey)
+		require.NoError(t, err)
+		require.Equal(t, false, ok)
+	})
+
+	t.Run("unknown pubkey returns false", func(t *testing.T) {
+		s, err := state_native.InitializeFromProtoElectra(&eth.BeaconStateElectra{
+			PendingDeposits: []*eth.PendingDeposit{validDeposit},
+		})
+		require.NoError(t, err)
+
+		ok, err := s.IsPendingValidator([]byte{9, 9, 9})
+		require.NoError(t, err)
+		require.Equal(t, false, ok)
+	})
+
+	t.Run("nil deposit skipped", func(t *testing.T) {
+		s, err := state_native.InitializeFromProtoElectra(&eth.BeaconStateElectra{
+			PendingDeposits: []*eth.PendingDeposit{nil, validDeposit},
+		})
+		require.NoError(t, err)
+
+		ok, err := s.IsPendingValidator(validDeposit.PublicKey)
+		require.NoError(t, err)
+		require.Equal(t, true, ok)
+	})
+
+	t.Run("pre-electra not supported", func(t *testing.T) {
+		s, err := state_native.InitializeFromProtoDeneb(&eth.BeaconStateDeneb{})
+		require.NoError(t, err)
+		_, err = s.IsPendingValidator([]byte{1, 2, 3})
+		require.ErrorContains(t, "not supported", err)
+	})
+}

changelog/t_bump-consensus-spec-alpha3.md

@@ -0,0 +1,7 @@
+### Changed
+
+- Bump consensus spec version from v1.7.0-alpha.2 to v1.7.0-alpha.3
+
+### Fixed
+
+- Check for pending validator deposits with valid signatures before applying builder deposits, preventing pubkey hijacking into the builder registry.

config/params/loader_test.go

@@ -35,11 +35,15 @@ var placeholderFields = []string{
 	"EIP7732_FORK_VERSION",
 	"EIP7928_FORK_EPOCH",
 	"EIP7928_FORK_VERSION",
+	"EIP8025_FORK_EPOCH",
+	"EIP8025_FORK_VERSION",
+	"EPOCHS_PER_SHUFFLING_PHASE",
 	"FIELD_ELEMENTS_PER_CELL",     // Configured as a constant in config/fieldparams/mainnet.go
 	"FIELD_ELEMENTS_PER_EXT_BLOB", // Configured in proto/ssz_proto_library.bzl
 	"HEZE_FORK_EPOCH",
 	"HEZE_FORK_VERSION",
 	"INCLUSION_LIST_COMMITTEE_SIZE",
+	"INCLUSION_LIST_SUBMISSION_DEADLINE",
 	"INCLUSION_LIST_SUBMISSION_DUE_BPS",
 	"KZG_COMMITMENTS_INCLUSION_PROOF_DEPTH", // Configured in proto/ssz_proto_library.bzl
 	"MAX_BYTES_PER_INCLUSION_LIST",

specrefs/containers.yml

@@ -683,6 +683,60 @@
         ptc_window: Vector[Vector[ValidatorIndex, PTC_SIZE], (2 + MIN_SEED_LOOKAHEAD) * SLOTS_PER_EPOCH]
     </spec>
 
+- name: BeaconState#heze
+  sources: []
+  spec: |
+    <spec ssz_object="BeaconState" fork="heze" hash="f73e357a">
+    class BeaconState(Container):
+        genesis_time: uint64
+        genesis_validators_root: Root
+        slot: Slot
+        fork: Fork
+        latest_block_header: BeaconBlockHeader
+        block_roots: Vector[Root, SLOTS_PER_HISTORICAL_ROOT]
+        state_roots: Vector[Root, SLOTS_PER_HISTORICAL_ROOT]
+        historical_roots: List[Root, HISTORICAL_ROOTS_LIMIT]
+        eth1_data: Eth1Data
+        eth1_data_votes: List[Eth1Data, EPOCHS_PER_ETH1_VOTING_PERIOD * SLOTS_PER_EPOCH]
+        eth1_deposit_index: uint64
+        validators: List[Validator, VALIDATOR_REGISTRY_LIMIT]
+        balances: List[Gwei, VALIDATOR_REGISTRY_LIMIT]
+        randao_mixes: Vector[Bytes32, EPOCHS_PER_HISTORICAL_VECTOR]
+        slashings: Vector[Gwei, EPOCHS_PER_SLASHINGS_VECTOR]
+        previous_epoch_participation: List[ParticipationFlags, VALIDATOR_REGISTRY_LIMIT]
+        current_epoch_participation: List[ParticipationFlags, VALIDATOR_REGISTRY_LIMIT]
+        justification_bits: Bitvector[JUSTIFICATION_BITS_LENGTH]
+        previous_justified_checkpoint: Checkpoint
+        current_justified_checkpoint: Checkpoint
+        finalized_checkpoint: Checkpoint
+        inactivity_scores: List[uint64, VALIDATOR_REGISTRY_LIMIT]
+        current_sync_committee: SyncCommittee
+        next_sync_committee: SyncCommittee
+        # [Modified in Heze:EIP7805]
+        latest_execution_payload_bid: ExecutionPayloadBid
+        next_withdrawal_index: WithdrawalIndex
+        next_withdrawal_validator_index: ValidatorIndex
+        historical_summaries: List[HistoricalSummary, HISTORICAL_ROOTS_LIMIT]
+        deposit_requests_start_index: uint64
+        deposit_balance_to_consume: Gwei
+        exit_balance_to_consume: Gwei
+        earliest_exit_epoch: Epoch
+        consolidation_balance_to_consume: Gwei
+        earliest_consolidation_epoch: Epoch
+        pending_deposits: List[PendingDeposit, PENDING_DEPOSITS_LIMIT]
+        pending_partial_withdrawals: List[PendingPartialWithdrawal, PENDING_PARTIAL_WITHDRAWALS_LIMIT]
+        pending_consolidations: List[PendingConsolidation, PENDING_CONSOLIDATIONS_LIMIT]
+        proposer_lookahead: Vector[ValidatorIndex, (MIN_SEED_LOOKAHEAD + 1) * SLOTS_PER_EPOCH]
+        builders: List[Builder, BUILDER_REGISTRY_LIMIT]
+        next_withdrawal_builder_index: BuilderIndex
+        execution_payload_availability: Bitvector[SLOTS_PER_HISTORICAL_ROOT]
+        builder_pending_payments: Vector[BuilderPendingPayment, 2 * SLOTS_PER_EPOCH]
+        builder_pending_withdrawals: List[BuilderPendingWithdrawal, BUILDER_PENDING_WITHDRAWALS_LIMIT]
+        latest_block_hash: Hash32
+        payload_expected_withdrawals: List[Withdrawal, MAX_WITHDRAWALS_PER_PAYLOAD]
+        ptc_window: Vector[Vector[ValidatorIndex, PTC_SIZE], (2 + MIN_SEED_LOOKAHEAD) * SLOTS_PER_EPOCH]
+    </spec>
+
 - name: BlobIdentifier#deneb
   sources:
     - file: proto/prysm/v1alpha1/blobs.proto