diff --git a/Makefile b/Makefile index e1fba5b42..41721c2d6 100644 --- a/Makefile +++ b/Makefile @@ -89,12 +89,12 @@ build-release-tarballs: # Runs the full workspace tests in **release**, without downloading any additional # test vectors. test-release: - cargo test --workspace --release --exclude ef_tests --exclude beacon_chain --exclude slasher + cargo test --workspace --features withdrawals-processing --release --exclude ef_tests --exclude beacon_chain --exclude slasher # Runs the full workspace tests in **debug**, without downloading any additional test # vectors. test-debug: - cargo test --workspace --exclude ef_tests --exclude beacon_chain + cargo test --workspace --features withdrawals-processing --exclude ef_tests --exclude beacon_chain # Runs cargo-fmt (linter). cargo-fmt: diff --git a/beacon_node/Cargo.toml b/beacon_node/Cargo.toml index bed32011f..d6b0b643a 100644 --- a/beacon_node/Cargo.toml +++ b/beacon_node/Cargo.toml @@ -17,7 +17,6 @@ withdrawals-processing = [ "beacon_chain/withdrawals-processing", "store/withdrawals-processing", "execution_layer/withdrawals-processing", - "http_api/withdrawals-processing", ] [dependencies] diff --git a/beacon_node/beacon_chain/Cargo.toml b/beacon_node/beacon_chain/Cargo.toml index a6ac66037..c89f1650e 100644 --- a/beacon_node/beacon_chain/Cargo.toml +++ b/beacon_node/beacon_chain/Cargo.toml @@ -12,9 +12,7 @@ participation_metrics = [] # Exposes validator participation metrics to Prometh fork_from_env = [] # Initialise the harness chain spec from the FORK_NAME env variable withdrawals-processing = [ "state_processing/withdrawals-processing", - "store/withdrawals-processing", "execution_layer/withdrawals-processing", - "operation_pool/withdrawals-processing" ] [dev-dependencies] diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index edf0e149c..798a9b808 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -362,7 +362,6 @@ pub struct BeaconChain { pub(crate) observed_attester_slashings: Mutex, T::EthSpec>>, /// Maintains a record of which validators we've seen BLS to execution changes for. - #[cfg(feature = "withdrawals-processing")] pub(crate) observed_bls_to_execution_changes: Mutex>, /// The most recently validated light client finality update received on gossip. @@ -2232,29 +2231,18 @@ impl BeaconChain { &self, bls_to_execution_change: SignedBlsToExecutionChange, ) -> Result, Error> { - #[cfg(feature = "withdrawals-processing")] - { - let current_fork = self.spec.fork_name_at_slot::(self.slot()?); - if let ForkName::Base | ForkName::Altair | ForkName::Merge = current_fork { - // Disallow BLS to execution changes prior to the Capella fork. - return Err(Error::BlsToExecutionChangeBadFork(current_fork)); - } - - let wall_clock_state = self.wall_clock_state()?; - - Ok(self - .observed_bls_to_execution_changes - .lock() - .verify_and_observe(bls_to_execution_change, &wall_clock_state, &self.spec)?) + let current_fork = self.spec.fork_name_at_slot::(self.slot()?); + if let ForkName::Base | ForkName::Altair | ForkName::Merge = current_fork { + // Disallow BLS to execution changes prior to the Capella fork. + return Err(Error::BlsToExecutionChangeBadFork(current_fork)); } - // TODO: remove this whole block once withdrawals-processing is removed - #[cfg(not(feature = "withdrawals-processing"))] - { - #[allow(clippy::drop_non_drop)] - drop(bls_to_execution_change); - Ok(ObservationOutcome::AlreadyKnown) - } + let wall_clock_state = self.wall_clock_state()?; + + Ok(self + .observed_bls_to_execution_changes + .lock() + .verify_and_observe(bls_to_execution_change, &wall_clock_state, &self.spec)?) } /// Import a BLS to execution change to the op pool. @@ -2263,12 +2251,8 @@ impl BeaconChain { bls_to_execution_change: SigVerifiedOp, ) { if self.eth1_chain.is_some() { - #[cfg(feature = "withdrawals-processing")] self.op_pool .insert_bls_to_execution_change(bls_to_execution_change); - - #[cfg(not(feature = "withdrawals-processing"))] - drop(bls_to_execution_change); } } diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 3073de9ca..b73ad2f25 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -798,7 +798,6 @@ where observed_voluntary_exits: <_>::default(), observed_proposer_slashings: <_>::default(), observed_attester_slashings: <_>::default(), - #[cfg(feature = "withdrawals-processing")] observed_bls_to_execution_changes: <_>::default(), latest_seen_finality_update: <_>::default(), latest_seen_optimistic_update: <_>::default(), diff --git a/beacon_node/beacon_chain/src/observed_operations.rs b/beacon_node/beacon_chain/src/observed_operations.rs index 5781f9b5b..6e5337393 100644 --- a/beacon_node/beacon_chain/src/observed_operations.rs +++ b/beacon_node/beacon_chain/src/observed_operations.rs @@ -6,12 +6,9 @@ use std::collections::HashSet; use std::marker::PhantomData; use types::{ AttesterSlashing, BeaconState, ChainSpec, EthSpec, ForkName, ProposerSlashing, - SignedVoluntaryExit, Slot, + SignedBlsToExecutionChange, SignedVoluntaryExit, Slot, }; -#[cfg(feature = "withdrawals-processing")] -use types::SignedBlsToExecutionChange; - /// Number of validator indices to store on the stack in `observed_validators`. pub const SMALL_VEC_SIZE: usize = 8; @@ -83,7 +80,6 @@ impl ObservableOperation for AttesterSlashing { } } -#[cfg(feature = "withdrawals-processing")] impl ObservableOperation for SignedBlsToExecutionChange { fn observed_validators(&self) -> SmallVec<[u64; SMALL_VEC_SIZE]> { smallvec![self.message.validator_index] diff --git a/beacon_node/execution_layer/Cargo.toml b/beacon_node/execution_layer/Cargo.toml index 47c1e0341..30312939d 100644 --- a/beacon_node/execution_layer/Cargo.toml +++ b/beacon_node/execution_layer/Cargo.toml @@ -5,7 +5,7 @@ edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [features] -withdrawals-processing = ["state_processing/withdrawals-processing", "eth2/withdrawals-processing"] +withdrawals-processing = ["state_processing/withdrawals-processing"] [dependencies] types = { path = "../../consensus/types"} diff --git a/beacon_node/http_api/Cargo.toml b/beacon_node/http_api/Cargo.toml index e8a97fd0b..077e3aa7c 100644 --- a/beacon_node/http_api/Cargo.toml +++ b/beacon_node/http_api/Cargo.toml @@ -5,9 +5,6 @@ authors = ["Paul Hauner "] edition = "2021" autotests = false # using a single test binary compiles faster -[features] -withdrawals-processing = [] - [dependencies] warp = { version = "0.3.2", features = ["tls"] } serde = { version = "1.0.116", features = ["derive"] } diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 2d6c44680..9d04938ad 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -1687,16 +1687,12 @@ pub fn serve( match chain.verify_bls_to_execution_change_for_gossip(address_change) { Ok(ObservationOutcome::New(verified_address_change)) => { - #[cfg(feature = "withdrawals-processing")] - { - publish_pubsub_message( - &network_tx, - PubsubMessage::BlsToExecutionChange(Box::new( - verified_address_change.as_inner().clone(), - )), - )?; - } - + publish_pubsub_message( + &network_tx, + PubsubMessage::BlsToExecutionChange(Box::new( + verified_address_change.as_inner().clone(), + )), + )?; chain.import_bls_to_execution_change(verified_address_change); } Ok(ObservationOutcome::AlreadyKnown) => { diff --git a/beacon_node/operation_pool/Cargo.toml b/beacon_node/operation_pool/Cargo.toml index d75235443..848323358 100644 --- a/beacon_node/operation_pool/Cargo.toml +++ b/beacon_node/operation_pool/Cargo.toml @@ -4,9 +4,6 @@ version = "0.2.0" authors = ["Michael Sproul "] edition = "2021" -[features] -withdrawals-processing = [] - [dependencies] derivative = "2.1.1" itertools = "0.10.0" diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index 1f4660fc2..a69e0a750 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -51,7 +51,6 @@ pub struct OperationPool { /// Map from exiting validator to their exit data. voluntary_exits: RwLock>>, /// Map from credential changing validator to their execution change data. - #[cfg(feature = "withdrawals-processing")] bls_to_execution_changes: RwLock>>, /// Reward cache for accelerating attestation packing. reward_cache: RwLock, @@ -518,17 +517,10 @@ impl OperationPool { &self, verified_change: SigVerifiedOp, ) { - #[cfg(feature = "withdrawals-processing")] - { - self.bls_to_execution_changes.write().insert( - verified_change.as_inner().message.validator_index, - verified_change, - ); - } - #[cfg(not(feature = "withdrawals-processing"))] - { - drop(verified_change); - } + self.bls_to_execution_changes.write().insert( + verified_change.as_inner().message.validator_index, + verified_change, + ); } /// Get a list of execution changes for inclusion in a block. @@ -539,32 +531,19 @@ impl OperationPool { state: &BeaconState, spec: &ChainSpec, ) -> Vec { - #[cfg(feature = "withdrawals-processing")] - { - filter_limit_operations( - self.bls_to_execution_changes.read().values(), - |address_change| { - address_change.signature_is_still_valid(&state.fork()) - && state - .get_validator( - address_change.as_inner().message.validator_index as usize, - ) - .map_or(false, |validator| { - !validator.has_eth1_withdrawal_credential(spec) - }) - }, - |address_change| address_change.as_inner().clone(), - T::MaxBlsToExecutionChanges::to_usize(), - ) - } - - // TODO: remove this whole block once withdrwals-processing is removed - #[cfg(not(feature = "withdrawals-processing"))] - { - #[allow(clippy::drop_copy)] - drop((state, spec)); - vec![] - } + filter_limit_operations( + self.bls_to_execution_changes.read().values(), + |address_change| { + address_change.signature_is_still_valid(&state.fork()) + && state + .get_validator(address_change.as_inner().message.validator_index as usize) + .map_or(false, |validator| { + !validator.has_eth1_withdrawal_credential(spec) + }) + }, + |address_change| address_change.as_inner().clone(), + T::MaxBlsToExecutionChanges::to_usize(), + ) } /// Prune BLS to execution changes that have been applied to the state more than 1 block ago. @@ -579,32 +558,22 @@ impl OperationPool { head_state: &BeaconState, spec: &ChainSpec, ) { - #[cfg(feature = "withdrawals-processing")] - { - prune_validator_hash_map( - &mut self.bls_to_execution_changes.write(), - |validator_index, validator| { - validator.has_eth1_withdrawal_credential(spec) - && head_block - .message() - .body() - .bls_to_execution_changes() - .map_or(true, |recent_changes| { - !recent_changes - .iter() - .any(|c| c.message.validator_index == validator_index) - }) - }, - head_state, - ); - } - - // TODO: remove this whole block once withdrwals-processing is removed - #[cfg(not(feature = "withdrawals-processing"))] - { - #[allow(clippy::drop_copy)] - drop((head_block, head_state, spec)); - } + prune_validator_hash_map( + &mut self.bls_to_execution_changes.write(), + |validator_index, validator| { + validator.has_eth1_withdrawal_credential(spec) + && head_block + .message() + .body() + .bls_to_execution_changes() + .map_or(true, |recent_changes| { + !recent_changes + .iter() + .any(|c| c.message.validator_index == validator_index) + }) + }, + head_state, + ); } /// Prune all types of transactions given the latest head state and head fork. @@ -691,17 +660,11 @@ impl OperationPool { /// /// This method may return objects that are invalid for block inclusion. pub fn get_all_bls_to_execution_changes(&self) -> Vec { - #[cfg(feature = "withdrawals-processing")] - { - self.bls_to_execution_changes - .read() - .iter() - .map(|(_, address_change)| address_change.as_inner().clone()) - .collect() - } - - #[cfg(not(feature = "withdrawals-processing"))] - vec![] + self.bls_to_execution_changes + .read() + .iter() + .map(|(_, address_change)| address_change.as_inner().clone()) + .collect() } } diff --git a/beacon_node/operation_pool/src/persistence.rs b/beacon_node/operation_pool/src/persistence.rs index 184b967db..b232e5a55 100644 --- a/beacon_node/operation_pool/src/persistence.rs +++ b/beacon_node/operation_pool/src/persistence.rs @@ -143,7 +143,6 @@ impl PersistedOperationPool { proposer_slashings, voluntary_exits, // FIXME(capella): implement schema migration for address changes in op pool - #[cfg(feature = "withdrawals-processing")] bls_to_execution_changes: Default::default(), reward_cache: Default::default(), _phantom: Default::default(), diff --git a/common/eth2/Cargo.toml b/common/eth2/Cargo.toml index fc5eba98e..eca086d83 100644 --- a/common/eth2/Cargo.toml +++ b/common/eth2/Cargo.toml @@ -35,4 +35,3 @@ procinfo = { version = "0.4.2", optional = true } [features] default = ["lighthouse"] lighthouse = ["proto_array", "psutil", "procinfo", "store", "slashing_protection"] -withdrawals-processing = ["store/withdrawals-processing"] \ No newline at end of file diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index f1a544099..0192fe0ce 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -19,7 +19,6 @@ pub use process_operations::process_operations; pub use verify_attestation::{ verify_attestation_for_block_inclusion, verify_attestation_for_state, }; -#[cfg(feature = "withdrawals-processing")] pub use verify_bls_to_execution_change::verify_bls_to_execution_change; pub use verify_deposit::{ get_existing_validator_index, verify_deposit_merkle_proof, verify_deposit_signature, @@ -36,13 +35,11 @@ pub mod signature_sets; pub mod tests; mod verify_attestation; mod verify_attester_slashing; -#[cfg(feature = "withdrawals-processing")] mod verify_bls_to_execution_change; mod verify_deposit; mod verify_exit; mod verify_proposer_slashing; -#[cfg(feature = "withdrawals-processing")] use crate::common::decrease_balance; #[cfg(feature = "arbitrary-fuzz")] @@ -165,7 +162,6 @@ pub fn per_block_processing>( // previous block. if is_execution_enabled(state, block.body()) { let payload = block.body().execution_payload()?; - #[cfg(feature = "withdrawals-processing")] process_withdrawals::(state, payload, spec)?; process_execution_payload::(state, payload, spec)?; } @@ -524,12 +520,14 @@ pub fn get_expected_withdrawals( } /// Apply withdrawals to the state. -#[cfg(feature = "withdrawals-processing")] pub fn process_withdrawals<'payload, T: EthSpec, Payload: AbstractExecPayload>( state: &mut BeaconState, payload: Payload::Ref<'payload>, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { + if cfg!(not(feature = "withdrawals-processing")) { + return Ok(()); + } match state { BeaconState::Merge(_) => Ok(()), BeaconState::Capella(_) | BeaconState::Eip4844(_) => { 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 f27fd48b4..eacb7617c 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -34,7 +34,6 @@ pub fn process_operations<'a, T: EthSpec, Payload: AbstractExecPayload>( process_deposits(state, block_body.deposits(), spec)?; process_exits(state, block_body.voluntary_exits(), verify_signatures, spec)?; - #[cfg(feature = "withdrawals-processing")] 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)?; } @@ -295,13 +294,15 @@ pub fn process_exits( /// /// Returns `Ok(())` if the validation and state updates completed successfully. Otherwise returns /// an `Err` describing the invalid object or cause of failure. -#[cfg(feature = "withdrawals-processing")] pub fn process_bls_to_execution_changes( state: &mut BeaconState, bls_to_execution_changes: &[SignedBlsToExecutionChange], verify_signatures: VerifySignatures, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { + if cfg!(not(feature = "withdrawals-processing")) { + return Ok(()); + } 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))?; diff --git a/consensus/state_processing/src/verify_operation.rs b/consensus/state_processing/src/verify_operation.rs index e2e434417..efd356462 100644 --- a/consensus/state_processing/src/verify_operation.rs +++ b/consensus/state_processing/src/verify_operation.rs @@ -1,8 +1,10 @@ use crate::per_block_processing::{ errors::{ - AttesterSlashingValidationError, ExitValidationError, ProposerSlashingValidationError, + AttesterSlashingValidationError, BlsExecutionChangeValidationError, ExitValidationError, + ProposerSlashingValidationError, }, - verify_attester_slashing, verify_exit, verify_proposer_slashing, + verify_attester_slashing, verify_bls_to_execution_change, verify_exit, + verify_proposer_slashing, }; use crate::VerifySignatures; use derivative::Derivative; @@ -12,15 +14,7 @@ use ssz_derive::{Decode, Encode}; use std::marker::PhantomData; use types::{ AttesterSlashing, BeaconState, ChainSpec, Epoch, EthSpec, Fork, ForkVersion, ProposerSlashing, - SignedVoluntaryExit, -}; - -#[cfg(feature = "withdrawals-processing")] -use { - crate::per_block_processing::{ - errors::BlsExecutionChangeValidationError, verify_bls_to_execution_change, - }, - types::SignedBlsToExecutionChange, + SignedBlsToExecutionChange, SignedVoluntaryExit, }; const MAX_FORKS_VERIFIED_AGAINST: usize = 2; @@ -202,7 +196,6 @@ impl VerifyOperation for ProposerSlashing { } } -#[cfg(feature = "withdrawals-processing")] impl VerifyOperation for SignedBlsToExecutionChange { type Error = BlsExecutionChangeValidationError; diff --git a/testing/ef_tests/src/cases/operations.rs b/testing/ef_tests/src/cases/operations.rs index a08ee1996..71954405c 100644 --- a/testing/ef_tests/src/cases/operations.rs +++ b/testing/ef_tests/src/cases/operations.rs @@ -4,30 +4,24 @@ 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 serde_derive::Deserialize; -#[cfg(feature = "withdrawals-processing")] -use state_processing::per_block_processing::process_operations::{ - process_bls_to_execution_changes, process_bls_to_execution_changes, -}; 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, }; use std::fmt::Debug; use std::path::Path; -#[cfg(feature = "withdrawals-processing")] -use types::SignedBlsToExecutionChange; use types::{ Attestation, AttesterSlashing, BeaconBlock, BeaconState, BlindedPayload, ChainSpec, Deposit, - EthSpec, ExecutionPayload, ForkName, FullPayload, ProposerSlashing, SignedVoluntaryExit, - SyncAggregate, + EthSpec, ExecutionPayload, ForkName, FullPayload, ProposerSlashing, SignedBlsToExecutionChange, + SignedVoluntaryExit, SyncAggregate, }; #[derive(Debug, Clone, Default, Deserialize)] @@ -42,7 +36,6 @@ struct ExecutionMetadata { } /// Newtype for testing withdrawals. -#[cfg(feature = "withdrawals-processing")] #[derive(Debug, Clone, Deserialize)] pub struct WithdrawalsPayload { payload: FullPayload, @@ -341,7 +334,6 @@ impl Operation for BlindedPayload { } } -#[cfg(feature = "withdrawals-processing")] impl Operation for WithdrawalsPayload { fn handler_name() -> String { "withdrawals".into() @@ -374,7 +366,6 @@ impl Operation for WithdrawalsPayload { } } -#[cfg(feature = "withdrawals-processing")] impl Operation for SignedBlsToExecutionChange { fn handler_name() -> String { "bls_to_execution_change".into() diff --git a/testing/ef_tests/src/lib.rs b/testing/ef_tests/src/lib.rs index a4d4f2d52..f52b7bca1 100644 --- a/testing/ef_tests/src/lib.rs +++ b/testing/ef_tests/src/lib.rs @@ -1,5 +1,4 @@ pub use case_result::CaseResult; -#[cfg(feature = "withdrawals-processing")] pub use cases::WithdrawalsPayload; pub use cases::{ Case, EffectiveBalanceUpdates, Eth1DataReset, HistoricalRootsUpdate, InactivityUpdates, diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index 66c4f83ec..f84be64da 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -82,14 +82,12 @@ fn operations_execution_payload_blinded() { OperationsHandler::>::default().run(); } -#[cfg(feature = "withdrawals-processing")] #[test] fn operations_withdrawals() { OperationsHandler::>::default().run(); OperationsHandler::>::default().run(); } -#[cfg(feature = "withdrawals-processing")] #[test] fn operations_bls_to_execution_change() { OperationsHandler::::default().run();