diff --git a/Cargo.lock b/Cargo.lock index 753763fe96e..90196ea5b31 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7653,6 +7653,7 @@ dependencies = [ "safe_arith", "serde", "slog", + "ssz_types", "strum", "tempfile", "tree_hash", diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 4c7a499697d..d9c9a3b6a74 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -2,7 +2,9 @@ use beacon_chain::block_verification_types::{AsBlock, ExecutedBlock, RpcBlock}; use beacon_chain::{ - test_utils::{AttestationStrategy, BeaconChainHarness, BlockStrategy, EphemeralHarnessType}, + test_utils::{ + test_spec, AttestationStrategy, BeaconChainHarness, BlockStrategy, EphemeralHarnessType, + }, AvailabilityProcessingStatus, BeaconChain, BeaconChainTypes, ExecutionPendingBlock, }; use beacon_chain::{ @@ -1210,8 +1212,14 @@ async fn block_gossip_verification() { #[tokio::test] async fn verify_block_for_gossip_slashing_detection() { let slasher_dir = tempdir().unwrap(); + let spec = Arc::new(test_spec::()); let slasher = Arc::new( - Slasher::open(SlasherConfig::new(slasher_dir.path().into()), test_logger()).unwrap(), + Slasher::open( + SlasherConfig::new(slasher_dir.path().into()), + spec, + test_logger(), + ) + .unwrap(), ); let inner_slasher = slasher.clone(); diff --git a/beacon_node/src/lib.rs b/beacon_node/src/lib.rs index ab400d2e730..385da5b4fe1 100644 --- a/beacon_node/src/lib.rs +++ b/beacon_node/src/lib.rs @@ -80,7 +80,7 @@ impl ProductionBeaconNode { let builder = ClientBuilder::new(context.eth_spec_instance.clone()) .runtime_context(context) - .chain_spec(spec) + .chain_spec(spec.clone()) .beacon_processor(client_config.beacon_processor.clone()) .http_api_config(client_config.http_api.clone()) .disk_store( @@ -113,8 +113,12 @@ impl ProductionBeaconNode { _ => {} } let slasher = Arc::new( - Slasher::open(slasher_config, log.new(slog::o!("service" => "slasher"))) - .map_err(|e| format!("Slasher open error: {:?}", e))?, + Slasher::open( + slasher_config, + Arc::new(spec), + log.new(slog::o!("service" => "slasher")), + ) + .map_err(|e| format!("Slasher open error: {:?}", e))?, ); builder.slasher(slasher) } else { diff --git a/consensus/types/src/indexed_attestation.rs b/consensus/types/src/indexed_attestation.rs index 900c9059389..19d15010753 100644 --- a/consensus/types/src/indexed_attestation.rs +++ b/consensus/types/src/indexed_attestation.rs @@ -240,38 +240,6 @@ 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::*; diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index 2a73ff0f37a..75456446bf2 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -176,8 +176,7 @@ 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, - IndexedAttestationOnDisk, IndexedAttestationRef, IndexedAttestationRefOnDisk, + IndexedAttestation, IndexedAttestationBase, IndexedAttestationElectra, IndexedAttestationRef, }; pub use crate::light_client_bootstrap::{ LightClientBootstrap, LightClientBootstrapAltair, LightClientBootstrapCapella, diff --git a/slasher/Cargo.toml b/slasher/Cargo.toml index ef5cb8249ec..563c4599d8b 100644 --- a/slasher/Cargo.toml +++ b/slasher/Cargo.toml @@ -29,6 +29,7 @@ tree_hash = { workspace = true } tree_hash_derive = { workspace = true } types = { workspace = true } strum = { workspace = true } +ssz_types = { workspace = true } # MDBX is pinned at the last version with Windows and macOS support. mdbx = { package = "libmdbx", git = "https://github.com/sigp/libmdbx-rs", tag = "v0.1.4", optional = true } diff --git a/slasher/src/database.rs b/slasher/src/database.rs index 16089706a00..801abe9283b 100644 --- a/slasher/src/database.rs +++ b/slasher/src/database.rs @@ -13,17 +13,19 @@ use parking_lot::Mutex; use serde::de::DeserializeOwned; use slog::{info, Logger}; use ssz::{Decode, Encode}; +use ssz_derive::{Decode, Encode}; use std::borrow::{Borrow, Cow}; use std::marker::PhantomData; use std::sync::Arc; use tree_hash::TreeHash; use types::{ - Epoch, EthSpec, Hash256, IndexedAttestation, IndexedAttestationOnDisk, - IndexedAttestationRefOnDisk, ProposerSlashing, SignedBeaconBlockHeader, Slot, + AggregateSignature, AttestationData, ChainSpec, Epoch, EthSpec, Hash256, IndexedAttestation, + IndexedAttestationBase, IndexedAttestationElectra, ProposerSlashing, SignedBeaconBlockHeader, + Slot, VariableList, }; /// Current database schema version, to check compatibility of on-disk DB with software. -pub const CURRENT_SCHEMA_VERSION: u64 = 4; +pub const CURRENT_SCHEMA_VERSION: u64 = 3; /// Metadata about the slashing database itself. const METADATA_DB: &str = "metadata"; @@ -70,6 +72,7 @@ pub struct SlasherDB { /// LRU cache mapping indexed attestation IDs to their attestation data roots. attestation_root_cache: Mutex>, pub(crate) config: Arc, + pub(crate) spec: Arc, _phantom: PhantomData, } @@ -236,6 +239,43 @@ impl AsRef<[u8]> for IndexedAttestationId { } } +/// Indexed attestation that abstracts over Phase0 and Electra variants by using a plain `Vec` for +/// the attesting indices. +/// +/// This allows us to avoid rewriting the entire indexed attestation database at Electra, which +/// saves a lot of execution time. The bytes that it encodes to are the same as the bytes that a +/// regular IndexedAttestation encodes to, because SSZ doesn't care about the length-bound. +#[derive(Debug, PartialEq, Decode, Encode)] +pub struct IndexedAttestationOnDisk { + attesting_indices: Vec, + data: AttestationData, + signature: AggregateSignature, +} + +impl IndexedAttestationOnDisk { + fn into_indexed_attestation( + self, + spec: &ChainSpec, + ) -> Result, Error> { + let fork_at_target_epoch = spec.fork_name_at_epoch(self.data.target.epoch); + if fork_at_target_epoch.electra_enabled() { + let attesting_indices = VariableList::new(self.attesting_indices)?; + Ok(IndexedAttestation::Electra(IndexedAttestationElectra { + attesting_indices, + data: self.data, + signature: self.signature, + })) + } else { + let attesting_indices = VariableList::new(self.attesting_indices)?; + Ok(IndexedAttestation::Base(IndexedAttestationBase { + attesting_indices, + data: self.data, + signature: self.signature, + })) + } + } +} + /// Bincode deserialization specialised to `Cow<[u8]>`. fn bincode_deserialize(bytes: Cow<[u8]>) -> Result { Ok(bincode::deserialize(bytes.borrow())?) @@ -246,7 +286,7 @@ fn ssz_decode(bytes: Cow<[u8]>) -> Result { } impl SlasherDB { - pub fn open(config: Arc, log: Logger) -> Result { + pub fn open(config: Arc, spec: Arc, log: Logger) -> Result { info!(log, "Opening slasher database"; "backend" => %config.backend); std::fs::create_dir_all(&config.database_path)?; @@ -269,6 +309,7 @@ impl SlasherDB { databases, attestation_root_cache, config, + spec, _phantom: PhantomData, }; @@ -458,9 +499,8 @@ impl SlasherDB { }; let attestation_key = IndexedAttestationId::new(indexed_att_id); - let indexed_attestation_on_disk: IndexedAttestationRefOnDisk = - indexed_attestation.into(); - let data = indexed_attestation_on_disk.as_ssz_bytes(); + // IndexedAttestationOnDisk and IndexedAttestation have compatible encodings. + let data = indexed_attestation.as_ssz_bytes(); cursor.put(attestation_key.as_ref(), &data)?; drop(cursor); @@ -484,8 +524,8 @@ impl SlasherDB { .ok_or(Error::MissingIndexedAttestation { id: indexed_attestation_id.as_u64(), })?; - let indexed_attestation: IndexedAttestationOnDisk = ssz_decode(bytes)?; - Ok(indexed_attestation.into()) + let indexed_attestation_on_disk: IndexedAttestationOnDisk = ssz_decode(bytes)?; + indexed_attestation_on_disk.into_indexed_attestation(&self.spec) } fn get_attestation_data_root( @@ -775,3 +815,93 @@ impl SlasherDB { Ok(()) } } + +#[cfg(test)] +mod test { + use super::*; + use types::{Checkpoint, ForkName, MainnetEthSpec, Unsigned}; + + type E = MainnetEthSpec; + + fn indexed_attestation_on_disk_roundtrip_test( + spec: &ChainSpec, + make_attestation: fn( + Vec, + AttestationData, + AggregateSignature, + ) -> IndexedAttestation, + committee_len: u64, + ) { + let attestation_data = AttestationData { + slot: Slot::new(1000), + index: 0, + beacon_block_root: Hash256::repeat_byte(0xaa), + source: Checkpoint { + epoch: Epoch::new(0), + root: Hash256::repeat_byte(0xbb), + }, + target: Checkpoint { + epoch: Epoch::new(31), + root: Hash256::repeat_byte(0xcc), + }, + }; + + let attesting_indices = (0..committee_len).collect::>(); + let signature = AggregateSignature::infinity(); + + let fork_attestation = make_attestation( + attesting_indices.clone(), + attestation_data.clone(), + signature.clone(), + ); + + let on_disk = IndexedAttestationOnDisk { + attesting_indices, + data: attestation_data, + signature, + }; + let encoded = on_disk.as_ssz_bytes(); + assert_eq!(encoded, fork_attestation.as_ssz_bytes()); + + let decoded_on_disk = IndexedAttestationOnDisk::from_ssz_bytes(&encoded).unwrap(); + assert_eq!(decoded_on_disk, on_disk); + + let decoded = on_disk.into_indexed_attestation(spec).unwrap(); + assert_eq!(decoded, fork_attestation); + } + + /// Check that `IndexedAttestationOnDisk` and `IndexedAttestation` have compatible encodings. + #[test] + fn indexed_attestation_on_disk_roundtrip_base() { + let spec = ForkName::Base.make_genesis_spec(E::default_spec()); + let make_attestation = |attesting_indices, data, signature| { + IndexedAttestation::::Base(IndexedAttestationBase { + attesting_indices: VariableList::new(attesting_indices).unwrap(), + data, + signature, + }) + }; + indexed_attestation_on_disk_roundtrip_test( + &spec, + make_attestation, + ::MaxValidatorsPerCommittee::to_u64(), + ) + } + + #[test] + fn indexed_attestation_on_disk_roundtrip_electra() { + let spec = ForkName::Electra.make_genesis_spec(E::default_spec()); + let make_attestation = |attesting_indices, data, signature| { + IndexedAttestation::::Electra(IndexedAttestationElectra { + attesting_indices: VariableList::new(attesting_indices).unwrap(), + data, + signature, + }) + }; + indexed_attestation_on_disk_roundtrip_test( + &spec, + make_attestation, + ::MaxValidatorsPerSlot::to_u64(), + ) + } +} diff --git a/slasher/src/error.rs b/slasher/src/error.rs index b939c281e9f..8d3295b22a8 100644 --- a/slasher/src/error.rs +++ b/slasher/src/error.rs @@ -13,6 +13,7 @@ pub enum Error { DatabaseIOError(io::Error), DatabasePermissionsError(filesystem::Error), SszDecodeError(ssz::DecodeError), + SszTypesError(ssz_types::Error), BincodeError(bincode::Error), ArithError(safe_arith::ArithError), ChunkIndexOutOfBounds(usize), @@ -100,6 +101,12 @@ impl From for Error { } } +impl From for Error { + fn from(e: ssz_types::Error) -> Self { + Error::SszTypesError(e) + } +} + impl From for Error { fn from(e: bincode::Error) -> Self { Error::BincodeError(e) diff --git a/slasher/src/migrate.rs b/slasher/src/migrate.rs index ce298caaf2c..674ab9c132d 100644 --- a/slasher/src/migrate.rs +++ b/slasher/src/migrate.rs @@ -17,10 +17,6 @@ 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 fc8e8453c89..0bb7c9c3ffe 100644 --- a/slasher/src/slasher.rs +++ b/slasher/src/slasher.rs @@ -13,7 +13,8 @@ use slog::{debug, error, info, Logger}; use std::collections::HashSet; use std::sync::Arc; use types::{ - AttesterSlashing, Epoch, EthSpec, IndexedAttestation, ProposerSlashing, SignedBeaconBlockHeader, + AttesterSlashing, ChainSpec, Epoch, EthSpec, IndexedAttestation, ProposerSlashing, + SignedBeaconBlockHeader, }; #[derive(Debug)] @@ -28,10 +29,10 @@ pub struct Slasher { } impl Slasher { - pub fn open(config: Config, log: Logger) -> Result { + pub fn open(config: Config, spec: Arc, log: Logger) -> Result { config.validate()?; let config = Arc::new(config); - let db = SlasherDB::open(config.clone(), log.clone())?; + let db = SlasherDB::open(config.clone(), spec, log.clone())?; let attester_slashings = Mutex::new(HashSet::new()); let proposer_slashings = Mutex::new(HashSet::new()); let attestation_queue = AttestationQueue::default(); diff --git a/slasher/src/test_utils.rs b/slasher/src/test_utils.rs index 7019a8aed76..453d0e66670 100644 --- a/slasher/src/test_utils.rs +++ b/slasher/src/test_utils.rs @@ -1,9 +1,10 @@ use std::collections::HashSet; +use std::sync::Arc; use types::{ indexed_attestation::{IndexedAttestationBase, IndexedAttestationElectra}, AggregateSignature, AttestationData, AttesterSlashing, AttesterSlashingBase, - AttesterSlashingElectra, BeaconBlockHeader, Checkpoint, Epoch, Hash256, IndexedAttestation, - MainnetEthSpec, Signature, SignedBeaconBlockHeader, Slot, + AttesterSlashingElectra, BeaconBlockHeader, ChainSpec, Checkpoint, Epoch, EthSpec, Hash256, + IndexedAttestation, MainnetEthSpec, Signature, SignedBeaconBlockHeader, Slot, }; pub type E = MainnetEthSpec; @@ -145,3 +146,7 @@ pub fn block(slot: u64, proposer_index: u64, block_root: u64) -> SignedBeaconBlo signature: Signature::empty(), } } + +pub fn chain_spec() -> Arc { + Arc::new(E::default_spec()) +} diff --git a/slasher/tests/attester_slashings.rs b/slasher/tests/attester_slashings.rs index b74e491e4b3..902141d9710 100644 --- a/slasher/tests/attester_slashings.rs +++ b/slasher/tests/attester_slashings.rs @@ -6,7 +6,8 @@ use rayon::prelude::*; use slasher::{ config::DEFAULT_CHUNK_SIZE, test_utils::{ - att_slashing, indexed_att, indexed_att_electra, slashed_validators_from_slashings, E, + att_slashing, chain_spec, indexed_att, indexed_att_electra, + slashed_validators_from_slashings, E, }, Config, Slasher, }; @@ -270,7 +271,8 @@ fn slasher_test( ) { let tempdir = tempdir().unwrap(); let config = Config::new(tempdir.path().into()); - let slasher = Slasher::open(config, test_logger()).unwrap(); + let spec = chain_spec(); + let slasher = Slasher::open(config, spec, test_logger()).unwrap(); let current_epoch = Epoch::new(current_epoch); for (i, attestation) in attestations.iter().enumerate() { @@ -299,7 +301,8 @@ fn parallel_slasher_test( ) { let tempdir = tempdir().unwrap(); let config = Config::new(tempdir.path().into()); - let slasher = Slasher::open(config, test_logger()).unwrap(); + let spec = chain_spec(); + let slasher = Slasher::open(config, spec, test_logger()).unwrap(); let current_epoch = Epoch::new(current_epoch); attestations diff --git a/slasher/tests/proposer_slashings.rs b/slasher/tests/proposer_slashings.rs index 3b7b8ed583c..2d2738087dc 100644 --- a/slasher/tests/proposer_slashings.rs +++ b/slasher/tests/proposer_slashings.rs @@ -2,7 +2,7 @@ use logging::test_logger; use slasher::{ - test_utils::{block as test_block, E}, + test_utils::{block as test_block, chain_spec, E}, Config, Slasher, }; use tempfile::tempdir; @@ -12,7 +12,8 @@ use types::{Epoch, EthSpec}; fn empty_pruning() { let tempdir = tempdir().unwrap(); let config = Config::new(tempdir.path().into()); - let slasher = Slasher::::open(config, test_logger()).unwrap(); + let spec = chain_spec(); + let slasher = Slasher::::open(config, spec, test_logger()).unwrap(); slasher.prune_database(Epoch::new(0)).unwrap(); } @@ -24,8 +25,9 @@ fn block_pruning() { let mut config = Config::new(tempdir.path().into()); config.chunk_size = 2; config.history_length = 2; + let spec = chain_spec(); - let slasher = Slasher::::open(config.clone(), test_logger()).unwrap(); + let slasher = Slasher::::open(config.clone(), spec, test_logger()).unwrap(); let current_epoch = Epoch::from(2 * config.history_length); // Pruning the empty database should be safe. diff --git a/slasher/tests/random.rs b/slasher/tests/random.rs index ce0e42df1d3..ebfe0ef4e97 100644 --- a/slasher/tests/random.rs +++ b/slasher/tests/random.rs @@ -4,7 +4,7 @@ use logging::test_logger; use rand::prelude::*; use slasher::{ test_utils::{ - block, indexed_att, slashed_validators_from_attestations, + block, chain_spec, indexed_att, slashed_validators_from_attestations, slashed_validators_from_slashings, E, }, Config, Slasher, @@ -49,7 +49,9 @@ fn random_test(seed: u64, test_config: TestConfig) { config.chunk_size = 1 << chunk_size_exponent; config.history_length = 1 << rng.gen_range(chunk_size_exponent..chunk_size_exponent + 3); - let slasher = Slasher::::open(config.clone(), test_logger()).unwrap(); + let spec = chain_spec(); + + let slasher = Slasher::::open(config.clone(), spec, test_logger()).unwrap(); let validators = (0..num_validators as u64).collect::>(); diff --git a/slasher/tests/wrap_around.rs b/slasher/tests/wrap_around.rs index d2c876d3630..9a42aeb60ba 100644 --- a/slasher/tests/wrap_around.rs +++ b/slasher/tests/wrap_around.rs @@ -1,7 +1,10 @@ #![cfg(any(feature = "mdbx", feature = "lmdb"))] use logging::test_logger; -use slasher::{test_utils::indexed_att, Config, Slasher}; +use slasher::{ + test_utils::{chain_spec, indexed_att}, + Config, Slasher, +}; use tempfile::tempdir; use types::Epoch; @@ -9,11 +12,12 @@ use types::Epoch; fn attestation_pruning_empty_wrap_around() { let tempdir = tempdir().unwrap(); let mut config = Config::new(tempdir.path().into()); + let spec = chain_spec(); config.validator_chunk_size = 1; config.chunk_size = 16; config.history_length = 16; - let slasher = Slasher::open(config.clone(), test_logger()).unwrap(); + let slasher = Slasher::open(config.clone(), spec, test_logger()).unwrap(); let v = vec![0]; let history_length = config.history_length as u64;