diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index c846de11f86..74c082cac7c 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -2059,16 +2059,36 @@ impl BeaconChain { )?) } - /// Accept some attester slashing and queue it for inclusion in an appropriate block. + /// Accept a verified attester slashing and: + /// + /// 1. Apply it to fork choice. + /// 2. Add it to the op pool. pub fn import_attester_slashing( &self, attester_slashing: SigVerifiedOp>, ) { - if self.eth1_chain.is_some() { - self.op_pool.insert_attester_slashing( - attester_slashing, - self.canonical_head.cached_head().head_fork(), + let cached_head = self.canonical_head.cached_head(); + + // Add to fork choice. + if let Err(e) = self + .canonical_head + .fork_choice_write_lock() + .on_attester_slashing( + attester_slashing.as_inner(), + &cached_head.snapshot.beacon_state, ) + { + warn!( + self.log, + "Unable to apply attester slashing to fork choice"; + "error" => ?e, + ); + } + + // Add to the op pool (if we have the ability to propose blocks). + if self.eth1_chain.is_some() { + self.op_pool + .insert_attester_slashing(attester_slashing, cached_head.head_fork()) } } @@ -2526,6 +2546,7 @@ impl BeaconChain { payload_verification_status: PayloadVerificationStatus, count_unrealized: CountUnrealized, ) -> Result> { + // FIXME(sproul): apply attester slashing (and attestations) to fork choice let current_slot = self.slot()?; let current_epoch = current_slot.epoch(T::EthSpec::slots_per_epoch()); 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 0d65b8aa62e..e5c13e9b4e0 100644 --- a/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs +++ b/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs @@ -8,6 +8,7 @@ use crate::{metrics, BeaconSnapshot}; use derivative::Derivative; use fork_choice::ForkChoiceStore; use ssz_derive::{Decode, Encode}; +use std::collections::BTreeSet; use std::marker::PhantomData; use std::sync::Arc; use store::{Error as StoreError, HotColdDB, ItemStore}; @@ -158,6 +159,7 @@ pub struct BeaconForkChoiceStore, Cold: ItemStore< unrealized_justified_checkpoint: Checkpoint, unrealized_finalized_checkpoint: Checkpoint, proposer_boost_root: Hash256, + equivocating_indices: BTreeSet, _phantom: PhantomData, } @@ -206,6 +208,7 @@ where unrealized_justified_checkpoint: justified_checkpoint, unrealized_finalized_checkpoint: finalized_checkpoint, proposer_boost_root: Hash256::zero(), + equivocating_indices: BTreeSet::new(), _phantom: PhantomData, } } @@ -223,6 +226,7 @@ where 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.clone(), } } @@ -242,6 +246,7 @@ where unrealized_justified_checkpoint: persisted.unrealized_justified_checkpoint, unrealized_finalized_checkpoint: persisted.unrealized_finalized_checkpoint, proposer_boost_root: persisted.proposer_boost_root, + equivocating_indices: persisted.equivocating_indices, _phantom: PhantomData, }) } @@ -350,30 +355,40 @@ where fn set_proposer_boost_root(&mut self, proposer_boost_root: Hash256) { self.proposer_boost_root = proposer_boost_root; } + + fn equivocating_indices(&self) -> &BTreeSet { + &self.equivocating_indices + } + + fn extend_equivocating_indices(&mut self, indices: Vec) { + self.equivocating_indices.extend(indices); + } } /// A container which allows persisting the `BeaconForkChoiceStore` to the on-disk database. #[superstruct( - variants(V1, V7, V8, V10), + variants(V1, V7, V8, V10, V11), variant_attributes(derive(Encode, Decode)), no_enum )] pub struct PersistedForkChoiceStore { #[superstruct(only(V1, V7))] pub balances_cache: BalancesCacheV1, - #[superstruct(only(V8, V10))] + #[superstruct(only(V8, V10, V11))] pub balances_cache: BalancesCacheV8, pub time: Slot, pub finalized_checkpoint: Checkpoint, pub justified_checkpoint: Checkpoint, pub justified_balances: Vec, pub best_justified_checkpoint: Checkpoint, - #[superstruct(only(V10))] + #[superstruct(only(V10, V11))] pub unrealized_justified_checkpoint: Checkpoint, - #[superstruct(only(V10))] + #[superstruct(only(V10, V11))] pub unrealized_finalized_checkpoint: Checkpoint, - #[superstruct(only(V7, V8, V10))] + #[superstruct(only(V7, V8, V10, V11))] pub proposer_boost_root: Hash256, + #[superstruct(only(V11))] + pub equivocating_indices: BTreeSet, } -pub type PersistedForkChoiceStore = PersistedForkChoiceStoreV10; +pub type PersistedForkChoiceStore = PersistedForkChoiceStoreV11; diff --git a/beacon_node/beacon_chain/src/persisted_fork_choice.rs b/beacon_node/beacon_chain/src/persisted_fork_choice.rs index eb5078df2c8..a60dacdc7c0 100644 --- a/beacon_node/beacon_chain/src/persisted_fork_choice.rs +++ b/beacon_node/beacon_chain/src/persisted_fork_choice.rs @@ -1,6 +1,6 @@ use crate::beacon_fork_choice_store::{ - PersistedForkChoiceStoreV1, PersistedForkChoiceStoreV10, PersistedForkChoiceStoreV7, - PersistedForkChoiceStoreV8, + PersistedForkChoiceStoreV1, PersistedForkChoiceStoreV10, PersistedForkChoiceStoreV11, + PersistedForkChoiceStoreV7, PersistedForkChoiceStoreV8, }; use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; @@ -8,10 +8,10 @@ use store::{DBColumn, Error, StoreItem}; use superstruct::superstruct; // If adding a new version you should update this type alias and fix the breakages. -pub type PersistedForkChoice = PersistedForkChoiceV10; +pub type PersistedForkChoice = PersistedForkChoiceV11; #[superstruct( - variants(V1, V7, V8, V10), + variants(V1, V7, V8, V10, V11), variant_attributes(derive(Encode, Decode)), no_enum )] @@ -25,6 +25,8 @@ pub struct PersistedForkChoice { pub fork_choice_store: PersistedForkChoiceStoreV8, #[superstruct(only(V10))] pub fork_choice_store: PersistedForkChoiceStoreV10, + #[superstruct(only(V11))] + pub fork_choice_store: PersistedForkChoiceStoreV11, } macro_rules! impl_store_item { @@ -49,3 +51,4 @@ impl_store_item!(PersistedForkChoiceV1); impl_store_item!(PersistedForkChoiceV7); impl_store_item!(PersistedForkChoiceV8); impl_store_item!(PersistedForkChoiceV10); +impl_store_item!(PersistedForkChoiceV11); diff --git a/beacon_node/beacon_chain/src/schema_change.rs b/beacon_node/beacon_chain/src/schema_change.rs index 411ef947d9b..b6c70b5435e 100644 --- a/beacon_node/beacon_chain/src/schema_change.rs +++ b/beacon_node/beacon_chain/src/schema_change.rs @@ -1,5 +1,6 @@ //! Utilities for managing database schema changes. mod migration_schema_v10; +mod migration_schema_v11; mod migration_schema_v6; mod migration_schema_v7; mod migration_schema_v8; @@ -8,7 +9,8 @@ mod types; use crate::beacon_chain::{BeaconChainTypes, FORK_CHOICE_DB_KEY}; use crate::persisted_fork_choice::{ - PersistedForkChoiceV1, PersistedForkChoiceV10, PersistedForkChoiceV7, PersistedForkChoiceV8, + PersistedForkChoiceV1, PersistedForkChoiceV10, PersistedForkChoiceV11, PersistedForkChoiceV7, + PersistedForkChoiceV8, }; use crate::types::ChainSpec; use slog::{warn, Logger}; @@ -36,6 +38,12 @@ pub fn migrate_schema( migrate_schema::(db.clone(), datadir, from, next, log.clone(), spec)?; migrate_schema::(db, datadir, next, to, log, spec) } + // Downgrade across multiple versions by recursively migrating one step at a time. + (_, _) if to.as_u64() + 1 < from.as_u64() => { + let next = SchemaVersion(from.as_u64() - 1); + migrate_schema::(db.clone(), datadir, from, next, log.clone(), spec)?; + migrate_schema::(db, datadir, next, to, log, spec) + } // // Migrations from before SchemaVersion(5) are deprecated. @@ -159,6 +167,35 @@ pub fn migrate_schema( Ok(()) } + // Upgrade from v10 to v11 adding support for equivocating indices to fork choice. + (SchemaVersion(10), SchemaVersion(11)) => { + let mut ops = vec![]; + let fork_choice_opt = db.get_item::(&FORK_CHOICE_DB_KEY)?; + if let Some(fork_choice) = fork_choice_opt { + let updated_fork_choice = migration_schema_v11::update_fork_choice(fork_choice); + + ops.push(updated_fork_choice.as_kv_store_op(FORK_CHOICE_DB_KEY)); + } + + db.store_schema_version_atomically(to, ops)?; + + Ok(()) + } + // Downgrade from v11 to v10 removing support for equivocating indices from fork choice. + (SchemaVersion(11), SchemaVersion(10)) => { + let mut ops = vec![]; + let fork_choice_opt = db.get_item::(&FORK_CHOICE_DB_KEY)?; + if let Some(fork_choice) = fork_choice_opt { + let updated_fork_choice = + migration_schema_v11::downgrade_fork_choice(fork_choice, log); + + ops.push(updated_fork_choice.as_kv_store_op(FORK_CHOICE_DB_KEY)); + } + + db.store_schema_version_atomically(to, ops)?; + + Ok(()) + } // Anything else is an error. (_, _) => Err(HotColdDBError::UnsupportedSchemaVersion { target_version: to, diff --git a/beacon_node/beacon_chain/src/schema_change/migration_schema_v11.rs b/beacon_node/beacon_chain/src/schema_change/migration_schema_v11.rs new file mode 100644 index 00000000000..dde80a5cac7 --- /dev/null +++ b/beacon_node/beacon_chain/src/schema_change/migration_schema_v11.rs @@ -0,0 +1,77 @@ +use crate::beacon_fork_choice_store::{PersistedForkChoiceStoreV10, PersistedForkChoiceStoreV11}; +use crate::persisted_fork_choice::{PersistedForkChoiceV10, PersistedForkChoiceV11}; +use slog::{warn, Logger}; +use std::collections::BTreeSet; + +/// Add the equivocating indices field. +pub fn update_fork_choice(fork_choice_v10: PersistedForkChoiceV10) -> PersistedForkChoiceV11 { + let PersistedForkChoiceStoreV10 { + balances_cache, + time, + finalized_checkpoint, + justified_checkpoint, + justified_balances, + best_justified_checkpoint, + unrealized_justified_checkpoint, + unrealized_finalized_checkpoint, + proposer_boost_root, + } = fork_choice_v10.fork_choice_store; + + PersistedForkChoiceV11 { + fork_choice: fork_choice_v10.fork_choice, + fork_choice_store: PersistedForkChoiceStoreV11 { + balances_cache, + time, + finalized_checkpoint, + justified_checkpoint, + justified_balances, + best_justified_checkpoint, + unrealized_justified_checkpoint, + unrealized_finalized_checkpoint, + proposer_boost_root, + equivocating_indices: BTreeSet::new(), + }, + } +} + +pub fn downgrade_fork_choice( + fork_choice_v11: PersistedForkChoiceV11, + log: Logger, +) -> PersistedForkChoiceV10 { + let PersistedForkChoiceStoreV11 { + balances_cache, + time, + finalized_checkpoint, + justified_checkpoint, + justified_balances, + best_justified_checkpoint, + unrealized_justified_checkpoint, + unrealized_finalized_checkpoint, + proposer_boost_root, + equivocating_indices, + } = fork_choice_v11.fork_choice_store; + + if !equivocating_indices.is_empty() { + warn!( + log, + "Deleting slashed validators from fork choice store"; + "count" => equivocating_indices.len(), + "message" => "this may make your node more susceptible to following the wrong chain", + ); + } + + PersistedForkChoiceV10 { + fork_choice: fork_choice_v11.fork_choice, + fork_choice_store: PersistedForkChoiceStoreV10 { + balances_cache, + time, + finalized_checkpoint, + justified_checkpoint, + justified_balances, + best_justified_checkpoint, + unrealized_justified_checkpoint, + unrealized_finalized_checkpoint, + proposer_boost_root, + }, + } +} diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 0fb2e1ad05f..b06e9933abf 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1,19 +1,23 @@ use crate::{ForkChoiceStore, InvalidationOperation}; use proto_array::{Block as ProtoBlock, ExecutionStatus, ProtoArrayForkChoice}; use ssz_derive::{Decode, Encode}; -use state_processing::per_epoch_processing; +use state_processing::{ + per_block_processing::{errors::AttesterSlashingValidationError, get_slashable_indices}, + per_epoch_processing, +}; use std::cmp::Ordering; use std::marker::PhantomData; use std::time::Duration; use types::{ - consts::merge::INTERVALS_PER_SLOT, AttestationShufflingId, BeaconBlockRef, BeaconState, - BeaconStateError, ChainSpec, Checkpoint, Epoch, EthSpec, ExecPayload, ExecutionBlockHash, - Hash256, IndexedAttestation, RelativeEpoch, SignedBeaconBlock, Slot, + consts::merge::INTERVALS_PER_SLOT, AttestationShufflingId, AttesterSlashing, BeaconBlockRef, + BeaconState, BeaconStateError, ChainSpec, Checkpoint, Epoch, EthSpec, ExecPayload, + ExecutionBlockHash, Hash256, IndexedAttestation, RelativeEpoch, SignedBeaconBlock, Slot, }; #[derive(Debug)] pub enum Error { InvalidAttestation(InvalidAttestation), + InvalidAttesterSlashing(AttesterSlashingValidationError), InvalidBlock(InvalidBlock), ProtoArrayError(String), InvalidProtoArrayBytes(String), @@ -63,6 +67,12 @@ impl From for Error { } } +impl From for Error { + fn from(e: AttesterSlashingValidationError) -> Self { + Error::InvalidAttesterSlashing(e) + } +} + impl From for Error { fn from(e: state_processing::EpochProcessingError) -> Self { Error::UnrealizedVoteProcessing(e) @@ -413,26 +423,6 @@ where Ok(fork_choice) } - /* - /// Instantiates `Self` from some existing components. - /// - /// This is useful if the existing components have been loaded from disk after a process - /// restart. - pub fn from_components( - fc_store: T, - proto_array: ProtoArrayForkChoice, - queued_attestations: Vec, - ) -> Self { - Self { - fc_store, - proto_array, - queued_attestations, - forkchoice_update_parameters: None, - _phantom: PhantomData, - } - } - */ - /// Returns cached information that can be used to issue a `forkchoiceUpdated` message to an /// execution engine. /// @@ -507,6 +497,7 @@ where *store.finalized_checkpoint(), store.justified_balances(), store.proposer_boost_root(), + store.equivocating_indices(), current_slot, spec, )?; @@ -1109,6 +1100,25 @@ where Ok(()) } + pub fn on_attester_slashing( + &mut self, + slashing: &AttesterSlashing, + head_state: &BeaconState, + ) -> Result<(), Error> { + // We assume that the attester slashing provided to this function has already been verified. + // + // Instead of loading the justified state as per the spec, we use the head state that the + // slashing was verified against. The differences should be negligible, if a validator is + // slashable at the justified state but not at the head state then their withdrawable epoch + // must fall between justification and the head, which means they are incapable of attesting + // anyway. If a validator is slashable at the head but not at justification then it means + // their activation has occurred since justification, and we arguably shouldn't be counting + // their attestations. + let slashed_indices = get_slashable_indices(head_state, slashing)?; + self.fc_store.extend_equivocating_indices(slashed_indices); + Ok(()) + } + /// Call `on_tick` for all slots between `fc_store.get_current_slot()` and the provided /// `current_slot`. Returns the value of `self.fc_store.get_current_slot`. pub fn update_time( @@ -1325,8 +1335,6 @@ where // If the parent block has execution enabled, always import the block. // - // TODO(bellatrix): this condition has not yet been merged into the spec. - // // See: // // https://github.com/ethereum/consensus-specs/pull/2844 diff --git a/consensus/fork_choice/src/fork_choice_store.rs b/consensus/fork_choice/src/fork_choice_store.rs index a7085b024a5..6d9b5e9efa5 100644 --- a/consensus/fork_choice/src/fork_choice_store.rs +++ b/consensus/fork_choice/src/fork_choice_store.rs @@ -1,3 +1,4 @@ +use std::collections::BTreeSet; use types::{BeaconBlockRef, BeaconState, Checkpoint, EthSpec, ExecPayload, Hash256, Slot}; /// Approximates the `Store` in "Ethereum 2.0 Phase 0 -- Beacon Chain Fork Choice": @@ -76,4 +77,10 @@ pub trait ForkChoiceStore: Sized { /// Sets the proposer boost root. fn set_proposer_boost_root(&mut self, proposer_boost_root: Hash256); + + /// Gets the equivocating indices. + fn equivocating_indices(&self) -> &BTreeSet; + + /// Adds to the set of equivocating indices. + fn extend_equivocating_indices(&mut self, indices: Vec); } diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index 0cfa3a194f7..fcb1b94d6fa 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -6,6 +6,7 @@ mod votes; use crate::proto_array_fork_choice::{Block, ExecutionStatus, ProtoArrayForkChoice}; use crate::InvalidationOperation; use serde_derive::{Deserialize, Serialize}; +use std::collections::BTreeSet; use types::{ AttestationShufflingId, Checkpoint, Epoch, EthSpec, ExecutionBlockHash, Hash256, MainnetEthSpec, Slot, @@ -88,6 +89,7 @@ impl ForkChoiceTestDefinition { ExecutionStatus::Optimistic(ExecutionBlockHash::zero()), ) .expect("should create fork choice struct"); + let equivocating_indices = BTreeSet::new(); for (op_index, op) in self.operations.into_iter().enumerate() { match op.clone() { @@ -103,6 +105,7 @@ impl ForkChoiceTestDefinition { finalized_checkpoint, &justified_state_balances, Hash256::zero(), + &equivocating_indices, Slot::new(0), &spec, ) @@ -130,6 +133,7 @@ impl ForkChoiceTestDefinition { finalized_checkpoint, &justified_state_balances, proposer_boost_root, + &equivocating_indices, Slot::new(0), &spec, ) @@ -154,6 +158,7 @@ impl ForkChoiceTestDefinition { finalized_checkpoint, &justified_state_balances, Hash256::zero(), + &equivocating_indices, Slot::new(0), &spec, ); diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 568cfa9640e..1c20c40a7cb 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -4,7 +4,7 @@ use crate::ssz_container::SszContainer; use serde_derive::{Deserialize, Serialize}; use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; -use std::collections::HashMap; +use std::collections::{BTreeSet, HashMap}; use types::{ AttestationShufflingId, ChainSpec, Checkpoint, Epoch, EthSpec, ExecutionBlockHash, Hash256, Slot, @@ -260,12 +260,14 @@ impl ProtoArrayForkChoice { .map_err(|e| format!("process_block_error: {:?}", e)) } + #[allow(clippy::too_many_arguments)] pub fn find_head( &mut self, justified_checkpoint: Checkpoint, finalized_checkpoint: Checkpoint, justified_state_balances: &[u64], proposer_boost_root: Hash256, + equivocating_indices: &BTreeSet, current_slot: Slot, spec: &ChainSpec, ) -> Result { @@ -278,6 +280,7 @@ impl ProtoArrayForkChoice { &mut self.votes, old_balances, new_balances, + equivocating_indices, ) .map_err(|e| format!("find_head compute_deltas failed: {:?}", e))?; @@ -439,6 +442,7 @@ fn compute_deltas( votes: &mut ElasticList, old_balances: &[u64], new_balances: &[u64], + equivocating_indices: &BTreeSet, ) -> Result, Error> { let mut deltas = vec![0_i64; indices.len()]; @@ -449,6 +453,38 @@ fn compute_deltas( continue; } + // Handle newly slashed validators by deducting their weight from their current vote. We + // determine if they are newly slashed by checking whether their `vote.current_root` is + // non-zero. After applying the deduction a single time we set their `current_root` to zero + // and never update it again (thus preventing repeat deductions). + // + // Even if they make new attestations which are processed by `process_attestation` these + // will only update their `vote.next_root`. + if equivocating_indices.contains(&(val_index as u64)) { + // First time we've processed this slashing in fork choice: + // + // 1. Add a negative delta for their `current_root`. + // 2. Set their `current_root` (permanently) to zero. + if !vote.current_root.is_zero() { + let old_balance = old_balances.get(val_index).copied().unwrap_or(0); + + if let Some(current_delta_index) = indices.get(&vote.current_root).copied() { + let delta = deltas + .get(current_delta_index) + .ok_or(Error::InvalidNodeDelta(current_delta_index))? + .checked_sub(old_balance as i64) + .ok_or(Error::DeltaOverflow(current_delta_index))?; + + // Array access safe due to check on previous line. + deltas[current_delta_index] = delta; + } + + vote.current_root = Hash256::zero(); + } + // We've handled this slashed validator, continue without applying an ordinary delta. + continue; + } + // If the validator was not included in the _old_ balances (i.e., it did not exist yet) // then say its balance was zero. let old_balance = old_balances.get(val_index).copied().unwrap_or(0); @@ -605,6 +641,7 @@ mod test_compute_deltas { let mut votes = ElasticList::default(); let mut old_balances = vec![]; let mut new_balances = vec![]; + let equivocating_indices = BTreeSet::new(); for i in 0..validator_count { indices.insert(hash_from_index(i), i); @@ -617,8 +654,14 @@ mod test_compute_deltas { new_balances.push(0); } - let deltas = compute_deltas(&indices, &mut votes, &old_balances, &new_balances) - .expect("should compute deltas"); + let deltas = compute_deltas( + &indices, + &mut votes, + &old_balances, + &new_balances, + &equivocating_indices, + ) + .expect("should compute deltas"); assert_eq!( deltas.len(), @@ -649,6 +692,7 @@ mod test_compute_deltas { let mut votes = ElasticList::default(); let mut old_balances = vec![]; let mut new_balances = vec![]; + let equivocating_indices = BTreeSet::new(); for i in 0..validator_count { indices.insert(hash_from_index(i), i); @@ -661,8 +705,14 @@ mod test_compute_deltas { new_balances.push(BALANCE); } - let deltas = compute_deltas(&indices, &mut votes, &old_balances, &new_balances) - .expect("should compute deltas"); + let deltas = compute_deltas( + &indices, + &mut votes, + &old_balances, + &new_balances, + &equivocating_indices, + ) + .expect("should compute deltas"); assert_eq!( deltas.len(), @@ -700,6 +750,7 @@ mod test_compute_deltas { let mut votes = ElasticList::default(); let mut old_balances = vec![]; let mut new_balances = vec![]; + let equivocating_indices = BTreeSet::new(); for i in 0..validator_count { indices.insert(hash_from_index(i), i); @@ -712,8 +763,14 @@ mod test_compute_deltas { new_balances.push(BALANCE); } - let deltas = compute_deltas(&indices, &mut votes, &old_balances, &new_balances) - .expect("should compute deltas"); + let deltas = compute_deltas( + &indices, + &mut votes, + &old_balances, + &new_balances, + &equivocating_indices, + ) + .expect("should compute deltas"); assert_eq!( deltas.len(), @@ -746,6 +803,7 @@ mod test_compute_deltas { let mut votes = ElasticList::default(); let mut old_balances = vec![]; let mut new_balances = vec![]; + let equivocating_indices = BTreeSet::new(); for i in 0..validator_count { indices.insert(hash_from_index(i), i); @@ -758,8 +816,14 @@ mod test_compute_deltas { new_balances.push(BALANCE); } - let deltas = compute_deltas(&indices, &mut votes, &old_balances, &new_balances) - .expect("should compute deltas"); + let deltas = compute_deltas( + &indices, + &mut votes, + &old_balances, + &new_balances, + &equivocating_indices, + ) + .expect("should compute deltas"); assert_eq!( deltas.len(), @@ -797,6 +861,7 @@ mod test_compute_deltas { let mut indices = HashMap::new(); let mut votes = ElasticList::default(); + let equivocating_indices = BTreeSet::new(); // There is only one block. indices.insert(hash_from_index(1), 0); @@ -819,8 +884,14 @@ mod test_compute_deltas { next_epoch: Epoch::new(0), }); - let deltas = compute_deltas(&indices, &mut votes, &old_balances, &new_balances) - .expect("should compute deltas"); + let deltas = compute_deltas( + &indices, + &mut votes, + &old_balances, + &new_balances, + &equivocating_indices, + ) + .expect("should compute deltas"); assert_eq!(deltas.len(), 1, "deltas should have expected length"); @@ -849,6 +920,7 @@ mod test_compute_deltas { let mut votes = ElasticList::default(); let mut old_balances = vec![]; let mut new_balances = vec![]; + let equivocating_indices = BTreeSet::new(); for i in 0..validator_count { indices.insert(hash_from_index(i), i); @@ -861,8 +933,14 @@ mod test_compute_deltas { new_balances.push(NEW_BALANCE); } - let deltas = compute_deltas(&indices, &mut votes, &old_balances, &new_balances) - .expect("should compute deltas"); + let deltas = compute_deltas( + &indices, + &mut votes, + &old_balances, + &new_balances, + &equivocating_indices, + ) + .expect("should compute deltas"); assert_eq!( deltas.len(), @@ -902,6 +980,7 @@ mod test_compute_deltas { let mut indices = HashMap::new(); let mut votes = ElasticList::default(); + let equivocating_indices = BTreeSet::new(); // There are two blocks. indices.insert(hash_from_index(1), 0); @@ -921,8 +1000,14 @@ mod test_compute_deltas { }); } - let deltas = compute_deltas(&indices, &mut votes, &old_balances, &new_balances) - .expect("should compute deltas"); + let deltas = compute_deltas( + &indices, + &mut votes, + &old_balances, + &new_balances, + &equivocating_indices, + ) + .expect("should compute deltas"); assert_eq!(deltas.len(), 2, "deltas should have expected length"); @@ -951,6 +1036,7 @@ mod test_compute_deltas { let mut indices = HashMap::new(); let mut votes = ElasticList::default(); + let equivocating_indices = BTreeSet::new(); // There are two blocks. indices.insert(hash_from_index(1), 0); @@ -970,8 +1056,14 @@ mod test_compute_deltas { }); } - let deltas = compute_deltas(&indices, &mut votes, &old_balances, &new_balances) - .expect("should compute deltas"); + let deltas = compute_deltas( + &indices, + &mut votes, + &old_balances, + &new_balances, + &equivocating_indices, + ) + .expect("should compute deltas"); assert_eq!(deltas.len(), 2, "deltas should have expected length"); @@ -992,4 +1084,6 @@ mod test_compute_deltas { ); } } + + // FIXME(sproul): add equivocation test here } diff --git a/consensus/state_processing/src/verify_operation.rs b/consensus/state_processing/src/verify_operation.rs index 25c2839edd3..e2ec984a493 100644 --- a/consensus/state_processing/src/verify_operation.rs +++ b/consensus/state_processing/src/verify_operation.rs @@ -12,7 +12,7 @@ use types::{ /// Wrapper around an operation type that acts as proof that its signature has been checked. /// /// The inner field is private, meaning instances of this type can only be constructed -/// by calling `validate`. +/// by calling `validate` or using `new_unsafe` (which should only be used in tests). #[derive(Debug, PartialEq, Eq, Clone)] pub struct SigVerifiedOp(T); @@ -24,6 +24,10 @@ impl SigVerifiedOp { pub fn as_inner(&self) -> &T { &self.0 } + + pub fn new_unsafe(val: T) -> Self { + SigVerifiedOp(val) + } } /// Trait for operations that can be verified and transformed into a `SigVerifiedOp`. diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 74469f9706c..77dfa8a2f74 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -11,7 +11,7 @@ use beacon_chain::{ }; use serde_derive::Deserialize; use ssz_derive::Decode; -use state_processing::state_advance::complete_state_advance; +use state_processing::{state_advance::complete_state_advance, SigVerifiedOp}; use std::future::Future; use std::sync::Arc; use std::time::Duration; @@ -119,10 +119,6 @@ impl LoadCase for ForkChoiceTest { ssz_decode_file(&path.join(format!("{}.ssz_snappy", pow_block))) .map(|pow_block| Step::PowBlock { pow_block }) } - Step::AttesterSlashing { attester_slashing } => { - ssz_decode_file(&path.join(format!("{}.ssz_snappy", attester_slashing))) - .map(|attester_slashing| Step::AttesterSlashing { attester_slashing }) - } Step::Checks { checks } => Ok(Step::Checks { checks }), }) .collect::>()?; @@ -163,10 +159,7 @@ impl Case for ForkChoiceTest { // TODO(merge): re-enable this test before production. // This test is skipped until we can do retrospective confirmations of the terminal // block after an optimistic sync. - if self.description == "block_lookup_failed" - //TODO(sean): enable once we implement equivocation logic (https://github.com/sigp/lighthouse/issues/3241) - || self.description == "discard_equivocations" - { + if self.description == "block_lookup_failed" { return Err(Error::SkippedKnownFailure); }; @@ -179,13 +172,9 @@ impl Case for ForkChoiceTest { } Step::Attestation { attestation } => tester.process_attestation(attestation)?, Step::AttesterSlashing { attester_slashing } => { - // FIXME(sproul): do something + tester.process_attester_slashing(attester_slashing.clone()) } Step::PowBlock { pow_block } => tester.process_pow_block(pow_block), - //TODO(sean): enable once we implement equivocation logic (https://github.com/sigp/lighthouse/issues/3241) - Step::AttesterSlashing { - attester_slashing: _, - } => (), Step::Checks { checks } => { let Checks { head, @@ -445,6 +434,13 @@ impl Tester { .map_err(|e| Error::InternalError(format!("attestation import failed with {:?}", e))) } + pub fn process_attester_slashing(&self, attester_slashing: AttesterSlashing) { + let verified_slashing = SigVerifiedOp::new_unsafe(attester_slashing); + self.harness + .chain + .import_attester_slashing(verified_slashing) + } + pub fn process_pow_block(&self, pow_block: &PowBlock) { let el = self.harness.mock_execution_layer.as_ref().unwrap();