diff --git a/beacon_node/beacon_chain/src/envelope_verification.rs b/beacon_node/beacon_chain/src/envelope_verification.rs index ad8a6ea539..222823285a 100644 --- a/beacon_node/beacon_chain/src/envelope_verification.rs +++ b/beacon_node/beacon_chain/src/envelope_verification.rs @@ -216,9 +216,9 @@ impl GossipVerifiedEnvelope { signed_envelope: Arc>, chain: &BeaconChain, ) -> Result { - let envelope = signed_envelope.message(); - let payload = envelope.payload(); - let beacon_block_root = envelope.beacon_block_root(); + let envelope = &signed_envelope.message; + let payload = &envelope.payload; + let beacon_block_root = envelope.beacon_block_root; // check that we've seen the parent block of this envelope and that it passes validation // TODO(EIP-7732): this check would fail if the block didn't pass validation right? @@ -255,26 +255,26 @@ impl GossipVerifiedEnvelope { // should these kinds of checks be included for envelopes as well? // check that the slot of the envelope matches the slot of the parent block - if envelope.slot() != parent_block.slot() { + if envelope.slot != parent_block.slot() { return Err(EnvelopeError::SlotMismatch { parent_block: parent_block.slot(), - envelope: envelope.slot(), + envelope: envelope.slot, }); } // builder index matches committed bid - if envelope.builder_index() != execution_bid.builder_index { + if envelope.builder_index != execution_bid.builder_index { return Err(EnvelopeError::BuilderIndexMismatch { committed_bid: execution_bid.builder_index, - envelope: envelope.builder_index(), + envelope: envelope.builder_index, }); } // the block hash should match the block hash of the execution bid - if payload.block_hash() != execution_bid.block_hash { + if payload.block_hash != execution_bid.block_hash { return Err(EnvelopeError::BlockHashMismatch { committed_bid: execution_bid.block_hash, - envelope: payload.block_hash(), + envelope: payload.block_hash, }); } @@ -283,7 +283,7 @@ impl GossipVerifiedEnvelope { // and so on. // get the fork from the cache so we can verify the signature - let block_slot = envelope.slot(); + let block_slot = envelope.slot; let block_epoch = block_slot.epoch(T::EthSpec::slots_per_epoch()); let proposer_shuffling_decision_block = parent_proto_block.proposer_shuffling_root_for_child_block(block_epoch, &chain.spec); @@ -311,9 +311,9 @@ impl GossipVerifiedEnvelope { let signature_is_valid = { let pubkey_cache = chain.validator_pubkey_cache.read(); let builder_pubkey = pubkey_cache - .get(envelope.builder_index() as usize) + .get(envelope.builder_index as usize) .ok_or_else(|| EnvelopeError::UnknownValidator { - builder_index: envelope.builder_index(), + builder_index: envelope.builder_index, })?; signed_envelope.verify_signature( builder_pubkey, @@ -360,13 +360,13 @@ impl IntoExecutionPendingEnvelope for GossipVerifiedEnve notify_execution_layer: NotifyExecutionLayer, ) -> Result, EnvelopeError> { let signed_envelope = self.signed_envelope; - let envelope = signed_envelope.message(); - let payload = &envelope.payload(); + let envelope = &signed_envelope.message; + let payload = &envelope.payload; // Verify the execution payload is valid let payload_notifier = PayloadNotifier::from_envelope(chain.clone(), envelope, notify_execution_layer)?; - let block_root = envelope.beacon_block_root(); + let block_root = envelope.beacon_block_root; let slot = self.parent_block.slot(); let payload_verification_future = async move { @@ -417,7 +417,7 @@ impl IntoExecutionPendingEnvelope for GossipVerifiedEnve Ok(ExecutionPendingEnvelope { signed_envelope: MaybeAvailableEnvelope::AvailabilityPending { - block_hash: payload.block_hash(), + block_hash: payload.block_hash, envelope: signed_envelope, }, import_data: EnvelopeImportData { diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index 7cad2104eb..7c27f418a5 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -110,7 +110,7 @@ impl PayloadNotifier { pub fn from_envelope( _chain: Arc>, - _envelope: ExecutionPayloadEnvelopeRef, + _envelope: &ExecutionPayloadEnvelope, _notify_execution_layer: NotifyExecutionLayer, ) -> Result { todo!( diff --git a/beacon_node/execution_layer/src/engine_api/new_payload_request.rs b/beacon_node/execution_layer/src/engine_api/new_payload_request.rs index a27c732307..ba94296b85 100644 --- a/beacon_node/execution_layer/src/engine_api/new_payload_request.rs +++ b/beacon_node/execution_layer/src/engine_api/new_payload_request.rs @@ -172,7 +172,7 @@ impl<'block, E: EthSpec> NewPayloadRequest<'block, E> { } } -//TODO(EIP7732): Consider implmenting these as methods on the NewPayloadRequest struct +//TODO(EIP7732): Consider implementing these as methods on the NewPayloadRequest struct impl<'a, E: EthSpec> TryFrom> for NewPayloadRequest<'a, E> { type Error = BeaconStateError; diff --git a/consensus/state_processing/src/envelope_processing.rs b/consensus/state_processing/src/envelope_processing.rs index af2b309c1e..9ee2be3f18 100644 --- a/consensus/state_processing/src/envelope_processing.rs +++ b/consensus/state_processing/src/envelope_processing.rs @@ -124,9 +124,9 @@ pub fn envelope_processing( } } - let envelope = signed_envelope.message(); - let payload = envelope.payload(); - let execution_requests = envelope.execution_requests(); + let envelope = &signed_envelope.message; + let payload = &envelope.payload; + let execution_requests = &envelope.execution_requests; // Cache latest block header state root if state.latest_block_header().state_root == Hash256::default() { @@ -138,16 +138,16 @@ pub fn envelope_processing( // Verify consistency with the beacon block envelope_verify!( - envelope.beacon_block_root() == state.latest_block_header().tree_hash_root(), + envelope.beacon_block_root == state.latest_block_header().tree_hash_root(), EnvelopeProcessingError::LatestBlockHeaderMismatch { - envelope_root: envelope.beacon_block_root(), + envelope_root: envelope.beacon_block_root, block_header_root: state.latest_block_header().tree_hash_root(), } ); envelope_verify!( - envelope.slot() == state.slot(), + envelope.slot == state.slot(), EnvelopeProcessingError::SlotMismatch { - envelope_slot: envelope.slot(), + envelope_slot: envelope.slot, parent_state_slot: state.slot(), } ); @@ -155,75 +155,75 @@ pub fn envelope_processing( // Verify consistency with the committed bid let committed_bid = state.latest_execution_payload_bid()?; // builder index match already verified - if committed_bid.blob_kzg_commitments_root != envelope.blob_kzg_commitments().tree_hash_root() { + if committed_bid.blob_kzg_commitments_root != envelope.blob_kzg_commitments.tree_hash_root() { return Err(EnvelopeProcessingError::BlobKzgCommitmentsRootMismatch { committed_bid: committed_bid.blob_kzg_commitments_root, - envelope: envelope.blob_kzg_commitments().tree_hash_root(), + envelope: envelope.blob_kzg_commitments.tree_hash_root(), }); }; // Verify the withdrawals root envelope_verify!( - payload.withdrawals()?.tree_hash_root() == *state.latest_withdrawals_root()?, + payload.withdrawals.tree_hash_root() == *state.latest_withdrawals_root()?, EnvelopeProcessingError::WithdrawalsRootMismatch { state: *state.latest_withdrawals_root()?, - envelope: payload.withdrawals()?.tree_hash_root(), + envelope: payload.withdrawals.tree_hash_root(), } ); // Verify the gas limit envelope_verify!( - payload.gas_limit() == committed_bid.gas_limit, + payload.gas_limit == committed_bid.gas_limit, EnvelopeProcessingError::GasLimitMismatch { committed_bid: committed_bid.gas_limit, - envelope: payload.gas_limit(), + envelope: payload.gas_limit, } ); // Verify the block hash envelope_verify!( - committed_bid.block_hash == payload.block_hash(), + committed_bid.block_hash == payload.block_hash, EnvelopeProcessingError::BlockHashMismatch { committed_bid: committed_bid.block_hash, - envelope: payload.block_hash(), + envelope: payload.block_hash, } ); // Verify consistency of the parent hash with respect to the previous execution payload envelope_verify!( - payload.parent_hash() == *state.latest_block_hash()?, + payload.parent_hash == *state.latest_block_hash()?, EnvelopeProcessingError::ParentHashMismatch { state: *state.latest_block_hash()?, - envelope: payload.parent_hash(), + envelope: payload.parent_hash, } ); // Verify prev_randao envelope_verify!( - payload.prev_randao() == *state.get_randao_mix(state.current_epoch())?, + payload.prev_randao == *state.get_randao_mix(state.current_epoch())?, EnvelopeProcessingError::PrevRandaoMismatch { state: *state.get_randao_mix(state.current_epoch())?, - envelope: payload.prev_randao(), + envelope: payload.prev_randao, } ); // Verify the timestamp let state_timestamp = compute_timestamp_at_slot(state, state.slot(), spec)?; envelope_verify!( - payload.timestamp() == state_timestamp, + payload.timestamp == state_timestamp, EnvelopeProcessingError::TimestampMismatch { state: state_timestamp, - envelope: payload.timestamp(), + envelope: payload.timestamp, } ); // Verify the commitments are under limit let max_blobs = spec.max_blobs_per_block(state.current_epoch()) as usize; envelope_verify!( - envelope.blob_kzg_commitments().len() <= max_blobs, + envelope.blob_kzg_commitments.len() <= max_blobs, EnvelopeProcessingError::BlobLimitExceeded { max: max_blobs, - envelope: envelope.blob_kzg_commitments().len(), + envelope: envelope.blob_kzg_commitments.len(), } ); @@ -269,14 +269,14 @@ pub fn envelope_processing( .execution_payload_availability_mut()? .set(availability_index, true) .map_err(EnvelopeProcessingError::BitFieldError)?; - *state.latest_block_hash_mut()? = payload.block_hash(); + *state.latest_block_hash_mut()? = payload.block_hash; // verify the state root envelope_verify!( - envelope.state_root() == state.canonical_root()?, + envelope.state_root == state.canonical_root()?, EnvelopeProcessingError::InvalidStateRoot { state: state.canonical_root()?, - envelope: envelope.state_root(), + envelope: envelope.state_root, } ); diff --git a/consensus/state_processing/src/per_block_processing/signature_sets.rs b/consensus/state_processing/src/per_block_processing/signature_sets.rs index d9efcad6fb..6d071ac894 100644 --- a/consensus/state_processing/src/per_block_processing/signature_sets.rs +++ b/consensus/state_processing/src/per_block_processing/signature_sets.rs @@ -378,13 +378,13 @@ where &state.fork(), state.genesis_validators_root(), ); - let message = signed_envelope.message().signing_root(domain); - let pubkey = get_pubkey(signed_envelope.message().builder_index() as usize).ok_or( - Error::ValidatorUnknown(signed_envelope.message().builder_index()), + let message = signed_envelope.message.signing_root(domain); + let pubkey = get_pubkey(signed_envelope.message.builder_index as usize).ok_or( + Error::ValidatorUnknown(signed_envelope.message.builder_index), )?; Ok(SignatureSet::single_pubkey( - signed_envelope.signature(), + &signed_envelope.signature, pubkey, message, )) diff --git a/consensus/types/src/execution_payload_envelope.rs b/consensus/types/src/execution_payload_envelope.rs index f58b2e9696..e2cbdd3346 100644 --- a/consensus/types/src/execution_payload_envelope.rs +++ b/consensus/types/src/execution_payload_envelope.rs @@ -2,107 +2,33 @@ use crate::test_utils::TestRandom; use crate::*; use beacon_block_body::KzgCommitments; use educe::Educe; -use serde::de::{Deserializer, Error as _}; use serde::{Deserialize, Serialize}; use ssz_derive::{Decode, Encode}; -use superstruct::superstruct; use test_random_derive::TestRandom; use tree_hash_derive::TreeHash; -// in all likelihood, this will be superstructed so might as well start early eh? -#[superstruct( - variants(Gloas, NextFork), - variant_attributes( - derive( - Debug, - Clone, - Serialize, - Deserialize, - Encode, - Decode, - TreeHash, - TestRandom, - Educe - ), - cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary)), - educe(PartialEq, Hash(bound(E: EthSpec))), - serde(bound = "E: EthSpec", deny_unknown_fields), - cfg_attr(feature = "arbitrary", arbitrary(bound = "E: EthSpec")) - ), - ref_attributes( - derive(Debug, PartialEq, TreeHash), - tree_hash(enum_behaviour = "transparent") - ), - cast_error(ty = "Error", expr = "BeaconStateError::IncorrectStateVariant"), - partial_getter_error(ty = "Error", expr = "BeaconStateError::IncorrectStateVariant") -)] -#[derive(Debug, Clone, Serialize, Encode, Deserialize, TreeHash, Educe)] +#[derive(Debug, Clone, Serialize, Encode, Decode, Deserialize, TestRandom, TreeHash, Educe)] #[educe(PartialEq, Hash(bound(E: EthSpec)))] -#[serde(bound = "E: EthSpec", untagged)] -#[ssz(enum_behaviour = "transparent")] -#[tree_hash(enum_behaviour = "transparent")] +#[context_deserialize(ForkName)] +#[serde(bound = "E: EthSpec")] pub struct ExecutionPayloadEnvelope { - #[superstruct(only(Gloas), partial_getter(rename = "payload_gloas"))] - pub payload: ExecutionPayloadGloas, - #[superstruct(only(NextFork), partial_getter(rename = "payload_next_fork"))] pub payload: ExecutionPayloadGloas, pub execution_requests: ExecutionRequests, #[serde(with = "serde_utils::quoted_u64")] - #[superstruct(getter(copy))] pub builder_index: u64, - #[superstruct(getter(copy))] pub beacon_block_root: Hash256, - #[superstruct(getter(copy))] pub slot: Slot, pub blob_kzg_commitments: KzgCommitments, - #[superstruct(getter(copy))] pub state_root: Hash256, } impl SignedRoot for ExecutionPayloadEnvelope {} -impl<'a, E: EthSpec> SignedRoot for ExecutionPayloadEnvelopeRef<'a, E> {} -impl<'a, E: EthSpec> ExecutionPayloadEnvelopeRef<'a, E> { - pub fn payload(&self) -> ExecutionPayloadRef<'a, E> { - match self { - Self::Gloas(envelope) => ExecutionPayloadRef::Gloas(&envelope.payload), - Self::NextFork(envelope) => ExecutionPayloadRef::Gloas(&envelope.payload), - } - } - - pub fn block_hash(&self) -> ExecutionBlockHash { - self.payload().block_hash() - } -} - -impl<'de, E: EthSpec> ContextDeserialize<'de, ForkName> for ExecutionPayloadEnvelope { - fn context_deserialize(deserializer: D, context: ForkName) -> Result - where - D: Deserializer<'de>, - { - let value: Self = serde::Deserialize::deserialize(deserializer)?; - - match (context, &value) { - (ForkName::Gloas, Self::Gloas { .. }) => Ok(value), - _ => Err(D::Error::custom(format!( - "ExecutionPayloadEnvelope does not support fork {context:?}" - ))), - } - } -} #[cfg(test)] mod tests { use super::*; use crate::MainnetEthSpec; - mod gloas { - use super::*; - ssz_and_tree_hash_tests!(ExecutionPayloadEnvelopeGloas); - } - - mod next_fork { - use super::*; - ssz_and_tree_hash_tests!(ExecutionPayloadEnvelopeNextFork); - } + ssz_and_tree_hash_tests!(ExecutionPayloadEnvelope); } diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index a5b8e6ca0e..df7e84a0dd 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -187,10 +187,7 @@ pub use crate::execution_payload::{ Transaction, Transactions, Withdrawals, }; pub use crate::execution_payload_bid::ExecutionPayloadBid; -pub use crate::execution_payload_envelope::{ - ExecutionPayloadEnvelope, ExecutionPayloadEnvelopeGloas, ExecutionPayloadEnvelopeNextFork, - ExecutionPayloadEnvelopeRef, -}; +pub use crate::execution_payload_envelope::ExecutionPayloadEnvelope; pub use crate::execution_payload_header::{ ExecutionPayloadHeader, ExecutionPayloadHeaderBellatrix, ExecutionPayloadHeaderCapella, ExecutionPayloadHeaderDeneb, ExecutionPayloadHeaderElectra, ExecutionPayloadHeaderFulu, diff --git a/consensus/types/src/signed_execution_payload_envelope.rs b/consensus/types/src/signed_execution_payload_envelope.rs index 08afc8bb14..e3cd26219c 100644 --- a/consensus/types/src/signed_execution_payload_envelope.rs +++ b/consensus/types/src/signed_execution_payload_envelope.rs @@ -1,82 +1,22 @@ use crate::test_utils::TestRandom; use crate::*; use educe::Educe; -use serde::de::{Deserializer, Error as _}; use serde::{Deserialize, Serialize}; use ssz_derive::{Decode, Encode}; -use superstruct::superstruct; use test_random_derive::TestRandom; use tree_hash_derive::TreeHash; -#[superstruct( - variants(Gloas, NextFork), - variant_attributes( - derive( - Debug, - Clone, - Serialize, - Deserialize, - Encode, - Decode, - TreeHash, - TestRandom, - Educe - ), - cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary)), - educe(PartialEq, Hash(bound(E: EthSpec))), - serde(bound = "E: EthSpec", deny_unknown_fields), - cfg_attr(feature = "arbitrary", arbitrary(bound = "E: EthSpec")) - ), - ref_attributes( - derive(Debug, PartialEq, TreeHash), - tree_hash(enum_behaviour = "transparent") - ), - cast_error(ty = "Error", expr = "BeaconStateError::IncorrectStateVariant"), - partial_getter_error(ty = "Error", expr = "BeaconStateError::IncorrectStateVariant") -)] -#[derive(Debug, Clone, Serialize, Encode, Deserialize, TreeHash, Educe)] +#[derive(Debug, Clone, Serialize, Encode, Decode, Deserialize, TestRandom, TreeHash, Educe)] #[educe(PartialEq, Hash(bound(E: EthSpec)))] -#[serde(bound = "E: EthSpec", untagged)] -#[ssz(enum_behaviour = "transparent")] -#[tree_hash(enum_behaviour = "transparent")] +#[serde(bound = "E: EthSpec")] pub struct SignedExecutionPayloadEnvelope { - #[superstruct(only(Gloas), partial_getter(rename = "message_gloas"))] - pub message: ExecutionPayloadEnvelopeGloas, - #[superstruct(only(NextFork), partial_getter(rename = "message_next_fork"))] - pub message: crate::execution_payload_envelope::ExecutionPayloadEnvelopeNextFork, + pub message: ExecutionPayloadEnvelope, pub signature: Signature, } impl SignedExecutionPayloadEnvelope { - /// Create a new `SignedExecutionPayloadEnvelope` from an `ExecutionPayloadEnvelope` and `Signature`. - pub fn from_envelope(envelope: ExecutionPayloadEnvelope, signature: Signature) -> Self { - match envelope { - ExecutionPayloadEnvelope::Gloas(message) => SignedExecutionPayloadEnvelope::Gloas( - signed_execution_payload_envelope::SignedExecutionPayloadEnvelopeGloas { - message, - signature, - }, - ), - ExecutionPayloadEnvelope::NextFork(message) => { - SignedExecutionPayloadEnvelope::NextFork( - signed_execution_payload_envelope::SignedExecutionPayloadEnvelopeNextFork { - message, - signature, - }, - ) - } - } - } - - pub fn message(&self) -> ExecutionPayloadEnvelopeRef<'_, E> { - match self { - Self::Gloas(signed) => ExecutionPayloadEnvelopeRef::Gloas(&signed.message), - Self::NextFork(signed) => ExecutionPayloadEnvelopeRef::NextFork(&signed.message), - } - } - pub fn slot(&self) -> Slot { - self.message().slot() + self.message.slot } pub fn epoch(&self) -> Epoch { @@ -84,11 +24,11 @@ impl SignedExecutionPayloadEnvelope { } pub fn beacon_block_root(&self) -> Hash256 { - self.message().beacon_block_root() + self.message.beacon_block_root } pub fn block_hash(&self) -> ExecutionBlockHash { - self.message().block_hash() + self.message.payload.block_hash } /// Verify `self.signature`. @@ -109,17 +49,17 @@ impl SignedExecutionPayloadEnvelope { ); let pubkey = parent_state .validators() - .get(self.message().builder_index() as usize) + .get(self.message.builder_index as usize) .and_then(|v| { let pk: Option = v.pubkey.decompress().ok(); pk }) .ok_or_else(|| { - BeaconStateError::UnknownValidator(self.message().builder_index() as usize) + BeaconStateError::UnknownValidator(self.message.builder_index as usize) })?; - let message = self.message().signing_root(domain); + let message = self.message.signing_root(domain); - Ok(self.signature().verify(&pubkey, message)) + Ok(self.signature.verify(&pubkey, message)) } /// Verify `self.signature`. @@ -140,25 +80,9 @@ impl SignedExecutionPayloadEnvelope { genesis_validators_root, ); - let message = self.message().signing_root(domain); + let message = self.message.signing_root(domain); - self.signature().verify(pubkey, message) - } -} - -impl<'de, E: EthSpec> ContextDeserialize<'de, ForkName> for SignedExecutionPayloadEnvelope { - fn context_deserialize(deserializer: D, context: ForkName) -> Result - where - D: Deserializer<'de>, - { - let value: Self = Deserialize::deserialize(deserializer)?; - - match (context, &value) { - (ForkName::Gloas, Self::Gloas { .. }) => Ok(value), - _ => Err(D::Error::custom(format!( - "SignedExecutionPayloadEnvelope does not support fork {context:?}" - ))), - } + self.signature.verify(pubkey, message) } } @@ -167,13 +91,5 @@ mod tests { use super::*; use crate::MainnetEthSpec; - mod gloas { - use super::*; - ssz_and_tree_hash_tests!(SignedExecutionPayloadEnvelopeGloas); - } - - mod next_fork { - use super::*; - ssz_and_tree_hash_tests!(SignedExecutionPayloadEnvelopeNextFork); - } + ssz_and_tree_hash_tests!(SignedExecutionPayloadEnvelope); }