From eda918d00a020e54368091e3a5a0861547a3d023 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Thu, 14 May 2026 18:15:07 -0700 Subject: [PATCH 01/22] Batch verify builder deposit signatures --- .../src/per_block_processing.rs | 3 +- .../process_operations.rs | 215 +++++++--- .../src/per_block_processing/tests.rs | 388 +++++++++++++++++- .../per_block_processing/verify_deposit.rs | 87 +++- .../state_processing/src/upgrade/gloas.rs | 1 + 5 files changed, 627 insertions(+), 67 deletions(-) diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index f13f2a339b8..33f4d96fcbf 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -27,7 +27,8 @@ pub use verify_attestation::{ }; pub use verify_bls_to_execution_change::verify_bls_to_execution_change; pub use verify_deposit::{ - get_existing_validator_index, is_valid_deposit_signature, verify_deposit_merkle_proof, + get_existing_validator_index, is_valid_deposit_signature, is_valid_deposit_signature_batch, + verify_deposit_merkle_proof, }; pub use verify_exit::verify_exit; pub use withdrawals::get_expected_withdrawals; diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index 422e0afe065..e7c45d147c0 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -1,3 +1,5 @@ +use std::time::Instant; + use super::*; use crate::VerifySignatures; use crate::common::{ @@ -901,21 +903,10 @@ pub fn process_deposit_requests_pre_gloas( Ok(()) } -pub fn process_deposit_requests_post_gloas( - state: &mut BeaconState, - deposit_requests: &[DepositRequest], - spec: &ChainSpec, -) -> Result<(), BlockProcessingError> { - for request in deposit_requests { - process_deposit_request_post_gloas(state, request, spec)?; - } - - Ok(()) -} - /// Check if there is a pending deposit for a new validator with the given pubkey. // TODO(gloas): cache the deposit signature validation or remove this loop entirely if possible, // it is `O(n * m)` where `n` is max 8192 and `m` is max 128M. +#[instrument(name = "is_pending_validator", skip_all, level = "debug")] fn is_pending_validator( state: &BeaconState, pubkey: &PublicKeyBytes, @@ -937,59 +928,142 @@ fn is_pending_validator( Ok(false) } -pub fn process_deposit_request_post_gloas( - state: &mut BeaconState, - deposit_request: &DepositRequest, - spec: &ChainSpec, -) -> Result<(), BlockProcessingError> { - // [New in Gloas:EIP7732] - // Regardless of the withdrawal credentials prefix, if a builder/validator - // already exists with this pubkey, apply the deposit to their balance - // TODO(gloas): this could be more efficient in the builder case, see: - // https://github.com/sigp/lighthouse/issues/8783 - let builder_index = state +#[derive(Copy, Clone)] +pub enum VerifyBuilderSignature { + /// Verification not done + Verify, + VerifiedValid, + VerifiedInvalid, +} + +/// Returns `Ok(true)` if a builder with the given pubkey exists in the state. +/// Otherwise returns `Ok(false)`. Returns an error if this function is called with a +/// pre-gloas state. +/// +/// TODO(gloas): this could be more efficient in the builder case, see: +/// https://github.com/sigp/lighthouse/issues/8783 +#[instrument(name = "get_builder_index", skip_all, level = "debug")] +fn get_builder_index( + state: &BeaconState, + pubkey: &PublicKeyBytes, +) -> Result, BlockProcessingError> { + Ok(state .builders()? .iter() .enumerate() - .find(|(_, builder)| builder.pubkey == deposit_request.pubkey) - .map(|(i, _)| i as u64); - let is_builder = builder_index.is_some(); + .find(|(_, builder)| builder.pubkey == *pubkey) + .map(|(i, _)| i as u64)) +} - let validator_index = state.get_validator_index(&deposit_request.pubkey)?; - let is_validator = validator_index.is_some(); +pub fn process_deposit_requests_post_gloas( + state: &mut BeaconState, + deposit_requests: &[DepositRequest], + spec: &ChainSpec, +) -> Result<(), BlockProcessingError> { + let now = Instant::now(); + let mut builder_signature_request_indices = Vec::with_capacity(deposit_requests.len()); + let mut builder_signature_deposits = Vec::with_capacity(deposit_requests.len()); + + // Step 1: Collect all requests that could create a new builder when evaluated against the current + // state. + for (request_index, deposit_request) in deposit_requests.iter().enumerate() { + let is_builder = get_builder_index(state, &deposit_request.pubkey)?.is_some(); + let is_validator = state + .get_validator_index(&deposit_request.pubkey)? + .is_some(); + let has_builder_prefix = + is_builder_withdrawal_credential(deposit_request.withdrawal_credentials, spec); + + // Intentionally not checking `!is_pending_validator` to avoid potentially expensive operation + // in the first pass. This may result in more signatures to verify but potentially avoids a double + // loop with a signature verification. + // TODO(pawan): reevaluate if this is best for the worst case. + if !is_builder && has_builder_prefix && !is_validator { + builder_signature_request_indices.push(request_index); + builder_signature_deposits.push(DepositData { + pubkey: deposit_request.pubkey, + withdrawal_credentials: deposit_request.withdrawal_credentials, + amount: deposit_request.amount, + signature: deposit_request.signature.clone(), + }); + } + } + + let sig_now = Instant::now(); + let batch_len = builder_signature_deposits.len(); + // Step 2: Batch verify all builder signatures that need to be verified + let builder_signature_results = + is_valid_deposit_signature_batch(builder_signature_deposits, spec); - let has_builder_prefix = - is_builder_withdrawal_credential(deposit_request.withdrawal_credentials, spec); + tracing::debug!( + count = batch_len, + time = sig_now.elapsed().as_millis(), + "Completed processing deposit signatures" + ); - if is_builder - || (has_builder_prefix - && !is_validator - && !is_pending_validator(state, &deposit_request.pubkey, spec)?) + // TODO(pawan): potentially use a hashmap for clarity. + let mut builder_signature_results_by_request = + vec![VerifyBuilderSignature::Verify; deposit_requests.len()]; + for (request_index, is_valid) in builder_signature_request_indices + .into_iter() + .zip(builder_signature_results) { - // Apply builder deposits immediately - apply_deposit_for_builder( - state, - builder_index, - deposit_request.pubkey, - deposit_request.withdrawal_credentials, - deposit_request.amount, - deposit_request.signature.clone(), - state.slot(), - spec, - )?; - return Ok(()); + builder_signature_results_by_request[request_index] = if is_valid { + VerifyBuilderSignature::VerifiedValid + } else { + VerifyBuilderSignature::VerifiedInvalid + } } - // Add validator deposits to the queue - let slot = state.slot(); - state.pending_deposits_mut()?.push(PendingDeposit { - pubkey: deposit_request.pubkey, - withdrawal_credentials: deposit_request.withdrawal_credentials, - amount: deposit_request.amount, - signature: deposit_request.signature.clone(), - slot, - })?; + // Step 3: Second pass over the requests that is equivalent to the spec function only + // with the signature verification cached. + for (request_index, deposit_request) in deposit_requests.iter().enumerate() { + let builder_index = get_builder_index(state, &deposit_request.pubkey)?; + let is_builder = builder_index.is_some(); + + let is_validator = state + .get_validator_index(&deposit_request.pubkey)? + .is_some(); + let has_builder_prefix = + is_builder_withdrawal_credential(deposit_request.withdrawal_credentials, spec); + + if is_builder + || (has_builder_prefix + && !is_validator + && !is_pending_validator(state, &deposit_request.pubkey, spec)?) + { + apply_deposit_for_builder( + state, + builder_index, + deposit_request.pubkey, + deposit_request.withdrawal_credentials, + deposit_request.amount, + deposit_request.signature.clone(), + state.slot(), + builder_signature_results_by_request + .get(request_index) + .copied() + .unwrap_or(VerifyBuilderSignature::Verify), + spec, + )?; + continue; + } + + let slot = state.slot(); + state.pending_deposits_mut()?.push(PendingDeposit { + pubkey: deposit_request.pubkey, + withdrawal_credentials: deposit_request.withdrawal_credentials, + amount: deposit_request.amount, + signature: deposit_request.signature.clone(), + slot, + })?; + } + tracing::debug!( + count = deposit_requests.len(), + time = now.elapsed().as_millis(), + "Completed processing deposits" + ); Ok(()) } @@ -1002,6 +1076,7 @@ pub fn apply_deposit_for_builder( amount: u64, signature: SignatureBytes, slot: Slot, + verify_signature: VerifyBuilderSignature, spec: &ChainSpec, ) -> Result<(), BeaconStateError> { match builder_index_opt { @@ -1013,14 +1088,28 @@ pub fn apply_deposit_for_builder( amount, signature, }; - if is_valid_deposit_signature(&deposit_data, spec).is_ok() { - state.add_builder_to_registry( - pubkey, - withdrawal_credentials, - amount, - slot, - spec, - )?; + match verify_signature { + VerifyBuilderSignature::Verify => { + if is_valid_deposit_signature(&deposit_data, spec).is_ok() { + state.add_builder_to_registry( + pubkey, + withdrawal_credentials, + amount, + slot, + spec, + )?; + } + } + VerifyBuilderSignature::VerifiedValid => { + state.add_builder_to_registry( + pubkey, + withdrawal_credentials, + amount, + slot, + spec, + )?; + } + VerifyBuilderSignature::VerifiedInvalid => {} } } Some(builder_index) => { diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index 593a2557e86..b542094eadb 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -34,12 +34,26 @@ static KEYPAIRS: LazyLock> = async fn get_harness( epoch_offset: u64, num_validators: usize, +) -> BeaconChainHarness> { + get_harness_at_fork::(epoch_offset, num_validators, ForkName::Electra).await +} + +async fn get_gloas_harness( + epoch_offset: u64, + num_validators: usize, +) -> BeaconChainHarness> { + get_harness_at_fork::(epoch_offset, num_validators, ForkName::Gloas).await +} + +async fn get_harness_at_fork( + epoch_offset: u64, + num_validators: usize, + fork_name: ForkName, ) -> BeaconChainHarness> { // Set the state and block to be in the last slot of the `epoch_offset`th epoch. let last_slot_of_epoch = (MainnetEthSpec::genesis_epoch() + epoch_offset).end_slot(E::slots_per_epoch()); - // Use Electra spec to ensure blocks are created at the same fork as the state - let spec = Arc::new(ForkName::Electra.make_genesis_spec(E::default_spec())); + let spec = Arc::new(fork_name.make_genesis_spec(E::default_spec())); let harness = BeaconChainHarness::>::builder(E::default()) .spec(spec.clone()) .keypairs(KEYPAIRS[0..num_validators].to_vec()) @@ -62,6 +76,47 @@ async fn get_harness( harness } +fn builder_withdrawal_credentials(spec: &ChainSpec) -> Hash256 { + let mut credentials = [0u8; 32]; + credentials[0] = spec.builder_withdrawal_prefix_byte; + Hash256::from_slice(&credentials) +} + +fn make_deposit_request( + keypair: &Keypair, + withdrawal_credentials: Hash256, + amount: u64, + spec: &ChainSpec, + index: u64, +) -> DepositRequest { + let mut deposit_data = DepositData { + pubkey: keypair.pk.compress(), + withdrawal_credentials, + amount, + signature: SignatureBytes::empty(), + }; + deposit_data.signature = deposit_data.create_signature(&keypair.sk, spec); + + DepositRequest { + pubkey: deposit_data.pubkey, + withdrawal_credentials: deposit_data.withdrawal_credentials, + amount: deposit_data.amount, + signature: deposit_data.signature, + index, + } +} + +fn find_builder_index( + state: &BeaconState, + pubkey: &PublicKeyBytes, +) -> Option { + state + .builders() + .unwrap() + .iter() + .position(|builder| builder.pubkey == *pubkey) +} + #[tokio::test] async fn valid_block_ok() { let harness = get_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; @@ -377,6 +432,335 @@ async fn invalid_deposit_invalid_pub_key() { assert_eq!(result, Ok(())); } +#[tokio::test] +async fn deposit_signature_batch_returns_true_for_valid_and_false_for_invalid() { + let harness = get_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; + let spec = harness.spec.clone(); + let mut state = harness.get_current_state(); + + let (mut deposits, _) = harness.make_deposits(&mut state, 10, None, None); + deposits[1].data.signature = SignatureBytes::empty(); + deposits[8].data.pubkey = PublicKeyBytes::empty(); + + let result = per_block_processing::is_valid_deposit_signature_batch( + deposits.into_iter().map(|deposit| deposit.data).collect(), + &spec, + ); + + assert_eq!(result.len(), 10); + assert_eq!(result[1], false); + assert_eq!(result[8], false); + assert!( + result + .iter() + .enumerate() + .all(|(index, is_valid)| matches!(index, 1 | 8) || *is_valid) + ); +} + +#[tokio::test] +async fn deposit_signature_batch_returns_true_for_all_valid_signatures() { + let harness = get_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; + let spec = harness.spec.clone(); + let mut state = harness.get_current_state(); + + let (deposits, _) = harness.make_deposits(&mut state, 10, None, None); + + let result = per_block_processing::is_valid_deposit_signature_batch( + deposits.into_iter().map(|deposit| deposit.data).collect(), + &spec, + ); + + assert_eq!(result, vec![true; 10]); +} + +#[tokio::test] +async fn deposit_signature_batch_falls_back_to_individual_verification() { + let harness = get_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; + let spec = harness.spec.clone(); + let mut state = harness.get_current_state(); + + let (mut deposits, _) = harness.make_deposits(&mut state, 10, None, None); + let wrong_signature = deposits[4].data.signature.clone(); + deposits[3].data.signature = wrong_signature; + + let result = per_block_processing::is_valid_deposit_signature_batch( + deposits.into_iter().map(|deposit| deposit.data).collect(), + &spec, + ); + + assert_eq!(result.len(), 10); + assert_eq!(result[3], false); + assert!( + result + .iter() + .enumerate() + .all(|(index, is_valid)| index == 3 || *is_valid) + ); +} + +#[tokio::test] +async fn process_deposit_requests_post_gloas_batches_new_builder_signature_verification() { + let harness = get_gloas_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; + let spec = harness.spec.clone(); + let mut state = harness.get_current_state(); + + let existing_builder_keypair = &KEYPAIRS[VALIDATOR_COUNT]; + let existing_builder_credentials = builder_withdrawal_credentials(&spec); + let existing_builder_amount = 11; + let slot = state.slot(); + state + .add_builder_to_registry( + existing_builder_keypair.pk.compress(), + existing_builder_credentials, + existing_builder_amount, + slot, + &spec, + ) + .unwrap(); + + let valid_builder_request = make_deposit_request( + &KEYPAIRS[VALIDATOR_COUNT + 1], + builder_withdrawal_credentials(&spec), + 13, + &spec, + 0, + ); + + let mut invalid_builder_request = make_deposit_request( + &KEYPAIRS[VALIDATOR_COUNT + 2], + builder_withdrawal_credentials(&spec), + 17, + &spec, + 1, + ); + invalid_builder_request.signature = SignatureBytes::empty(); + + let mut existing_builder_top_up = make_deposit_request( + existing_builder_keypair, + existing_builder_credentials, + 19, + &spec, + 2, + ); + existing_builder_top_up.signature = SignatureBytes::empty(); + + let mut pending_validator_request = + make_deposit_request(&KEYPAIRS[VALIDATOR_COUNT + 3], Hash256::ZERO, 23, &spec, 3); + pending_validator_request.signature = SignatureBytes::empty(); + + process_operations::process_deposit_requests_post_gloas( + &mut state, + &[ + valid_builder_request.clone(), + invalid_builder_request.clone(), + existing_builder_top_up.clone(), + pending_validator_request.clone(), + ], + &spec, + ) + .unwrap(); + + let valid_builder_index = find_builder_index(&state, &valid_builder_request.pubkey); + assert!(valid_builder_index.is_some()); + + let invalid_builder_index = find_builder_index(&state, &invalid_builder_request.pubkey); + assert!(invalid_builder_index.is_none()); + + let existing_builder_index = find_builder_index(&state, &existing_builder_top_up.pubkey) + .unwrap(); + let existing_builder = state + .builders() + .unwrap() + .get(existing_builder_index) + .unwrap(); + assert_eq!( + existing_builder.balance, + existing_builder_amount + existing_builder_top_up.amount + ); + + let pending_deposits = state.pending_deposits().unwrap(); + assert_eq!(pending_deposits.len(), 1); + assert_eq!( + pending_deposits.get(0).unwrap().pubkey, + pending_validator_request.pubkey + ); + assert_eq!( + pending_deposits.get(0).unwrap().amount, + pending_validator_request.amount + ); +} + +#[tokio::test] +async fn process_deposit_requests_post_gloas_preserves_existing_builder_path() { + let harness = get_gloas_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; + let spec = harness.spec.clone(); + let mut state = harness.get_current_state(); + + let pubkey = KEYPAIRS[VALIDATOR_COUNT + 4].pk.compress(); + let withdrawal_credentials = builder_withdrawal_credentials(&spec); + + let first_request = make_deposit_request( + &KEYPAIRS[VALIDATOR_COUNT + 4], + withdrawal_credentials, + 29, + &spec, + 0, + ); + let mut second_request = make_deposit_request( + &KEYPAIRS[VALIDATOR_COUNT + 4], + withdrawal_credentials, + 31, + &spec, + 1, + ); + second_request.signature = SignatureBytes::empty(); + + process_operations::process_deposit_requests_post_gloas( + &mut state, + &[first_request.clone(), second_request.clone()], + &spec, + ) + .unwrap(); + + let builder_index = find_builder_index(&state, &pubkey).unwrap(); + let builder = state.builders().unwrap().get(builder_index).unwrap(); + assert_eq!( + builder.balance, + first_request.amount + second_request.amount + ); +} + +#[tokio::test] +async fn process_deposit_requests_post_gloas_preserves_pending_validator_path() { + let harness = get_gloas_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; + let spec = harness.spec.clone(); + let mut state = harness.get_current_state(); + + let pending_validator_request = + make_deposit_request(&KEYPAIRS[VALIDATOR_COUNT + 5], Hash256::ZERO, 37, &spec, 0); + let builder_prefixed_request = make_deposit_request( + &KEYPAIRS[VALIDATOR_COUNT + 5], + builder_withdrawal_credentials(&spec), + 41, + &spec, + 1, + ); + + process_operations::process_deposit_requests_post_gloas( + &mut state, + &[ + pending_validator_request.clone(), + builder_prefixed_request.clone(), + ], + &spec, + ) + .unwrap(); + + assert!(find_builder_index(&state, &pending_validator_request.pubkey).is_none()); + + let pending_deposits = state.pending_deposits().unwrap(); + assert_eq!(pending_deposits.len(), 2); + assert_eq!( + pending_deposits.get(0).unwrap().pubkey, + pending_validator_request.pubkey + ); + assert_eq!( + pending_deposits.get(1).unwrap().pubkey, + builder_prefixed_request.pubkey + ); +} + +#[tokio::test] +async fn process_deposit_requests_post_gloas_preserves_existing_builder_before_validator_path() { + let harness = get_gloas_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; + let spec = harness.spec.clone(); + let mut state = harness.get_current_state(); + + let builder_prefixed_request = make_deposit_request( + &KEYPAIRS[VALIDATOR_COUNT + 6], + builder_withdrawal_credentials(&spec), + 43, + &spec, + 0, + ); + let validator_prefixed_request = make_deposit_request( + &KEYPAIRS[VALIDATOR_COUNT + 6], + Hash256::ZERO, + 47, + &spec, + 1, + ); + + process_operations::process_deposit_requests_post_gloas( + &mut state, + &[ + builder_prefixed_request.clone(), + validator_prefixed_request.clone(), + ], + &spec, + ) + .unwrap(); + + let builder_index = find_builder_index(&state, &builder_prefixed_request.pubkey).unwrap(); + let builder = state.builders().unwrap().get(builder_index).unwrap(); + assert_eq!( + builder.balance, + builder_prefixed_request.amount + validator_prefixed_request.amount + ); + assert!(state.pending_deposits().unwrap().is_empty()); +} + +#[tokio::test] +async fn process_deposit_requests_post_gloas_preserves_pre_state_pending_validator_path() { + let harness = get_gloas_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; + let spec = harness.spec.clone(); + let mut state = harness.get_current_state(); + + let pending_validator_request = + make_deposit_request(&KEYPAIRS[VALIDATOR_COUNT + 7], Hash256::ZERO, 53, &spec, 0); + let slot = state.slot(); + state + .pending_deposits_mut() + .unwrap() + .push(PendingDeposit { + pubkey: pending_validator_request.pubkey, + withdrawal_credentials: pending_validator_request.withdrawal_credentials, + amount: pending_validator_request.amount, + signature: pending_validator_request.signature.clone(), + slot, + }) + .unwrap(); + + let builder_prefixed_request = make_deposit_request( + &KEYPAIRS[VALIDATOR_COUNT + 7], + builder_withdrawal_credentials(&spec), + 59, + &spec, + 1, + ); + + process_operations::process_deposit_requests_post_gloas( + &mut state, + &[builder_prefixed_request.clone()], + &spec, + ) + .unwrap(); + + assert!(find_builder_index(&state, &builder_prefixed_request.pubkey).is_none()); + + let pending_deposits = state.pending_deposits().unwrap(); + assert_eq!(pending_deposits.len(), 2); + assert_eq!( + pending_deposits.get(0).unwrap().pubkey, + pending_validator_request.pubkey + ); + assert_eq!( + pending_deposits.get(1).unwrap().pubkey, + builder_prefixed_request.pubkey + ); +} + #[tokio::test] async fn invalid_attestation_no_committee_for_index() { let harness = get_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; diff --git a/consensus/state_processing/src/per_block_processing/verify_deposit.rs b/consensus/state_processing/src/per_block_processing/verify_deposit.rs index d403bfa82b6..97f984242ed 100644 --- a/consensus/state_processing/src/per_block_processing/verify_deposit.rs +++ b/consensus/state_processing/src/per_block_processing/verify_deposit.rs @@ -1,13 +1,18 @@ use super::errors::{BlockOperationError, DepositInvalid}; use crate::per_block_processing::signature_sets::deposit_pubkey_signature_message; -use bls::PublicKeyBytes; +use bls::{PublicKey, PublicKeyBytes, Signature, SignatureSet, verify_signature_sets}; use merkle_proof::verify_merkle_proof; +use rayon::prelude::*; use safe_arith::SafeArith; +use std::borrow::Cow; +use tracing::instrument; use tree_hash::TreeHash; use types::*; type Result = std::result::Result>; +const DEPOSIT_SIGNATURE_BATCH_SIZE: usize = 8; + fn error(reason: DepositInvalid) -> BlockOperationError { BlockOperationError::invalid(reason) } @@ -66,3 +71,83 @@ pub fn verify_deposit_merkle_proof( Ok(()) } + +/// Batch verify a slice of deposit signatures +fn verify_deposit_signature_sets(entries: &[&(usize, PublicKey, Signature, Hash256)]) -> bool { + if entries.is_empty() { + return true; + } + + let signature_sets = entries + .iter() + .map(|(_, public_key, signature, message)| { + SignatureSet::single_pubkey(signature, Cow::Borrowed(public_key), *message) + }) + .collect::>(); + + verify_signature_sets(signature_sets.iter()) +} + +/// Helper for verifying a single deposit signature. +fn verify_deposit_signature(entry: &(usize, PublicKey, Signature, Hash256)) -> bool { + let (_, public_key, signature, message) = entry; + signature.verify(public_key, *message) +} + +/// Verify `Deposit.pubkey` signed `Deposit.signature` for each deposit in batches of `DEPOSIT_SIGNATURE_BATCH_SIZE`. +/// +/// Returns `true` for valid signatures and `false` for invalid signatures. +/// +/// Note: decompression failures are also considered as invalid signatures. +#[instrument(name = "is_valid_deposit_signature_batch", skip_all, level = "debug")] +pub fn is_valid_deposit_signature_batch( + deposit_data: Vec, + spec: &ChainSpec, +) -> Vec { + let decompressed = deposit_data + .par_iter() + .enumerate() + .map(|(index, deposit)| { + deposit_pubkey_signature_message(deposit, spec) + .map(|(public_key, signature, message)| (index, public_key, signature, message)) + }) + .collect::>(); + + // Initialize with false to ensure signatures that fail decompression above are also + // marked as signature verification failures + let mut results = vec![false; decompressed.len()]; + + let batch_results = decompressed + .par_chunks(DEPOSIT_SIGNATURE_BATCH_SIZE) + .map(|chunk| { + let valid_entries = chunk + .iter() + .filter_map(|entry| entry.as_ref()) + .collect::>(); + + // All signatures in this batch are valid + if verify_deposit_signature_sets(&valid_entries) { + valid_entries + .into_iter() + .map(|entry| (entry.0, true)) + .collect::>() + // There were some invalid signatures in this batch, + // verify individually to detect the invalid signatures. + } else { + valid_entries + .into_iter() + .map(|entry| (entry.0, verify_deposit_signature(entry))) + .collect::>() + } + }) + .collect::>(); + + for (index, is_valid) in batch_results.into_iter().flatten() { + debug_assert!(index < results.len()); + if let Some(res) = results.get_mut(index) { + *res = is_valid; + } + } + + results +} diff --git a/consensus/state_processing/src/upgrade/gloas.rs b/consensus/state_processing/src/upgrade/gloas.rs index 84cdbf22c29..b9f9b356d2e 100644 --- a/consensus/state_processing/src/upgrade/gloas.rs +++ b/consensus/state_processing/src/upgrade/gloas.rs @@ -208,6 +208,7 @@ fn onboard_builders_from_pending_deposits( deposit.amount, deposit.signature.clone(), deposit.slot, + crate::per_block_processing::process_operations::VerifyBuilderSignature::Verify, spec, )?; continue; From ed15ac69f0ea40ba9ad71ccbee434276f7d05f60 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Fri, 15 May 2026 16:13:53 -0700 Subject: [PATCH 02/22] Refactor `onboard_builders_from_pending_deposits` based on new spec --- .../process_operations.rs | 19 +++--- .../state_processing/src/upgrade/gloas.rs | 63 +++++++------------ 2 files changed, 34 insertions(+), 48 deletions(-) diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index e7c45d147c0..b859d11a80d 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -13,6 +13,7 @@ use crate::per_block_processing::errors::{BlockProcessingError, ExitInvalid, Int use crate::per_block_processing::signature_sets::{exit_signature_set, get_pubkey_from_state}; use crate::per_block_processing::verify_payload_attestation::verify_payload_attestation; use bls::{PublicKeyBytes, SignatureBytes}; +use milhouse::List; use ssz_types::FixedVector; use typenum::U33; use types::consts::altair::{PARTICIPATION_FLAG_WEIGHTS, PROPOSER_WEIGHT, WEIGHT_DENOMINATOR}; @@ -907,12 +908,12 @@ pub fn process_deposit_requests_pre_gloas( // TODO(gloas): cache the deposit signature validation or remove this loop entirely if possible, // it is `O(n * m)` where `n` is max 8192 and `m` is max 128M. #[instrument(name = "is_pending_validator", skip_all, level = "debug")] -fn is_pending_validator( - state: &BeaconState, +pub fn is_pending_validator( + deposits: &List, pubkey: &PublicKeyBytes, spec: &ChainSpec, -) -> Result { - for deposit in state.pending_deposits()?.iter() { +) -> bool { + for deposit in deposits.iter() { if deposit.pubkey == *pubkey { let deposit_data = DepositData { pubkey: deposit.pubkey, @@ -921,11 +922,11 @@ fn is_pending_validator( signature: deposit.signature.clone(), }; if is_valid_deposit_signature(&deposit_data, spec).is_ok() { - return Ok(true); + return true; } } } - Ok(false) + false } #[derive(Copy, Clone)] @@ -1030,7 +1031,11 @@ pub fn process_deposit_requests_post_gloas( if is_builder || (has_builder_prefix && !is_validator - && !is_pending_validator(state, &deposit_request.pubkey, spec)?) + && !is_pending_validator::( + state.pending_deposits()?, + &deposit_request.pubkey, + spec, + )) { apply_deposit_for_builder( state, diff --git a/consensus/state_processing/src/upgrade/gloas.rs b/consensus/state_processing/src/upgrade/gloas.rs index b9f9b356d2e..56a73665c12 100644 --- a/consensus/state_processing/src/upgrade/gloas.rs +++ b/consensus/state_processing/src/upgrade/gloas.rs @@ -1,3 +1,4 @@ +use crate::per_block_processing::process_operations::is_pending_validator; use crate::per_block_processing::{ is_valid_deposit_signature, process_operations::apply_deposit_for_builder, }; @@ -167,11 +168,6 @@ fn onboard_builders_from_pending_deposits( state: &mut BeaconState, spec: &ChainSpec, ) -> Result<(), Error> { - // Rather than tracking all `validator_pubkeys` in one place as the spec does, we keep a - // hashset for *just* the new validator pubkeys, and use the state's efficient - // `get_validator_index` function instead of an O(n) iteration over the full validator list. - let mut new_validator_pubkeys = HashSet::new(); - // Clone pending deposits to avoid borrow conflicts when mutating state. let current_pending_deposits = state.pending_deposits()?.clone(); @@ -179,9 +175,7 @@ fn onboard_builders_from_pending_deposits( for deposit in ¤t_pending_deposits { // Deposits for existing validators stay in the pending queue. - if new_validator_pubkeys.contains(&deposit.pubkey) - || state.get_validator_index(&deposit.pubkey)?.is_some() - { + if state.get_validator_index(&deposit.pubkey)?.is_some() { pending_deposits.push(deposit.clone())?; continue; } @@ -195,40 +189,27 @@ fn onboard_builders_from_pending_deposits( .iter() .position(|b| b.pubkey == deposit.pubkey); - let has_builder_credentials = - is_builder_withdrawal_credential(deposit.withdrawal_credentials, spec); - - if builder_index.is_some() || has_builder_credentials { - let builder_index_opt = builder_index.map(|i| i as u64); - apply_deposit_for_builder( - state, - builder_index_opt, - deposit.pubkey, - deposit.withdrawal_credentials, - deposit.amount, - deposit.signature.clone(), - deposit.slot, - crate::per_block_processing::process_operations::VerifyBuilderSignature::Verify, - spec, - )?; - continue; - } - - // If there is a pending deposit for a new validator that has a valid signature, - // track the pubkey so that subsequent builder deposits for the same pubkey stay - // in pending (applied to the validator later) rather than creating a builder. - // Deposits with invalid signatures are dropped since they would fail in - // apply_pending_deposit anyway. - let deposit_data = DepositData { - pubkey: deposit.pubkey, - withdrawal_credentials: deposit.withdrawal_credentials, - amount: deposit.amount, - signature: deposit.signature.clone(), - }; - if is_valid_deposit_signature(&deposit_data, spec).is_ok() { - new_validator_pubkeys.insert(deposit.pubkey); - pending_deposits.push(deposit.clone())?; + // Equivalent to if deposit.pubkey not in builder_pubkeys: + if builder_index.is_none() { + if !is_builder_withdrawal_credential(deposit.withdrawal_credentials, spec) + || is_pending_validator::(&pending_deposits, &deposit.pubkey, spec) + { + pending_deposits.push(deposit.clone())?; + continue; + } } + let builder_index_opt = builder_index.map(|i| i as u64); + apply_deposit_for_builder( + state, + builder_index_opt, + deposit.pubkey, + deposit.withdrawal_credentials, + deposit.amount, + deposit.signature.clone(), + deposit.slot, + crate::per_block_processing::process_operations::VerifyBuilderSignature::Verify, + spec, + )?; } *state.pending_deposits_mut()? = pending_deposits; From 369eecdaa787d25970060e33d0bee150d2f62d38 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Fri, 15 May 2026 16:14:21 -0700 Subject: [PATCH 03/22] Add a builder onboarding cache --- beacon_node/beacon_chain/src/beacon_chain.rs | 11 ++ beacon_node/beacon_chain/src/builder.rs | 12 ++ .../src/builder_deposits_cache.rs | 158 ++++++++++++++++++ beacon_node/beacon_chain/src/lib.rs | 1 + 4 files changed, 182 insertions(+) create mode 100644 beacon_node/beacon_chain/src/builder_deposits_cache.rs diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index af8cd477d6c..6f53de55fdc 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -15,6 +15,7 @@ use crate::block_verification::{ use crate::block_verification_types::{ AsBlock, AvailableExecutedBlock, BlockImportData, ExecutedBlock, RangeSyncBlock, }; +use crate::builder_deposits_cache::OnboardBuildersCache; pub use crate::canonical_head::CanonicalHead; use crate::chain_config::ChainConfig; use crate::custody_context::CustodyContextSsz; @@ -511,6 +512,7 @@ pub struct BeaconChain { /// The KZG trusted setup used by this chain. pub kzg: Arc, /// RNG instance used by the chain. Currently used for shuffling column sidecars in block publishing. + pub builder_onboarding_cache: Option>, pub rng: Arc>>, } @@ -4536,6 +4538,15 @@ impl BeaconChain { current_slot, ); + if let Some(builder_onboarding_cache) = &self.builder_onboarding_cache { + let cache = builder_onboarding_cache.clone(); + let spec = self.spec.clone(); + self.task_executor.spawn_blocking( + move || cache.add_new_pending_deposits::(&state, &spec), + "pre_verify_deposits", + ); + } + Ok(block_root) } diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index e668bef7c03..891b2778076 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -4,6 +4,7 @@ use crate::beacon_chain::{ BEACON_CHAIN_DB_KEY, CanonicalHead, LightClientProducerEvent, OP_POOL_DB_KEY, }; use crate::beacon_proposer_cache::BeaconProposerCache; +use crate::builder_deposits_cache::OnboardBuildersCache; use crate::custody_context::NodeCustodyType; use crate::data_availability_checker::DataAvailabilityChecker; use crate::fork_choice_signal::ForkChoiceSignalTx; @@ -1084,6 +1085,7 @@ where rng: Arc::new(Mutex::new(rng)), gossip_verified_payload_bid_cache: <_>::default(), gossip_verified_proposer_preferences_cache: <_>::default(), + builder_onboarding_cache: OnboardBuildersCache::new(&self.spec).map(Arc::new), }; let head = beacon_chain.head_snapshot(); @@ -1155,6 +1157,16 @@ where .process_prune_blobs(data_availability_boundary); } + // Seed the builder onboarding cache in the background from the current head state. + if let Some(onboarding_cache) = &beacon_chain.builder_onboarding_cache { + let cache = onboarding_cache.clone(); + let spec = self.spec.clone(); + beacon_chain.task_executor.spawn_blocking( + move || cache.seed_from_state(&head.beacon_state, &spec), + "initialize_builder_onboarding_cache", + ); + } + Ok(beacon_chain) } } diff --git a/beacon_node/beacon_chain/src/builder_deposits_cache.rs b/beacon_node/beacon_chain/src/builder_deposits_cache.rs new file mode 100644 index 00000000000..74a9a84ce93 --- /dev/null +++ b/beacon_node/beacon_chain/src/builder_deposits_cache.rs @@ -0,0 +1,158 @@ +use lru::LruCache; +use parking_lot::Mutex; +use state_processing::per_block_processing::is_valid_deposit_signature_batch; +use tracing::debug; +use tree_hash::{Hash256, TreeHash}; +use types::{ + BeaconState, ChainSpec, DepositData, EthSpec, PendingDeposit, is_builder_withdrawal_credential, + new_non_zero_usize, +}; + +use std::num::NonZeroUsize; + +// TODO(pawan): analyze size of cache +const CACHE_SIZE: NonZeroUsize = new_non_zero_usize(64000); + +pub struct OnboardBuildersCache { + cache: Mutex>, +} + +impl OnboardBuildersCache { + pub fn new(spec: &ChainSpec) -> Option { + if spec.is_gloas_scheduled() { + Some(Self { + cache: Mutex::new(LruCache::new(CACHE_SIZE)), + }) + } else { + None + } + } + + /// Initializes the cache with all the `pending_deposits` from the passed state + /// that would need to be onboarded at the gloas fork. + /// + /// Further block imports that result in additional deposits should be handled by the + /// [`Self::add_new_pending_deposits`] method. + pub fn seed_from_state(&self, state: &BeaconState, spec: &ChainSpec) { + let Some(pending_deposits) = state.pending_deposits().ok() else { + return; + }; + let pending_deposits = pending_deposits.iter().collect::>(); + if pending_deposits.is_empty() { + return; + } + + debug!( + pending_deposits_count = pending_deposits.len(), + "Seeding builder onboarding cache from head state" + ); + + self.cache_pending_deposits(pending_deposits, spec); + } + + /// Gets the new deposits added to the `pending_cache` for `state.slot. + /// Signature verifies and caches them for later use. + pub fn add_new_pending_deposits( + &self, + current_state: &BeaconState, + spec: &ChainSpec, + ) { + let pending_deposits = pending_deposits_to_verify(current_state); + if pending_deposits.is_empty() { + return; + } + + debug!( + pending_deposits_count = pending_deposits.len(), + slot = %current_state.slot(), + "Adding new pending deposits to builder onboarding cache" + ); + + self.cache_pending_deposits(pending_deposits, &spec); + } + + /// Takes a list of pending deposits, signature verifies them and caches the result. + fn cache_pending_deposits(&self, deposits: Vec<&PendingDeposit>, spec: &ChainSpec) { + let mut builder_deposits = Vec::new(); + let mut builder_deposit_keys = Vec::new(); + + { + let mut cache = self.cache.lock(); + for deposit in deposits { + if !is_builder_withdrawal_credential(deposit.withdrawal_credentials, spec) { + continue; + } + + let deposit_data = DepositData { + amount: deposit.amount, + pubkey: deposit.pubkey, + signature: deposit.signature.clone(), + withdrawal_credentials: deposit.withdrawal_credentials, + }; + let key = deposit_data.tree_hash_root(); + if cache.get(&key).is_some() { + continue; + } + + builder_deposit_keys.push(key); + builder_deposits.push(deposit_data); + } + } + + if builder_deposits.is_empty() { + return; + } + + debug!( + builder_deposits_count = builder_deposits.len(), + "Pre-verifying builder onboarding deposit signatures" + ); + + let verified = is_valid_deposit_signature_batch(builder_deposits, spec); + let mut cache = self.cache.lock(); + for (key, value) in builder_deposit_keys.into_iter().zip(verified.into_iter()) { + cache.push(key, value); + } + } + + /// Returns `Some(true)` if the deposit exists in the cache and has a valid signature. + /// Returns `Some(false)` if the deposit exists and failed signature verification. + /// Returns `None` if the deposit doesn't exist in the cache. + pub fn cached_is_valid_signature(&self, deposit: &PendingDeposit) -> Option { + let deposit_data = DepositData { + amount: deposit.amount, + pubkey: deposit.pubkey, + signature: deposit.signature.clone(), + withdrawal_credentials: deposit.withdrawal_credentials, + }; + let key = deposit_data.tree_hash_root(); + self.cache.lock().get(&key).copied() + } +} + +/// Returns a list of `pending_deposits` that were added for the same slot as the passed state. +fn pending_deposits_to_verify(state: &BeaconState) -> Vec<&PendingDeposit> { + let current_slot = state.slot(); + let Some(pending_deposits) = state.pending_deposits().ok() else { + return Vec::new(); + }; + // Get the index of the first `pending_deposit` for the current slot + // + // Need to do this roundabout way because milhouse iterators aren't double ended, so + // rev().take_while() won't work. + let mut first_current_slot_index = 0; + for index in (0..pending_deposits.len()).rev() { + if pending_deposits + .get(index) + .is_some_and(|deposit| deposit.slot != current_slot) + { + first_current_slot_index = index + 1; + break; + } + } + + pending_deposits + .iter() + .skip(first_current_slot_index) + .collect() +} diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index 804268a6139..6a41fb65b76 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -3,6 +3,7 @@ pub mod attestation_simulator; pub mod attestation_verification; pub mod beacon_block_reward; mod beacon_block_streamer; +mod builder_deposits_cache; mod beacon_chain; mod beacon_fork_choice_store; pub mod beacon_proposer_cache; From 112b09384e85ec085535ccb3022529f8151bb97f Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Fri, 15 May 2026 17:16:26 -0700 Subject: [PATCH 04/22] Working wiring with pre-gloas cache --- Cargo.lock | 2 ++ beacon_node/beacon_chain/src/beacon_chain.rs | 9 +++++++-- .../beacon_chain/src/block_verification.rs | 2 +- beacon_node/beacon_chain/src/builder.rs | 4 ++-- beacon_node/beacon_chain/src/lib.rs | 1 - .../beacon_chain/src/state_advance_timer.rs | 2 +- beacon_node/store/src/reconstruct.rs | 2 +- consensus/state_processing/Cargo.toml | 2 ++ .../state_processing/src/block_replayer.rs | 10 ++++++---- .../src/builder_deposits_cache.rs | 2 +- consensus/state_processing/src/genesis.rs | 2 +- consensus/state_processing/src/lib.rs | 1 + .../process_operations.rs | 10 ++++++++++ .../src/per_slot_processing.rs | 4 +++- .../state_processing/src/state_advance.rs | 6 ++++-- .../state_processing/src/upgrade/gloas.rs | 19 +++++++++++++++---- testing/ef_tests/src/cases/fork.rs | 4 +++- testing/ef_tests/src/cases/sanity_blocks.rs | 4 ++-- testing/ef_tests/src/cases/sanity_slots.rs | 2 +- 19 files changed, 63 insertions(+), 25 deletions(-) rename {beacon_node/beacon_chain => consensus/state_processing}/src/builder_deposits_cache.rs (98%) diff --git a/Cargo.lock b/Cargo.lock index 078f699f3c8..09621b8406b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8426,9 +8426,11 @@ dependencies = [ "int_to_bytes", "integer-sqrt", "itertools 0.14.0", + "lru 0.12.5", "merkle_proof", "metrics", "milhouse", + "parking_lot", "rand 0.9.2", "rayon", "safe_arith", diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 6f53de55fdc..654c3a6ef4a 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -15,7 +15,6 @@ use crate::block_verification::{ use crate::block_verification_types::{ AsBlock, AvailableExecutedBlock, BlockImportData, ExecutedBlock, RangeSyncBlock, }; -use crate::builder_deposits_cache::OnboardBuildersCache; pub use crate::canonical_head::CanonicalHead; use crate::chain_config::ChainConfig; use crate::custody_context::CustodyContextSsz; @@ -122,6 +121,7 @@ use slot_clock::SlotClock; use ssz::Encode; use state_processing::{ BlockSignatureStrategy, ConsensusContext, SigVerifiedOp, VerifyBlockRoot, VerifyOperation, + builder_deposits_cache::OnboardBuildersCache, common::get_attesting_indices_from_state, epoch_cache::initialize_epoch_cache, per_block_processing, @@ -1515,7 +1515,12 @@ impl BeaconChain { while state.slot() < slot { // Note: supplying some `state_root` when it is known would be a cheap and easy // optimization. - match per_slot_processing(&mut state, skip_state_root, &self.spec) { + match per_slot_processing( + &mut state, + skip_state_root, + self.builder_onboarding_cache.as_deref(), + &self.spec, + ) { Ok(_) => (), Err(e) => { warn!( diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 24f971f736f..5c142adc870 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -1542,7 +1542,7 @@ impl ExecutionPendingBlock { state_root }; - if let Some(summary) = per_slot_processing(&mut state, Some(state_root), &chain.spec)? { + if let Some(summary) = per_slot_processing(&mut state, Some(state_root), chain.builder_onboarding_cache.as_deref(), &chain.spec)? { // Expose Prometheus metrics. if let Err(e) = summary.observe_metrics() { error!( diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 891b2778076..9e32def7581 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -4,7 +4,6 @@ use crate::beacon_chain::{ BEACON_CHAIN_DB_KEY, CanonicalHead, LightClientProducerEvent, OP_POOL_DB_KEY, }; use crate::beacon_proposer_cache::BeaconProposerCache; -use crate::builder_deposits_cache::OnboardBuildersCache; use crate::custody_context::NodeCustodyType; use crate::data_availability_checker::DataAvailabilityChecker; use crate::fork_choice_signal::ForkChoiceSignalTx; @@ -37,6 +36,7 @@ use rayon::prelude::*; use slasher::Slasher; use slot_clock::{SlotClock, TestingSlotClock}; use state_processing::AllCaches; +use state_processing::builder_deposits_cache::OnboardBuildersCache; use state_processing::genesis::genesis_block; use state_processing::per_slot_processing; use std::marker::PhantomData; @@ -446,7 +446,7 @@ where "Advancing checkpoint state to boundary" ); while weak_subj_state.slot() % slots_per_epoch != 0 { - per_slot_processing(&mut weak_subj_state, None, &self.spec) + per_slot_processing(&mut weak_subj_state, None, None, &self.spec) .map_err(|e| format!("Error advancing state: {e:?}"))?; } } diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index 6a41fb65b76..804268a6139 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -3,7 +3,6 @@ pub mod attestation_simulator; pub mod attestation_verification; pub mod beacon_block_reward; mod beacon_block_streamer; -mod builder_deposits_cache; mod beacon_chain; mod beacon_fork_choice_store; pub mod beacon_proposer_cache; diff --git a/beacon_node/beacon_chain/src/state_advance_timer.rs b/beacon_node/beacon_chain/src/state_advance_timer.rs index cb916cb5142..36305306a92 100644 --- a/beacon_node/beacon_chain/src/state_advance_timer.rs +++ b/beacon_node/beacon_chain/src/state_advance_timer.rs @@ -287,7 +287,7 @@ fn advance_head(beacon_chain: &Arc>) -> Resu // Advance the state a single slot. if let Some(summary) = - per_slot_processing(&mut state, Some(head_state_root), &beacon_chain.spec) + per_slot_processing(&mut state, Some(head_state_root), beacon_chain.builder_onboarding_cache.as_deref(), &beacon_chain.spec) .map_err(BeaconChainError::from)? { // Expose Prometheus metrics. diff --git a/beacon_node/store/src/reconstruct.rs b/beacon_node/store/src/reconstruct.rs index 04a519af020..5afcefc9bef 100644 --- a/beacon_node/store/src/reconstruct.rs +++ b/beacon_node/store/src/reconstruct.rs @@ -145,7 +145,7 @@ where }; // Advance state to slot. - per_slot_processing(&mut state, prev_state_root.take(), &self.spec) + per_slot_processing(&mut state, prev_state_root.take(), None, &self.spec) .map_err(HotColdDBError::BlockReplaySlotError)?; // Apply block. diff --git a/consensus/state_processing/Cargo.toml b/consensus/state_processing/Cargo.toml index 72d0e17d999..c8eb985ea3d 100644 --- a/consensus/state_processing/Cargo.toml +++ b/consensus/state_processing/Cargo.toml @@ -29,9 +29,11 @@ fixed_bytes = { workspace = true } int_to_bytes = { workspace = true } integer-sqrt = "0.1.5" itertools = { workspace = true } +lru = {workspace = true } merkle_proof = { workspace = true } metrics = { workspace = true } milhouse = { workspace = true } +parking_lot = {workspace = true } rand = { workspace = true } rayon = { workspace = true } safe_arith = { workspace = true } diff --git a/consensus/state_processing/src/block_replayer.rs b/consensus/state_processing/src/block_replayer.rs index 56e667cdd37..005885f5fad 100644 --- a/consensus/state_processing/src/block_replayer.rs +++ b/consensus/state_processing/src/block_replayer.rs @@ -230,8 +230,9 @@ where pre_slot_hook(state_root, &mut self.state)?; } - let summary = per_slot_processing(&mut self.state, Some(state_root), self.spec) - .map_err(BlockReplayError::from)?; + let summary = + per_slot_processing(&mut self.state, Some(state_root), None, self.spec) + .map_err(BlockReplayError::from)?; if let Some(ref mut post_slot_hook) = self.post_slot_hook { let is_skipped_slot = self.state.slot() < block.slot(); @@ -276,8 +277,9 @@ where pre_slot_hook(state_root, &mut self.state)?; } - let summary = per_slot_processing(&mut self.state, Some(state_root), self.spec) - .map_err(BlockReplayError::from)?; + let summary = + per_slot_processing(&mut self.state, Some(state_root), None, self.spec) + .map_err(BlockReplayError::from)?; if let Some(ref mut post_slot_hook) = self.post_slot_hook { // No more blocks to apply (from our perspective) so we consider these slots diff --git a/beacon_node/beacon_chain/src/builder_deposits_cache.rs b/consensus/state_processing/src/builder_deposits_cache.rs similarity index 98% rename from beacon_node/beacon_chain/src/builder_deposits_cache.rs rename to consensus/state_processing/src/builder_deposits_cache.rs index 74a9a84ce93..da1f330713a 100644 --- a/beacon_node/beacon_chain/src/builder_deposits_cache.rs +++ b/consensus/state_processing/src/builder_deposits_cache.rs @@ -1,6 +1,6 @@ +use crate::per_block_processing::is_valid_deposit_signature_batch; use lru::LruCache; use parking_lot::Mutex; -use state_processing::per_block_processing::is_valid_deposit_signature_batch; use tracing::debug; use tree_hash::{Hash256, TreeHash}; use types::{ diff --git a/consensus/state_processing/src/genesis.rs b/consensus/state_processing/src/genesis.rs index c643ad56e34..f945cc40761 100644 --- a/consensus/state_processing/src/genesis.rs +++ b/consensus/state_processing/src/genesis.rs @@ -162,7 +162,7 @@ pub fn initialize_beacon_state_from_eth1( .gloas_fork_epoch .is_some_and(|fork_epoch| fork_epoch == E::genesis_epoch()) { - upgrade_to_gloas(&mut state, spec)?; + upgrade_to_gloas(&mut state, None, spec)?; // Remove intermediate Fulu fork from `state.fork`. state.fork_mut().previous_version = spec.gloas_fork_version; diff --git a/consensus/state_processing/src/lib.rs b/consensus/state_processing/src/lib.rs index e37c5265799..46f9d29dc57 100644 --- a/consensus/state_processing/src/lib.rs +++ b/consensus/state_processing/src/lib.rs @@ -18,6 +18,7 @@ mod metrics; pub mod all_caches; pub mod block_replayer; +pub mod builder_deposits_cache; pub mod common; pub mod consensus_context; pub mod envelope_processing; diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index b859d11a80d..07fcf983a82 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -937,6 +937,16 @@ pub enum VerifyBuilderSignature { VerifiedInvalid, } +impl From> for VerifyBuilderSignature { + fn from(value: Option) -> Self { + match value { + Some(true) => VerifyBuilderSignature::VerifiedValid, + Some(false) => VerifyBuilderSignature::VerifiedInvalid, + None => VerifyBuilderSignature::Verify, + } + } +} + /// Returns `Ok(true)` if a builder with the given pubkey exists in the state. /// Otherwise returns `Ok(false)`. Returns an error if this function is called with a /// pre-gloas state. diff --git a/consensus/state_processing/src/per_slot_processing.rs b/consensus/state_processing/src/per_slot_processing.rs index f26ea567a26..b604ea39202 100644 --- a/consensus/state_processing/src/per_slot_processing.rs +++ b/consensus/state_processing/src/per_slot_processing.rs @@ -1,3 +1,4 @@ +use crate::builder_deposits_cache::OnboardBuildersCache; use crate::upgrade::{ upgrade_to_altair, upgrade_to_bellatrix, upgrade_to_capella, upgrade_to_deneb, upgrade_to_electra, upgrade_to_fulu, upgrade_to_gloas, @@ -38,6 +39,7 @@ impl From for Error { pub fn per_slot_processing( state: &mut BeaconState, state_root: Option, + builder_onboarding_cache: Option<&OnboardBuildersCache>, spec: &ChainSpec, ) -> Result>, Error> { // Verify that the `BeaconState` instantiation matches the fork at `state.slot()`. @@ -100,7 +102,7 @@ pub fn per_slot_processing( // Gloas. if spec.gloas_fork_epoch == Some(state.current_epoch()) { - upgrade_to_gloas(state, spec)?; + upgrade_to_gloas(state, builder_onboarding_cache, spec)?; } // Additionally build all caches so that all valid states that are advanced always have diff --git a/consensus/state_processing/src/state_advance.rs b/consensus/state_processing/src/state_advance.rs index 11145621553..fd057b101a4 100644 --- a/consensus/state_processing/src/state_advance.rs +++ b/consensus/state_processing/src/state_advance.rs @@ -40,7 +40,8 @@ pub fn complete_state_advance( // future iterations. let state_root_opt = state_root_opt.take(); - per_slot_processing(state, state_root_opt, spec).map_err(Error::PerSlotProcessing)?; + per_slot_processing(state, state_root_opt, None, spec) + .map_err(Error::PerSlotProcessing)?; } Ok(()) @@ -95,7 +96,8 @@ pub fn partial_state_advance( // with the correct state root. let state_root = initial_state_root.take().unwrap_or_else(Hash256::zero); - per_slot_processing(state, Some(state_root), spec).map_err(Error::PerSlotProcessing)?; + per_slot_processing(state, Some(state_root), None, spec) + .map_err(Error::PerSlotProcessing)?; } Ok(()) diff --git a/consensus/state_processing/src/upgrade/gloas.rs b/consensus/state_processing/src/upgrade/gloas.rs index 56a73665c12..0679f97336f 100644 --- a/consensus/state_processing/src/upgrade/gloas.rs +++ b/consensus/state_processing/src/upgrade/gloas.rs @@ -1,4 +1,7 @@ -use crate::per_block_processing::process_operations::is_pending_validator; +use crate::builder_deposits_cache::OnboardBuildersCache; +use crate::per_block_processing::process_operations::{ + VerifyBuilderSignature, is_pending_validator, +}; use crate::per_block_processing::{ is_valid_deposit_signature, process_operations::apply_deposit_for_builder, }; @@ -19,9 +22,10 @@ use types::{ /// Transform a `Fulu` state into a `Gloas` state. pub fn upgrade_to_gloas( pre_state: &mut BeaconState, + builder_onboarding_cache: Option<&OnboardBuildersCache>, spec: &ChainSpec, ) -> Result<(), Error> { - let post = upgrade_state_to_gloas(pre_state, spec)?; + let post = upgrade_state_to_gloas(pre_state, builder_onboarding_cache, spec)?; *pre_state = post; @@ -30,6 +34,7 @@ pub fn upgrade_to_gloas( pub fn upgrade_state_to_gloas( pre_state: &mut BeaconState, + builder_onboarding_cache: Option<&OnboardBuildersCache>, spec: &ChainSpec, ) -> Result, Error> { let epoch = pre_state.current_epoch(); @@ -123,7 +128,7 @@ pub fn upgrade_state_to_gloas( epoch_cache: mem::take(&mut pre.epoch_cache), }); // [New in Gloas:EIP7732] - onboard_builders_from_pending_deposits(&mut post, spec)?; + onboard_builders_from_pending_deposits(&mut post, builder_onboarding_cache, spec)?; initialize_ptc_window(&mut post, spec)?; Ok(post) @@ -166,6 +171,7 @@ fn initialize_ptc_window( /// Applies any pending deposit for builders, effectively onboarding builders at the fork. fn onboard_builders_from_pending_deposits( state: &mut BeaconState, + builder_onboarding_cache: Option<&OnboardBuildersCache>, spec: &ChainSpec, ) -> Result<(), Error> { // Clone pending deposits to avoid borrow conflicts when mutating state. @@ -199,6 +205,11 @@ fn onboard_builders_from_pending_deposits( } } let builder_index_opt = builder_index.map(|i| i as u64); + let verify_signature = if let Some(cache) = builder_onboarding_cache { + cache.cached_is_valid_signature(deposit).into() + } else { + VerifyBuilderSignature::Verify + }; apply_deposit_for_builder( state, builder_index_opt, @@ -207,7 +218,7 @@ fn onboard_builders_from_pending_deposits( deposit.amount, deposit.signature.clone(), deposit.slot, - crate::per_block_processing::process_operations::VerifyBuilderSignature::Verify, + verify_signature, spec, )?; } diff --git a/testing/ef_tests/src/cases/fork.rs b/testing/ef_tests/src/cases/fork.rs index 54efb9f9cec..6e483dc041d 100644 --- a/testing/ef_tests/src/cases/fork.rs +++ b/testing/ef_tests/src/cases/fork.rs @@ -72,7 +72,9 @@ impl Case for ForkTest { ForkName::Deneb => upgrade_to_deneb(&mut result_state, spec).map(|_| result_state), ForkName::Electra => upgrade_to_electra(&mut result_state, spec).map(|_| result_state), ForkName::Fulu => upgrade_to_fulu(&mut result_state, spec).map(|_| result_state), - ForkName::Gloas => upgrade_to_gloas(&mut result_state, spec).map(|_| result_state), + ForkName::Gloas => { + upgrade_to_gloas(&mut result_state, None, spec).map(|_| result_state) + } }; compare_beacon_state_results_without_caches(&mut result, &mut expected) diff --git a/testing/ef_tests/src/cases/sanity_blocks.rs b/testing/ef_tests/src/cases/sanity_blocks.rs index 538783eaa90..fded04bb125 100644 --- a/testing/ef_tests/src/cases/sanity_blocks.rs +++ b/testing/ef_tests/src/cases/sanity_blocks.rs @@ -79,8 +79,8 @@ impl Case for SanityBlocks { .try_for_each(|signed_block| { let block = signed_block.message(); while bulk_state.slot() < block.slot() { - per_slot_processing(&mut bulk_state, None, spec).unwrap(); - per_slot_processing(&mut indiv_state, None, spec).unwrap(); + per_slot_processing(&mut bulk_state, None, None, spec).unwrap(); + per_slot_processing(&mut indiv_state, None, None, spec).unwrap(); } bulk_state diff --git a/testing/ef_tests/src/cases/sanity_slots.rs b/testing/ef_tests/src/cases/sanity_slots.rs index 71c782c78f4..fa444917582 100644 --- a/testing/ef_tests/src/cases/sanity_slots.rs +++ b/testing/ef_tests/src/cases/sanity_slots.rs @@ -64,7 +64,7 @@ impl Case for SanitySlots { state.build_caches(spec).unwrap(); let mut result = (0..self.slots) - .try_for_each(|_| per_slot_processing(&mut state, None, spec).map(|_| ())) + .try_for_each(|_| per_slot_processing(&mut state, None, None, spec).map(|_| ())) .map(|_| state); compare_beacon_state_results_without_caches(&mut result, &mut expected) From 0aa7d7aaf6b68dff0c609061e4e9af2384b2274c Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Fri, 15 May 2026 17:45:13 -0700 Subject: [PATCH 05/22] More threading --- beacon_node/beacon_chain/src/beacon_chain.rs | 2 + .../src/block_production/gloas.rs | 2 + .../beacon_chain/src/block_verification.rs | 1 + .../payload_envelope_verification/import.rs | 8 +++ beacon_node/beacon_chain/src/test_utils.rs | 1 + .../beacon_chain/tests/block_verification.rs | 6 ++- .../beacon_chain/tests/prepare_payload.rs | 2 + beacon_node/store/src/reconstruct.rs | 1 + .../state_processing/src/block_replayer.rs | 1 + .../src/builder_deposits_cache.rs | 54 ++++++++++++++++++- .../src/per_block_processing.rs | 15 ++++-- .../process_operations.rs | 53 ++++++++++++------ .../src/per_block_processing/tests.rs | 10 ++++ .../src/per_epoch_processing/tests.rs | 8 +-- .../state_processing/src/upgrade/gloas.rs | 8 +-- lcli/src/transition_blocks.rs | 1 + testing/ef_tests/src/cases/operations.rs | 4 +- testing/ef_tests/src/cases/sanity_blocks.rs | 2 + testing/ef_tests/src/cases/transition.rs | 1 + testing/state_transition_vectors/src/exit.rs | 1 + 20 files changed, 145 insertions(+), 36 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 654c3a6ef4a..68435670df9 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -5204,6 +5204,7 @@ impl BeaconChain { apply_parent_execution_payload( &mut advanced_state, &envelope.message.execution_requests, + self.builder_onboarding_cache.as_deref(), &self.spec, ) .map_err(Error::PrepareProposerFailed)?; @@ -6207,6 +6208,7 @@ impl BeaconChain { signature_strategy, VerifyBlockRoot::True, &mut ctxt, + self.builder_onboarding_cache.as_deref(), &self.spec, )?; drop(process_timer); diff --git a/beacon_node/beacon_chain/src/block_production/gloas.rs b/beacon_node/beacon_chain/src/block_production/gloas.rs index 6510c20ba7b..afacf4f8731 100644 --- a/beacon_node/beacon_chain/src/block_production/gloas.rs +++ b/beacon_node/beacon_chain/src/block_production/gloas.rs @@ -619,6 +619,7 @@ impl BeaconChain { signature_strategy, VerifyBlockRoot::True, &mut ctxt, + self.builder_onboarding_cache.as_deref(), &self.spec, )?; drop(process_timer); @@ -965,6 +966,7 @@ fn get_execution_payload_gloas( apply_parent_execution_payload( &mut withdrawals_state, &envelope.message.execution_requests, + chain.builder_onboarding_cache.as_deref(), spec, )?; Withdrawals::::from(get_expected_withdrawals(&withdrawals_state, spec)?) diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 5c142adc870..504870978a7 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -1611,6 +1611,7 @@ impl ExecutionPendingBlock { BlockSignatureStrategy::NoVerification, VerifyBlockRoot::True, &mut consensus_context, + chain.builder_onboarding_cache.as_deref(), &chain.spec, ) { match err { diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs index 73ddb43273f..b3fe24e0d25 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs @@ -68,6 +68,14 @@ impl BeaconChain { .into_execution_pending_envelope(&chain, notify_execution_layer)?; publish_fn()?; + // Pre-verify builder deposit signatures while awaiting EL verification. + if let Some(ref cache) = chain.builder_onboarding_cache { + cache.cache_deposit_requests( + &execution_pending.signed_envelope.message.execution_requests.deposits, + &chain.spec, + ); + } + // Record the time it took to complete consensus verification. if let Some(timestamp) = chain.slot_clock.now_duration() { chain diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 8e9cc612080..18bf6d39da2 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -1425,6 +1425,7 @@ where BlockSignatureStrategy::NoVerification, VerifyBlockRoot::False, &mut ctxt, + None, &self.spec, ) .unwrap_or_else(|e| panic!("per_block_processing failed at slot {}: {e:?}", slot)); diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 533ef612197..87d7b8170ad 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -1696,7 +1696,7 @@ async fn add_base_block_to_altair_chain() { { let mut state = state; let mut ctxt = ConsensusContext::new(base_block.slot()); - per_slot_processing(&mut state, None, &harness.chain.spec).unwrap(); + per_slot_processing(&mut state, None, None, &harness.chain.spec).unwrap(); assert!(matches!( per_block_processing( &mut state, @@ -1704,6 +1704,7 @@ async fn add_base_block_to_altair_chain() { BlockSignatureStrategy::NoVerification, VerifyBlockRoot::True, &mut ctxt, + None, &harness.chain.spec, ), Err(BlockProcessingError::InconsistentBlockFork( @@ -1847,7 +1848,7 @@ async fn add_altair_block_to_base_chain() { { let mut state = state; let mut ctxt = ConsensusContext::new(altair_block.slot()); - per_slot_processing(&mut state, None, &harness.chain.spec).unwrap(); + per_slot_processing(&mut state, None, None, &harness.chain.spec).unwrap(); assert!(matches!( per_block_processing( &mut state, @@ -1855,6 +1856,7 @@ async fn add_altair_block_to_base_chain() { BlockSignatureStrategy::NoVerification, VerifyBlockRoot::True, &mut ctxt, + None, &harness.chain.spec, ), Err(BlockProcessingError::InconsistentBlockFork( diff --git a/beacon_node/beacon_chain/tests/prepare_payload.rs b/beacon_node/beacon_chain/tests/prepare_payload.rs index 47dd1ef517e..9557447271b 100644 --- a/beacon_node/beacon_chain/tests/prepare_payload.rs +++ b/beacon_node/beacon_chain/tests/prepare_payload.rs @@ -237,6 +237,7 @@ async fn prepare_payload_generic( apply_parent_execution_payload( &mut unadvanced_full_state, &envelope.message.execution_requests, + None, &spec, ) .unwrap(); @@ -245,6 +246,7 @@ async fn prepare_payload_generic( apply_parent_execution_payload( &mut advanced_full_state, &envelope.message.execution_requests, + None, &spec, ) .unwrap(); diff --git a/beacon_node/store/src/reconstruct.rs b/beacon_node/store/src/reconstruct.rs index 5afcefc9bef..0c62212c132 100644 --- a/beacon_node/store/src/reconstruct.rs +++ b/beacon_node/store/src/reconstruct.rs @@ -160,6 +160,7 @@ where BlockSignatureStrategy::NoVerification, VerifyBlockRoot::True, &mut ctxt, + None, &self.spec, ) .map_err(HotColdDBError::BlockReplayBlockError)?; diff --git a/consensus/state_processing/src/block_replayer.rs b/consensus/state_processing/src/block_replayer.rs index 005885f5fad..17b97fb8b83 100644 --- a/consensus/state_processing/src/block_replayer.rs +++ b/consensus/state_processing/src/block_replayer.rs @@ -260,6 +260,7 @@ where self.block_sig_strategy, verify_block_root, &mut ctxt, + None, self.spec, ) .map_err(BlockReplayError::from)?; diff --git a/consensus/state_processing/src/builder_deposits_cache.rs b/consensus/state_processing/src/builder_deposits_cache.rs index da1f330713a..be1916292c8 100644 --- a/consensus/state_processing/src/builder_deposits_cache.rs +++ b/consensus/state_processing/src/builder_deposits_cache.rs @@ -4,8 +4,8 @@ use parking_lot::Mutex; use tracing::debug; use tree_hash::{Hash256, TreeHash}; use types::{ - BeaconState, ChainSpec, DepositData, EthSpec, PendingDeposit, is_builder_withdrawal_credential, - new_non_zero_usize, + BeaconState, ChainSpec, DepositData, DepositRequest, EthSpec, PendingDeposit, + is_builder_withdrawal_credential, new_non_zero_usize, }; use std::num::NonZeroUsize; @@ -115,6 +115,51 @@ impl OnboardBuildersCache { } } + /// Pre-verifies builder deposit signatures from execution payload deposit requests + /// and caches the results for later use during `process_deposit_requests_post_gloas`. + pub fn cache_deposit_requests(&self, deposit_requests: &[DepositRequest], spec: &ChainSpec) { + let mut builder_deposits = Vec::new(); + let mut builder_deposit_keys = Vec::new(); + + { + let mut cache = self.cache.lock(); + for request in deposit_requests { + if !is_builder_withdrawal_credential(request.withdrawal_credentials, spec) { + continue; + } + + let deposit_data = DepositData { + amount: request.amount, + pubkey: request.pubkey, + signature: request.signature.clone(), + withdrawal_credentials: request.withdrawal_credentials, + }; + let key = deposit_data.tree_hash_root(); + if cache.get(&key).is_some() { + continue; + } + + builder_deposit_keys.push(key); + builder_deposits.push(deposit_data); + } + } + + if builder_deposits.is_empty() { + return; + } + + debug!( + builder_deposits_count = builder_deposits.len(), + "Pre-verifying builder deposit signatures from payload envelope" + ); + + let verified = is_valid_deposit_signature_batch(builder_deposits, spec); + let mut cache = self.cache.lock(); + for (key, value) in builder_deposit_keys.into_iter().zip(verified.into_iter()) { + cache.push(key, value); + } + } + /// Returns `Some(true)` if the deposit exists in the cache and has a valid signature. /// Returns `Some(false)` if the deposit exists and failed signature verification. /// Returns `None` if the deposit doesn't exist in the cache. @@ -125,6 +170,11 @@ impl OnboardBuildersCache { signature: deposit.signature.clone(), withdrawal_credentials: deposit.withdrawal_credentials, }; + self.get(&deposit_data) + } + + /// Looks up a `DepositData` in the cache by its tree hash root. + pub fn get(&self, deposit_data: &DepositData) -> Option { let key = deposit_data.tree_hash_root(); self.cache.lock().get(&key).copied() } diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index 33f4d96fcbf..ee772960346 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -1,3 +1,4 @@ +use crate::builder_deposits_cache::OnboardBuildersCache; use crate::consensus_context::ConsensusContext; use errors::{ BlockOperationError, BlockProcessingError, ExecutionPayloadBidInvalid, HeaderInvalid, @@ -116,6 +117,7 @@ pub fn per_block_processing>( block_signature_strategy: BlockSignatureStrategy, verify_block_root: VerifyBlockRoot, ctxt: &mut ConsensusContext, + builder_onboarding_cache: Option<&OnboardBuildersCache>, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { let block = signed_block.message(); @@ -132,7 +134,7 @@ pub fn per_block_processing>( // Process deferred execution requests from the parent's envelope. if fork_name.gloas_enabled() { - process_parent_execution_payload(state, block, spec)?; + process_parent_execution_payload(state, block, builder_onboarding_cache, spec)?; } // Build epoch cache if it hasn't already been built, or if it is no longer valid @@ -549,6 +551,7 @@ pub fn compute_timestamp_at_slot( pub fn process_parent_execution_payload>( state: &mut BeaconState, block: BeaconBlockRef<'_, E, Payload>, + builder_onboarding_cache: Option<&OnboardBuildersCache>, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { let bid_parent_block_hash = block @@ -578,7 +581,7 @@ pub fn process_parent_execution_payload( state: &mut BeaconState, requests: &ExecutionRequests, + onboarding_cache: Option<&OnboardBuildersCache>, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { let parent_bid = state.latest_execution_payload_bid()?.clone(); @@ -597,7 +601,12 @@ pub fn apply_parent_execution_payload( let parent_epoch = parent_slot.epoch(E::slots_per_epoch()); // Process execution requests from the parent's payload - process_operations::process_deposit_requests_post_gloas(state, &requests.deposits, spec)?; + process_operations::process_deposit_requests_post_gloas( + state, + &requests.deposits, + onboarding_cache, + spec, + )?; process_operations::process_withdrawal_requests(state, &requests.withdrawals, spec)?; process_operations::process_consolidation_requests(state, &requests.consolidations, spec)?; diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index 07fcf983a82..9bb7b39af5a 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -2,6 +2,7 @@ use std::time::Instant; use super::*; use crate::VerifySignatures; +use crate::builder_deposits_cache::OnboardBuildersCache; use crate::common::{ get_attestation_participation_flag_indices, increase_balance, initiate_validator_exit, slash_validator, @@ -969,6 +970,7 @@ fn get_builder_index( pub fn process_deposit_requests_post_gloas( state: &mut BeaconState, deposit_requests: &[DepositRequest], + onboarding_cache: Option<&OnboardBuildersCache>, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { let now = Instant::now(); @@ -1000,32 +1002,49 @@ pub fn process_deposit_requests_post_gloas( } } + // Step 2: Check cache for already-verified signatures, batch verify the rest. let sig_now = Instant::now(); - let batch_len = builder_signature_deposits.len(); - // Step 2: Batch verify all builder signatures that need to be verified - let builder_signature_results = - is_valid_deposit_signature_batch(builder_signature_deposits, spec); - - tracing::debug!( - count = batch_len, - time = sig_now.elapsed().as_millis(), - "Completed processing deposit signatures" - ); - - // TODO(pawan): potentially use a hashmap for clarity. let mut builder_signature_results_by_request = vec![VerifyBuilderSignature::Verify; deposit_requests.len()]; - for (request_index, is_valid) in builder_signature_request_indices - .into_iter() - .zip(builder_signature_results) - { + + let mut uncached_indices = Vec::new(); + let mut uncached_deposits = Vec::new(); + + for (i, deposit_data) in builder_signature_deposits.iter().enumerate() { + let cached_result = onboarding_cache.and_then(|c| c.get(deposit_data)); + if let Some(is_valid) = cached_result { + let request_index = builder_signature_request_indices[i]; + builder_signature_results_by_request[request_index] = if is_valid { + VerifyBuilderSignature::VerifiedValid + } else { + VerifyBuilderSignature::VerifiedInvalid + }; + } else { + uncached_indices.push(i); + uncached_deposits.push(deposit_data.clone()); + } + } + + let cache_hits = builder_signature_deposits.len() - uncached_deposits.len(); + let batch_len = uncached_deposits.len(); + let batch_results = is_valid_deposit_signature_batch(uncached_deposits, spec); + + for (batch_i, is_valid) in uncached_indices.into_iter().zip(batch_results) { + let request_index = builder_signature_request_indices[batch_i]; builder_signature_results_by_request[request_index] = if is_valid { VerifyBuilderSignature::VerifiedValid } else { VerifyBuilderSignature::VerifiedInvalid - } + }; } + tracing::debug!( + verified = batch_len, + cache_hits, + time = sig_now.elapsed().as_millis(), + "Completed processing deposit signatures" + ); + // Step 3: Second pass over the requests that is equivalent to the spec function only // with the signature verification cached. for (request_index, deposit_request) in deposit_requests.iter().enumerate() { diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index b542094eadb..f93420ea495 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -135,6 +135,7 @@ async fn valid_block_ok() { BlockSignatureStrategy::VerifyIndividual, VerifyBlockRoot::True, &mut ctxt, + None, &spec, ); @@ -160,6 +161,7 @@ async fn invalid_block_header_state_slot() { BlockSignatureStrategy::VerifyIndividual, VerifyBlockRoot::True, &mut ctxt, + None, &spec, ); @@ -192,6 +194,7 @@ async fn invalid_parent_block_root() { BlockSignatureStrategy::VerifyIndividual, VerifyBlockRoot::True, &mut ctxt, + None, &spec, ); @@ -222,6 +225,7 @@ async fn invalid_block_signature() { BlockSignatureStrategy::VerifyIndividual, VerifyBlockRoot::True, &mut ctxt, + None, &spec, ); @@ -254,6 +258,7 @@ async fn invalid_randao_reveal_signature() { BlockSignatureStrategy::VerifyIndividual, VerifyBlockRoot::True, &mut ctxt, + None, &spec, ); @@ -557,6 +562,7 @@ async fn process_deposit_requests_post_gloas_batches_new_builder_signature_verif existing_builder_top_up.clone(), pending_validator_request.clone(), ], + None, &spec, ) .unwrap(); @@ -619,6 +625,7 @@ async fn process_deposit_requests_post_gloas_preserves_existing_builder_path() { process_operations::process_deposit_requests_post_gloas( &mut state, &[first_request.clone(), second_request.clone()], + None, &spec, ) .unwrap(); @@ -653,6 +660,7 @@ async fn process_deposit_requests_post_gloas_preserves_pending_validator_path() pending_validator_request.clone(), builder_prefixed_request.clone(), ], + None, &spec, ) .unwrap(); @@ -698,6 +706,7 @@ async fn process_deposit_requests_post_gloas_preserves_existing_builder_before_v builder_prefixed_request.clone(), validator_prefixed_request.clone(), ], + None, &spec, ) .unwrap(); @@ -743,6 +752,7 @@ async fn process_deposit_requests_post_gloas_preserves_pre_state_pending_validat process_operations::process_deposit_requests_post_gloas( &mut state, &[builder_prefixed_request.clone()], + None, &spec, ) .unwrap(); diff --git a/consensus/state_processing/src/per_epoch_processing/tests.rs b/consensus/state_processing/src/per_epoch_processing/tests.rs index 29716866b56..bde94425484 100644 --- a/consensus/state_processing/src/per_epoch_processing/tests.rs +++ b/consensus/state_processing/src/per_epoch_processing/tests.rs @@ -81,7 +81,7 @@ mod release_tests { // Check the state is valid before starting this test. process_epoch(&mut altair_state.clone(), &spec) .expect("state passes intial epoch processing"); - per_slot_processing(&mut altair_state.clone(), None, &spec) + per_slot_processing(&mut altair_state.clone(), None, None, &spec) .expect("state passes intial slot processing"); // Modify the spec so altair never happens. @@ -98,7 +98,7 @@ mod release_tests { Err(EpochProcessingError::InconsistentStateFork(expected_err)) ); assert_eq!( - per_slot_processing(&mut altair_state.clone(), None, &spec), + per_slot_processing(&mut altair_state.clone(), None, None, &spec), Err(SlotProcessingError::InconsistentStateFork(expected_err)) ); } @@ -141,7 +141,7 @@ mod release_tests { // Check the state is valid before starting this test. process_epoch(&mut base_state.clone(), &spec) .expect("state passes intial epoch processing"); - per_slot_processing(&mut base_state.clone(), None, &spec) + per_slot_processing(&mut base_state.clone(), None, None, &spec) .expect("state passes intial slot processing"); // Modify the spec so Altair happens at the first epoch. @@ -158,7 +158,7 @@ mod release_tests { Err(EpochProcessingError::InconsistentStateFork(expected_err)) ); assert_eq!( - per_slot_processing(&mut base_state.clone(), None, &spec), + per_slot_processing(&mut base_state.clone(), None, None, &spec), Err(SlotProcessingError::InconsistentStateFork(expected_err)) ); } diff --git a/consensus/state_processing/src/upgrade/gloas.rs b/consensus/state_processing/src/upgrade/gloas.rs index 0679f97336f..ad296e67993 100644 --- a/consensus/state_processing/src/upgrade/gloas.rs +++ b/consensus/state_processing/src/upgrade/gloas.rs @@ -2,21 +2,17 @@ use crate::builder_deposits_cache::OnboardBuildersCache; use crate::per_block_processing::process_operations::{ VerifyBuilderSignature, is_pending_validator, }; -use crate::per_block_processing::{ - is_valid_deposit_signature, process_operations::apply_deposit_for_builder, -}; +use crate::per_block_processing::process_operations::apply_deposit_for_builder; use milhouse::{List, Vector}; use safe_arith::SafeArith; use ssz_types::BitVector; use ssz_types::FixedVector; -use std::collections::HashSet; use std::mem; use tree_hash::TreeHash; use typenum::Unsigned; use types::{ BeaconState, BeaconStateError as Error, BeaconStateGloas, BuilderPendingPayment, ChainSpec, - DepositData, EthSpec, ExecutionPayloadBid, ExecutionRequests, Fork, - is_builder_withdrawal_credential, + EthSpec, ExecutionPayloadBid, ExecutionRequests, Fork, is_builder_withdrawal_credential, }; /// Transform a `Fulu` state into a `Gloas` state. diff --git a/lcli/src/transition_blocks.rs b/lcli/src/transition_blocks.rs index 69d3975d09b..afaed71b3eb 100644 --- a/lcli/src/transition_blocks.rs +++ b/lcli/src/transition_blocks.rs @@ -399,6 +399,7 @@ fn do_transition( BlockSignatureStrategy::NoVerification, VerifyBlockRoot::True, &mut ctxt, + None, spec, ) .map_err(|e| format!("State transition failed: {:?}", e))?; diff --git a/testing/ef_tests/src/cases/operations.rs b/testing/ef_tests/src/cases/operations.rs index f5c999920dc..7405b9e2b68 100644 --- a/testing/ef_tests/src/cases/operations.rs +++ b/testing/ef_tests/src/cases/operations.rs @@ -582,7 +582,7 @@ impl Operation for ParentExecutionPayloadBlock { spec: &ChainSpec, _: &Operations, ) -> Result<(), BlockProcessingError> { - process_parent_execution_payload(state, self.block.to_ref(), spec) + process_parent_execution_payload(state, self.block.to_ref(), None, spec) } } @@ -716,7 +716,7 @@ impl Operation for DepositRequest { _extra: &Operations, ) -> Result<(), BlockProcessingError> { if state.fork_name_unchecked().gloas_enabled() { - process_deposit_requests_post_gloas(state, std::slice::from_ref(self), spec) + process_deposit_requests_post_gloas(state, std::slice::from_ref(self), None, spec) } else { process_deposit_requests_pre_gloas(state, std::slice::from_ref(self), spec) } diff --git a/testing/ef_tests/src/cases/sanity_blocks.rs b/testing/ef_tests/src/cases/sanity_blocks.rs index fded04bb125..ea35935452f 100644 --- a/testing/ef_tests/src/cases/sanity_blocks.rs +++ b/testing/ef_tests/src/cases/sanity_blocks.rs @@ -98,6 +98,7 @@ impl Case for SanityBlocks { BlockSignatureStrategy::VerifyIndividual, VerifyBlockRoot::True, &mut ctxt, + None, spec, )?; @@ -108,6 +109,7 @@ impl Case for SanityBlocks { BlockSignatureStrategy::VerifyBulk, VerifyBlockRoot::True, &mut ctxt, + None, spec, )?; diff --git a/testing/ef_tests/src/cases/transition.rs b/testing/ef_tests/src/cases/transition.rs index 06aa8136506..66598625d8e 100644 --- a/testing/ef_tests/src/cases/transition.rs +++ b/testing/ef_tests/src/cases/transition.rs @@ -133,6 +133,7 @@ impl Case for TransitionTest { BlockSignatureStrategy::VerifyBulk, VerifyBlockRoot::True, &mut ctxt, + None, spec, ) .map_err(|e| format!("Block processing failed: {:?}", e))?; diff --git a/testing/state_transition_vectors/src/exit.rs b/testing/state_transition_vectors/src/exit.rs index 3b0fe7d8ec2..8264fd17e43 100644 --- a/testing/state_transition_vectors/src/exit.rs +++ b/testing/state_transition_vectors/src/exit.rs @@ -71,6 +71,7 @@ impl ExitTest { BlockSignatureStrategy::VerifyIndividual, VerifyBlockRoot::True, &mut ctxt, + None, &test_spec::(), ) } From 5e46d387afe0b64aad80f8ab18daaac7f80d7f5c Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Sat, 16 May 2026 18:29:22 -0700 Subject: [PATCH 06/22] Clean up threading with a GloasVerificationContext --- beacon_node/beacon_chain/src/beacon_chain.rs | 6 +- .../beacon_chain/src/block_verification.rs | 6 +- beacon_node/beacon_chain/src/builder.rs | 4 +- .../beacon_chain/src/state_advance_timer.rs | 4 +- .../tests/attestation_verification.rs | 8 +-- .../beacon_chain/tests/block_verification.rs | 8 +-- beacon_node/beacon_chain/tests/tests.rs | 6 +- beacon_node/http_api/tests/tests.rs | 4 +- beacon_node/store/src/reconstruct.rs | 6 +- .../state_processing/src/block_replayer.rs | 10 ++-- consensus/state_processing/src/genesis.rs | 6 +- consensus/state_processing/src/lib.rs | 1 + .../src/per_epoch_processing/tests.rs | 11 ++-- .../src/per_slot_processing.rs | 6 +- .../state_processing/src/state_advance.rs | 5 +- consensus/state_processing/src/upgrade.rs | 2 +- .../state_processing/src/upgrade/gloas.rs | 55 +++++++++++++++++-- testing/ef_tests/src/cases/fork.rs | 6 +- testing/ef_tests/src/cases/sanity_blocks.rs | 8 +-- testing/ef_tests/src/cases/sanity_slots.rs | 4 +- 20 files changed, 108 insertions(+), 58 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 68435670df9..ecce8c369eb 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -120,8 +120,8 @@ use slasher::Slasher; use slot_clock::SlotClock; use ssz::Encode; use state_processing::{ - BlockSignatureStrategy, ConsensusContext, SigVerifiedOp, VerifyBlockRoot, VerifyOperation, - builder_deposits_cache::OnboardBuildersCache, + BlockSignatureStrategy, ConsensusContext, GloasVerificationContext, SigVerifiedOp, + VerifyBlockRoot, VerifyOperation, builder_deposits_cache::OnboardBuildersCache, common::get_attesting_indices_from_state, epoch_cache::initialize_epoch_cache, per_block_processing, @@ -1518,7 +1518,7 @@ impl BeaconChain { match per_slot_processing( &mut state, skip_state_root, - self.builder_onboarding_cache.as_deref(), + GloasVerificationContext::from_cache(self.builder_onboarding_cache.clone()), &self.spec, ) { Ok(_) => (), diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 504870978a7..9d9482aa90f 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -81,8 +81,8 @@ use ssz::Encode; use ssz_derive::{Decode, Encode}; use state_processing::per_block_processing::errors::IntoWithIndex; use state_processing::{ - AllCaches, BlockProcessingError, BlockSignatureStrategy, ConsensusContext, SlotProcessingError, - VerifyBlockRoot, + AllCaches, BlockProcessingError, BlockSignatureStrategy, ConsensusContext, + GloasVerificationContext, SlotProcessingError, VerifyBlockRoot, block_signature_verifier::{BlockSignatureVerifier, Error as BlockSignatureVerifierError}, per_block_processing, per_slot_processing, state_advance::partial_state_advance, @@ -1542,7 +1542,7 @@ impl ExecutionPendingBlock { state_root }; - if let Some(summary) = per_slot_processing(&mut state, Some(state_root), chain.builder_onboarding_cache.as_deref(), &chain.spec)? { + if let Some(summary) = per_slot_processing(&mut state, Some(state_root), GloasVerificationContext::from_cache(chain.builder_onboarding_cache.clone()), &chain.spec)? { // Expose Prometheus metrics. if let Err(e) = summary.observe_metrics() { error!( diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 9e32def7581..b167453f914 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -38,7 +38,7 @@ use slot_clock::{SlotClock, TestingSlotClock}; use state_processing::AllCaches; use state_processing::builder_deposits_cache::OnboardBuildersCache; use state_processing::genesis::genesis_block; -use state_processing::per_slot_processing; +use state_processing::{GloasVerificationContext, per_slot_processing}; use std::marker::PhantomData; use std::sync::Arc; use std::time::Duration; @@ -446,7 +446,7 @@ where "Advancing checkpoint state to boundary" ); while weak_subj_state.slot() % slots_per_epoch != 0 { - per_slot_processing(&mut weak_subj_state, None, None, &self.spec) + per_slot_processing(&mut weak_subj_state, None, GloasVerificationContext::FullVerification, &self.spec) .map_err(|e| format!("Error advancing state: {e:?}"))?; } } diff --git a/beacon_node/beacon_chain/src/state_advance_timer.rs b/beacon_node/beacon_chain/src/state_advance_timer.rs index 36305306a92..f5a45c2c64a 100644 --- a/beacon_node/beacon_chain/src/state_advance_timer.rs +++ b/beacon_node/beacon_chain/src/state_advance_timer.rs @@ -18,7 +18,7 @@ use crate::{ BeaconChain, BeaconChainError, BeaconChainTypes, chain_config::FORK_CHOICE_LOOKAHEAD_FACTOR, }; use slot_clock::SlotClock; -use state_processing::per_slot_processing; +use state_processing::{GloasVerificationContext, per_slot_processing}; use std::sync::{ Arc, atomic::{AtomicBool, Ordering}, @@ -287,7 +287,7 @@ fn advance_head(beacon_chain: &Arc>) -> Resu // Advance the state a single slot. if let Some(summary) = - per_slot_processing(&mut state, Some(head_state_root), beacon_chain.builder_onboarding_cache.as_deref(), &beacon_chain.spec) + per_slot_processing(&mut state, Some(head_state_root), GloasVerificationContext::from_cache(beacon_chain.builder_onboarding_cache.clone()), &beacon_chain.spec) .map_err(BeaconChainError::from)? { // Expose Prometheus metrics. diff --git a/beacon_node/beacon_chain/tests/attestation_verification.rs b/beacon_node/beacon_chain/tests/attestation_verification.rs index da7f380e361..a722aa321f1 100644 --- a/beacon_node/beacon_chain/tests/attestation_verification.rs +++ b/beacon_node/beacon_chain/tests/attestation_verification.rs @@ -20,7 +20,7 @@ use fixed_bytes::FixedBytesExtended; use genesis::{DEFAULT_ETH1_BLOCK_HASH, interop_genesis_state}; use int_to_bytes::int_to_bytes32; use slasher::{Config as SlasherConfig, Slasher}; -use state_processing::per_slot_processing; +use state_processing::{GloasVerificationContext, per_slot_processing}; use std::sync::{Arc, LazyLock}; use tempfile::tempdir; use tree_hash::TreeHash; @@ -1217,7 +1217,7 @@ async fn attestation_that_skips_epochs() { .expect("should find state"); while state.slot() < current_slot { - per_slot_processing(&mut state, None, &harness.spec).expect("should process slot"); + per_slot_processing(&mut state, None, GloasVerificationContext::FullVerification, &harness.spec).expect("should process slot"); } let state_root = state.update_tree_hash_cache().unwrap(); @@ -1328,7 +1328,7 @@ async fn attestation_validator_receive_proposer_reward_and_withdrawals() { .expect("should find state"); while state.slot() < current_slot { - per_slot_processing(&mut state, None, &harness.spec).expect("should process slot"); + per_slot_processing(&mut state, None, GloasVerificationContext::FullVerification, &harness.spec).expect("should process slot"); } let state_root = state.update_tree_hash_cache().unwrap(); @@ -1405,7 +1405,7 @@ async fn attestation_to_finalized_block() { .expect("should find state"); while state.slot() < current_slot { - per_slot_processing(&mut state, None, &harness.spec).expect("should process slot"); + per_slot_processing(&mut state, None, GloasVerificationContext::FullVerification, &harness.spec).expect("should process slot"); } let state_root = state.update_tree_hash_cache().unwrap(); diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 87d7b8170ad..45f2e2d90c6 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -20,8 +20,8 @@ use fixed_bytes::FixedBytesExtended; use logging::create_test_tracing_subscriber; use slasher::{Config as SlasherConfig, Slasher}; use state_processing::{ - BlockProcessingError, BlockSignatureStrategy, ConsensusContext, VerifyBlockRoot, - common::{attesting_indices_base, attesting_indices_electra}, + BlockProcessingError, BlockSignatureStrategy, ConsensusContext, GloasVerificationContext, + VerifyBlockRoot, common::{attesting_indices_base, attesting_indices_electra}, per_block_processing, per_slot_processing, }; use std::marker::PhantomData; @@ -1696,7 +1696,7 @@ async fn add_base_block_to_altair_chain() { { let mut state = state; let mut ctxt = ConsensusContext::new(base_block.slot()); - per_slot_processing(&mut state, None, None, &harness.chain.spec).unwrap(); + per_slot_processing(&mut state, None, GloasVerificationContext::FullVerification, &harness.chain.spec).unwrap(); assert!(matches!( per_block_processing( &mut state, @@ -1848,7 +1848,7 @@ async fn add_altair_block_to_base_chain() { { let mut state = state; let mut ctxt = ConsensusContext::new(altair_block.slot()); - per_slot_processing(&mut state, None, None, &harness.chain.spec).unwrap(); + per_slot_processing(&mut state, None, GloasVerificationContext::FullVerification, &harness.chain.spec).unwrap(); assert!(matches!( per_block_processing( &mut state, diff --git a/beacon_node/beacon_chain/tests/tests.rs b/beacon_node/beacon_chain/tests/tests.rs index 3958ce6c6df..8cc568dcb1b 100644 --- a/beacon_node/beacon_chain/tests/tests.rs +++ b/beacon_node/beacon_chain/tests/tests.rs @@ -12,7 +12,9 @@ use beacon_chain::{ use bls::Keypair; use operation_pool::PersistedOperationPool; use state_processing::EpochProcessingError; -use state_processing::{per_slot_processing, per_slot_processing::Error as SlotProcessingError}; +use state_processing::{ + GloasVerificationContext, per_slot_processing, per_slot_processing::Error as SlotProcessingError, +}; use std::sync::LazyLock; use types::{ BeaconState, BeaconStateError, BlockImportSource, ChainSpec, Checkpoint, @@ -107,7 +109,7 @@ fn massive_skips() { // Run per_slot_processing until it returns an error. let error = loop { - match per_slot_processing(&mut state, None, spec) { + match per_slot_processing(&mut state, None, GloasVerificationContext::FullVerification, spec) { Ok(_) => continue, Err(e) => break e, } diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index a7fe34593a7..b012378cb22 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -40,7 +40,7 @@ use sensitive_url::SensitiveUrl; use slot_clock::SlotClock; use ssz::{BitList, Decode}; use state_processing::per_block_processing::get_expected_withdrawals; -use state_processing::per_slot_processing; +use state_processing::{GloasVerificationContext, per_slot_processing}; use state_processing::state_advance::partial_state_advance; use std::convert::TryInto; use std::sync::Arc; @@ -5011,7 +5011,7 @@ impl ApiTester { let mut head = self.chain.head_snapshot().as_ref().clone(); while head.beacon_state.current_epoch() < epoch { - per_slot_processing(&mut head.beacon_state, None, &self.chain.spec).unwrap(); + per_slot_processing(&mut head.beacon_state, None, GloasVerificationContext::FullVerification, &self.chain.spec).unwrap(); } head.beacon_state .build_committee_cache(RelativeEpoch::Current, &self.chain.spec) diff --git a/beacon_node/store/src/reconstruct.rs b/beacon_node/store/src/reconstruct.rs index 0c62212c132..fd20591f9e6 100644 --- a/beacon_node/store/src/reconstruct.rs +++ b/beacon_node/store/src/reconstruct.rs @@ -5,8 +5,8 @@ use crate::metrics; use crate::{DBColumn, Error, ItemStore}; use itertools::{Itertools, process_results}; use state_processing::{ - BlockSignatureStrategy, ConsensusContext, VerifyBlockRoot, per_block_processing, - per_slot_processing, + BlockSignatureStrategy, ConsensusContext, GloasVerificationContext, VerifyBlockRoot, + per_block_processing, per_slot_processing, }; use std::sync::Arc; use tracing::{debug, info}; @@ -145,7 +145,7 @@ where }; // Advance state to slot. - per_slot_processing(&mut state, prev_state_root.take(), None, &self.spec) + per_slot_processing(&mut state, prev_state_root.take(), GloasVerificationContext::FullVerification, &self.spec) .map_err(HotColdDBError::BlockReplaySlotError)?; // Apply block. diff --git a/consensus/state_processing/src/block_replayer.rs b/consensus/state_processing/src/block_replayer.rs index 17b97fb8b83..31e21beb084 100644 --- a/consensus/state_processing/src/block_replayer.rs +++ b/consensus/state_processing/src/block_replayer.rs @@ -1,7 +1,7 @@ use crate::{ - BlockProcessingError, BlockSignatureStrategy, ConsensusContext, SlotProcessingError, - VerifyBlockRoot, per_block_processing, per_epoch_processing::EpochProcessingSummary, - per_slot_processing, + BlockProcessingError, BlockSignatureStrategy, ConsensusContext, GloasVerificationContext, + SlotProcessingError, VerifyBlockRoot, per_block_processing, + per_epoch_processing::EpochProcessingSummary, per_slot_processing, }; use itertools::Itertools; use std::iter::Peekable; @@ -231,7 +231,7 @@ where } let summary = - per_slot_processing(&mut self.state, Some(state_root), None, self.spec) + per_slot_processing(&mut self.state, Some(state_root), GloasVerificationContext::FullVerification, self.spec) .map_err(BlockReplayError::from)?; if let Some(ref mut post_slot_hook) = self.post_slot_hook { @@ -279,7 +279,7 @@ where } let summary = - per_slot_processing(&mut self.state, Some(state_root), None, self.spec) + per_slot_processing(&mut self.state, Some(state_root), GloasVerificationContext::FullVerification, self.spec) .map_err(BlockReplayError::from)?; if let Some(ref mut post_slot_hook) = self.post_slot_hook { diff --git a/consensus/state_processing/src/genesis.rs b/consensus/state_processing/src/genesis.rs index f945cc40761..ff3f1d7d2fd 100644 --- a/consensus/state_processing/src/genesis.rs +++ b/consensus/state_processing/src/genesis.rs @@ -4,8 +4,8 @@ use super::per_block_processing::{ use crate::common::DepositDataTree; use crate::upgrade::electra::upgrade_state_to_electra; use crate::upgrade::{ - upgrade_to_altair, upgrade_to_bellatrix, upgrade_to_capella, upgrade_to_deneb, upgrade_to_fulu, - upgrade_to_gloas, + GloasVerificationContext, upgrade_to_altair, upgrade_to_bellatrix, upgrade_to_capella, + upgrade_to_deneb, upgrade_to_fulu, upgrade_to_gloas, }; use fixed_bytes::FixedBytesExtended; use safe_arith::{ArithError, SafeArith}; @@ -162,7 +162,7 @@ pub fn initialize_beacon_state_from_eth1( .gloas_fork_epoch .is_some_and(|fork_epoch| fork_epoch == E::genesis_epoch()) { - upgrade_to_gloas(&mut state, None, spec)?; + upgrade_to_gloas(&mut state, GloasVerificationContext::FullVerification, spec)?; // Remove intermediate Fulu fork from `state.fork`. state.fork_mut().previous_version = spec.gloas_fork_version; diff --git a/consensus/state_processing/src/lib.rs b/consensus/state_processing/src/lib.rs index 46f9d29dc57..a7d821f169e 100644 --- a/consensus/state_processing/src/lib.rs +++ b/consensus/state_processing/src/lib.rs @@ -46,5 +46,6 @@ pub use per_epoch_processing::{ errors::EpochProcessingError, process_epoch as per_epoch_processing, }; pub use per_slot_processing::{Error as SlotProcessingError, per_slot_processing}; +pub use upgrade::GloasVerificationContext; pub use types::{EpochCache, EpochCacheError, EpochCacheKey}; pub use verify_operation::{SigVerifiedOp, TransformPersist, VerifyOperation, VerifyOperationAt}; diff --git a/consensus/state_processing/src/per_epoch_processing/tests.rs b/consensus/state_processing/src/per_epoch_processing/tests.rs index bde94425484..cc8b824a679 100644 --- a/consensus/state_processing/src/per_epoch_processing/tests.rs +++ b/consensus/state_processing/src/per_epoch_processing/tests.rs @@ -37,7 +37,8 @@ async fn runs_without_error() { mod release_tests { use super::*; use crate::{ - EpochProcessingError, SlotProcessingError, per_slot_processing::per_slot_processing, + EpochProcessingError, GloasVerificationContext, SlotProcessingError, + per_slot_processing::per_slot_processing, }; use beacon_chain::test_utils::{AttestationStrategy, BlockStrategy}; use std::sync::Arc; @@ -81,7 +82,7 @@ mod release_tests { // Check the state is valid before starting this test. process_epoch(&mut altair_state.clone(), &spec) .expect("state passes intial epoch processing"); - per_slot_processing(&mut altair_state.clone(), None, None, &spec) + per_slot_processing(&mut altair_state.clone(), None, GloasVerificationContext::FullVerification, &spec) .expect("state passes intial slot processing"); // Modify the spec so altair never happens. @@ -98,7 +99,7 @@ mod release_tests { Err(EpochProcessingError::InconsistentStateFork(expected_err)) ); assert_eq!( - per_slot_processing(&mut altair_state.clone(), None, None, &spec), + per_slot_processing(&mut altair_state.clone(), None, GloasVerificationContext::FullVerification, &spec), Err(SlotProcessingError::InconsistentStateFork(expected_err)) ); } @@ -141,7 +142,7 @@ mod release_tests { // Check the state is valid before starting this test. process_epoch(&mut base_state.clone(), &spec) .expect("state passes intial epoch processing"); - per_slot_processing(&mut base_state.clone(), None, None, &spec) + per_slot_processing(&mut base_state.clone(), None, GloasVerificationContext::FullVerification, &spec) .expect("state passes intial slot processing"); // Modify the spec so Altair happens at the first epoch. @@ -158,7 +159,7 @@ mod release_tests { Err(EpochProcessingError::InconsistentStateFork(expected_err)) ); assert_eq!( - per_slot_processing(&mut base_state.clone(), None, None, &spec), + per_slot_processing(&mut base_state.clone(), None, GloasVerificationContext::FullVerification, &spec), Err(SlotProcessingError::InconsistentStateFork(expected_err)) ); } diff --git a/consensus/state_processing/src/per_slot_processing.rs b/consensus/state_processing/src/per_slot_processing.rs index b604ea39202..024e9be75c0 100644 --- a/consensus/state_processing/src/per_slot_processing.rs +++ b/consensus/state_processing/src/per_slot_processing.rs @@ -1,8 +1,8 @@ -use crate::builder_deposits_cache::OnboardBuildersCache; use crate::upgrade::{ upgrade_to_altair, upgrade_to_bellatrix, upgrade_to_capella, upgrade_to_deneb, upgrade_to_electra, upgrade_to_fulu, upgrade_to_gloas, }; +use crate::upgrade::gloas::GloasVerificationContext; use crate::{per_epoch_processing::EpochProcessingSummary, *}; use fixed_bytes::FixedBytesExtended; use safe_arith::{ArithError, SafeArith}; @@ -39,7 +39,7 @@ impl From for Error { pub fn per_slot_processing( state: &mut BeaconState, state_root: Option, - builder_onboarding_cache: Option<&OnboardBuildersCache>, + gloas_context: GloasVerificationContext, spec: &ChainSpec, ) -> Result>, Error> { // Verify that the `BeaconState` instantiation matches the fork at `state.slot()`. @@ -102,7 +102,7 @@ pub fn per_slot_processing( // Gloas. if spec.gloas_fork_epoch == Some(state.current_epoch()) { - upgrade_to_gloas(state, builder_onboarding_cache, spec)?; + upgrade_to_gloas(state, gloas_context, spec)?; } // Additionally build all caches so that all valid states that are advanced always have diff --git a/consensus/state_processing/src/state_advance.rs b/consensus/state_processing/src/state_advance.rs index fd057b101a4..c2f5b195c51 100644 --- a/consensus/state_processing/src/state_advance.rs +++ b/consensus/state_processing/src/state_advance.rs @@ -5,6 +5,7 @@ //! duplication and protect against some easy-to-make mistakes when performing state advances. use crate::*; +use crate::upgrade::GloasVerificationContext; use fixed_bytes::FixedBytesExtended; use tracing::instrument; use types::{BeaconState, ChainSpec, EthSpec, Hash256, Slot}; @@ -40,7 +41,7 @@ pub fn complete_state_advance( // future iterations. let state_root_opt = state_root_opt.take(); - per_slot_processing(state, state_root_opt, None, spec) + per_slot_processing(state, state_root_opt, GloasVerificationContext::FullVerification, spec) .map_err(Error::PerSlotProcessing)?; } @@ -96,7 +97,7 @@ pub fn partial_state_advance( // with the correct state root. let state_root = initial_state_root.take().unwrap_or_else(Hash256::zero); - per_slot_processing(state, Some(state_root), None, spec) + per_slot_processing(state, Some(state_root), GloasVerificationContext::CommitteesOnly, spec) .map_err(Error::PerSlotProcessing)?; } diff --git a/consensus/state_processing/src/upgrade.rs b/consensus/state_processing/src/upgrade.rs index d175c3ae408..5f516e79911 100644 --- a/consensus/state_processing/src/upgrade.rs +++ b/consensus/state_processing/src/upgrade.rs @@ -12,4 +12,4 @@ pub use capella::upgrade_to_capella; pub use deneb::upgrade_to_deneb; pub use electra::upgrade_to_electra; pub use fulu::upgrade_to_fulu; -pub use gloas::upgrade_to_gloas; +pub use gloas::{GloasVerificationContext, upgrade_to_gloas}; diff --git a/consensus/state_processing/src/upgrade/gloas.rs b/consensus/state_processing/src/upgrade/gloas.rs index ad296e67993..144a192cccf 100644 --- a/consensus/state_processing/src/upgrade/gloas.rs +++ b/consensus/state_processing/src/upgrade/gloas.rs @@ -8,6 +8,7 @@ use safe_arith::SafeArith; use ssz_types::BitVector; use ssz_types::FixedVector; use std::mem; +use std::sync::Arc; use tree_hash::TreeHash; use typenum::Unsigned; use types::{ @@ -15,13 +16,45 @@ use types::{ EthSpec, ExecutionPayloadBid, ExecutionRequests, Fork, is_builder_withdrawal_credential, }; +/// Controls how builder onboarding is handled during the Gloas fork `upgrade_to_gloas`. +/// +/// This is also useful for caching builder deposit signatures post gloas so that +/// potentially expensive operations doesn't slow down block processing in the block verification +/// path. +pub enum GloasVerificationContext { + /// Use the pre-computed builder signature cache. + CachedVerification(Arc), + /// Verify each builder deposit signature individually. + /// + /// This can be significantly slower if there are many builder deposits + /// that need to be onboarded at the fork boundary. This variant should be used + /// for tests and other non-production paths. + FullVerification, + /// Skip `onboard_builders_from_pending_deposits` entirely which is sufficient for + /// paths that do a state advance only for committee calculation. + /// + /// `onboard_builders_from_pending_deposits` only modifies `state.pending_deposits` and + /// `state.builders` which don't affect committee calculation. + /// TODO(pawan): triple check this is true. + CommitteesOnly, +} + +impl GloasVerificationContext { + pub fn from_cache(cache: Option>) -> Self { + match cache { + Some(c) => GloasVerificationContext::CachedVerification(c), + None => GloasVerificationContext::FullVerification, + } + } +} + /// Transform a `Fulu` state into a `Gloas` state. pub fn upgrade_to_gloas( pre_state: &mut BeaconState, - builder_onboarding_cache: Option<&OnboardBuildersCache>, + context: GloasVerificationContext, spec: &ChainSpec, ) -> Result<(), Error> { - let post = upgrade_state_to_gloas(pre_state, builder_onboarding_cache, spec)?; + let post = upgrade_state_to_gloas(pre_state, context, spec)?; *pre_state = post; @@ -30,7 +63,7 @@ pub fn upgrade_to_gloas( pub fn upgrade_state_to_gloas( pre_state: &mut BeaconState, - builder_onboarding_cache: Option<&OnboardBuildersCache>, + context: GloasVerificationContext, spec: &ChainSpec, ) -> Result, Error> { let epoch = pre_state.current_epoch(); @@ -124,8 +157,20 @@ pub fn upgrade_state_to_gloas( epoch_cache: mem::take(&mut pre.epoch_cache), }); // [New in Gloas:EIP7732] - onboard_builders_from_pending_deposits(&mut post, builder_onboarding_cache, spec)?; - initialize_ptc_window(&mut post, spec)?; + match context { + GloasVerificationContext::CachedVerification(ref cache) => { + onboard_builders_from_pending_deposits(&mut post, Some(cache.as_ref()), spec)?; + initialize_ptc_window(&mut post, spec)?; + } + GloasVerificationContext::FullVerification => { + onboard_builders_from_pending_deposits(&mut post, None, spec)?; + initialize_ptc_window(&mut post, spec)?; + } + GloasVerificationContext::CommitteesOnly => { + // TODO(pawan): check if `initialize_ptc_window` can be done here. + // In some isolated benchmarks, it took 300ms, but that seems too high. + } + } Ok(post) } diff --git a/testing/ef_tests/src/cases/fork.rs b/testing/ef_tests/src/cases/fork.rs index 6e483dc041d..fea4969da15 100644 --- a/testing/ef_tests/src/cases/fork.rs +++ b/testing/ef_tests/src/cases/fork.rs @@ -3,8 +3,8 @@ use crate::case_result::compare_beacon_state_results_without_caches; use crate::decode::{ssz_decode_state, yaml_decode_file}; use serde::Deserialize; use state_processing::upgrade::{ - upgrade_to_altair, upgrade_to_bellatrix, upgrade_to_capella, upgrade_to_deneb, - upgrade_to_electra, upgrade_to_fulu, upgrade_to_gloas, + GloasVerificationContext, upgrade_to_altair, upgrade_to_bellatrix, upgrade_to_capella, + upgrade_to_deneb, upgrade_to_electra, upgrade_to_fulu, upgrade_to_gloas, }; use types::BeaconState; @@ -73,7 +73,7 @@ impl Case for ForkTest { ForkName::Electra => upgrade_to_electra(&mut result_state, spec).map(|_| result_state), ForkName::Fulu => upgrade_to_fulu(&mut result_state, spec).map(|_| result_state), ForkName::Gloas => { - upgrade_to_gloas(&mut result_state, None, spec).map(|_| result_state) + upgrade_to_gloas(&mut result_state, GloasVerificationContext::FullVerification, spec).map(|_| result_state) } }; diff --git a/testing/ef_tests/src/cases/sanity_blocks.rs b/testing/ef_tests/src/cases/sanity_blocks.rs index ea35935452f..3495b1c1566 100644 --- a/testing/ef_tests/src/cases/sanity_blocks.rs +++ b/testing/ef_tests/src/cases/sanity_blocks.rs @@ -4,8 +4,8 @@ use crate::case_result::compare_beacon_state_results_without_caches; use crate::decode::{ssz_decode_file_with, ssz_decode_state, yaml_decode_file}; use serde::Deserialize; use state_processing::{ - BlockProcessingError, BlockSignatureStrategy, ConsensusContext, VerifyBlockRoot, - per_block_processing, per_slot_processing, + BlockProcessingError, BlockSignatureStrategy, ConsensusContext, GloasVerificationContext, + VerifyBlockRoot, per_block_processing, per_slot_processing, }; use types::{BeaconState, RelativeEpoch, SignedBeaconBlock}; @@ -79,8 +79,8 @@ impl Case for SanityBlocks { .try_for_each(|signed_block| { let block = signed_block.message(); while bulk_state.slot() < block.slot() { - per_slot_processing(&mut bulk_state, None, None, spec).unwrap(); - per_slot_processing(&mut indiv_state, None, None, spec).unwrap(); + per_slot_processing(&mut bulk_state, None, GloasVerificationContext::FullVerification, spec).unwrap(); + per_slot_processing(&mut indiv_state, None, GloasVerificationContext::FullVerification, spec).unwrap(); } bulk_state diff --git a/testing/ef_tests/src/cases/sanity_slots.rs b/testing/ef_tests/src/cases/sanity_slots.rs index fa444917582..8f737f7d6fc 100644 --- a/testing/ef_tests/src/cases/sanity_slots.rs +++ b/testing/ef_tests/src/cases/sanity_slots.rs @@ -3,7 +3,7 @@ use crate::bls_setting::BlsSetting; use crate::case_result::compare_beacon_state_results_without_caches; use crate::decode::{ssz_decode_state, yaml_decode_file}; use serde::Deserialize; -use state_processing::per_slot_processing; +use state_processing::{GloasVerificationContext, per_slot_processing}; use types::BeaconState; #[derive(Debug, Clone, Default, Deserialize)] @@ -64,7 +64,7 @@ impl Case for SanitySlots { state.build_caches(spec).unwrap(); let mut result = (0..self.slots) - .try_for_each(|_| per_slot_processing(&mut state, None, None, spec).map(|_| ())) + .try_for_each(|_| per_slot_processing(&mut state, None, GloasVerificationContext::FullVerification, spec).map(|_| ())) .map(|_| state); compare_beacon_state_results_without_caches(&mut result, &mut expected) From 18bcc249d832d3a7b968be32df7d97a3a633d559 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Sat, 16 May 2026 18:52:05 -0700 Subject: [PATCH 07/22] Add a hashmap for keeping track of added builders --- .../state_processing/src/upgrade/gloas.rs | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/consensus/state_processing/src/upgrade/gloas.rs b/consensus/state_processing/src/upgrade/gloas.rs index 144a192cccf..1fb99c5f516 100644 --- a/consensus/state_processing/src/upgrade/gloas.rs +++ b/consensus/state_processing/src/upgrade/gloas.rs @@ -7,13 +7,16 @@ use milhouse::{List, Vector}; use safe_arith::SafeArith; use ssz_types::BitVector; use ssz_types::FixedVector; +use std::collections::HashMap; use std::mem; use std::sync::Arc; use tree_hash::TreeHash; use typenum::Unsigned; +use bls::PublicKeyBytes; use types::{ BeaconState, BeaconStateError as Error, BeaconStateGloas, BuilderPendingPayment, ChainSpec, - EthSpec, ExecutionPayloadBid, ExecutionRequests, Fork, is_builder_withdrawal_credential, + EthSpec, ExecutionPayloadBid, ExecutionRequests, Fork, + is_builder_withdrawal_credential, }; /// Controls how builder onboarding is handled during the Gloas fork `upgrade_to_gloas`. @@ -215,26 +218,20 @@ fn onboard_builders_from_pending_deposits( builder_onboarding_cache: Option<&OnboardBuildersCache>, spec: &ChainSpec, ) -> Result<(), Error> { - // Clone pending deposits to avoid borrow conflicts when mutating state. let current_pending_deposits = state.pending_deposits()?.clone(); + // A temporary lookup for builders that are added in the loop. + // TODO(gloas): can be removed if we end up creating a builder pubkey cache. + let mut builder_pubkey_to_index: HashMap = HashMap::new(); let mut pending_deposits = List::empty(); for deposit in ¤t_pending_deposits { - // Deposits for existing validators stay in the pending queue. if state.get_validator_index(&deposit.pubkey)?.is_some() { pending_deposits.push(deposit.clone())?; continue; } - // Re-scan builder list each iteration because `apply_deposit_for_builder` may add - // new builders to the registry. - // TODO(gloas): this linear scan could be optimized, see: - // https://github.com/sigp/lighthouse/issues/8783 - let builder_index = state - .builders()? - .iter() - .position(|b| b.pubkey == deposit.pubkey); + let builder_index = builder_pubkey_to_index.get(&deposit.pubkey).copied(); // Equivalent to if deposit.pubkey not in builder_pubkeys: if builder_index.is_none() { @@ -245,15 +242,21 @@ fn onboard_builders_from_pending_deposits( continue; } } - let builder_index_opt = builder_index.map(|i| i as u64); + let verify_signature = if let Some(cache) = builder_onboarding_cache { cache.cached_is_valid_signature(deposit).into() } else { VerifyBuilderSignature::Verify }; + + if builder_index.is_none() { + let next_index = state.builders()?.len() as u64; + builder_pubkey_to_index.insert(deposit.pubkey, next_index); + } + apply_deposit_for_builder( state, - builder_index_opt, + builder_index, deposit.pubkey, deposit.withdrawal_credentials, deposit.amount, From 826a521c495773129e5b3706124251ddc73de7de Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Sat, 16 May 2026 19:48:44 -0700 Subject: [PATCH 08/22] Update onboarding cache size --- consensus/state_processing/src/builder_deposits_cache.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/consensus/state_processing/src/builder_deposits_cache.rs b/consensus/state_processing/src/builder_deposits_cache.rs index be1916292c8..4e4b0d13dbe 100644 --- a/consensus/state_processing/src/builder_deposits_cache.rs +++ b/consensus/state_processing/src/builder_deposits_cache.rs @@ -11,7 +11,7 @@ use types::{ use std::num::NonZeroUsize; // TODO(pawan): analyze size of cache -const CACHE_SIZE: NonZeroUsize = new_non_zero_usize(64000); +const CACHE_SIZE: NonZeroUsize = new_non_zero_usize(1<<18); pub struct OnboardBuildersCache { cache: Mutex>, From 49410fa30f25604507197a2e6c20b0d3898580c3 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Sat, 16 May 2026 20:26:31 -0700 Subject: [PATCH 09/22] Add better docs --- .../src/builder_deposits_cache.rs | 34 +++++- .../state_processing/src/upgrade/gloas.rs | 108 ++++++++++++------ 2 files changed, 105 insertions(+), 37 deletions(-) diff --git a/consensus/state_processing/src/builder_deposits_cache.rs b/consensus/state_processing/src/builder_deposits_cache.rs index 4e4b0d13dbe..bd5cc73a6af 100644 --- a/consensus/state_processing/src/builder_deposits_cache.rs +++ b/consensus/state_processing/src/builder_deposits_cache.rs @@ -1,7 +1,7 @@ use crate::per_block_processing::is_valid_deposit_signature_batch; use lru::LruCache; use parking_lot::Mutex; -use tracing::debug; +use tracing::{debug, instrument}; use tree_hash::{Hash256, TreeHash}; use types::{ BeaconState, ChainSpec, DepositData, DepositRequest, EthSpec, PendingDeposit, @@ -10,14 +10,37 @@ use types::{ use std::num::NonZeroUsize; -// TODO(pawan): analyze size of cache -const CACHE_SIZE: NonZeroUsize = new_non_zero_usize(1<<18); - +/// This is a very high limit to enable worst case testing. +/// The actual lru storage ends up being quite small in the worst case. +/// +/// The internal hashmap representation would take 32 bytes for key + 1 byte for the bool. +/// With additional overhead in the LRU cache, each entry would come out to ~88 bytes. +/// +/// So worst case storage is ~25MB. +/// +/// In practice, current mainnet gas limits would not allow more than a couple hundred deposits +/// every slot. +const CACHE_SIZE: NonZeroUsize = new_non_zero_usize(262144); + + +/// A simple cache that performs signature verification on `PendingDeposit` entries in the +/// beacon state for 0x03 credentials and caches the result. +/// +/// The key is the hash_tree_root of the `Deposit` and the value is the verification result. +/// In gloas, there are 2 places where we need to do bulk signature verification: +/// 1. `onboard_builders_from_pending_deposits` in `upgrade_to_gloas` happens at the fork transition. +/// If the `pending_deposits` queue at fork has many signatures to verify, then verifying them +/// in the hot path could be very expensive. +/// 2. `process_deposit_requests_post_gloas` in `process_operations` can contain upto 8192 signatures to +/// verify in the hot block verification path based on limits today. Since the deposits are received a +/// couple seconds before the actual deposits processing, we can cache those signatures too to ensure +/// the deposit processing is just a lookup operation in the worst case. pub struct OnboardBuildersCache { cache: Mutex>, } impl OnboardBuildersCache { + /// Returns `None` if gloas is not scheduled currently. pub fn new(spec: &ChainSpec) -> Option { if spec.is_gloas_scheduled() { Some(Self { @@ -33,6 +56,7 @@ impl OnboardBuildersCache { /// /// Further block imports that result in additional deposits should be handled by the /// [`Self::add_new_pending_deposits`] method. + #[instrument(skip_all)] pub fn seed_from_state(&self, state: &BeaconState, spec: &ChainSpec) { let Some(pending_deposits) = state.pending_deposits().ok() else { return; @@ -52,6 +76,7 @@ impl OnboardBuildersCache { /// Gets the new deposits added to the `pending_cache` for `state.slot. /// Signature verifies and caches them for later use. + #[instrument(skip_all)] pub fn add_new_pending_deposits( &self, current_state: &BeaconState, @@ -117,6 +142,7 @@ impl OnboardBuildersCache { /// Pre-verifies builder deposit signatures from execution payload deposit requests /// and caches the results for later use during `process_deposit_requests_post_gloas`. + #[instrument(skip_all)] pub fn cache_deposit_requests(&self, deposit_requests: &[DepositRequest], spec: &ChainSpec) { let mut builder_deposits = Vec::new(); let mut builder_deposit_keys = Vec::new(); diff --git a/consensus/state_processing/src/upgrade/gloas.rs b/consensus/state_processing/src/upgrade/gloas.rs index 1fb99c5f516..e34da454d4d 100644 --- a/consensus/state_processing/src/upgrade/gloas.rs +++ b/consensus/state_processing/src/upgrade/gloas.rs @@ -1,8 +1,7 @@ use crate::builder_deposits_cache::OnboardBuildersCache; -use crate::per_block_processing::process_operations::{ - VerifyBuilderSignature, is_pending_validator, -}; -use crate::per_block_processing::process_operations::apply_deposit_for_builder; +use crate::per_block_processing::is_valid_deposit_signature; +use crate::per_block_processing::process_operations::is_pending_validator; +use bls::PublicKeyBytes; use milhouse::{List, Vector}; use safe_arith::SafeArith; use ssz_types::BitVector; @@ -10,13 +9,13 @@ use ssz_types::FixedVector; use std::collections::HashMap; use std::mem; use std::sync::Arc; +use tracing::instrument; use tree_hash::TreeHash; use typenum::Unsigned; -use bls::PublicKeyBytes; use types::{ - BeaconState, BeaconStateError as Error, BeaconStateGloas, BuilderPendingPayment, ChainSpec, - EthSpec, ExecutionPayloadBid, ExecutionRequests, Fork, - is_builder_withdrawal_credential, + Address, BeaconState, BeaconStateError as Error, BeaconStateGloas, Builder, + BuilderPendingPayment, ChainSpec, DepositData, EthSpec, ExecutionPayloadBid, ExecutionRequests, + Fork, is_builder_withdrawal_credential, }; /// Controls how builder onboarding is handled during the Gloas fork `upgrade_to_gloas`. @@ -52,6 +51,7 @@ impl GloasVerificationContext { } /// Transform a `Fulu` state into a `Gloas` state. +#[instrument(skip_all)] pub fn upgrade_to_gloas( pre_state: &mut BeaconState, context: GloasVerificationContext, @@ -169,10 +169,7 @@ pub fn upgrade_state_to_gloas( onboard_builders_from_pending_deposits(&mut post, None, spec)?; initialize_ptc_window(&mut post, spec)?; } - GloasVerificationContext::CommitteesOnly => { - // TODO(pawan): check if `initialize_ptc_window` can be done here. - // In some isolated benchmarks, it took 300ms, but that seems too high. - } + GloasVerificationContext::CommitteesOnly => {} } Ok(post) @@ -183,6 +180,7 @@ pub fn upgrade_state_to_gloas( /// The window contains: /// - One epoch of empty entries (previous epoch) /// - Computed PTC for the current epoch through `1 + MIN_SEED_LOOKAHEAD` epochs +#[instrument(skip_all)] fn initialize_ptc_window( state: &mut BeaconState, spec: &ChainSpec, @@ -213,6 +211,10 @@ fn initialize_ptc_window( } /// Applies any pending deposit for builders, effectively onboarding builders at the fork. +/// +/// This function is optimised to use the builder onboarding cache which signature verifies +/// and caches all 0x03 `PendingDeposits` before the fork to ensure that the fork upgrade is fast. +#[instrument(skip_all)] fn onboard_builders_from_pending_deposits( state: &mut BeaconState, builder_onboarding_cache: Option<&OnboardBuildersCache>, @@ -220,9 +222,9 @@ fn onboard_builders_from_pending_deposits( ) -> Result<(), Error> { let current_pending_deposits = state.pending_deposits()?.clone(); - // A temporary lookup for builders that are added in the loop. - // TODO(gloas): can be removed if we end up creating a builder pubkey cache. + // At fork time the builders list is empty and all deposits are new registrations. let mut builder_pubkey_to_index: HashMap = HashMap::new(); + let mut builders_vec: Vec = Vec::with_capacity(current_pending_deposits.len()); let mut pending_deposits = List::empty(); for deposit in ¤t_pending_deposits { @@ -233,7 +235,6 @@ fn onboard_builders_from_pending_deposits( let builder_index = builder_pubkey_to_index.get(&deposit.pubkey).copied(); - // Equivalent to if deposit.pubkey not in builder_pubkeys: if builder_index.is_none() { if !is_builder_withdrawal_credential(deposit.withdrawal_credentials, spec) || is_pending_validator::(&pending_deposits, &deposit.pubkey, spec) @@ -243,30 +244,71 @@ fn onboard_builders_from_pending_deposits( } } - let verify_signature = if let Some(cache) = builder_onboarding_cache { - cache.cached_is_valid_signature(deposit).into() + // Use the builder onboarding cache to get the signature verification result. + let is_valid_signature = if let Some(cache) = builder_onboarding_cache { + cache.cached_is_valid_signature(deposit) } else { - VerifyBuilderSignature::Verify + None }; - if builder_index.is_none() { - let next_index = state.builders()?.len() as u64; - builder_pubkey_to_index.insert(deposit.pubkey, next_index); - } + // Note: this is a deviation from the spec. The spec simply calls `state.add_builder_to_registry`. + // `state.add_builder_to_registry` adds the deposits sequentially + // with `milhouse::List::push`. For a high number of deposits, this get significantly slower, + // since its a O(nlogn) operation. + match builder_index { + Some(idx) => { + // Top-up existing builder. + builders_vec + .get_mut(idx as usize) + .ok_or(Error::UnknownBuilder(idx))? + .balance + .safe_add_assign(deposit.amount)?; + } + None => { + // New builder registration — verify signature then add to vec. + let valid = match is_valid_signature { + Some(v) => v, + None => { + let deposit_data = DepositData { + pubkey: deposit.pubkey, + withdrawal_credentials: deposit.withdrawal_credentials, + amount: deposit.amount, + signature: deposit.signature.clone(), + }; + is_valid_deposit_signature(&deposit_data, spec).is_ok() + } + }; + + if valid { + let next_index = builders_vec.len() as u64; + builder_pubkey_to_index.insert(deposit.pubkey, next_index); - apply_deposit_for_builder( - state, - builder_index, - deposit.pubkey, - deposit.withdrawal_credentials, - deposit.amount, - deposit.signature.clone(), - deposit.slot, - verify_signature, - spec, - )?; + let version = *deposit + .withdrawal_credentials + .as_slice() + .first() + .ok_or(Error::WithdrawalCredentialMissingVersion)?; + let execution_address = deposit + .withdrawal_credentials + .as_slice() + .get(12..) + .and_then(|bytes| Address::try_from(bytes).ok()) + .ok_or(Error::WithdrawalCredentialMissingAddress)?; + + builders_vec.push(Builder { + pubkey: deposit.pubkey, + version, + execution_address, + balance: deposit.amount, + deposit_epoch: deposit.slot.epoch(E::slots_per_epoch()), + withdrawable_epoch: spec.far_future_epoch, + }); + } + } + } } + *state.builders_mut()? = List::new(builders_vec)?; *state.pending_deposits_mut()? = pending_deposits; Ok(()) From e2e8406ede4f0b8f002f8366702f94853720d32c Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Sat, 16 May 2026 20:26:55 -0700 Subject: [PATCH 10/22] fmt --- beacon_node/beacon_chain/src/beacon_chain.rs | 3 +- .../beacon_chain/src/block_verification.rs | 7 +++- beacon_node/beacon_chain/src/builder.rs | 9 ++++-- .../payload_envelope_verification/import.rs | 6 +++- .../beacon_chain/src/state_advance_timer.rs | 10 ++++-- .../tests/attestation_verification.rs | 24 ++++++++++++-- .../beacon_chain/tests/block_verification.rs | 19 +++++++++-- beacon_node/beacon_chain/tests/tests.rs | 10 ++++-- beacon_node/http_api/tests/tests.rs | 10 ++++-- beacon_node/store/src/reconstruct.rs | 9 ++++-- .../state_processing/src/block_replayer.rs | 20 ++++++++---- .../src/builder_deposits_cache.rs | 1 - consensus/state_processing/src/lib.rs | 2 +- .../src/per_block_processing/tests.rs | 13 +++----- .../src/per_epoch_processing/tests.rs | 32 +++++++++++++++---- .../src/per_slot_processing.rs | 2 +- .../state_processing/src/state_advance.rs | 20 +++++++++--- testing/ef_tests/src/cases/fork.rs | 9 ++++-- testing/ef_tests/src/cases/sanity_blocks.rs | 16 ++++++++-- testing/ef_tests/src/cases/sanity_slots.rs | 10 +++++- 20 files changed, 177 insertions(+), 55 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index ecce8c369eb..bd4edc8cad4 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -121,7 +121,8 @@ use slot_clock::SlotClock; use ssz::Encode; use state_processing::{ BlockSignatureStrategy, ConsensusContext, GloasVerificationContext, SigVerifiedOp, - VerifyBlockRoot, VerifyOperation, builder_deposits_cache::OnboardBuildersCache, + VerifyBlockRoot, VerifyOperation, + builder_deposits_cache::OnboardBuildersCache, common::get_attesting_indices_from_state, epoch_cache::initialize_epoch_cache, per_block_processing, diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 9d9482aa90f..87c65bee8e6 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -1542,7 +1542,12 @@ impl ExecutionPendingBlock { state_root }; - if let Some(summary) = per_slot_processing(&mut state, Some(state_root), GloasVerificationContext::from_cache(chain.builder_onboarding_cache.clone()), &chain.spec)? { + if let Some(summary) = per_slot_processing( + &mut state, + Some(state_root), + GloasVerificationContext::from_cache(chain.builder_onboarding_cache.clone()), + &chain.spec, + )? { // Expose Prometheus metrics. if let Err(e) = summary.observe_metrics() { error!( diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index b167453f914..852434f8006 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -446,8 +446,13 @@ where "Advancing checkpoint state to boundary" ); while weak_subj_state.slot() % slots_per_epoch != 0 { - per_slot_processing(&mut weak_subj_state, None, GloasVerificationContext::FullVerification, &self.spec) - .map_err(|e| format!("Error advancing state: {e:?}"))?; + per_slot_processing( + &mut weak_subj_state, + None, + GloasVerificationContext::FullVerification, + &self.spec, + ) + .map_err(|e| format!("Error advancing state: {e:?}"))?; } } diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs index b3fe24e0d25..0bfa9d4a5a1 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs @@ -71,7 +71,11 @@ impl BeaconChain { // Pre-verify builder deposit signatures while awaiting EL verification. if let Some(ref cache) = chain.builder_onboarding_cache { cache.cache_deposit_requests( - &execution_pending.signed_envelope.message.execution_requests.deposits, + &execution_pending + .signed_envelope + .message + .execution_requests + .deposits, &chain.spec, ); } diff --git a/beacon_node/beacon_chain/src/state_advance_timer.rs b/beacon_node/beacon_chain/src/state_advance_timer.rs index f5a45c2c64a..4833a391e37 100644 --- a/beacon_node/beacon_chain/src/state_advance_timer.rs +++ b/beacon_node/beacon_chain/src/state_advance_timer.rs @@ -286,9 +286,13 @@ fn advance_head(beacon_chain: &Arc>) -> Resu let initial_epoch = state.current_epoch(); // Advance the state a single slot. - if let Some(summary) = - per_slot_processing(&mut state, Some(head_state_root), GloasVerificationContext::from_cache(beacon_chain.builder_onboarding_cache.clone()), &beacon_chain.spec) - .map_err(BeaconChainError::from)? + if let Some(summary) = per_slot_processing( + &mut state, + Some(head_state_root), + GloasVerificationContext::from_cache(beacon_chain.builder_onboarding_cache.clone()), + &beacon_chain.spec, + ) + .map_err(BeaconChainError::from)? { // Expose Prometheus metrics. if let Err(e) = summary.observe_metrics() { diff --git a/beacon_node/beacon_chain/tests/attestation_verification.rs b/beacon_node/beacon_chain/tests/attestation_verification.rs index a722aa321f1..922c2c8e52f 100644 --- a/beacon_node/beacon_chain/tests/attestation_verification.rs +++ b/beacon_node/beacon_chain/tests/attestation_verification.rs @@ -1217,7 +1217,13 @@ async fn attestation_that_skips_epochs() { .expect("should find state"); while state.slot() < current_slot { - per_slot_processing(&mut state, None, GloasVerificationContext::FullVerification, &harness.spec).expect("should process slot"); + per_slot_processing( + &mut state, + None, + GloasVerificationContext::FullVerification, + &harness.spec, + ) + .expect("should process slot"); } let state_root = state.update_tree_hash_cache().unwrap(); @@ -1328,7 +1334,13 @@ async fn attestation_validator_receive_proposer_reward_and_withdrawals() { .expect("should find state"); while state.slot() < current_slot { - per_slot_processing(&mut state, None, GloasVerificationContext::FullVerification, &harness.spec).expect("should process slot"); + per_slot_processing( + &mut state, + None, + GloasVerificationContext::FullVerification, + &harness.spec, + ) + .expect("should process slot"); } let state_root = state.update_tree_hash_cache().unwrap(); @@ -1405,7 +1417,13 @@ async fn attestation_to_finalized_block() { .expect("should find state"); while state.slot() < current_slot { - per_slot_processing(&mut state, None, GloasVerificationContext::FullVerification, &harness.spec).expect("should process slot"); + per_slot_processing( + &mut state, + None, + GloasVerificationContext::FullVerification, + &harness.spec, + ) + .expect("should process slot"); } let state_root = state.update_tree_hash_cache().unwrap(); diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 45f2e2d90c6..cb7fee9e834 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -21,7 +21,8 @@ use logging::create_test_tracing_subscriber; use slasher::{Config as SlasherConfig, Slasher}; use state_processing::{ BlockProcessingError, BlockSignatureStrategy, ConsensusContext, GloasVerificationContext, - VerifyBlockRoot, common::{attesting_indices_base, attesting_indices_electra}, + VerifyBlockRoot, + common::{attesting_indices_base, attesting_indices_electra}, per_block_processing, per_slot_processing, }; use std::marker::PhantomData; @@ -1696,7 +1697,13 @@ async fn add_base_block_to_altair_chain() { { let mut state = state; let mut ctxt = ConsensusContext::new(base_block.slot()); - per_slot_processing(&mut state, None, GloasVerificationContext::FullVerification, &harness.chain.spec).unwrap(); + per_slot_processing( + &mut state, + None, + GloasVerificationContext::FullVerification, + &harness.chain.spec, + ) + .unwrap(); assert!(matches!( per_block_processing( &mut state, @@ -1848,7 +1855,13 @@ async fn add_altair_block_to_base_chain() { { let mut state = state; let mut ctxt = ConsensusContext::new(altair_block.slot()); - per_slot_processing(&mut state, None, GloasVerificationContext::FullVerification, &harness.chain.spec).unwrap(); + per_slot_processing( + &mut state, + None, + GloasVerificationContext::FullVerification, + &harness.chain.spec, + ) + .unwrap(); assert!(matches!( per_block_processing( &mut state, diff --git a/beacon_node/beacon_chain/tests/tests.rs b/beacon_node/beacon_chain/tests/tests.rs index 8cc568dcb1b..9099c50d74b 100644 --- a/beacon_node/beacon_chain/tests/tests.rs +++ b/beacon_node/beacon_chain/tests/tests.rs @@ -13,7 +13,8 @@ use bls::Keypair; use operation_pool::PersistedOperationPool; use state_processing::EpochProcessingError; use state_processing::{ - GloasVerificationContext, per_slot_processing, per_slot_processing::Error as SlotProcessingError, + GloasVerificationContext, per_slot_processing, + per_slot_processing::Error as SlotProcessingError, }; use std::sync::LazyLock; use types::{ @@ -109,7 +110,12 @@ fn massive_skips() { // Run per_slot_processing until it returns an error. let error = loop { - match per_slot_processing(&mut state, None, GloasVerificationContext::FullVerification, spec) { + match per_slot_processing( + &mut state, + None, + GloasVerificationContext::FullVerification, + spec, + ) { Ok(_) => continue, Err(e) => break e, } diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index b012378cb22..d1922f2d883 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -40,8 +40,8 @@ use sensitive_url::SensitiveUrl; use slot_clock::SlotClock; use ssz::{BitList, Decode}; use state_processing::per_block_processing::get_expected_withdrawals; -use state_processing::{GloasVerificationContext, per_slot_processing}; use state_processing::state_advance::partial_state_advance; +use state_processing::{GloasVerificationContext, per_slot_processing}; use std::convert::TryInto; use std::sync::Arc; use tokio::time::Duration; @@ -5011,7 +5011,13 @@ impl ApiTester { let mut head = self.chain.head_snapshot().as_ref().clone(); while head.beacon_state.current_epoch() < epoch { - per_slot_processing(&mut head.beacon_state, None, GloasVerificationContext::FullVerification, &self.chain.spec).unwrap(); + per_slot_processing( + &mut head.beacon_state, + None, + GloasVerificationContext::FullVerification, + &self.chain.spec, + ) + .unwrap(); } head.beacon_state .build_committee_cache(RelativeEpoch::Current, &self.chain.spec) diff --git a/beacon_node/store/src/reconstruct.rs b/beacon_node/store/src/reconstruct.rs index fd20591f9e6..d3b058d098a 100644 --- a/beacon_node/store/src/reconstruct.rs +++ b/beacon_node/store/src/reconstruct.rs @@ -145,8 +145,13 @@ where }; // Advance state to slot. - per_slot_processing(&mut state, prev_state_root.take(), GloasVerificationContext::FullVerification, &self.spec) - .map_err(HotColdDBError::BlockReplaySlotError)?; + per_slot_processing( + &mut state, + prev_state_root.take(), + GloasVerificationContext::FullVerification, + &self.spec, + ) + .map_err(HotColdDBError::BlockReplaySlotError)?; // Apply block. if let Some(block) = block { diff --git a/consensus/state_processing/src/block_replayer.rs b/consensus/state_processing/src/block_replayer.rs index 31e21beb084..2c75a060dea 100644 --- a/consensus/state_processing/src/block_replayer.rs +++ b/consensus/state_processing/src/block_replayer.rs @@ -230,9 +230,13 @@ where pre_slot_hook(state_root, &mut self.state)?; } - let summary = - per_slot_processing(&mut self.state, Some(state_root), GloasVerificationContext::FullVerification, self.spec) - .map_err(BlockReplayError::from)?; + let summary = per_slot_processing( + &mut self.state, + Some(state_root), + GloasVerificationContext::FullVerification, + self.spec, + ) + .map_err(BlockReplayError::from)?; if let Some(ref mut post_slot_hook) = self.post_slot_hook { let is_skipped_slot = self.state.slot() < block.slot(); @@ -278,9 +282,13 @@ where pre_slot_hook(state_root, &mut self.state)?; } - let summary = - per_slot_processing(&mut self.state, Some(state_root), GloasVerificationContext::FullVerification, self.spec) - .map_err(BlockReplayError::from)?; + let summary = per_slot_processing( + &mut self.state, + Some(state_root), + GloasVerificationContext::FullVerification, + self.spec, + ) + .map_err(BlockReplayError::from)?; if let Some(ref mut post_slot_hook) = self.post_slot_hook { // No more blocks to apply (from our perspective) so we consider these slots diff --git a/consensus/state_processing/src/builder_deposits_cache.rs b/consensus/state_processing/src/builder_deposits_cache.rs index bd5cc73a6af..55a8cbfa17b 100644 --- a/consensus/state_processing/src/builder_deposits_cache.rs +++ b/consensus/state_processing/src/builder_deposits_cache.rs @@ -22,7 +22,6 @@ use std::num::NonZeroUsize; /// every slot. const CACHE_SIZE: NonZeroUsize = new_non_zero_usize(262144); - /// A simple cache that performs signature verification on `PendingDeposit` entries in the /// beacon state for 0x03 credentials and caches the result. /// diff --git a/consensus/state_processing/src/lib.rs b/consensus/state_processing/src/lib.rs index a7d821f169e..b0d4d1aaedf 100644 --- a/consensus/state_processing/src/lib.rs +++ b/consensus/state_processing/src/lib.rs @@ -46,6 +46,6 @@ pub use per_epoch_processing::{ errors::EpochProcessingError, process_epoch as per_epoch_processing, }; pub use per_slot_processing::{Error as SlotProcessingError, per_slot_processing}; -pub use upgrade::GloasVerificationContext; pub use types::{EpochCache, EpochCacheError, EpochCacheKey}; +pub use upgrade::GloasVerificationContext; pub use verify_operation::{SigVerifiedOp, TransformPersist, VerifyOperation, VerifyOperationAt}; diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index f93420ea495..b8b3857fcc2 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -573,8 +573,8 @@ async fn process_deposit_requests_post_gloas_batches_new_builder_signature_verif let invalid_builder_index = find_builder_index(&state, &invalid_builder_request.pubkey); assert!(invalid_builder_index.is_none()); - let existing_builder_index = find_builder_index(&state, &existing_builder_top_up.pubkey) - .unwrap(); + let existing_builder_index = + find_builder_index(&state, &existing_builder_top_up.pubkey).unwrap(); let existing_builder = state .builders() .unwrap() @@ -692,13 +692,8 @@ async fn process_deposit_requests_post_gloas_preserves_existing_builder_before_v &spec, 0, ); - let validator_prefixed_request = make_deposit_request( - &KEYPAIRS[VALIDATOR_COUNT + 6], - Hash256::ZERO, - 47, - &spec, - 1, - ); + let validator_prefixed_request = + make_deposit_request(&KEYPAIRS[VALIDATOR_COUNT + 6], Hash256::ZERO, 47, &spec, 1); process_operations::process_deposit_requests_post_gloas( &mut state, diff --git a/consensus/state_processing/src/per_epoch_processing/tests.rs b/consensus/state_processing/src/per_epoch_processing/tests.rs index cc8b824a679..d6c37147017 100644 --- a/consensus/state_processing/src/per_epoch_processing/tests.rs +++ b/consensus/state_processing/src/per_epoch_processing/tests.rs @@ -82,8 +82,13 @@ mod release_tests { // Check the state is valid before starting this test. process_epoch(&mut altair_state.clone(), &spec) .expect("state passes intial epoch processing"); - per_slot_processing(&mut altair_state.clone(), None, GloasVerificationContext::FullVerification, &spec) - .expect("state passes intial slot processing"); + per_slot_processing( + &mut altair_state.clone(), + None, + GloasVerificationContext::FullVerification, + &spec, + ) + .expect("state passes intial slot processing"); // Modify the spec so altair never happens. spec.altair_fork_epoch = None; @@ -99,7 +104,12 @@ mod release_tests { Err(EpochProcessingError::InconsistentStateFork(expected_err)) ); assert_eq!( - per_slot_processing(&mut altair_state.clone(), None, GloasVerificationContext::FullVerification, &spec), + per_slot_processing( + &mut altair_state.clone(), + None, + GloasVerificationContext::FullVerification, + &spec + ), Err(SlotProcessingError::InconsistentStateFork(expected_err)) ); } @@ -142,8 +152,13 @@ mod release_tests { // Check the state is valid before starting this test. process_epoch(&mut base_state.clone(), &spec) .expect("state passes intial epoch processing"); - per_slot_processing(&mut base_state.clone(), None, GloasVerificationContext::FullVerification, &spec) - .expect("state passes intial slot processing"); + per_slot_processing( + &mut base_state.clone(), + None, + GloasVerificationContext::FullVerification, + &spec, + ) + .expect("state passes intial slot processing"); // Modify the spec so Altair happens at the first epoch. spec.altair_fork_epoch = Some(Epoch::new(1)); @@ -159,7 +174,12 @@ mod release_tests { Err(EpochProcessingError::InconsistentStateFork(expected_err)) ); assert_eq!( - per_slot_processing(&mut base_state.clone(), None, GloasVerificationContext::FullVerification, &spec), + per_slot_processing( + &mut base_state.clone(), + None, + GloasVerificationContext::FullVerification, + &spec + ), Err(SlotProcessingError::InconsistentStateFork(expected_err)) ); } diff --git a/consensus/state_processing/src/per_slot_processing.rs b/consensus/state_processing/src/per_slot_processing.rs index 024e9be75c0..ccf7ab447a2 100644 --- a/consensus/state_processing/src/per_slot_processing.rs +++ b/consensus/state_processing/src/per_slot_processing.rs @@ -1,8 +1,8 @@ +use crate::upgrade::gloas::GloasVerificationContext; use crate::upgrade::{ upgrade_to_altair, upgrade_to_bellatrix, upgrade_to_capella, upgrade_to_deneb, upgrade_to_electra, upgrade_to_fulu, upgrade_to_gloas, }; -use crate::upgrade::gloas::GloasVerificationContext; use crate::{per_epoch_processing::EpochProcessingSummary, *}; use fixed_bytes::FixedBytesExtended; use safe_arith::{ArithError, SafeArith}; diff --git a/consensus/state_processing/src/state_advance.rs b/consensus/state_processing/src/state_advance.rs index c2f5b195c51..76ff18ef7d3 100644 --- a/consensus/state_processing/src/state_advance.rs +++ b/consensus/state_processing/src/state_advance.rs @@ -4,8 +4,8 @@ //! These functions are not in the specification, however they're defined here to reduce code //! duplication and protect against some easy-to-make mistakes when performing state advances. -use crate::*; use crate::upgrade::GloasVerificationContext; +use crate::*; use fixed_bytes::FixedBytesExtended; use tracing::instrument; use types::{BeaconState, ChainSpec, EthSpec, Hash256, Slot}; @@ -41,8 +41,13 @@ pub fn complete_state_advance( // future iterations. let state_root_opt = state_root_opt.take(); - per_slot_processing(state, state_root_opt, GloasVerificationContext::FullVerification, spec) - .map_err(Error::PerSlotProcessing)?; + per_slot_processing( + state, + state_root_opt, + GloasVerificationContext::FullVerification, + spec, + ) + .map_err(Error::PerSlotProcessing)?; } Ok(()) @@ -97,8 +102,13 @@ pub fn partial_state_advance( // with the correct state root. let state_root = initial_state_root.take().unwrap_or_else(Hash256::zero); - per_slot_processing(state, Some(state_root), GloasVerificationContext::CommitteesOnly, spec) - .map_err(Error::PerSlotProcessing)?; + per_slot_processing( + state, + Some(state_root), + GloasVerificationContext::CommitteesOnly, + spec, + ) + .map_err(Error::PerSlotProcessing)?; } Ok(()) diff --git a/testing/ef_tests/src/cases/fork.rs b/testing/ef_tests/src/cases/fork.rs index fea4969da15..b352bb01880 100644 --- a/testing/ef_tests/src/cases/fork.rs +++ b/testing/ef_tests/src/cases/fork.rs @@ -72,9 +72,12 @@ impl Case for ForkTest { ForkName::Deneb => upgrade_to_deneb(&mut result_state, spec).map(|_| result_state), ForkName::Electra => upgrade_to_electra(&mut result_state, spec).map(|_| result_state), ForkName::Fulu => upgrade_to_fulu(&mut result_state, spec).map(|_| result_state), - ForkName::Gloas => { - upgrade_to_gloas(&mut result_state, GloasVerificationContext::FullVerification, spec).map(|_| result_state) - } + ForkName::Gloas => upgrade_to_gloas( + &mut result_state, + GloasVerificationContext::FullVerification, + spec, + ) + .map(|_| result_state), }; compare_beacon_state_results_without_caches(&mut result, &mut expected) diff --git a/testing/ef_tests/src/cases/sanity_blocks.rs b/testing/ef_tests/src/cases/sanity_blocks.rs index 3495b1c1566..28707bbb51c 100644 --- a/testing/ef_tests/src/cases/sanity_blocks.rs +++ b/testing/ef_tests/src/cases/sanity_blocks.rs @@ -79,8 +79,20 @@ impl Case for SanityBlocks { .try_for_each(|signed_block| { let block = signed_block.message(); while bulk_state.slot() < block.slot() { - per_slot_processing(&mut bulk_state, None, GloasVerificationContext::FullVerification, spec).unwrap(); - per_slot_processing(&mut indiv_state, None, GloasVerificationContext::FullVerification, spec).unwrap(); + per_slot_processing( + &mut bulk_state, + None, + GloasVerificationContext::FullVerification, + spec, + ) + .unwrap(); + per_slot_processing( + &mut indiv_state, + None, + GloasVerificationContext::FullVerification, + spec, + ) + .unwrap(); } bulk_state diff --git a/testing/ef_tests/src/cases/sanity_slots.rs b/testing/ef_tests/src/cases/sanity_slots.rs index 8f737f7d6fc..1abe00811aa 100644 --- a/testing/ef_tests/src/cases/sanity_slots.rs +++ b/testing/ef_tests/src/cases/sanity_slots.rs @@ -64,7 +64,15 @@ impl Case for SanitySlots { state.build_caches(spec).unwrap(); let mut result = (0..self.slots) - .try_for_each(|_| per_slot_processing(&mut state, None, GloasVerificationContext::FullVerification, spec).map(|_| ())) + .try_for_each(|_| { + per_slot_processing( + &mut state, + None, + GloasVerificationContext::FullVerification, + spec, + ) + .map(|_| ()) + }) .map(|_| state); compare_beacon_state_results_without_caches(&mut result, &mut expected) From 9738905cc3ce78c488f3b513d62d000387f79a5b Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Sat, 16 May 2026 20:36:35 -0700 Subject: [PATCH 11/22] lint --- .../src/builder_deposits_cache.rs | 20 +++++----- .../process_operations.rs | 37 +++++++++++-------- .../state_processing/src/upgrade/gloas.rs | 13 +++---- 3 files changed, 37 insertions(+), 33 deletions(-) diff --git a/consensus/state_processing/src/builder_deposits_cache.rs b/consensus/state_processing/src/builder_deposits_cache.rs index 55a8cbfa17b..8ba736243a1 100644 --- a/consensus/state_processing/src/builder_deposits_cache.rs +++ b/consensus/state_processing/src/builder_deposits_cache.rs @@ -28,12 +28,12 @@ const CACHE_SIZE: NonZeroUsize = new_non_zero_usize(262144); /// The key is the hash_tree_root of the `Deposit` and the value is the verification result. /// In gloas, there are 2 places where we need to do bulk signature verification: /// 1. `onboard_builders_from_pending_deposits` in `upgrade_to_gloas` happens at the fork transition. -/// If the `pending_deposits` queue at fork has many signatures to verify, then verifying them -/// in the hot path could be very expensive. -/// 2. `process_deposit_requests_post_gloas` in `process_operations` can contain upto 8192 signatures to -/// verify in the hot block verification path based on limits today. Since the deposits are received a -/// couple seconds before the actual deposits processing, we can cache those signatures too to ensure -/// the deposit processing is just a lookup operation in the worst case. +/// If the `pending_deposits` queue at fork has many signatures to verify, then verifying them +/// in the hot path could be very expensive. +/// 2. `process_deposit_requests_post_gloas` in `process_operations` can contain upto 8192 signatures +/// to verify in the hot block verification path based on limits today. Since the deposits are +/// received a couple seconds before the actual deposits processing, we can cache those signatures +/// too to ensure the deposit processing is just a lookup operation in the worst case. pub struct OnboardBuildersCache { cache: Mutex>, } @@ -92,7 +92,7 @@ impl OnboardBuildersCache { "Adding new pending deposits to builder onboarding cache" ); - self.cache_pending_deposits(pending_deposits, &spec); + self.cache_pending_deposits(pending_deposits, spec); } /// Takes a list of pending deposits, signature verifies them and caches the result. @@ -134,7 +134,7 @@ impl OnboardBuildersCache { let verified = is_valid_deposit_signature_batch(builder_deposits, spec); let mut cache = self.cache.lock(); - for (key, value) in builder_deposit_keys.into_iter().zip(verified.into_iter()) { + for (key, value) in builder_deposit_keys.into_iter().zip(verified) { cache.push(key, value); } } @@ -180,7 +180,7 @@ impl OnboardBuildersCache { let verified = is_valid_deposit_signature_batch(builder_deposits, spec); let mut cache = self.cache.lock(); - for (key, value) in builder_deposit_keys.into_iter().zip(verified.into_iter()) { + for (key, value) in builder_deposit_keys.into_iter().zip(verified) { cache.push(key, value); } } @@ -221,7 +221,7 @@ fn pending_deposits_to_verify(state: &BeaconState) -> Vec<&Pendin .get(index) .is_some_and(|deposit| deposit.slot != current_slot) { - first_current_slot_index = index + 1; + first_current_slot_index = index.saturating_add(1); break; } } diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index 9bb7b39af5a..c17110a86fd 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -1010,32 +1010,37 @@ pub fn process_deposit_requests_post_gloas( let mut uncached_indices = Vec::new(); let mut uncached_deposits = Vec::new(); - for (i, deposit_data) in builder_signature_deposits.iter().enumerate() { + for (deposit_data, &request_index) in builder_signature_deposits + .iter() + .zip(builder_signature_request_indices.iter()) + { let cached_result = onboarding_cache.and_then(|c| c.get(deposit_data)); if let Some(is_valid) = cached_result { - let request_index = builder_signature_request_indices[i]; - builder_signature_results_by_request[request_index] = if is_valid { - VerifyBuilderSignature::VerifiedValid - } else { - VerifyBuilderSignature::VerifiedInvalid - }; + if let Some(res) = builder_signature_results_by_request.get_mut(request_index) { + *res = if is_valid { + VerifyBuilderSignature::VerifiedValid + } else { + VerifyBuilderSignature::VerifiedInvalid + }; + } } else { - uncached_indices.push(i); + uncached_indices.push(request_index); uncached_deposits.push(deposit_data.clone()); } } - let cache_hits = builder_signature_deposits.len() - uncached_deposits.len(); + let cache_hits = builder_signature_deposits.len().saturating_sub(uncached_deposits.len()); let batch_len = uncached_deposits.len(); let batch_results = is_valid_deposit_signature_batch(uncached_deposits, spec); - for (batch_i, is_valid) in uncached_indices.into_iter().zip(batch_results) { - let request_index = builder_signature_request_indices[batch_i]; - builder_signature_results_by_request[request_index] = if is_valid { - VerifyBuilderSignature::VerifiedValid - } else { - VerifyBuilderSignature::VerifiedInvalid - }; + for (&request_index, is_valid) in uncached_indices.iter().zip(batch_results) { + if let Some(slot) = builder_signature_results_by_request.get_mut(request_index) { + *slot = if is_valid { + VerifyBuilderSignature::VerifiedValid + } else { + VerifyBuilderSignature::VerifiedInvalid + }; + } } tracing::debug!( diff --git a/consensus/state_processing/src/upgrade/gloas.rs b/consensus/state_processing/src/upgrade/gloas.rs index e34da454d4d..7ac3213c263 100644 --- a/consensus/state_processing/src/upgrade/gloas.rs +++ b/consensus/state_processing/src/upgrade/gloas.rs @@ -235,13 +235,12 @@ fn onboard_builders_from_pending_deposits( let builder_index = builder_pubkey_to_index.get(&deposit.pubkey).copied(); - if builder_index.is_none() { - if !is_builder_withdrawal_credential(deposit.withdrawal_credentials, spec) - || is_pending_validator::(&pending_deposits, &deposit.pubkey, spec) - { - pending_deposits.push(deposit.clone())?; - continue; - } + if builder_index.is_none() + && (!is_builder_withdrawal_credential(deposit.withdrawal_credentials, spec) + || is_pending_validator::(&pending_deposits, &deposit.pubkey, spec)) + { + pending_deposits.push(deposit.clone())?; + continue; } // Use the builder onboarding cache to get the signature verification result. From fd43fdc637e5267099478ca888420cd913979f7a Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Mon, 18 May 2026 16:44:40 -0700 Subject: [PATCH 12/22] Cleanup --- beacon_node/beacon_chain/src/beacon_chain.rs | 4 ++- .../payload_envelope_verification/import.rs | 27 ++++++++++--------- .../process_operations.rs | 4 ++- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index bd4edc8cad4..72e6db59daa 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -512,8 +512,10 @@ pub struct BeaconChain { pub pending_payload_cache: Arc>, /// The KZG trusted setup used by this chain. pub kzg: Arc, - /// RNG instance used by the chain. Currently used for shuffling column sidecars in block publishing. + /// Pre-validates builder deposit signatures. + /// Only required when gloas is enabled. pub builder_onboarding_cache: Option>, + /// RNG instance used by the chain. Currently used for shuffling column sidecars in block publishing. pub rng: Arc>>, } diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs index 0bfa9d4a5a1..8bd03a437bd 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs @@ -55,6 +55,21 @@ impl BeaconChain { ); } + // Pre-verify builder deposit signatures while awaiting EL verification. + if let Some(ref cache) = self.builder_onboarding_cache { + let cache = cache.clone(); + let deposits = unverified_envelope + .signed_envelope + .message + .execution_requests + .deposits + .clone(); + let spec = self.spec.clone(); + self.task_executor.spawn_blocking( + move || cache.cache_deposit_requests(&deposits, &spec), + "pre_verify_builder_deposits", + ); + } // TODO(gloas) insert the pre-executed envelope into some type of cache? let _full_timer = metrics::start_timer(&metrics::ENVELOPE_PROCESSING_TIMES); @@ -68,18 +83,6 @@ impl BeaconChain { .into_execution_pending_envelope(&chain, notify_execution_layer)?; publish_fn()?; - // Pre-verify builder deposit signatures while awaiting EL verification. - if let Some(ref cache) = chain.builder_onboarding_cache { - cache.cache_deposit_requests( - &execution_pending - .signed_envelope - .message - .execution_requests - .deposits, - &chain.spec, - ); - } - // Record the time it took to complete consensus verification. if let Some(timestamp) = chain.slot_clock.now_duration() { chain diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index c17110a86fd..f648fbd83f3 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -1029,7 +1029,9 @@ pub fn process_deposit_requests_post_gloas( } } - let cache_hits = builder_signature_deposits.len().saturating_sub(uncached_deposits.len()); + let cache_hits = builder_signature_deposits + .len() + .saturating_sub(uncached_deposits.len()); let batch_len = uncached_deposits.len(); let batch_results = is_valid_deposit_signature_batch(uncached_deposits, spec); From 44dcf5d14bdadc6bae50025321595b60dcff29f0 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Mon, 18 May 2026 17:01:57 -0700 Subject: [PATCH 13/22] Fix lint --- .../state_processing/src/per_block_processing/tests.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index b8b3857fcc2..09b46dc658e 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -453,8 +453,8 @@ async fn deposit_signature_batch_returns_true_for_valid_and_false_for_invalid() ); assert_eq!(result.len(), 10); - assert_eq!(result[1], false); - assert_eq!(result[8], false); + assert!(!result[1]); + assert!(!result[8]); assert!( result .iter() @@ -495,7 +495,7 @@ async fn deposit_signature_batch_falls_back_to_individual_verification() { ); assert_eq!(result.len(), 10); - assert_eq!(result[3], false); + assert!(!result[3]); assert!( result .iter() @@ -746,7 +746,7 @@ async fn process_deposit_requests_post_gloas_preserves_pre_state_pending_validat process_operations::process_deposit_requests_post_gloas( &mut state, - &[builder_prefixed_request.clone()], + std::slice::from_ref(&builder_prefixed_request), None, &spec, ) From bc1a15698f295a7ed0a1ef74a9f652e059f925da Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Mon, 18 May 2026 17:33:23 -0700 Subject: [PATCH 14/22] Call initialize_ptc in all cases and rename variant --- consensus/state_processing/src/state_advance.rs | 2 +- consensus/state_processing/src/upgrade/gloas.rs | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/consensus/state_processing/src/state_advance.rs b/consensus/state_processing/src/state_advance.rs index 76ff18ef7d3..cb3c0d554dd 100644 --- a/consensus/state_processing/src/state_advance.rs +++ b/consensus/state_processing/src/state_advance.rs @@ -105,7 +105,7 @@ pub fn partial_state_advance( per_slot_processing( state, Some(state_root), - GloasVerificationContext::CommitteesOnly, + GloasVerificationContext::SkipBuilderOnboarding, spec, ) .map_err(Error::PerSlotProcessing)?; diff --git a/consensus/state_processing/src/upgrade/gloas.rs b/consensus/state_processing/src/upgrade/gloas.rs index 7ac3213c263..7ed832c3286 100644 --- a/consensus/state_processing/src/upgrade/gloas.rs +++ b/consensus/state_processing/src/upgrade/gloas.rs @@ -32,13 +32,13 @@ pub enum GloasVerificationContext { /// that need to be onboarded at the fork boundary. This variant should be used /// for tests and other non-production paths. FullVerification, - /// Skip `onboard_builders_from_pending_deposits` entirely which is sufficient for - /// paths that do a state advance only for committee calculation. + /// Skip `onboard_builders_from_pending_deposits` but still initialize the PTC window. /// /// `onboard_builders_from_pending_deposits` only modifies `state.pending_deposits` and - /// `state.builders` which don't affect committee calculation. - /// TODO(pawan): triple check this is true. - CommitteesOnly, + /// `state.builders` which don't affect committee or PTC calculation. This variant is + /// used by `partial_state_advance` where callers need committee and PTC lookups but + /// don't need the builder registry to be fully populated. + SkipBuilderOnboarding, } impl GloasVerificationContext { @@ -169,7 +169,9 @@ pub fn upgrade_state_to_gloas( onboard_builders_from_pending_deposits(&mut post, None, spec)?; initialize_ptc_window(&mut post, spec)?; } - GloasVerificationContext::CommitteesOnly => {} + GloasVerificationContext::SkipBuilderOnboarding => { + initialize_ptc_window(&mut post, spec)?; + } } Ok(post) From 50cd378ff97775a0baf4a4a74279c3054a72ea37 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Tue, 19 May 2026 14:04:08 -0700 Subject: [PATCH 15/22] Use spawn_blocking_with_rayon for batch signature verification tasks --- beacon_node/beacon_chain/src/beacon_chain.rs | 7 ++++++- beacon_node/beacon_chain/src/builder.rs | 6 +++++- .../src/payload_envelope_verification/import.rs | 6 +++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 72e6db59daa..1f8a4609f86 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -4549,8 +4549,13 @@ impl BeaconChain { if let Some(builder_onboarding_cache) = &self.builder_onboarding_cache { let cache = builder_onboarding_cache.clone(); let spec = self.spec.clone(); - self.task_executor.spawn_blocking( + let executor = self.task_executor.clone(); + // Using rayon pool here since `add_new_pending_deposits` uses rayon threads to + // perform the signature verification in batches. + // // We have until the fork transition for the cache to be used, so we use the low priority pool. + executor.spawn_blocking_with_rayon( move || cache.add_new_pending_deposits::(&state, &spec), + RayonPoolType::LowPriority, "pre_verify_deposits", ); } diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 852434f8006..8259e25a9f7 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -1166,8 +1166,12 @@ where if let Some(onboarding_cache) = &beacon_chain.builder_onboarding_cache { let cache = onboarding_cache.clone(); let spec = self.spec.clone(); - beacon_chain.task_executor.spawn_blocking( + let executor = beacon_chain.task_executor.clone(); + // Using rayon pool here since `seed_from_state` uses rayon threads to + // perform the signature verification in batches. + executor.spawn_blocking_with_rayon( move || cache.seed_from_state(&head.beacon_state, &spec), + task_executor::RayonPoolType::HighPriority, "initialize_builder_onboarding_cache", ); } diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs index 8bd03a437bd..9ecd151e90e 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs @@ -65,8 +65,12 @@ impl BeaconChain { .deposits .clone(); let spec = self.spec.clone(); - self.task_executor.spawn_blocking( + let executor = self.task_executor.clone(); + // Using rayon pool here since `cache_deposit_requests` uses rayon threads to + // perform the signature verification in batches. + executor.spawn_blocking_with_rayon( move || cache.cache_deposit_requests(&deposits, &spec), + task_executor::RayonPoolType::HighPriority, "pre_verify_builder_deposits", ); } From 16f615e1d4c669f8c18a220b2faac2df1098fa1b Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Tue, 19 May 2026 14:04:17 -0700 Subject: [PATCH 16/22] Review comments --- .../src/per_block_processing/process_operations.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index f648fbd83f3..8bdff56e42d 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -908,7 +908,7 @@ pub fn process_deposit_requests_pre_gloas( /// Check if there is a pending deposit for a new validator with the given pubkey. // TODO(gloas): cache the deposit signature validation or remove this loop entirely if possible, // it is `O(n * m)` where `n` is max 8192 and `m` is max 128M. -#[instrument(name = "is_pending_validator", skip_all, level = "debug")] +#[instrument(skip_all, level = "debug")] pub fn is_pending_validator( deposits: &List, pubkey: &PublicKeyBytes, @@ -954,7 +954,7 @@ impl From> for VerifyBuilderSignature { /// /// TODO(gloas): this could be more efficient in the builder case, see: /// https://github.com/sigp/lighthouse/issues/8783 -#[instrument(name = "get_builder_index", skip_all, level = "debug")] +#[instrument(skip_all, level = "debug")] fn get_builder_index( state: &BeaconState, pubkey: &PublicKeyBytes, From 33bccd0a63c0cffebaed1ce6866a28d51b66a6fc Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Tue, 19 May 2026 14:22:54 -0700 Subject: [PATCH 17/22] Add unit tests --- .../src/builder_deposits_cache.rs | 264 ++++++++++++++++++ 1 file changed, 264 insertions(+) diff --git a/consensus/state_processing/src/builder_deposits_cache.rs b/consensus/state_processing/src/builder_deposits_cache.rs index 8ba736243a1..346ad646ada 100644 --- a/consensus/state_processing/src/builder_deposits_cache.rs +++ b/consensus/state_processing/src/builder_deposits_cache.rs @@ -231,3 +231,267 @@ fn pending_deposits_to_verify(state: &BeaconState) -> Vec<&Pendin .skip(first_current_slot_index) .collect() } + +#[cfg(all(test, not(feature = "fake_crypto")))] +mod tests { + use super::*; + use bls::{Keypair, SignatureBytes}; + use std::sync::LazyLock; + use types::{ForkName, MainnetEthSpec, Slot}; + + static KEYPAIRS: LazyLock> = + LazyLock::new(|| types::test_utils::generate_deterministic_keypairs(10)); + + fn gloas_spec() -> ChainSpec { + ForkName::Gloas.make_genesis_spec(MainnetEthSpec::default_spec()) + } + + fn non_gloas_spec() -> ChainSpec { + ForkName::Fulu.make_genesis_spec(MainnetEthSpec::default_spec()) + } + + fn builder_credentials(spec: &ChainSpec) -> Hash256 { + let mut credentials = [0u8; 32]; + credentials[0] = spec.builder_withdrawal_prefix_byte; + Hash256::from_slice(&credentials) + } + + fn non_builder_credentials() -> Hash256 { + let mut credentials = [0u8; 32]; + credentials[0] = 0x01; // ETH1 withdrawal credentials + Hash256::from_slice(&credentials) + } + + fn make_valid_builder_deposit(keypair: &Keypair, spec: &ChainSpec) -> PendingDeposit { + let withdrawal_credentials = builder_credentials(spec); + let mut deposit_data = DepositData { + pubkey: keypair.pk.compress(), + withdrawal_credentials, + amount: 256_000_000_000, + signature: SignatureBytes::empty(), + }; + deposit_data.signature = deposit_data.create_signature(&keypair.sk, spec); + + PendingDeposit { + pubkey: deposit_data.pubkey, + withdrawal_credentials: deposit_data.withdrawal_credentials, + amount: deposit_data.amount, + signature: deposit_data.signature, + slot: Slot::new(0), + } + } + + fn make_invalid_builder_deposit(keypair: &Keypair, spec: &ChainSpec) -> PendingDeposit { + let withdrawal_credentials = builder_credentials(spec); + PendingDeposit { + pubkey: keypair.pk.compress(), + withdrawal_credentials, + amount: 256_000_000_000, + signature: SignatureBytes::empty(), + slot: Slot::new(0), + } + } + + fn make_non_builder_deposit(keypair: &Keypair, spec: &ChainSpec) -> PendingDeposit { + let mut deposit_data = DepositData { + pubkey: keypair.pk.compress(), + withdrawal_credentials: non_builder_credentials(), + amount: 32_000_000_000, + signature: SignatureBytes::empty(), + }; + deposit_data.signature = deposit_data.create_signature(&keypair.sk, spec); + + PendingDeposit { + pubkey: deposit_data.pubkey, + withdrawal_credentials: deposit_data.withdrawal_credentials, + amount: deposit_data.amount, + signature: deposit_data.signature, + slot: Slot::new(0), + } + } + + #[test] + fn new_returns_none_when_gloas_not_scheduled() { + let spec = non_gloas_spec(); + assert!(OnboardBuildersCache::new(&spec).is_none()); + } + + #[test] + fn new_returns_some_when_gloas_scheduled() { + let spec = gloas_spec(); + assert!(OnboardBuildersCache::new(&spec).is_some()); + } + + #[test] + fn cache_valid_builder_deposit() { + let spec = gloas_spec(); + let cache = OnboardBuildersCache::new(&spec).unwrap(); + let deposit = make_valid_builder_deposit(&KEYPAIRS[0], &spec); + + cache.cache_pending_deposits(vec![&deposit], &spec); + + assert_eq!(cache.cached_is_valid_signature(&deposit), Some(true)); + } + + #[test] + fn cache_invalid_builder_deposit() { + let spec = gloas_spec(); + let cache = OnboardBuildersCache::new(&spec).unwrap(); + let deposit = make_invalid_builder_deposit(&KEYPAIRS[0], &spec); + + cache.cache_pending_deposits(vec![&deposit], &spec); + + assert_eq!(cache.cached_is_valid_signature(&deposit), Some(false)); + } + + #[test] + fn cache_miss_returns_none() { + let spec = gloas_spec(); + let cache = OnboardBuildersCache::new(&spec).unwrap(); + let deposit = make_valid_builder_deposit(&KEYPAIRS[0], &spec); + + assert_eq!(cache.cached_is_valid_signature(&deposit), None); + } + + #[test] + fn non_builder_deposits_filtered_out() { + let spec = gloas_spec(); + let cache = OnboardBuildersCache::new(&spec).unwrap(); + let non_builder = make_non_builder_deposit(&KEYPAIRS[0], &spec); + + cache.cache_pending_deposits(vec![&non_builder], &spec); + + assert_eq!(cache.cached_is_valid_signature(&non_builder), None); + } + + #[test] + fn mixed_deposits_only_caches_builder() { + let spec = gloas_spec(); + let cache = OnboardBuildersCache::new(&spec).unwrap(); + let builder_deposit = make_valid_builder_deposit(&KEYPAIRS[0], &spec); + let non_builder_deposit = make_non_builder_deposit(&KEYPAIRS[1], &spec); + + cache.cache_pending_deposits(vec![&non_builder_deposit, &builder_deposit], &spec); + + assert_eq!( + cache.cached_is_valid_signature(&builder_deposit), + Some(true) + ); + assert_eq!(cache.cached_is_valid_signature(&non_builder_deposit), None); + } + + #[test] + fn duplicate_deposits_not_reverified() { + let spec = gloas_spec(); + let cache = OnboardBuildersCache::new(&spec).unwrap(); + let deposit = make_valid_builder_deposit(&KEYPAIRS[0], &spec); + + cache.cache_pending_deposits(vec![&deposit], &spec); + // Second call with same deposit - should be skipped (already in cache) + cache.cache_pending_deposits(vec![&deposit], &spec); + + assert_eq!(cache.cached_is_valid_signature(&deposit), Some(true)); + } + + #[test] + fn multiple_valid_and_invalid_deposits() { + let spec = gloas_spec(); + let cache = OnboardBuildersCache::new(&spec).unwrap(); + + let valid_0 = make_valid_builder_deposit(&KEYPAIRS[0], &spec); + let valid_1 = make_valid_builder_deposit(&KEYPAIRS[1], &spec); + let invalid_0 = make_invalid_builder_deposit(&KEYPAIRS[2], &spec); + let invalid_1 = make_invalid_builder_deposit(&KEYPAIRS[3], &spec); + + cache.cache_pending_deposits( + vec![&valid_0, &invalid_0, &valid_1, &invalid_1], + &spec, + ); + + assert_eq!(cache.cached_is_valid_signature(&valid_0), Some(true)); + assert_eq!(cache.cached_is_valid_signature(&valid_1), Some(true)); + assert_eq!(cache.cached_is_valid_signature(&invalid_0), Some(false)); + assert_eq!(cache.cached_is_valid_signature(&invalid_1), Some(false)); + } + + #[test] + fn cache_deposit_requests_works() { + let spec = gloas_spec(); + let cache = OnboardBuildersCache::new(&spec).unwrap(); + let withdrawal_credentials = builder_credentials(&spec); + + let mut deposit_data = DepositData { + pubkey: KEYPAIRS[0].pk.compress(), + withdrawal_credentials, + amount: 256_000_000_000, + signature: SignatureBytes::empty(), + }; + deposit_data.signature = deposit_data.create_signature(&KEYPAIRS[0].sk, &spec); + + let request = DepositRequest { + pubkey: deposit_data.pubkey, + withdrawal_credentials: deposit_data.withdrawal_credentials, + amount: deposit_data.amount, + signature: deposit_data.signature.clone(), + index: 0, + }; + + cache.cache_deposit_requests(&[request], &spec); + + assert_eq!(cache.get(&deposit_data), Some(true)); + } + + #[test] + fn cache_deposit_requests_filters_non_builder() { + let spec = gloas_spec(); + let cache = OnboardBuildersCache::new(&spec).unwrap(); + + let mut deposit_data = DepositData { + pubkey: KEYPAIRS[0].pk.compress(), + withdrawal_credentials: non_builder_credentials(), + amount: 32_000_000_000, + signature: SignatureBytes::empty(), + }; + deposit_data.signature = deposit_data.create_signature(&KEYPAIRS[0].sk, &spec); + + let request = DepositRequest { + pubkey: deposit_data.pubkey, + withdrawal_credentials: deposit_data.withdrawal_credentials, + amount: deposit_data.amount, + signature: deposit_data.signature.clone(), + index: 0, + }; + + cache.cache_deposit_requests(&[request], &spec); + + assert_eq!(cache.get(&deposit_data), None); + } + + #[test] + fn get_by_deposit_data() { + let spec = gloas_spec(); + let cache = OnboardBuildersCache::new(&spec).unwrap(); + let deposit = make_valid_builder_deposit(&KEYPAIRS[0], &spec); + + let deposit_data = DepositData { + pubkey: deposit.pubkey, + withdrawal_credentials: deposit.withdrawal_credentials, + amount: deposit.amount, + signature: deposit.signature.clone(), + }; + + cache.cache_pending_deposits(vec![&deposit], &spec); + + assert_eq!(cache.get(&deposit_data), Some(true)); + } + + #[test] + fn empty_deposits_list_is_noop() { + let spec = gloas_spec(); + let cache = OnboardBuildersCache::new(&spec).unwrap(); + + cache.cache_pending_deposits(vec![], &spec); + cache.cache_deposit_requests(&[], &spec); + // No panic, no entries + } +} From f89c099d14dd8f27fae468e31cbe35265baafc70 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Tue, 19 May 2026 16:17:54 -0700 Subject: [PATCH 18/22] Remove SkipBuilderOnboarding variant and thread the onboard builder cache everywhere. --- beacon_node/beacon_chain/src/beacon_chain.rs | 15 ++++++++-- .../beacon_chain/src/beacon_proposer_cache.rs | 15 ++++++++-- .../beacon_chain/src/block_verification.rs | 17 +++++++++-- .../gossip_verified_payload_attestation.rs | 13 ++++++-- .../payload_attestation_verification/tests.rs | 2 ++ .../gossip_verified_envelope.rs | 4 +++ .../beacon_chain/src/state_advance_timer.rs | 2 +- beacon_node/beacon_chain/tests/store_tests.rs | 1 + beacon_node/http_api/src/attester_duties.rs | 15 ++++++++-- beacon_node/http_api/src/builder_states.rs | 9 ++++-- beacon_node/http_api/src/proposer_duties.rs | 29 ++++++++++-------- beacon_node/http_api/src/ptc_duties.rs | 15 ++++++++-- beacon_node/http_api/tests/tests.rs | 1 + .../src/builder_deposits_cache.rs | 5 +--- .../src/per_slot_processing.rs | 2 +- .../state_processing/src/state_advance.rs | 4 ++- .../state_processing/src/upgrade/gloas.rs | 30 ++++++------------- lcli/src/skip_slots.rs | 2 +- 18 files changed, 123 insertions(+), 58 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 1f8a4609f86..f58a0b829e1 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1521,7 +1521,9 @@ impl BeaconChain { match per_slot_processing( &mut state, skip_state_root, - GloasVerificationContext::from_cache(self.builder_onboarding_cache.clone()), + GloasVerificationContext::from_cache( + self.builder_onboarding_cache.as_deref(), + ), &self.spec, ) { Ok(_) => (), @@ -2126,6 +2128,7 @@ impl BeaconChain { &mut state, Some(advanced_state_root), request_epoch.start_slot(T::EthSpec::slots_per_epoch()), + self.builder_onboarding_cache.as_deref(), &self.spec, ) .map_err(Error::StateAdvanceError)?; @@ -5194,6 +5197,7 @@ impl BeaconChain { &mut advanced_state, Some(unadvanced_state_root), proposal_slot, + self.builder_onboarding_cache.as_deref(), &self.spec, )?; @@ -7003,6 +7007,7 @@ impl BeaconChain { proposal_epoch, accessor, state_provider, + self.builder_onboarding_cache.as_deref(), &self.spec, ) } @@ -7160,7 +7165,13 @@ impl BeaconChain { if state.current_epoch() + 1 < shuffling_epoch { // Advance the state into the required slot, using the "partial" method since the // state roots are not relevant for the shuffling. - partial_state_advance(&mut state, Some(state_root), target_slot, &self.spec)?; + partial_state_advance( + &mut state, + Some(state_root), + target_slot, + self.builder_onboarding_cache.as_deref(), + &self.spec, + )?; } metrics::stop_timer(state_skip_timer); diff --git a/beacon_node/beacon_chain/src/beacon_proposer_cache.rs b/beacon_node/beacon_chain/src/beacon_proposer_cache.rs index b258d7471f7..d8c337856db 100644 --- a/beacon_node/beacon_chain/src/beacon_proposer_cache.rs +++ b/beacon_node/beacon_chain/src/beacon_proposer_cache.rs @@ -15,6 +15,7 @@ use once_cell::sync::OnceCell; use parking_lot::Mutex; use safe_arith::SafeArith; use smallvec::SmallVec; +use state_processing::builder_deposits_cache::OnboardBuildersCache; use state_processing::state_advance::partial_state_advance; use std::num::NonZeroUsize; use std::sync::Arc; @@ -178,6 +179,7 @@ pub fn with_proposer_cache( proposal_epoch: Epoch, accessor: impl Fn(&EpochBlockProposers) -> Result, state_provider: impl FnOnce() -> Result<(Hash256, BeaconState), Err>, + builder_onboarding_cache: Option<&OnboardBuildersCache>, spec: &ChainSpec, ) -> Result where @@ -204,6 +206,7 @@ where &mut state, state_root, proposal_epoch, + builder_onboarding_cache, spec, )?; @@ -275,6 +278,7 @@ pub fn compute_proposer_duties_from_head( &mut state, head_state_root, request_epoch, + chain.builder_onboarding_cache.as_deref(), &chain.spec, )?; @@ -318,6 +322,7 @@ pub fn ensure_state_can_determine_proposers_for_epoch( state: &mut BeaconState, state_root: Hash256, target_epoch: Epoch, + builder_onboarding_cache: Option<&OnboardBuildersCache>, spec: &ChainSpec, ) -> Result<(), BeaconChainError> { // The decision slot is the end of an epoch, so we add 1 to reach the first slot of the epoch @@ -338,7 +343,13 @@ pub fn ensure_state_can_determine_proposers_for_epoch( } else { // State's current epoch is less than the minimum epoch. // Advance the state up to the minimum epoch. - partial_state_advance(state, Some(state_root), minimum_slot, spec) - .map_err(BeaconChainError::from) + partial_state_advance( + state, + Some(state_root), + minimum_slot, + builder_onboarding_cache, + spec, + ) + .map_err(BeaconChainError::from) } } diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 87c65bee8e6..83744e47345 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -79,6 +79,7 @@ use safe_arith::ArithError; use slot_clock::SlotClock; use ssz::Encode; use ssz_derive::{Decode, Encode}; +use state_processing::builder_deposits_cache::OnboardBuildersCache; use state_processing::per_block_processing::errors::IntoWithIndex; use state_processing::{ AllCaches, BlockProcessingError, BlockSignatureStrategy, ConsensusContext, @@ -609,6 +610,7 @@ pub fn signature_verify_chain_segment( &mut parent.pre_state, parent.beacon_state_root, highest_slot, + chain.builder_onboarding_cache.as_deref(), &chain.spec, )?; @@ -1109,6 +1111,7 @@ impl SignatureVerifiedBlock { &mut parent.pre_state, parent.beacon_state_root, block.slot(), + chain.builder_onboarding_cache.as_deref(), &chain.spec, )?; @@ -1180,6 +1183,7 @@ impl SignatureVerifiedBlock { &mut parent.pre_state, parent.beacon_state_root, block.slot(), + chain.builder_onboarding_cache.as_deref(), &chain.spec, )?; @@ -1545,7 +1549,7 @@ impl ExecutionPendingBlock { if let Some(summary) = per_slot_processing( &mut state, Some(state_root), - GloasVerificationContext::from_cache(chain.builder_onboarding_cache.clone()), + GloasVerificationContext::from_cache(chain.builder_onboarding_cache.as_deref()), &chain.spec, )? { // Expose Prometheus metrics. @@ -2094,6 +2098,7 @@ pub fn cheap_state_advance_to_obtain_committees<'a, E: EthSpec, Err: BlockBlobEr state: &'a mut BeaconState, state_root_opt: Option, block_slot: Slot, + builder_onboarding_cache: Option<&OnboardBuildersCache>, spec: &ChainSpec, ) -> Result>, Err> { let block_epoch = block_slot.epoch(E::slots_per_epoch()); @@ -2113,8 +2118,14 @@ pub fn cheap_state_advance_to_obtain_committees<'a, E: EthSpec, Err: BlockBlobEr // Advance the state into the same epoch as the block. Use the "partial" method since state // roots are not important for proposer/attester shuffling. - partial_state_advance(&mut state, state_root_opt, target_slot, spec) - .map_err(BeaconChainError::from)?; + partial_state_advance( + &mut state, + state_root_opt, + target_slot, + builder_onboarding_cache, + spec, + ) + .map_err(BeaconChainError::from)?; state.build_committee_cache(RelativeEpoch::Previous, spec)?; state.build_committee_cache(RelativeEpoch::Current, spec)?; diff --git a/beacon_node/beacon_chain/src/payload_attestation_verification/gossip_verified_payload_attestation.rs b/beacon_node/beacon_chain/src/payload_attestation_verification/gossip_verified_payload_attestation.rs index c36c73b344d..8d9935f55c0 100644 --- a/beacon_node/beacon_chain/src/payload_attestation_verification/gossip_verified_payload_attestation.rs +++ b/beacon_node/beacon_chain/src/payload_attestation_verification/gossip_verified_payload_attestation.rs @@ -10,6 +10,7 @@ use eth2::types::{EventKind, ForkVersionedResponse}; use parking_lot::RwLock; use safe_arith::SafeArith; use slot_clock::SlotClock; +use state_processing::builder_deposits_cache::OnboardBuildersCache; use state_processing::per_block_processing::signature_sets::indexed_payload_attestation_signature_set; use state_processing::state_advance::partial_state_advance; use std::borrow::Cow; @@ -18,6 +19,7 @@ use types::{ChainSpec, EthSpec, IndexedPayloadAttestation, PTC, PayloadAttestati pub struct GossipVerificationContext<'a, T: BeaconChainTypes> { pub slot_clock: &'a T::SlotClock, pub spec: &'a ChainSpec, + pub builder_onboarding_cache: Option<&'a OnboardBuildersCache>, pub observed_payload_attesters: &'a RwLock>, pub canonical_head: &'a CanonicalHead, pub validator_pubkey_cache: &'a RwLock>, @@ -113,8 +115,14 @@ impl VerifiedPayloadAttestationMessage { .map_err(BeaconChainError::from)? < message_epoch { - partial_state_advance(&mut state, Some(state_root), target_slot, ctx.spec) - .map_err(BeaconChainError::from)?; + partial_state_advance( + &mut state, + Some(state_root), + target_slot, + ctx.builder_onboarding_cache, + ctx.spec, + ) + .map_err(BeaconChainError::from)?; } Some(state) @@ -202,6 +210,7 @@ impl BeaconChain { GossipVerificationContext { slot_clock: &self.slot_clock, spec: &self.spec, + builder_onboarding_cache: self.builder_onboarding_cache.as_deref(), observed_payload_attesters: &self.observed_payload_attesters, canonical_head: &self.canonical_head, validator_pubkey_cache: &self.validator_pubkey_cache, diff --git a/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs b/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs index 7faad98e550..ca66e948875 100644 --- a/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs +++ b/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs @@ -115,6 +115,7 @@ impl TestContext { GossipVerificationContext { slot_clock: &self.slot_clock, spec: &self.spec, + builder_onboarding_cache: None, observed_payload_attesters: &self.observed_payload_attesters, canonical_head: &self.canonical_head, validator_pubkey_cache: &self.validator_pubkey_cache, @@ -373,6 +374,7 @@ async fn stale_head_with_partial_advance() { &mut reference_state, Some(head.snapshot.beacon_state_root()), target_slot, + harness.chain.builder_onboarding_cache.as_deref(), &harness.spec, ) .expect("should advance reference state"); diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs index a20963302b0..8b484d55314 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/gossip_verified_envelope.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use educe::Educe; use eth2::types::{EventKind, SseExecutionPayloadGossip}; use parking_lot::{Mutex, RwLock}; +use state_processing::builder_deposits_cache::OnboardBuildersCache; use store::DatabaseBlock; use tracing::debug; use types::{ @@ -26,6 +27,7 @@ pub struct GossipVerificationContext<'a, T: BeaconChainTypes> { pub canonical_head: &'a CanonicalHead, pub store: &'a BeaconStore, pub spec: &'a ChainSpec, + pub builder_onboarding_cache: Option<&'a OnboardBuildersCache>, pub beacon_proposer_cache: &'a Mutex, pub validator_pubkey_cache: &'a RwLock>, pub genesis_validators_root: Hash256, @@ -173,6 +175,7 @@ impl GossipVerifiedEnvelope { opt_snapshot = Some(Box::new(snapshot.clone())); Ok::<_, EnvelopeError>((snapshot.state_root, snapshot.pre_state)) }, + ctx.builder_onboarding_cache, ctx.spec, )?; let expected_proposer = proposer.index; @@ -247,6 +250,7 @@ impl BeaconChain { canonical_head: &self.canonical_head, store: &self.store, spec: &self.spec, + builder_onboarding_cache: self.builder_onboarding_cache.as_deref(), beacon_proposer_cache: &self.beacon_proposer_cache, validator_pubkey_cache: &self.validator_pubkey_cache, genesis_validators_root: self.genesis_validators_root, diff --git a/beacon_node/beacon_chain/src/state_advance_timer.rs b/beacon_node/beacon_chain/src/state_advance_timer.rs index 4833a391e37..7c32324b637 100644 --- a/beacon_node/beacon_chain/src/state_advance_timer.rs +++ b/beacon_node/beacon_chain/src/state_advance_timer.rs @@ -289,7 +289,7 @@ fn advance_head(beacon_chain: &Arc>) -> Resu if let Some(summary) = per_slot_processing( &mut state, Some(head_state_root), - GloasVerificationContext::from_cache(beacon_chain.builder_onboarding_cache.clone()), + GloasVerificationContext::from_cache(beacon_chain.builder_onboarding_cache.as_deref()), &beacon_chain.spec, ) .map_err(BeaconChainError::from)? diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 0ff9f6841de..3e5ff2dd770 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -1687,6 +1687,7 @@ async fn proposer_lookahead_gloas_fork_epoch() { &mut head_state, head_state_root, gloas_fork_epoch, + None, spec, ) .unwrap(); diff --git a/beacon_node/http_api/src/attester_duties.rs b/beacon_node/http_api/src/attester_duties.rs index b42e474b5c4..805e5d5aba3 100644 --- a/beacon_node/http_api/src/attester_duties.rs +++ b/beacon_node/http_api/src/attester_duties.rs @@ -4,6 +4,7 @@ use crate::state_id::StateId; use beacon_chain::{BeaconChain, BeaconChainError, BeaconChainTypes}; use eth2::types::{self as api_types}; use slot_clock::SlotClock; +use state_processing::builder_deposits_cache::OnboardBuildersCache; use state_processing::state_advance::partial_state_advance; use types::{AttestationDuty, BeaconState, ChainSpec, Epoch, EthSpec, Hash256, RelativeEpoch}; @@ -113,6 +114,7 @@ fn compute_historic_attester_duties( &mut state, state_root, request_epoch, + chain.builder_onboarding_cache.as_deref(), &chain.spec, )?; (state, execution_optimistic) @@ -171,6 +173,7 @@ fn ensure_state_knows_attester_duties_for_epoch( state: &mut BeaconState, state_root: Hash256, target_epoch: Epoch, + builder_onboarding_cache: Option<&OnboardBuildersCache>, spec: &ChainSpec, ) -> Result<(), warp::reject::Rejection> { // Protect against an inconsistent slot clock. @@ -188,9 +191,15 @@ fn ensure_state_knows_attester_duties_for_epoch( .start_slot(E::slots_per_epoch()); // A "partial" state advance is adequate since attester duties don't rely on state roots. - partial_state_advance(state, Some(state_root), target_slot, spec) - .map_err(BeaconChainError::from) - .map_err(warp_utils::reject::unhandled_error)?; + partial_state_advance( + state, + Some(state_root), + target_slot, + builder_onboarding_cache, + spec, + ) + .map_err(BeaconChainError::from) + .map_err(warp_utils::reject::unhandled_error)?; } Ok(()) diff --git a/beacon_node/http_api/src/builder_states.rs b/beacon_node/http_api/src/builder_states.rs index 73e01debcd7..cfec7167428 100644 --- a/beacon_node/http_api/src/builder_states.rs +++ b/beacon_node/http_api/src/builder_states.rs @@ -22,8 +22,13 @@ pub fn get_next_withdrawals( let proposal_epoch = proposal_slot.epoch(T::EthSpec::slots_per_epoch()); let (state_root, _, _) = state_id.root(chain)?; if proposal_epoch != state.current_epoch() - && let Err(e) = - partial_state_advance(&mut state, Some(state_root), proposal_slot, &chain.spec) + && let Err(e) = partial_state_advance( + &mut state, + Some(state_root), + proposal_slot, + chain.builder_onboarding_cache.as_deref(), + &chain.spec, + ) { return Err(warp_utils::reject::custom_server_error(format!( "failed to advance to the epoch of the proposal slot: {:?}", diff --git a/beacon_node/http_api/src/proposer_duties.rs b/beacon_node/http_api/src/proposer_duties.rs index 0b0926f955c..3a832661bab 100644 --- a/beacon_node/http_api/src/proposer_duties.rs +++ b/beacon_node/http_api/src/proposer_duties.rs @@ -251,19 +251,24 @@ fn compute_historic_proposer_duties( } }; - let (state, execution_optimistic) = if let Some((state_root, mut state, execution_optimistic)) = - state_opt - { - // If we've loaded the head state it might be from a previous epoch, ensure it's in a - // suitable epoch. - ensure_state_can_determine_proposers_for_epoch(&mut state, state_root, epoch, &chain.spec) + let (state, execution_optimistic) = + if let Some((state_root, mut state, execution_optimistic)) = state_opt { + // If we've loaded the head state it might be from a previous epoch, ensure it's in a + // suitable epoch. + ensure_state_can_determine_proposers_for_epoch( + &mut state, + state_root, + epoch, + chain.builder_onboarding_cache.as_deref(), + &chain.spec, + ) .map_err(warp_utils::reject::unhandled_error)?; - (state, execution_optimistic) - } else { - let (state, execution_optimistic, _finalized) = - StateId::from_slot(epoch.start_slot(T::EthSpec::slots_per_epoch())).state(chain)?; - (state, execution_optimistic) - }; + (state, execution_optimistic) + } else { + let (state, execution_optimistic, _finalized) = + StateId::from_slot(epoch.start_slot(T::EthSpec::slots_per_epoch())).state(chain)?; + (state, execution_optimistic) + }; // Ensure the state lookup was correct. if state.current_epoch() != epoch && state.current_epoch() + 1 != epoch { diff --git a/beacon_node/http_api/src/ptc_duties.rs b/beacon_node/http_api/src/ptc_duties.rs index f727b840048..6d27fa565a2 100644 --- a/beacon_node/http_api/src/ptc_duties.rs +++ b/beacon_node/http_api/src/ptc_duties.rs @@ -4,6 +4,7 @@ use crate::state_id::StateId; use beacon_chain::{BeaconChain, BeaconChainError, BeaconChainTypes}; use eth2::types::{self as api_types, PtcDuty}; use slot_clock::SlotClock; +use state_processing::builder_deposits_cache::OnboardBuildersCache; use state_processing::state_advance::partial_state_advance; use types::{BeaconState, ChainSpec, Epoch, EthSpec, Hash256}; @@ -114,6 +115,7 @@ fn compute_ptc_duties_from_state( &mut state, state_root, request_epoch, + chain.builder_onboarding_cache.as_deref(), &chain.spec, )?; (state, execution_optimistic) @@ -148,6 +150,7 @@ fn ensure_state_knows_ptc_duties_for_epoch( state: &mut BeaconState, state_root: Hash256, target_epoch: Epoch, + builder_onboarding_cache: Option<&OnboardBuildersCache>, spec: &ChainSpec, ) -> Result<(), warp::reject::Rejection> { if state.current_epoch() > target_epoch { @@ -161,9 +164,15 @@ fn ensure_state_knows_ptc_duties_for_epoch( .saturating_sub(1_u64) .start_slot(E::slots_per_epoch()); - partial_state_advance(state, Some(state_root), target_slot, spec) - .map_err(BeaconChainError::from) - .map_err(warp_utils::reject::unhandled_error)?; + partial_state_advance( + state, + Some(state_root), + target_slot, + builder_onboarding_cache, + spec, + ) + .map_err(BeaconChainError::from) + .map_err(warp_utils::reject::unhandled_error)?; } Ok(()) diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index d1922f2d883..561727edbba 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -7539,6 +7539,7 @@ impl ApiTester { &mut state, Some(state_root), proposal_slot, + self.chain.builder_onboarding_cache.as_deref(), &self.chain.spec, ); } diff --git a/consensus/state_processing/src/builder_deposits_cache.rs b/consensus/state_processing/src/builder_deposits_cache.rs index 346ad646ada..49fe12ae08c 100644 --- a/consensus/state_processing/src/builder_deposits_cache.rs +++ b/consensus/state_processing/src/builder_deposits_cache.rs @@ -403,10 +403,7 @@ mod tests { let invalid_0 = make_invalid_builder_deposit(&KEYPAIRS[2], &spec); let invalid_1 = make_invalid_builder_deposit(&KEYPAIRS[3], &spec); - cache.cache_pending_deposits( - vec![&valid_0, &invalid_0, &valid_1, &invalid_1], - &spec, - ); + cache.cache_pending_deposits(vec![&valid_0, &invalid_0, &valid_1, &invalid_1], &spec); assert_eq!(cache.cached_is_valid_signature(&valid_0), Some(true)); assert_eq!(cache.cached_is_valid_signature(&valid_1), Some(true)); diff --git a/consensus/state_processing/src/per_slot_processing.rs b/consensus/state_processing/src/per_slot_processing.rs index ccf7ab447a2..75430896269 100644 --- a/consensus/state_processing/src/per_slot_processing.rs +++ b/consensus/state_processing/src/per_slot_processing.rs @@ -39,7 +39,7 @@ impl From for Error { pub fn per_slot_processing( state: &mut BeaconState, state_root: Option, - gloas_context: GloasVerificationContext, + gloas_context: GloasVerificationContext<'_>, spec: &ChainSpec, ) -> Result>, Error> { // Verify that the `BeaconState` instantiation matches the fork at `state.slot()`. diff --git a/consensus/state_processing/src/state_advance.rs b/consensus/state_processing/src/state_advance.rs index cb3c0d554dd..a72e66ce0cd 100644 --- a/consensus/state_processing/src/state_advance.rs +++ b/consensus/state_processing/src/state_advance.rs @@ -4,6 +4,7 @@ //! These functions are not in the specification, however they're defined here to reduce code //! duplication and protect against some easy-to-make mistakes when performing state advances. +use crate::builder_deposits_cache::OnboardBuildersCache; use crate::upgrade::GloasVerificationContext; use crate::*; use fixed_bytes::FixedBytesExtended; @@ -72,6 +73,7 @@ pub fn partial_state_advance( state: &mut BeaconState, state_root_opt: Option, target_slot: Slot, + builder_onboarding_cache: Option<&OnboardBuildersCache>, spec: &ChainSpec, ) -> Result<(), Error> { check_target_slot(state.slot(), target_slot)?; @@ -105,7 +107,7 @@ pub fn partial_state_advance( per_slot_processing( state, Some(state_root), - GloasVerificationContext::SkipBuilderOnboarding, + GloasVerificationContext::from_cache(builder_onboarding_cache), spec, ) .map_err(Error::PerSlotProcessing)?; diff --git a/consensus/state_processing/src/upgrade/gloas.rs b/consensus/state_processing/src/upgrade/gloas.rs index 7ed832c3286..005bd9afc19 100644 --- a/consensus/state_processing/src/upgrade/gloas.rs +++ b/consensus/state_processing/src/upgrade/gloas.rs @@ -8,7 +8,6 @@ use ssz_types::BitVector; use ssz_types::FixedVector; use std::collections::HashMap; use std::mem; -use std::sync::Arc; use tracing::instrument; use tree_hash::TreeHash; use typenum::Unsigned; @@ -23,26 +22,19 @@ use types::{ /// This is also useful for caching builder deposit signatures post gloas so that /// potentially expensive operations doesn't slow down block processing in the block verification /// path. -pub enum GloasVerificationContext { +pub enum GloasVerificationContext<'a> { /// Use the pre-computed builder signature cache. - CachedVerification(Arc), + CachedVerification(&'a OnboardBuildersCache), /// Verify each builder deposit signature individually. /// /// This can be significantly slower if there are many builder deposits /// that need to be onboarded at the fork boundary. This variant should be used /// for tests and other non-production paths. FullVerification, - /// Skip `onboard_builders_from_pending_deposits` but still initialize the PTC window. - /// - /// `onboard_builders_from_pending_deposits` only modifies `state.pending_deposits` and - /// `state.builders` which don't affect committee or PTC calculation. This variant is - /// used by `partial_state_advance` where callers need committee and PTC lookups but - /// don't need the builder registry to be fully populated. - SkipBuilderOnboarding, } -impl GloasVerificationContext { - pub fn from_cache(cache: Option>) -> Self { +impl<'a> GloasVerificationContext<'a> { + pub fn from_cache(cache: Option<&'a OnboardBuildersCache>) -> Self { match cache { Some(c) => GloasVerificationContext::CachedVerification(c), None => GloasVerificationContext::FullVerification, @@ -54,7 +46,7 @@ impl GloasVerificationContext { #[instrument(skip_all)] pub fn upgrade_to_gloas( pre_state: &mut BeaconState, - context: GloasVerificationContext, + context: GloasVerificationContext<'_>, spec: &ChainSpec, ) -> Result<(), Error> { let post = upgrade_state_to_gloas(pre_state, context, spec)?; @@ -66,7 +58,7 @@ pub fn upgrade_to_gloas( pub fn upgrade_state_to_gloas( pre_state: &mut BeaconState, - context: GloasVerificationContext, + context: GloasVerificationContext<'_>, spec: &ChainSpec, ) -> Result, Error> { let epoch = pre_state.current_epoch(); @@ -160,17 +152,13 @@ pub fn upgrade_state_to_gloas( epoch_cache: mem::take(&mut pre.epoch_cache), }); // [New in Gloas:EIP7732] + initialize_ptc_window(&mut post, spec)?; match context { - GloasVerificationContext::CachedVerification(ref cache) => { - onboard_builders_from_pending_deposits(&mut post, Some(cache.as_ref()), spec)?; - initialize_ptc_window(&mut post, spec)?; + GloasVerificationContext::CachedVerification(cache) => { + onboard_builders_from_pending_deposits(&mut post, Some(cache), spec)?; } GloasVerificationContext::FullVerification => { onboard_builders_from_pending_deposits(&mut post, None, spec)?; - initialize_ptc_window(&mut post, spec)?; - } - GloasVerificationContext::SkipBuilderOnboarding => { - initialize_ptc_window(&mut post, spec)?; } } diff --git a/lcli/src/skip_slots.rs b/lcli/src/skip_slots.rs index 88332c1a850..3ccfdf2c68a 100644 --- a/lcli/src/skip_slots.rs +++ b/lcli/src/skip_slots.rs @@ -134,7 +134,7 @@ pub fn run( let start = Instant::now(); if partial { - partial_state_advance(&mut state, Some(state_root), target_slot, spec) + partial_state_advance(&mut state, Some(state_root), target_slot, None, spec) .map_err(|e| format!("Unable to perform partial advance: {:?}", e))?; } else { complete_state_advance(&mut state, Some(state_root), target_slot, spec) From de89987bc100a6d8e3ce2198d6c48264e2db1ed2 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Tue, 19 May 2026 16:49:24 -0700 Subject: [PATCH 19/22] Add more tests --- .../src/per_block_processing/tests.rs | 278 ++++++++++++++++++ 1 file changed, 278 insertions(+) diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index 09b46dc658e..a031fa1a49f 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -1,10 +1,12 @@ #![cfg(all(test, not(feature = "fake_crypto"), not(debug_assertions)))] +use crate::builder_deposits_cache::OnboardBuildersCache; use crate::per_block_processing::errors::{ AttestationInvalid, AttesterSlashingInvalid, BlockOperationError, BlockProcessingError, DepositInvalid, HeaderInvalid, IndexedAttestationInvalid, IntoWithIndex, ProposerSlashingInvalid, }; +use crate::upgrade::gloas::GloasVerificationContext; use crate::{BlockReplayError, BlockReplayer, per_block_processing}; use crate::{ BlockSignatureStrategy, ConsensusContext, VerifyBlockRoot, VerifySignatures, @@ -766,6 +768,282 @@ async fn process_deposit_requests_post_gloas_preserves_pre_state_pending_validat ); } +// Tests that exercise the cached verification path used in production. + +#[tokio::test] +async fn process_deposit_requests_with_pre_seeded_cache() { + let harness = get_gloas_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; + let spec = harness.spec.clone(); + let mut state = harness.get_current_state(); + + let valid_builder_request = make_deposit_request( + &KEYPAIRS[VALIDATOR_COUNT + 1], + builder_withdrawal_credentials(&spec), + 13, + &spec, + 0, + ); + + let mut invalid_builder_request = make_deposit_request( + &KEYPAIRS[VALIDATOR_COUNT + 2], + builder_withdrawal_credentials(&spec), + 17, + &spec, + 1, + ); + invalid_builder_request.signature = SignatureBytes::empty(); + + let deposit_requests = [ + valid_builder_request.clone(), + invalid_builder_request.clone(), + ]; + + // Pre-seed the cache with these deposit requests (simulating payload envelope arrival) + let cache = OnboardBuildersCache::new(&spec).unwrap(); + cache.cache_deposit_requests(&deposit_requests, &spec); + + process_operations::process_deposit_requests_post_gloas( + &mut state, + &deposit_requests, + Some(&cache), + &spec, + ) + .unwrap(); + + // Valid builder should be onboarded via cache hit + assert!(find_builder_index(&state, &valid_builder_request.pubkey).is_some()); + // Invalid signature should be rejected via cache hit + assert!(find_builder_index(&state, &invalid_builder_request.pubkey).is_none()); +} + +#[tokio::test] +async fn process_deposit_requests_cache_partial_hit() { + let harness = get_gloas_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; + let spec = harness.spec.clone(); + let mut state = harness.get_current_state(); + + let cached_request = make_deposit_request( + &KEYPAIRS[VALIDATOR_COUNT + 1], + builder_withdrawal_credentials(&spec), + 13, + &spec, + 0, + ); + + let uncached_request = make_deposit_request( + &KEYPAIRS[VALIDATOR_COUNT + 2], + builder_withdrawal_credentials(&spec), + 17, + &spec, + 1, + ); + + // Only seed cache with first request + let cache = OnboardBuildersCache::new(&spec).unwrap(); + cache.cache_deposit_requests(std::slice::from_ref(&cached_request), &spec); + + process_operations::process_deposit_requests_post_gloas( + &mut state, + &[cached_request.clone(), uncached_request.clone()], + Some(&cache), + &spec, + ) + .unwrap(); + + // Both should be onboarded: one from cache, one from batch verification fallback + assert!(find_builder_index(&state, &cached_request.pubkey).is_some()); + assert!(find_builder_index(&state, &uncached_request.pubkey).is_some()); +} + +/// Helper to create a harness with Fulu genesis and gloas at a later epoch. +async fn get_fulu_harness_with_gloas_scheduled( + gloas_epoch: u64, + num_validators: usize, +) -> BeaconChainHarness> { + let mut spec = ForkName::Fulu.make_genesis_spec(E::default_spec()); + spec.gloas_fork_epoch = Some(Epoch::new(gloas_epoch)); + let spec = Arc::new(spec); + let last_slot_of_pre_gloas_epoch = Epoch::new(gloas_epoch).start_slot(E::slots_per_epoch()) - 1; + let harness = BeaconChainHarness::>::builder(E::default()) + .spec(spec.clone()) + .keypairs(KEYPAIRS[0..num_validators].to_vec()) + .fresh_ephemeral_store() + .mock_execution_layer() + .build(); + let state = harness.get_current_state(); + if last_slot_of_pre_gloas_epoch > Slot::new(0) { + harness + .add_attested_blocks_at_slots( + state, + (1..=last_slot_of_pre_gloas_epoch.as_u64()) + .map(Slot::new) + .collect::>() + .as_slice(), + (0..num_validators).collect::>().as_slice(), + ) + .await; + } + harness +} + +#[tokio::test] +async fn upgrade_to_gloas_with_cached_verification() { + let harness = + get_fulu_harness_with_gloas_scheduled::(EPOCH_OFFSET, VALIDATOR_COUNT) + .await; + let spec = harness.spec.clone(); + let mut state = harness.get_current_state(); + + // Add builder pending deposits to the pre-gloas state + let valid_keypair = &KEYPAIRS[VALIDATOR_COUNT + 1]; + let invalid_keypair = &KEYPAIRS[VALIDATOR_COUNT + 2]; + let builder_creds = builder_withdrawal_credentials(&spec); + + let mut valid_deposit_data = DepositData { + pubkey: valid_keypair.pk.compress(), + withdrawal_credentials: builder_creds, + amount: 256_000_000_000, + signature: SignatureBytes::empty(), + }; + valid_deposit_data.signature = valid_deposit_data.create_signature(&valid_keypair.sk, &spec); + + let invalid_deposit_data = DepositData { + pubkey: invalid_keypair.pk.compress(), + withdrawal_credentials: builder_creds, + amount: 256_000_000_000, + signature: SignatureBytes::empty(), + }; + + let slot = state.slot(); + state + .pending_deposits_mut() + .unwrap() + .push(PendingDeposit { + pubkey: valid_deposit_data.pubkey, + withdrawal_credentials: valid_deposit_data.withdrawal_credentials, + amount: valid_deposit_data.amount, + signature: valid_deposit_data.signature.clone(), + slot, + }) + .unwrap(); + state + .pending_deposits_mut() + .unwrap() + .push(PendingDeposit { + pubkey: invalid_deposit_data.pubkey, + withdrawal_credentials: invalid_deposit_data.withdrawal_credentials, + amount: invalid_deposit_data.amount, + signature: invalid_deposit_data.signature.clone(), + slot, + }) + .unwrap(); + + // Seed the cache from the state (production path) + let cache = OnboardBuildersCache::new(&spec).unwrap(); + cache.seed_from_state(&state, &spec); + + // Run upgrade_to_gloas with cached verification + crate::upgrade::upgrade_to_gloas( + &mut state, + GloasVerificationContext::CachedVerification(&cache), + &spec, + ) + .unwrap(); + + // Valid builder deposit should be onboarded + assert!(find_builder_index(&state, &valid_deposit_data.pubkey).is_some()); + // Invalid signature should be rejected (not added to builders) + assert!(find_builder_index(&state, &invalid_deposit_data.pubkey).is_none()); + // Invalid builder deposit is consumed (dropped) during onboarding, not kept in pending + let pending = state.pending_deposits().unwrap(); + assert!( + !pending + .iter() + .any(|d| d.pubkey == invalid_deposit_data.pubkey) + ); +} + +#[tokio::test] +async fn upgrade_to_gloas_cached_matches_full_verification() { + let harness = + get_fulu_harness_with_gloas_scheduled::(EPOCH_OFFSET, VALIDATOR_COUNT) + .await; + let spec = harness.spec.clone(); + let mut state = harness.get_current_state(); + + // Add multiple builder deposits: some valid, some invalid + let builder_creds = builder_withdrawal_credentials(&spec); + let slot = state.slot(); + + for i in 0..4 { + let keypair = &KEYPAIRS[VALIDATOR_COUNT + i + 1]; + let mut deposit_data = DepositData { + pubkey: keypair.pk.compress(), + withdrawal_credentials: builder_creds, + amount: 256_000_000_000, + signature: SignatureBytes::empty(), + }; + // Make even-indexed deposits valid, odd-indexed invalid + if i % 2 == 0 { + deposit_data.signature = deposit_data.create_signature(&keypair.sk, &spec); + } + state + .pending_deposits_mut() + .unwrap() + .push(PendingDeposit { + pubkey: deposit_data.pubkey, + withdrawal_credentials: deposit_data.withdrawal_credentials, + amount: deposit_data.amount, + signature: deposit_data.signature.clone(), + slot, + }) + .unwrap(); + } + + // Run with full verification to get ground truth + let mut state_full = state.clone(); + crate::upgrade::upgrade_to_gloas( + &mut state_full, + GloasVerificationContext::FullVerification, + &spec, + ) + .unwrap(); + + // Run with cached verification + let mut state_cached = state.clone(); + let cache = OnboardBuildersCache::new(&spec).unwrap(); + cache.seed_from_state(&state_cached, &spec); + crate::upgrade::upgrade_to_gloas( + &mut state_cached, + GloasVerificationContext::CachedVerification(&cache), + &spec, + ) + .unwrap(); + + // Both paths should produce identical builder registries + let builders_full = state_full.builders().unwrap(); + let builders_cached = state_cached.builders().unwrap(); + assert_eq!(builders_full.len(), builders_cached.len()); + for i in 0..builders_full.len() { + let bf = builders_full.get(i).unwrap(); + let bc = builders_cached.get(i).unwrap(); + assert_eq!(bf.pubkey, bc.pubkey); + assert_eq!(bf.balance, bc.balance); + assert_eq!(bf.execution_address, bc.execution_address); + } + + // Both paths should produce identical pending_deposits + let pending_full = state_full.pending_deposits().unwrap(); + let pending_cached = state_cached.pending_deposits().unwrap(); + assert_eq!(pending_full.len(), pending_cached.len()); + for i in 0..pending_full.len() { + assert_eq!( + pending_full.get(i).unwrap().pubkey, + pending_cached.get(i).unwrap().pubkey + ); + } +} + #[tokio::test] async fn invalid_attestation_no_committee_for_index() { let harness = get_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; From 70b859491f8565c885e2648a192a92419a1deb01 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Thu, 21 May 2026 14:44:01 -0700 Subject: [PATCH 20/22] Optimise by not iterating through the state.builders list for every insertion --- beacon_node/beacon_chain/src/beacon_chain.rs | 2 +- .../process_operations.rs | 294 ++++++++---------- consensus/types/src/deposit/deposit_data.rs | 23 ++ 3 files changed, 158 insertions(+), 161 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index f58a0b829e1..3448524365b 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -4555,7 +4555,7 @@ impl BeaconChain { let executor = self.task_executor.clone(); // Using rayon pool here since `add_new_pending_deposits` uses rayon threads to // perform the signature verification in batches. - // // We have until the fork transition for the cache to be used, so we use the low priority pool. + // We have until the fork transition for the cache to be used, so we use the low priority pool. executor.spawn_blocking_with_rayon( move || cache.add_new_pending_deposits::(&state, &spec), RayonPoolType::LowPriority, diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index 8bdff56e42d..aef9a20f8d6 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -1,4 +1,4 @@ -use std::time::Instant; +use std::collections::{HashMap, VecDeque}; use super::*; use crate::VerifySignatures; @@ -13,11 +13,12 @@ use crate::per_block_processing::builder::{ use crate::per_block_processing::errors::{BlockProcessingError, ExitInvalid, IntoWithIndex}; use crate::per_block_processing::signature_sets::{exit_signature_set, get_pubkey_from_state}; use crate::per_block_processing::verify_payload_attestation::verify_payload_attestation; -use bls::{PublicKeyBytes, SignatureBytes}; +use bls::PublicKeyBytes; use milhouse::List; use ssz_types::FixedVector; use typenum::U33; use types::consts::altair::{PARTICIPATION_FLAG_WEIGHTS, PROPOSER_WEIGHT, WEIGHT_DENOMINATOR}; +use types::{Address, Builder}; pub fn process_operations>( state: &mut BeaconState, @@ -908,7 +909,6 @@ pub fn process_deposit_requests_pre_gloas( /// Check if there is a pending deposit for a new validator with the given pubkey. // TODO(gloas): cache the deposit signature validation or remove this loop entirely if possible, // it is `O(n * m)` where `n` is max 8192 and `m` is max 128M. -#[instrument(skip_all, level = "debug")] pub fn is_pending_validator( deposits: &List, pubkey: &PublicKeyBytes, @@ -938,106 +938,86 @@ pub enum VerifyBuilderSignature { VerifiedInvalid, } -impl From> for VerifyBuilderSignature { - fn from(value: Option) -> Self { - match value { - Some(true) => VerifyBuilderSignature::VerifiedValid, - Some(false) => VerifyBuilderSignature::VerifiedInvalid, - None => VerifyBuilderSignature::Verify, - } - } -} - -/// Returns `Ok(true)` if a builder with the given pubkey exists in the state. -/// Otherwise returns `Ok(false)`. Returns an error if this function is called with a -/// pre-gloas state. -/// -/// TODO(gloas): this could be more efficient in the builder case, see: -/// https://github.com/sigp/lighthouse/issues/8783 -#[instrument(skip_all, level = "debug")] -fn get_builder_index( - state: &BeaconState, - pubkey: &PublicKeyBytes, -) -> Result, BlockProcessingError> { - Ok(state - .builders()? - .iter() - .enumerate() - .find(|(_, builder)| builder.pubkey == *pubkey) - .map(|(i, _)| i as u64)) -} - pub fn process_deposit_requests_post_gloas( state: &mut BeaconState, deposit_requests: &[DepositRequest], onboarding_cache: Option<&OnboardBuildersCache>, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { - let now = Instant::now(); - let mut builder_signature_request_indices = Vec::with_capacity(deposit_requests.len()); - let mut builder_signature_deposits = Vec::with_capacity(deposit_requests.len()); + let mut new_builder_signature_candidates = Vec::with_capacity(deposit_requests.len()); + let slot = state.slot(); + let current_epoch = state.current_epoch(); - // Step 1: Collect all requests that could create a new builder when evaluated against the current - // state. + // Contains the pubkey to index mapping for existing builders and any new builders + // added with the `deposit_requests` coming in. + // TODO(gloas): can potentially skip this if we add a builder pubkey cache later. + let state_builders = state.builders()?; + let mut builder_index_map = HashMap::with_capacity(state_builders.len()); + // A one pass cache for repeated calls to `get_index_for_new_builder`. + // state.builders() allows reusing indices. This holds the list of indices where + // potentially new builders can be inserted. + // Doing this avoids walking through the entire `state.builders` list and appending + // in the end everytime we want to insert a new builder. + // For a high number of builder deposits, this can take quite long. + let mut reusable_builder_indices = VecDeque::new(); + + for (index, builder) in state_builders.iter().enumerate() { + builder_index_map.insert(builder.pubkey, index as BuilderIndex); + if builder.withdrawable_epoch <= current_epoch && builder.balance == 0 { + reusable_builder_indices.push_back(index as BuilderIndex); + } + } + + // Step 1: Collect all requests that could create a new builder when evaluated against the + // current state. for (request_index, deposit_request) in deposit_requests.iter().enumerate() { - let is_builder = get_builder_index(state, &deposit_request.pubkey)?.is_some(); + let is_builder = builder_index_map.contains_key(&deposit_request.pubkey); let is_validator = state .get_validator_index(&deposit_request.pubkey)? .is_some(); let has_builder_prefix = is_builder_withdrawal_credential(deposit_request.withdrawal_credentials, spec); - // Intentionally not checking `!is_pending_validator` to avoid potentially expensive operation - // in the first pass. This may result in more signatures to verify but potentially avoids a double - // loop with a signature verification. - // TODO(pawan): reevaluate if this is best for the worst case. if !is_builder && has_builder_prefix && !is_validator { - builder_signature_request_indices.push(request_index); - builder_signature_deposits.push(DepositData { - pubkey: deposit_request.pubkey, - withdrawal_credentials: deposit_request.withdrawal_credentials, - amount: deposit_request.amount, - signature: deposit_request.signature.clone(), - }); + new_builder_signature_candidates.push(( + request_index, + DepositData { + pubkey: deposit_request.pubkey, + withdrawal_credentials: deposit_request.withdrawal_credentials, + amount: deposit_request.amount, + signature: deposit_request.signature.clone(), + }, + )); } } // Step 2: Check cache for already-verified signatures, batch verify the rest. - let sig_now = Instant::now(); let mut builder_signature_results_by_request = vec![VerifyBuilderSignature::Verify; deposit_requests.len()]; - let mut uncached_indices = Vec::new(); + let mut uncached_request_indices = Vec::new(); let mut uncached_deposits = Vec::new(); - for (deposit_data, &request_index) in builder_signature_deposits - .iter() - .zip(builder_signature_request_indices.iter()) - { + for (request_index, deposit_data) in &new_builder_signature_candidates { let cached_result = onboarding_cache.and_then(|c| c.get(deposit_data)); - if let Some(is_valid) = cached_result { - if let Some(res) = builder_signature_results_by_request.get_mut(request_index) { - *res = if is_valid { - VerifyBuilderSignature::VerifiedValid - } else { - VerifyBuilderSignature::VerifiedInvalid - }; - } - } else { - uncached_indices.push(request_index); - uncached_deposits.push(deposit_data.clone()); + if let Some(result) = builder_signature_results_by_request.get_mut(*request_index) { + *result = match cached_result { + Some(true) => VerifyBuilderSignature::VerifiedValid, + Some(false) => VerifyBuilderSignature::VerifiedInvalid, + None => { + uncached_request_indices.push(*request_index); + uncached_deposits.push(deposit_data.clone()); + VerifyBuilderSignature::Verify + } + }; } } - let cache_hits = builder_signature_deposits - .len() - .saturating_sub(uncached_deposits.len()); - let batch_len = uncached_deposits.len(); let batch_results = is_valid_deposit_signature_batch(uncached_deposits, spec); - for (&request_index, is_valid) in uncached_indices.iter().zip(batch_results) { - if let Some(slot) = builder_signature_results_by_request.get_mut(request_index) { - *slot = if is_valid { + for (&request_index, is_valid) in uncached_request_indices.iter().zip(batch_results) { + if let Some(res) = builder_signature_results_by_request.get_mut(request_index) { + *res = if is_valid { VerifyBuilderSignature::VerifiedValid } else { VerifyBuilderSignature::VerifiedInvalid @@ -1045,17 +1025,10 @@ pub fn process_deposit_requests_post_gloas( } } - tracing::debug!( - verified = batch_len, - cache_hits, - time = sig_now.elapsed().as_millis(), - "Completed processing deposit signatures" - ); - // Step 3: Second pass over the requests that is equivalent to the spec function only // with the signature verification cached. for (request_index, deposit_request) in deposit_requests.iter().enumerate() { - let builder_index = get_builder_index(state, &deposit_request.pubkey)?; + let builder_index = builder_index_map.get(&deposit_request.pubkey).copied(); let is_builder = builder_index.is_some(); let is_validator = state @@ -1063,7 +1036,6 @@ pub fn process_deposit_requests_post_gloas( .is_some(); let has_builder_prefix = is_builder_withdrawal_credential(deposit_request.withdrawal_credentials, spec); - if is_builder || (has_builder_prefix && !is_validator @@ -1073,24 +1045,88 @@ pub fn process_deposit_requests_post_gloas( spec, )) { - apply_deposit_for_builder( - state, - builder_index, - deposit_request.pubkey, - deposit_request.withdrawal_credentials, - deposit_request.amount, - deposit_request.signature.clone(), - state.slot(), - builder_signature_results_by_request - .get(request_index) - .copied() - .unwrap_or(VerifyBuilderSignature::Verify), - spec, - )?; - continue; + // Directly increase the balance if existing builder and move on + // to the next deposit. + // The signature validity is irrelevant in the case when + // the builder already exists in the state. + if let Some(builder_index) = builder_index { + state + .builders_mut()? + .get_mut(builder_index as usize) + .ok_or(BeaconStateError::UnknownBuilder(builder_index))? + .balance + .safe_add_assign(deposit_request.amount)?; + continue; + } + + // If the builder does not exist in the state, then we need to + // verify the signature and only add builders that have a valid + // signature. + let verify_signature = builder_signature_results_by_request + .get(request_index) + .copied() + .unwrap_or(VerifyBuilderSignature::Verify); + + let is_valid = match verify_signature { + VerifyBuilderSignature::Verify => { + let deposit_data = deposit_request.into(); + is_valid_deposit_signature(&deposit_data, spec).is_ok() + } + VerifyBuilderSignature::VerifiedValid => true, + VerifyBuilderSignature::VerifiedInvalid => false, + }; + + // Signature is valid, create the builder and insert it in the correct location + if is_valid { + let version = *deposit_request + .withdrawal_credentials + .as_slice() + .first() + .ok_or(BeaconStateError::WithdrawalCredentialMissingVersion)?; + let execution_address = deposit_request + .withdrawal_credentials + .as_slice() + .get(12..) + .and_then(|bytes| Address::try_from(bytes).ok()) + .ok_or(BeaconStateError::WithdrawalCredentialMissingAddress)?; + + let builder = Builder { + pubkey: deposit_request.pubkey, + version, + execution_address, + balance: deposit_request.amount, + deposit_epoch: slot.epoch(E::slots_per_epoch()), + withdrawable_epoch: spec.far_future_epoch, + }; + // There are reusable indices that opened up because of older builders getting + // evicted, reuse the indices by inserting the new builder in the correct + // location + let new_index = if let Some(reusable_index) = reusable_builder_indices.pop_front() { + let old_pubkey = state + .builders()? + .get(reusable_index as usize) + .ok_or(BeaconStateError::UnknownBuilder(reusable_index))? + .pubkey; + builder_index_map.remove(&old_pubkey); + *state + .builders_mut()? + .get_mut(reusable_index as usize) + .ok_or(BeaconStateError::UnknownBuilder(reusable_index))? = builder; + reusable_index + } else { + // There are no reusable indices, insert at the end. + let builders = state.builders_mut()?; + let next_index = builders.len() as BuilderIndex; + builders.push(builder)?; + next_index + }; + + // Keep the local mapping updated + builder_index_map.insert(deposit_request.pubkey, new_index); + continue; + } } - let slot = state.slot(); state.pending_deposits_mut()?.push(PendingDeposit { pubkey: deposit_request.pubkey, withdrawal_credentials: deposit_request.withdrawal_credentials, @@ -1100,68 +1136,6 @@ pub fn process_deposit_requests_post_gloas( })?; } - tracing::debug!( - count = deposit_requests.len(), - time = now.elapsed().as_millis(), - "Completed processing deposits" - ); - Ok(()) -} - -#[allow(clippy::too_many_arguments)] -pub fn apply_deposit_for_builder( - state: &mut BeaconState, - builder_index_opt: Option, - pubkey: PublicKeyBytes, - withdrawal_credentials: Hash256, - amount: u64, - signature: SignatureBytes, - slot: Slot, - verify_signature: VerifyBuilderSignature, - spec: &ChainSpec, -) -> Result<(), BeaconStateError> { - match builder_index_opt { - None => { - // Verify the deposit signature (proof of possession) which is not checked by the deposit contract - let deposit_data = DepositData { - pubkey, - withdrawal_credentials, - amount, - signature, - }; - match verify_signature { - VerifyBuilderSignature::Verify => { - if is_valid_deposit_signature(&deposit_data, spec).is_ok() { - state.add_builder_to_registry( - pubkey, - withdrawal_credentials, - amount, - slot, - spec, - )?; - } - } - VerifyBuilderSignature::VerifiedValid => { - state.add_builder_to_registry( - pubkey, - withdrawal_credentials, - amount, - slot, - spec, - )?; - } - VerifyBuilderSignature::VerifiedInvalid => {} - } - } - Some(builder_index) => { - state - .builders_mut()? - .get_mut(builder_index as usize) - .ok_or(BeaconStateError::UnknownBuilder(builder_index))? - .balance - .safe_add_assign(amount)?; - } - } Ok(()) } diff --git a/consensus/types/src/deposit/deposit_data.rs b/consensus/types/src/deposit/deposit_data.rs index bd39643ebd5..f0e07c986d0 100644 --- a/consensus/types/src/deposit/deposit_data.rs +++ b/consensus/types/src/deposit/deposit_data.rs @@ -5,6 +5,7 @@ use ssz_derive::{Decode, Encode}; use tree_hash_derive::TreeHash; use crate::{ + DepositRequest, PendingDeposit, core::{ChainSpec, Hash256, SignedRoot}, deposit::DepositMessage, fork::ForkName, @@ -47,6 +48,28 @@ impl DepositData { } } +impl From<&DepositRequest> for DepositData { + fn from(value: &DepositRequest) -> Self { + DepositData { + pubkey: value.pubkey, + withdrawal_credentials: value.withdrawal_credentials, + amount: value.amount, + signature: value.signature.clone(), + } + } +} + +impl From<&PendingDeposit> for DepositData { + fn from(value: &PendingDeposit) -> Self { + DepositData { + pubkey: value.pubkey, + withdrawal_credentials: value.withdrawal_credentials, + amount: value.amount, + signature: value.signature.clone(), + } + } +} + #[cfg(test)] mod tests { use super::*; From ecbb7e7c0c89f41ce8670cdc2ed9c32f7853c646 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Fri, 22 May 2026 16:54:42 -0700 Subject: [PATCH 21/22] Only onboard state deposits before gloas --- beacon_node/beacon_chain/src/beacon_chain.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 3448524365b..618be22157b 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -4549,7 +4549,11 @@ impl BeaconChain { current_slot, ); - if let Some(builder_onboarding_cache) = &self.builder_onboarding_cache { + // If gloas is enabled, we get the deposits from the payload instead of + // the state. + if !state.fork_name_unchecked().gloas_enabled() + && let Some(builder_onboarding_cache) = &self.builder_onboarding_cache + { let cache = builder_onboarding_cache.clone(); let spec = self.spec.clone(); let executor = self.task_executor.clone(); From 656e70aeb16272a7c4b735cdd17b2e57d409f2b0 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Fri, 22 May 2026 16:54:59 -0700 Subject: [PATCH 22/22] Fix consensus bugs and add test --- .../process_operations.rs | 17 ++- .../src/per_block_processing/tests.rs | 126 +++++++++++++----- 2 files changed, 108 insertions(+), 35 deletions(-) diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index aef9a20f8d6..46028d0683e 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -1,4 +1,4 @@ -use std::collections::{HashMap, VecDeque}; +use std::collections::{BTreeSet, HashMap}; use super::*; use crate::VerifySignatures; @@ -959,12 +959,12 @@ pub fn process_deposit_requests_post_gloas( // Doing this avoids walking through the entire `state.builders` list and appending // in the end everytime we want to insert a new builder. // For a high number of builder deposits, this can take quite long. - let mut reusable_builder_indices = VecDeque::new(); + let mut reusable_builder_indices = BTreeSet::new(); for (index, builder) in state_builders.iter().enumerate() { builder_index_map.insert(builder.pubkey, index as BuilderIndex); if builder.withdrawable_epoch <= current_epoch && builder.balance == 0 { - reusable_builder_indices.push_back(index as BuilderIndex); + reusable_builder_indices.insert(index as BuilderIndex); } } @@ -1056,6 +1056,12 @@ pub fn process_deposit_requests_post_gloas( .ok_or(BeaconStateError::UnknownBuilder(builder_index))? .balance .safe_add_assign(deposit_request.amount)?; + // If the existing builder's balance was 0, then its no longer the + // case after the top-up. We cannot reuse its index anymore, remove + // the index from the set. + if deposit_request.amount > 0 && reusable_builder_indices.contains(&builder_index) { + reusable_builder_indices.remove(&builder_index); + } continue; } @@ -1101,7 +1107,7 @@ pub fn process_deposit_requests_post_gloas( // There are reusable indices that opened up because of older builders getting // evicted, reuse the indices by inserting the new builder in the correct // location - let new_index = if let Some(reusable_index) = reusable_builder_indices.pop_front() { + let new_index = if let Some(reusable_index) = reusable_builder_indices.pop_first() { let old_pubkey = state .builders()? .get(reusable_index as usize) @@ -1124,6 +1130,9 @@ pub fn process_deposit_requests_post_gloas( // Keep the local mapping updated builder_index_map.insert(deposit_request.pubkey, new_index); continue; + } else { + // signature is invalid, cannot create a new builder. drop the deposit and continue + continue; } } diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index a031fa1a49f..61e10916a00 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -78,6 +78,37 @@ async fn get_harness_at_fork( harness } +/// Helper to create a harness with Fulu genesis and gloas at a later epoch. +async fn get_fulu_harness_with_gloas_scheduled( + gloas_epoch: u64, + num_validators: usize, +) -> BeaconChainHarness> { + let mut spec = ForkName::Fulu.make_genesis_spec(E::default_spec()); + spec.gloas_fork_epoch = Some(Epoch::new(gloas_epoch)); + let spec = Arc::new(spec); + let last_slot_of_pre_gloas_epoch = Epoch::new(gloas_epoch).start_slot(E::slots_per_epoch()) - 1; + let harness = BeaconChainHarness::>::builder(E::default()) + .spec(spec.clone()) + .keypairs(KEYPAIRS[0..num_validators].to_vec()) + .fresh_ephemeral_store() + .mock_execution_layer() + .build(); + let state = harness.get_current_state(); + if last_slot_of_pre_gloas_epoch > Slot::new(0) { + harness + .add_attested_blocks_at_slots( + state, + (1..=last_slot_of_pre_gloas_epoch.as_u64()) + .map(Slot::new) + .collect::>() + .as_slice(), + (0..num_validators).collect::>().as_slice(), + ) + .await; + } + harness +} + fn builder_withdrawal_credentials(spec: &ChainSpec) -> Hash256 { let mut credentials = [0u8; 32]; credentials[0] = spec.builder_withdrawal_prefix_byte; @@ -717,6 +748,70 @@ async fn process_deposit_requests_post_gloas_preserves_existing_builder_before_v assert!(state.pending_deposits().unwrap().is_empty()); } +#[tokio::test] +async fn process_deposit_requests_post_gloas_does_not_reuse_topped_up_builder_index() { + let harness = get_gloas_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; + let spec = harness.spec.clone(); + let mut state = harness.get_current_state(); + + let reusable_builder_keypair = &KEYPAIRS[VALIDATOR_COUNT + 7]; + let reusable_builder_credentials = builder_withdrawal_credentials(&spec); + let slot = state.slot(); + let reusable_builder_index = state + .add_builder_to_registry( + reusable_builder_keypair.pk.compress(), + reusable_builder_credentials, + 11, + slot, + &spec, + ) + .unwrap() as usize; + + let current_epoch = state.current_epoch(); + let reusable_builder = state + .builders_mut() + .unwrap() + .get_mut(reusable_builder_index) + .unwrap(); + reusable_builder.balance = 0; + reusable_builder.withdrawable_epoch = current_epoch; + + let mut existing_builder_top_up = make_deposit_request( + reusable_builder_keypair, + reusable_builder_credentials, + 29, + &spec, + 0, + ); + existing_builder_top_up.signature = SignatureBytes::empty(); + + let new_builder_request = make_deposit_request( + &KEYPAIRS[VALIDATOR_COUNT + 8], + builder_withdrawal_credentials(&spec), + 31, + &spec, + 1, + ); + + process_operations::process_deposit_requests_post_gloas( + &mut state, + &[existing_builder_top_up.clone(), new_builder_request.clone()], + None, + &spec, + ) + .unwrap(); + + let builders = state.builders().unwrap(); + assert_eq!(builders.len(), 2); + + let topped_up_builder = builders.get(reusable_builder_index).unwrap(); + assert_eq!(topped_up_builder.pubkey, existing_builder_top_up.pubkey); + assert_eq!(topped_up_builder.balance, existing_builder_top_up.amount); + + let new_builder_index = find_builder_index(&state, &new_builder_request.pubkey).unwrap(); + assert_ne!(new_builder_index, reusable_builder_index); +} + #[tokio::test] async fn process_deposit_requests_post_gloas_preserves_pre_state_pending_validator_path() { let harness = get_gloas_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; @@ -855,37 +950,6 @@ async fn process_deposit_requests_cache_partial_hit() { assert!(find_builder_index(&state, &uncached_request.pubkey).is_some()); } -/// Helper to create a harness with Fulu genesis and gloas at a later epoch. -async fn get_fulu_harness_with_gloas_scheduled( - gloas_epoch: u64, - num_validators: usize, -) -> BeaconChainHarness> { - let mut spec = ForkName::Fulu.make_genesis_spec(E::default_spec()); - spec.gloas_fork_epoch = Some(Epoch::new(gloas_epoch)); - let spec = Arc::new(spec); - let last_slot_of_pre_gloas_epoch = Epoch::new(gloas_epoch).start_slot(E::slots_per_epoch()) - 1; - let harness = BeaconChainHarness::>::builder(E::default()) - .spec(spec.clone()) - .keypairs(KEYPAIRS[0..num_validators].to_vec()) - .fresh_ephemeral_store() - .mock_execution_layer() - .build(); - let state = harness.get_current_state(); - if last_slot_of_pre_gloas_epoch > Slot::new(0) { - harness - .add_attested_blocks_at_slots( - state, - (1..=last_slot_of_pre_gloas_epoch.as_u64()) - .map(Slot::new) - .collect::>() - .as_slice(), - (0..num_validators).collect::>().as_slice(), - ) - .await; - } - harness -} - #[tokio::test] async fn upgrade_to_gloas_with_cached_verification() { let harness =