diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 75063ee2e0..416179445f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -64,7 +64,7 @@ jobs: - uses: KyleMayes/install-llvm-action@v1 if: env.SELF_HOSTED_RUNNERS == 'false' && startsWith(matrix.arch, 'x86_64-windows') with: - version: "16.0" + version: "17.0" directory: ${{ runner.temp }}/llvm - name: Set LIBCLANG_PATH if: startsWith(matrix.arch, 'x86_64-windows') diff --git a/.github/workflows/test-suite.yml b/.github/workflows/test-suite.yml index 0840651bbc..769b889de4 100644 --- a/.github/workflows/test-suite.yml +++ b/.github/workflows/test-suite.yml @@ -112,11 +112,6 @@ jobs: - name: Install make if: env.SELF_HOSTED_RUNNERS == 'false' run: choco install -y make -# - uses: KyleMayes/install-llvm-action@v1 -# if: env.SELF_HOSTED_RUNNERS == 'false' -# with: -# version: "16.0" -# directory: ${{ runner.temp }}/llvm - name: Set LIBCLANG_PATH run: echo "LIBCLANG_PATH=$((gcm clang).source -replace "clang.exe")" >> $env:GITHUB_ENV - name: Run tests in release diff --git a/Cargo.lock b/Cargo.lock index 5c165cec89..753763fe96 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1716,16 +1716,15 @@ dependencies = [ [[package]] name = "curve25519-dalek" -version = "4.1.2" +version = "4.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0a677b8922c94e01bdbb12126b0bc852f00447528dee1782229af9c720c3f348" +checksum = "97fb8b7c4503de7d6ae7b42ab72a5a59857b4c937ec27a3d4539dba95b5ab2be" dependencies = [ "cfg-if", "cpufeatures", "curve25519-dalek-derive", "digest 0.10.7", "fiat-crypto", - "platforms 3.4.0", "rustc_version 0.4.0", "subtle", "zeroize", @@ -4964,6 +4963,7 @@ dependencies = [ "libp2p-mplex", "lighthouse_metrics", "lighthouse_version", + "logging", "lru", "lru_cache", "parking_lot 0.12.3", @@ -6143,12 +6143,6 @@ version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e8d0eef3571242013a0d5dc84861c3ae4a652e56e12adf8bdc26ff5f8cb34c94" -[[package]] -name = "platforms" -version = "3.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "db23d408679286588f4d4644f965003d056e3dd5abcaaa938116871d7ce2fee7" - [[package]] name = "plotters" version = "0.3.6" @@ -6455,7 +6449,7 @@ dependencies = [ "nix 0.24.3", "num_cpus", "once_cell", - "platforms 2.0.0", + "platforms", "thiserror", "unescape", ] @@ -8041,7 +8035,8 @@ checksum = "81cdd64d312baedb58e21336b31bc043b77e01cc99033ce76ef539f78e965ebc" [[package]] name = "superstruct" version = "0.8.0" -source = "git+https://github.com/sigp/superstruct?rev=45eecbfb9708c9fe11dbb6a6a5bd8d618f02269e#45eecbfb9708c9fe11dbb6a6a5bd8d618f02269e" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf0f31f730ad9e579364950e10d6172b4a9bd04b447edf5988b066a860cc340e" dependencies = [ "darling", "itertools", diff --git a/Cargo.toml b/Cargo.toml index 1059c74625..d67f6edf1d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -162,7 +162,7 @@ smallvec = "1.11.2" snap = "1" ssz_types = "0.6" strum = { version = "0.24", features = ["derive"] } -superstruct = { git = "https://github.com/sigp/superstruct", rev = "45eecbfb9708c9fe11dbb6a6a5bd8d618f02269e" } +superstruct = "0.8" syn = "1" sysinfo = "0.26" tempfile = "3" diff --git a/FUNDING.json b/FUNDING.json new file mode 100644 index 0000000000..5001999927 --- /dev/null +++ b/FUNDING.json @@ -0,0 +1,7 @@ +{ + "drips": { + "ethereum": { + "ownedBy": "0x25c4a76E7d118705e7Ea2e9b7d8C59930d8aCD3b" + } + } +} \ No newline at end of file diff --git a/Makefile b/Makefile index 3e6934e6b5..c3c8d13238 100644 --- a/Makefile +++ b/Makefile @@ -229,7 +229,6 @@ lint: -D clippy::manual_let_else \ -D warnings \ -A clippy::derive_partial_eq_without_eq \ - -A clippy::from-over-into \ -A clippy::upper-case-acronyms \ -A clippy::vec-init-then-push \ -A clippy::question-mark \ diff --git a/beacon_node/beacon_chain/src/attestation_verification.rs b/beacon_node/beacon_chain/src/attestation_verification.rs index d82685c48c..32c33f3e61 100644 --- a/beacon_node/beacon_chain/src/attestation_verification.rs +++ b/beacon_node/beacon_chain/src/attestation_verification.rs @@ -35,8 +35,10 @@ mod batch; use crate::{ - beacon_chain::VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT, metrics, - observed_aggregates::ObserveOutcome, observed_attesters::Error as ObservedAttestersError, + beacon_chain::VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT, + metrics, + observed_aggregates::{ObserveOutcome, ObservedAttestationKey}, + observed_attesters::Error as ObservedAttestersError, BeaconChain, BeaconChainError, BeaconChainTypes, }; use bls::verify_signature_sets; @@ -58,11 +60,11 @@ use state_processing::{ use std::borrow::Cow; use strum::AsRefStr; use tree_hash::TreeHash; -use tree_hash_derive::TreeHash; use types::{ - Attestation, AttestationData, AttestationRef, BeaconCommittee, BeaconStateError, - BeaconStateError::NoCommitteeFound, ChainSpec, CommitteeIndex, Epoch, EthSpec, ForkName, - Hash256, IndexedAttestation, SelectionProof, SignedAggregateAndProof, Slot, SubnetId, + Attestation, AttestationRef, BeaconCommittee, + BeaconStateError::{self, NoCommitteeFound}, + ChainSpec, CommitteeIndex, Epoch, EthSpec, ForkName, Hash256, IndexedAttestation, + SelectionProof, SignedAggregateAndProof, Slot, SubnetId, }; pub use batch::{batch_verify_aggregated_attestations, batch_verify_unaggregated_attestations}; @@ -309,12 +311,6 @@ struct IndexedAggregatedAttestation<'a, T: BeaconChainTypes> { observed_attestation_key_root: Hash256, } -#[derive(TreeHash)] -pub struct ObservedAttestationKey { - pub committee_index: u64, - pub attestation_data: AttestationData, -} - /// Wraps a `Attestation` that has been verified up until the point that an `IndexedAttestation` can /// be derived. /// @@ -524,7 +520,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> { .tree_hash_root(); // [New in Electra:EIP7549] - verify_committee_index(attestation, &chain.spec)?; + verify_committee_index(attestation)?; if chain .observed_attestations @@ -601,59 +597,62 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> { )) } }; + + // Committees must be sorted by ascending index order 0..committees_per_slot let get_indexed_attestation_with_committee = |(committees, _): (Vec, CommitteesPerSlot)| { + let (index, aggregator_index, selection_proof, data) = match signed_aggregate { + SignedAggregateAndProof::Base(signed_aggregate) => ( + signed_aggregate.message.aggregate.data.index, + signed_aggregate.message.aggregator_index, + // Note: this clones the signature which is known to be a relatively slow operation. + // Future optimizations should remove this clone. + signed_aggregate.message.selection_proof.clone(), + signed_aggregate.message.aggregate.data.clone(), + ), + SignedAggregateAndProof::Electra(signed_aggregate) => ( + signed_aggregate + .message + .aggregate + .committee_index() + .ok_or(Error::NotExactlyOneCommitteeBitSet(0))?, + signed_aggregate.message.aggregator_index, + signed_aggregate.message.selection_proof.clone(), + signed_aggregate.message.aggregate.data.clone(), + ), + }; + let slot = data.slot; + + let committee = committees + .get(index as usize) + .ok_or(Error::NoCommitteeForSlotAndIndex { slot, index })?; + + if !SelectionProof::from(selection_proof) + .is_aggregator(committee.committee.len(), &chain.spec) + .map_err(|e| Error::BeaconChainError(e.into()))? + { + return Err(Error::InvalidSelectionProof { aggregator_index }); + } + + // Ensure the aggregator is a member of the committee for which it is aggregating. + if !committee.committee.contains(&(aggregator_index as usize)) { + return Err(Error::AggregatorNotInCommittee { aggregator_index }); + } + + // p2p aggregates have a single committee, we can assert that aggregation_bits is always + // less then MaxValidatorsPerCommittee match signed_aggregate { SignedAggregateAndProof::Base(signed_aggregate) => { - let att = &signed_aggregate.message.aggregate; - let aggregator_index = signed_aggregate.message.aggregator_index; - let committee = committees - .iter() - .filter(|&committee| committee.index == att.data.index) - .at_most_one() - .map_err(|_| Error::NoCommitteeForSlotAndIndex { - slot: att.data.slot, - index: att.data.index, - })?; - - // TODO(electra): - // Note: this clones the signature which is known to be a relatively slow operation. - // - // Future optimizations should remove this clone. - if let Some(committee) = committee { - let selection_proof = SelectionProof::from( - signed_aggregate.message.selection_proof.clone(), - ); - - if !selection_proof - .is_aggregator(committee.committee.len(), &chain.spec) - .map_err(|e| Error::BeaconChainError(e.into()))? - { - return Err(Error::InvalidSelectionProof { aggregator_index }); - } - - // Ensure the aggregator is a member of the committee for which it is aggregating. - if !committee.committee.contains(&(aggregator_index as usize)) { - return Err(Error::AggregatorNotInCommittee { aggregator_index }); - } - - attesting_indices_base::get_indexed_attestation( - committee.committee, - att, - ) - .map_err(|e| BeaconChainError::from(e).into()) - } else { - Err(Error::NoCommitteeForSlotAndIndex { - slot: att.data.slot, - index: att.data.index, - }) - } + attesting_indices_base::get_indexed_attestation( + committee.committee, + &signed_aggregate.message.aggregate, + ) + .map_err(|e| BeaconChainError::from(e).into()) } SignedAggregateAndProof::Electra(signed_aggregate) => { - attesting_indices_electra::get_indexed_attestation_from_signed_aggregate( + attesting_indices_electra::get_indexed_attestation( &committees, - signed_aggregate, - &chain.spec, + &signed_aggregate.message.aggregate, ) .map_err(|e| BeaconChainError::from(e).into()) } @@ -842,7 +841,7 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> { } // [New in Electra:EIP7549] - verify_committee_index(attestation, &chain.spec)?; + verify_committee_index(attestation)?; // Attestations must be for a known block. If the block is unknown, we simply drop the // attestation and do not delay consideration for later. @@ -1342,17 +1341,10 @@ pub fn verify_signed_aggregate_signatures( /// Verify that the `attestation` committee index is properly set for the attestation's fork. /// This function will only apply verification post-Electra. -pub fn verify_committee_index( - attestation: AttestationRef, - spec: &ChainSpec, -) -> Result<(), Error> { - if spec.fork_name_at_slot::(attestation.data().slot) >= ForkName::Electra { +pub fn verify_committee_index(attestation: AttestationRef) -> Result<(), Error> { + if let Ok(committee_bits) = attestation.committee_bits() { // Check to ensure that the attestation is for a single committee. - let num_committee_bits = get_committee_indices::( - attestation - .committee_bits() - .map_err(|e| Error::BeaconChainError(e.into()))?, - ); + let num_committee_bits = get_committee_indices::(committee_bits); if num_committee_bits.len() != 1 { return Err(Error::NotExactlyOneCommitteeBitSet( num_committee_bits.len(), @@ -1405,8 +1397,7 @@ pub fn obtain_indexed_attestation_and_committees_per_slot( attesting_indices_electra::get_indexed_attestation(&committees, att) .map(|attestation| (attestation, committees_per_slot)) .map_err(|e| { - let index = att.committee_index().unwrap_or(0); - if e == BlockOperationError::BeaconStateError(NoCommitteeFound(index)) { + if let BlockOperationError::BeaconStateError(NoCommitteeFound(index)) = e { Error::NoCommitteeForSlotAndIndex { slot: att.data.slot, index, @@ -1429,6 +1420,8 @@ pub fn obtain_indexed_attestation_and_committees_per_slot( /// /// If the committees for an `attestation`'s slot isn't found in the `shuffling_cache`, we will read a state /// from disk and then update the `shuffling_cache`. +/// +/// Committees are sorted by ascending index order 0..committees_per_slot fn map_attestation_committees( chain: &BeaconChain, attestation: AttestationRef, diff --git a/beacon_node/beacon_chain/src/attester_cache.rs b/beacon_node/beacon_chain/src/attester_cache.rs index 2e07cd32ed..b5012e8e4e 100644 --- a/beacon_node/beacon_chain/src/attester_cache.rs +++ b/beacon_node/beacon_chain/src/attester_cache.rs @@ -15,6 +15,7 @@ use state_processing::state_advance::{partial_state_advance, Error as StateAdvan use std::collections::HashMap; use std::ops::Range; use types::{ + attestation::Error as AttestationError, beacon_state::{ compute_committee_index_in_epoch, compute_committee_range_in_epoch, epoch_committee_count, }, @@ -59,6 +60,7 @@ pub enum Error { InverseRange { range: Range, }, + AttestationError(AttestationError), } impl From for Error { diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 3d14747a5f..66e2964669 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -122,7 +122,6 @@ use store::{ use task_executor::{ShutdownReason, TaskExecutor}; use tokio_stream::Stream; use tree_hash::TreeHash; -use types::attestation::AttestationBase; use types::blob_sidecar::FixedBlobSidecarList; use types::payload::BlockProductionVersion; use types::*; @@ -1994,36 +1993,15 @@ impl BeaconChain { }; drop(cache_timer); - if self.spec.fork_name_at_slot::(request_slot) >= ForkName::Electra { - let mut committee_bits = BitVector::default(); - if committee_len > 0 { - committee_bits.set(request_index as usize, true)?; - } - Ok(Attestation::Electra(AttestationElectra { - aggregation_bits: BitList::with_capacity(committee_len)?, - data: AttestationData { - slot: request_slot, - index: 0u64, - beacon_block_root, - source: justified_checkpoint, - target, - }, - committee_bits, - signature: AggregateSignature::empty(), - })) - } else { - Ok(Attestation::Base(AttestationBase { - aggregation_bits: BitList::with_capacity(committee_len)?, - data: AttestationData { - slot: request_slot, - index: request_index, - beacon_block_root, - source: justified_checkpoint, - target, - }, - signature: AggregateSignature::empty(), - })) - } + Ok(Attestation::::empty_for_signing( + request_index, + committee_len, + request_slot, + beacon_block_root, + justified_checkpoint, + target, + &self.spec, + )?) } /// Performs the same validation as `Self::verify_unaggregated_attestation_for_gossip`, but for diff --git a/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs b/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs index 2a42b49b42..f7f389c543 100644 --- a/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs +++ b/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs @@ -389,35 +389,35 @@ pub struct PersistedForkChoiceStore { pub equivocating_indices: BTreeSet, } -impl Into for PersistedForkChoiceStoreV11 { - fn into(self) -> PersistedForkChoiceStore { +impl From for PersistedForkChoiceStore { + fn from(from: PersistedForkChoiceStoreV11) -> PersistedForkChoiceStore { PersistedForkChoiceStore { - balances_cache: self.balances_cache, - time: self.time, - finalized_checkpoint: self.finalized_checkpoint, - justified_checkpoint: self.justified_checkpoint, - justified_balances: self.justified_balances, - unrealized_justified_checkpoint: self.unrealized_justified_checkpoint, - unrealized_finalized_checkpoint: self.unrealized_finalized_checkpoint, - proposer_boost_root: self.proposer_boost_root, - equivocating_indices: self.equivocating_indices, + balances_cache: from.balances_cache, + time: from.time, + finalized_checkpoint: from.finalized_checkpoint, + justified_checkpoint: from.justified_checkpoint, + justified_balances: from.justified_balances, + unrealized_justified_checkpoint: from.unrealized_justified_checkpoint, + unrealized_finalized_checkpoint: from.unrealized_finalized_checkpoint, + proposer_boost_root: from.proposer_boost_root, + equivocating_indices: from.equivocating_indices, } } } -impl Into for PersistedForkChoiceStore { - fn into(self) -> PersistedForkChoiceStoreV11 { +impl From for PersistedForkChoiceStoreV11 { + fn from(from: PersistedForkChoiceStore) -> PersistedForkChoiceStoreV11 { PersistedForkChoiceStoreV11 { - balances_cache: self.balances_cache, - time: self.time, - finalized_checkpoint: self.finalized_checkpoint, - justified_checkpoint: self.justified_checkpoint, - justified_balances: self.justified_balances, + balances_cache: from.balances_cache, + time: from.time, + finalized_checkpoint: from.finalized_checkpoint, + justified_checkpoint: from.justified_checkpoint, + justified_balances: from.justified_balances, best_justified_checkpoint: JUNK_BEST_JUSTIFIED_CHECKPOINT, - unrealized_justified_checkpoint: self.unrealized_justified_checkpoint, - unrealized_finalized_checkpoint: self.unrealized_finalized_checkpoint, - proposer_boost_root: self.proposer_boost_root, - equivocating_indices: self.equivocating_indices, + unrealized_justified_checkpoint: from.unrealized_justified_checkpoint, + unrealized_finalized_checkpoint: from.unrealized_finalized_checkpoint, + proposer_boost_root: from.proposer_boost_root, + equivocating_indices: from.equivocating_indices, } } } diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index fdf8ee2b97..2b62a83194 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -10,7 +10,6 @@ use crate::block_verification::{ use crate::kzg_utils::{validate_blob, validate_blobs}; use crate::{metrics, BeaconChainError}; use kzg::{Error as KzgError, Kzg, KzgCommitment}; -use merkle_proof::MerkleTreeError; use slog::debug; use ssz_derive::{Decode, Encode}; use ssz_types::VariableList; @@ -128,13 +127,6 @@ pub enum GossipBlobError { /// The blob sidecar is invalid and the peer is faulty. KzgError(kzg::Error), - /// The kzg commitment inclusion proof failed. - /// - /// ## Peer scoring - /// - /// The blob sidecar is invalid - InclusionProof(MerkleTreeError), - /// The pubkey cache timed out. /// /// ## Peer scoring @@ -459,10 +451,7 @@ pub fn validate_blob_sidecar_for_gossip( // Verify the inclusion proof in the sidecar let _timer = metrics::start_timer(&metrics::BLOB_SIDECAR_INCLUSION_PROOF_VERIFICATION); - if !blob_sidecar - .verify_blob_sidecar_inclusion_proof() - .map_err(GossipBlobError::InclusionProof)? - { + if !blob_sidecar.verify_blob_sidecar_inclusion_proof() { return Err(GossipBlobError::InvalidInclusionProof); } drop(_timer); diff --git a/beacon_node/beacon_chain/src/early_attester_cache.rs b/beacon_node/beacon_chain/src/early_attester_cache.rs index 936f4de3ee..dda699cc6c 100644 --- a/beacon_node/beacon_chain/src/early_attester_cache.rs +++ b/beacon_node/beacon_chain/src/early_attester_cache.rs @@ -6,7 +6,6 @@ use crate::{ use parking_lot::RwLock; use proto_array::Block as ProtoBlock; use std::sync::Arc; -use types::attestation::AttestationBase; use types::*; pub struct CacheItem { @@ -123,40 +122,16 @@ impl EarlyAttesterCache { item.committee_lengths .get_committee_length::(request_slot, request_index, spec)?; - let attestation = if spec.fork_name_at_slot::(request_slot) >= ForkName::Electra { - let mut committee_bits = BitVector::default(); - if committee_len > 0 { - committee_bits - .set(request_index as usize, true) - .map_err(BeaconStateError::from)?; - } - Attestation::Electra(AttestationElectra { - aggregation_bits: BitList::with_capacity(committee_len) - .map_err(BeaconStateError::from)?, - committee_bits, - data: AttestationData { - slot: request_slot, - index: 0u64, - beacon_block_root: item.beacon_block_root, - source: item.source, - target: item.target, - }, - signature: AggregateSignature::empty(), - }) - } else { - Attestation::Base(AttestationBase { - aggregation_bits: BitList::with_capacity(committee_len) - .map_err(BeaconStateError::from)?, - data: AttestationData { - slot: request_slot, - index: request_index, - beacon_block_root: item.beacon_block_root, - source: item.source, - target: item.target, - }, - signature: AggregateSignature::empty(), - }) - }; + let attestation = Attestation::empty_for_signing( + request_index, + committee_len, + request_slot, + item.beacon_block_root, + item.source, + item.target, + spec, + ) + .map_err(Error::AttestationError)?; metrics::inc_counter(&metrics::BEACON_EARLY_ATTESTER_CACHE_HITS); diff --git a/beacon_node/beacon_chain/src/eth1_chain.rs b/beacon_node/beacon_chain/src/eth1_chain.rs index 229b3fd525..6e80abffc7 100644 --- a/beacon_node/beacon_chain/src/eth1_chain.rs +++ b/beacon_node/beacon_chain/src/eth1_chain.rs @@ -693,7 +693,7 @@ mod test { fn get_eth1_data(i: u64) -> Eth1Data { Eth1Data { block_hash: Hash256::from_low_u64_be(i), - deposit_root: Hash256::from_low_u64_be(u64::max_value() - i), + deposit_root: Hash256::from_low_u64_be(u64::MAX - i), deposit_count: i, } } diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index f419429e09..466ab0b67e 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -39,7 +39,7 @@ mod light_client_server_cache; pub mod metrics; pub mod migrate; mod naive_aggregation_pool; -mod observed_aggregates; +pub mod observed_aggregates; mod observed_attesters; mod observed_blob_sidecars; pub mod observed_block_producers; diff --git a/beacon_node/beacon_chain/src/naive_aggregation_pool.rs b/beacon_node/beacon_chain/src/naive_aggregation_pool.rs index b16ced789f..211aecfe63 100644 --- a/beacon_node/beacon_chain/src/naive_aggregation_pool.rs +++ b/beacon_node/beacon_chain/src/naive_aggregation_pool.rs @@ -241,24 +241,12 @@ impl AggregateMap for AggregatedAttestationMap { fn insert(&mut self, a: AttestationRef) -> Result { let _timer = metrics::start_timer(&metrics::ATTESTATION_PROCESSING_AGG_POOL_CORE_INSERT); - let aggregation_bit = match a { - AttestationRef::Base(att) => att - .aggregation_bits - .iter() - .enumerate() - .filter_map(|(i, bit)| if bit { Some(i) } else { None }) - .at_most_one() - .map_err(|iter| Error::MoreThanOneAggregationBitSet(iter.count()))? - .ok_or(Error::NoAggregationBitsSet)?, - AttestationRef::Electra(att) => att - .aggregation_bits - .iter() - .enumerate() - .filter_map(|(i, bit)| if bit { Some(i) } else { None }) - .at_most_one() - .map_err(|iter| Error::MoreThanOneAggregationBitSet(iter.count()))? - .ok_or(Error::NoAggregationBitsSet)?, - }; + let aggregation_bit = *a + .set_aggregation_bits() + .iter() + .at_most_one() + .map_err(|iter| Error::MoreThanOneAggregationBitSet(iter.count()))? + .ok_or(Error::NoAggregationBitsSet)?; let attestation_key = AttestationKey::from_attestation_ref(a)?; let attestation_key_root = attestation_key.tree_hash_root(); diff --git a/beacon_node/beacon_chain/src/observed_aggregates.rs b/beacon_node/beacon_chain/src/observed_aggregates.rs index 5e161a998b..00476bfe7a 100644 --- a/beacon_node/beacon_chain/src/observed_aggregates.rs +++ b/beacon_node/beacon_chain/src/observed_aggregates.rs @@ -6,11 +6,14 @@ use ssz_types::{BitList, BitVector}; use std::collections::HashMap; use std::marker::PhantomData; use tree_hash::TreeHash; +use tree_hash_derive::TreeHash; use types::consts::altair::{ SYNC_COMMITTEE_SUBNET_COUNT, TARGET_AGGREGATORS_PER_SYNC_SUBCOMMITTEE, }; use types::slot_data::SlotData; -use types::{Attestation, AttestationRef, EthSpec, Hash256, Slot, SyncCommitteeContribution}; +use types::{ + Attestation, AttestationData, AttestationRef, EthSpec, Hash256, Slot, SyncCommitteeContribution, +}; pub type ObservedSyncContributions = ObservedAggregates< SyncCommitteeContribution, @@ -20,6 +23,17 @@ pub type ObservedSyncContributions = ObservedAggregates< pub type ObservedAggregateAttestations = ObservedAggregates, E, BitList<::MaxValidatorsPerSlot>>; +/// Attestation data augmented with committee index +/// +/// This is hashed and used to key the map of observed aggregate attestations. This is important +/// post-Electra where the attestation data committee index is 0 and we want to avoid accidentally +/// comparing aggregation bits for *different* committees. +#[derive(TreeHash)] +pub struct ObservedAttestationKey { + pub committee_index: u64, + pub attestation_data: AttestationData, +} + /// A trait use to associate capacity constants with the type being stored in `ObservedAggregates`. pub trait Consts { /// The default capacity of items stored per slot, in a single `SlotHashSet`. @@ -92,11 +106,11 @@ pub trait SubsetItem { /// Returns the item that gets stored in `ObservedAggregates` for later subset /// comparison with incoming aggregates. - fn get_item(&self) -> Self::Item; + fn get_item(&self) -> Result; /// Returns a unique value that keys the object to the item that is being stored /// in `ObservedAggregates`. - fn root(&self) -> Hash256; + fn root(&self) -> Result; } impl<'a, E: EthSpec> SubsetItem for AttestationRef<'a, E> { @@ -126,19 +140,22 @@ impl<'a, E: EthSpec> SubsetItem for AttestationRef<'a, E> { } /// Returns the sync contribution aggregation bits. - fn get_item(&self) -> Self::Item { + fn get_item(&self) -> Result { match self { - Self::Base(att) => { - // TODO(electra) fix unwrap - att.extend_aggregation_bits().unwrap() - } - Self::Electra(att) => att.aggregation_bits.clone(), + Self::Base(att) => att + .extend_aggregation_bits() + .map_err(|_| Error::GetItemError), + Self::Electra(att) => Ok(att.aggregation_bits.clone()), } } - /// Returns the hash tree root of the attestation data. - fn root(&self) -> Hash256 { - self.data().tree_hash_root() + /// Returns the hash tree root of the attestation data augmented with the committee index. + fn root(&self) -> Result { + Ok(ObservedAttestationKey { + committee_index: self.committee_index().ok_or(Error::RootError)?, + attestation_data: self.data().clone(), + } + .tree_hash_root()) } } @@ -153,19 +170,19 @@ impl<'a, E: EthSpec> SubsetItem for &'a SyncCommitteeContribution { } /// Returns the sync contribution aggregation bits. - fn get_item(&self) -> Self::Item { - self.aggregation_bits.clone() + fn get_item(&self) -> Result { + Ok(self.aggregation_bits.clone()) } /// Returns the hash tree root of the root, slot and subcommittee index /// of the sync contribution. - fn root(&self) -> Hash256 { - SyncCommitteeData { + fn root(&self) -> Result { + Ok(SyncCommitteeData { root: self.beacon_block_root, slot: self.slot, subcommittee_index: self.subcommittee_index, } - .tree_hash_root() + .tree_hash_root()) } } @@ -192,6 +209,8 @@ pub enum Error { expected: Slot, attestation: Slot, }, + GetItemError, + RootError, } /// A `HashMap` that contains entries related to some `Slot`. @@ -234,7 +253,7 @@ impl SlotHashSet { // If true, we replace the new item with its existing subset. This allows us // to hold fewer items in the list. } else if item.is_superset(existing) { - *existing = item.get_item(); + *existing = item.get_item()?; return Ok(ObserveOutcome::New); } } @@ -252,7 +271,7 @@ impl SlotHashSet { return Err(Error::ReachedMaxObservationsPerSlot(self.max_capacity)); } - let item = item.get_item(); + let item = item.get_item()?; self.map.entry(root).or_default().push(item); Ok(ObserveOutcome::New) } @@ -345,7 +364,7 @@ where root_opt: Option, ) -> Result { let index = self.get_set_index(item.get_slot())?; - let root = root_opt.unwrap_or_else(|| item.root()); + let root = root_opt.map_or_else(|| item.root(), Ok)?; self.sets .get_mut(index) @@ -454,12 +473,13 @@ where #[cfg(not(debug_assertions))] mod tests { use super::*; - use types::{test_utils::test_random_instance, Hash256}; + use types::{test_utils::test_random_instance, AttestationBase, Hash256}; type E = types::MainnetEthSpec; fn get_attestation(slot: Slot, beacon_block_root: u64) -> Attestation { - let mut a: Attestation = test_random_instance(); + let a: AttestationBase = test_random_instance(); + let mut a = Attestation::Base(a); a.data_mut().slot = slot; a.data_mut().beacon_block_root = Hash256::from_low_u64_be(beacon_block_root); a @@ -487,7 +507,10 @@ mod tests { for a in &items { assert_eq!( - store.is_known_subset(a.as_reference(), a.as_reference().root()), + store.is_known_subset( + a.as_reference(), + a.as_reference().root().unwrap() + ), Ok(false), "should indicate an unknown attestation is unknown" ); @@ -500,12 +523,18 @@ mod tests { for a in &items { assert_eq!( - store.is_known_subset(a.as_reference(), a.as_reference().root()), + store.is_known_subset( + a.as_reference(), + a.as_reference().root().unwrap() + ), Ok(true), "should indicate a known attestation is known" ); assert_eq!( - store.observe_item(a.as_reference(), Some(a.as_reference().root())), + store.observe_item( + a.as_reference(), + Some(a.as_reference().root().unwrap()) + ), Ok(ObserveOutcome::Subset), "should acknowledge an existing attestation" ); diff --git a/beacon_node/beacon_chain/src/persisted_fork_choice.rs b/beacon_node/beacon_chain/src/persisted_fork_choice.rs index 8297ea9345..5e50c3433a 100644 --- a/beacon_node/beacon_chain/src/persisted_fork_choice.rs +++ b/beacon_node/beacon_chain/src/persisted_fork_choice.rs @@ -20,20 +20,20 @@ pub struct PersistedForkChoice { pub fork_choice_store: PersistedForkChoiceStoreV17, } -impl Into for PersistedForkChoiceV11 { - fn into(self) -> PersistedForkChoice { +impl From for PersistedForkChoice { + fn from(from: PersistedForkChoiceV11) -> PersistedForkChoice { PersistedForkChoice { - fork_choice: self.fork_choice, - fork_choice_store: self.fork_choice_store.into(), + fork_choice: from.fork_choice, + fork_choice_store: from.fork_choice_store.into(), } } } -impl Into for PersistedForkChoice { - fn into(self) -> PersistedForkChoiceV11 { +impl From for PersistedForkChoiceV11 { + fn from(from: PersistedForkChoice) -> PersistedForkChoiceV11 { PersistedForkChoiceV11 { - fork_choice: self.fork_choice, - fork_choice_store: self.fork_choice_store.into(), + fork_choice: from.fork_choice, + fork_choice_store: from.fork_choice_store.into(), } } } diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 34ad6d4324..bd98f19af6 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -58,7 +58,6 @@ use store::{config::StoreConfig, HotColdDB, ItemStore, LevelDB, MemoryStore}; use task_executor::TaskExecutor; use task_executor::{test_utils::TestRuntime, ShutdownReason}; use tree_hash::TreeHash; -use types::attestation::AttestationBase; use types::indexed_attestation::IndexedAttestationBase; use types::payload::BlockProductionVersion; pub use types::test_utils::generate_deterministic_keypairs; @@ -1033,40 +1032,18 @@ where *state.get_block_root(target_slot)? }; - if self.spec.fork_name_at_slot::(slot) >= ForkName::Electra { - let mut committee_bits = BitVector::default(); - committee_bits.set(index as usize, true)?; - Ok(Attestation::Electra(AttestationElectra { - aggregation_bits: BitList::with_capacity(committee_len)?, - committee_bits, - data: AttestationData { - slot, - index: 0u64, - beacon_block_root, - source: state.current_justified_checkpoint(), - target: Checkpoint { - epoch, - root: target_root, - }, - }, - signature: AggregateSignature::empty(), - })) - } else { - Ok(Attestation::Base(AttestationBase { - aggregation_bits: BitList::with_capacity(committee_len)?, - data: AttestationData { - slot, - index, - beacon_block_root, - source: state.current_justified_checkpoint(), - target: Checkpoint { - epoch, - root: target_root, - }, - }, - signature: AggregateSignature::empty(), - })) - } + Ok(Attestation::empty_for_signing( + index, + committee_len, + slot, + beacon_block_root, + state.current_justified_checkpoint(), + Checkpoint { + epoch, + root: target_root, + }, + &self.spec, + )?) } /// A list of attestations for each committee for the given slot. @@ -1376,37 +1353,26 @@ where let fork_name = self.spec.fork_name_at_slot::(slot); - let aggregate = if fork_name >= ForkName::Electra { - self.chain - .get_aggregated_attestation_electra( - slot, - &attestation.data().tree_hash_root(), - bc.index, - ) - .unwrap() - .unwrap_or_else(|| { - committee_attestations.iter().skip(1).fold( - attestation.clone(), - |mut agg, (att, _)| { - agg.aggregate(att.to_ref()); - agg - }, - ) - }) + let aggregate = if fork_name.electra_enabled() { + self.chain.get_aggregated_attestation_electra( + slot, + &attestation.data().tree_hash_root(), + bc.index, + ) } else { self.chain .get_aggregated_attestation_base(attestation.data()) - .unwrap() - .unwrap_or_else(|| { - committee_attestations.iter().skip(1).fold( - attestation.clone(), - |mut agg, (att, _)| { - agg.aggregate(att.to_ref()); - agg - }, - ) - }) - }; + } + .unwrap() + .unwrap_or_else(|| { + committee_attestations.iter().skip(1).fold( + attestation.clone(), + |mut agg, (att, _)| { + agg.aggregate(att.to_ref()); + agg + }, + ) + }); // If the chain is able to produce an aggregate, use that. Otherwise, build an // aggregate locally. @@ -1541,40 +1507,29 @@ where let fork_name = self.spec.fork_name_at_slot::(Slot::new(0)); - let mut attestation_1 = if fork_name >= ForkName::Electra { + let data = AttestationData { + slot: Slot::new(0), + index: 0, + beacon_block_root: Hash256::zero(), + target: Checkpoint { + root: Hash256::zero(), + epoch: target1.unwrap_or(fork.epoch), + }, + source: Checkpoint { + root: Hash256::zero(), + epoch: source1.unwrap_or(Epoch::new(0)), + }, + }; + let mut attestation_1 = if fork_name.electra_enabled() { IndexedAttestation::Electra(IndexedAttestationElectra { attesting_indices: VariableList::new(validator_indices).unwrap(), - data: AttestationData { - slot: Slot::new(0), - index: 0, - beacon_block_root: Hash256::zero(), - target: Checkpoint { - root: Hash256::zero(), - epoch: target1.unwrap_or(fork.epoch), - }, - source: Checkpoint { - root: Hash256::zero(), - epoch: source1.unwrap_or(Epoch::new(0)), - }, - }, + data, signature: AggregateSignature::infinity(), }) } else { IndexedAttestation::Base(IndexedAttestationBase { attesting_indices: VariableList::new(validator_indices).unwrap(), - data: AttestationData { - slot: Slot::new(0), - index: 0, - beacon_block_root: Hash256::zero(), - target: Checkpoint { - root: Hash256::zero(), - epoch: target1.unwrap_or(fork.epoch), - }, - source: Checkpoint { - root: Hash256::zero(), - epoch: source1.unwrap_or(Epoch::new(0)), - }, - }, + data, signature: AggregateSignature::infinity(), }) }; @@ -1623,7 +1578,7 @@ where } } - if fork_name >= ForkName::Electra { + if fork_name.electra_enabled() { AttesterSlashing::Electra(AttesterSlashingElectra { attestation_1: attestation_1.as_electra().unwrap().clone(), attestation_2: attestation_2.as_electra().unwrap().clone(), @@ -1657,7 +1612,7 @@ where }, }; - let (mut attestation_1, mut attestation_2) = if fork_name >= ForkName::Electra { + let (mut attestation_1, mut attestation_2) = if fork_name.electra_enabled() { let attestation_1 = IndexedAttestationElectra { attesting_indices: VariableList::new(validator_indices_1).unwrap(), data: data.clone(), @@ -1735,7 +1690,7 @@ where } } - if fork_name >= ForkName::Electra { + if fork_name.electra_enabled() { AttesterSlashing::Electra(AttesterSlashingElectra { attestation_1: attestation_1.as_electra().unwrap().clone(), attestation_2: attestation_2.as_electra().unwrap().clone(), diff --git a/beacon_node/beacon_chain/src/validator_monitor.rs b/beacon_node/beacon_chain/src/validator_monitor.rs index 058da369bc..4ce5383819 100644 --- a/beacon_node/beacon_chain/src/validator_monitor.rs +++ b/beacon_node/beacon_chain/src/validator_monitor.rs @@ -2067,7 +2067,7 @@ pub fn timestamp_now() -> Duration { } fn u64_to_i64(n: impl Into) -> i64 { - i64::try_from(n.into()).unwrap_or(i64::max_value()) + i64::try_from(n.into()).unwrap_or(i64::MAX) } /// Returns the delay between the start of `block.slot` and `seen_timestamp`. diff --git a/beacon_node/beacon_chain/tests/attestation_production.rs b/beacon_node/beacon_chain/tests/attestation_production.rs index a58cfd9f2d..697e449dc6 100644 --- a/beacon_node/beacon_chain/tests/attestation_production.rs +++ b/beacon_node/beacon_chain/tests/attestation_production.rs @@ -190,35 +190,22 @@ async fn produces_attestations() { .produce_unaggregated_attestation(slot, index) .expect("should produce attestation"); - match &attestation { + let (aggregation_bits_len, aggregation_bits_zero) = match &attestation { Attestation::Base(att) => { - assert_eq!( - att.aggregation_bits.len(), - committee_len, - "bad committee len" - ); - assert!( - att.aggregation_bits.is_zero(), - "some committee bits are set" - ); + (att.aggregation_bits.len(), att.aggregation_bits.is_zero()) } Attestation::Electra(att) => { - assert_eq!( - att.aggregation_bits.len(), - committee_len, - "bad committee len" - ); - assert!( - att.aggregation_bits.is_zero(), - "some committee bits are set" - ); + (att.aggregation_bits.len(), att.aggregation_bits.is_zero()) } - } + }; + assert_eq!(aggregation_bits_len, committee_len, "bad committee len"); + assert!(aggregation_bits_zero, "some committee bits are set"); + let data = attestation.data(); assert_eq!( attestation.signature(), - &AggregateSignature::empty(), + &AggregateSignature::infinity(), "bad signature" ); assert_eq!(data.index, index, "bad index"); diff --git a/beacon_node/beacon_chain/tests/attestation_verification.rs b/beacon_node/beacon_chain/tests/attestation_verification.rs index 63740c4736..19efe10c6d 100644 --- a/beacon_node/beacon_chain/tests/attestation_verification.rs +++ b/beacon_node/beacon_chain/tests/attestation_verification.rs @@ -2,8 +2,8 @@ use beacon_chain::attestation_verification::{ batch_verify_aggregated_attestations, batch_verify_unaggregated_attestations, Error, - ObservedAttestationKey, }; +use beacon_chain::observed_aggregates::ObservedAttestationKey; use beacon_chain::test_utils::{MakeAttestationOptions, HARNESS_GENESIS_TIME}; use beacon_chain::{ attestation_verification::Error as AttnError, @@ -233,7 +233,6 @@ fn get_non_aggregator( let state = &head.beacon_state; let current_slot = chain.slot().expect("should get slot"); - // TODO(electra) make fork-agnostic let committee = state .get_beacon_committee( current_slot, @@ -852,7 +851,9 @@ async fn aggregated_gossip_verification() { err, AttnError::AttestationSupersetKnown(hash) if hash == ObservedAttestationKey { - committee_index: tester.valid_aggregate.message().aggregate().expect("should get committee index"), + committee_index: tester.valid_aggregate.message().aggregate() + .committee_index() + .expect("should get committee index"), attestation_data: tester.valid_aggregate.message().aggregate().data().clone(), }.tree_hash_root() )) diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 0fe406a017..4c7a499697 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -668,7 +668,7 @@ async fn invalid_signature_attester_slashing() { let mut snapshots = chain_segment.clone(); let fork_name = harness.chain.spec.fork_name_at_slot::(Slot::new(0)); - let attester_slashing = if fork_name >= ForkName::Electra { + let attester_slashing = if fork_name.electra_enabled() { let indexed_attestation = IndexedAttestationElectra { attesting_indices: vec![0].into(), data: AttestationData { @@ -1308,62 +1308,34 @@ async fn verify_block_for_gossip_doppelganger_detection() { } }; - match indexed_attestation { - IndexedAttestation::Base(indexed_attestation) => { - for &index in &indexed_attestation.attesting_indices { - let index = index as usize; + for index in match indexed_attestation { + IndexedAttestation::Base(att) => att.attesting_indices.into_iter(), + IndexedAttestation::Electra(att) => att.attesting_indices.into_iter(), + } { + let index = index as usize; - assert!(harness.chain.validator_seen_at_epoch(index, epoch)); + assert!(harness.chain.validator_seen_at_epoch(index, epoch)); - // Check the correct beacon cache is populated - assert!(harness - .chain - .observed_block_attesters - .read() - .validator_has_been_observed(epoch, index) - .expect("should check if block attester was observed")); - assert!(!harness - .chain - .observed_gossip_attesters - .read() - .validator_has_been_observed(epoch, index) - .expect("should check if gossip attester was observed")); - assert!(!harness - .chain - .observed_aggregators - .read() - .validator_has_been_observed(epoch, index) - .expect("should check if gossip aggregator was observed")); - } - } - IndexedAttestation::Electra(indexed_attestation) => { - for &index in &indexed_attestation.attesting_indices { - let index = index as usize; - - assert!(harness.chain.validator_seen_at_epoch(index, epoch)); - - // Check the correct beacon cache is populated - assert!(harness - .chain - .observed_block_attesters - .read() - .validator_has_been_observed(epoch, index) - .expect("should check if block attester was observed")); - assert!(!harness - .chain - .observed_gossip_attesters - .read() - .validator_has_been_observed(epoch, index) - .expect("should check if gossip attester was observed")); - assert!(!harness - .chain - .observed_aggregators - .read() - .validator_has_been_observed(epoch, index) - .expect("should check if gossip aggregator was observed")); - } - } - }; + // Check the correct beacon cache is populated + assert!(harness + .chain + .observed_block_attesters + .read() + .validator_has_been_observed(epoch, index) + .expect("should check if block attester was observed")); + assert!(!harness + .chain + .observed_gossip_attesters + .read() + .validator_has_been_observed(epoch, index) + .expect("should check if gossip attester was observed")); + assert!(!harness + .chain + .observed_aggregators + .read() + .validator_has_been_observed(epoch, index) + .expect("should check if gossip aggregator was observed")); + } } } diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index ef87390930..c2468102ee 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -1025,7 +1025,7 @@ async fn multiple_attestations_per_block() { let slot = snapshot.beacon_block.slot(); let fork_name = harness.chain.spec.fork_name_at_slot::(slot); - if fork_name >= ForkName::Electra { + if fork_name.electra_enabled() { assert_eq!( snapshot .beacon_block diff --git a/beacon_node/client/src/lib.rs b/beacon_node/client/src/lib.rs index fd92c28255..e6042103e1 100644 --- a/beacon_node/client/src/lib.rs +++ b/beacon_node/client/src/lib.rs @@ -1,5 +1,3 @@ -extern crate slog; - mod compute_light_client_updates; pub mod config; mod metrics; diff --git a/beacon_node/client/src/notifier.rs b/beacon_node/client/src/notifier.rs index 0a2f24748d..632188014e 100644 --- a/beacon_node/client/src/notifier.rs +++ b/beacon_node/client/src/notifier.rs @@ -551,7 +551,7 @@ async fn electra_readiness_logging( .snapshot .beacon_state .fork_name_unchecked() - >= ForkName::Electra; + .electra_enabled(); let has_execution_layer = beacon_chain.execution_layer.is_some(); @@ -721,10 +721,10 @@ fn eth1_logging(beacon_chain: &BeaconChain, log: &Logger } } -/// Returns the peer count, returning something helpful if it's `usize::max_value` (effectively a +/// Returns the peer count, returning something helpful if it's `usize::MAX` (effectively a /// `None` value). fn peer_count_pretty(peer_count: usize) -> String { - if peer_count == usize::max_value() { + if peer_count == usize::MAX { String::from("--") } else { format!("{}", peer_count) @@ -824,7 +824,7 @@ impl Speedo { /// Returns the average of the speeds between each observation. /// - /// Does not gracefully handle slots that are above `u32::max_value()`. + /// Does not gracefully handle slots that are above `u32::MAX`. pub fn slots_per_second(&self) -> Option { let speeds = self .0 diff --git a/beacon_node/eth1/src/lib.rs b/beacon_node/eth1/src/lib.rs index 3b288de490..9d6cb7c847 100644 --- a/beacon_node/eth1/src/lib.rs +++ b/beacon_node/eth1/src/lib.rs @@ -1,6 +1,3 @@ -#[macro_use] -extern crate lazy_static; - mod block_cache; mod deposit_cache; mod inner; diff --git a/beacon_node/eth1/src/metrics.rs b/beacon_node/eth1/src/metrics.rs index 5441b40d7e..ad94d42ecb 100644 --- a/beacon_node/eth1/src/metrics.rs +++ b/beacon_node/eth1/src/metrics.rs @@ -1,5 +1,7 @@ pub use lighthouse_metrics::*; +use lazy_static::lazy_static; + lazy_static! { /* * Eth1 blocks diff --git a/beacon_node/eth1/src/service.rs b/beacon_node/eth1/src/service.rs index d68a8b6f28..9cc1da1382 100644 --- a/beacon_node/eth1/src/service.rs +++ b/beacon_node/eth1/src/service.rs @@ -853,7 +853,7 @@ impl Service { let max_log_requests_per_update = self .config() .max_log_requests_per_update - .unwrap_or_else(usize::max_value); + .unwrap_or(usize::MAX); let range = { match new_block_numbers { @@ -996,10 +996,7 @@ impl Service { ) -> Result { let client = self.client(); let block_cache_truncation = self.config().block_cache_truncation; - let max_blocks_per_update = self - .config() - .max_blocks_per_update - .unwrap_or_else(usize::max_value); + let max_blocks_per_update = self.config().max_blocks_per_update.unwrap_or(usize::MAX); let range = { match new_block_numbers { @@ -1025,7 +1022,7 @@ impl Service { let range_size = range.end() - range.start(); let max_size = block_cache_truncation .map(|n| n as u64) - .unwrap_or_else(u64::max_value); + .unwrap_or_else(|| u64::MAX); if range_size > max_size { // If the range of required blocks is larger than `max_size`, drop all // existing blocks and download `max_size` count of blocks. diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index 3f0b2ff602..1c03cc81fc 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -261,9 +261,9 @@ pub mod deposit_methods { Latest, } - impl Into for Eth1Id { - fn into(self) -> u64 { - match self { + impl From for u64 { + fn from(from: Eth1Id) -> u64 { + match from { Eth1Id::Mainnet => 1, Eth1Id::Custom(id) => id, } @@ -416,7 +416,7 @@ pub mod deposit_methods { .ok_or("Block number was not string")?, )?; - if number <= usize::max_value() as u64 { + if number <= usize::MAX as u64 { Ok(Block { hash, timestamp, diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 6e674b220e..6ac4b6eb9a 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -2191,7 +2191,7 @@ fn verify_builder_bid( // Avoid logging values that we can't represent with our Prometheus library. let payload_value_gwei = bid.data.message.value() / 1_000_000_000; - if payload_value_gwei <= Uint256::from(i64::max_value()) { + if payload_value_gwei <= Uint256::from(i64::MAX) { metrics::set_gauge_vec( &metrics::EXECUTION_LAYER_PAYLOAD_BIDS, &[metrics::BUILDER], diff --git a/beacon_node/execution_layer/src/metrics.rs b/beacon_node/execution_layer/src/metrics.rs index 3ed99ca606..6aaada3dff 100644 --- a/beacon_node/execution_layer/src/metrics.rs +++ b/beacon_node/execution_layer/src/metrics.rs @@ -81,7 +81,7 @@ lazy_static::lazy_static! { ); pub static ref EXECUTION_LAYER_PAYLOAD_BIDS: Result = try_create_int_gauge_vec( "execution_layer_payload_bids", - "The gwei bid value of payloads received by local EEs or builders. Only shows values up to i64::max_value.", + "The gwei bid value of payloads received by local EEs or builders. Only shows values up to i64::MAX.", &["source"] ); } diff --git a/beacon_node/execution_layer/src/versioned_hashes.rs b/beacon_node/execution_layer/src/versioned_hashes.rs index 37bd35646d..9bf87596b4 100644 --- a/beacon_node/execution_layer/src/versioned_hashes.rs +++ b/beacon_node/execution_layer/src/versioned_hashes.rs @@ -1,5 +1,3 @@ -extern crate alloy_consensus; -extern crate alloy_rlp; use alloy_consensus::TxEnvelope; use alloy_rlp::Decodable; use types::{EthSpec, ExecutionPayloadRef, Hash256, Unsigned, VersionedHash}; diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index c637fb6aa8..5baf96091b 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -2279,9 +2279,9 @@ impl ApiTester { vec![validator_count], vec![validator_count, 1], vec![validator_count, 1, 3], - vec![u64::max_value()], - vec![u64::max_value(), 1], - vec![u64::max_value(), 1, 3], + vec![u64::MAX], + vec![u64::MAX, 1], + vec![u64::MAX, 1, 3], ]; interesting.push((0..validator_count).collect()); diff --git a/beacon_node/lighthouse_network/Cargo.toml b/beacon_node/lighthouse_network/Cargo.toml index e850bced16..56a8fe99c7 100644 --- a/beacon_node/lighthouse_network/Cargo.toml +++ b/beacon_node/lighthouse_network/Cargo.toml @@ -60,6 +60,7 @@ tempfile = { workspace = true } quickcheck = { workspace = true } quickcheck_macros = { workspace = true } async-channel = { workspace = true } +logging = { workspace = true } [features] libp2p-websocket = [] diff --git a/beacon_node/lighthouse_network/src/discovery/mod.rs b/beacon_node/lighthouse_network/src/discovery/mod.rs index 5c937a1e0b..73f51c001a 100644 --- a/beacon_node/lighthouse_network/src/discovery/mod.rs +++ b/beacon_node/lighthouse_network/src/discovery/mod.rs @@ -561,8 +561,8 @@ impl Discovery { /// Updates the `eth2` field of our local ENR. pub fn update_eth2_enr(&mut self, enr_fork_id: EnrForkId) { // to avoid having a reference to the spec constant, for the logging we assume - // FAR_FUTURE_EPOCH is u64::max_value() - let next_fork_epoch_log = if enr_fork_id.next_fork_epoch == u64::max_value() { + // FAR_FUTURE_EPOCH is u64::MAX + let next_fork_epoch_log = if enr_fork_id.next_fork_epoch == u64::MAX { String::from("No other fork") } else { format!("{:?}", enr_fork_id.next_fork_epoch) diff --git a/beacon_node/lighthouse_network/src/lib.rs b/beacon_node/lighthouse_network/src/lib.rs index 264795844a..0b827164fc 100644 --- a/beacon_node/lighthouse_network/src/lib.rs +++ b/beacon_node/lighthouse_network/src/lib.rs @@ -2,9 +2,6 @@ /// all required libp2p functionality. /// /// This crate builds and manages the libp2p services required by the beacon node. -#[macro_use] -extern crate lazy_static; - mod config; pub mod service; diff --git a/beacon_node/lighthouse_network/src/metrics.rs b/beacon_node/lighthouse_network/src/metrics.rs index fc441f2533..8efed44eb4 100644 --- a/beacon_node/lighthouse_network/src/metrics.rs +++ b/beacon_node/lighthouse_network/src/metrics.rs @@ -1,5 +1,7 @@ pub use lighthouse_metrics::*; +use lazy_static::lazy_static; + lazy_static! { pub static ref NAT_OPEN: Result = try_create_int_gauge_vec( "nat_open", diff --git a/beacon_node/lighthouse_network/src/peer_manager/peerdb/score.rs b/beacon_node/lighthouse_network/src/peer_manager/peerdb/score.rs index ba9bd31472..8187dc4ba4 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/peerdb/score.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/peerdb/score.rs @@ -6,6 +6,7 @@ //! //! The scoring algorithms are currently experimental. use crate::service::gossipsub_scoring_parameters::GREYLIST_THRESHOLD as GOSSIPSUB_GREYLIST_THRESHOLD; +use lazy_static::lazy_static; use serde::Serialize; use std::time::Instant; use strum::AsRefStr; diff --git a/beacon_node/lighthouse_network/src/rpc/handler.rs b/beacon_node/lighthouse_network/src/rpc/handler.rs index 48f69c64c5..b7166efc37 100644 --- a/beacon_node/lighthouse_network/src/rpc/handler.rs +++ b/beacon_node/lighthouse_network/src/rpc/handler.rs @@ -374,6 +374,12 @@ where id: outbound_info.req_id, }))); } + + // Also handle any events that are awaiting to be sent to the behaviour + if !self.events_out.is_empty() { + return Poll::Ready(Some(self.events_out.remove(0))); + } + Poll::Ready(None) } diff --git a/beacon_node/lighthouse_network/src/rpc/mod.rs b/beacon_node/lighthouse_network/src/rpc/mod.rs index 1fa6862351..027af89edf 100644 --- a/beacon_node/lighthouse_network/src/rpc/mod.rs +++ b/beacon_node/lighthouse_network/src/rpc/mod.rs @@ -316,6 +316,27 @@ where self.events.push(error_msg); } } + + // Replace the pending Requests to the disconnected peer + // with reports of failed requests. + self.events.iter_mut().for_each(|event| match &event { + ToSwarm::NotifyHandler { + peer_id: p, + event: RPCSend::Request(request_id, req), + .. + } if *p == peer_id => { + *event = ToSwarm::GenerateEvent(RPCMessage { + peer_id, + conn_id: connection_id, + event: HandlerEvent::Err(HandlerErr::Outbound { + id: *request_id, + proto: req.versioned_protocol().protocol(), + error: RPCError::Disconnected, + }), + }); + } + _ => {} + }); } } diff --git a/beacon_node/lighthouse_network/src/rpc/protocol.rs b/beacon_node/lighthouse_network/src/rpc/protocol.rs index 12a7f09338..bfaaef9b3b 100644 --- a/beacon_node/lighthouse_network/src/rpc/protocol.rs +++ b/beacon_node/lighthouse_network/src/rpc/protocol.rs @@ -3,6 +3,7 @@ use crate::rpc::codec::{base::BaseInboundCodec, ssz_snappy::SSZSnappyInboundCode use futures::future::BoxFuture; use futures::prelude::{AsyncRead, AsyncWrite}; use futures::{FutureExt, StreamExt}; +use lazy_static::lazy_static; use libp2p::core::{InboundUpgrade, UpgradeInfo}; use ssz::Encode; use ssz_types::VariableList; diff --git a/beacon_node/lighthouse_network/src/rpc/self_limiter.rs b/beacon_node/lighthouse_network/src/rpc/self_limiter.rs index be4c572308..115ae45a98 100644 --- a/beacon_node/lighthouse_network/src/rpc/self_limiter.rs +++ b/beacon_node/lighthouse_network/src/rpc/self_limiter.rs @@ -147,7 +147,7 @@ impl SelfRateLimiter { Err((rate_limited_req, wait_time)) => { let key = (peer_id, protocol); self.next_peer_request.insert(key, wait_time); - queued_requests.push_back(rate_limited_req); + queued_requests.push_front(rate_limited_req); // If one fails just wait for the next window that allows sending requests. return; } @@ -205,3 +205,72 @@ impl SelfRateLimiter { Poll::Pending } } + +#[cfg(test)] +mod tests { + use crate::rpc::config::{OutboundRateLimiterConfig, RateLimiterConfig}; + use crate::rpc::rate_limiter::Quota; + use crate::rpc::self_limiter::SelfRateLimiter; + use crate::rpc::{OutboundRequest, Ping, Protocol}; + use crate::service::api_types::RequestId; + use libp2p::PeerId; + use std::time::Duration; + use types::MainnetEthSpec; + + /// Test that `next_peer_request_ready` correctly maintains the queue. + #[tokio::test] + async fn test_next_peer_request_ready() { + let log = logging::test_logger(); + let config = OutboundRateLimiterConfig(RateLimiterConfig { + ping_quota: Quota::n_every(1, 2), + ..Default::default() + }); + let mut limiter: SelfRateLimiter, MainnetEthSpec> = + SelfRateLimiter::new(config, log).unwrap(); + let peer_id = PeerId::random(); + + for i in 1..=5 { + let _ = limiter.allows( + peer_id, + RequestId::Application(i), + OutboundRequest::Ping(Ping { data: i }), + ); + } + + { + let queue = limiter + .delayed_requests + .get(&(peer_id, Protocol::Ping)) + .unwrap(); + assert_eq!(4, queue.len()); + + // Check that requests in the queue are ordered in the sequence 2, 3, 4, 5. + let mut iter = queue.iter(); + for i in 2..=5 { + assert_eq!(iter.next().unwrap().request_id, RequestId::Application(i)); + } + + assert_eq!(limiter.ready_requests.len(), 0); + } + + // Wait until the tokens have been regenerated, then run `next_peer_request_ready`. + tokio::time::sleep(Duration::from_secs(3)).await; + limiter.next_peer_request_ready(peer_id, Protocol::Ping); + + { + let queue = limiter + .delayed_requests + .get(&(peer_id, Protocol::Ping)) + .unwrap(); + assert_eq!(3, queue.len()); + + // Check that requests in the queue are ordered in the sequence 3, 4, 5. + let mut iter = queue.iter(); + for i in 3..=5 { + assert_eq!(iter.next().unwrap().request_id, RequestId::Application(i)); + } + + assert_eq!(limiter.ready_requests.len(), 1); + } + } +} diff --git a/beacon_node/lighthouse_network/src/types/pubsub.rs b/beacon_node/lighthouse_network/src/types/pubsub.rs index 7be68f879f..b443ecd1b9 100644 --- a/beacon_node/lighthouse_network/src/types/pubsub.rs +++ b/beacon_node/lighthouse_network/src/types/pubsub.rs @@ -158,18 +158,19 @@ impl PubsubMessage { GossipKind::BeaconAggregateAndProof => { let signed_aggregate_and_proof = match fork_context.from_context_bytes(gossip_topic.fork_digest) { - Some(ForkName::Base) - | Some(ForkName::Altair) - | Some(ForkName::Bellatrix) - | Some(ForkName::Capella) - | Some(ForkName::Deneb) => SignedAggregateAndProof::Base( - SignedAggregateAndProofBase::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ), - Some(ForkName::Electra) => SignedAggregateAndProof::Electra( - SignedAggregateAndProofElectra::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ), + Some(&fork_name) => { + if fork_name.electra_enabled() { + SignedAggregateAndProof::Electra( + SignedAggregateAndProofElectra::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ) + } else { + SignedAggregateAndProof::Base( + SignedAggregateAndProofBase::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ) + } + } None => { return Err(format!( "Unknown gossipsub fork digest: {:?}", @@ -184,18 +185,19 @@ impl PubsubMessage { GossipKind::Attestation(subnet_id) => { let attestation = match fork_context.from_context_bytes(gossip_topic.fork_digest) { - Some(ForkName::Base) - | Some(ForkName::Altair) - | Some(ForkName::Bellatrix) - | Some(ForkName::Capella) - | Some(ForkName::Deneb) => Attestation::Base( - AttestationBase::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ), - Some(ForkName::Electra) => Attestation::Electra( - AttestationElectra::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ), + Some(&fork_name) => { + if fork_name.electra_enabled() { + Attestation::Electra( + AttestationElectra::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ) + } else { + Attestation::Base( + AttestationBase::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ) + } + } None => { return Err(format!( "Unknown gossipsub fork digest: {:?}", @@ -281,18 +283,19 @@ impl PubsubMessage { GossipKind::AttesterSlashing => { let attester_slashing = match fork_context.from_context_bytes(gossip_topic.fork_digest) { - Some(ForkName::Base) - | Some(ForkName::Altair) - | Some(ForkName::Bellatrix) - | Some(ForkName::Capella) - | Some(ForkName::Deneb) => AttesterSlashing::Base( - AttesterSlashingBase::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ), - Some(ForkName::Electra) => AttesterSlashing::Electra( - AttesterSlashingElectra::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?, - ), + Some(&fork_name) => { + if fork_name.electra_enabled() { + AttesterSlashing::Electra( + AttesterSlashingElectra::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ) + } else { + AttesterSlashing::Base( + AttesterSlashingBase::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ) + } + } None => { return Err(format!( "Unknown gossipsub fork digest: {:?}", diff --git a/beacon_node/network/src/lib.rs b/beacon_node/network/src/lib.rs index da64368b16..1149e6e6e3 100644 --- a/beacon_node/network/src/lib.rs +++ b/beacon_node/network/src/lib.rs @@ -1,6 +1,3 @@ -#[macro_use] -extern crate lazy_static; - /// This crate provides the network server for Lighthouse. pub mod error; #[allow(clippy::mutable_key_type)] // PeerId in hashmaps are no longer permitted by clippy diff --git a/beacon_node/network/src/metrics.rs b/beacon_node/network/src/metrics.rs index bf4cbd09ab..32e57da8ae 100644 --- a/beacon_node/network/src/metrics.rs +++ b/beacon_node/network/src/metrics.rs @@ -5,6 +5,7 @@ use beacon_chain::{ sync_committee_verification::Error as SyncCommitteeError, }; use fnv::FnvHashMap; +use lazy_static::lazy_static; pub use lighthouse_metrics::*; use lighthouse_network::{ peer_manager::peerdb::client::ClientKind, types::GossipKind, GossipTopic, Gossipsub, diff --git a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs index d78091b313..8baa0f773d 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -105,12 +105,7 @@ impl VerifiedAttestation for VerifiedAggregate { /// Efficient clone-free implementation that moves out of the `Box`. fn into_attestation_and_indices(self) -> (Attestation, Vec) { - // TODO(electra): technically we shouldn't have to clone.. - let attestation = self - .signed_aggregate - .message() - .aggregate() - .clone_as_attestation(); + let attestation = self.signed_aggregate.into_attestation(); let attesting_indices = self.indexed_attestation.attesting_indices_to_vec(); (attestation, attesting_indices) } @@ -695,7 +690,6 @@ impl NetworkBeaconProcessor { | GossipBlobError::InvalidSubnet { .. } | GossipBlobError::InvalidInclusionProof | GossipBlobError::KzgError(_) - | GossipBlobError::InclusionProof(_) | GossipBlobError::NotFinalizedDescendant { .. } => { warn!( self.log, @@ -706,7 +700,7 @@ impl NetworkBeaconProcessor { "index" => %index, "commitment" => %commitment, ); - // Prevent recurring behaviour by penalizing the peer slightly. + // Prevent recurring behaviour by penalizing the peer. self.gossip_penalize_peer( peer_id, PeerAction::LowToleranceError, @@ -718,10 +712,8 @@ impl NetworkBeaconProcessor { MessageAcceptance::Reject, ); } - GossipBlobError::FutureSlot { .. } - | GossipBlobError::RepeatBlob { .. } - | GossipBlobError::PastFinalizedSlot { .. } => { - warn!( + GossipBlobError::FutureSlot { .. } | GossipBlobError::RepeatBlob { .. } => { + debug!( self.log, "Could not verify blob sidecar for gossip. Ignoring the blob sidecar"; "error" => ?err, @@ -742,6 +734,30 @@ impl NetworkBeaconProcessor { MessageAcceptance::Ignore, ); } + GossipBlobError::PastFinalizedSlot { .. } => { + debug!( + self.log, + "Could not verify blob sidecar for gossip. Ignoring the blob sidecar"; + "error" => ?err, + "slot" => %slot, + "root" => %root, + "index" => %index, + "commitment" => %commitment, + ); + // Prevent recurring behaviour by penalizing the peer. A low-tolerance + // error is fine because there's no reason for peers to be propagating old + // blobs on gossip, even if their view of finality is lagging. + self.gossip_penalize_peer( + peer_id, + PeerAction::LowToleranceError, + "gossip_blob_low", + ); + self.propagate_validation_result( + message_id, + peer_id, + MessageAcceptance::Ignore, + ); + } } } } diff --git a/beacon_node/network/src/network_beacon_processor/tests.rs b/beacon_node/network/src/network_beacon_processor/tests.rs index 06b12c14ae..a9b9f64a79 100644 --- a/beacon_node/network/src/network_beacon_processor/tests.rs +++ b/beacon_node/network/src/network_beacon_processor/tests.rs @@ -793,9 +793,7 @@ async fn aggregate_attestation_to_unknown_block(import_method: BlockImportMethod let mut rig = TestRig::new(SMALL_CHAIN).await; // Empty the op pool. - rig.chain - .op_pool - .prune_attestations(u64::max_value().into()); + rig.chain.op_pool.prune_attestations(u64::MAX.into()); assert_eq!(rig.chain.op_pool.num_attestations(), 0); // Send the attestation but not the block, and check that it was not imported. diff --git a/beacon_node/network/src/sync/network_context/requests.rs b/beacon_node/network/src/sync/network_context/requests.rs index cd73b4beba..6e4683701b 100644 --- a/beacon_node/network/src/sync/network_context/requests.rs +++ b/beacon_node/network/src/sync/network_context/requests.rs @@ -120,7 +120,7 @@ impl ActiveBlobsByRootRequest { if self.request.block_root != block_root { return Err(LookupVerifyError::UnrequestedBlockRoot(block_root)); } - if !blob.verify_blob_sidecar_inclusion_proof().unwrap_or(false) { + if !blob.verify_blob_sidecar_inclusion_proof() { return Err(LookupVerifyError::InvalidInclusionProof); } if !self.request.indices.contains(&blob.index) { diff --git a/beacon_node/operation_pool/src/attestation_storage.rs b/beacon_node/operation_pool/src/attestation_storage.rs index 43b1c3abbb..ad4ffe6d3c 100644 --- a/beacon_node/operation_pool/src/attestation_storage.rs +++ b/beacon_node/operation_pool/src/attestation_storage.rs @@ -217,7 +217,6 @@ impl CompactIndexedAttestationElectra { } pub fn aggregate_same_committee(&mut self, other: &Self) -> Option<()> { - // TODO(electra): remove assert in favour of Result if self.committee_bits != other.committee_bits { return None; } diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index c7659651da..49fbed5686 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -286,7 +286,7 @@ impl OperationPool { // TODO(electra): Work out how to do this more elegantly. This is a bit of a hack. let mut all_attestations = self.attestations.write(); - if fork_name >= ForkName::Electra { + if fork_name.electra_enabled() { all_attestations.aggregate_across_committees(prev_epoch_key); all_attestations.aggregate_across_committees(curr_epoch_key); } @@ -615,7 +615,7 @@ impl OperationPool { }) }, |address_change| address_change.as_inner().clone(), - usize::max_value(), + usize::MAX, ); changes.shuffle(&mut thread_rng()); changes @@ -1404,7 +1404,7 @@ mod release_tests { // Set of indices covered by previous attestations in `best_attestations`. let mut seen_indices = BTreeSet::::new(); // Used for asserting that rewards are in decreasing order. - let mut prev_reward = u64::max_value(); + let mut prev_reward = u64::MAX; let mut reward_cache = RewardCache::default(); reward_cache.update(&state).unwrap(); diff --git a/beacon_node/operation_pool/src/persistence.rs b/beacon_node/operation_pool/src/persistence.rs index 99cb1aafbc..79509e5f6c 100644 --- a/beacon_node/operation_pool/src/persistence.rs +++ b/beacon_node/operation_pool/src/persistence.rs @@ -258,8 +258,8 @@ impl StoreItem for PersistedOperationPool { fn from_store_bytes(bytes: &[u8]) -> Result { // Default deserialization to the latest variant. - PersistedOperationPoolV15::from_ssz_bytes(bytes) - .map(Self::V15) + PersistedOperationPoolV20::from_ssz_bytes(bytes) + .map(Self::V20) .map_err(Into::into) } } diff --git a/beacon_node/src/lib.rs b/beacon_node/src/lib.rs index 4ca084c316..ab400d2e73 100644 --- a/beacon_node/src/lib.rs +++ b/beacon_node/src/lib.rs @@ -1,5 +1,3 @@ -extern crate clap; - mod cli; mod config; diff --git a/beacon_node/store/src/lib.rs b/beacon_node/store/src/lib.rs index 66032d89c5..0247bea554 100644 --- a/beacon_node/store/src/lib.rs +++ b/beacon_node/store/src/lib.rs @@ -7,9 +7,6 @@ //! //! Provides a simple API for storing/retrieving all types that sometimes needs type-hints. See //! tests for implementation examples. -#[macro_use] -extern crate lazy_static; - mod chunk_writer; pub mod chunked_iter; pub mod chunked_vector; diff --git a/beacon_node/store/src/metrics.rs b/beacon_node/store/src/metrics.rs index 2d901fdd93..1e614036ea 100644 --- a/beacon_node/store/src/metrics.rs +++ b/beacon_node/store/src/metrics.rs @@ -1,6 +1,7 @@ pub use lighthouse_metrics::{set_gauge, try_create_int_gauge, *}; use directory::size_of_dir; +use lazy_static::lazy_static; use std::path::Path; lazy_static! { diff --git a/beacon_node/store/src/partial_beacon_state.rs b/beacon_node/store/src/partial_beacon_state.rs index e56d0580ac..5e6054bc06 100644 --- a/beacon_node/store/src/partial_beacon_state.rs +++ b/beacon_node/store/src/partial_beacon_state.rs @@ -133,7 +133,6 @@ where #[superstruct(only(Electra))] pub earliest_consolidation_epoch: Epoch, - // TODO(electra) should these be optional? #[superstruct(only(Electra))] pub pending_balance_deposits: List, #[superstruct(only(Electra))] diff --git a/common/compare_fields_derive/src/lib.rs b/common/compare_fields_derive/src/lib.rs index 01c5a8f6ef..1a89ccf4fd 100644 --- a/common/compare_fields_derive/src/lib.rs +++ b/common/compare_fields_derive/src/lib.rs @@ -1,5 +1,3 @@ -extern crate proc_macro; - use proc_macro::TokenStream; use quote::quote; use syn::{parse_macro_input, DeriveInput}; diff --git a/common/deposit_contract/src/lib.rs b/common/deposit_contract/src/lib.rs index 785b952213..5b54a05396 100644 --- a/common/deposit_contract/src/lib.rs +++ b/common/deposit_contract/src/lib.rs @@ -96,7 +96,7 @@ mod tests { let mut deposit_data = DepositData { pubkey: keypair.pk.into(), withdrawal_credentials: Hash256::from_slice(&[42; 32]), - amount: u64::max_value(), + amount: u64::MAX, signature: Signature::empty().into(), }; deposit_data.signature = deposit_data.create_signature(&keypair.sk, spec); diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index 4863b72cf0..d399bc2bd0 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -716,6 +716,21 @@ pub struct AttesterData { pub slot: Slot, } +impl AttesterData { + pub fn match_attestation_data( + &self, + attestation_data: &AttestationData, + spec: &ChainSpec, + ) -> bool { + if spec.fork_name_at_slot::(attestation_data.slot) < ForkName::Electra { + self.slot == attestation_data.slot && self.committee_index == attestation_data.index + } else { + // After electra `attestation_data.index` is set to 0 and does not match the duties + self.slot == attestation_data.slot + } + } +} + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct ProposerData { pub pubkey: PublicKeyBytes, @@ -1702,11 +1717,11 @@ impl ForkVersionDeserialize for FullBlockContents { } } -impl Into> for FullBlockContents { - fn into(self) -> BeaconBlock { - match self { - Self::BlockContents(block_and_sidecars) => block_and_sidecars.block, - Self::Block(block) => block, +impl From> for BeaconBlock { + fn from(from: FullBlockContents) -> BeaconBlock { + match from { + FullBlockContents::::BlockContents(block_and_sidecars) => block_and_sidecars.block, + FullBlockContents::::Block(block) => block, } } } diff --git a/common/eth2_interop_keypairs/src/lib.rs b/common/eth2_interop_keypairs/src/lib.rs index 3031e1c4dc..34c3d6f87c 100644 --- a/common/eth2_interop_keypairs/src/lib.rs +++ b/common/eth2_interop_keypairs/src/lib.rs @@ -16,11 +16,9 @@ //! //! This implementation passes the [reference implementation //! tests](https://github.com/ethereum/eth2.0-pm/blob/6e41fcf383ebeb5125938850d8e9b4e9888389b4/interop/mocked_start/keygen_test_vector.yaml). -#[macro_use] -extern crate lazy_static; - use bls::{Keypair, PublicKey, SecretKey}; use ethereum_hashing::hash; +use lazy_static::lazy_static; use num_bigint::BigUint; use serde::{Deserialize, Serialize}; use std::fs::File; diff --git a/common/lighthouse_metrics/src/lib.rs b/common/lighthouse_metrics/src/lib.rs index 5d25bb313f..4a76184b8a 100644 --- a/common/lighthouse_metrics/src/lib.rs +++ b/common/lighthouse_metrics/src/lib.rs @@ -20,8 +20,7 @@ //! ## Example //! //! ```rust -//! #[macro_use] -//! extern crate lazy_static; +//! use lazy_static::lazy_static; //! use lighthouse_metrics::*; //! //! // These metrics are "magically" linked to the global registry defined in `lighthouse_metrics`. diff --git a/common/logging/src/lib.rs b/common/logging/src/lib.rs index b0e1da00e9..50d04fc088 100644 --- a/common/logging/src/lib.rs +++ b/common/logging/src/lib.rs @@ -1,6 +1,4 @@ -#[macro_use] -extern crate lazy_static; - +use lazy_static::lazy_static; use lighthouse_metrics::{ inc_counter, try_create_int_counter, IntCounter, Result as MetricsResult, }; diff --git a/common/logging/src/tracing_metrics_layer.rs b/common/logging/src/tracing_metrics_layer.rs index 08c323ee89..b9dde584b4 100644 --- a/common/logging/src/tracing_metrics_layer.rs +++ b/common/logging/src/tracing_metrics_layer.rs @@ -1,5 +1,6 @@ //! Exposes [`MetricsLayer`]: A tracing layer that registers metrics of logging events. +use lazy_static::lazy_static; use lighthouse_metrics as metrics; use tracing_log::NormalizeEvent; diff --git a/common/slot_clock/src/lib.rs b/common/slot_clock/src/lib.rs index 4f54b2ee76..a742e29457 100644 --- a/common/slot_clock/src/lib.rs +++ b/common/slot_clock/src/lib.rs @@ -1,6 +1,3 @@ -#[macro_use] -extern crate lazy_static; - mod manual_slot_clock; mod metrics; mod system_time_slot_clock; diff --git a/common/slot_clock/src/metrics.rs b/common/slot_clock/src/metrics.rs index 23a793b203..ae3a9b599f 100644 --- a/common/slot_clock/src/metrics.rs +++ b/common/slot_clock/src/metrics.rs @@ -1,4 +1,5 @@ use crate::SlotClock; +use lazy_static::lazy_static; pub use lighthouse_metrics::*; use types::{EthSpec, Slot}; diff --git a/common/test_random_derive/src/lib.rs b/common/test_random_derive/src/lib.rs index 648c20121a..8c4b1ef7c3 100644 --- a/common/test_random_derive/src/lib.rs +++ b/common/test_random_derive/src/lib.rs @@ -1,6 +1,4 @@ -extern crate proc_macro; - -use crate::proc_macro::TokenStream; +use proc_macro::TokenStream; use quote::quote; use syn::{parse_macro_input, DeriveInput}; diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index ebb639819d..5764849975 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -285,17 +285,17 @@ impl ForkChoiceTestDefinition { } } -/// Gives a root that is not the zero hash (unless i is `usize::max_value)`. +/// Gives a root that is not the zero hash (unless i is `usize::MAX)`. fn get_root(i: u64) -> Hash256 { Hash256::from_low_u64_be(i + 1) } -/// Gives a hash that is not the zero hash (unless i is `usize::max_value)`. +/// Gives a hash that is not the zero hash (unless i is `usize::MAX)`. fn get_hash(i: u64) -> ExecutionBlockHash { ExecutionBlockHash::from_root(get_root(i)) } -/// Gives a checkpoint with a root that is not the zero hash (unless i is `usize::max_value)`. +/// Gives a checkpoint with a root that is not the zero hash (unless i is `usize::MAX)`. /// `Epoch` will always equal `i`. fn get_checkpoint(i: u64) -> Checkpoint { Checkpoint { diff --git a/consensus/proto_array/src/fork_choice_test_definition/votes.rs b/consensus/proto_array/src/fork_choice_test_definition/votes.rs index 58ac6af60b..01994fff9b 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/votes.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/votes.rs @@ -738,7 +738,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { // Ensure that pruning below the prune threshold does not prune. ops.push(Operation::Prune { finalized_root: get_root(5), - prune_threshold: usize::max_value(), + prune_threshold: usize::MAX, expected_len: 11, }); diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 7c2ecfe26a..d50153bfe8 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -145,24 +145,24 @@ impl TryInto for ProtoNodeV16 { } } -impl Into for ProtoNode { - fn into(self) -> ProtoNodeV16 { +impl From for ProtoNodeV16 { + fn from(from: ProtoNode) -> ProtoNodeV16 { ProtoNodeV16 { - slot: self.slot, - state_root: self.state_root, - target_root: self.target_root, - current_epoch_shuffling_id: self.current_epoch_shuffling_id, - next_epoch_shuffling_id: self.next_epoch_shuffling_id, - root: self.root, - parent: self.parent, - justified_checkpoint: Some(self.justified_checkpoint), - finalized_checkpoint: Some(self.finalized_checkpoint), - weight: self.weight, - best_child: self.best_child, - best_descendant: self.best_descendant, - execution_status: self.execution_status, - unrealized_justified_checkpoint: self.unrealized_justified_checkpoint, - unrealized_finalized_checkpoint: self.unrealized_finalized_checkpoint, + slot: from.slot, + state_root: from.state_root, + target_root: from.target_root, + current_epoch_shuffling_id: from.current_epoch_shuffling_id, + next_epoch_shuffling_id: from.next_epoch_shuffling_id, + root: from.root, + parent: from.parent, + justified_checkpoint: Some(from.justified_checkpoint), + finalized_checkpoint: Some(from.finalized_checkpoint), + weight: from.weight, + best_child: from.best_child, + best_descendant: from.best_descendant, + execution_status: from.execution_status, + unrealized_justified_checkpoint: from.unrealized_justified_checkpoint, + unrealized_finalized_checkpoint: from.unrealized_finalized_checkpoint, } } } diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index e1faba369f..4b7050df7d 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -995,7 +995,7 @@ mod test_compute_deltas { use super::*; use types::MainnetEthSpec; - /// Gives a hash that is not the zero hash (unless i is `usize::max_value)`. + /// Gives a hash that is not the zero hash (unless i is `usize::MAX)`. fn hash_from_index(i: usize) -> Hash256 { Hash256::from_low_u64_be(i as u64 + 1) } diff --git a/consensus/proto_array/src/ssz_container.rs b/consensus/proto_array/src/ssz_container.rs index 3208584dc4..a6d585758b 100644 --- a/consensus/proto_array/src/ssz_container.rs +++ b/consensus/proto_array/src/ssz_container.rs @@ -55,19 +55,19 @@ impl TryInto for SszContainerV16 { } } -impl Into for SszContainer { - fn into(self) -> SszContainerV16 { - let nodes = self.nodes.into_iter().map(Into::into).collect(); +impl From for SszContainerV16 { + fn from(from: SszContainer) -> SszContainerV16 { + let nodes = from.nodes.into_iter().map(Into::into).collect(); SszContainerV16 { - votes: self.votes, - balances: self.balances, - prune_threshold: self.prune_threshold, - justified_checkpoint: self.justified_checkpoint, - finalized_checkpoint: self.finalized_checkpoint, + votes: from.votes, + balances: from.balances, + prune_threshold: from.prune_threshold, + justified_checkpoint: from.justified_checkpoint, + finalized_checkpoint: from.finalized_checkpoint, nodes, - indices: self.indices, - previous_proposer_boost: self.previous_proposer_boost, + indices: from.indices, + previous_proposer_boost: from.previous_proposer_boost, } } } diff --git a/consensus/safe_arith/src/lib.rs b/consensus/safe_arith/src/lib.rs index c1dbff4c7c..aa397c0603 100644 --- a/consensus/safe_arith/src/lib.rs +++ b/consensus/safe_arith/src/lib.rs @@ -155,12 +155,12 @@ mod test { #[test] fn errors() { - assert!(u32::max_value().safe_add(1).is_err()); - assert!(u32::min_value().safe_sub(1).is_err()); - assert!(u32::max_value().safe_mul(2).is_err()); - assert!(u32::max_value().safe_div(0).is_err()); - assert!(u32::max_value().safe_rem(0).is_err()); - assert!(u32::max_value().safe_shl(32).is_err()); - assert!(u32::max_value().safe_shr(32).is_err()); + assert!(u32::MAX.safe_add(1).is_err()); + assert!(u32::MIN.safe_sub(1).is_err()); + assert!(u32::MAX.safe_mul(2).is_err()); + assert!(u32::MAX.safe_div(0).is_err()); + assert!(u32::MAX.safe_rem(0).is_err()); + assert!(u32::MAX.safe_shl(32).is_err()); + assert!(u32::MAX.safe_shr(32).is_err()); } } diff --git a/consensus/state_processing/src/common/get_attesting_indices.rs b/consensus/state_processing/src/common/get_attesting_indices.rs index a79604003e..b131f7679a 100644 --- a/consensus/state_processing/src/common/get_attesting_indices.rs +++ b/consensus/state_processing/src/common/get_attesting_indices.rs @@ -44,107 +44,15 @@ pub mod attesting_indices_base { } pub mod attesting_indices_electra { - use std::collections::{HashMap, HashSet}; + use std::collections::HashSet; use crate::per_block_processing::errors::{AttestationInvalid as Invalid, BlockOperationError}; - use itertools::Itertools; use safe_arith::SafeArith; use types::*; - // TODO(electra) remove duplicate code - // get_indexed_attestation is almost an exact duplicate - // the only differences are the invalid selection proof - // and aggregator not in committee checks - pub fn get_indexed_attestation_from_signed_aggregate( - committees: &[BeaconCommittee], - signed_aggregate: &SignedAggregateAndProofElectra, - spec: &ChainSpec, - ) -> Result, BeaconStateError> { - let mut output: HashSet = HashSet::new(); - - let committee_bits = &signed_aggregate.message.aggregate.committee_bits; - let aggregation_bits = &signed_aggregate.message.aggregate.aggregation_bits; - let aggregator_index = signed_aggregate.message.aggregator_index; - let attestation = &signed_aggregate.message.aggregate; - - let committee_indices = get_committee_indices::(committee_bits); - - let mut committee_offset = 0; - - let committees_map: HashMap = committees - .iter() - .map(|committee| (committee.index, committee)) - .collect(); - - let committee_count_per_slot = committees.len() as u64; - let mut participant_count = 0; - - // TODO(electra): - // Note: this clones the signature which is known to be a relatively slow operation. - // - // Future optimizations should remove this clone. - let selection_proof = - SelectionProof::from(signed_aggregate.message.selection_proof.clone()); - - for index in committee_indices { - if let Some(&beacon_committee) = committees_map.get(&index) { - if !selection_proof - .is_aggregator(beacon_committee.committee.len(), spec) - .map_err(BeaconStateError::ArithError)? - { - return Err(BeaconStateError::InvalidSelectionProof { aggregator_index }); - } - - if !beacon_committee - .committee - .contains(&(aggregator_index as usize)) - { - return Err(BeaconStateError::AggregatorNotInCommittee { aggregator_index }); - } - - // This check is new to the spec's `process_attestation` in Electra. - if index >= committee_count_per_slot { - return Err(BeaconStateError::InvalidCommitteeIndex(index)); - } - - participant_count.safe_add_assign(beacon_committee.committee.len() as u64)?; - let committee_attesters = beacon_committee - .committee - .iter() - .enumerate() - .filter_map(|(i, &index)| { - if let Ok(aggregation_bit_index) = committee_offset.safe_add(i) { - if aggregation_bits.get(aggregation_bit_index).unwrap_or(false) { - return Some(index as u64); - } - } - None - }) - .collect::>(); - - output.extend(committee_attesters); - - committee_offset.safe_add_assign(beacon_committee.committee.len())?; - } else { - return Err(Error::NoCommitteeFound(index)); - } - } - - // This check is new to the spec's `process_attestation` in Electra. - if participant_count as usize != aggregation_bits.len() { - return Err(Error::InvalidBitfield); - } - - let mut indices = output.into_iter().collect_vec(); - indices.sort_unstable(); - - Ok(IndexedAttestation::Electra(IndexedAttestationElectra { - attesting_indices: VariableList::new(indices)?, - data: attestation.data.clone(), - signature: attestation.signature.clone(), - })) - } - + /// Compute an Electra IndexedAttestation given a list of committees. + /// + /// Committees must be sorted by ascending order 0..committees_per_slot pub fn get_indexed_attestation( committees: &[BeaconCommittee], attestation: &AttestationElectra, @@ -167,17 +75,7 @@ pub mod attesting_indices_electra { attestation: &AttestationElectra, ) -> Result, BlockOperationError> { let committees = beacon_state.get_beacon_committees_at_slot(attestation.data.slot)?; - let attesting_indices = get_attesting_indices::( - &committees, - &attestation.aggregation_bits, - &attestation.committee_bits, - )?; - - Ok(IndexedAttestation::Electra(IndexedAttestationElectra { - attesting_indices: VariableList::new(attesting_indices)?, - data: attestation.data.clone(), - signature: attestation.signature.clone(), - })) + get_indexed_attestation(&committees, attestation) } /// Shortcut for getting the attesting indices while fetching the committee from the state's cache. @@ -190,51 +88,47 @@ pub mod attesting_indices_electra { } /// Returns validator indices which participated in the attestation, sorted by increasing index. + /// + /// Committees must be sorted by ascending order 0..committees_per_slot pub fn get_attesting_indices( committees: &[BeaconCommittee], aggregation_bits: &BitList, committee_bits: &BitVector, ) -> Result, BeaconStateError> { - let mut output: HashSet = HashSet::new(); + let mut attesting_indices = vec![]; let committee_indices = get_committee_indices::(committee_bits); let mut committee_offset = 0; - let committees_map: HashMap = committees - .iter() - .map(|committee| (committee.index, committee)) - .collect(); - let committee_count_per_slot = committees.len() as u64; let mut participant_count = 0; for index in committee_indices { - if let Some(&beacon_committee) = committees_map.get(&index) { - // This check is new to the spec's `process_attestation` in Electra. - if index >= committee_count_per_slot { - return Err(BeaconStateError::InvalidCommitteeIndex(index)); - } - participant_count.safe_add_assign(beacon_committee.committee.len() as u64)?; - let committee_attesters = beacon_committee - .committee - .iter() - .enumerate() - .filter_map(|(i, &index)| { - if let Ok(aggregation_bit_index) = committee_offset.safe_add(i) { - if aggregation_bits.get(aggregation_bit_index).unwrap_or(false) { - return Some(index as u64); - } - } - None - }) - .collect::>(); + let beacon_committee = committees + .get(index as usize) + .ok_or(Error::NoCommitteeFound(index))?; - output.extend(committee_attesters); - - committee_offset.safe_add_assign(beacon_committee.committee.len())?; - } else { - return Err(Error::NoCommitteeFound(index)); + // This check is new to the spec's `process_attestation` in Electra. + if index >= committee_count_per_slot { + return Err(BeaconStateError::InvalidCommitteeIndex(index)); } + participant_count.safe_add_assign(beacon_committee.committee.len() as u64)?; + let committee_attesters = beacon_committee + .committee + .iter() + .enumerate() + .filter_map(|(i, &index)| { + if let Ok(aggregation_bit_index) = committee_offset.safe_add(i) { + if aggregation_bits.get(aggregation_bit_index).unwrap_or(false) { + return Some(index as u64); + } + } + None + }) + .collect::>(); + + attesting_indices.extend(committee_attesters); + committee_offset.safe_add_assign(beacon_committee.committee.len())?; } // This check is new to the spec's `process_attestation` in Electra. @@ -242,10 +136,9 @@ pub mod attesting_indices_electra { return Err(BeaconStateError::InvalidBitfield); } - let mut indices = output.into_iter().collect_vec(); - indices.sort_unstable(); + attesting_indices.sort_unstable(); - Ok(indices) + Ok(attesting_indices) } pub fn get_committee_indices( diff --git a/consensus/state_processing/src/per_block_processing/errors.rs b/consensus/state_processing/src/per_block_processing/errors.rs index 46665a8839..cebb10b607 100644 --- a/consensus/state_processing/src/per_block_processing/errors.rs +++ b/consensus/state_processing/src/per_block_processing/errors.rs @@ -129,6 +129,7 @@ pub enum BlockProcessingError { target_address: Address, }, InavlidConsolidationSignature, + PendingAttestationInElectra, } impl From for BlockProcessingError { 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 09c7c540ed..02cf1d942c 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -91,33 +91,30 @@ pub mod base { ) .map_err(|e| e.into_with_index(i))?; - match attestation { - AttestationRef::Base(att) => { - let pending_attestation = PendingAttestation { - aggregation_bits: att.aggregation_bits.clone(), - data: att.data.clone(), - inclusion_delay: state.slot().safe_sub(att.data.slot)?.as_u64(), - proposer_index, - }; - - if attestation.data().target.epoch == state.current_epoch() { - state - .as_base_mut()? - .current_epoch_attestations - .push(pending_attestation)?; - } else { - state - .as_base_mut()? - .previous_epoch_attestations - .push(pending_attestation)?; - } - } - AttestationRef::Electra(_) => { - // TODO(electra) pending attestations are only phase 0 - // so we should just raise a relevant error here - todo!() - } + let AttestationRef::Base(attestation) = attestation else { + // Pending attestations have been deprecated in a altair, this branch should + // never happen + return Err(BlockProcessingError::PendingAttestationInElectra); }; + + let pending_attestation = PendingAttestation { + aggregation_bits: attestation.aggregation_bits.clone(), + data: attestation.data.clone(), + inclusion_delay: state.slot().safe_sub(attestation.data.slot)?.as_u64(), + proposer_index, + }; + + if attestation.data.target.epoch == state.current_epoch() { + state + .as_base_mut()? + .current_epoch_attestations + .push(pending_attestation)?; + } else { + state + .as_base_mut()? + .previous_epoch_attestations + .push(pending_attestation)?; + } } Ok(()) 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 85dbe4da79..3c683766ad 100644 --- a/consensus/state_processing/src/per_block_processing/signature_sets.rs +++ b/consensus/state_processing/src/per_block_processing/signature_sets.rs @@ -326,7 +326,6 @@ where genesis_validators_root, ); - // TODO(electra), signing root isnt unique in the case of electra let message = indexed_attestation.data().signing_root(domain); Ok(SignatureSet::multiple_pubkeys(signature, pubkeys, message)) diff --git a/consensus/state_processing/src/per_epoch_processing/base/validator_statuses.rs b/consensus/state_processing/src/per_epoch_processing/base/validator_statuses.rs index edf75e7c5e..c5ec80b92a 100644 --- a/consensus/state_processing/src/per_epoch_processing/base/validator_statuses.rs +++ b/consensus/state_processing/src/per_epoch_processing/base/validator_statuses.rs @@ -30,7 +30,7 @@ impl Default for InclusionInfo { /// Defaults to `delay` at its maximum value and `proposer_index` at zero. fn default() -> Self { Self { - delay: u64::max_value(), + delay: u64::MAX, proposer_index: 0, } } diff --git a/consensus/swap_or_not_shuffle/src/compute_shuffled_index.rs b/consensus/swap_or_not_shuffle/src/compute_shuffled_index.rs index e71f3ca18e..5f25c517b0 100644 --- a/consensus/swap_or_not_shuffle/src/compute_shuffled_index.rs +++ b/consensus/swap_or_not_shuffle/src/compute_shuffled_index.rs @@ -17,7 +17,7 @@ use std::cmp::max; /// - `list_size == 0` /// - `index >= list_size` /// - `list_size > 2**24` -/// - `list_size > usize::max_value() / 2` +/// - `list_size > usize::MAX / 2` pub fn compute_shuffled_index( index: usize, list_size: usize, @@ -26,7 +26,7 @@ pub fn compute_shuffled_index( ) -> Option { if list_size == 0 || index >= list_size - || list_size > usize::max_value() / 2 + || list_size > usize::MAX / 2 || list_size > 2_usize.pow(24) { return None; @@ -140,7 +140,7 @@ mod tests { fn returns_none_for_too_large_list() { assert_eq!( None, - compute_shuffled_index(100, usize::max_value() / 2, &[42, 42], 90) + compute_shuffled_index(100, usize::MAX / 2, &[42, 42], 90) ); } } diff --git a/consensus/swap_or_not_shuffle/src/shuffle_list.rs b/consensus/swap_or_not_shuffle/src/shuffle_list.rs index 2b9a256554..b49a26cc37 100644 --- a/consensus/swap_or_not_shuffle/src/shuffle_list.rs +++ b/consensus/swap_or_not_shuffle/src/shuffle_list.rs @@ -75,7 +75,7 @@ impl Buf { /// Returns `None` under any of the following conditions: /// - `list_size == 0` /// - `list_size > 2**24` -/// - `list_size > usize::max_value() / 2` +/// - `list_size > usize::MAX / 2` pub fn shuffle_list( mut input: Vec, rounds: u8, @@ -84,10 +84,7 @@ pub fn shuffle_list( ) -> Option> { let list_size = input.len(); - if input.is_empty() - || list_size > usize::max_value() / 2 - || list_size > 2_usize.pow(24) - || rounds == 0 + if input.is_empty() || list_size > usize::MAX / 2 || list_size > 2_usize.pow(24) || rounds == 0 { return None; } diff --git a/consensus/types/benches/benches.rs b/consensus/types/benches/benches.rs index c6dda142b2..56c48e6cb1 100644 --- a/consensus/types/benches/benches.rs +++ b/consensus/types/benches/benches.rs @@ -36,8 +36,8 @@ fn get_state(validator_count: usize) -> BeaconState { slashed: false, activation_eligibility_epoch: Epoch::new(0), activation_epoch: Epoch::new(0), - exit_epoch: Epoch::from(u64::max_value()), - withdrawable_epoch: Epoch::from(u64::max_value()), + exit_epoch: Epoch::from(u64::MAX), + withdrawable_epoch: Epoch::from(u64::MAX), }) .collect(), ) diff --git a/consensus/types/src/aggregate_and_proof.rs b/consensus/types/src/aggregate_and_proof.rs index e8fa01c143..6ae0df4ad7 100644 --- a/consensus/types/src/aggregate_and_proof.rs +++ b/consensus/types/src/aggregate_and_proof.rs @@ -4,6 +4,7 @@ use super::{ SignedRoot, }; use crate::test_utils::TestRandom; +use crate::Attestation; use serde::{Deserialize, Serialize}; use ssz_derive::{Decode, Encode}; use superstruct::superstruct; @@ -30,7 +31,7 @@ use tree_hash_derive::TreeHash; ), ref_attributes( derive(Debug, PartialEq, TreeHash, Serialize,), - serde(bound = "E: EthSpec"), + serde(untagged, bound = "E: EthSpec"), tree_hash(enum_behaviour = "transparent") ) )] @@ -88,28 +89,39 @@ impl AggregateAndProof { genesis_validators_root: Hash256, spec: &ChainSpec, ) -> Self { - let selection_proof = selection_proof - .unwrap_or_else(|| { - SelectionProof::new::( - aggregate.data().slot, - secret_key, - fork, - genesis_validators_root, - spec, - ) - }) - .into(); + let selection_proof = selection_proof.unwrap_or_else(|| { + SelectionProof::new::( + aggregate.data().slot, + secret_key, + fork, + genesis_validators_root, + spec, + ) + }); + Self::from_attestation( + aggregator_index, + aggregate.clone_as_attestation(), + selection_proof, + ) + } + + /// Produces a new `AggregateAndProof` given a `selection_proof` + pub fn from_attestation( + aggregator_index: u64, + aggregate: Attestation, + selection_proof: SelectionProof, + ) -> Self { match aggregate { - AttestationRef::Base(attestation) => Self::Base(AggregateAndProofBase { + Attestation::Base(aggregate) => Self::Base(AggregateAndProofBase { aggregator_index, - aggregate: attestation.clone(), - selection_proof, + aggregate, + selection_proof: selection_proof.into(), }), - AttestationRef::Electra(attestation) => Self::Electra(AggregateAndProofElectra { + Attestation::Electra(aggregate) => Self::Electra(AggregateAndProofElectra { aggregator_index, - aggregate: attestation.clone(), - selection_proof, + aggregate, + selection_proof: selection_proof.into(), }), } } diff --git a/consensus/types/src/attestation.rs b/consensus/types/src/attestation.rs index 3df14feade..c8d1c3fb9b 100644 --- a/consensus/types/src/attestation.rs +++ b/consensus/types/src/attestation.rs @@ -1,7 +1,7 @@ use crate::slot_data::SlotData; use crate::{test_utils::TestRandom, Hash256, Slot}; +use crate::{Checkpoint, ForkName}; use derivative::Derivative; -use rand::RngCore; use safe_arith::ArithError; use serde::{Deserialize, Serialize}; use ssz_derive::{Decode, Encode}; @@ -22,6 +22,8 @@ pub enum Error { AlreadySigned(usize), SubnetCountIsZero(ArithError), IncorrectStateVariant, + InvalidCommitteeLength, + InvalidCommitteeIndex, } #[superstruct( @@ -74,23 +76,6 @@ pub struct Attestation { pub signature: AggregateSignature, } -// TODO(electra): think about how to handle fork variants here -impl TestRandom for Attestation { - fn random_for_test(rng: &mut impl RngCore) -> Self { - let aggregation_bits = BitList::random_for_test(rng); - let data = AttestationData::random_for_test(rng); - let signature = AggregateSignature::random_for_test(rng); - let committee_bits = BitVector::random_for_test(rng); - - Self::Electra(AttestationElectra { - aggregation_bits, - committee_bits, - data, - signature, - }) - } -} - impl Hash for Attestation { fn hash(&self, state: &mut H) where @@ -104,6 +89,50 @@ impl Hash for Attestation { } impl Attestation { + /// Produces an attestation with empty signature. + pub fn empty_for_signing( + committee_index: u64, + committee_length: usize, + slot: Slot, + beacon_block_root: Hash256, + source: Checkpoint, + target: Checkpoint, + spec: &ChainSpec, + ) -> Result { + if spec.fork_name_at_slot::(slot) >= ForkName::Electra { + let mut committee_bits: BitVector = BitVector::default(); + committee_bits + .set(committee_index as usize, true) + .map_err(|_| Error::InvalidCommitteeIndex)?; + Ok(Attestation::Electra(AttestationElectra { + aggregation_bits: BitList::with_capacity(committee_length) + .map_err(|_| Error::InvalidCommitteeLength)?, + data: AttestationData { + slot, + index: 0u64, + beacon_block_root, + source, + target, + }, + committee_bits, + signature: AggregateSignature::infinity(), + })) + } else { + Ok(Attestation::Base(AttestationBase { + aggregation_bits: BitList::with_capacity(committee_length) + .map_err(|_| Error::InvalidCommitteeLength)?, + data: AttestationData { + slot, + index: committee_index, + beacon_block_root, + source, + target, + }, + signature: AggregateSignature::infinity(), + })) + } + } + /// Aggregate another Attestation into this one. /// /// The aggregation bitfields must be disjoint, and the data must be the same. diff --git a/consensus/types/src/beacon_block_body.rs b/consensus/types/src/beacon_block_body.rs index 76a2fc1872..9cf66184f8 100644 --- a/consensus/types/src/beacon_block_body.rs +++ b/consensus/types/src/beacon_block_body.rs @@ -820,6 +820,11 @@ impl From>> } impl BeaconBlockBody { + /// Returns the name of the fork pertaining to `self`. + pub fn fork_name(&self) -> ForkName { + self.to_ref().fork_name() + } + pub fn block_body_merkle_proof(&self, generalized_index: usize) -> Result, Error> { let field_index = match generalized_index { light_client_update::EXECUTION_PAYLOAD_INDEX => { @@ -834,22 +839,16 @@ impl BeaconBlockBody { _ => return Err(Error::IndexNotSupported(generalized_index)), }; - let attestations_root = match self { - BeaconBlockBody::Base(_) - | BeaconBlockBody::Altair(_) - | BeaconBlockBody::Bellatrix(_) - | BeaconBlockBody::Capella(_) - | BeaconBlockBody::Deneb(_) => self.attestations_base()?.tree_hash_root(), - BeaconBlockBody::Electra(_) => self.attestations_electra()?.tree_hash_root(), + let attestations_root = if self.fork_name() > ForkName::Electra { + self.attestations_electra()?.tree_hash_root() + } else { + self.attestations_base()?.tree_hash_root() }; - let attester_slashings_root = match self { - BeaconBlockBody::Base(_) - | BeaconBlockBody::Altair(_) - | BeaconBlockBody::Bellatrix(_) - | BeaconBlockBody::Capella(_) - | BeaconBlockBody::Deneb(_) => self.attester_slashings_base()?.tree_hash_root(), - BeaconBlockBody::Electra(_) => self.attester_slashings_electra()?.tree_hash_root(), + let attester_slashings_root = if self.fork_name() > ForkName::Electra { + self.attester_slashings_electra()?.tree_hash_root() + } else { + self.attester_slashings_base()?.tree_hash_root() }; let mut leaves = vec![ diff --git a/consensus/types/src/beacon_state/committee_cache.rs b/consensus/types/src/beacon_state/committee_cache.rs index 7913df8e00..161f854157 100644 --- a/consensus/types/src/beacon_state/committee_cache.rs +++ b/consensus/types/src/beacon_state/committee_cache.rs @@ -86,7 +86,7 @@ impl CommitteeCache { } // The use of `NonZeroUsize` reduces the maximum number of possible validators by one. - if state.validators().len() == usize::max_value() { + if state.validators().len() == usize::MAX { return Err(Error::TooManyValidators); } @@ -183,6 +183,8 @@ impl CommitteeCache { } /// Get all the Beacon committees at a given `slot`. + /// + /// Committees are sorted by ascending index order 0..committees_per_slot pub fn get_beacon_committees_at_slot(&self, slot: Slot) -> Result, Error> { if self.initialized_epoch.is_none() { return Err(Error::CommitteeCacheUninitialized(None)); diff --git a/consensus/types/src/beacon_state/committee_cache/tests.rs b/consensus/types/src/beacon_state/committee_cache/tests.rs index a227476569..4dc06feab3 100644 --- a/consensus/types/src/beacon_state/committee_cache/tests.rs +++ b/consensus/types/src/beacon_state/committee_cache/tests.rs @@ -2,6 +2,7 @@ use crate::test_utils::*; use beacon_chain::test_utils::{BeaconChainHarness, EphemeralHarnessType}; use beacon_chain::types::*; +use lazy_static::lazy_static; use swap_or_not_shuffle::shuffle_list; pub const VALIDATOR_COUNT: usize = 16; diff --git a/consensus/types/src/beacon_state/tests.rs b/consensus/types/src/beacon_state/tests.rs index 38a76e44c5..16c7ff152f 100644 --- a/consensus/types/src/beacon_state/tests.rs +++ b/consensus/types/src/beacon_state/tests.rs @@ -6,6 +6,7 @@ use beacon_chain::types::{ ChainSpec, Domain, Epoch, EthSpec, Hash256, Keypair, MainnetEthSpec, MinimalEthSpec, RelativeEpoch, Slot, Vector, }; +use lazy_static::lazy_static; use ssz::Encode; use std::ops::Mul; use swap_or_not_shuffle::compute_shuffled_index; diff --git a/consensus/types/src/blob_sidecar.rs b/consensus/types/src/blob_sidecar.rs index e54bc2f4f9..1f60f429db 100644 --- a/consensus/types/src/blob_sidecar.rs +++ b/consensus/types/src/blob_sidecar.rs @@ -6,13 +6,10 @@ use crate::{ use crate::{KzgProofs, SignedBeaconBlock}; use bls::Signature; use derivative::Derivative; -use kzg::{ - Blob as KzgBlob, Kzg, KzgCommitment, KzgProof, BYTES_PER_BLOB, BYTES_PER_FIELD_ELEMENT, - FIELD_ELEMENTS_PER_BLOB, -}; +use kzg::{Blob as KzgBlob, Kzg, KzgCommitment, KzgProof, BYTES_PER_BLOB, BYTES_PER_FIELD_ELEMENT}; use merkle_proof::{merkle_root_from_branch, verify_merkle_proof, MerkleTreeError}; use rand::Rng; -use safe_arith::{ArithError, SafeArith}; +use safe_arith::ArithError; use serde::{Deserialize, Serialize}; use ssz::Encode; use ssz_derive::{Decode, Encode}; @@ -190,33 +187,30 @@ impl BlobSidecar { } /// Verifies the kzg commitment inclusion merkle proof. - pub fn verify_blob_sidecar_inclusion_proof(&self) -> Result { - // Depth of the subtree rooted at `blob_kzg_commitments` in the `BeaconBlockBody` - // is equal to depth of the ssz List max size + 1 for the length mixin - let kzg_commitments_tree_depth = (E::max_blob_commitments_per_block() - .next_power_of_two() - .ilog2() - .safe_add(1))? as usize; + pub fn verify_blob_sidecar_inclusion_proof(&self) -> bool { + let kzg_commitments_tree_depth = E::kzg_commitments_tree_depth(); + + // EthSpec asserts that kzg_commitments_tree_depth is less than KzgCommitmentInclusionProofDepth + let (kzg_commitment_subtree_proof, kzg_commitments_proof) = self + .kzg_commitment_inclusion_proof + .split_at(kzg_commitments_tree_depth); + // Compute the `tree_hash_root` of the `blob_kzg_commitments` subtree using the // inclusion proof branches let blob_kzg_commitments_root = merkle_root_from_branch( self.kzg_commitment.tree_hash_root(), - self.kzg_commitment_inclusion_proof - .get(0..kzg_commitments_tree_depth) - .ok_or(MerkleTreeError::PleaseNotifyTheDevs)?, + kzg_commitment_subtree_proof, kzg_commitments_tree_depth, self.index as usize, ); // The remaining inclusion proof branches are for the top level `BeaconBlockBody` tree - Ok(verify_merkle_proof( + verify_merkle_proof( blob_kzg_commitments_root, - self.kzg_commitment_inclusion_proof - .get(kzg_commitments_tree_depth..E::kzg_proof_inclusion_proof_depth()) - .ok_or(MerkleTreeError::PleaseNotifyTheDevs)?, - E::kzg_proof_inclusion_proof_depth().safe_sub(kzg_commitments_tree_depth)?, + kzg_commitments_proof, + E::block_body_tree_depth(), BLOB_KZG_COMMITMENTS_INDEX, self.signed_block_header.message.body_root, - )) + ) } pub fn random_valid(rng: &mut R, kzg: &Kzg) -> Result { @@ -224,13 +218,7 @@ impl BlobSidecar { rng.fill_bytes(&mut blob_bytes); // Ensure that the blob is canonical by ensuring that // each field element contained in the blob is < BLS_MODULUS - for i in 0..FIELD_ELEMENTS_PER_BLOB { - let Some(byte) = blob_bytes.get_mut( - i.checked_mul(BYTES_PER_FIELD_ELEMENT) - .ok_or("overflow".to_string())?, - ) else { - return Err(format!("blob byte index out of bounds: {:?}", i)); - }; + for byte in blob_bytes.iter_mut().step_by(BYTES_PER_FIELD_ELEMENT) { *byte = 0; } diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index 81b2fecf1e..a8c087d6d2 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -376,7 +376,7 @@ impl ChainSpec { state: &BeaconState, ) -> u64 { let fork_name = state.fork_name_unchecked(); - if fork_name >= ForkName::Electra { + if fork_name.electra_enabled() { self.min_slashing_penalty_quotient_electra } else if fork_name >= ForkName::Bellatrix { self.min_slashing_penalty_quotient_bellatrix diff --git a/consensus/types/src/eth_spec.rs b/consensus/types/src/eth_spec.rs index 14949e6753..1c379f5de4 100644 --- a/consensus/types/src/eth_spec.rs +++ b/consensus/types/src/eth_spec.rs @@ -294,6 +294,22 @@ pub trait EthSpec: Self::KzgCommitmentInclusionProofDepth::to_usize() } + fn kzg_commitments_tree_depth() -> usize { + // Depth of the subtree rooted at `blob_kzg_commitments` in the `BeaconBlockBody` + // is equal to depth of the ssz List max size + 1 for the length mixin + Self::max_blob_commitments_per_block() + .next_power_of_two() + .ilog2() + .safe_add(1) + .expect("The log of max_blob_commitments_per_block can not overflow") as usize + } + + fn block_body_tree_depth() -> usize { + Self::kzg_proof_inclusion_proof_depth() + .safe_sub(Self::kzg_commitments_tree_depth()) + .expect("Preset values are not configurable and never result in non-positive block body depth") + } + /// Returns the `PENDING_BALANCE_DEPOSITS_LIMIT` constant for this specification. fn pending_balance_deposits_limit() -> usize { Self::PendingBalanceDepositsLimit::to_usize() @@ -525,3 +541,28 @@ impl EthSpec for GnosisEthSpec { EthSpecId::Gnosis } } + +#[cfg(test)] +mod test { + use crate::{EthSpec, GnosisEthSpec, MainnetEthSpec, MinimalEthSpec}; + use ssz_types::typenum::Unsigned; + + fn assert_valid_spec() { + E::kzg_commitments_tree_depth(); + E::block_body_tree_depth(); + assert!(E::MaxValidatorsPerSlot::to_i32() >= E::MaxValidatorsPerCommittee::to_i32()); + } + + #[test] + fn mainnet_spec() { + assert_valid_spec::(); + } + #[test] + fn minimal_spec() { + assert_valid_spec::(); + } + #[test] + fn gnosis_spec() { + assert_valid_spec::(); + } +} diff --git a/consensus/types/src/fork_name.rs b/consensus/types/src/fork_name.rs index 5cc6621473..f0810e2bdb 100644 --- a/consensus/types/src/fork_name.rs +++ b/consensus/types/src/fork_name.rs @@ -119,6 +119,10 @@ impl ForkName { ForkName::Electra => None, } } + + pub fn electra_enabled(self) -> bool { + self >= ForkName::Electra + } } /// Map a fork name into a fork-versioned superstruct type like `BeaconBlock`. diff --git a/consensus/types/src/graffiti.rs b/consensus/types/src/graffiti.rs index c7a0081c9f..3d8f411caf 100644 --- a/consensus/types/src/graffiti.rs +++ b/consensus/types/src/graffiti.rs @@ -37,9 +37,9 @@ impl From<[u8; GRAFFITI_BYTES_LEN]> for Graffiti { } } -impl Into<[u8; GRAFFITI_BYTES_LEN]> for Graffiti { - fn into(self) -> [u8; GRAFFITI_BYTES_LEN] { - self.0 +impl From for [u8; GRAFFITI_BYTES_LEN] { + fn from(from: Graffiti) -> [u8; GRAFFITI_BYTES_LEN] { + from.0 } } @@ -77,9 +77,9 @@ impl<'de> Deserialize<'de> for GraffitiString { } } -impl Into for GraffitiString { - fn into(self) -> Graffiti { - let graffiti_bytes = self.0.as_bytes(); +impl From for Graffiti { + fn from(from: GraffitiString) -> Graffiti { + let graffiti_bytes = from.0.as_bytes(); let mut graffiti = [0; GRAFFITI_BYTES_LEN]; let graffiti_len = std::cmp::min(graffiti_bytes.len(), GRAFFITI_BYTES_LEN); diff --git a/consensus/types/src/indexed_attestation.rs b/consensus/types/src/indexed_attestation.rs index d282e2f259..900c905938 100644 --- a/consensus/types/src/indexed_attestation.rs +++ b/consensus/types/src/indexed_attestation.rs @@ -1,9 +1,7 @@ use crate::{test_utils::TestRandom, AggregateSignature, AttestationData, EthSpec, VariableList}; use core::slice::Iter; use derivative::Derivative; -use rand::RngCore; use serde::{Deserialize, Serialize}; -use ssz::Decode; use ssz::Encode; use ssz_derive::{Decode, Encode}; use std::hash::{Hash, Hasher}; @@ -116,11 +114,14 @@ impl IndexedAttestation { } } - pub fn to_electra(self) -> Result, ssz_types::Error> { - Ok(match self { + pub fn to_electra(self) -> IndexedAttestationElectra { + match self { Self::Base(att) => { let extended_attesting_indices: VariableList = - VariableList::new(att.attesting_indices.to_vec())?; + VariableList::new(att.attesting_indices.to_vec()) + .expect("MaxValidatorsPerSlot must be >= MaxValidatorsPerCommittee"); + // Note a unit test in consensus/types/src/eth_spec.rs asserts this invariant for + // all known specs IndexedAttestationElectra { attesting_indices: extended_attesting_indices, @@ -129,7 +130,7 @@ impl IndexedAttestation { } } Self::Electra(att) => att, - }) + } } } @@ -186,43 +187,6 @@ impl<'a, E: EthSpec> IndexedAttestationRef<'a, E> { } } -impl Decode for IndexedAttestation { - fn is_ssz_fixed_len() -> bool { - false - } - - fn from_ssz_bytes(bytes: &[u8]) -> Result { - if let Ok(result) = IndexedAttestationBase::from_ssz_bytes(bytes) { - return Ok(IndexedAttestation::Base(result)); - } - - if let Ok(result) = IndexedAttestationElectra::from_ssz_bytes(bytes) { - return Ok(IndexedAttestation::Electra(result)); - } - - Err(ssz::DecodeError::BytesInvalid(String::from( - "bytes not valid for any fork variant", - ))) - } -} - -// TODO(electra): think about how to handle fork variants here -impl TestRandom for IndexedAttestation { - fn random_for_test(rng: &mut impl RngCore) -> Self { - let attesting_indices = VariableList::random_for_test(rng); - // let committee_bits: BitList = BitList::random_for_test(rng); - let data = AttestationData::random_for_test(rng); - let signature = AggregateSignature::random_for_test(rng); - - Self::Base(IndexedAttestationBase { - attesting_indices, - // committee_bits, - data, - signature, - }) - } -} - /// Implementation of non-crypto-secure `Hash`, for use with `HashMap` and `HashSet`. /// /// Guarantees `att1 == att2 -> hash(att1) == hash(att2)`. @@ -276,6 +240,38 @@ mod quoted_variable_list_u64 { } } +#[derive(Debug, Clone, Encode, Decode, PartialEq)] +#[ssz(enum_behaviour = "union")] +pub enum IndexedAttestationOnDisk { + Base(IndexedAttestationBase), + Electra(IndexedAttestationElectra), +} + +#[derive(Debug, Clone, Encode, PartialEq)] +#[ssz(enum_behaviour = "union")] +pub enum IndexedAttestationRefOnDisk<'a, E: EthSpec> { + Base(&'a IndexedAttestationBase), + Electra(&'a IndexedAttestationElectra), +} + +impl<'a, E: EthSpec> From<&'a IndexedAttestation> for IndexedAttestationRefOnDisk<'a, E> { + fn from(attestation: &'a IndexedAttestation) -> Self { + match attestation { + IndexedAttestation::Base(attestation) => Self::Base(attestation), + IndexedAttestation::Electra(attestation) => Self::Electra(attestation), + } + } +} + +impl From> for IndexedAttestation { + fn from(attestation: IndexedAttestationOnDisk) -> Self { + match attestation { + IndexedAttestationOnDisk::Base(attestation) => Self::Base(attestation), + IndexedAttestationOnDisk::Electra(attestation) => Self::Electra(attestation), + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -331,14 +327,22 @@ mod tests { assert!(!indexed_vote_first.is_surround_vote(&indexed_vote_second)); } - ssz_and_tree_hash_tests!(IndexedAttestation); + mod base { + use super::*; + ssz_and_tree_hash_tests!(IndexedAttestationBase); + } + mod electra { + use super::*; + ssz_and_tree_hash_tests!(IndexedAttestationElectra); + } fn create_indexed_attestation( target_epoch: u64, source_epoch: u64, ) -> IndexedAttestation { let mut rng = XorShiftRng::from_seed([42; 16]); - let mut indexed_vote = IndexedAttestation::random_for_test(&mut rng); + let mut indexed_vote = + IndexedAttestation::Base(IndexedAttestationBase::random_for_test(&mut rng)); indexed_vote.data_mut().source.epoch = Epoch::new(source_epoch); indexed_vote.data_mut().target.epoch = Epoch::new(target_epoch); diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index 716c831478..2a73ff0f37 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -9,8 +9,6 @@ ) )] -#[macro_use] -extern crate lazy_static; #[macro_use] pub mod test_utils; @@ -178,7 +176,8 @@ pub use crate::fork_versioned_response::{ForkVersionDeserialize, ForkVersionedRe pub use crate::graffiti::{Graffiti, GRAFFITI_BYTES_LEN}; pub use crate::historical_batch::HistoricalBatch; pub use crate::indexed_attestation::{ - IndexedAttestation, IndexedAttestationBase, IndexedAttestationElectra, IndexedAttestationRef, + IndexedAttestation, IndexedAttestationBase, IndexedAttestationElectra, + IndexedAttestationOnDisk, IndexedAttestationRef, IndexedAttestationRefOnDisk, }; pub use crate::light_client_bootstrap::{ LightClientBootstrap, LightClientBootstrapAltair, LightClientBootstrapCapella, diff --git a/consensus/types/src/selection_proof.rs b/consensus/types/src/selection_proof.rs index cd9f9c06d0..c80a00c3d1 100644 --- a/consensus/types/src/selection_proof.rs +++ b/consensus/types/src/selection_proof.rs @@ -77,9 +77,9 @@ impl SelectionProof { } } -impl Into for SelectionProof { - fn into(self) -> Signature { - self.0 +impl From for Signature { + fn from(from: SelectionProof) -> Signature { + from.0 } } diff --git a/consensus/types/src/signed_aggregate_and_proof.rs b/consensus/types/src/signed_aggregate_and_proof.rs index ddf1dedb04..d339cecaae 100644 --- a/consensus/types/src/signed_aggregate_and_proof.rs +++ b/consensus/types/src/signed_aggregate_and_proof.rs @@ -6,6 +6,7 @@ use super::{ Signature, SignedRoot, }; use crate::test_utils::TestRandom; +use crate::Attestation; use serde::{Deserialize, Serialize}; use ssz_derive::{Decode, Encode}; use superstruct::superstruct; @@ -83,17 +84,19 @@ impl SignedAggregateAndProof { ); let signing_message = message.signing_root(domain); - match message { + Self::from_aggregate_and_proof(message, secret_key.sign(signing_message)) + } + + /// Produces a new `SignedAggregateAndProof` given a `signature` of `aggregate` + pub fn from_aggregate_and_proof(aggregate: AggregateAndProof, signature: Signature) -> Self { + match aggregate { AggregateAndProof::Base(message) => { - SignedAggregateAndProof::Base(SignedAggregateAndProofBase { - message, - signature: secret_key.sign(signing_message), - }) + SignedAggregateAndProof::Base(SignedAggregateAndProofBase { message, signature }) } AggregateAndProof::Electra(message) => { SignedAggregateAndProof::Electra(SignedAggregateAndProofElectra { message, - signature: secret_key.sign(signing_message), + signature, }) } } @@ -107,4 +110,11 @@ impl SignedAggregateAndProof { } } } + + pub fn into_attestation(self) -> Attestation { + match self { + Self::Base(att) => Attestation::Base(att.message.aggregate), + Self::Electra(att) => Attestation::Electra(att.message.aggregate), + } + } } diff --git a/consensus/types/src/slot_epoch.rs b/consensus/types/src/slot_epoch.rs index 79d0064911..8c8f2d073d 100644 --- a/consensus/types/src/slot_epoch.rs +++ b/consensus/types/src/slot_epoch.rs @@ -70,7 +70,7 @@ impl Slot { } pub fn max_value() -> Slot { - Slot(u64::max_value()) + Slot(u64::MAX) } } @@ -80,7 +80,7 @@ impl Epoch { } pub fn max_value() -> Epoch { - Epoch(u64::max_value()) + Epoch(u64::MAX) } /// The first slot in the epoch. @@ -176,10 +176,10 @@ mod epoch_tests { let slots_per_epoch = 32; // The last epoch which can be represented by u64. - let epoch = Epoch::new(u64::max_value() / slots_per_epoch); + let epoch = Epoch::new(u64::MAX / slots_per_epoch); // A slot number on the epoch should be equal to u64::max_value. - assert_eq!(epoch.end_slot(slots_per_epoch), Slot::new(u64::max_value())); + assert_eq!(epoch.end_slot(slots_per_epoch), Slot::new(u64::MAX)); } #[test] diff --git a/consensus/types/src/slot_epoch_macros.rs b/consensus/types/src/slot_epoch_macros.rs index fafc455ef8..42e7a0f2ee 100644 --- a/consensus/types/src/slot_epoch_macros.rs +++ b/consensus/types/src/slot_epoch_macros.rs @@ -6,9 +6,9 @@ macro_rules! impl_from_into_u64 { } } - impl Into for $main { - fn into(self) -> u64 { - self.0 + impl From<$main> for u64 { + fn from(from: $main) -> u64 { + from.0 } } @@ -28,9 +28,9 @@ macro_rules! impl_from_into_usize { } } - impl Into for $main { - fn into(self) -> usize { - self.0 as usize + impl From<$main> for usize { + fn from(from: $main) -> usize { + from.0 as usize } } @@ -352,7 +352,7 @@ macro_rules! new_tests { fn new() { assert_eq!($type(0), $type::new(0)); assert_eq!($type(3), $type::new(3)); - assert_eq!($type(u64::max_value()), $type::new(u64::max_value())); + assert_eq!($type(u64::MAX), $type::new(u64::MAX)); } }; } @@ -368,17 +368,17 @@ macro_rules! from_into_tests { let x: $other = $type(3).into(); assert_eq!(x, 3); - let x: $other = $type(u64::max_value()).into(); + let x: $other = $type(u64::MAX).into(); // Note: this will fail on 32 bit systems. This is expected as we don't have a proper // 32-bit system strategy in place. - assert_eq!(x, $other::max_value()); + assert_eq!(x, $other::MAX); } #[test] fn from() { assert_eq!($type(0), $type::from(0_u64)); assert_eq!($type(3), $type::from(3_u64)); - assert_eq!($type(u64::max_value()), $type::from($other::max_value())); + assert_eq!($type(u64::MAX), $type::from($other::MAX)); } }; } @@ -396,8 +396,8 @@ macro_rules! math_between_tests { assert_partial_ord(1, Ordering::Less, 2); assert_partial_ord(2, Ordering::Greater, 1); - assert_partial_ord(0, Ordering::Less, u64::max_value()); - assert_partial_ord(u64::max_value(), Ordering::Greater, 0); + assert_partial_ord(0, Ordering::Less, u64::MAX); + assert_partial_ord(u64::MAX, Ordering::Greater, 0); } #[test] @@ -412,9 +412,9 @@ macro_rules! math_between_tests { assert_partial_eq(1, 0, false); assert_partial_eq(1, 1, true); - assert_partial_eq(u64::max_value(), u64::max_value(), true); - assert_partial_eq(0, u64::max_value(), false); - assert_partial_eq(u64::max_value(), 0, false); + assert_partial_eq(u64::MAX, u64::MAX, true); + assert_partial_eq(0, u64::MAX, false); + assert_partial_eq(u64::MAX, 0, false); } #[test] @@ -436,8 +436,8 @@ macro_rules! math_between_tests { assert_add(7, 7, 14); // Addition should be saturating. - assert_add(u64::max_value(), 1, u64::max_value()); - assert_add(u64::max_value(), u64::max_value(), u64::max_value()); + assert_add(u64::MAX, 1, u64::MAX); + assert_add(u64::MAX, u64::MAX, u64::MAX); } #[test] @@ -455,8 +455,8 @@ macro_rules! math_between_tests { assert_sub(1, 0, 1); assert_sub(2, 1, 1); assert_sub(14, 7, 7); - assert_sub(u64::max_value(), 1, u64::max_value() - 1); - assert_sub(u64::max_value(), u64::max_value(), 0); + assert_sub(u64::MAX, 1, u64::MAX - 1); + assert_sub(u64::MAX, u64::MAX, 0); // Subtraction should be saturating assert_sub(0, 1, 0); @@ -480,7 +480,7 @@ macro_rules! math_between_tests { assert_mul(0, 2, 0); // Multiplication should be saturating. - assert_mul(u64::max_value(), 2, u64::max_value()); + assert_mul(u64::MAX, 2, u64::MAX); } #[test] @@ -499,7 +499,7 @@ macro_rules! math_between_tests { assert_div(2, 2, 1); assert_div(100, 50, 2); assert_div(128, 2, 64); - assert_div(u64::max_value(), 2, 2_u64.pow(63) - 1); + assert_div(u64::MAX, 2, 2_u64.pow(63) - 1); } #[test] @@ -544,8 +544,8 @@ macro_rules! math_tests { assert_saturating_sub(1, 0, 1); assert_saturating_sub(2, 1, 1); assert_saturating_sub(14, 7, 7); - assert_saturating_sub(u64::max_value(), 1, u64::max_value() - 1); - assert_saturating_sub(u64::max_value(), u64::max_value(), 0); + assert_saturating_sub(u64::MAX, 1, u64::MAX - 1); + assert_saturating_sub(u64::MAX, u64::MAX, 0); // Subtraction should be saturating assert_saturating_sub(0, 1, 0); @@ -565,8 +565,8 @@ macro_rules! math_tests { assert_saturating_add(7, 7, 14); // Addition should be saturating. - assert_saturating_add(u64::max_value(), 1, u64::max_value()); - assert_saturating_add(u64::max_value(), u64::max_value(), u64::max_value()); + assert_saturating_add(u64::MAX, 1, u64::MAX); + assert_saturating_add(u64::MAX, u64::MAX, u64::MAX); } #[test] @@ -581,11 +581,11 @@ macro_rules! math_tests { assert_checked_div(2, 2, Some(1)); assert_checked_div(100, 50, Some(2)); assert_checked_div(128, 2, Some(64)); - assert_checked_div(u64::max_value(), 2, Some(2_u64.pow(63) - 1)); + assert_checked_div(u64::MAX, 2, Some(2_u64.pow(63) - 1)); assert_checked_div(2, 0, None); assert_checked_div(0, 0, None); - assert_checked_div(u64::max_value(), 0, None); + assert_checked_div(u64::MAX, 0, None); } #[test] @@ -607,7 +607,7 @@ macro_rules! math_tests { assert_is_power_of_two(4, true); assert_is_power_of_two(2_u64.pow(4), true); - assert_is_power_of_two(u64::max_value(), false); + assert_is_power_of_two(u64::MAX, false); } #[test] @@ -619,8 +619,8 @@ macro_rules! math_tests { assert_ord(1, Ordering::Less, 2); assert_ord(2, Ordering::Greater, 1); - assert_ord(0, Ordering::Less, u64::max_value()); - assert_ord(u64::max_value(), Ordering::Greater, 0); + assert_ord(0, Ordering::Less, u64::MAX); + assert_ord(u64::MAX, Ordering::Greater, 0); } }; } @@ -647,8 +647,8 @@ macro_rules! all_tests { let x = $type(3).as_u64(); assert_eq!(x, 3); - let x = $type(u64::max_value()).as_u64(); - assert_eq!(x, u64::max_value()); + let x = $type(u64::MAX).as_u64(); + assert_eq!(x, u64::MAX); } } @@ -665,8 +665,8 @@ macro_rules! all_tests { let x = $type(3).as_usize(); assert_eq!(x, 3); - let x = $type(u64::max_value()).as_usize(); - assert_eq!(x, usize::max_value()); + let x = $type(u64::MAX).as_usize(); + assert_eq!(x, usize::MAX); } } }; diff --git a/consensus/types/src/subnet_id.rs b/consensus/types/src/subnet_id.rs index b5b05e710b..66786b5129 100644 --- a/consensus/types/src/subnet_id.rs +++ b/consensus/types/src/subnet_id.rs @@ -1,5 +1,6 @@ //! Identifies each shard by an integer identifier. use crate::{AttestationRef, ChainSpec, CommitteeIndex, Epoch, EthSpec, Slot}; +use lazy_static::lazy_static; use safe_arith::{ArithError, SafeArith}; use serde::{Deserialize, Serialize}; use std::ops::{Deref, DerefMut}; @@ -151,15 +152,15 @@ impl From for SubnetId { } } -impl Into for SubnetId { - fn into(self) -> u64 { - self.0 +impl From for u64 { + fn from(from: SubnetId) -> u64 { + from.0 } } -impl Into for &SubnetId { - fn into(self) -> u64 { - self.0 +impl From<&SubnetId> for u64 { + fn from(from: &SubnetId) -> u64 { + from.0 } } diff --git a/consensus/types/src/sync_selection_proof.rs b/consensus/types/src/sync_selection_proof.rs index da7370a0c1..0b32c6981b 100644 --- a/consensus/types/src/sync_selection_proof.rs +++ b/consensus/types/src/sync_selection_proof.rs @@ -90,9 +90,9 @@ impl SyncSelectionProof { } } -impl Into for SyncSelectionProof { - fn into(self) -> Signature { - self.0 +impl From for Signature { + fn from(from: SyncSelectionProof) -> Signature { + from.0 } } diff --git a/consensus/types/src/sync_subnet_id.rs b/consensus/types/src/sync_subnet_id.rs index dd0807f21c..7806aecfca 100644 --- a/consensus/types/src/sync_subnet_id.rs +++ b/consensus/types/src/sync_subnet_id.rs @@ -1,6 +1,7 @@ //! Identifies each sync committee subnet by an integer identifier. use crate::consts::altair::SYNC_COMMITTEE_SUBNET_COUNT; use crate::EthSpec; +use lazy_static::lazy_static; use safe_arith::{ArithError, SafeArith}; use serde::{Deserialize, Serialize}; use ssz_types::typenum::Unsigned; @@ -77,15 +78,15 @@ impl From for SyncSubnetId { } } -impl Into for SyncSubnetId { - fn into(self) -> u64 { - self.0 +impl From for u64 { + fn from(from: SyncSubnetId) -> u64 { + from.0 } } -impl Into for &SyncSubnetId { - fn into(self) -> u64 { - self.0 +impl From<&SyncSubnetId> for u64 { + fn from(from: &SyncSubnetId) -> u64 { + from.0 } } diff --git a/consensus/types/src/test_utils/generate_random_block_and_blobs.rs b/consensus/types/src/test_utils/generate_random_block_and_blobs.rs new file mode 100644 index 0000000000..ab7ded0409 --- /dev/null +++ b/consensus/types/src/test_utils/generate_random_block_and_blobs.rs @@ -0,0 +1,96 @@ +use rand::Rng; + +use kzg::{KzgCommitment, KzgProof}; + +use crate::beacon_block_body::KzgCommitments; +use crate::*; + +use super::*; + +type BlobsBundle = (KzgCommitments, KzgProofs, BlobsList); + +pub fn generate_rand_block_and_blobs( + fork_name: ForkName, + num_blobs: usize, + rng: &mut impl Rng, +) -> (SignedBeaconBlock>, Vec>) { + let inner = map_fork_name!(fork_name, BeaconBlock, <_>::random_for_test(rng)); + let mut block = SignedBeaconBlock::from_block(inner, Signature::random_for_test(rng)); + let mut blob_sidecars = vec![]; + + if block.fork_name_unchecked() < ForkName::Deneb { + return (block, blob_sidecars); + } + + let (commitments, proofs, blobs) = generate_blobs::(num_blobs).unwrap(); + *block + .message_mut() + .body_mut() + .blob_kzg_commitments_mut() + .expect("kzg commitment expected from Deneb") = commitments.clone(); + + for (index, ((blob, kzg_commitment), kzg_proof)) in blobs + .into_iter() + .zip(commitments.into_iter()) + .zip(proofs.into_iter()) + .enumerate() + { + blob_sidecars.push(BlobSidecar { + index: index as u64, + blob: blob.clone(), + kzg_commitment, + kzg_proof, + signed_block_header: block.signed_block_header(), + kzg_commitment_inclusion_proof: block + .message() + .body() + .kzg_commitment_merkle_proof(index) + .unwrap(), + }); + } + (block, blob_sidecars) +} + +pub fn generate_blobs(n_blobs: usize) -> Result, String> { + let (mut commitments, mut proofs, mut blobs) = BlobsBundle::::default(); + + for blob_index in 0..n_blobs { + blobs + .push(Blob::::default()) + .map_err(|_| format!("blobs are full, blob index: {:?}", blob_index))?; + commitments + .push(KzgCommitment::empty_for_testing()) + .map_err(|_| format!("blobs are full, blob index: {:?}", blob_index))?; + proofs + .push(KzgProof::empty()) + .map_err(|_| format!("blobs are full, blob index: {:?}", blob_index))?; + } + + Ok((commitments, proofs, blobs)) +} + +#[cfg(test)] +mod test { + use super::*; + use rand::thread_rng; + + #[test] + fn test_verify_blob_inclusion_proof() { + let (_block, blobs) = + generate_rand_block_and_blobs::(ForkName::Deneb, 6, &mut thread_rng()); + for blob in blobs { + assert!(blob.verify_blob_sidecar_inclusion_proof()); + } + } + + #[test] + fn test_verify_blob_inclusion_proof_invalid() { + let (_block, blobs) = + generate_rand_block_and_blobs::(ForkName::Deneb, 6, &mut thread_rng()); + + for mut blob in blobs { + blob.kzg_commitment_inclusion_proof = FixedVector::random_for_test(&mut thread_rng()); + assert!(!blob.verify_blob_sidecar_inclusion_proof()); + } + } +} diff --git a/consensus/types/src/test_utils/mod.rs b/consensus/types/src/test_utils/mod.rs index d172342ee6..9599bcd364 100644 --- a/consensus/types/src/test_utils/mod.rs +++ b/consensus/types/src/test_utils/mod.rs @@ -15,6 +15,8 @@ use tree_hash::TreeHash; #[macro_use] mod macros; mod generate_deterministic_keypairs; +#[cfg(test)] +mod generate_random_block_and_blobs; mod test_random; pub fn test_ssz_tree_hash_pair(v1: &T, v2: &U) diff --git a/consensus/types/src/validator.rs b/consensus/types/src/validator.rs index b5753a6b94..b5e92d1f5d 100644 --- a/consensus/types/src/validator.rs +++ b/consensus/types/src/validator.rs @@ -63,7 +63,7 @@ impl Validator { spec: &ChainSpec, current_fork: ForkName, ) -> bool { - if current_fork >= ForkName::Electra { + if current_fork.electra_enabled() { self.is_eligible_for_activation_queue_electra(spec) } else { self.is_eligible_for_activation_queue_base(spec) @@ -172,7 +172,7 @@ impl Validator { spec: &ChainSpec, current_fork: ForkName, ) -> bool { - if current_fork >= ForkName::Electra { + if current_fork.electra_enabled() { self.is_fully_withdrawable_at_electra(balance, epoch, spec) } else { self.is_fully_withdrawable_at_capella(balance, epoch, spec) @@ -212,7 +212,7 @@ impl Validator { spec: &ChainSpec, current_fork: ForkName, ) -> bool { - if current_fork >= ForkName::Electra { + if current_fork.electra_enabled() { self.is_partially_withdrawable_validator_electra(balance, spec, current_fork) } else { self.is_partially_withdrawable_validator_capella(balance, spec) @@ -283,12 +283,12 @@ impl Default for Validator { Self { pubkey: PublicKeyBytes::empty(), withdrawal_credentials: Hash256::default(), - activation_eligibility_epoch: Epoch::from(std::u64::MAX), - activation_epoch: Epoch::from(std::u64::MAX), - exit_epoch: Epoch::from(std::u64::MAX), - withdrawable_epoch: Epoch::from(std::u64::MAX), + activation_eligibility_epoch: Epoch::from(u64::MAX), + activation_epoch: Epoch::from(u64::MAX), + exit_epoch: Epoch::from(u64::MAX), + withdrawable_epoch: Epoch::from(u64::MAX), slashed: false, - effective_balance: std::u64::MAX, + effective_balance: u64::MAX, } } } diff --git a/crypto/eth2_key_derivation/tests/tests.rs b/crypto/eth2_key_derivation/tests/tests.rs index b18a7b0e26..a344b7b3fc 100644 --- a/crypto/eth2_key_derivation/tests/tests.rs +++ b/crypto/eth2_key_derivation/tests/tests.rs @@ -22,7 +22,7 @@ fn deterministic() { fn children_deterministic() { let master = DerivedKey::from_seed(&[42]).unwrap(); assert_eq!( - master.child(u32::max_value()).secret(), - master.child(u32::max_value()).secret(), + master.child(u32::MAX).secret(), + master.child(u32::MAX).secret(), ) } diff --git a/crypto/eth2_keystore/src/json_keystore/checksum_module.rs b/crypto/eth2_keystore/src/json_keystore/checksum_module.rs index dbb21e4de1..834c5eda5f 100644 --- a/crypto/eth2_keystore/src/json_keystore/checksum_module.rs +++ b/crypto/eth2_keystore/src/json_keystore/checksum_module.rs @@ -14,9 +14,9 @@ pub enum ChecksumFunction { Sha256, } -impl Into for ChecksumFunction { - fn into(self) -> String { - match self { +impl From for String { + fn from(from: ChecksumFunction) -> String { + match from { ChecksumFunction::Sha256 => "sha256".into(), } } @@ -38,8 +38,8 @@ impl TryFrom for ChecksumFunction { #[serde(try_from = "Value", into = "Value")] pub struct EmptyMap; -impl Into for EmptyMap { - fn into(self) -> Value { +impl From for Value { + fn from(_from: EmptyMap) -> Value { Value::Object(Map::default()) } } diff --git a/crypto/eth2_keystore/src/json_keystore/cipher_module.rs b/crypto/eth2_keystore/src/json_keystore/cipher_module.rs index 03a9d305a2..c801a40923 100644 --- a/crypto/eth2_keystore/src/json_keystore/cipher_module.rs +++ b/crypto/eth2_keystore/src/json_keystore/cipher_module.rs @@ -13,9 +13,9 @@ pub enum CipherFunction { Aes128Ctr, } -impl Into for CipherFunction { - fn into(self) -> String { - match self { +impl From for String { + fn from(from: CipherFunction) -> String { + match from { CipherFunction::Aes128Ctr => "aes-128-ctr".into(), } } diff --git a/crypto/eth2_keystore/src/json_keystore/hex_bytes.rs b/crypto/eth2_keystore/src/json_keystore/hex_bytes.rs index cc61f13d97..e891693040 100644 --- a/crypto/eth2_keystore/src/json_keystore/hex_bytes.rs +++ b/crypto/eth2_keystore/src/json_keystore/hex_bytes.rs @@ -25,9 +25,9 @@ impl From> for HexBytes { } } -impl Into for HexBytes { - fn into(self) -> String { - hex::encode(self.0) +impl From for String { + fn from(from: HexBytes) -> String { + hex::encode(from.0) } } diff --git a/crypto/eth2_keystore/src/json_keystore/kdf_module.rs b/crypto/eth2_keystore/src/json_keystore/kdf_module.rs index a29b895c95..2086ac7b21 100644 --- a/crypto/eth2_keystore/src/json_keystore/kdf_module.rs +++ b/crypto/eth2_keystore/src/json_keystore/kdf_module.rs @@ -23,8 +23,8 @@ pub struct KdfModule { #[serde(try_from = "String", into = "String")] pub struct EmptyString; -impl Into for EmptyString { - fn into(self) -> String { +impl From for String { + fn from(_from: EmptyString) -> String { "".into() } } @@ -91,9 +91,9 @@ pub enum KdfFunction { Pbkdf2, } -impl Into for KdfFunction { - fn into(self) -> String { - match self { +impl From for String { + fn from(from: KdfFunction) -> String { + match from { KdfFunction::Scrypt => "scrypt".into(), KdfFunction::Pbkdf2 => "pbkdf2".into(), } diff --git a/crypto/eth2_wallet/src/json_wallet/mod.rs b/crypto/eth2_wallet/src/json_wallet/mod.rs index d2092508a2..f9a4105911 100644 --- a/crypto/eth2_wallet/src/json_wallet/mod.rs +++ b/crypto/eth2_wallet/src/json_wallet/mod.rs @@ -39,9 +39,9 @@ pub enum TypeField { Hd, } -impl Into for TypeField { - fn into(self) -> String { - match self { +impl From for String { + fn from(from: TypeField) -> String { + match from { TypeField::Hd => "hierarchical deterministic".into(), } } diff --git a/crypto/eth2_wallet/src/wallet.rs b/crypto/eth2_wallet/src/wallet.rs index 7a7d65f654..8bf7091216 100644 --- a/crypto/eth2_wallet/src/wallet.rs +++ b/crypto/eth2_wallet/src/wallet.rs @@ -179,7 +179,7 @@ impl Wallet { /// /// - If `wallet_password` is unable to decrypt `self`. /// - If `keystore_password.is_empty()`. - /// - If `self.nextaccount == u32::max_value()`. + /// - If `self.nextaccount == u32::MAX`. pub fn next_validator( &mut self, wallet_password: &[u8], diff --git a/crypto/kzg/src/kzg_proof.rs b/crypto/kzg/src/kzg_proof.rs index c9a138a31c..5a83466d0c 100644 --- a/crypto/kzg/src/kzg_proof.rs +++ b/crypto/kzg/src/kzg_proof.rs @@ -38,9 +38,9 @@ impl From<[u8; BYTES_PER_PROOF]> for KzgProof { } } -impl Into<[u8; BYTES_PER_PROOF]> for KzgProof { - fn into(self) -> [u8; BYTES_PER_PROOF] { - self.0 +impl From for [u8; BYTES_PER_PROOF] { + fn from(from: KzgProof) -> [u8; BYTES_PER_PROOF] { + from.0 } } diff --git a/lcli/src/block_root.rs b/lcli/src/block_root.rs index 0ee304c8a5..a90a4843d8 100644 --- a/lcli/src/block_root.rs +++ b/lcli/src/block_root.rs @@ -32,6 +32,7 @@ use clap_utils::{parse_optional, parse_required}; use environment::Environment; use eth2::{types::BlockId, BeaconNodeHttpClient, SensitiveUrl, Timeouts}; use eth2_network_config::Eth2NetworkConfig; +use log::info; use std::path::PathBuf; use std::time::{Duration, Instant}; use types::{EthSpec, FullPayload, SignedBeaconBlock}; diff --git a/lcli/src/main.rs b/lcli/src/main.rs index 911e9bdcac..85898b60ee 100644 --- a/lcli/src/main.rs +++ b/lcli/src/main.rs @@ -1,5 +1,3 @@ -#[macro_use] -extern crate log; mod block_root; mod check_deposit_data; mod generate_bootnode_enr; diff --git a/lcli/src/parse_ssz.rs b/lcli/src/parse_ssz.rs index 3aa77e5700..dd13f6847b 100644 --- a/lcli/src/parse_ssz.rs +++ b/lcli/src/parse_ssz.rs @@ -1,6 +1,7 @@ use clap::ArgMatches; use clap_utils::parse_required; use eth2_network_config::Eth2NetworkConfig; +use log::info; use serde::Serialize; use snap::raw::Decoder; use ssz::Decode; diff --git a/lcli/src/skip_slots.rs b/lcli/src/skip_slots.rs index a2173f10df..2ad79051ea 100644 --- a/lcli/src/skip_slots.rs +++ b/lcli/src/skip_slots.rs @@ -50,6 +50,7 @@ use clap_utils::{parse_optional, parse_required}; use environment::Environment; use eth2::{types::StateId, BeaconNodeHttpClient, SensitiveUrl, Timeouts}; use eth2_network_config::Eth2NetworkConfig; +use log::info; use ssz::Encode; use state_processing::state_advance::{complete_state_advance, partial_state_advance}; use state_processing::AllCaches; diff --git a/lcli/src/state_root.rs b/lcli/src/state_root.rs index 06293b79b3..17a947b2f0 100644 --- a/lcli/src/state_root.rs +++ b/lcli/src/state_root.rs @@ -4,6 +4,7 @@ use clap_utils::{parse_optional, parse_required}; use environment::Environment; use eth2::{types::StateId, BeaconNodeHttpClient, SensitiveUrl, Timeouts}; use eth2_network_config::Eth2NetworkConfig; +use log::info; use std::path::PathBuf; use std::time::{Duration, Instant}; use types::{BeaconState, EthSpec}; diff --git a/lcli/src/transition_blocks.rs b/lcli/src/transition_blocks.rs index 7097908677..62ae602187 100644 --- a/lcli/src/transition_blocks.rs +++ b/lcli/src/transition_blocks.rs @@ -72,6 +72,7 @@ use eth2::{ BeaconNodeHttpClient, SensitiveUrl, Timeouts, }; use eth2_network_config::Eth2NetworkConfig; +use log::{debug, info}; use ssz::Encode; use state_processing::state_advance::complete_state_advance; use state_processing::{ diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs index 5743bedfd7..abee30737c 100644 --- a/lighthouse/src/main.rs +++ b/lighthouse/src/main.rs @@ -688,9 +688,15 @@ fn run( let executor = context.executor.clone(); let mut config = beacon_node::get_config::(matches, &context)?; config.logger_config = logger_config; - let shutdown_flag = matches.get_flag("immediate-shutdown"); // Dump configs if `dump-config` or `dump-chain-config` flags are set clap_utils::check_dump_configs::<_, E>(matches, &config, &context.eth2_config.spec)?; + + let shutdown_flag = matches.get_flag("immediate-shutdown"); + if shutdown_flag { + info!(log, "Beacon node immediate shutdown triggered."); + return Ok(()); + } + executor.clone().spawn( async move { if let Err(e) = ProductionBeaconNode::new(context.clone(), config).await { @@ -700,10 +706,6 @@ fn run( let _ = executor .shutdown_sender() .try_send(ShutdownReason::Failure("Failed to start beacon node")); - } else if shutdown_flag { - let _ = executor.shutdown_sender().try_send(ShutdownReason::Success( - "Beacon node immediate shutdown triggered.", - )); } }, "beacon_node", @@ -715,31 +717,31 @@ fn run( let executor = context.executor.clone(); let config = validator_client::Config::from_cli(matches, context.log()) .map_err(|e| format!("Unable to initialize validator config: {}", e))?; - let shutdown_flag = matches.get_flag("immediate-shutdown"); // Dump configs if `dump-config` or `dump-chain-config` flags are set clap_utils::check_dump_configs::<_, E>(matches, &config, &context.eth2_config.spec)?; - if !shutdown_flag { - executor.clone().spawn( - async move { - if let Err(e) = ProductionValidatorClient::new(context, config) - .and_then(|mut vc| async move { vc.start_service().await }) - .await - { - crit!(log, "Failed to start validator client"; "reason" => e); - // Ignore the error since it always occurs during normal operation when - // shutting down. - let _ = executor.shutdown_sender().try_send(ShutdownReason::Failure( - "Failed to start validator client", - )); - } - }, - "validator_client", - ); - } else { - let _ = executor.shutdown_sender().try_send(ShutdownReason::Success( - "Validator client immediate shutdown triggered.", - )); + + let shutdown_flag = matches.get_flag("immediate-shutdown"); + if shutdown_flag { + info!(log, "Validator client immediate shutdown triggered."); + return Ok(()); } + + executor.clone().spawn( + async move { + if let Err(e) = ProductionValidatorClient::new(context, config) + .and_then(|mut vc| async move { vc.start_service().await }) + .await + { + crit!(log, "Failed to start validator client"; "reason" => e); + // Ignore the error since it always occurs during normal operation when + // shutting down. + let _ = executor + .shutdown_sender() + .try_send(ShutdownReason::Failure("Failed to start validator client")); + } + }, + "validator_client", + ); } _ => { crit!(log, "No subcommand supplied. See --help ."); diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index 73badac913..caadf208e3 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -58,13 +58,22 @@ impl CommandLineTest { fn run_with_zero_port(&mut self) -> CompletedTest { // Required since Deneb was enabled on mainnet. - self.cmd.arg("--allow-insecure-genesis-sync"); - self.run_with_zero_port_and_no_genesis_sync() + self.set_allow_insecure_genesis_sync() + .run_with_zero_port_and_no_genesis_sync() } fn run_with_zero_port_and_no_genesis_sync(&mut self) -> CompletedTest { + self.set_zero_port().run() + } + + fn set_allow_insecure_genesis_sync(&mut self) -> &mut Self { + self.cmd.arg("--allow-insecure-genesis-sync"); + self + } + + fn set_zero_port(&mut self) -> &mut Self { self.cmd.arg("-z"); - self.run() + self } } @@ -101,9 +110,20 @@ fn staking_flag() { } #[test] -#[should_panic] fn allow_insecure_genesis_sync_default() { - CommandLineTest::new().run_with_zero_port_and_no_genesis_sync(); + CommandLineTest::new() + .run_with_zero_port_and_no_genesis_sync() + .with_config(|config| { + assert_eq!(config.allow_insecure_genesis_sync, false); + }); +} + +#[test] +#[should_panic] +fn insecure_genesis_sync_should_panic() { + CommandLineTest::new() + .set_zero_port() + .run_with_immediate_shutdown(false); } #[test] @@ -2252,7 +2272,9 @@ fn ensure_panic_on_failed_launch() { CommandLineTest::new() .flag("slasher", None) .flag("slasher-chunk-size", Some("10")) - .run_with_zero_port() + .set_allow_insecure_genesis_sync() + .set_zero_port() + .run_with_immediate_shutdown(false) .with_config(|config| { let slasher_config = config .slasher diff --git a/lighthouse/tests/exec.rs b/lighthouse/tests/exec.rs index 61e0677ca8..9d6453908c 100644 --- a/lighthouse/tests/exec.rs +++ b/lighthouse/tests/exec.rs @@ -21,12 +21,19 @@ pub trait CommandLineTestExec { self } + fn run(&mut self) -> CompletedTest { + self.run_with_immediate_shutdown(true) + } + /// Executes the `Command` returned by `Self::cmd_mut` with temporary data directory, dumps - /// the configuration and shuts down immediately. + /// the configuration and shuts down immediately if `immediate_shutdown` is set to true. /// /// Options `--datadir`, `--dump-config`, `--dump-chain-config` and `--immediate-shutdown` must /// not be set on the command. - fn run(&mut self) -> CompletedTest { + fn run_with_immediate_shutdown( + &mut self, + immediate_shutdown: bool, + ) -> CompletedTest { // Setup temp directory. let tmp_dir = TempDir::new().expect("Unable to create temporary directory"); let tmp_config_path: PathBuf = tmp_dir.path().join("config.json"); @@ -39,8 +46,11 @@ pub trait CommandLineTestExec { .arg(format!("--{}", "dump-config")) .arg(tmp_config_path.as_os_str()) .arg(format!("--{}", "dump-chain-config")) - .arg(tmp_chain_config_path.as_os_str()) - .arg("--immediate-shutdown"); + .arg(tmp_chain_config_path.as_os_str()); + + if immediate_shutdown { + cmd.arg("--immediate-shutdown"); + } // Run the command. let output = output_result(cmd); diff --git a/scripts/local_testnet/README.md b/scripts/local_testnet/README.md index 77c9d62c1c..0275cb217f 100644 --- a/scripts/local_testnet/README.md +++ b/scripts/local_testnet/README.md @@ -1,201 +1,85 @@ # Simple Local Testnet -These scripts allow for running a small local testnet with multiple beacon nodes and validator clients and a geth execution client. +These scripts allow for running a small local testnet with a default of 4 beacon nodes, 4 validator clients and 4 geth execution clients using Kurtosis. This setup can be useful for testing and development. -## Requirements +## Installation -The scripts require `lcli`, `lighthouse`, `geth`, `bootnode` to be installed on `PATH` (run `echo $PATH` to view all `PATH` directories). +1. Install [Docker](https://docs.docker.com/get-docker/). Verify that Docker has been successfully installed by running `sudo docker run hello-world`. +1. Install [Kurtosis](https://docs.kurtosis.com/install/). Verify that Kurtosis has been successfully installed by running `kurtosis version` which should display the version. -MacOS users need to install GNU `sed` and GNU `grep`, and add them both to `PATH` as well. - -The first step is to install Rust and dependencies. Refer to the [Lighthouse Book](https://lighthouse-book.sigmaprime.io/installation-source.html#dependencies) for installation. We will also need [jq](https://jqlang.github.io/jq/), which can be installed with `sudo apt install jq`. - -Then, we clone the Lighthouse repository: -```bash -cd ~ -git clone https://github.com/sigp/lighthouse.git -cd lighthouse -``` -We are now ready to build Lighthouse. Run the command: - -```bash -make -make install-lcli -``` - -This will build `lighthouse` and `lcli`. For `geth` and `bootnode`, go to [geth website](https://geth.ethereum.org/downloads) and download the `Geth & Tools`. For example, to download and extract `Geth & Tools 1.13.1`: - -```bash -cd ~ -curl -LO https://gethstore.blob.core.windows.net/builds/geth-alltools-linux-amd64-1.13.1-3f40e65c.tar.gz -tar xvf geth-alltools-linux-amd64-1.13.1-3f40e65c.tar.gz -``` - -After extraction, copy `geth` and `bootnode` to the `PATH`. A typical directory is `/usr/local/bin`. - -```bash -cd geth-alltools-linux-amd64-1.13.1-3f40e65c -sudo cp geth bootnode /usr/local/bin -``` - -After that We can remove the downloaded files: - -```bash -cd ~ -rm -r geth-alltools-linux-amd64-1.13.1-3f40e65c geth-alltools-linux-amd64-1.13.1-3f40e65c.tar.gz -``` - -We are now ready to start a local testnet. +1. Install [yq](https://github.com/mikefarah/yq). If you are on Ubuntu, you can install `yq` by running `sudo apt install yq -y`. ## Starting the testnet -To start a testnet using the predetermined settings: +To start a testnet, from the Lighthouse root repository: ```bash -cd ~ -cd ./lighthouse/scripts/local_testnet -./start_local_testnet.sh genesis.json +cd ./scripts/local_testnet +./start_local_testnet.sh ``` -This will execute the script and if the testnet setup is successful, you will see "Started!" at the end. +It will build a Lighthouse docker image from the root of the directory and will take an approximately 12 minutes to complete. Once built, the testing will be started automatically. You will see a list of services running and "Started!" at the end. +You can also select your own Lighthouse docker image to use by specifying it in `network_params.yml` under the `cl_image` key. +Full configuration reference for kurtosis is specified [here](https://github.com/ethpandaops/ethereum-package?tab=readme-ov-file#configuration). -The testnet starts with a post-merge genesis state. -The testnet starts a consensus layer and execution layer boot node along with `BN_COUNT` -(the number of beacon nodes) each connected to a geth execution client and `VC_COUNT` (the number of validator clients). By default, `BN_COUNT=4`, `VC_COUNT=4`. - -The `start_local_testnet.sh` script takes four options `-v VC_COUNT`, `-d DEBUG_LEVEL`, `-p` to enable builder proposals and `-h` for help. It also takes a mandatory `GENESIS_FILE` for initialising geth's state. -A sample `genesis.json` is provided in this directory. - -The options may be in any order or absent in which case they take the default value specified. -- VC_COUNT: the number of validator clients to create, default: `BN_COUNT` -- DEBUG_LEVEL: one of { error, warn, info, debug, trace }, default: `info` - -The `ETH1_BLOCK_HASH` environment variable is set to the block_hash of the genesis execution layer block which depends on the contents of `genesis.json`. Users of these scripts need to ensure that the `ETH1_BLOCK_HASH` variable is updated if genesis file is modified. - -To view the beacon, validator client and geth logs: +To view all running services: ```bash -tail -f ~/.lighthouse/local-testnet/testnet/beacon_node_1.log -tail -f ~/.lighthouse/local-testnet/testnet/validator_node_1.log -tail -f ~/.lighthouse/local-testnet/testnet/geth_1.log +kurtosis enclave inspect local-testnet ``` -where `beacon_node_1` can be changed to `beacon_node_2`, `beacon_node_3` or `beacon_node_4` to view logs for different beacon nodes. The same applies to validator clients and geth nodes. +To view the logs: + +```bash +kurtosis service logs local-testnet $SERVICE_NAME +``` + +where `$SERVICE_NAME` is obtained by inspecting the running services above. For example, to view the logs of the first beacon node, validator client and geth: + +```bash +kurtosis service logs local-testnet -f cl-1-lighthouse-geth +kurtosis service logs local-testnet -f vc-1-geth-lighthouse +kurtosis service logs local-testnet -f el-1-geth-lighthouse +``` + +If you would like to save the logs, use the command: + +```bash +kurtosis dump $OUTPUT_DIRECTORY +``` + +This will create a folder named `$OUTPUT_DIRECTORY` in the present working directory that contains all logs and other information. If you want the logs for a particular service and saved to a file named `logs.txt`: + +```bash +kurtosis service logs local-testnet $SERVICE_NAME -a > logs.txt +``` +where `$SERVICE_NAME` can be viewed by running `kurtosis enclave inspect local-testnet`. + +Kurtosis comes with a Dora explorer which can be opened with: + +```bash +open $(kurtosis port print local-testnet dora http) +``` + +Some testnet parameters can be varied by modifying the `network_params.yaml` file. Kurtosis also comes with a web UI which can be open with `kurtosis web`. ## Stopping the testnet -To stop the testnet, navigate to the directory `cd ~/lighthouse/scripts/local_testnet`, then run the command: +To stop the testnet, from the Lighthouse root repository: ```bash +cd ./scripts/local_testnet ./stop_local_testnet.sh ``` -Once a testnet is stopped, it cannot be continued from where it left off. When the start local testnet command is run, it will start a new local testnet. +You will see "Local testnet stopped." at the end. -## Manual creation of local testnet +## CLI options -In [Starting the testnet](./README.md#starting-the-testnet), the testnet is started automatically with predetermined parameters (database directory, ports used etc). This section describes some modifications of the local testnet settings, e.g., changing the database directory, or changing the ports used. - - -The testnet also contains parameters that are specified in `vars.env`, such as the slot time `SECONDS_PER_SLOT=3` (instead of 12 seconds on mainnet). You may change these parameters to suit your testing purposes. After that, in the `local_testnet` directory, run the following command to create genesis state with embedded validators and validator keys, and also to update the time in `genesis.json`: +The script comes with some CLI options, which can be viewed with `./start_local_testnet.sh --help`. One of the CLI options is to avoid rebuilding Lighthouse each time the testnet starts, which can be configured with the command: ```bash -./setup.sh -./setup_time.sh genesis.json -``` - -Note: The generated genesis validators are embedded into the genesis state as genesis validators and hence do not require manual deposits to activate. - -Generate bootnode enr and start an EL and CL bootnode so that multiple nodes can find each other -```bash -./bootnode.sh -./el_bootnode.sh -``` - -Start a geth node: -```bash -./geth.sh -``` -e.g. -```bash -./geth.sh $HOME/.lighthouse/local-testnet/geth_1 7001 6001 5001 genesis.json -``` - -Start a beacon node: - -```bash -./beacon_node.sh -``` -e.g. -```bash -./beacon_node.sh $HOME/.lighthouse/local-testnet/node_1 9001 9101 8001 http://localhost:5001 ~/.lighthouse/local-testnet/geth_1/geth/jwtsecret -``` - -In a new terminal, start the validator client which will attach to the first -beacon node: - -```bash -./validator_client.sh -``` -e.g. to attach to the above created beacon node -```bash -./validator_client.sh $HOME/.lighthouse/local-testnet/node_1 http://localhost:8001 -``` - -You can create additional geth, beacon node and validator client instances by changing the ports, e.g., for a second geth, beacon node and validator client: - -```bash -./geth.sh $HOME/.lighthouse/local-testnet/geth_2 7002 6002 5002 genesis.json -./beacon_node.sh $HOME/.lighthouse/local-testnet/node_2 9002 9102 8002 http://localhost:5002 ~/.lighthouse/local-testnet/geth_2/geth/jwtsecret -./validator_client.sh $HOME/.lighthouse/local-testnet/node_2 http://localhost:8002 -``` - -## Additional Info - -### Adjusting number and distribution of validators -The `VALIDATOR_COUNT` parameter is used to specify the number of insecure validator keystores to generate and make deposits for. -The `BN_COUNT` parameter is used to adjust the division of these generated keys among separate validator client instances. -For e.g. for `VALIDATOR_COUNT=80` and `BN_COUNT=4`, the validator keys are distributed over 4 datadirs with 20 keystores per datadir. The datadirs are located in `$DATADIR/node_{i}` which can be passed to separate validator client -instances using the `--datadir` parameter. - -### Starting fresh - -You can delete the current testnet and all related files using the following command. Alternatively, if you wish to start another testnet, doing the steps [Starting the testnet](./README.md#starting-the-testnet) will automatically delete the files and start a fresh local testnet. - -```bash -./clean.sh -``` - -### Updating the genesis time of the beacon state - -If it's been a while since you ran `./setup` then the genesis time of the -genesis state will be far in the future, causing lots of skip slots. - -Update the genesis time to now using: - -```bash -./reset_genesis_time.sh -``` - -> Note: you probably want to just rerun `./start_local_testnet.sh` to start over -> but this is another option. - -### Testing builder flow - -1. Add builder URL to `BN_ARGS` in `./vars.env`, e.g. `--builder http://localhost:8650`. Some mock builder server options: - - [`mock-relay`](https://github.com/realbigsean/mock-relay) - - [`dummy-builder`](https://github.com/michaelsproul/dummy_builder) -2. The above mock builders do not support non-mainnet presets as of now, and will require setting `SECONDS_PER_SLOT` and `SECONDS_PER_ETH1_BLOCK` to `12` in `./vars.env`. -3. Start the testnet with the following command (the `-p` flag enables the validator client `--builder-proposals` flag): - ```bash - ./start_local_testnet.sh -p genesis.json - ``` -4. Block production using builder flow will start at epoch 4. - -### Testing sending a transaction - -Some addresses in the local testnet are seeded with testnet ETH, allowing users to carry out transactions. To send a transaction, we first add the address to a wallet, such as [Metamask](https://metamask.io/). The private keys for the addresses are listed [here](https://github.com/sigp/lighthouse/blob/441fc1691b69f9edc4bbdc6665f3efab16265c9b/testing/execution_engine_integration/src/execution_engine.rs#L13-L14). - -Next, we add the local testnet to Metamask, a brief guide can be found [here](https://support.metamask.io/hc/en-us/articles/360043227612-How-to-add-a-custom-network-RPC). If you start the local testnet with default settings, the network RPC is: http://localhost:6001 and the `Chain ID` is `4242`, as defined in [`vars.env`](https://github.com/sigp/lighthouse/blob/441fc1691b69f9edc4bbdc6665f3efab16265c9b/scripts/local_testnet/vars.env#L42). Once the network and account are added, you should see that the account contains testnet ETH which allow us to carry out transactions. +./start_local_testnet.sh -b false +``` \ No newline at end of file diff --git a/scripts/local_testnet/start_local_testnet.sh b/scripts/local_testnet/start_local_testnet.sh index e0172e6b28..4b03b1e010 100755 --- a/scripts/local_testnet/start_local_testnet.sh +++ b/scripts/local_testnet/start_local_testnet.sh @@ -78,6 +78,6 @@ fi # Stop local testnet kurtosis enclave rm -f $ENCLAVE_NAME 2>/dev/null || true -kurtosis run --enclave $ENCLAVE_NAME github.com/kurtosis-tech/ethereum-package --args-file $NETWORK_PARAMS_FILE +kurtosis run --enclave $ENCLAVE_NAME github.com/ethpandaops/ethereum-package --args-file $NETWORK_PARAMS_FILE echo "Started!" diff --git a/scripts/tests/network_params.yaml b/scripts/tests/network_params.yaml index 1725203138..21114df0e8 100644 --- a/scripts/tests/network_params.yaml +++ b/scripts/tests/network_params.yaml @@ -1,4 +1,4 @@ -# Full configuration reference [here](https://github.com/kurtosis-tech/ethereum-package?tab=readme-ov-file#configuration). +# Full configuration reference [here](https://github.com/ethpandaops/ethereum-package?tab=readme-ov-file#configuration). participants: - el_type: geth el_image: ethereum/client-go:latest diff --git a/slasher/src/array.rs b/slasher/src/array.rs index 714ccf4e77..77ddceb85f 100644 --- a/slasher/src/array.rs +++ b/slasher/src/array.rs @@ -5,7 +5,6 @@ use crate::{ }; use flate2::bufread::{ZlibDecoder, ZlibEncoder}; use serde::{Deserialize, Serialize}; -use slog::Logger; use std::borrow::Borrow; use std::collections::{btree_map::Entry, BTreeMap, HashSet}; use std::io::Read; @@ -486,7 +485,6 @@ pub fn update( batch: Vec>>, current_epoch: Epoch, config: &Config, - log: &Logger, ) -> Result>, Error> { // Split the batch up into horizontal segments. // Map chunk indexes in the range `0..self.config.chunk_size` to attestations @@ -506,7 +504,6 @@ pub fn update( &chunk_attestations, current_epoch, config, - log, )?; slashings.extend(update_array::<_, MaxTargetChunk>( db, @@ -515,7 +512,6 @@ pub fn update( &chunk_attestations, current_epoch, config, - log, )?); // Update all current epochs. @@ -574,7 +570,6 @@ pub fn update_array( chunk_attestations: &BTreeMap>>>, current_epoch: Epoch, config: &Config, - log: &Logger, ) -> Result>, Error> { let mut slashings = HashSet::new(); // Map from chunk index to updated chunk at that index. @@ -608,19 +603,8 @@ pub fn update_array( current_epoch, config, )?; - match slashing_status.into_slashing(&attestation.indexed) { - Ok(Some(slashing)) => { - slashings.insert(slashing); - } - Err(e) => { - slog::error!( - log, - "Invalid slashing conversion"; - "validator_index" => validator_index, - "error" => e - ); - } - Ok(None) => {} + if let Some(slashing) = slashing_status.into_slashing(&attestation.indexed) { + slashings.insert(slashing); } } } diff --git a/slasher/src/database.rs b/slasher/src/database.rs index d80112c553..16089706a0 100644 --- a/slasher/src/database.rs +++ b/slasher/src/database.rs @@ -18,11 +18,12 @@ use std::marker::PhantomData; use std::sync::Arc; use tree_hash::TreeHash; use types::{ - Epoch, EthSpec, Hash256, IndexedAttestation, ProposerSlashing, SignedBeaconBlockHeader, Slot, + Epoch, EthSpec, Hash256, IndexedAttestation, IndexedAttestationOnDisk, + IndexedAttestationRefOnDisk, ProposerSlashing, SignedBeaconBlockHeader, Slot, }; /// Current database schema version, to check compatibility of on-disk DB with software. -pub const CURRENT_SCHEMA_VERSION: u64 = 3; +pub const CURRENT_SCHEMA_VERSION: u64 = 4; /// Metadata about the slashing database itself. const METADATA_DB: &str = "metadata"; @@ -457,7 +458,9 @@ impl SlasherDB { }; let attestation_key = IndexedAttestationId::new(indexed_att_id); - let data = indexed_attestation.as_ssz_bytes(); + let indexed_attestation_on_disk: IndexedAttestationRefOnDisk = + indexed_attestation.into(); + let data = indexed_attestation_on_disk.as_ssz_bytes(); cursor.put(attestation_key.as_ref(), &data)?; drop(cursor); @@ -481,7 +484,8 @@ impl SlasherDB { .ok_or(Error::MissingIndexedAttestation { id: indexed_attestation_id.as_u64(), })?; - ssz_decode(bytes) + let indexed_attestation: IndexedAttestationOnDisk = ssz_decode(bytes)?; + Ok(indexed_attestation.into()) } fn get_attestation_data_root( diff --git a/slasher/src/lib.rs b/slasher/src/lib.rs index 339ca1f7d3..4d58fa7702 100644 --- a/slasher/src/lib.rs +++ b/slasher/src/lib.rs @@ -53,56 +53,41 @@ impl AttesterSlashingStatus { pub fn into_slashing( self, new_attestation: &IndexedAttestation, - ) -> Result>, String> { + ) -> Option> { use AttesterSlashingStatus::*; // The surrounding attestation must be in `attestation_1` to be valid. - Ok(match self { + match self { NotSlashable => None, AlreadyDoubleVoted => None, - DoubleVote(existing) | SurroundedByExisting(existing) => match *existing { - IndexedAttestation::Base(existing_att) => { - Some(AttesterSlashing::Base(AttesterSlashingBase { - attestation_1: existing_att, - attestation_2: new_attestation - .as_base() - .map_err(|e| format!("{e:?}"))? - .clone(), - })) - } - IndexedAttestation::Electra(existing_att) => { - Some(AttesterSlashing::Electra(AttesterSlashingElectra { - attestation_1: existing_att, - // A double vote should never convert, a surround vote where the surrounding - // vote is electra may convert. - attestation_2: new_attestation - .clone() - .to_electra() - .map_err(|e| format!("{e:?}"))?, - })) - } - }, - SurroundsExisting(existing) => { - match new_attestation { - IndexedAttestation::Base(new_attestation) => { + DoubleVote(existing) | SurroundedByExisting(existing) => { + match (&*existing, new_attestation) { + (IndexedAttestation::Base(existing_att), IndexedAttestation::Base(new)) => { Some(AttesterSlashing::Base(AttesterSlashingBase { - attestation_1: existing - .as_base() - .map_err(|e| format!("{e:?}"))? - .clone(), - attestation_2: new_attestation.clone(), - })) - } - IndexedAttestation::Electra(new_attestation) => { - Some(AttesterSlashing::Electra(AttesterSlashingElectra { - attestation_1: existing.to_electra().map_err(|e| format!("{e:?}"))?, - // A double vote should never convert, a surround vote where the surrounding - // vote is electra may convert. - attestation_2: new_attestation.clone(), + attestation_1: existing_att.clone(), + attestation_2: new.clone(), })) } + // A slashing involving an electra attestation type must return an `AttesterSlashingElectra` type + (_, _) => Some(AttesterSlashing::Electra(AttesterSlashingElectra { + attestation_1: existing.clone().to_electra(), + attestation_2: new_attestation.clone().to_electra(), + })), } } - }) + SurroundsExisting(existing) => match (&*existing, new_attestation) { + (IndexedAttestation::Base(existing_att), IndexedAttestation::Base(new)) => { + Some(AttesterSlashing::Base(AttesterSlashingBase { + attestation_1: new.clone(), + attestation_2: existing_att.clone(), + })) + } + // A slashing involving an electra attestation type must return an `AttesterSlashingElectra` type + (_, _) => Some(AttesterSlashing::Electra(AttesterSlashingElectra { + attestation_1: new_attestation.clone().to_electra(), + attestation_2: existing.clone().to_electra(), + })), + }, + } } } diff --git a/slasher/src/migrate.rs b/slasher/src/migrate.rs index 674ab9c132..ce298caaf2 100644 --- a/slasher/src/migrate.rs +++ b/slasher/src/migrate.rs @@ -17,6 +17,10 @@ impl SlasherDB { software_schema_version: CURRENT_SCHEMA_VERSION, }), (x, y) if x == y => Ok(self), + (3, 4) => { + // TODO(electra): db migration due to `IndexedAttestationOnDisk` + Ok(self) + } (_, _) => Err(Error::IncompatibleSchemaVersion { database_schema_version: schema_version, software_schema_version: CURRENT_SCHEMA_VERSION, diff --git a/slasher/src/slasher.rs b/slasher/src/slasher.rs index ef6dcce9b1..fc8e8453c8 100644 --- a/slasher/src/slasher.rs +++ b/slasher/src/slasher.rs @@ -247,7 +247,6 @@ impl Slasher { batch, current_epoch, &self.config, - &self.log, ) { Ok(slashings) => { if !slashings.is_empty() { @@ -295,25 +294,14 @@ impl Slasher { indexed_attestation_id, )?; - match slashing_status.into_slashing(attestation) { - Ok(Some(slashing)) => { - debug!( - self.log, - "Found double-vote slashing"; - "validator_index" => validator_index, - "epoch" => slashing.attestation_1().data().target.epoch, - ); - slashings.insert(slashing); - } - Err(e) => { - error!( - self.log, - "Invalid slashing conversion"; - "validator_index" => validator_index, - "error" => e - ); - } - Ok(None) => {} + if let Some(slashing) = slashing_status.into_slashing(attestation) { + debug!( + self.log, + "Found double-vote slashing"; + "validator_index" => validator_index, + "epoch" => slashing.attestation_1().data().target.epoch, + ); + slashings.insert(slashing); } } diff --git a/slasher/src/test_utils.rs b/slasher/src/test_utils.rs index 8bbf042c2d..7019a8aed7 100644 --- a/slasher/src/test_utils.rs +++ b/slasher/src/test_utils.rs @@ -39,7 +39,6 @@ pub fn indexed_att( target_epoch: u64, target_root: u64, ) -> IndexedAttestation { - // TODO(electra) make fork-agnostic IndexedAttestation::Base(IndexedAttestationBase { attesting_indices: attesting_indices.as_ref().to_vec().into(), data: AttestationData { @@ -63,7 +62,6 @@ pub fn att_slashing( attestation_1: &IndexedAttestation, attestation_2: &IndexedAttestation, ) -> AttesterSlashing { - // TODO(electra): fix this one we superstruct IndexedAttestation (return the correct type) match (attestation_1, attestation_2) { (IndexedAttestation::Base(att1), IndexedAttestation::Base(att2)) => { AttesterSlashing::Base(AttesterSlashingBase { @@ -71,13 +69,11 @@ pub fn att_slashing( attestation_2: att2.clone(), }) } - (IndexedAttestation::Electra(att1), IndexedAttestation::Electra(att2)) => { - AttesterSlashing::Electra(AttesterSlashingElectra { - attestation_1: att1.clone(), - attestation_2: att2.clone(), - }) - } - _ => panic!("attestations must be of the same type"), + // A slashing involving an electra attestation type must return an electra AttesterSlashing type + (_, _) => AttesterSlashing::Electra(AttesterSlashingElectra { + attestation_1: attestation_1.clone().to_electra(), + attestation_2: attestation_2.clone().to_electra(), + }), } } @@ -124,11 +120,9 @@ pub fn slashed_validators_from_attestations( continue; } - // TODO(electra) in a nested loop, copying to vecs feels bad - let attesting_indices_1 = att1.attesting_indices_to_vec(); - let attesting_indices_2 = att2.attesting_indices_to_vec(); - if att1.is_double_vote(att2) || att1.is_surround_vote(att2) { + let attesting_indices_1 = att1.attesting_indices_to_vec(); + let attesting_indices_2 = att2.attesting_indices_to_vec(); slashed_validators.extend(hashset_intersection( &attesting_indices_1, &attesting_indices_2, diff --git a/slasher/tests/attester_slashings.rs b/slasher/tests/attester_slashings.rs index 40d9fa511c..b74e491e4b 100644 --- a/slasher/tests/attester_slashings.rs +++ b/slasher/tests/attester_slashings.rs @@ -5,7 +5,9 @@ use maplit::hashset; use rayon::prelude::*; use slasher::{ config::DEFAULT_CHUNK_SIZE, - test_utils::{att_slashing, indexed_att, slashed_validators_from_slashings, E}, + test_utils::{ + att_slashing, indexed_att, indexed_att_electra, slashed_validators_from_slashings, E, + }, Config, Slasher, }; use std::collections::HashSet; @@ -15,23 +17,35 @@ use types::{AttesterSlashing, Epoch, IndexedAttestation}; #[test] fn double_vote_single_val() { let v = vec![99]; - let att1 = indexed_att(&v, 0, 1, 0); - let att2 = indexed_att(&v, 0, 1, 1); - let slashings = hashset![att_slashing(&att1, &att2)]; - let attestations = vec![att1, att2]; - slasher_test_indiv(&attestations, &slashings, 1); - slasher_test_indiv(&attestations, &slashings, 1000); + for (att1, att2) in [ + (indexed_att(&v, 0, 1, 0), indexed_att(&v, 0, 1, 1)), + ( + indexed_att_electra(&v, 0, 1, 0), + indexed_att_electra(&v, 0, 1, 1), + ), + ] { + let slashings = hashset![att_slashing(&att1, &att2)]; + let attestations = vec![att1, att2]; + slasher_test_indiv(&attestations, &slashings, 1); + slasher_test_indiv(&attestations, &slashings, 1000); + } } #[test] fn double_vote_multi_vals() { let v = vec![0, 1, 2]; - let att1 = indexed_att(&v, 0, 1, 0); - let att2 = indexed_att(&v, 0, 1, 1); - let slashings = hashset![att_slashing(&att1, &att2)]; - let attestations = vec![att1, att2]; - slasher_test_indiv(&attestations, &slashings, 1); - slasher_test_indiv(&attestations, &slashings, 1000); + for (att1, att2) in [ + (indexed_att(&v, 0, 1, 0), indexed_att(&v, 0, 1, 1)), + ( + indexed_att_electra(&v, 0, 1, 0), + indexed_att_electra(&v, 0, 1, 1), + ), + ] { + let slashings = hashset![att_slashing(&att1, &att2)]; + let attestations = vec![att1, att2]; + slasher_test_indiv(&attestations, &slashings, 1); + slasher_test_indiv(&attestations, &slashings, 1000); + } } // A subset of validators double vote. @@ -39,12 +53,18 @@ fn double_vote_multi_vals() { fn double_vote_some_vals() { let v1 = vec![0, 1, 2, 3, 4, 5, 6]; let v2 = vec![0, 2, 4, 6]; - let att1 = indexed_att(v1, 0, 1, 0); - let att2 = indexed_att(v2, 0, 1, 1); - let slashings = hashset![att_slashing(&att1, &att2)]; - let attestations = vec![att1, att2]; - slasher_test_indiv(&attestations, &slashings, 1); - slasher_test_indiv(&attestations, &slashings, 1000); + for (att1, att2) in [ + (indexed_att(&v1, 0, 1, 0), indexed_att(&v2, 0, 1, 1)), + ( + indexed_att_electra(&v1, 0, 1, 0), + indexed_att_electra(&v2, 0, 1, 1), + ), + ] { + let slashings = hashset![att_slashing(&att1, &att2)]; + let attestations = vec![att1, att2]; + slasher_test_indiv(&attestations, &slashings, 1); + slasher_test_indiv(&attestations, &slashings, 1000); + } } // A subset of validators double vote, others vote twice for the same thing. @@ -53,13 +73,23 @@ fn double_vote_some_vals_repeat() { let v1 = vec![0, 1, 2, 3, 4, 5, 6]; let v2 = vec![0, 2, 4, 6]; let v3 = vec![1, 3, 5]; - let att1 = indexed_att(v1, 0, 1, 0); - let att2 = indexed_att(v2, 0, 1, 1); - let att3 = indexed_att(v3, 0, 1, 0); - let slashings = hashset![att_slashing(&att1, &att2)]; - let attestations = vec![att1, att2, att3]; - slasher_test_indiv(&attestations, &slashings, 1); - slasher_test_indiv(&attestations, &slashings, 1000); + for (att1, att2, att3) in [ + ( + indexed_att(&v1, 0, 1, 0), + indexed_att(&v2, 0, 1, 1), + indexed_att(&v3, 0, 1, 0), + ), + ( + indexed_att_electra(&v1, 0, 1, 0), + indexed_att_electra(&v2, 0, 1, 1), + indexed_att_electra(&v3, 0, 1, 0), + ), + ] { + let slashings = hashset![att_slashing(&att1, &att2)]; + let attestations = vec![att1, att2, att3]; + slasher_test_indiv(&attestations, &slashings, 1); + slasher_test_indiv(&attestations, &slashings, 1000); + } } // Nobody double votes, nobody gets slashed. @@ -67,11 +97,17 @@ fn double_vote_some_vals_repeat() { fn no_double_vote_same_target() { let v1 = vec![0, 1, 2, 3, 4, 5, 6]; let v2 = vec![0, 1, 2, 3, 4, 5, 7, 8]; - let att1 = indexed_att(v1, 0, 1, 0); - let att2 = indexed_att(v2, 0, 1, 0); - let attestations = vec![att1, att2]; - slasher_test_indiv(&attestations, &hashset! {}, 1); - slasher_test_indiv(&attestations, &hashset! {}, 1000); + for (att1, att2) in [ + (indexed_att(&v1, 0, 1, 0), indexed_att(&v2, 0, 1, 0)), + ( + indexed_att_electra(&v1, 0, 1, 0), + indexed_att_electra(&v2, 0, 1, 0), + ), + ] { + let attestations = vec![att1, att2]; + slasher_test_indiv(&attestations, &hashset! {}, 1); + slasher_test_indiv(&attestations, &hashset! {}, 1000); + } } // Two groups votes for different things, no slashings. @@ -79,73 +115,133 @@ fn no_double_vote_same_target() { fn no_double_vote_distinct_vals() { let v1 = vec![0, 1, 2, 3]; let v2 = vec![4, 5, 6, 7]; - let att1 = indexed_att(v1, 0, 1, 0); - let att2 = indexed_att(v2, 0, 1, 1); - let attestations = vec![att1, att2]; - slasher_test_indiv(&attestations, &hashset! {}, 1); - slasher_test_indiv(&attestations, &hashset! {}, 1000); + for (att1, att2) in [ + (indexed_att(&v1, 0, 1, 0), indexed_att(&v2, 0, 1, 0)), + ( + indexed_att_electra(&v1, 0, 1, 0), + indexed_att_electra(&v2, 0, 1, 1), + ), + ] { + let attestations = vec![att1, att2]; + slasher_test_indiv(&attestations, &hashset! {}, 1); + slasher_test_indiv(&attestations, &hashset! {}, 1000); + } } #[test] fn no_double_vote_repeated() { let v = vec![0, 1, 2, 3, 4]; - let att1 = indexed_att(v, 0, 1, 0); - let att2 = att1.clone(); - let attestations = vec![att1, att2]; - slasher_test_indiv(&attestations, &hashset! {}, 1); - slasher_test_batch(&attestations, &hashset! {}, 1); - parallel_slasher_test(&attestations, hashset! {}, 1); + for att1 in [indexed_att(&v, 0, 1, 0), indexed_att_electra(&v, 0, 1, 0)] { + let att2 = att1.clone(); + let attestations = vec![att1, att2]; + slasher_test_indiv(&attestations, &hashset! {}, 1); + slasher_test_batch(&attestations, &hashset! {}, 1); + parallel_slasher_test(&attestations, hashset! {}, 1); + } } #[test] fn surrounds_existing_single_val_single_chunk() { let v = vec![0]; - let att1 = indexed_att(&v, 1, 2, 0); - let att2 = indexed_att(&v, 0, 3, 0); - let slashings = hashset![att_slashing(&att2, &att1)]; - slasher_test_indiv(&[att1, att2], &slashings, 3); + for (att1, att2) in [ + (indexed_att(&v, 1, 2, 0), indexed_att(&v, 0, 3, 0)), + (indexed_att(&v, 1, 2, 0), indexed_att_electra(&v, 0, 3, 0)), + ( + indexed_att_electra(&v, 1, 2, 0), + indexed_att_electra(&v, 0, 3, 0), + ), + ] { + let slashings = hashset![att_slashing(&att2, &att1)]; + slasher_test_indiv(&[att1, att2], &slashings, 3); + } } #[test] fn surrounds_existing_multi_vals_single_chunk() { let validators = vec![0, 16, 1024, 300_000, 300_001]; - let att1 = indexed_att(validators.clone(), 1, 2, 0); - let att2 = indexed_att(validators, 0, 3, 0); - let slashings = hashset![att_slashing(&att2, &att1)]; - slasher_test_indiv(&[att1, att2], &slashings, 3); + for (att1, att2) in [ + ( + indexed_att(&validators, 1, 2, 0), + indexed_att(&validators, 0, 3, 0), + ), + ( + indexed_att(&validators, 1, 2, 0), + indexed_att_electra(&validators, 0, 3, 0), + ), + ( + indexed_att_electra(&validators, 1, 2, 0), + indexed_att_electra(&validators, 0, 3, 0), + ), + ] { + let slashings = hashset![att_slashing(&att2, &att1)]; + slasher_test_indiv(&[att1, att2], &slashings, 3); + } } #[test] fn surrounds_existing_many_chunks() { let v = vec![0]; let chunk_size = DEFAULT_CHUNK_SIZE as u64; - let att1 = indexed_att(&v, 3 * chunk_size, 3 * chunk_size + 1, 0); - let att2 = indexed_att(&v, 0, 3 * chunk_size + 2, 0); - let slashings = hashset![att_slashing(&att2, &att1)]; - let attestations = vec![att1, att2]; - slasher_test_indiv(&attestations, &slashings, 4 * chunk_size); + for (att1, att2) in [ + ( + indexed_att(&v, 3 * chunk_size, 3 * chunk_size + 1, 0), + indexed_att(&v, 0, 3 * chunk_size + 2, 0), + ), + ( + indexed_att(&v, 3 * chunk_size, 3 * chunk_size + 1, 0), + indexed_att_electra(&v, 0, 3 * chunk_size + 2, 0), + ), + ( + indexed_att_electra(&v, 3 * chunk_size, 3 * chunk_size + 1, 0), + indexed_att_electra(&v, 0, 3 * chunk_size + 2, 0), + ), + ] { + let slashings = hashset![att_slashing(&att2, &att1)]; + let attestations = vec![att1, att2]; + slasher_test_indiv(&attestations, &slashings, 4 * chunk_size); + } } #[test] fn surrounded_by_single_val_single_chunk() { let v = vec![0]; - let att1 = indexed_att(&v, 0, 15, 0); - let att2 = indexed_att(&v, 1, 14, 0); - let slashings = hashset![att_slashing(&att1, &att2)]; - let attestations = vec![att1, att2]; - slasher_test_indiv(&attestations, &slashings, 15); + for (att1, att2) in [ + (indexed_att(&v, 0, 15, 0), indexed_att(&v, 1, 14, 0)), + (indexed_att(&v, 0, 15, 0), indexed_att_electra(&v, 1, 14, 0)), + ( + indexed_att_electra(&v, 0, 15, 0), + indexed_att_electra(&v, 1, 14, 0), + ), + ] { + let slashings = hashset![att_slashing(&att1, &att2)]; + let attestations = vec![att1, att2]; + slasher_test_indiv(&attestations, &slashings, 15); + } } #[test] fn surrounded_by_single_val_multi_chunk() { let v = vec![0]; let chunk_size = DEFAULT_CHUNK_SIZE as u64; - let att1 = indexed_att(&v, 0, 3 * chunk_size, 0); - let att2 = indexed_att(&v, chunk_size, chunk_size + 1, 0); - let slashings = hashset![att_slashing(&att1, &att2)]; - let attestations = vec![att1, att2]; - slasher_test_indiv(&attestations, &slashings, 3 * chunk_size); - slasher_test_indiv(&attestations, &slashings, 4 * chunk_size); + for (att1, att2) in [ + ( + indexed_att(&v, 0, 3 * chunk_size, 0), + indexed_att(&v, chunk_size, chunk_size + 1, 0), + ), + ( + indexed_att(&v, 0, 3 * chunk_size, 0), + indexed_att_electra(&v, chunk_size, chunk_size + 1, 0), + ), + ( + indexed_att_electra(&v, 0, 3 * chunk_size, 0), + indexed_att_electra(&v, chunk_size, chunk_size + 1, 0), + ), + ] { + let slashings = hashset![att_slashing(&att1, &att2)]; + let attestations = vec![att1, att2]; + slasher_test_indiv(&attestations, &slashings, 3 * chunk_size); + slasher_test_indiv(&attestations, &slashings, 4 * chunk_size); + } } // Process each attestation individually, and confirm that the slashings produced are as expected. diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index b3b20d6efe..2a2cc067e5 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -131,7 +131,6 @@ pub struct ForkChoiceTest { pub anchor_state: BeaconState, pub anchor_block: BeaconBlock, #[allow(clippy::type_complexity)] - // TODO(electra): these tests will need to be updated to use new types pub steps: Vec< Step, BlobsList, Attestation, AttesterSlashing, PowBlock>, >, @@ -180,42 +179,34 @@ impl LoadCase for ForkChoiceTest { valid, }) } - Step::Attestation { attestation } => match fork_name { - ForkName::Base - | ForkName::Altair - | ForkName::Bellatrix - | ForkName::Capella - | ForkName::Deneb => ssz_decode_file( - &path.join(format!("{}.ssz_snappy", attestation)), - ) - .map(|attestation| Step::Attestation { - attestation: Attestation::Base(attestation), - }), - ForkName::Electra => ssz_decode_file( - &path.join(format!("{}.ssz_snappy", attestation)), - ) - .map(|attestation| Step::Attestation { - attestation: Attestation::Electra(attestation), - }), - }, - Step::AttesterSlashing { attester_slashing } => match fork_name { - ForkName::Base - | ForkName::Altair - | ForkName::Bellatrix - | ForkName::Capella - | ForkName::Deneb => { + Step::Attestation { attestation } => { + if fork_name.electra_enabled() { + ssz_decode_file(&path.join(format!("{}.ssz_snappy", attestation))).map( + |attestation| Step::Attestation { + attestation: Attestation::Electra(attestation), + }, + ) + } else { + ssz_decode_file(&path.join(format!("{}.ssz_snappy", attestation))).map( + |attestation| Step::Attestation { + attestation: Attestation::Base(attestation), + }, + ) + } + } + Step::AttesterSlashing { attester_slashing } => { + if fork_name.electra_enabled() { + ssz_decode_file(&path.join(format!("{}.ssz_snappy", attester_slashing))) + .map(|attester_slashing| Step::AttesterSlashing { + attester_slashing: AttesterSlashing::Electra(attester_slashing), + }) + } else { ssz_decode_file(&path.join(format!("{}.ssz_snappy", attester_slashing))) .map(|attester_slashing| Step::AttesterSlashing { attester_slashing: AttesterSlashing::Base(attester_slashing), }) } - ForkName::Electra => { - ssz_decode_file(&path.join(format!("{}.ssz_snappy", attester_slashing))) - .map(|attester_slashing| Step::AttesterSlashing { - attester_slashing: AttesterSlashing::Electra(attester_slashing), - }) - } - }, + } Step::PowBlock { pow_block } => { ssz_decode_file(&path.join(format!("{}.ssz_snappy", pow_block))) .map(|pow_block| Step::PowBlock { pow_block }) diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index ab033b1342..80b5cb458a 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -247,7 +247,6 @@ mod ssz_static { PendingPartialWithdrawal, *, }; - ssz_static_test!(attestation, Attestation<_>); ssz_static_test!(attestation_data, AttestationData); ssz_static_test!(beacon_block, SszStaticWithSpecHandler, BeaconBlock<_>); ssz_static_test!(beacon_block_header, BeaconBlockHeader); @@ -274,6 +273,16 @@ mod ssz_static { ssz_static_test!(validator, Validator); ssz_static_test!(voluntary_exit, VoluntaryExit); + #[test] + fn attestation() { + SszStaticHandler::, MinimalEthSpec>::pre_electra().run(); + SszStaticHandler::, MainnetEthSpec>::pre_electra().run(); + SszStaticHandler::, MinimalEthSpec>::electra_only() + .run(); + SszStaticHandler::, MainnetEthSpec>::electra_only() + .run(); + } + #[test] fn attester_slashing() { SszStaticHandler::, MinimalEthSpec>::pre_electra() diff --git a/testing/execution_engine_integration/src/nethermind.rs b/testing/execution_engine_integration/src/nethermind.rs index aad37c32bd..c3b8651789 100644 --- a/testing/execution_engine_integration/src/nethermind.rs +++ b/testing/execution_engine_integration/src/nethermind.rs @@ -11,7 +11,7 @@ use unused_port::unused_tcp4_port; /// We've pinned the Nethermind version since our method of using the `master` branch to /// find the latest tag isn't working. It appears Nethermind don't always tag on `master`. /// We should fix this so we always pull the latest version of Nethermind. -const NETHERMIND_BRANCH: &str = "release/1.21.0"; +const NETHERMIND_BRANCH: &str = "release/1.27.0"; const NETHERMIND_REPO_URL: &str = "https://github.com/NethermindEth/nethermind"; fn build_result(repo_dir: &Path) -> Output { @@ -70,11 +70,10 @@ impl NethermindEngine { .join("nethermind") .join("src") .join("Nethermind") - .join("Nethermind.Runner") + .join("artifacts") .join("bin") - .join("Release") - .join("net7.0") - .join("linux-x64") + .join("Nethermind.Runner") + .join("release") .join("nethermind") } } diff --git a/testing/simulator/src/main.rs b/testing/simulator/src/main.rs index 03ee902c77..a259ac1133 100644 --- a/testing/simulator/src/main.rs +++ b/testing/simulator/src/main.rs @@ -10,9 +10,6 @@ //! simulation uses `println` to communicate some info. It might be nice if the nodes logged to //! easy-to-find files and stdout only contained info from the simulation. //! - -extern crate clap; - mod basic_sim; mod checks; mod cli; diff --git a/validator_client/slashing_protection/src/lib.rs b/validator_client/slashing_protection/src/lib.rs index c4fa32b611..e5606d4042 100644 --- a/validator_client/slashing_protection/src/lib.rs +++ b/validator_client/slashing_protection/src/lib.rs @@ -67,9 +67,9 @@ impl From for SigningRoot { } } -impl Into for SigningRoot { - fn into(self) -> Hash256 { - self.0 +impl From for Hash256 { + fn from(from: SigningRoot) -> Hash256 { + from.0 } } diff --git a/validator_client/src/attestation_service.rs b/validator_client/src/attestation_service.rs index bf6b2c49a2..1a6504dd49 100644 --- a/validator_client/src/attestation_service.rs +++ b/validator_client/src/attestation_service.rs @@ -14,11 +14,7 @@ use std::ops::Deref; use std::sync::Arc; use tokio::time::{sleep, sleep_until, Duration, Instant}; use tree_hash::TreeHash; -use types::ForkName; -use types::{ - attestation::AttestationBase, AggregateSignature, Attestation, AttestationData, - AttestationElectra, BitList, BitVector, ChainSpec, CommitteeIndex, EthSpec, Slot, -}; +use types::{Attestation, AttestationData, ChainSpec, CommitteeIndex, EthSpec, Slot}; /// Builds an `AttestationService`. pub struct AttestationServiceBuilder { @@ -367,17 +363,8 @@ impl AttestationService { let duty = &duty_and_proof.duty; let attestation_data = attestation_data_ref; - let fork_name = self - .context - .eth2_config - .spec - .fork_name_at_slot::(attestation_data.slot); - // Ensure that the attestation matches the duties. - #[allow(clippy::suspicious_operation_groupings)] - if duty.slot != attestation_data.slot - || (fork_name < ForkName::Electra && duty.committee_index != attestation_data.index) - { + if !duty.match_attestation_data::(attestation_data, &self.context.eth2_config.spec) { crit!( log, "Inconsistent validator duties during signing"; @@ -390,25 +377,26 @@ impl AttestationService { return None; } - let mut attestation = if fork_name >= ForkName::Electra { - let mut committee_bits: BitVector = BitVector::default(); - committee_bits - .set(duty.committee_index as usize, true) - .unwrap(); - Attestation::Electra(AttestationElectra { - aggregation_bits: BitList::with_capacity(duty.committee_length as usize) - .unwrap(), - data: attestation_data.clone(), - committee_bits, - signature: AggregateSignature::infinity(), - }) - } else { - Attestation::Base(AttestationBase { - aggregation_bits: BitList::with_capacity(duty.committee_length as usize) - .unwrap(), - data: attestation_data.clone(), - signature: AggregateSignature::infinity(), - }) + let mut attestation = match Attestation::::empty_for_signing( + duty.committee_index, + duty.committee_length as usize, + attestation_data.slot, + attestation_data.beacon_block_root, + attestation_data.source, + attestation_data.target, + &self.context.eth2_config.spec, + ) { + Ok(attestation) => attestation, + Err(err) => { + crit!( + log, + "Invalid validator duties during signing"; + "validator" => ?duty.pubkey, + "duty" => ?duty, + "err" => ?err, + ); + return None; + } }; match self @@ -577,23 +565,12 @@ impl AttestationService { .await .map_err(|e| e.to_string())?; - let fork_name = self - .context - .eth2_config - .spec - .fork_name_at_slot::(attestation_data.slot); - // Create futures to produce the signed aggregated attestations. let signing_futures = validator_duties.iter().map(|duty_and_proof| async move { let duty = &duty_and_proof.duty; let selection_proof = duty_and_proof.selection_proof.as_ref()?; - let slot = attestation_data.slot; - let committee_index = attestation_data.index; - - if duty.slot != slot - || (fork_name < ForkName::Electra && duty.committee_index != committee_index) - { + if !duty.match_attestation_data::(attestation_data, &self.context.eth2_config.spec) { crit!(log, "Inconsistent validator duties during signing"); return None; } diff --git a/validator_client/src/doppelganger_service.rs b/validator_client/src/doppelganger_service.rs index 442a950ddc..9f93795e29 100644 --- a/validator_client/src/doppelganger_service.rs +++ b/validator_client/src/doppelganger_service.rs @@ -1115,7 +1115,7 @@ mod test { ) // All validators should still be disabled. .assert_all_disabled() - // The states of all validators should be jammed with `u64::max_value()`. + // The states of all validators should be jammed with `u64:MAX`. .assert_all_states(&DoppelgangerState { next_check_epoch: starting_epoch + 1, remaining_epochs: u64::MAX, @@ -1347,7 +1347,7 @@ mod test { ) .assert_all_states(&DoppelgangerState { next_check_epoch: initial_epoch + 1, - remaining_epochs: u64::max_value(), + remaining_epochs: u64::MAX, }); } diff --git a/validator_client/src/validator_store.rs b/validator_client/src/validator_store.rs index a173e5da12..e6e19b6e06 100644 --- a/validator_client/src/validator_store.rs +++ b/validator_client/src/validator_store.rs @@ -18,16 +18,12 @@ use std::sync::Arc; use task_executor::TaskExecutor; use types::{ attestation::Error as AttestationError, graffiti::GraffitiString, AbstractExecPayload, Address, - Attestation, BeaconBlock, BlindedPayload, ChainSpec, ContributionAndProof, Domain, Epoch, - EthSpec, Fork, ForkName, Graffiti, Hash256, PublicKeyBytes, SelectionProof, Signature, - SignedBeaconBlock, SignedContributionAndProof, SignedRoot, SignedValidatorRegistrationData, - SignedVoluntaryExit, Slot, SyncAggregatorSelectionData, SyncCommitteeContribution, - SyncCommitteeMessage, SyncSelectionProof, SyncSubnetId, ValidatorRegistrationData, - VoluntaryExit, -}; -use types::{ - AggregateAndProof, AggregateAndProofBase, AggregateAndProofElectra, SignedAggregateAndProof, - SignedAggregateAndProofBase, SignedAggregateAndProofElectra, + AggregateAndProof, Attestation, BeaconBlock, BlindedPayload, ChainSpec, ContributionAndProof, + Domain, Epoch, EthSpec, Fork, ForkName, Graffiti, Hash256, PublicKeyBytes, SelectionProof, + Signature, SignedAggregateAndProof, SignedBeaconBlock, SignedContributionAndProof, SignedRoot, + SignedValidatorRegistrationData, SignedVoluntaryExit, Slot, SyncAggregatorSelectionData, + SyncCommitteeContribution, SyncCommitteeMessage, SyncSelectionProof, SyncSubnetId, + ValidatorRegistrationData, VoluntaryExit, }; pub use crate::doppelganger_service::DoppelgangerStatus; @@ -805,18 +801,8 @@ impl ValidatorStore { let signing_epoch = aggregate.data().target.epoch; let signing_context = self.signing_context(Domain::AggregateAndProof, signing_epoch); - let message = match aggregate { - Attestation::Base(att) => AggregateAndProof::Base(AggregateAndProofBase { - aggregator_index, - aggregate: att, - selection_proof: selection_proof.into(), - }), - Attestation::Electra(att) => AggregateAndProof::Electra(AggregateAndProofElectra { - aggregator_index, - aggregate: att, - selection_proof: selection_proof.into(), - }), - }; + let message = + AggregateAndProof::from_attestation(aggregator_index, aggregate, selection_proof); let signing_method = self.doppelganger_checked_signing_method(validator_pubkey)?; let signature = signing_method @@ -830,17 +816,9 @@ impl ValidatorStore { metrics::inc_counter_vec(&metrics::SIGNED_AGGREGATES_TOTAL, &[metrics::SUCCESS]); - match message { - AggregateAndProof::Base(message) => { - Ok(SignedAggregateAndProof::Base(SignedAggregateAndProofBase { - message, - signature, - })) - } - AggregateAndProof::Electra(message) => Ok(SignedAggregateAndProof::Electra( - SignedAggregateAndProofElectra { message, signature }, - )), - } + Ok(SignedAggregateAndProof::from_aggregate_and_proof( + message, signature, + )) } /// Produces a `SelectionProof` for the `slot`, signed by with corresponding secret key to diff --git a/watch/src/blockprint/database.rs b/watch/src/blockprint/database.rs index afa35c81b6..f0bc3f8ac8 100644 --- a/watch/src/blockprint/database.rs +++ b/watch/src/blockprint/database.rs @@ -33,6 +33,7 @@ pub struct WatchBlockprint { } #[derive(Debug, QueryableByName, diesel::FromSqlRow)] +#[allow(dead_code)] pub struct WatchValidatorBlockprint { #[diesel(sql_type = Integer)] pub proposer_index: i32, diff --git a/watch/src/blockprint/mod.rs b/watch/src/blockprint/mod.rs index 532776f425..319090c656 100644 --- a/watch/src/blockprint/mod.rs +++ b/watch/src/blockprint/mod.rs @@ -24,6 +24,7 @@ pub use server::blockprint_routes; const TIMEOUT: Duration = Duration::from_secs(50); #[derive(Debug)] +#[allow(dead_code)] pub enum Error { Reqwest(reqwest::Error), Url(url::ParseError), diff --git a/watch/src/server/error.rs b/watch/src/server/error.rs index 0db3df2a0d..e2c8f0f42a 100644 --- a/watch/src/server/error.rs +++ b/watch/src/server/error.rs @@ -6,6 +6,7 @@ use serde_json::json; use std::io::Error as IoError; #[derive(Debug)] +#[allow(dead_code)] pub enum Error { Axum(AxumError), Hyper(HyperError), diff --git a/watch/src/updater/error.rs b/watch/src/updater/error.rs index 74091c8f21..13c83bcf01 100644 --- a/watch/src/updater/error.rs +++ b/watch/src/updater/error.rs @@ -5,6 +5,7 @@ use eth2::{Error as Eth2Error, SensitiveError}; use std::fmt; #[derive(Debug)] +#[allow(dead_code)] pub enum Error { BeaconChain(BeaconChainError), Eth2(Eth2Error), diff --git a/watch/src/updater/handler.rs b/watch/src/updater/handler.rs index a0bfc0b9a4..3ee32560ad 100644 --- a/watch/src/updater/handler.rs +++ b/watch/src/updater/handler.rs @@ -9,6 +9,7 @@ use eth2::{ }; use log::{debug, error, info, warn}; use std::collections::HashSet; +use std::marker::PhantomData; use types::{BeaconBlockHeader, EthSpec, Hash256, SignedBeaconBlock, Slot}; use crate::updater::{get_beacon_block, get_header, get_validators}; @@ -47,7 +48,7 @@ pub struct UpdateHandler { pub blockprint: Option, pub config: Config, pub slots_per_epoch: u64, - pub spec: WatchSpec, + pub _phantom: PhantomData, } impl UpdateHandler { @@ -84,7 +85,7 @@ impl UpdateHandler { blockprint, config: config.updater, slots_per_epoch: spec.slots_per_epoch(), - spec, + _phantom: PhantomData, }) }