From 0cdd049da96f9970a057ea1b0555b065ff9066e4 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 15 Nov 2022 06:14:31 +1100 Subject: [PATCH] Fixes to make EF Capella tests pass (#3719) * Fixes to make EF Capella tests pass * Clippy for state_processing --- Makefile | 9 +- .../src/rpc/codec/ssz_snappy.rs | 4 +- .../lighthouse_network/src/rpc/protocol.rs | 4 +- beacon_node/store/src/hot_cold_store.rs | 18 +-- .../state_processing/src/consensus_context.rs | 2 +- .../src/per_block_processing.rs | 40 +++-- .../src/per_block_processing/eip4844.rs | 1 + .../per_block_processing/eip4844/eip4844.rs | 6 +- .../src/per_block_processing/errors.rs | 1 + .../process_operations.rs | 45 +++--- .../verify_bls_to_execution_change.rs | 4 +- .../src/per_slot_processing.rs | 12 +- .../state_processing/src/upgrade/capella.rs | 1 - consensus/types/src/beacon_block.rs | 110 +++++++++++--- consensus/types/src/beacon_block_body.rs | 2 +- consensus/types/src/beacon_state.rs | 6 +- .../types/src/beacon_state/tree_hash_cache.rs | 10 ++ consensus/types/src/blobs_sidecar.rs | 2 +- consensus/types/src/eth_spec.rs | 2 +- consensus/types/src/execution_payload.rs | 13 +- .../types/src/execution_payload_header.rs | 39 ++--- consensus/types/src/kzg_commitment.rs | 2 +- consensus/types/src/kzg_proof.rs | 3 +- consensus/types/src/lib.rs | 2 +- consensus/types/src/payload.rs | 10 +- consensus/types/src/tree_hash_impls.rs | 6 +- consensus/types/src/validator.rs | 16 +- testing/ef_tests/Makefile | 2 +- .../ef_tests/src/cases/epoch_processing.rs | 5 +- testing/ef_tests/src/cases/fork.rs | 4 +- .../src/cases/genesis_initialization.rs | 17 +-- testing/ef_tests/src/cases/operations.rs | 138 +++++++++++++----- testing/ef_tests/src/cases/transition.rs | 13 +- testing/ef_tests/src/handler.rs | 15 +- testing/ef_tests/src/lib.rs | 5 +- testing/ef_tests/src/type_name.rs | 11 ++ testing/ef_tests/tests/tests.rs | 50 ++++++- 37 files changed, 434 insertions(+), 196 deletions(-) diff --git a/Makefile b/Makefile index 33077a6c93..56e05fffcb 100644 --- a/Makefile +++ b/Makefile @@ -20,6 +20,9 @@ CROSS_FEATURES ?= gnosis,slasher-lmdb,slasher-mdbx # Cargo profile for Cross builds. Default is for local builds, CI uses an override. CROSS_PROFILE ?= release +# List of features to use when running EF tests. +EF_TEST_FEATURES ?= beacon_chain/withdrawals,beacon_chain/withdrawals-processing + # Cargo profile for regular builds. PROFILE ?= release @@ -108,9 +111,9 @@ check-consensus: # Runs only the ef-test vectors. run-ef-tests: rm -rf $(EF_TESTS)/.accessed_file_log.txt - cargo test --release -p ef_tests --features "ef_tests" - cargo test --release -p ef_tests --features "ef_tests,fake_crypto" - cargo test --release -p ef_tests --features "ef_tests,milagro" + cargo test --release -p ef_tests --features "ef_tests,$(EF_TEST_FEATURES)" + cargo test --release -p ef_tests --features "ef_tests,$(EF_TEST_FEATURES),fake_crypto" + cargo test --release -p ef_tests --features "ef_tests,$(EF_TEST_FEATURES),milagro" ./$(EF_TESTS)/check_all_files_accessed.py $(EF_TESTS)/.accessed_file_log.txt $(EF_TESTS)/consensus-spec-tests # Run the tests in the `beacon_chain` crate for all known forks. diff --git a/beacon_node/lighthouse_network/src/rpc/codec/ssz_snappy.rs b/beacon_node/lighthouse_network/src/rpc/codec/ssz_snappy.rs index 3c40fdf8b3..8f2867e04e 100644 --- a/beacon_node/lighthouse_network/src/rpc/codec/ssz_snappy.rs +++ b/beacon_node/lighthouse_network/src/rpc/codec/ssz_snappy.rs @@ -683,8 +683,8 @@ mod tests { }; use std::sync::Arc; use types::{ - BeaconBlock, BeaconBlockAltair, BeaconBlockBase, BeaconBlockMerge, Epoch, ForkContext, - FullPayload, Hash256, Signature, SignedBeaconBlock, Slot, + BeaconBlock, BeaconBlockAltair, BeaconBlockBase, BeaconBlockMerge, EmptyBlock, Epoch, + ForkContext, FullPayload, Hash256, Signature, SignedBeaconBlock, Slot, }; use snap::write::FrameEncoder; diff --git a/beacon_node/lighthouse_network/src/rpc/protocol.rs b/beacon_node/lighthouse_network/src/rpc/protocol.rs index 8511d26208..f71b8e6055 100644 --- a/beacon_node/lighthouse_network/src/rpc/protocol.rs +++ b/beacon_node/lighthouse_network/src/rpc/protocol.rs @@ -22,8 +22,8 @@ use tokio_util::{ }; use types::BlobsSidecar; use types::{ - BeaconBlock, BeaconBlockAltair, BeaconBlockBase, BeaconBlockMerge, Blob, EthSpec, ForkContext, - ForkName, Hash256, MainnetEthSpec, Signature, SignedBeaconBlock, + BeaconBlock, BeaconBlockAltair, BeaconBlockBase, BeaconBlockMerge, Blob, EmptyBlock, EthSpec, + ForkContext, ForkName, Hash256, MainnetEthSpec, Signature, SignedBeaconBlock, }; lazy_static! { diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index c0fbef973f..e8c782b8c5 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -492,17 +492,15 @@ impl, Cold: ItemStore> HotColdDB pub fn get_blobs(&self, block_root: &Hash256) -> Result>, Error> { if let Some(blobs) = self.blob_cache.lock().get(block_root) { Ok(Some(blobs.clone())) + } else if let Some(bytes) = self + .hot_db + .get_bytes(DBColumn::BeaconBlob.into(), block_root.as_bytes())? + { + let ret = BlobsSidecar::from_ssz_bytes(&bytes)?; + self.blob_cache.lock().put(*block_root, ret.clone()); + Ok(Some(ret)) } else { - if let Some(bytes) = self - .hot_db - .get_bytes(DBColumn::BeaconBlob.into(), block_root.as_bytes())? - { - let ret = BlobsSidecar::from_ssz_bytes(&bytes)?; - self.blob_cache.lock().put(*block_root, ret.clone()); - Ok(Some(ret)) - } else { - Ok(None) - } + Ok(None) } } diff --git a/consensus/state_processing/src/consensus_context.rs b/consensus/state_processing/src/consensus_context.rs index 18ae5ad3b7..121a9eccb9 100644 --- a/consensus/state_processing/src/consensus_context.rs +++ b/consensus/state_processing/src/consensus_context.rs @@ -1,7 +1,7 @@ use std::marker::PhantomData; use tree_hash::TreeHash; use types::{ - AbstractExecPayload, BeaconState, BeaconStateError, ChainSpec, EthSpec, ExecPayload, Hash256, + AbstractExecPayload, BeaconState, BeaconStateError, ChainSpec, EthSpec, Hash256, SignedBeaconBlock, Slot, }; diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index 2d8a01ff48..5e59a0132c 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -42,7 +42,9 @@ mod verify_deposit; mod verify_exit; mod verify_proposer_slashing; +#[cfg(feature = "withdrawals-processing")] use crate::common::decrease_balance; + #[cfg(feature = "arbitrary-fuzz")] use arbitrary::Arbitrary; @@ -466,21 +468,22 @@ pub fn get_expected_withdrawals( spec: &ChainSpec, ) -> Result, BlockProcessingError> { let epoch = state.current_epoch(); - let mut withdrawal_index = *state.next_withdrawal_index()?; - let mut validator_index = *state.next_withdrawal_validator_index()?; + let mut withdrawal_index = state.next_withdrawal_index()?; + let mut validator_index = state.next_withdrawal_validator_index()?; let mut withdrawals = vec![]; for _ in 0..state.validators().len() { let validator = state.get_validator(validator_index as usize)?; - let balance = *state - .balances() - .get(validator_index as usize) - .ok_or_else(|| BeaconStateError::BalancesOutOfBounds(validator_index as usize))?; + let balance = *state.balances().get(validator_index as usize).ok_or( + BeaconStateError::BalancesOutOfBounds(validator_index as usize), + )?; if validator.is_fully_withdrawable_at(balance, epoch, spec) { withdrawals.push(Withdrawal { index: withdrawal_index, validator_index, - address: Address::from_slice(&validator.withdrawal_credentials[12..]), + address: validator + .get_eth1_withdrawal_address(spec) + .ok_or(BlockProcessingError::WithdrawalCredentialsInvalid)?, amount: balance, }); withdrawal_index.safe_add_assign(1)?; @@ -488,7 +491,9 @@ pub fn get_expected_withdrawals( withdrawals.push(Withdrawal { index: withdrawal_index, validator_index, - address: Address::from_slice(&validator.withdrawal_credentials[12..]), + address: validator + .get_eth1_withdrawal_address(spec) + .ok_or(BlockProcessingError::WithdrawalCredentialsInvalid)?, amount: balance.safe_sub(spec.max_effective_balance)?, }); withdrawal_index.safe_add_assign(1)?; @@ -496,7 +501,9 @@ pub fn get_expected_withdrawals( if withdrawals.len() == T::max_withdrawals_per_payload() { break; } - validator_index = validator_index.safe_add(1)? % state.validators().len() as u64; + validator_index = validator_index + .safe_add(1)? + .safe_rem(state.validators().len() as u64)?; } Ok(withdrawals.into()) @@ -513,12 +520,13 @@ pub fn process_withdrawals<'payload, T: EthSpec, Payload: AbstractExecPayload BeaconState::Merge(_) => Ok(()), BeaconState::Capella(_) | BeaconState::Eip4844(_) => { let expected_withdrawals = get_expected_withdrawals(state, spec)?; + let expected_root = expected_withdrawals.tree_hash_root(); let withdrawals_root = payload.withdrawals_root()?; - if expected_withdrawals.tree_hash_root() != payload.withdrawals_root()? { + if expected_root != withdrawals_root { return Err(BlockProcessingError::WithdrawalsRootMismatch { - expected: expected_withdrawals.tree_hash_root(), - found: payload.withdrawals_root()?, + expected: expected_root, + found: withdrawals_root, }); } @@ -531,9 +539,11 @@ pub fn process_withdrawals<'payload, T: EthSpec, Payload: AbstractExecPayload } if let Some(latest_withdrawal) = expected_withdrawals.last() { - *state.next_withdrawal_index_mut()? = latest_withdrawal.index + 1; - let next_validator_index = - (latest_withdrawal.validator_index + 1) % state.validators().len() as u64; + *state.next_withdrawal_index_mut()? = latest_withdrawal.index.safe_add(1)?; + let next_validator_index = latest_withdrawal + .validator_index + .safe_add(1)? + .safe_rem(state.validators().len() as u64)?; *state.next_withdrawal_validator_index_mut()? = next_validator_index; } diff --git a/consensus/state_processing/src/per_block_processing/eip4844.rs b/consensus/state_processing/src/per_block_processing/eip4844.rs index 120ba304d0..23ab3c5c07 100644 --- a/consensus/state_processing/src/per_block_processing/eip4844.rs +++ b/consensus/state_processing/src/per_block_processing/eip4844.rs @@ -1 +1,2 @@ +#[allow(clippy::module_inception)] pub mod eip4844; diff --git a/consensus/state_processing/src/per_block_processing/eip4844/eip4844.rs b/consensus/state_processing/src/per_block_processing/eip4844/eip4844.rs index 56b3ed58a6..55b1ab967e 100644 --- a/consensus/state_processing/src/per_block_processing/eip4844/eip4844.rs +++ b/consensus/state_processing/src/per_block_processing/eip4844/eip4844.rs @@ -6,8 +6,8 @@ use ssz::Decode; use ssz_types::VariableList; use types::consts::eip4844::{BLOB_TX_TYPE, VERSIONED_HASH_VERSION_KZG}; use types::{ - AbstractExecPayload, BeaconBlockBodyRef, EthSpec, ExecPayload, FullPayload, FullPayloadRef, - KzgCommitment, Transaction, Transactions, VersionedHash, + AbstractExecPayload, BeaconBlockBodyRef, EthSpec, ExecPayload, KzgCommitment, Transaction, + Transactions, VersionedHash, }; pub fn process_blob_kzg_commitments>( @@ -34,7 +34,7 @@ pub fn verify_kzg_commitments_against_transactions( let nested_iter = transactions .into_iter() .filter(|tx| { - tx.get(0) + tx.first() .map(|tx_type| *tx_type == BLOB_TX_TYPE) .unwrap_or(false) }) diff --git a/consensus/state_processing/src/per_block_processing/errors.rs b/consensus/state_processing/src/per_block_processing/errors.rs index 8de0fd337a..7b355b0ddc 100644 --- a/consensus/state_processing/src/per_block_processing/errors.rs +++ b/consensus/state_processing/src/per_block_processing/errors.rs @@ -94,6 +94,7 @@ pub enum BlockProcessingError { index: usize, length: usize, }, + WithdrawalCredentialsInvalid, } impl From for BlockProcessingError { 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 9b24be39e7..32e36c6ce6 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -33,8 +33,11 @@ pub fn process_operations<'a, T: EthSpec, Payload: AbstractExecPayload>( process_attestations(state, block_body, verify_signatures, ctxt, spec)?; process_deposits(state, block_body.deposits(), spec)?; process_exits(state, block_body.voluntary_exits(), verify_signatures, spec)?; + #[cfg(all(feature = "withdrawals", feature = "withdrawals-processing"))] - process_bls_to_execution_changes(state, block_body, verify_signatures, spec)?; + if let Ok(bls_to_execution_changes) = block_body.bls_to_execution_changes() { + process_bls_to_execution_changes(state, bls_to_execution_changes, verify_signatures, spec)?; + } Ok(()) } @@ -287,39 +290,25 @@ pub fn process_exits( /// Returns `Ok(())` if the validation and state updates completed successfully. Otherwise returs /// an `Err` describing the invalid object or cause of failure. #[cfg(all(feature = "withdrawals", feature = "withdrawals-processing"))] -pub fn process_bls_to_execution_changes<'a, T: EthSpec, Payload: AbstractExecPayload>( +pub fn process_bls_to_execution_changes( state: &mut BeaconState, - block_body: BeaconBlockBodyRef<'a, T, Payload>, + bls_to_execution_changes: &[SignedBlsToExecutionChange], verify_signatures: VerifySignatures, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { - match block_body { - BeaconBlockBodyRef::Base(_) - | BeaconBlockBodyRef::Altair(_) - | BeaconBlockBodyRef::Merge(_) => Ok(()), - BeaconBlockBodyRef::Capella(_) | BeaconBlockBodyRef::Eip4844(_) => { - for (i, signed_address_change) in - block_body.bls_to_execution_changes()?.iter().enumerate() - { - verify_bls_to_execution_change( - state, - &signed_address_change, - verify_signatures, - spec, - ) - .map_err(|e| e.into_with_index(i))?; + for (i, signed_address_change) in bls_to_execution_changes.iter().enumerate() { + verify_bls_to_execution_change(state, signed_address_change, verify_signatures, spec) + .map_err(|e| e.into_with_index(i))?; - state - .get_validator_mut(signed_address_change.message.validator_index as usize)? - .change_withdrawal_credentials( - &signed_address_change.message.to_execution_address, - spec, - ); - } - - Ok(()) - } + state + .get_validator_mut(signed_address_change.message.validator_index as usize)? + .change_withdrawal_credentials( + &signed_address_change.message.to_execution_address, + spec, + ); } + + Ok(()) } /// Validates each `Deposit` and updates the state, short-circuiting on an invalid object. diff --git a/consensus/state_processing/src/per_block_processing/verify_bls_to_execution_change.rs b/consensus/state_processing/src/per_block_processing/verify_bls_to_execution_change.rs index 3c15691453..34700a33e4 100644 --- a/consensus/state_processing/src/per_block_processing/verify_bls_to_execution_change.rs +++ b/consensus/state_processing/src/per_block_processing/verify_bls_to_execution_change.rs @@ -42,13 +42,13 @@ pub fn verify_bls_to_execution_change( // FIXME: Should this check be put inside the verify_signatures.is_true() condition? // I believe that's used for fuzzing so this is a Mehdi question.. verify!( - validator.withdrawal_credentials.as_bytes()[1..] == pubkey_hash[1..], + validator.withdrawal_credentials.as_bytes().get(1..) == pubkey_hash.get(1..), Invalid::WithdrawalCredentialsMismatch ); if verify_signatures.is_true() { verify!( - bls_execution_change_signature_set(state, signed_address_change, spec,)?.verify(), + bls_execution_change_signature_set(state, signed_address_change, spec)?.verify(), Invalid::BadSignature ); } diff --git a/consensus/state_processing/src/per_slot_processing.rs b/consensus/state_processing/src/per_slot_processing.rs index 9018db65bc..8d2600bb41 100644 --- a/consensus/state_processing/src/per_slot_processing.rs +++ b/consensus/state_processing/src/per_slot_processing.rs @@ -1,4 +1,6 @@ -use crate::upgrade::{upgrade_to_altair, upgrade_to_bellatrix}; +use crate::upgrade::{ + upgrade_to_altair, upgrade_to_bellatrix, upgrade_to_capella, upgrade_to_eip4844, +}; use crate::{per_epoch_processing::EpochProcessingSummary, *}; use safe_arith::{ArithError, SafeArith}; use types::*; @@ -55,6 +57,14 @@ pub fn per_slot_processing( if spec.bellatrix_fork_epoch == Some(state.current_epoch()) { upgrade_to_bellatrix(state, spec)?; } + // Capella. + if spec.capella_fork_epoch == Some(state.current_epoch()) { + upgrade_to_capella(state, spec)?; + } + // Eip4844 + if spec.eip4844_fork_epoch == Some(state.current_epoch()) { + upgrade_to_eip4844(state, spec)?; + } } Ok(summary) diff --git a/consensus/state_processing/src/upgrade/capella.rs b/consensus/state_processing/src/upgrade/capella.rs index ad106a8422..9a88369883 100644 --- a/consensus/state_processing/src/upgrade/capella.rs +++ b/consensus/state_processing/src/upgrade/capella.rs @@ -1,4 +1,3 @@ -use ssz_types::VariableList; use std::mem; use types::{BeaconState, BeaconStateCapella, BeaconStateError as Error, ChainSpec, EthSpec, Fork}; diff --git a/consensus/types/src/beacon_block.rs b/consensus/types/src/beacon_block.rs index d58e890c60..124cb08bcc 100644 --- a/consensus/types/src/beacon_block.rs +++ b/consensus/types/src/beacon_block.rs @@ -78,17 +78,20 @@ impl<'a, T: EthSpec, Payload: AbstractExecPayload> SignedRoot { } +/// Empty block trait for each block variant to implement. +pub trait EmptyBlock { + /// Returns an empty block to be used during genesis. + fn empty(spec: &ChainSpec) -> Self; +} + impl> BeaconBlock { - // FIXME: deal with capella / eip4844 forks here as well /// Returns an empty block to be used during genesis. pub fn empty(spec: &ChainSpec) -> Self { - if spec.bellatrix_fork_epoch == Some(T::genesis_epoch()) { - Self::Merge(BeaconBlockMerge::empty(spec)) - } else if spec.altair_fork_epoch == Some(T::genesis_epoch()) { - Self::Altair(BeaconBlockAltair::empty(spec)) - } else { - Self::Base(BeaconBlockBase::empty(spec)) - } + map_fork_name!( + spec.fork_name_at_epoch(T::genesis_epoch()), + Self, + EmptyBlock::empty(spec) + ) } /// Custom SSZ decoder that takes a `ChainSpec` as context. @@ -117,13 +120,12 @@ impl> BeaconBlock { /// Usually it's better to prefer `from_ssz_bytes` which will decode the correct variant based /// on the fork slot. pub fn any_from_ssz_bytes(bytes: &[u8]) -> Result { - BeaconBlockMerge::from_ssz_bytes(bytes) - .map(BeaconBlock::Merge) - .or_else(|_| { - BeaconBlockAltair::from_ssz_bytes(bytes) - .map(BeaconBlock::Altair) - .or_else(|_| BeaconBlockBase::from_ssz_bytes(bytes).map(BeaconBlock::Base)) - }) + BeaconBlockEip4844::from_ssz_bytes(bytes) + .map(BeaconBlock::Eip4844) + .or_else(|_| BeaconBlockCapella::from_ssz_bytes(bytes).map(BeaconBlock::Capella)) + .or_else(|_| BeaconBlockMerge::from_ssz_bytes(bytes).map(BeaconBlock::Merge)) + .or_else(|_| BeaconBlockAltair::from_ssz_bytes(bytes).map(BeaconBlock::Altair)) + .or_else(|_| BeaconBlockBase::from_ssz_bytes(bytes).map(BeaconBlock::Base)) } /// Convenience accessor for the `body` as a `BeaconBlockBodyRef`. @@ -266,9 +268,8 @@ impl<'a, T: EthSpec, Payload: AbstractExecPayload> BeaconBlockRefMut<'a, T, P } } -impl> BeaconBlockBase { - /// Returns an empty block to be used during genesis. - pub fn empty(spec: &ChainSpec) -> Self { +impl> EmptyBlock for BeaconBlockBase { + fn empty(spec: &ChainSpec) -> Self { BeaconBlockBase { slot: spec.genesis_slot, proposer_index: 0, @@ -291,7 +292,9 @@ impl> BeaconBlockBase { }, } } +} +impl> BeaconBlockBase { /// Return a block where the block has maximum size. pub fn full(spec: &ChainSpec) -> Self { let header = BeaconBlockHeader { @@ -387,9 +390,9 @@ impl> BeaconBlockBase { } } -impl> BeaconBlockAltair { +impl> EmptyBlock for BeaconBlockAltair { /// Returns an empty Altair block to be used during genesis. - pub fn empty(spec: &ChainSpec) -> Self { + fn empty(spec: &ChainSpec) -> Self { BeaconBlockAltair { slot: spec.genesis_slot, proposer_index: 0, @@ -413,7 +416,9 @@ impl> BeaconBlockAltair }, } } +} +impl> BeaconBlockAltair { /// Return an Altair block where the block has maximum size. pub fn full(spec: &ChainSpec) -> Self { let base_block: BeaconBlockBase<_, Payload> = BeaconBlockBase::full(spec); @@ -446,9 +451,9 @@ impl> BeaconBlockAltair } } -impl> BeaconBlockMerge { +impl> EmptyBlock for BeaconBlockMerge { /// Returns an empty Merge block to be used during genesis. - pub fn empty(spec: &ChainSpec) -> Self { + fn empty(spec: &ChainSpec) -> Self { BeaconBlockMerge { slot: spec.genesis_slot, proposer_index: 0, @@ -474,6 +479,67 @@ impl> BeaconBlockMerge { } } +impl> EmptyBlock for BeaconBlockCapella { + /// Returns an empty Capella block to be used during genesis. + fn empty(spec: &ChainSpec) -> Self { + BeaconBlockCapella { + slot: spec.genesis_slot, + proposer_index: 0, + parent_root: Hash256::zero(), + state_root: Hash256::zero(), + body: BeaconBlockBodyCapella { + randao_reveal: Signature::empty(), + eth1_data: Eth1Data { + deposit_root: Hash256::zero(), + block_hash: Hash256::zero(), + deposit_count: 0, + }, + graffiti: Graffiti::default(), + proposer_slashings: VariableList::empty(), + attester_slashings: VariableList::empty(), + attestations: VariableList::empty(), + deposits: VariableList::empty(), + voluntary_exits: VariableList::empty(), + sync_aggregate: SyncAggregate::empty(), + execution_payload: Payload::Capella::default(), + #[cfg(feature = "withdrawals")] + bls_to_execution_changes: VariableList::empty(), + }, + } + } +} + +impl> EmptyBlock for BeaconBlockEip4844 { + /// Returns an empty Eip4844 block to be used during genesis. + fn empty(spec: &ChainSpec) -> Self { + BeaconBlockEip4844 { + slot: spec.genesis_slot, + proposer_index: 0, + parent_root: Hash256::zero(), + state_root: Hash256::zero(), + body: BeaconBlockBodyEip4844 { + randao_reveal: Signature::empty(), + eth1_data: Eth1Data { + deposit_root: Hash256::zero(), + block_hash: Hash256::zero(), + deposit_count: 0, + }, + graffiti: Graffiti::default(), + proposer_slashings: VariableList::empty(), + attester_slashings: VariableList::empty(), + attestations: VariableList::empty(), + deposits: VariableList::empty(), + voluntary_exits: VariableList::empty(), + sync_aggregate: SyncAggregate::empty(), + execution_payload: Payload::Eip4844::default(), + #[cfg(feature = "withdrawals")] + bls_to_execution_changes: VariableList::empty(), + blob_kzg_commitments: VariableList::empty(), + }, + } + } +} + // We can convert pre-Bellatrix blocks without payloads into blocks "with" payloads. impl From>> for BeaconBlockBase> diff --git a/consensus/types/src/beacon_block_body.rs b/consensus/types/src/beacon_block_body.rs index c0d7b243eb..1dd938ac46 100644 --- a/consensus/types/src/beacon_block_body.rs +++ b/consensus/types/src/beacon_block_body.rs @@ -76,7 +76,7 @@ pub struct BeaconBlockBody = FullPay } impl> BeaconBlockBody { - pub fn execution_payload<'a>(&'a self) -> Result, Error> { + pub fn execution_payload(&self) -> Result, Error> { self.to_ref().execution_payload() } } diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index 66125c2976..000e6f6714 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -296,10 +296,10 @@ where // Withdrawals #[cfg(feature = "withdrawals")] - #[superstruct(only(Capella, Eip4844))] + #[superstruct(only(Capella, Eip4844), partial_getter(copy))] pub next_withdrawal_index: u64, #[cfg(feature = "withdrawals")] - #[superstruct(only(Capella, Eip4844))] + #[superstruct(only(Capella, Eip4844), partial_getter(copy))] pub next_withdrawal_validator_index: u64, // Caching (not in the spec) @@ -1784,6 +1784,8 @@ impl CompareFields for BeaconState { (BeaconState::Base(x), BeaconState::Base(y)) => x.compare_fields(y), (BeaconState::Altair(x), BeaconState::Altair(y)) => x.compare_fields(y), (BeaconState::Merge(x), BeaconState::Merge(y)) => x.compare_fields(y), + (BeaconState::Capella(x), BeaconState::Capella(y)) => x.compare_fields(y), + (BeaconState::Eip4844(x), BeaconState::Eip4844(y)) => x.compare_fields(y), _ => panic!("compare_fields: mismatched state variants",), } } diff --git a/consensus/types/src/beacon_state/tree_hash_cache.rs b/consensus/types/src/beacon_state/tree_hash_cache.rs index e67d4096dd..e50265e660 100644 --- a/consensus/types/src/beacon_state/tree_hash_cache.rs +++ b/consensus/types/src/beacon_state/tree_hash_cache.rs @@ -363,6 +363,16 @@ impl BeaconTreeHashCacheInner { hasher.write(payload_header.tree_hash_root().as_bytes())?; } + // Withdrawal indices (Capella and later). + #[cfg(feature = "withdrawals")] + if let Ok(next_withdrawal_index) = state.next_withdrawal_index() { + hasher.write(next_withdrawal_index.tree_hash_root().as_bytes())?; + } + #[cfg(feature = "withdrawals")] + if let Ok(next_withdrawal_validator_index) = state.next_withdrawal_validator_index() { + hasher.write(next_withdrawal_validator_index.tree_hash_root().as_bytes())?; + } + let root = hasher.finish()?; self.previous_state = Some((root, state.slot())); diff --git a/consensus/types/src/blobs_sidecar.rs b/consensus/types/src/blobs_sidecar.rs index 4e9174d94c..d4e7796060 100644 --- a/consensus/types/src/blobs_sidecar.rs +++ b/consensus/types/src/blobs_sidecar.rs @@ -4,7 +4,6 @@ use serde_derive::{Deserialize, Serialize}; use ssz::Encode; use ssz_derive::{Decode, Encode}; use ssz_types::VariableList; -use tree_hash::TreeHash; use tree_hash_derive::TreeHash; #[cfg_attr(feature = "arbitrary-fuzz", derive(arbitrary::Arbitrary))] @@ -23,6 +22,7 @@ impl BlobsSidecar { pub fn empty() -> Self { Self::default() } + #[allow(clippy::integer_arithmetic)] pub fn max_size() -> usize { // Fixed part Self::empty().as_ssz_bytes().len() diff --git a/consensus/types/src/eth_spec.rs b/consensus/types/src/eth_spec.rs index 4cba9c7950..661484fde8 100644 --- a/consensus/types/src/eth_spec.rs +++ b/consensus/types/src/eth_spec.rs @@ -324,6 +324,7 @@ impl EthSpec for MinimalEthSpec { type SyncSubcommitteeSize = U8; // 32 committee size / 4 sync committee subnet count type MaxPendingAttestations = U1024; // 128 max attestations * 8 slots per epoch type SlotsPerEth1VotingPeriod = U32; // 4 epochs * 8 slots per epoch + type MaxWithdrawalsPerPayload = U4; params_from_eth_spec!(MainnetEthSpec { JustificationBitsLength, @@ -345,7 +346,6 @@ impl EthSpec for MinimalEthSpec { MinGasLimit, MaxExtraDataBytes, MaxBlsToExecutionChanges, - MaxWithdrawalsPerPayload, MaxBlobsPerBlock, FieldElementsPerBlob }); diff --git a/consensus/types/src/execution_payload.rs b/consensus/types/src/execution_payload.rs index eec88b97eb..6036973d5e 100644 --- a/consensus/types/src/execution_payload.rs +++ b/consensus/types/src/execution_payload.rs @@ -1,7 +1,7 @@ use crate::{test_utils::TestRandom, *}; use derivative::Derivative; use serde_derive::{Deserialize, Serialize}; -use ssz::Encode; +use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; use test_random_derive::TestRandom; use tree_hash_derive::TreeHash; @@ -87,6 +87,17 @@ pub struct ExecutionPayload { } impl ExecutionPayload { + pub fn from_ssz_bytes(bytes: &[u8], fork_name: ForkName) -> Result { + match fork_name { + ForkName::Base | ForkName::Altair => Err(ssz::DecodeError::BytesInvalid(format!( + "unsupported fork for ExecutionPayload: {fork_name}", + ))), + ForkName::Merge => ExecutionPayloadMerge::from_ssz_bytes(bytes).map(Self::Merge), + ForkName::Capella => ExecutionPayloadCapella::from_ssz_bytes(bytes).map(Self::Capella), + ForkName::Eip4844 => ExecutionPayloadEip4844::from_ssz_bytes(bytes).map(Self::Eip4844), + } + } + #[allow(clippy::integer_arithmetic)] /// Returns the maximum size of an execution payload. pub fn max_execution_payload_merge_size() -> usize { diff --git a/consensus/types/src/execution_payload_header.rs b/consensus/types/src/execution_payload_header.rs index 7546ca2e5f..6f6b5aa953 100644 --- a/consensus/types/src/execution_payload_header.rs +++ b/consensus/types/src/execution_payload_header.rs @@ -1,6 +1,7 @@ use crate::{test_utils::TestRandom, *}; use derivative::Derivative; use serde_derive::{Deserialize, Serialize}; +use ssz::Decode; use ssz_derive::{Decode, Encode}; use test_random_derive::TestRandom; use tree_hash::TreeHash; @@ -84,31 +85,34 @@ impl ExecutionPayloadHeader { pub fn transactions(&self) -> Option<&Transactions> { None } -} -impl<'a, T: EthSpec> ExecutionPayloadHeaderRef<'a, T> { - // FIXME: maybe this could be a derived trait.. - pub fn is_default(self) -> bool { - match self { - ExecutionPayloadHeaderRef::Merge(header) => { - *header == ExecutionPayloadHeaderMerge::default() + pub fn from_ssz_bytes(bytes: &[u8], fork_name: ForkName) -> Result { + match fork_name { + ForkName::Base | ForkName::Altair => Err(ssz::DecodeError::BytesInvalid(format!( + "unsupported fork for ExecutionPayloadHeader: {fork_name}", + ))), + ForkName::Merge => ExecutionPayloadHeaderMerge::from_ssz_bytes(bytes).map(Self::Merge), + ForkName::Capella => { + ExecutionPayloadHeaderCapella::from_ssz_bytes(bytes).map(Self::Capella) } - ExecutionPayloadHeaderRef::Capella(header) => { - *header == ExecutionPayloadHeaderCapella::default() - } - ExecutionPayloadHeaderRef::Eip4844(header) => { - *header == ExecutionPayloadHeaderEip4844::default() + ForkName::Eip4844 => { + ExecutionPayloadHeaderEip4844::from_ssz_bytes(bytes).map(Self::Eip4844) } } } } +impl<'a, T: EthSpec> ExecutionPayloadHeaderRef<'a, T> { + pub fn is_default(self) -> bool { + map_execution_payload_header_ref!(&'a _, self, |inner, cons| { + let _ = cons(inner); + *inner == Default::default() + }) + } +} + impl ExecutionPayloadHeaderMerge { pub fn upgrade_to_capella(&self) -> ExecutionPayloadHeaderCapella { - #[cfg(feature = "withdrawals")] - // TODO: if this is correct we should calculate and hardcode this.. - let empty_withdrawals_root = - VariableList::::empty().tree_hash_root(); ExecutionPayloadHeaderCapella { parent_hash: self.parent_hash, fee_recipient: self.fee_recipient, @@ -125,8 +129,7 @@ impl ExecutionPayloadHeaderMerge { block_hash: self.block_hash, transactions_root: self.transactions_root, #[cfg(feature = "withdrawals")] - // FIXME: the spec doesn't seem to define what to do here.. - withdrawals_root: empty_withdrawals_root, + withdrawals_root: Hash256::zero(), } } } diff --git a/consensus/types/src/kzg_commitment.rs b/consensus/types/src/kzg_commitment.rs index eaa429a139..9844df0282 100644 --- a/consensus/types/src/kzg_commitment.rs +++ b/consensus/types/src/kzg_commitment.rs @@ -14,7 +14,7 @@ pub struct KzgCommitment(#[serde(with = "BigArray")] pub [u8; 48]); impl Display for KzgCommitment { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "{}", eth2_serde_utils::hex::encode(&self.0)) + write!(f, "{}", eth2_serde_utils::hex::encode(self.0)) } } diff --git a/consensus/types/src/kzg_proof.rs b/consensus/types/src/kzg_proof.rs index 7cd6a8e58b..1c8e49a443 100644 --- a/consensus/types/src/kzg_proof.rs +++ b/consensus/types/src/kzg_proof.rs @@ -1,7 +1,6 @@ use crate::test_utils::{RngCore, TestRandom}; use serde::{Deserialize, Serialize}; use serde_big_array::BigArray; -use ssz::{Decode, DecodeError, Encode}; use ssz_derive::{Decode, Encode}; use std::fmt; use tree_hash::{PackedEncoding, TreeHash}; @@ -15,7 +14,7 @@ pub struct KzgProof(#[serde(with = "BigArray")] pub [u8; KZG_PROOF_BYTES_LEN]); impl fmt::Display for KzgProof { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", eth2_serde_utils::hex::encode(&self.0)) + write!(f, "{}", eth2_serde_utils::hex::encode(self.0)) } } diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index a129a22db7..ec66070b22 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -109,7 +109,7 @@ pub use crate::attestation_duty::AttestationDuty; pub use crate::attester_slashing::AttesterSlashing; pub use crate::beacon_block::{ BeaconBlock, BeaconBlockAltair, BeaconBlockBase, BeaconBlockCapella, BeaconBlockEip4844, - BeaconBlockMerge, BeaconBlockRef, BeaconBlockRefMut, BlindedBeaconBlock, + BeaconBlockMerge, BeaconBlockRef, BeaconBlockRefMut, BlindedBeaconBlock, EmptyBlock, }; pub use crate::beacon_block_body::{ BeaconBlockBody, BeaconBlockBodyAltair, BeaconBlockBodyBase, BeaconBlockBodyCapella, diff --git a/consensus/types/src/payload.rs b/consensus/types/src/payload.rs index 84cc70ed8f..3081dd1cbe 100644 --- a/consensus/types/src/payload.rs +++ b/consensus/types/src/payload.rs @@ -221,7 +221,7 @@ impl ExecPayload for FullPayload { }) } - fn transactions<'a>(&'a self) -> Option<&Transactions> { + fn transactions<'a>(&'a self) -> Option<&'a Transactions> { map_full_payload_ref!(&'a _, self.to_ref(), move |payload, cons| { cons(payload); Some(&payload.execution_payload.transactions) @@ -265,7 +265,7 @@ impl<'b, T: EthSpec> ExecPayload for FullPayloadRef<'b, T> { fn to_execution_payload_header<'a>(&'a self) -> ExecutionPayloadHeader { map_full_payload_ref!(&'a _, self, move |payload, cons| { cons(payload); - ExecutionPayloadHeader::from(payload.to_execution_payload_header()) + payload.to_execution_payload_header() }) } @@ -318,7 +318,7 @@ impl<'b, T: EthSpec> ExecPayload for FullPayloadRef<'b, T> { }) } - fn transactions<'a>(&'a self) -> Option<&Transactions> { + fn transactions<'a>(&'a self) -> Option<&'a Transactions> { map_full_payload_ref!(&'a _, self, move |payload, cons| { cons(payload); Some(&payload.execution_payload.transactions) @@ -488,7 +488,7 @@ impl ExecPayload for BlindedPayload { }) } - fn transactions<'a>(&'a self) -> Option<&Transactions> { + fn transactions(&self) -> Option<&Transactions> { None } @@ -574,7 +574,7 @@ impl<'b, T: EthSpec> ExecPayload for BlindedPayloadRef<'b, T> { }) } - fn transactions<'a>(&'a self) -> Option<&Transactions> { + fn transactions(&self) -> Option<&Transactions> { None } diff --git a/consensus/types/src/tree_hash_impls.rs b/consensus/types/src/tree_hash_impls.rs index ec23927d30..34043c0e83 100644 --- a/consensus/types/src/tree_hash_impls.rs +++ b/consensus/types/src/tree_hash_impls.rs @@ -17,7 +17,7 @@ impl CachedTreeHash for Validator { /// Efficiently tree hash a `Validator`, assuming it was updated by a valid state transition. /// - /// Specifically, we assume that the `pubkey` and `withdrawal_credentials` fields are constant. + /// Specifically, we assume that the `pubkey` field is constant. fn recalculate_tree_hash_root( &self, arena: &mut CacheArena, @@ -29,8 +29,8 @@ impl CachedTreeHash for Validator { .iter_mut(arena)? .enumerate() .flat_map(|(i, leaf)| { - // Fields pubkey and withdrawal_credentials are constant - if (i == 0 || i == 1) && cache.initialized { + // Pubkey field (index 0) is constant. + if i == 0 && cache.initialized { None } else if process_field_by_index(self, i, leaf, !cache.initialized) { Some(i) diff --git a/consensus/types/src/validator.rs b/consensus/types/src/validator.rs index 3e93474927..e4497c809e 100644 --- a/consensus/types/src/validator.rs +++ b/consensus/types/src/validator.rs @@ -1,6 +1,6 @@ use crate::{ - test_utils::TestRandom, Address, BeaconState, BlsToExecutionChange, ChainSpec, Epoch, EthSpec, - Hash256, PublicKeyBytes, + test_utils::TestRandom, Address, BeaconState, ChainSpec, Epoch, EthSpec, Hash256, + PublicKeyBytes, }; use serde_derive::{Deserialize, Serialize}; use ssz_derive::{Decode, Encode}; @@ -76,6 +76,18 @@ impl Validator { .unwrap_or(false) } + /// Get the eth1 withdrawal address if this validator has one initialized. + pub fn get_eth1_withdrawal_address(&self, spec: &ChainSpec) -> Option
{ + self.has_eth1_withdrawal_credential(spec) + .then(|| { + self.withdrawal_credentials + .as_bytes() + .get(12..) + .map(Address::from_slice) + }) + .flatten() + } + /// Changes withdrawal credentials to the provided eth1 execution address /// /// WARNING: this function does NO VALIDATION - it just does it! diff --git a/testing/ef_tests/Makefile b/testing/ef_tests/Makefile index e05ef0b06b..5dd22de8d6 100644 --- a/testing/ef_tests/Makefile +++ b/testing/ef_tests/Makefile @@ -1,4 +1,4 @@ -TESTS_TAG := v1.2.0 +TESTS_TAG := f5c7cf78 TESTS = general minimal mainnet TARBALLS = $(patsubst %,%-$(TESTS_TAG).tar.gz,$(TESTS)) diff --git a/testing/ef_tests/src/cases/epoch_processing.rs b/testing/ef_tests/src/cases/epoch_processing.rs index 26a05715b9..b0e16e12c7 100644 --- a/testing/ef_tests/src/cases/epoch_processing.rs +++ b/testing/ef_tests/src/cases/epoch_processing.rs @@ -289,8 +289,9 @@ impl> Case for EpochProcessing { && T::name() != "participation_flag_updates" } // No phase0 tests for Altair and later. - ForkName::Altair | ForkName::Merge => T::name() != "participation_record_updates", - ForkName::Capella => false, // TODO: revisit when tests are out + ForkName::Altair | ForkName::Merge | ForkName::Capella => { + T::name() != "participation_record_updates" + } ForkName::Eip4844 => false, // TODO: revisit when tests are out } } diff --git a/testing/ef_tests/src/cases/fork.rs b/testing/ef_tests/src/cases/fork.rs index bcc76b8550..f79e13005a 100644 --- a/testing/ef_tests/src/cases/fork.rs +++ b/testing/ef_tests/src/cases/fork.rs @@ -3,7 +3,7 @@ use crate::case_result::compare_beacon_state_results_without_caches; use crate::cases::common::previous_fork; use crate::decode::{ssz_decode_state, yaml_decode_file}; use serde_derive::Deserialize; -use state_processing::upgrade::{upgrade_to_altair, upgrade_to_bellatrix}; +use state_processing::upgrade::{upgrade_to_altair, upgrade_to_bellatrix, upgrade_to_capella}; use types::{BeaconState, ForkName}; #[derive(Debug, Clone, Default, Deserialize)] @@ -61,8 +61,8 @@ impl Case for ForkTest { ForkName::Base => panic!("phase0 not supported"), ForkName::Altair => upgrade_to_altair(&mut result_state, spec).map(|_| result_state), ForkName::Merge => upgrade_to_bellatrix(&mut result_state, spec).map(|_| result_state), + ForkName::Capella => upgrade_to_capella(&mut result_state, spec).map(|_| result_state), ForkName::Eip4844 => panic!("eip4844 not supported"), - ForkName::Capella => panic!("capella not supported"), }; compare_beacon_state_results_without_caches(&mut result, &mut expected) diff --git a/testing/ef_tests/src/cases/genesis_initialization.rs b/testing/ef_tests/src/cases/genesis_initialization.rs index d447fbd8f4..dbf6c70b29 100644 --- a/testing/ef_tests/src/cases/genesis_initialization.rs +++ b/testing/ef_tests/src/cases/genesis_initialization.rs @@ -1,13 +1,10 @@ use super::*; use crate::case_result::compare_beacon_state_results_without_caches; -use crate::decode::{ssz_decode_file, ssz_decode_state, yaml_decode_file}; +use crate::decode::{ssz_decode_file, ssz_decode_file_with, ssz_decode_state, yaml_decode_file}; use serde_derive::Deserialize; use state_processing::initialize_beacon_state_from_eth1; use std::path::PathBuf; -use types::{ - BeaconState, Deposit, EthSpec, ExecutionPayloadHeader, ExecutionPayloadHeaderMerge, ForkName, - Hash256, -}; +use types::{BeaconState, Deposit, EthSpec, ExecutionPayloadHeader, ForkName, Hash256}; #[derive(Debug, Clone, Deserialize)] struct Metadata { @@ -41,14 +38,10 @@ impl LoadCase for GenesisInitialization { let meta: Metadata = yaml_decode_file(&path.join("meta.yaml"))?; let execution_payload_header: Option> = if meta.execution_payload_header.unwrap_or(false) { - //FIXME(sean) we could decode based on timestamp - we probably don't do decode a payload - // without a block this elsewhere at presetn. But when we support SSZ in the builder api we may need to. - // Although that API should include fork info. Hardcoding this for now - Some(ExecutionPayloadHeader::Merge(ssz_decode_file::< - ExecutionPayloadHeaderMerge, - >( + Some(ssz_decode_file_with( &path.join("execution_payload_header.ssz_snappy"), - )?)) + |bytes| ExecutionPayloadHeader::from_ssz_bytes(bytes, fork_name), + )?) } else { None }; diff --git a/testing/ef_tests/src/cases/operations.rs b/testing/ef_tests/src/cases/operations.rs index e3dfb7f67b..8c2ddd1368 100644 --- a/testing/ef_tests/src/cases/operations.rs +++ b/testing/ef_tests/src/cases/operations.rs @@ -3,17 +3,16 @@ use crate::bls_setting::BlsSetting; use crate::case_result::compare_beacon_state_results_without_caches; use crate::decode::{ssz_decode_file, ssz_decode_file_with, ssz_decode_state, yaml_decode_file}; use crate::testing_spec; -use crate::type_name::TypeName; use serde_derive::Deserialize; use state_processing::{ per_block_processing::{ errors::BlockProcessingError, process_block_header, process_execution_payload, process_operations::{ - altair, base, process_attester_slashings, process_deposits, process_exits, - process_proposer_slashings, + altair, base, process_attester_slashings, process_bls_to_execution_changes, + process_deposits, process_exits, process_proposer_slashings, }, - process_sync_aggregate, VerifyBlockRoot, VerifySignatures, + process_sync_aggregate, process_withdrawals, VerifyBlockRoot, VerifySignatures, }, ConsensusContext, }; @@ -21,7 +20,7 @@ use std::fmt::Debug; use std::path::Path; use types::{ Attestation, AttesterSlashing, BeaconBlock, BeaconState, BlindedPayload, ChainSpec, Deposit, - EthSpec, ExecutionPayload, ExecutionPayloadMerge, ForkName, FullPayload, ProposerSlashing, + EthSpec, ExecutionPayload, ForkName, FullPayload, ProposerSlashing, SignedBlsToExecutionChange, SignedVoluntaryExit, SyncAggregate, }; @@ -36,6 +35,12 @@ struct ExecutionMetadata { execution_valid: bool, } +/// Newtype for testing withdrawals. +#[derive(Debug, Clone, Deserialize)] +pub struct WithdrawalsPayload { + payload: FullPayload, +} + #[derive(Debug, Clone)] pub struct Operations> { metadata: Metadata, @@ -45,10 +50,8 @@ pub struct Operations> { pub post: Option>, } -pub trait Operation: TypeName + Debug + Sync + Sized { - fn handler_name() -> String { - Self::name().to_lowercase() - } +pub trait Operation: Debug + Sync + Sized { + fn handler_name() -> String; fn filename() -> String { format!("{}.ssz_snappy", Self::handler_name()) @@ -58,7 +61,7 @@ pub trait Operation: TypeName + Debug + Sync + Sized { true } - fn decode(path: &Path, spec: &ChainSpec) -> Result; + fn decode(path: &Path, fork_name: ForkName, spec: &ChainSpec) -> Result; fn apply_to( &self, @@ -69,7 +72,11 @@ pub trait Operation: TypeName + Debug + Sync + Sized { } impl Operation for Attestation { - fn decode(path: &Path, _spec: &ChainSpec) -> Result { + fn handler_name() -> String { + "attestation".into() + } + + fn decode(path: &Path, _fork_name: ForkName, _spec: &ChainSpec) -> Result { ssz_decode_file(path) } @@ -109,7 +116,7 @@ impl Operation for AttesterSlashing { "attester_slashing".into() } - fn decode(path: &Path, _spec: &ChainSpec) -> Result { + fn decode(path: &Path, _fork_name: ForkName, _spec: &ChainSpec) -> Result { ssz_decode_file(path) } @@ -131,7 +138,11 @@ impl Operation for AttesterSlashing { } impl Operation for Deposit { - fn decode(path: &Path, _spec: &ChainSpec) -> Result { + fn handler_name() -> String { + "deposit".into() + } + + fn decode(path: &Path, _fork_name: ForkName, _spec: &ChainSpec) -> Result { ssz_decode_file(path) } @@ -155,7 +166,7 @@ impl Operation for ProposerSlashing { "proposer_slashing".into() } - fn decode(path: &Path, _spec: &ChainSpec) -> Result { + fn decode(path: &Path, _fork_name: ForkName, _spec: &ChainSpec) -> Result { ssz_decode_file(path) } @@ -181,7 +192,7 @@ impl Operation for SignedVoluntaryExit { "voluntary_exit".into() } - fn decode(path: &Path, _spec: &ChainSpec) -> Result { + fn decode(path: &Path, _fork_name: ForkName, _spec: &ChainSpec) -> Result { ssz_decode_file(path) } @@ -204,7 +215,7 @@ impl Operation for BeaconBlock { "block.ssz_snappy".into() } - fn decode(path: &Path, spec: &ChainSpec) -> Result { + fn decode(path: &Path, _fork_name: ForkName, spec: &ChainSpec) -> Result { ssz_decode_file_with(path, |bytes| BeaconBlock::from_ssz_bytes(bytes, spec)) } @@ -239,7 +250,7 @@ impl Operation for SyncAggregate { fork_name != ForkName::Base } - fn decode(path: &Path, _spec: &ChainSpec) -> Result { + fn decode(path: &Path, _fork_name: ForkName, _spec: &ChainSpec) -> Result { ssz_decode_file(path) } @@ -267,13 +278,11 @@ impl Operation for FullPayload { fork_name != ForkName::Base && fork_name != ForkName::Altair } - //FIXME(sean) we could decode based on timestamp - we probably don't do decode a payload - // without a block this elsewhere at presetn. But when we support SSZ in the builder api we may need to. - // Although that API should include fork info. Hardcoding this for now - fn decode(path: &Path, _spec: &ChainSpec) -> Result { - ssz_decode_file::>(path) - .map(ExecutionPayload::Merge) - .map(Into::into) + fn decode(path: &Path, fork_name: ForkName, _spec: &ChainSpec) -> Result { + ssz_decode_file_with(path, |bytes| { + ExecutionPayload::from_ssz_bytes(bytes, fork_name) + }) + .map(Into::into) } fn apply_to( @@ -306,13 +315,11 @@ impl Operation for BlindedPayload { fork_name != ForkName::Base && fork_name != ForkName::Altair } - fn decode(path: &Path, _spec: &ChainSpec) -> Result { - //FIXME(sean) we could decode based on timestamp - we probably don't do decode a payload - // without a block this elsewhere at presetn. But when we support SSZ in the builder api we may need to. - // Although that API should include fork info. Hardcoding this for now - let payload: Result, Error> = - ssz_decode_file::>(path).map(Into::into); - payload.map(Into::into) + fn decode(path: &Path, fork_name: ForkName, _spec: &ChainSpec) -> Result { + ssz_decode_file_with(path, |bytes| { + ExecutionPayload::from_ssz_bytes(bytes, fork_name) + }) + .map(Into::into) } fn apply_to( @@ -333,6 +340,65 @@ impl Operation for BlindedPayload { } } +impl Operation for WithdrawalsPayload { + fn handler_name() -> String { + "withdrawals".into() + } + + fn filename() -> String { + "execution_payload.ssz_snappy".into() + } + + fn is_enabled_for_fork(fork_name: ForkName) -> bool { + fork_name != ForkName::Base && fork_name != ForkName::Altair && fork_name != ForkName::Merge + } + + fn decode(path: &Path, fork_name: ForkName, _spec: &ChainSpec) -> Result { + ssz_decode_file_with(path, |bytes| { + ExecutionPayload::from_ssz_bytes(bytes, fork_name) + }) + .map(|payload| WithdrawalsPayload { + payload: payload.into(), + }) + } + + fn apply_to( + &self, + state: &mut BeaconState, + spec: &ChainSpec, + _: &Operations, + ) -> Result<(), BlockProcessingError> { + process_withdrawals::<_, FullPayload<_>>(state, self.payload.to_ref(), spec) + } +} + +impl Operation for SignedBlsToExecutionChange { + fn handler_name() -> String { + "bls_to_execution_change".into() + } + + fn filename() -> String { + "address_change.ssz_snappy".into() + } + + fn is_enabled_for_fork(fork_name: ForkName) -> bool { + fork_name != ForkName::Base && fork_name != ForkName::Altair && fork_name != ForkName::Merge + } + + fn decode(path: &Path, _fork_name: ForkName, _spec: &ChainSpec) -> Result { + ssz_decode_file(path) + } + + fn apply_to( + &self, + state: &mut BeaconState, + spec: &ChainSpec, + _extra: &Operations, + ) -> Result<(), BlockProcessingError> { + process_bls_to_execution_changes(state, &[self.clone()], VerifySignatures::True, spec) + } +} + impl> LoadCase for Operations { fn load_from_dir(path: &Path, fork_name: ForkName) -> Result { let spec = &testing_spec::(fork_name); @@ -356,7 +422,7 @@ impl> LoadCase for Operations { // Check BLS setting here before SSZ deserialization, as most types require signatures // to be valid. let (operation, bls_error) = if metadata.bls_setting.unwrap_or_default().check().is_ok() { - match O::decode(&path.join(O::filename()), spec) { + match O::decode(&path.join(O::filename()), fork_name, spec) { Ok(op) => (Some(op), None), Err(Error::InvalidBLSInput(error)) => (None, Some(error)), Err(e) => return Err(e), @@ -399,9 +465,11 @@ impl> Case for Operations { let mut expected = self.post.clone(); // Processing requires the committee caches. - state - .build_all_committee_caches(spec) - .expect("committee caches OK"); + // NOTE: some of the withdrawals tests have 0 active validators, do not try + // to build the commitee cache in this case. + if O::handler_name() != "withdrawals" { + state.build_all_committee_caches(spec).unwrap(); + } let mut result = self .operation diff --git a/testing/ef_tests/src/cases/transition.rs b/testing/ef_tests/src/cases/transition.rs index 469285ab0f..fb7ccfea64 100644 --- a/testing/ef_tests/src/cases/transition.rs +++ b/testing/ef_tests/src/cases/transition.rs @@ -42,14 +42,17 @@ impl LoadCase for TransitionTest { spec.altair_fork_epoch = Some(Epoch::new(0)); spec.bellatrix_fork_epoch = Some(metadata.fork_epoch); } - ForkName::Eip4844 => { - spec.bellatrix_fork_epoch = Some(Epoch::new(0)); - spec.eip4844_fork_epoch = Some(metadata.fork_epoch); - } ForkName::Capella => { - spec.capella_fork_epoch = Some(Epoch::new(0)); + spec.altair_fork_epoch = Some(Epoch::new(0)); + spec.bellatrix_fork_epoch = Some(Epoch::new(0)); spec.capella_fork_epoch = Some(metadata.fork_epoch); } + ForkName::Eip4844 => { + spec.altair_fork_epoch = Some(Epoch::new(0)); + spec.bellatrix_fork_epoch = Some(Epoch::new(0)); + spec.capella_fork_epoch = Some(Epoch::new(0)); + spec.eip4844_fork_epoch = Some(metadata.fork_epoch); + } } // Load blocks diff --git a/testing/ef_tests/src/handler.rs b/testing/ef_tests/src/handler.rs index dd5ed82da7..ed376af444 100644 --- a/testing/ef_tests/src/handler.rs +++ b/testing/ef_tests/src/handler.rs @@ -24,6 +24,11 @@ pub trait Handler { fn run(&self) { for fork_name in ForkName::list_all() { + // FIXME(eip4844): enable eip4844 + if fork_name == ForkName::Eip4844 { + continue; + } + if self.is_enabled_for_fork(fork_name) { self.run_for_fork(fork_name) } @@ -218,6 +223,10 @@ impl SszStaticHandler { Self::for_forks(vec![ForkName::Merge]) } + pub fn capella_only() -> Self { + Self::for_forks(vec![ForkName::Capella]) + } + pub fn merge_and_later() -> Self { Self::for_forks(ForkName::list_all()[2..].to_vec()) } @@ -533,10 +542,8 @@ impl Handler for ForkChoiceHandler { } fn is_enabled_for_fork(&self, fork_name: ForkName) -> bool { - // Merge block tests are only enabled for Bellatrix or later. - if self.handler_name == "on_merge_block" - && (fork_name == ForkName::Base || fork_name == ForkName::Altair) - { + // Merge block tests are only enabled for Bellatrix. + if self.handler_name == "on_merge_block" && fork_name != ForkName::Merge { return false; } diff --git a/testing/ef_tests/src/lib.rs b/testing/ef_tests/src/lib.rs index 5c2ca3fb55..d45b1e15c7 100644 --- a/testing/ef_tests/src/lib.rs +++ b/testing/ef_tests/src/lib.rs @@ -1,10 +1,9 @@ pub use case_result::CaseResult; -pub use cases::Case; pub use cases::{ - EffectiveBalanceUpdates, Eth1DataReset, HistoricalRootsUpdate, InactivityUpdates, + Case, EffectiveBalanceUpdates, Eth1DataReset, HistoricalRootsUpdate, InactivityUpdates, JustificationAndFinalization, ParticipationFlagUpdates, ParticipationRecordUpdates, RandaoMixesReset, RegistryUpdates, RewardsAndPenalties, Slashings, SlashingsReset, - SyncCommitteeUpdates, + SyncCommitteeUpdates, WithdrawalsPayload, }; pub use decode::log_file_access; pub use error::Error; diff --git a/testing/ef_tests/src/type_name.rs b/testing/ef_tests/src/type_name.rs index c075e89b3f..bee2d9b03d 100644 --- a/testing/ef_tests/src/type_name.rs +++ b/testing/ef_tests/src/type_name.rs @@ -45,6 +45,8 @@ type_name_generic!(BeaconBlockBody); type_name_generic!(BeaconBlockBodyBase, "BeaconBlockBody"); type_name_generic!(BeaconBlockBodyAltair, "BeaconBlockBody"); type_name_generic!(BeaconBlockBodyMerge, "BeaconBlockBody"); +type_name_generic!(BeaconBlockBodyCapella, "BeaconBlockBody"); +type_name_generic!(BeaconBlockBodyEip4844, "BeaconBlockBody"); type_name!(BeaconBlockHeader); type_name_generic!(BeaconState); type_name!(Checkpoint); @@ -54,8 +56,14 @@ type_name!(DepositData); type_name!(DepositMessage); type_name!(Eth1Data); type_name_generic!(ExecutionPayload); +type_name_generic!(ExecutionPayloadMerge, "ExecutionPayload"); +type_name_generic!(ExecutionPayloadCapella, "ExecutionPayload"); +type_name_generic!(ExecutionPayloadEip4844, "ExecutionPayload"); type_name_generic!(FullPayload, "ExecutionPayload"); type_name_generic!(ExecutionPayloadHeader); +type_name_generic!(ExecutionPayloadHeaderMerge, "ExecutionPayloadHeader"); +type_name_generic!(ExecutionPayloadHeaderCapella, "ExecutionPayloadHeader"); +type_name_generic!(ExecutionPayloadHeaderEip4844, "ExecutionPayloadHeader"); type_name_generic!(BlindedPayload, "ExecutionPayloadHeader"); type_name!(Fork); type_name!(ForkData); @@ -76,3 +84,6 @@ type_name_generic!(SyncAggregate); type_name_generic!(SyncCommittee); type_name!(Validator); type_name!(VoluntaryExit); +type_name!(Withdrawal); +type_name!(BlsToExecutionChange, "BLSToExecutionChange"); +type_name!(SignedBlsToExecutionChange, "SignedBLSToExecutionChange"); diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index 28c57028cf..338a56b9f0 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -82,6 +82,18 @@ fn operations_execution_payload_blinded() { OperationsHandler::>::default().run(); } +#[test] +fn operations_withdrawals() { + OperationsHandler::>::default().run(); + OperationsHandler::>::default().run(); +} + +#[test] +fn operations_bls_to_execution_change() { + OperationsHandler::::default().run(); + OperationsHandler::::default().run(); +} + #[test] fn sanity_blocks() { SanityBlocksHandler::::default().run(); @@ -250,6 +262,10 @@ mod ssz_static { .run(); SszStaticHandler::, MainnetEthSpec>::merge_only() .run(); + SszStaticHandler::, MinimalEthSpec>::capella_only() + .run(); + SszStaticHandler::, MainnetEthSpec>::capella_only() + .run(); } // Altair and later @@ -302,18 +318,44 @@ mod ssz_static { // Merge and later #[test] fn execution_payload() { - SszStaticHandler::, MinimalEthSpec>::merge_and_later() + SszStaticHandler::, MinimalEthSpec>::merge_only() .run(); - SszStaticHandler::, MainnetEthSpec>::merge_and_later() + SszStaticHandler::, MainnetEthSpec>::merge_only() + .run(); + SszStaticHandler::, MinimalEthSpec>::capella_only() + .run(); + SszStaticHandler::, MainnetEthSpec>::capella_only() .run(); } #[test] fn execution_payload_header() { - SszStaticHandler::, MinimalEthSpec>::merge_and_later() + SszStaticHandler::, MinimalEthSpec>::merge_only() .run(); - SszStaticHandler::, MainnetEthSpec>::merge_and_later() + SszStaticHandler::, MainnetEthSpec>::merge_only() .run(); + SszStaticHandler::, MinimalEthSpec> + ::capella_only().run(); + SszStaticHandler::, MainnetEthSpec> + ::capella_only().run(); + } + + #[test] + fn withdrawal() { + SszStaticHandler::::capella_only().run(); + SszStaticHandler::::capella_only().run(); + } + + #[test] + fn bls_to_execution_change() { + SszStaticHandler::::capella_only().run(); + SszStaticHandler::::capella_only().run(); + } + + #[test] + fn signed_bls_to_execution_change() { + SszStaticHandler::::capella_only().run(); + SszStaticHandler::::capella_only().run(); } }