From f7ddc9e0d3baa91531dbcafcafccd98092705673 Mon Sep 17 00:00:00 2001 From: Your Name Date: Wed, 27 Nov 2024 12:07:18 +0100 Subject: [PATCH 1/4] Add test case for two deposits within the same epoch to a given validator. --- Cargo.toml | 3 + .../src/per_block_processing/tests.rs | 125 ++++++++++++++++++ 2 files changed, 128 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 0be462754e0..91c2a0589f3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -278,5 +278,8 @@ lto = "fat" codegen-units = 1 incremental = false +[profile.release] +debug = true # Enables debug symbols + [patch.crates-io] quick-protobuf = { git = "https://github.com/sigp/quick-protobuf.git", rev = "681f413312404ab6e51f0b46f39b0075c6f4ebfd" } diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index c59449634ac..d9559ad1be5 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -15,6 +15,7 @@ use ssz_types::Bitfield; use std::sync::{Arc, LazyLock}; use test_utils::generate_deterministic_keypairs; use types::*; +use types::test_utils::generate_deterministic_keypair; pub const MAX_VALIDATOR_COUNT: usize = 97; pub const NUM_DEPOSITS: u64 = 1; @@ -1142,3 +1143,127 @@ async fn block_replayer_peeking_state_roots() { (dummy_state_root, dummy_slot) ); } + +/// Test that multiple deposits to the same validator within an epoch are handled correctly. +/// This test verifies: +/// 1. Deposits are not incorrectly combined +/// 2. Validator is only added once to the registry +/// 3. Final balance reflects all deposits +#[tokio::test] +async fn test_multiple_deposits_same_validator_same_epoch() { + let spec = MainnetEthSpec::default_spec(); + let harness = get_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; + + let keypair = generate_deterministic_keypair(1); + let pubkey: PublicKeyBytes = keypair.pk.clone().into(); + + let deposit_amount = 1_000_000_000; // 1 ETH in Gwei + + // Create first deposit and get updated state + let mut current_state = harness.get_current_state(); + // Get initial balance before any deposits + let initial_balance = if let Some(index) = current_state + .validators() + .iter() + .position(|v| v.pubkey == pubkey) + { + current_state.balances().get(index).copied().unwrap_or(0) + } else { + 0 + }; + + let (deposits1, state) = harness.make_deposits( + &mut current_state, + 1, + Some(pubkey.clone()), + None, + ); + + // Process first deposit + let mut head_block = harness + .chain + .head_beacon_block() + .as_ref() + .clone() + .deconstruct() + .0; + *head_block.to_mut().body_mut().deposits_mut() = deposits1.clone().into(); + + let result = process_operations::process_deposits( + state, + head_block.body().deposits(), + &spec, + ); + assert_eq!(result, Ok(())); + + // Advance a few slots but stay in same epoch + for _ in 0..3 { + harness.advance_slot(); + } + + // Create second deposit for same validator + let (deposits2, state) = harness.make_deposits( + state, + 1, + Some(pubkey.clone()), + None, + ); + + // Process second deposit + *head_block.to_mut().body_mut().deposits_mut() = deposits2.into(); + let result = process_operations::process_deposits( + state, + head_block.body().deposits(), + &spec, + ); + assert_eq!(result, Ok(())); + + // Collect all validation errors + let mut errors = Vec::new(); + + // Verify final state + let validator_indices: Vec<_> = state + .validators() + .iter() + .enumerate() + .filter(|(_, v)| v.pubkey == pubkey) + .map(|(i, _)| i) + .collect(); + + if validator_indices.len() != 1 { + errors.push(format!( + "Expected exactly one validator index, found {}", + validator_indices.len() + )); + } + + // Continue with balance check even if we have multiple indices + for validator_index in &validator_indices { + let balance = state.balances().get(*validator_index).copied().unwrap_or(0); + if balance != initial_balance + (deposit_amount * 2) { + errors.push(format!( + "Validator {} balance is {} gwei, expected {} gwei (initial: {} + deposits: {})", + validator_index, + balance, + initial_balance + (deposit_amount * 2), + initial_balance, + deposit_amount * 2 + )); + } + + let validator = state.validators().get(*validator_index).unwrap(); + if !validator.is_active_at(state.current_epoch()) { + errors.push(format!( + "Validator {} is not active at current epoch", + validator_index + )); + } + } + + // Report all errors at once + assert!( + errors.is_empty(), + "Found the following validation errors:\n{}", + errors.join("\n") + ); +} From 0818005789717764218c97ba3203c7fd554bf6e5 Mon Sep 17 00:00:00 2001 From: Your Name Date: Wed, 27 Nov 2024 12:27:33 +0100 Subject: [PATCH 2/4] Add pre/post electra tests. --- .../src/per_block_processing/tests.rs | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index d9559ad1be5..20dd5304402 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -1144,15 +1144,29 @@ async fn block_replayer_peeking_state_roots() { ); } +#[tokio::test] +async fn test_multiple_deposits_same_validator_same_epoch_pre_electra() { + test_multiple_deposits_same_validator_same_epoch_internal(false).await; +} + +#[tokio::test] +async fn test_multiple_deposits_same_validator_same_epoch_post_electra() { + test_multiple_deposits_same_validator_same_epoch_internal(true).await; +} + /// Test that multiple deposits to the same validator within an epoch are handled correctly. /// This test verifies: /// 1. Deposits are not incorrectly combined /// 2. Validator is only added once to the registry /// 3. Final balance reflects all deposits -#[tokio::test] -async fn test_multiple_deposits_same_validator_same_epoch() { - let spec = MainnetEthSpec::default_spec(); +async fn test_multiple_deposits_same_validator_same_epoch_internal(enable_electra: bool) { + let mut spec = MainnetEthSpec::default_spec(); let harness = get_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; + let slots_per_epoch = MainnetEthSpec::slots_per_epoch(); + if enable_electra { + spec.electra_fork_epoch = Some(Epoch::new(6)); + harness.extend_to_slot(spec.electra_fork_epoch.unwrap().start_slot(slots_per_epoch)).await; + } let keypair = generate_deterministic_keypair(1); let pubkey: PublicKeyBytes = keypair.pk.clone().into(); From 268b427ff1b0ad7fb616bd6b42f83ae3a168d2b0 Mon Sep 17 00:00:00 2001 From: Bhargava Shastry Date: Fri, 29 Nov 2024 11:23:12 +0100 Subject: [PATCH 3/4] WIP: Try to write a test for electra. --- .../src/per_block_processing/tests.rs | 37 +++++++++++++++++-- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index 20dd5304402..1a10bd0769e 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -10,7 +10,7 @@ use crate::{ per_block_processing::{process_operations, verify_exit::verify_exit}, BlockSignatureStrategy, ConsensusContext, VerifyBlockRoot, VerifySignatures, }; -use beacon_chain::test_utils::{BeaconChainHarness, EphemeralHarnessType}; +use beacon_chain::test_utils::{BeaconChainHarness, EphemeralHarnessType, HARNESS_GENESIS_TIME}; use ssz_types::Bitfield; use std::sync::{Arc, LazyLock}; use test_utils::generate_deterministic_keypairs; @@ -1161,20 +1161,51 @@ async fn test_multiple_deposits_same_validator_same_epoch_post_electra() { /// 3. Final balance reflects all deposits async fn test_multiple_deposits_same_validator_same_epoch_internal(enable_electra: bool) { let mut spec = MainnetEthSpec::default_spec(); - let harness = get_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; - let slots_per_epoch = MainnetEthSpec::slots_per_epoch(); if enable_electra { + spec.altair_fork_epoch = Some(Epoch::new(2)); + spec.bellatrix_fork_epoch = Some(Epoch::new(3)); + spec.capella_fork_epoch = Some(Epoch::new(4)); + spec.deneb_fork_epoch = Some(Epoch::new(5)); spec.electra_fork_epoch = Some(Epoch::new(6)); + } + let slots_per_epoch = MainnetEthSpec::slots_per_epoch(); + let spec = Arc::new(spec); + let harness = BeaconChainHarness::builder(MainnetEthSpec) + .spec(spec.clone()) + .deterministic_keypairs(VALIDATOR_COUNT) + .mock_execution_layer() + .mock_execution_layer_all_payloads_valid() + .recalculate_fork_times_with_genesis(HARNESS_GENESIS_TIME) + .fresh_ephemeral_store() + .build(); + + if enable_electra { harness.extend_to_slot(spec.electra_fork_epoch.unwrap().start_slot(slots_per_epoch)).await; } let keypair = generate_deterministic_keypair(1); let pubkey: PublicKeyBytes = keypair.pk.clone().into(); + // make_deposits uses spec.min_deposit_amount as the deposit amount + // spec.min_deposit_amount is 1 ETH for mainnet let deposit_amount = 1_000_000_000; // 1 ETH in Gwei // Create first deposit and get updated state let mut current_state = harness.get_current_state(); + // debug + println!( + "Fork state: {:?}, Electra enabled: {}", + current_state.fork(), + current_state.fork_name_unchecked().electra_enabled(), + ); + + println!( + "Test setup: Electra fork epoch: {:?}, Current slot: {}", + spec.electra_fork_epoch, + current_state.slot(), + ); + + // Get initial balance before any deposits let initial_balance = if let Some(index) = current_state .validators() From 30fe8c165c8af08418f554d3c3a28c2169f39937 Mon Sep 17 00:00:00 2001 From: Bhargava Shastry Date: Thu, 12 Dec 2024 12:01:28 +0100 Subject: [PATCH 4/4] For electra process_deposit_requests. --- .../src/per_block_processing/tests.rs | 161 ++++++++++-------- 1 file changed, 87 insertions(+), 74 deletions(-) diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index 1a10bd0769e..dd65b304204 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -5,17 +5,21 @@ use crate::per_block_processing::errors::{ DepositInvalid, HeaderInvalid, IndexedAttestationInvalid, IntoWithIndex, ProposerSlashingInvalid, }; +use crate::upgrade::{ + upgrade_to_altair, upgrade_to_bellatrix, upgrade_to_capella, upgrade_to_deneb, + upgrade_to_electra, +}; use crate::{per_block_processing, BlockReplayError, BlockReplayer}; use crate::{ per_block_processing::{process_operations, verify_exit::verify_exit}, BlockSignatureStrategy, ConsensusContext, VerifyBlockRoot, VerifySignatures, }; -use beacon_chain::test_utils::{BeaconChainHarness, EphemeralHarnessType, HARNESS_GENESIS_TIME}; +use beacon_chain::test_utils::{BeaconChainHarness, EphemeralHarnessType}; use ssz_types::Bitfield; use std::sync::{Arc, LazyLock}; use test_utils::generate_deterministic_keypairs; -use types::*; use types::test_utils::generate_deterministic_keypair; +use types::*; pub const MAX_VALIDATOR_COUNT: usize = 97; pub const NUM_DEPOSITS: u64 = 1; @@ -1160,27 +1164,25 @@ async fn test_multiple_deposits_same_validator_same_epoch_post_electra() { /// 2. Validator is only added once to the registry /// 3. Final balance reflects all deposits async fn test_multiple_deposits_same_validator_same_epoch_internal(enable_electra: bool) { + use safe_arith::SafeArith; let mut spec = MainnetEthSpec::default_spec(); if enable_electra { - spec.altair_fork_epoch = Some(Epoch::new(2)); - spec.bellatrix_fork_epoch = Some(Epoch::new(3)); - spec.capella_fork_epoch = Some(Epoch::new(4)); - spec.deneb_fork_epoch = Some(Epoch::new(5)); - spec.electra_fork_epoch = Some(Epoch::new(6)); + spec.altair_fork_epoch = Some(Epoch::new(5)); + spec.bellatrix_fork_epoch = Some(Epoch::new(6)); + spec.capella_fork_epoch = Some(Epoch::new(7)); + spec.deneb_fork_epoch = Some(Epoch::new(8)); + spec.electra_fork_epoch = Some(Epoch::new(9)); } - let slots_per_epoch = MainnetEthSpec::slots_per_epoch(); - let spec = Arc::new(spec); - let harness = BeaconChainHarness::builder(MainnetEthSpec) - .spec(spec.clone()) - .deterministic_keypairs(VALIDATOR_COUNT) - .mock_execution_layer() - .mock_execution_layer_all_payloads_valid() - .recalculate_fork_times_with_genesis(HARNESS_GENESIS_TIME) - .fresh_ephemeral_store() - .build(); + + let harness = get_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; + let mut state = harness.get_current_state(); if enable_electra { - harness.extend_to_slot(spec.electra_fork_epoch.unwrap().start_slot(slots_per_epoch)).await; + upgrade_to_altair(&mut state, &spec).expect("upgrade to altair"); + upgrade_to_bellatrix(&mut state, &spec).expect("upgrade to bellatrix"); + upgrade_to_capella(&mut state, &spec).expect("upgrade to capella"); + upgrade_to_deneb(&mut state, &spec).expect("upgrade to deneb"); + upgrade_to_electra(&mut state, &spec).expect("upgrade to electra"); } let keypair = generate_deterministic_keypair(1); @@ -1190,79 +1192,90 @@ async fn test_multiple_deposits_same_validator_same_epoch_internal(enable_electr // spec.min_deposit_amount is 1 ETH for mainnet let deposit_amount = 1_000_000_000; // 1 ETH in Gwei - // Create first deposit and get updated state - let mut current_state = harness.get_current_state(); // debug println!( "Fork state: {:?}, Electra enabled: {}", - current_state.fork(), - current_state.fork_name_unchecked().electra_enabled(), + state.fork(), + state.fork_name_unchecked().electra_enabled(), ); println!( "Test setup: Electra fork epoch: {:?}, Current slot: {}", spec.electra_fork_epoch, - current_state.slot(), + state.slot(), ); - // Get initial balance before any deposits - let initial_balance = if let Some(index) = current_state - .validators() - .iter() - .position(|v| v.pubkey == pubkey) - { - current_state.balances().get(index).copied().unwrap_or(0) - } else { - 0 - }; + let initial_balance = + if let Some(index) = state.validators().iter().position(|v| v.pubkey == pubkey) { + state.balances().get(index).copied().unwrap_or(0) + } else { + 0 + }; - let (deposits1, state) = harness.make_deposits( - &mut current_state, - 1, - Some(pubkey.clone()), - None, - ); + if enable_electra { + // Create deposit request + let deposit_requests = vec![DepositRequest { + pubkey: pubkey.clone(), + withdrawal_credentials: Hash256::zero(), + amount: deposit_amount, + signature: Signature::empty(), + index: state.eth1_deposit_index().safe_add(1).unwrap(), + }]; + + let result = + process_operations::process_deposit_requests(&mut state, &deposit_requests, &spec); + assert_eq!(result, Ok(())); + + // Advance a few slots but stay in same epoch + for _ in 0..3 { + harness.advance_slot(); + } - // Process first deposit - let mut head_block = harness - .chain - .head_beacon_block() - .as_ref() - .clone() - .deconstruct() - .0; - *head_block.to_mut().body_mut().deposits_mut() = deposits1.clone().into(); + // Create second deposit request + let deposit_requests = vec![DepositRequest { + pubkey: pubkey.clone(), + withdrawal_credentials: Hash256::zero(), + amount: deposit_amount, + signature: Signature::empty(), + index: state.eth1_deposit_index().safe_add(1).unwrap(), + }]; + + let result = + process_operations::process_deposit_requests(&mut state, &deposit_requests, &spec); + assert_eq!(result, Ok(())); + } else { + let (deposits1, state) = harness.make_deposits(&mut state, 1, Some(pubkey.clone()), None); + + // Process first deposit + let mut head_block = harness + .chain + .head_beacon_block() + .as_ref() + .clone() + .deconstruct() + .0; + *head_block.to_mut().body_mut().deposits_mut() = deposits1.clone().into(); + + let result = + process_operations::process_deposits(state, head_block.body().deposits(), &spec); + assert_eq!(result, Ok(())); + + // Advance a few slots but stay in same epoch + for _ in 0..3 { + harness.advance_slot(); + } - let result = process_operations::process_deposits( - state, - head_block.body().deposits(), - &spec, - ); - assert_eq!(result, Ok(())); + // Create second deposit for same validator + let (deposits2, state) = harness.make_deposits(state, 1, Some(pubkey.clone()), None); - // Advance a few slots but stay in same epoch - for _ in 0..3 { - harness.advance_slot(); + // Process second deposit + *head_block.to_mut().body_mut().deposits_mut() = deposits2.into(); + let result = + process_operations::process_deposits(state, head_block.body().deposits(), &spec); + assert_eq!(result, Ok(())); } - // Create second deposit for same validator - let (deposits2, state) = harness.make_deposits( - state, - 1, - Some(pubkey.clone()), - None, - ); - - // Process second deposit - *head_block.to_mut().body_mut().deposits_mut() = deposits2.into(); - let result = process_operations::process_deposits( - state, - head_block.body().deposits(), - &spec, - ); - assert_eq!(result, Ok(())); - // Collect all validation errors let mut errors = Vec::new();