From 330863d970e9ade920267268885522183fd9a255 Mon Sep 17 00:00:00 2001 From: Mark Mackey Date: Mon, 9 Sep 2024 15:25:17 -0500 Subject: [PATCH] Some Block Processing --- beacon_node/store/src/consensus_context.rs | 2 + .../common/get_payload_attesting_indices.rs | 40 ++++++ consensus/state_processing/src/common/mod.rs | 4 + .../state_processing/src/consensus_context.rs | 32 ++++- .../src/per_block_processing.rs | 4 + .../src/per_block_processing/errors.rs | 58 +++++++- .../is_valid_indexed_payload_attestation.rs | 55 +++++++ .../process_operations.rs | 58 +++++++- .../per_block_processing/signature_sets.rs | 38 ++++- .../verify_payload_attestation.rs | 42 ++++++ consensus/types/src/beacon_block.rs | 8 +- consensus/types/src/beacon_state.rs | 38 +++++ .../types/src/indexed_payload_attestation.rs | 2 +- consensus/types/src/lib.rs | 4 +- consensus/types/src/payload_attestation.rs | 131 +---------------- .../types/src/payload_attestation_data.rs | 134 +++++++++++++++++- 16 files changed, 505 insertions(+), 145 deletions(-) create mode 100644 consensus/state_processing/src/common/get_payload_attesting_indices.rs create mode 100644 consensus/state_processing/src/per_block_processing/is_valid_indexed_payload_attestation.rs create mode 100644 consensus/state_processing/src/per_block_processing/verify_payload_attestation.rs diff --git a/beacon_node/store/src/consensus_context.rs b/beacon_node/store/src/consensus_context.rs index 281106d9aa..af800ebdae 100644 --- a/beacon_node/store/src/consensus_context.rs +++ b/beacon_node/store/src/consensus_context.rs @@ -35,6 +35,8 @@ impl OnDiskConsensusContext { proposer_index, current_block_root, indexed_attestations, + indexed_payload_attestations: _, + // TODO(EIP-77342): add indexed_payload_attestations to the on-disk format. } = ctxt; OnDiskConsensusContext { slot, diff --git a/consensus/state_processing/src/common/get_payload_attesting_indices.rs b/consensus/state_processing/src/common/get_payload_attesting_indices.rs new file mode 100644 index 0000000000..499c070a2b --- /dev/null +++ b/consensus/state_processing/src/common/get_payload_attesting_indices.rs @@ -0,0 +1,40 @@ +use crate::per_block_processing::errors::{ + BlockOperationError, PayloadAttestationInvalid as Invalid, +}; +use types::*; + +pub fn get_indexed_payload_attestation( + state: &BeaconState, + slot: Slot, + payload_attestation: &PayloadAttestation, +) -> Result, BlockOperationError> { + let attesting_indices = get_payload_attesting_indices(state, slot, payload_attestation)?; + + Ok(IndexedPayloadAttestation { + attesting_indices: VariableList::new(attesting_indices)?, + data: payload_attestation.data.clone(), + signature: payload_attestation.signature.clone(), + }) +} + +pub fn get_payload_attesting_indices( + state: &BeaconState, + slot: Slot, + payload_attestation: &PayloadAttestation, +) -> Result, BeaconStateError> { + let ptc = state.get_ptc(slot)?; + let bitlist = &payload_attestation.aggregation_bits; + if bitlist.len() != ptc.len() { + return Err(BeaconStateError::InvalidBitfield); + } + + let mut attesting_indices = Vec::::new(); + for (i, index) in ptc.into_iter().enumerate() { + if let Ok(true) = bitlist.get(i) { + attesting_indices.push(*index as u64); + } + } + attesting_indices.sort_unstable(); + + Ok(attesting_indices) +} diff --git a/consensus/state_processing/src/common/mod.rs b/consensus/state_processing/src/common/mod.rs index 0287748fd0..e550a6c48b 100644 --- a/consensus/state_processing/src/common/mod.rs +++ b/consensus/state_processing/src/common/mod.rs @@ -1,6 +1,7 @@ mod deposit_data_tree; mod get_attestation_participation; mod get_attesting_indices; +mod get_payload_attesting_indices; mod initiate_validator_exit; mod slash_validator; @@ -13,6 +14,9 @@ pub use get_attestation_participation::get_attestation_participation_flag_indice pub use get_attesting_indices::{ attesting_indices_base, attesting_indices_electra, get_attesting_indices_from_state, }; +pub use get_payload_attesting_indices::{ + get_indexed_payload_attestation, get_payload_attesting_indices, +}; pub use initiate_validator_exit::initiate_validator_exit; pub use slash_validator::slash_validator; diff --git a/consensus/state_processing/src/consensus_context.rs b/consensus/state_processing/src/consensus_context.rs index 0c176d4ab1..eef9c22e60 100644 --- a/consensus/state_processing/src/consensus_context.rs +++ b/consensus/state_processing/src/consensus_context.rs @@ -1,11 +1,16 @@ -use crate::common::{attesting_indices_base, attesting_indices_electra}; -use crate::per_block_processing::errors::{AttestationInvalid, BlockOperationError}; +use crate::common::{ + attesting_indices_base, attesting_indices_electra, get_indexed_payload_attestation, +}; +use crate::per_block_processing::errors::{ + AttestationInvalid, BlockOperationError, PayloadAttestationInvalid, +}; use crate::EpochCacheError; use std::collections::{hash_map::Entry, HashMap}; use tree_hash::TreeHash; use types::{ AbstractExecPayload, AttestationRef, BeaconState, BeaconStateError, ChainSpec, Epoch, EthSpec, - Hash256, IndexedAttestation, IndexedAttestationRef, SignedBeaconBlock, Slot, + Hash256, IndexedAttestation, IndexedAttestationRef, IndexedPayloadAttestation, + PayloadAttestation, SignedBeaconBlock, Slot, }; #[derive(Debug, PartialEq, Clone)] @@ -22,6 +27,8 @@ pub struct ConsensusContext { pub current_block_root: Option, /// Cache of indexed attestations constructed during block processing. pub indexed_attestations: HashMap>, + /// Cache of indexed payload attestations constructed during block processing. + pub indexed_payload_attestations: HashMap>, } #[derive(Debug, PartialEq, Clone)] @@ -55,6 +62,7 @@ impl ConsensusContext { proposer_index: None, current_block_root: None, indexed_attestations: HashMap::new(), + indexed_payload_attestations: HashMap::new(), } } @@ -177,6 +185,24 @@ impl ConsensusContext { .map(|indexed_attestation| (*indexed_attestation).to_ref()) } + pub fn get_indexed_payload_attestation<'a>( + &'a mut self, + state: &BeaconState, + slot: Slot, + payload_attestation: &'a PayloadAttestation, + ) -> Result<&'a IndexedPayloadAttestation, BlockOperationError> + { + let key = payload_attestation.tree_hash_root(); + match self.indexed_payload_attestations.entry(key) { + Entry::Occupied(occupied) => Ok(occupied.into_mut()), + Entry::Vacant(vacant) => { + let indexed_payload_attestation = + get_indexed_payload_attestation(state, slot, payload_attestation)?; + Ok(vacant.insert(indexed_payload_attestation)) + } + } + } + pub fn num_cached_indexed_attestations(&self) -> usize { self.indexed_attestations.len() } diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index 33ee8c2099..9d2e5c83dd 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -14,6 +14,7 @@ pub use self::verify_proposer_slashing::verify_proposer_slashing; pub use altair::sync_committee::process_sync_aggregate; pub use block_signature_verifier::{BlockSignatureVerifier, ParallelSignatureSets}; pub use is_valid_indexed_attestation::is_valid_indexed_attestation; +pub use is_valid_indexed_payload_attestation::is_valid_indexed_payload_attestation; pub use process_operations::process_operations; pub use verify_attestation::{ verify_attestation_for_block_inclusion, verify_attestation_for_state, @@ -23,12 +24,14 @@ pub use verify_deposit::{ get_existing_validator_index, is_valid_deposit_signature, verify_deposit_merkle_proof, }; pub use verify_exit::verify_exit; +pub use verify_payload_attestation::verify_payload_attestation; pub mod altair; pub mod block_signature_verifier; pub mod deneb; pub mod errors; mod is_valid_indexed_attestation; +mod is_valid_indexed_payload_attestation; pub mod process_operations; pub mod signature_sets; pub mod tests; @@ -37,6 +40,7 @@ mod verify_attester_slashing; mod verify_bls_to_execution_change; mod verify_deposit; mod verify_exit; +mod verify_payload_attestation; mod verify_proposer_slashing; use crate::common::decrease_balance; diff --git a/consensus/state_processing/src/per_block_processing/errors.rs b/consensus/state_processing/src/per_block_processing/errors.rs index fdeec6f08c..0ac5528f3c 100644 --- a/consensus/state_processing/src/per_block_processing/errors.rs +++ b/consensus/state_processing/src/per_block_processing/errors.rs @@ -41,6 +41,10 @@ pub enum BlockProcessingError { index: usize, reason: AttestationInvalid, }, + PayloadAttestationInvalid { + index: usize, + reason: PayloadAttestationInvalid, + }, DepositInvalid { index: usize, reason: DepositInvalid, @@ -197,7 +201,8 @@ impl_into_block_processing_error_with_index!( AttestationInvalid, DepositInvalid, ExitInvalid, - BlsExecutionChangeInvalid + BlsExecutionChangeInvalid, + PayloadAttestationInvalid ); pub type HeaderValidationError = BlockOperationError; @@ -390,6 +395,57 @@ pub enum IndexedAttestationInvalid { SignatureSetError(SignatureSetError), } +#[derive(Debug, PartialEq, Clone)] +pub enum PayloadAttestationInvalid { + /// Block root does not match the parent beacon block root. + BlockRootMismatch { + expected: Hash256, + found: Hash256, + }, + /// The attestation slot is not the previous slot. + SlotMismatch { + expected: Slot, + found: Slot, + }, + BadIndexedPayloadAttestation(IndexedPayloadAttestationInvalid), +} + +impl From> + for BlockOperationError +{ + fn from(e: BlockOperationError) -> Self { + match e { + BlockOperationError::Invalid(e) => BlockOperationError::invalid( + PayloadAttestationInvalid::BadIndexedPayloadAttestation(e), + ), + BlockOperationError::BeaconStateError(e) => BlockOperationError::BeaconStateError(e), + BlockOperationError::SignatureSetError(e) => BlockOperationError::SignatureSetError(e), + BlockOperationError::SszTypesError(e) => BlockOperationError::SszTypesError(e), + BlockOperationError::ConsensusContext(e) => BlockOperationError::ConsensusContext(e), + BlockOperationError::ArithError(e) => BlockOperationError::ArithError(e), + } + } +} + +#[derive(Debug, PartialEq, Clone)] +pub enum IndexedPayloadAttestationInvalid { + /// The number of indices is 0. + IndicesEmpty, + /// The validator indices were not in increasing order. + /// + /// The error occurred between the given `index` and `index + 1` + BadValidatorIndicesOrdering(usize), + /// The validator index is unknown. One cannot slash one who does not exist. + UnknownValidator(u64), + /// The indexed attestation aggregate signature was not valid. + BadSignature, + /// There was an error whilst attempting to get a set of signatures. The signatures may have + /// been invalid or an internal error occurred. + SignatureSetError(SignatureSetError), + /// Invalid Payload Status + PayloadStatusInvalid, +} + #[derive(Debug, PartialEq, Clone)] pub enum DepositInvalid { /// The signature (proof-of-possession) does not match the given pubkey. diff --git a/consensus/state_processing/src/per_block_processing/is_valid_indexed_payload_attestation.rs b/consensus/state_processing/src/per_block_processing/is_valid_indexed_payload_attestation.rs new file mode 100644 index 0000000000..0d45e194bd --- /dev/null +++ b/consensus/state_processing/src/per_block_processing/is_valid_indexed_payload_attestation.rs @@ -0,0 +1,55 @@ +use super::errors::{BlockOperationError, IndexedPayloadAttestationInvalid as Invalid}; +use super::signature_sets::{get_pubkey_from_state, indexed_payload_attestation_signature_set}; +use crate::VerifySignatures; +use itertools::Itertools; +use types::*; + +fn error(reason: Invalid) -> BlockOperationError { + BlockOperationError::invalid(reason) +} + +pub fn is_valid_indexed_payload_attestation( + state: &BeaconState, + indexed_payload_attestation: &IndexedPayloadAttestation, + verify_signatures: VerifySignatures, + spec: &ChainSpec, +) -> Result<(), BlockOperationError> { + // Verify the data is valid + if indexed_payload_attestation.data.payload_status >= PayloadStatus::PayloadInvalidStatus { + return Err(error(Invalid::PayloadStatusInvalid)); + } + + // Verify indices are sorted and unique + let indices = &indexed_payload_attestation.attesting_indices; + verify!(!indices.is_empty(), Invalid::IndicesEmpty); + let check_sorted = |list: &[u64]| -> Result<(), BlockOperationError> { + list.iter() + .tuple_windows() + .enumerate() + .try_for_each(|(i, (x, y))| { + if x < y { + Ok(()) + } else { + Err(error(Invalid::BadValidatorIndicesOrdering(i))) + } + })?; + Ok(()) + }; + check_sorted(&indices)?; + + if verify_signatures.is_true() { + verify!( + indexed_payload_attestation_signature_set( + state, + |i| get_pubkey_from_state(state, i), + &indexed_payload_attestation.signature, + &indexed_payload_attestation, + spec + )? + .verify(), + Invalid::BadSignature + ); + } + + Ok(()) +} 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 92d856bfe2..eb23a39150 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -38,7 +38,7 @@ pub fn process_operations>( process_bls_to_execution_changes(state, bls_to_execution_changes, verify_signatures, spec)?; } - if state.fork_name_unchecked().electra_enabled() { + if state.fork_name_unchecked() == ForkName::Electra { state.update_pubkey_cache()?; process_deposit_requests(state, &block_body.execution_requests()?.deposits, spec)?; process_withdrawal_requests(state, &block_body.execution_requests()?.withdrawals, spec)?; @@ -49,6 +49,16 @@ pub fn process_operations>( )?; } + if state.fork_name_unchecked().eip7732_enabled() { + process_payload_attestations( + state, + block_body.payload_attestations()?.iter(), + verify_signatures, + ctxt, + spec, + )?; + } + Ok(()) } @@ -704,3 +714,49 @@ pub fn process_consolidation_request( Ok(()) } + +pub fn process_payload_attestation( + state: &mut BeaconState, + payload_attestation: &PayloadAttestation, + att_index: usize, + verify_signatures: VerifySignatures, + ctxt: &mut ConsensusContext, + spec: &ChainSpec, +) -> Result<(), BlockProcessingError> { + let _indexed = + verify_payload_attestation(state, payload_attestation, ctxt, verify_signatures, spec) + .map_err(|e| e.into_with_index(att_index))?; + + // TODO((EIP-7732): finish implementing this function + + Ok(()) +} + +pub fn process_payload_attestations<'a, E: EthSpec, I>( + state: &mut BeaconState, + payload_attestations: I, + verify_signatures: VerifySignatures, + ctxt: &mut ConsensusContext, + spec: &ChainSpec, +) -> Result<(), BlockProcessingError> +where + I: Iterator>, +{ + // Ensure required caches are all built. These should be no-ops during regular operation. + // TODO(EIP-7732): check necessary caches + state.build_committee_cache(RelativeEpoch::Current, spec)?; + state.build_committee_cache(RelativeEpoch::Previous, spec)?; + + payload_attestations + .enumerate() + .try_for_each(|(i, payload_attestation)| { + process_payload_attestation( + state, + payload_attestation, + i, + verify_signatures, + ctxt, + spec, + ) + }) +} 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 668945cf7c..944c16862a 100644 --- a/consensus/state_processing/src/per_block_processing/signature_sets.rs +++ b/consensus/state_processing/src/per_block_processing/signature_sets.rs @@ -9,10 +9,11 @@ use tree_hash::TreeHash; use types::{ AbstractExecPayload, AggregateSignature, AttesterSlashingRef, BeaconBlockRef, BeaconState, BeaconStateError, ChainSpec, DepositData, Domain, Epoch, EthSpec, Fork, Hash256, - InconsistentFork, IndexedAttestation, IndexedAttestationRef, ProposerSlashing, PublicKey, - PublicKeyBytes, Signature, SignedAggregateAndProof, SignedBeaconBlock, SignedBeaconBlockHeader, - SignedBlsToExecutionChange, SignedContributionAndProof, SignedRoot, SignedVoluntaryExit, - SigningData, Slot, SyncAggregate, SyncAggregatorSelectionData, Unsigned, + InconsistentFork, IndexedAttestation, IndexedAttestationRef, IndexedPayloadAttestation, + ProposerSlashing, PublicKey, PublicKeyBytes, Signature, SignedAggregateAndProof, + SignedBeaconBlock, SignedBeaconBlockHeader, SignedBlsToExecutionChange, + SignedContributionAndProof, SignedRoot, SignedVoluntaryExit, SigningData, Slot, SyncAggregate, + SyncAggregatorSelectionData, Unsigned, }; pub type Result = std::result::Result; @@ -298,6 +299,35 @@ where Ok(SignatureSet::multiple_pubkeys(signature, pubkeys, message)) } +pub fn indexed_payload_attestation_signature_set<'a, 'b, E, F>( + state: &'a BeaconState, + get_pubkey: F, + signature: &'a AggregateSignature, + indexed_payload_attestation: &'b IndexedPayloadAttestation, + spec: &'a ChainSpec, +) -> Result> +where + E: EthSpec, + F: Fn(usize) -> Option>, +{ + let mut pubkeys = Vec::with_capacity(indexed_payload_attestation.attesting_indices.len()); + for &validator_idx in indexed_payload_attestation.attesting_indices.iter() { + pubkeys.push( + get_pubkey(validator_idx as usize).ok_or(Error::ValidatorUnknown(validator_idx))?, + ); + } + + let domain = spec.compute_domain( + Domain::PTCAttester, + spec.genesis_fork_version, + state.genesis_validators_root(), + ); + + let message = indexed_payload_attestation.data.signing_root(domain); + + Ok(SignatureSet::multiple_pubkeys(signature, pubkeys, message)) +} + /// Returns the signature set for the given `indexed_attestation` but pubkeys are supplied directly /// instead of from the state. pub fn indexed_attestation_signature_set_from_pubkeys<'a, 'b, E, F>( diff --git a/consensus/state_processing/src/per_block_processing/verify_payload_attestation.rs b/consensus/state_processing/src/per_block_processing/verify_payload_attestation.rs new file mode 100644 index 0000000000..85271976f7 --- /dev/null +++ b/consensus/state_processing/src/per_block_processing/verify_payload_attestation.rs @@ -0,0 +1,42 @@ +use super::errors::{BlockOperationError, PayloadAttestationInvalid as Invalid}; +use super::VerifySignatures; +use crate::per_block_processing::is_valid_indexed_payload_attestation; +use crate::ConsensusContext; +use types::*; + +pub fn verify_payload_attestation<'ctxt, E: EthSpec>( + state: &BeaconState, + payload_attestation: &'ctxt PayloadAttestation, + ctxt: &'ctxt mut ConsensusContext, + verify_signatures: VerifySignatures, + spec: &ChainSpec, +) -> Result<&'ctxt IndexedPayloadAttestation, BlockOperationError> { + // Check that the attestation is for the parent beacon block + let data = &payload_attestation.data; + verify!( + data.beacon_block_root == state.latest_block_header().parent_root, + Invalid::BlockRootMismatch { + expected: state.latest_block_header().parent_root, + found: data.beacon_block_root, + } + ); + // Check that the attestation is for the previous slot + verify!( + data.slot + 1 == state.slot(), + Invalid::SlotMismatch { + expected: state.slot().saturating_sub(Slot::new(1)), + found: data.slot, + } + ); + + let indexed_payload_attestation = + ctxt.get_indexed_payload_attestation(state, data.slot, &payload_attestation)?; + is_valid_indexed_payload_attestation( + state, + &indexed_payload_attestation, + verify_signatures, + spec, + )?; + + Ok(indexed_payload_attestation) +} diff --git a/consensus/types/src/beacon_block.rs b/consensus/types/src/beacon_block.rs index f9703430fc..e6a2bf483e 100644 --- a/consensus/types/src/beacon_block.rs +++ b/consensus/types/src/beacon_block.rs @@ -725,8 +725,12 @@ impl> BeaconBlockEIP7732 let payload_attestations = vec![ PayloadAttestation:: { aggregation_bits: BitList::with_capacity(E::PTCSize::to_usize()).unwrap(), - slot: Slot::new(0), - payload_status: PayloadStatus::PayloadPresent, + data: PayloadAttestationData { + beacon_block_root: Hash256::zero(), + slot: Slot::new(0), + payload_status: PayloadStatus::PayloadPresent, + }, + signature: AggregateSignature::empty(), }; E::max_payload_attestations() ] diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index 66af3a74fa..13ce8b92f2 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -1841,6 +1841,44 @@ impl BeaconState { }) } + /// Get the PTC + /// Requires the committee cache to be initialized. + /// TODO(EIP-7732): is it easier to return u64 or usize? + /// TODO(EIP-7732): definitely gonna have to cache this.. + pub fn get_ptc(&self, slot: Slot) -> Result, Error> { + // this function is only used here and + // I have no idea where else to put it + fn bit_floor(n: u64) -> u64 { + if n == 0 { + 0 + } else { + 1 << (n.leading_zeros() as u64 ^ 63) + } + } + + let committee_cache = self.committee_cache_at_slot(slot)?; + let committee_count_per_slot = committee_cache.committees_per_slot(); + + let committees_per_slot = bit_floor(std::cmp::min( + committee_count_per_slot as u64, + E::PTCSize::to_u64(), + )); + let members_per_committee = + committee_count_per_slot.safe_div(committees_per_slot)? as usize; + + let committees = committee_cache.get_beacon_committees_at_slot(slot)?; + let mut validator_indices = Vec::with_capacity(committees_per_slot as usize); + for idx in 0..committees_per_slot { + let beacon_committee = committees + .get(idx as usize) + .ok_or_else(|| Error::InvalidCommitteeIndex(idx))?; + validator_indices + .extend_from_slice(&beacon_committee.committee[..members_per_committee]); + } + + Ok(FixedVector::new(validator_indices)?) + } + /// Build all caches (except the tree hash cache), if they need to be built. pub fn build_caches(&mut self, spec: &ChainSpec) -> Result<(), Error> { self.build_all_committee_caches(spec)?; diff --git a/consensus/types/src/indexed_payload_attestation.rs b/consensus/types/src/indexed_payload_attestation.rs index 0cc62ed08a..3f04c44dc1 100644 --- a/consensus/types/src/indexed_payload_attestation.rs +++ b/consensus/types/src/indexed_payload_attestation.rs @@ -23,7 +23,7 @@ pub struct IndexedPayloadAttestation { #[serde(with = "ssz_types::serde_utils::quoted_u64_var_list")] pub attesting_indices: VariableList, pub data: PayloadAttestationData, - signature: AggregateSignature, + pub signature: AggregateSignature, } #[cfg(test)] diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index 9671619752..20af39e3d2 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -224,8 +224,8 @@ pub use crate::payload::{ FullPayload, FullPayloadBellatrix, FullPayloadCapella, FullPayloadDeneb, FullPayloadElectra, FullPayloadRef, OwnedExecPayload, }; -pub use crate::payload_attestation::{PayloadAttestation, PayloadStatus}; -pub use crate::payload_attestation_data::PayloadAttestationData; +pub use crate::payload_attestation::PayloadAttestation; +pub use crate::payload_attestation_data::{PayloadAttestationData, PayloadStatus}; pub use crate::payload_attestation_message::PayloadAttestationMessage; pub use crate::pending_attestation::PendingAttestation; pub use crate::pending_balance_deposit::PendingBalanceDeposit; diff --git a/consensus/types/src/payload_attestation.rs b/consensus/types/src/payload_attestation.rs index 07525657bb..16a49d2efa 100644 --- a/consensus/types/src/payload_attestation.rs +++ b/consensus/types/src/payload_attestation.rs @@ -1,12 +1,9 @@ use crate::test_utils::TestRandom; use crate::*; use derivative::Derivative; -use rand::Rng; -use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use ssz::{Decode, Encode}; +use serde::{Deserialize, Serialize}; use ssz_derive::{Decode, Encode}; use test_random_derive::TestRandom; -use tree_hash::TreeHash; use tree_hash_derive::TreeHash; #[derive( @@ -26,123 +23,8 @@ use tree_hash_derive::TreeHash; #[derivative(PartialEq, Hash)] pub struct PayloadAttestation { pub aggregation_bits: BitList, - pub slot: Slot, - pub payload_status: PayloadStatus, -} - -#[repr(u8)] -#[derive(arbitrary::Arbitrary, Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub enum PayloadStatus { - PayloadAbsent = 0, - PayloadPresent = 1, - PayloadWithheld = 2, - PayloadInvalidStatus = 3, -} - -impl TryFrom for PayloadStatus { - type Error = String; - - fn try_from(byte: u8) -> Result { - match byte { - 0 => Ok(PayloadStatus::PayloadAbsent), - 1 => Ok(PayloadStatus::PayloadPresent), - 2 => Ok(PayloadStatus::PayloadWithheld), - 3 => Ok(PayloadStatus::PayloadInvalidStatus), - _ => Err(format!("Invalid byte for PayloadStatus: {}", byte)), - } - } -} - -impl TestRandom for PayloadStatus { - fn random_for_test(rng: &mut impl rand::RngCore) -> Self { - rng.gen_range(0u8..=3u8) - .try_into() - .expect("PayloadStatus: random byte is valid") - } -} - -impl TreeHash for PayloadStatus { - fn tree_hash_type() -> tree_hash::TreeHashType { - u8::tree_hash_type() - } - - fn tree_hash_packed_encoding(&self) -> tree_hash::PackedEncoding { - (*self as u8).tree_hash_packed_encoding() - } - - fn tree_hash_packing_factor() -> usize { - u8::tree_hash_packing_factor() - } - - fn tree_hash_root(&self) -> Hash256 { - (*self as u8).tree_hash_root() - } -} - -// ssz(enum_behaviour = "tag") would probably work but we want to ensure -// that the mapping between the variant and u8 matches the spec -impl Encode for PayloadStatus { - fn is_ssz_fixed_len() -> bool { - ::is_ssz_fixed_len() - } - - fn ssz_fixed_len() -> usize { - ::ssz_fixed_len() - } - - fn ssz_bytes_len(&self) -> usize { - ::ssz_bytes_len(&(*self as u8)) - } - - fn ssz_append(&self, buf: &mut Vec) { - ::ssz_append(&(*self as u8), buf) - } -} - -impl Decode for PayloadStatus { - fn is_ssz_fixed_len() -> bool { - ::is_ssz_fixed_len() - } - - fn ssz_fixed_len() -> usize { - ::ssz_fixed_len() - } - - fn from_ssz_bytes(bytes: &[u8]) -> Result { - (*bytes - // the u8 is just the first byte of the slice - .get(0) - .ok_or(ssz::DecodeError::InvalidByteLength { - len: 0, - expected: 1, - })?) - .try_into() - .map_err(|s| ssz::DecodeError::BytesInvalid(s)) - } -} - -impl Serialize for PayloadStatus { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - serde_utils::quoted_u8::Quoted::::serialize( - &serde_utils::quoted_u8::Quoted { - value: (*self as u8), - }, - serializer, - ) - } -} - -impl<'de> Deserialize<'de> for PayloadStatus { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - let quoted = serde_utils::quoted_u8::Quoted::::deserialize(deserializer)?; - PayloadStatus::try_from(quoted.value).map_err(serde::de::Error::custom) - } + pub data: PayloadAttestationData, + pub signature: AggregateSignature, } #[cfg(test)] @@ -151,10 +33,3 @@ mod payload_attestation_tests { ssz_and_tree_hash_tests!(PayloadAttestation); } - -#[cfg(test)] -mod payload_status_tests { - use super::*; - - ssz_and_tree_hash_tests!(PayloadStatus); -} diff --git a/consensus/types/src/payload_attestation_data.rs b/consensus/types/src/payload_attestation_data.rs index 9546713e2c..639014f6cc 100644 --- a/consensus/types/src/payload_attestation_data.rs +++ b/consensus/types/src/payload_attestation_data.rs @@ -1,8 +1,11 @@ use crate::test_utils::TestRandom; use crate::*; -use serde::{Deserialize, Serialize}; +use rand::Rng; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; use test_random_derive::TestRandom; +use tree_hash::TreeHash; use tree_hash_derive::TreeHash; #[derive( @@ -17,16 +20,141 @@ use tree_hash_derive::TreeHash; Decode, Serialize, Deserialize, + Hash, )] pub struct PayloadAttestationData { pub beacon_block_root: Hash256, pub slot: Slot, - pub payload_status: u8, + pub payload_status: PayloadStatus, +} + +impl SignedRoot for PayloadAttestationData {} + +#[repr(u8)] +#[derive(arbitrary::Arbitrary, Debug, Clone, Copy, PartialEq, Eq, Hash, Ord, PartialOrd)] +pub enum PayloadStatus { + PayloadAbsent = 0, + PayloadPresent = 1, + PayloadWithheld = 2, + PayloadInvalidStatus = 3, +} + +impl TryFrom for PayloadStatus { + type Error = String; + + fn try_from(byte: u8) -> Result { + match byte { + 0 => Ok(PayloadStatus::PayloadAbsent), + 1 => Ok(PayloadStatus::PayloadPresent), + 2 => Ok(PayloadStatus::PayloadWithheld), + 3 => Ok(PayloadStatus::PayloadInvalidStatus), + _ => Err(format!("Invalid byte for PayloadStatus: {}", byte)), + } + } +} + +impl TestRandom for PayloadStatus { + fn random_for_test(rng: &mut impl rand::RngCore) -> Self { + rng.gen_range(0u8..=3u8) + .try_into() + .expect("PayloadStatus: random byte is valid") + } +} + +impl TreeHash for PayloadStatus { + fn tree_hash_type() -> tree_hash::TreeHashType { + u8::tree_hash_type() + } + + fn tree_hash_packed_encoding(&self) -> tree_hash::PackedEncoding { + (*self as u8).tree_hash_packed_encoding() + } + + fn tree_hash_packing_factor() -> usize { + u8::tree_hash_packing_factor() + } + + fn tree_hash_root(&self) -> Hash256 { + (*self as u8).tree_hash_root() + } +} + +// ssz(enum_behaviour = "tag") would probably work but we want to ensure +// that the mapping between the variant and u8 matches the spec +impl Encode for PayloadStatus { + fn is_ssz_fixed_len() -> bool { + ::is_ssz_fixed_len() + } + + fn ssz_fixed_len() -> usize { + ::ssz_fixed_len() + } + + fn ssz_bytes_len(&self) -> usize { + ::ssz_bytes_len(&(*self as u8)) + } + + fn ssz_append(&self, buf: &mut Vec) { + ::ssz_append(&(*self as u8), buf) + } +} + +impl Decode for PayloadStatus { + fn is_ssz_fixed_len() -> bool { + ::is_ssz_fixed_len() + } + + fn ssz_fixed_len() -> usize { + ::ssz_fixed_len() + } + + fn from_ssz_bytes(bytes: &[u8]) -> Result { + (*bytes + // the u8 is just the first byte of the slice + .get(0) + .ok_or(ssz::DecodeError::InvalidByteLength { + len: 0, + expected: 1, + })?) + .try_into() + .map_err(|s| ssz::DecodeError::BytesInvalid(s)) + } +} + +impl Serialize for PayloadStatus { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + serde_utils::quoted_u8::Quoted::::serialize( + &serde_utils::quoted_u8::Quoted { + value: (*self as u8), + }, + serializer, + ) + } +} + +impl<'de> Deserialize<'de> for PayloadStatus { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let quoted = serde_utils::quoted_u8::Quoted::::deserialize(deserializer)?; + PayloadStatus::try_from(quoted.value).map_err(serde::de::Error::custom) + } } #[cfg(test)] -mod tests { +mod payload_attestation_data_tests { use super::*; ssz_and_tree_hash_tests!(PayloadAttestationData); } + +#[cfg(test)] +mod payload_status_tests { + use super::*; + + ssz_and_tree_hash_tests!(PayloadStatus); +}