diff --git a/Cargo.lock b/Cargo.lock index 923d013ab..63f346942 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -401,6 +401,7 @@ dependencies = [ "eth1", "eth2", "eth2_hashing", + "eth2_network_config", "eth2_ssz", "eth2_ssz_derive", "eth2_ssz_types", diff --git a/beacon_node/beacon_chain/Cargo.toml b/beacon_node/beacon_chain/Cargo.toml index 0ab489c89..6f519dc4c 100644 --- a/beacon_node/beacon_chain/Cargo.toml +++ b/beacon_node/beacon_chain/Cargo.toml @@ -22,6 +22,8 @@ environment = { path = "../../lighthouse/environment" } serde_json = "1.0.58" [dependencies] +serde_json = "1.0.58" +eth2_network_config = { path = "../../common/eth2_network_config"} merkle_proof = { path = "../../consensus/merkle_proof" } store = { path = "../store" } parking_lot = "0.12.0" diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index c4bb9f3d8..9114cf105 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -23,6 +23,7 @@ use fork_choice::CountUnrealized; use futures::channel::mpsc::Receiver; pub use genesis::{interop_genesis_state, DEFAULT_ETH1_BLOCK_HASH}; use int_to_bytes::int_to_bytes32; +use kzg::TrustedSetup; use merkle_proof::MerkleTree; use parking_lot::Mutex; use parking_lot::RwLockWriteGuard; @@ -493,6 +494,10 @@ where let validator_keypairs = self .validator_keypairs .expect("cannot build without validator keypairs"); + let trusted_setup: TrustedSetup = + serde_json::from_reader(eth2_network_config::TRUSTED_SETUP) + .map_err(|e| format!("Unable to read trusted setup file: {}", e)) + .unwrap(); let mut builder = BeaconChainBuilder::new(self.eth_spec_instance) .logger(log.clone()) @@ -509,7 +514,8 @@ where log.clone(), 5, ))) - .monitor_validators(true, vec![], log); + .monitor_validators(true, vec![], log) + .trusted_setup(trusted_setup); builder = if let Some(mutator) = self.initial_mutator { mutator(builder) diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index 38cef4332..63c259c39 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -889,11 +889,11 @@ impl HttpJsonRpc { pub async fn supported_apis_v1(&self) -> Result { Ok(SupportedApis { new_payload_v1: true, - new_payload_v2: cfg!(any(feature = "withdrawals-processing", test)), + new_payload_v2: cfg!(test), forkchoice_updated_v1: true, - forkchoice_updated_v2: cfg!(any(feature = "withdrawals-processing", test)), + forkchoice_updated_v2: cfg!(test), get_payload_v1: true, - get_payload_v2: cfg!(any(feature = "withdrawals-processing", test)), + get_payload_v2: cfg!(test), exchange_transition_configuration_v1: true, }) } diff --git a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs index f01ae00e8..a25b9e5ab 100644 --- a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs +++ b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs @@ -74,7 +74,7 @@ pub async fn handle_rpc( .unwrap()) } } - ENGINE_NEW_PAYLOAD_V1 | ENGINE_NEW_PAYLOAD_V2 => { + ENGINE_NEW_PAYLOAD_V1 | ENGINE_NEW_PAYLOAD_V2 | ENGINE_NEW_PAYLOAD_V3 => { let request = match method { ENGINE_NEW_PAYLOAD_V1 => { JsonExecutionPayload::V1(get_param::>(params, 0)?) @@ -82,7 +82,9 @@ pub async fn handle_rpc( ENGINE_NEW_PAYLOAD_V2 => { JsonExecutionPayload::V2(get_param::>(params, 0)?) } - // TODO(4844) add that here.. + ENGINE_NEW_PAYLOAD_V3 => { + JsonExecutionPayload::V2(get_param::>(params, 0)?) + } _ => unreachable!(), }; @@ -114,7 +116,30 @@ pub async fn handle_rpc( )); } } - // TODO(4844) add 4844 error checking here + ForkName::Eip4844 => { + //FIXME(sean) + if method == ENGINE_NEW_PAYLOAD_V1 { + return Err(format!("{} called after capella fork!", method)); + } + if request.withdrawals().is_err() + || (request.withdrawals().is_ok() + && request.withdrawals().unwrap().is_none()) + { + return Err(format!( + "{} called without `withdrawals` after eip4844 fork!", + method + )); + } + if request.excess_data_gas().is_err() + || (request.excess_data_gas().is_ok() + && request.excess_data_gas().unwrap().is_none()) + { + return Err(format!( + "{} called without `excess_data_gas` after eip4844 fork!", + method + )); + } + } _ => unreachable!(), }; @@ -148,7 +173,7 @@ pub async fn handle_rpc( Ok(serde_json::to_value(JsonPayloadStatusV1::from(response)).unwrap()) } - ENGINE_GET_PAYLOAD_V1 | ENGINE_GET_PAYLOAD_V2 => { + ENGINE_GET_PAYLOAD_V1 | ENGINE_GET_PAYLOAD_V2 | ENGINE_GET_PAYLOAD_V3 => { let request: JsonPayloadIdRequest = get_param(params, 0)?; let id = request.into(); @@ -168,7 +193,17 @@ pub async fn handle_rpc( { return Err(format!("{} called after capella fork!", method)); } - // TODO(4844) add 4844 error checking here + // validate method called correctly according to eip4844 fork time + if ctx + .execution_block_generator + .read() + .get_fork_at_timestamp(response.timestamp()) + == ForkName::Eip4844 + //FIXME(sean) + && method == ENGINE_GET_PAYLOAD_V1 + { + return Err(format!("{} called after capella fork!", method)); + } match method { ENGINE_GET_PAYLOAD_V1 => Ok(serde_json::to_value( @@ -224,7 +259,20 @@ pub async fn handle_rpc( )); } } - // TODO(4844) add 4844 error checking here + ForkName::Eip4844 => { + //FIXME(sean) + if method == ENGINE_FORKCHOICE_UPDATED_V1 { + return Err(format!("{} called after capella fork!", method)); + } + if pa.withdrawals().is_err() + || (pa.withdrawals().is_ok() && pa.withdrawals().unwrap().is_none()) + { + return Err(format!( + "{} called without `withdrawals` after capella fork!", + method + )); + } + } _ => unreachable!(), }; } diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index 48541bf41..01e7614ac 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -477,10 +477,6 @@ pub fn get_expected_withdrawals( let mut validator_index = state.next_withdrawal_validator_index()?; let mut withdrawals = vec![]; - if cfg!(not(feature = "withdrawals-processing")) { - return Ok(withdrawals.into()); - } - let bound = std::cmp::min( state.validators().len() as u64, spec.max_validators_per_withdrawals_sweep, @@ -528,9 +524,6 @@ pub fn process_withdrawals<'payload, T: EthSpec, Payload: AbstractExecPayload 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 eacb7617c..48c524fd4 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -300,9 +300,6 @@ pub fn process_bls_to_execution_changes( 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/testing/ef_tests/src/cases/operations.rs b/testing/ef_tests/src/cases/operations.rs index 09bd79d14..71954405c 100644 --- a/testing/ef_tests/src/cases/operations.rs +++ b/testing/ef_tests/src/cases/operations.rs @@ -344,10 +344,6 @@ impl Operation for WithdrawalsPayload { } fn is_enabled_for_fork(fork_name: ForkName) -> bool { - if fork_name == ForkName::Capella && !cfg!(feature = "withdrawals-processing") { - return false; - } - fork_name != ForkName::Base && fork_name != ForkName::Altair && fork_name != ForkName::Merge } @@ -366,12 +362,7 @@ impl Operation for WithdrawalsPayload { spec: &ChainSpec, _: &Operations, ) -> Result<(), BlockProcessingError> { - //FIXME(sean) remove this once the spec tests sort this out - if matches!(state, BeaconState::Eip4844(_)) { - Ok(()) - } else { - process_withdrawals::<_, FullPayload<_>>(state, self.payload.to_ref(), spec) - } + process_withdrawals::<_, FullPayload<_>>(state, self.payload.to_ref(), spec) } } @@ -385,9 +376,6 @@ impl Operation for SignedBlsToExecutionChange { } fn is_enabled_for_fork(fork_name: ForkName) -> bool { - if fork_name == ForkName::Capella && !cfg!(feature = "withdrawals-processing") { - return false; - } fork_name != ForkName::Base && fork_name != ForkName::Altair && fork_name != ForkName::Merge } @@ -401,12 +389,7 @@ impl Operation for SignedBlsToExecutionChange { spec: &ChainSpec, _extra: &Operations, ) -> Result<(), BlockProcessingError> { - //FIXME(sean) remove this once the spec tests sort this out - if matches!(state, BeaconState::Eip4844(_)) { - Ok(()) - } else { - process_bls_to_execution_changes(state, &[self.clone()], VerifySignatures::True, spec) - } + process_bls_to_execution_changes(state, &[self.clone()], VerifySignatures::True, spec) } } diff --git a/testing/ef_tests/src/cases/sanity_blocks.rs b/testing/ef_tests/src/cases/sanity_blocks.rs index d8d128e4d..8a7578972 100644 --- a/testing/ef_tests/src/cases/sanity_blocks.rs +++ b/testing/ef_tests/src/cases/sanity_blocks.rs @@ -60,14 +60,6 @@ impl Case for SanityBlocks { } fn result(&self, _case_index: usize, fork_name: ForkName) -> Result<(), Error> { - if cfg!(feature = "withdrawals-processing") && fork_name == ForkName::Eip4844 { - return Ok(()); - } - - if !cfg!(feature = "withdrawals-processing") && fork_name == ForkName::Capella { - return Ok(()); - } - self.metadata.bls_setting.unwrap_or_default().check()?; let mut bulk_state = self.pre.clone();