From 7aa52a4141c14acf3cfdfa8877f3bbff9d4c14ab Mon Sep 17 00:00:00 2001 From: realbigsean Date: Wed, 23 Nov 2022 11:27:37 -0500 Subject: [PATCH] ef-test fixes --- Cargo.lock | 1 + beacon_node/beacon_chain/src/beacon_chain.rs | 123 ++++++++++--------- consensus/types/src/eth_spec.rs | 2 +- testing/ef_tests/Cargo.toml | 2 + testing/ef_tests/Makefile | 2 +- testing/ef_tests/src/cases/operations.rs | 9 +- 6 files changed, 73 insertions(+), 66 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 57c8b1591..81363c7e2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -403,6 +403,7 @@ dependencies = [ "hex", "int_to_bytes", "itertools", + "kzg", "lazy_static", "lighthouse_metrics", "logging", diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 40f5e990e..cfb4eafd5 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -3706,10 +3706,8 @@ impl BeaconChain { bls_to_execution_changes, } = partial_beacon_block; - let (payload, kzg_commitments_opt, blobs) = block_contents.deconstruct(); - - let inner_block = match &state { - BeaconState::Base(_) => BeaconBlock::Base(BeaconBlockBase { + let (inner_block, blobs_opt) = match &state { + BeaconState::Base(_) => (BeaconBlock::Base(BeaconBlockBase { slot, proposer_index, parent_root, @@ -3725,8 +3723,8 @@ impl BeaconChain { voluntary_exits: voluntary_exits.into(), _phantom: PhantomData, }, - }), - BeaconState::Altair(_) => BeaconBlock::Altair(BeaconBlockAltair { + }), None), + BeaconState::Altair(_) => (BeaconBlock::Altair(BeaconBlockAltair { slot, proposer_index, parent_root, @@ -3744,57 +3742,62 @@ impl BeaconChain { .ok_or(BlockProductionError::MissingSyncAggregate)?, _phantom: PhantomData, }, - }), - BeaconState::Merge(_) => BeaconBlock::Merge(BeaconBlockMerge { - slot, - proposer_index, - parent_root, - state_root: Hash256::zero(), - body: BeaconBlockBodyMerge { - randao_reveal, - eth1_data, - graffiti, - proposer_slashings: proposer_slashings.into(), - attester_slashings: attester_slashings.into(), - attestations: attestations.into(), - deposits: deposits.into(), - voluntary_exits: voluntary_exits.into(), - sync_aggregate: sync_aggregate - .ok_or(BlockProductionError::MissingSyncAggregate)?, - execution_payload: payload - .ok_or(BlockProductionError::MissingExecutionPayload)? - .try_into() - .map_err(|_| BlockProductionError::InvalidPayloadFork)?, - }, - }), - BeaconState::Capella(_) => BeaconBlock::Capella(BeaconBlockCapella { - slot, - proposer_index, - parent_root, - state_root: Hash256::zero(), - body: BeaconBlockBodyCapella { - randao_reveal, - eth1_data, - graffiti, - proposer_slashings: proposer_slashings.into(), - attester_slashings: attester_slashings.into(), - attestations: attestations.into(), - deposits: deposits.into(), - voluntary_exits: voluntary_exits.into(), - sync_aggregate: sync_aggregate - .ok_or(BlockProductionError::MissingSyncAggregate)?, - execution_payload: payload - .ok_or(BlockProductionError::MissingExecutionPayload)? - .try_into() - .map_err(|_| BlockProductionError::InvalidPayloadFork)?, - #[cfg(feature = "withdrawals")] - bls_to_execution_changes: bls_to_execution_changes.into(), - }, - }), + }), None), + BeaconState::Merge(_) => { + let (payload, _, _) = block_contents.ok_or(BlockProductionError::MissingExecutionPayload)?.deconstruct(); + (BeaconBlock::Merge(BeaconBlockMerge { + slot, + proposer_index, + parent_root, + state_root: Hash256::zero(), + body: BeaconBlockBodyMerge { + randao_reveal, + eth1_data, + graffiti, + proposer_slashings: proposer_slashings.into(), + attester_slashings: attester_slashings.into(), + attestations: attestations.into(), + deposits: deposits.into(), + voluntary_exits: voluntary_exits.into(), + sync_aggregate: sync_aggregate + .ok_or(BlockProductionError::MissingSyncAggregate)?, + execution_payload: payload + .try_into() + .map_err(|_| BlockProductionError::InvalidPayloadFork)?, + }, + }), None) + }, + BeaconState::Capella(_) => { + let (payload, _, _) = block_contents.ok_or(BlockProductionError::MissingExecutionPayload)?.deconstruct(); + + (BeaconBlock::Capella(BeaconBlockCapella { + slot, + proposer_index, + parent_root, + state_root: Hash256::zero(), + body: BeaconBlockBodyCapella { + randao_reveal, + eth1_data, + graffiti, + proposer_slashings: proposer_slashings.into(), + attester_slashings: attester_slashings.into(), + attestations: attestations.into(), + deposits: deposits.into(), + voluntary_exits: voluntary_exits.into(), + sync_aggregate: sync_aggregate + .ok_or(BlockProductionError::MissingSyncAggregate)?, + execution_payload: payload + .try_into() + .map_err(|_| BlockProductionError::InvalidPayloadFork)?, + #[cfg(feature = "withdrawals")] + bls_to_execution_changes: bls_to_execution_changes.into(), + }, + }), None) + }, BeaconState::Eip4844(_) => { - let kzg_commitments = - kzg_commitments_opt.ok_or(BlockProductionError::InvalidPayloadFork)?; - BeaconBlock::Eip4844(BeaconBlockEip4844 { + let (payload, kzg_commitments, blobs) = block_contents.ok_or(BlockProductionError::MissingExecutionPayload)?.deconstruct(); + + (BeaconBlock::Eip4844(BeaconBlockEip4844 { slot, proposer_index, parent_root, @@ -3811,14 +3814,13 @@ impl BeaconChain { sync_aggregate: sync_aggregate .ok_or(BlockProductionError::MissingSyncAggregate)?, execution_payload: payload - .ok_or(BlockProductionError::MissingExecutionPayload)? .try_into() .map_err(|_| BlockProductionError::InvalidPayloadFork)?, #[cfg(feature = "withdrawals")] bls_to_execution_changes: bls_to_execution_changes.into(), - blob_kzg_commitments: VariableList::from(kzg_commitments), + blob_kzg_commitments: VariableList::from(kzg_commitments.ok_or(BlockProductionError::InvalidPayloadFork)?), }, - }) + }), blobs) } }; @@ -3873,7 +3875,8 @@ impl BeaconChain { //FIXME(sean) // - generate kzg proof // - validate blobs then cache them - if let Some(blobs) = blobs { + // - add a new timer for processing here + if let Some(blobs) = blobs_opt { let beacon_block_root = block.canonical_root(); let blobs_sidecar = BlobsSidecar { beacon_block_slot: slot, diff --git a/consensus/types/src/eth_spec.rs b/consensus/types/src/eth_spec.rs index fd1064a34..823380a78 100644 --- a/consensus/types/src/eth_spec.rs +++ b/consensus/types/src/eth_spec.rs @@ -338,6 +338,7 @@ impl EthSpec for MinimalEthSpec { type MaxPendingAttestations = U1024; // 128 max attestations * 8 slots per epoch type SlotsPerEth1VotingPeriod = U32; // 4 epochs * 8 slots per epoch type MaxWithdrawalsPerPayload = U4; + type FieldElementsPerBlob = U4; //FIXME(sean) this is spec'd out currently but will likely change params_from_eth_spec!(MainnetEthSpec { JustificationBitsLength, @@ -360,7 +361,6 @@ impl EthSpec for MinimalEthSpec { MaxExtraDataBytes, MaxBlsToExecutionChanges, MaxBlobsPerBlock, - FieldElementsPerBlob, BytesPerFieldElement, BytesPerBlob }); diff --git a/testing/ef_tests/Cargo.toml b/testing/ef_tests/Cargo.toml index 1f9ed4da3..1b42b42ff 100644 --- a/testing/ef_tests/Cargo.toml +++ b/testing/ef_tests/Cargo.toml @@ -9,6 +9,8 @@ edition = "2021" ef_tests = [] milagro = ["bls/milagro"] fake_crypto = ["bls/fake_crypto"] +withdrawals = ["state_processing/withdrawals", "store/withdrawals", "beacon_chain/withdrawals", "types/withdrawals", "execution_layer/withdrawals"] +withdrawals-processing = ["state_processing/withdrawals-processing", "store/withdrawals-processing", "beacon_chain/withdrawals-processing", "execution_layer/withdrawals-processing"] [dependencies] bls = { path = "../../crypto/bls", default-features = false } diff --git a/testing/ef_tests/Makefile b/testing/ef_tests/Makefile index 5dd22de8d..10230ccf9 100644 --- a/testing/ef_tests/Makefile +++ b/testing/ef_tests/Makefile @@ -1,4 +1,4 @@ -TESTS_TAG := f5c7cf78 +TESTS_TAG := v1.3.0-alpha.1-hotfix TESTS = general minimal mainnet TARBALLS = $(patsubst %,%-$(TESTS_TAG).tar.gz,$(TESTS)) diff --git a/testing/ef_tests/src/cases/operations.rs b/testing/ef_tests/src/cases/operations.rs index 9e3562bc7..027d7bb03 100644 --- a/testing/ef_tests/src/cases/operations.rs +++ b/testing/ef_tests/src/cases/operations.rs @@ -4,9 +4,9 @@ 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(all(feature = "withdrawals", feature = "withdrawals-processing"))] +// #[cfg(all(feature = "withdrawals", feature = "withdrawals-processing"))] use state_processing::per_block_processing::process_operations::{ - process_bls_to_execution_changes, process_bls_to_execution_changes, + process_bls_to_execution_changes, }; use state_processing::{ per_block_processing::{ @@ -22,6 +22,7 @@ use state_processing::{ }; use std::fmt::Debug; use std::path::Path; +use state_processing::per_block_processing::process_withdrawals; use types::{ Attestation, AttesterSlashing, BeaconBlock, BeaconState, BlindedPayload, ChainSpec, Deposit, EthSpec, ExecutionPayload, ForkName, FullPayload, ProposerSlashing, SignedBlsToExecutionChange, @@ -344,7 +345,7 @@ impl Operation for BlindedPayload { } } -#[cfg(all(feature = "withdrawals", feature = "withdrawals-processing"))] +// #[cfg(all(feature = "withdrawals", feature = "withdrawals-processing"))] impl Operation for WithdrawalsPayload { fn handler_name() -> String { "withdrawals".into() @@ -377,7 +378,7 @@ impl Operation for WithdrawalsPayload { } } -#[cfg(all(feature = "withdrawals", feature = "withdrawals-processing"))] +// #[cfg(all(feature = "withdrawals", feature = "withdrawals-processing"))] impl Operation for SignedBlsToExecutionChange { fn handler_name() -> String { "bls_to_execution_change".into()