From e1f6128bf353b649116d4e2667226cfb67c44fc2 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 17 Nov 2020 16:30:19 +1100 Subject: [PATCH 01/15] Read beacon state inside migrator --- beacon_node/beacon_chain/src/beacon_chain.rs | 1 - beacon_node/beacon_chain/src/migrate.rs | 37 +++++++++----------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 1f3a18b3e3e..ab074fbc9eb 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -2110,7 +2110,6 @@ impl BeaconChain { self.store_migrator.process_finalization( new_finalized_state_root.into(), - finalized_state, new_finalized_checkpoint, self.head_tracker.clone(), )?; diff --git a/beacon_node/beacon_chain/src/migrate.rs b/beacon_node/beacon_chain/src/migrate.rs index 00a5385d040..22233f82218 100644 --- a/beacon_node/beacon_chain/src/migrate.rs +++ b/beacon_node/beacon_chain/src/migrate.rs @@ -30,12 +30,7 @@ const COMPACTION_FINALITY_DISTANCE: u64 = 1024; pub struct BackgroundMigrator, Cold: ItemStore> { db: Arc>, #[allow(clippy::type_complexity)] - tx_thread: Option< - Mutex<( - mpsc::Sender>, - thread::JoinHandle<()>, - )>, - >, + tx_thread: Option, thread::JoinHandle<()>)>>, /// Genesis block root, for persisting the `PersistedBeaconChain`. genesis_block_root: Hash256, log: Logger, @@ -78,9 +73,8 @@ pub enum PruningError { } /// Message sent to the migration thread containing the information it needs to run. -pub struct MigrationNotification { +pub struct MigrationNotification { finalized_state_root: BeaconStateHash, - finalized_state: BeaconState, finalized_checkpoint: Checkpoint, head_tracker: Arc, genesis_block_root: Hash256, @@ -115,13 +109,11 @@ impl, Cold: ItemStore> BackgroundMigrator, finalized_checkpoint: Checkpoint, head_tracker: Arc, ) -> Result<(), BeaconChainError> { let notif = MigrationNotification { finalized_state_root, - finalized_state, finalized_checkpoint, head_tracker, genesis_block_root: self.genesis_block_root, @@ -161,13 +153,21 @@ impl, Cold: ItemStore> BackgroundMigrator>, - notif: MigrationNotification, - log: &Logger, - ) { + fn run_migration(db: Arc>, notif: MigrationNotification, log: &Logger) { let finalized_state_root = notif.finalized_state_root; - let finalized_state = notif.finalized_state; + + let finalized_state = match db.get_state(&finalized_state_root.into(), None) { + Ok(Some(state)) => state, + other => { + error!( + log, + "Migrator failed to load state"; + "state_root" => ?finalized_state_root, + "error" => ?other + ); + return; + } + }; let old_finalized_checkpoint = match Self::prune_abandoned_forks( db.clone(), @@ -231,10 +231,7 @@ impl, Cold: ItemStore> BackgroundMigrator>, log: Logger, - ) -> ( - mpsc::Sender>, - thread::JoinHandle<()>, - ) { + ) -> (mpsc::Sender, thread::JoinHandle<()>) { let (tx, rx) = mpsc::channel(); let thread = thread::spawn(move || { while let Ok(notif) = rx.recv() { From 1fa5506a65fa98ca8323d0075d23964c2959b2ae Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 17 Nov 2020 16:51:25 +1100 Subject: [PATCH 02/15] Read all messages from channel --- beacon_node/beacon_chain/src/migrate.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/migrate.rs b/beacon_node/beacon_chain/src/migrate.rs index 22233f82218..636bb9788b9 100644 --- a/beacon_node/beacon_chain/src/migrate.rs +++ b/beacon_node/beacon_chain/src/migrate.rs @@ -234,7 +234,15 @@ impl, Cold: ItemStore> BackgroundMigrator (mpsc::Sender, thread::JoinHandle<()>) { let (tx, rx) = mpsc::channel(); let thread = thread::spawn(move || { - while let Ok(notif) = rx.recv() { + while let Ok(mut notif) = rx.recv() { + // Read the rest of the messages in the channel, ultimately choosing the `notif` + // with the highest finalized epoch. + rx.try_iter().for_each(|other: MigrationNotification| { + if other.finalized_checkpoint.epoch > notif.finalized_checkpoint.epoch { + notif = other + } + }); + Self::run_migration(db.clone(), notif, &log); } }); From 478f2c27737ea889e6724af443b8d389bcb32ceb Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 17 Nov 2020 17:29:46 +1100 Subject: [PATCH 03/15] Avoid state read --- beacon_node/beacon_chain/src/beacon_chain.rs | 60 +++++++++++--------- beacon_node/operation_pool/src/lib.rs | 21 +++---- 2 files changed, 45 insertions(+), 36 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index ab074fbc9eb..da9f69093ce 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1926,25 +1926,6 @@ impl BeaconChain { }; let new_finalized_checkpoint = new_head.beacon_state.finalized_checkpoint; - // State root of the finalized state on the epoch boundary, NOT the state - // of the finalized block. We need to use an iterator in case the state is beyond - // the reach of the new head's `state_roots` array. - let new_finalized_slot = new_finalized_checkpoint - .epoch - .start_slot(T::EthSpec::slots_per_epoch()); - let new_finalized_state_root = process_results( - StateRootsIterator::new(self.store.clone(), &new_head.beacon_state), - |mut iter| { - iter.find_map(|(state_root, slot)| { - if slot == new_finalized_slot { - Some(state_root) - } else { - None - } - }) - }, - )? - .ok_or_else(|| Error::MissingFinalizedStateRoot(new_finalized_slot))?; // It is an error to try to update to a head with a lesser finalized epoch. if new_finalized_checkpoint.epoch < old_finalized_checkpoint.epoch { @@ -1991,7 +1972,37 @@ impl BeaconChain { }); if new_finalized_checkpoint.epoch != old_finalized_checkpoint.epoch { - self.after_finalization(new_finalized_checkpoint, new_finalized_state_root)?; + // Due to race conditions, it's technically possible that the head we load here is + // different to the one earlier in this function. + // + // Since the head can't move backwards in terms of finalized epoch, we can only load a + // head with a *later* finalized state. There is no harm in this. + let head = self + .canonical_head + .try_read_for(HEAD_LOCK_TIMEOUT) + .ok_or_else(|| Error::CanonicalHeadLockTimeout)?; + + // State root of the finalized state on the epoch boundary, NOT the state + // of the finalized block. We need to use an iterator in case the state is beyond + // the reach of the new head's `state_roots` array. + let new_finalized_slot = new_finalized_checkpoint + .epoch + .start_slot(T::EthSpec::slots_per_epoch()); + let new_finalized_state_root = process_results( + StateRootsIterator::new(self.store.clone(), &head.beacon_state), + |mut iter| { + iter.find_map(|(state_root, slot)| { + if slot == new_finalized_slot { + Some(state_root) + } else { + None + } + }) + }, + )? + .ok_or_else(|| Error::MissingFinalizedStateRoot(new_finalized_slot))?; + + self.after_finalization(&head.beacon_state, new_finalized_state_root)?; } let _ = self.event_handler.register(EventKind::BeaconHeadChanged { @@ -2072,10 +2083,11 @@ impl BeaconChain { /// Performs pruning and finality-based optimizations. fn after_finalization( &self, - new_finalized_checkpoint: Checkpoint, + head_state: &BeaconState, new_finalized_state_root: Hash256, ) -> Result<(), Error> { self.fork_choice.write().prune()?; + let new_finalized_checkpoint = head_state.finalized_checkpoint; self.observed_block_producers.prune( new_finalized_checkpoint @@ -2097,12 +2109,8 @@ impl BeaconChain { ); }); - let finalized_state = self - .get_state(&new_finalized_state_root, None)? - .ok_or_else(|| Error::MissingBeaconState(new_finalized_state_root))?; - self.op_pool.prune_all( - &finalized_state, + head_state, self.epoch()?, self.head_info()?.fork, &self.spec, diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index a22c7b085ff..e2f17b5bbe4 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -236,30 +236,31 @@ impl OperationPool { } /// Prune proposer slashings for all slashed or withdrawn validators. - pub fn prune_proposer_slashings(&self, finalized_state: &BeaconState) { + pub fn prune_proposer_slashings(&self, head_state: &BeaconState) { prune_validator_hash_map( &mut self.proposer_slashings.write(), |validator| { - validator.slashed || validator.is_withdrawable_at(finalized_state.current_epoch()) + validator.slashed + || validator.is_withdrawable_at(head_state.finalized_checkpoint.epoch) }, - finalized_state, + head_state, ); } /// Prune attester slashings for all slashed or withdrawn validators, or attestations on another /// fork. - pub fn prune_attester_slashings(&self, finalized_state: &BeaconState, head_fork: Fork) { + pub fn prune_attester_slashings(&self, head_state: &BeaconState, head_fork: Fork) { self.attester_slashings .write() .retain(|(slashing, fork_version)| { // Any slashings for forks older than the finalized state's previous fork can be // discarded. We allow the head_fork's current version too in case a fork has // occurred between the finalized state and the head. - let fork_ok = *fork_version == finalized_state.fork.previous_version - || *fork_version == finalized_state.fork.current_version + let fork_ok = *fork_version == head_state.fork.previous_version + || *fork_version == head_state.fork.current_version || *fork_version == head_fork.current_version; // Slashings that don't slash any validators can also be dropped. - let slashing_ok = get_slashable_indices(finalized_state, slashing).is_ok(); + let slashing_ok = get_slashable_indices(head_state, slashing).is_ok(); fork_ok && slashing_ok }); } @@ -296,11 +297,11 @@ impl OperationPool { } /// Prune if validator has already exited at the last finalized state. - pub fn prune_voluntary_exits(&self, finalized_state: &BeaconState, spec: &ChainSpec) { + pub fn prune_voluntary_exits(&self, head_state: &BeaconState, spec: &ChainSpec) { prune_validator_hash_map( &mut self.voluntary_exits.write(), - |validator| validator.exit_epoch != spec.far_future_epoch, - finalized_state, + |validator| validator.exit_epoch <= head_state.finalized_checkpoint.epoch, + head_state, ); } From 6a4f6bb78beceb26840a7a99ff00e6294b9e39a8 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 17 Nov 2020 17:36:55 +1100 Subject: [PATCH 04/15] Add missed things --- beacon_node/beacon_chain/src/beacon_chain.rs | 8 ++----- beacon_node/operation_pool/src/lib.rs | 25 ++++++++++---------- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index da9f69093ce..05923431d78 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -2109,12 +2109,8 @@ impl BeaconChain { ); }); - self.op_pool.prune_all( - head_state, - self.epoch()?, - self.head_info()?.fork, - &self.spec, - ); + self.op_pool + .prune_all(head_state, self.epoch()?, self.head_info()?.fork); self.store_migrator.process_finalization( new_finalized_state_root.into(), diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index e2f17b5bbe4..8648a44664e 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -297,26 +297,25 @@ impl OperationPool { } /// Prune if validator has already exited at the last finalized state. - pub fn prune_voluntary_exits(&self, head_state: &BeaconState, spec: &ChainSpec) { + pub fn prune_voluntary_exits(&self, head_state: &BeaconState) { prune_validator_hash_map( &mut self.voluntary_exits.write(), + // This condition is slightly too loose, since there will be some finalized exits that + // are missed here. + // + // We choose simplicity over the gain of pruning exits, which are small and should be + // infrequent. |validator| validator.exit_epoch <= head_state.finalized_checkpoint.epoch, head_state, ); } /// Prune all types of transactions given the latest finalized state and head fork. - pub fn prune_all( - &self, - finalized_state: &BeaconState, - current_epoch: Epoch, - head_fork: Fork, - spec: &ChainSpec, - ) { + pub fn prune_all(&self, head_state: &BeaconState, current_epoch: Epoch, head_fork: Fork) { self.prune_attestations(current_epoch); - self.prune_proposer_slashings(finalized_state); - self.prune_attester_slashings(finalized_state, head_fork); - self.prune_voluntary_exits(finalized_state, spec); + self.prune_proposer_slashings(head_state); + self.prune_attester_slashings(head_state, head_fork); + self.prune_voluntary_exits(head_state); } /// Total number of voluntary exits in the pool. @@ -393,12 +392,12 @@ where fn prune_validator_hash_map( map: &mut HashMap, prune_if: F, - finalized_state: &BeaconState, + head_state: &BeaconState, ) where F: Fn(&Validator) -> bool, { map.retain(|&validator_index, _| { - finalized_state + head_state .validators .get(validator_index as usize) .map_or(true, |validator| !prune_if(validator)) From 7ae9291b5ac9e7d4ff87a3c69afdbef95cb86a52 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 17 Nov 2020 17:42:14 +1100 Subject: [PATCH 05/15] Update slashing check --- beacon_node/operation_pool/src/lib.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index 8648a44664e..a8950368656 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -13,7 +13,8 @@ use max_cover::maximum_cover; use parking_lot::RwLock; use state_processing::per_block_processing::errors::AttestationValidationError; use state_processing::per_block_processing::{ - get_slashable_indices, verify_attestation_for_block_inclusion, verify_exit, VerifySignatures, + get_slashable_indices_modular, verify_attestation_for_block_inclusion, verify_exit, + VerifySignatures, }; use state_processing::SigVerifiedOp; use std::collections::{hash_map, HashMap, HashSet}; @@ -260,7 +261,11 @@ impl OperationPool { || *fork_version == head_state.fork.current_version || *fork_version == head_fork.current_version; // Slashings that don't slash any validators can also be dropped. - let slashing_ok = get_slashable_indices(head_state, slashing).is_ok(); + let slashing_ok = + get_slashable_indices_modular(head_state, slashing, |_, validator| { + validator.is_slashable_at(head_state.finalized_checkpoint.epoch) + }) + .map_or(false, |indices| !indices.is_empty()); fork_ok && slashing_ok }); } From e634c2944d8c68728bb6cd3a9536f95e5fc2784a Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 17 Nov 2020 17:43:31 +1100 Subject: [PATCH 06/15] Fix race condition --- beacon_node/beacon_chain/src/beacon_chain.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 05923431d78..d37d111635d 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1985,7 +1985,9 @@ impl BeaconChain { // State root of the finalized state on the epoch boundary, NOT the state // of the finalized block. We need to use an iterator in case the state is beyond // the reach of the new head's `state_roots` array. - let new_finalized_slot = new_finalized_checkpoint + let new_finalized_slot = head + .beacon_state + .finalized_checkpoint .epoch .start_slot(T::EthSpec::slots_per_epoch()); let new_finalized_state_root = process_results( From 96faab424f3a83beb01cc12985eb8ff54cdeeae5 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 17 Nov 2020 17:47:33 +1100 Subject: [PATCH 07/15] Use fold --- beacon_node/beacon_chain/src/migrate.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/beacon_node/beacon_chain/src/migrate.rs b/beacon_node/beacon_chain/src/migrate.rs index 636bb9788b9..bf0cd1f7dd0 100644 --- a/beacon_node/beacon_chain/src/migrate.rs +++ b/beacon_node/beacon_chain/src/migrate.rs @@ -234,14 +234,18 @@ impl, Cold: ItemStore> BackgroundMigrator (mpsc::Sender, thread::JoinHandle<()>) { let (tx, rx) = mpsc::channel(); let thread = thread::spawn(move || { - while let Ok(mut notif) = rx.recv() { + while let Ok(notif) = rx.recv() { // Read the rest of the messages in the channel, ultimately choosing the `notif` // with the highest finalized epoch. - rx.try_iter().for_each(|other: MigrationNotification| { - if other.finalized_checkpoint.epoch > notif.finalized_checkpoint.epoch { - notif = other - } - }); + let notif = rx + .try_iter() + .fold(notif, |best, other: MigrationNotification| { + if other.finalized_checkpoint.epoch > best.finalized_checkpoint.epoch { + other + } else { + best + } + }); Self::run_migration(db.clone(), notif, &log); } From 317aabdbbbb2f1cd2f2dceefff4a006289a0f577 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 17 Nov 2020 17:59:50 +1100 Subject: [PATCH 08/15] Fix fork issue, comments --- beacon_node/beacon_chain/src/beacon_chain.rs | 3 +-- beacon_node/operation_pool/src/lib.rs | 23 ++++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index d37d111635d..c97159d9a47 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -2111,8 +2111,7 @@ impl BeaconChain { ); }); - self.op_pool - .prune_all(head_state, self.epoch()?, self.head_info()?.fork); + self.op_pool.prune_all(head_state, self.epoch()?); self.store_migrator.process_finalization( new_finalized_state_root.into(), diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index a8950368656..23d9dcc6e10 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -250,16 +250,17 @@ impl OperationPool { /// Prune attester slashings for all slashed or withdrawn validators, or attestations on another /// fork. - pub fn prune_attester_slashings(&self, head_state: &BeaconState, head_fork: Fork) { + pub fn prune_attester_slashings(&self, head_state: &BeaconState) { self.attester_slashings .write() .retain(|(slashing, fork_version)| { - // Any slashings for forks older than the finalized state's previous fork can be - // discarded. We allow the head_fork's current version too in case a fork has - // occurred between the finalized state and the head. - let fork_ok = *fork_version == head_state.fork.previous_version - || *fork_version == head_state.fork.current_version - || *fork_version == head_fork.current_version; + let previous_fork_is_finalized = + head_state.finalized_checkpoint.epoch <= head_state.fork.epoch; + // Prune any slashings which don't match the current fork version, or the previous + // fork version if it is not finalized yet. + let fork_ok = (fork_version == &head_state.fork.current_version) + || (fork_version == &head_state.fork.previous_version + && previous_fork_is_finalized); // Slashings that don't slash any validators can also be dropped. let slashing_ok = get_slashable_indices_modular(head_state, slashing, |_, validator| { @@ -301,7 +302,7 @@ impl OperationPool { ) } - /// Prune if validator has already exited at the last finalized state. + /// Prune if validator has already exited at or before the finalized checkpoint of the head. pub fn prune_voluntary_exits(&self, head_state: &BeaconState) { prune_validator_hash_map( &mut self.voluntary_exits.write(), @@ -315,11 +316,11 @@ impl OperationPool { ); } - /// Prune all types of transactions given the latest finalized state and head fork. - pub fn prune_all(&self, head_state: &BeaconState, current_epoch: Epoch, head_fork: Fork) { + /// Prune all types of transactions given the latest head state and head fork. + pub fn prune_all(&self, head_state: &BeaconState, current_epoch: Epoch) { self.prune_attestations(current_epoch); self.prune_proposer_slashings(head_state); - self.prune_attester_slashings(head_state, head_fork); + self.prune_attester_slashings(head_state); self.prune_voluntary_exits(head_state); } From 72aad056289610a47539dfab517cd3caf3a21de8 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 17 Nov 2020 18:01:59 +1100 Subject: [PATCH 09/15] Update comment --- beacon_node/operation_pool/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index 23d9dcc6e10..cf5534e0343 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -309,8 +309,8 @@ impl OperationPool { // This condition is slightly too loose, since there will be some finalized exits that // are missed here. // - // We choose simplicity over the gain of pruning exits, which are small and should be - // infrequent. + // We choose simplicity over the gain of pruning more exits since they are small and + // should not be seen frequently. |validator| validator.exit_epoch <= head_state.finalized_checkpoint.epoch, head_state, ); From 73af53171545323b711d18d71fab03d8fcfff047 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 17 Nov 2020 18:03:28 +1100 Subject: [PATCH 10/15] Fix not --- beacon_node/operation_pool/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index cf5534e0343..4ca27247b08 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -260,7 +260,7 @@ impl OperationPool { // fork version if it is not finalized yet. let fork_ok = (fork_version == &head_state.fork.current_version) || (fork_version == &head_state.fork.previous_version - && previous_fork_is_finalized); + && !previous_fork_is_finalized); // Slashings that don't slash any validators can also be dropped. let slashing_ok = get_slashable_indices_modular(head_state, slashing, |_, validator| { From 6a5e1f06c4bb22e54f0743c52175a7b844916bc7 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 17 Nov 2020 18:14:25 +1100 Subject: [PATCH 11/15] Fix slashings check --- beacon_node/operation_pool/src/lib.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index 4ca27247b08..90c96d11227 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -264,9 +264,15 @@ impl OperationPool { // Slashings that don't slash any validators can also be dropped. let slashing_ok = get_slashable_indices_modular(head_state, slashing, |_, validator| { - validator.is_slashable_at(head_state.finalized_checkpoint.epoch) + // Declare that a validator is still slashable if they have not exited prior + // to the finalized epoch. + // + // We cannot check the `slashed` field since the `head` is not finalized and + // a fork could un-slash someone. + validator.exit_epoch <= head_state.finalized_checkpoint.epoch }) .map_or(false, |indices| !indices.is_empty()); + fork_ok && slashing_ok }); } From 95bd03e9f61b5d2514b4f5cdffa271c2a980122c Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 17 Nov 2020 19:04:56 +1100 Subject: [PATCH 12/15] Fix failing test --- beacon_node/operation_pool/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index 90c96d11227..308761aaeac 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -1024,7 +1024,7 @@ mod release_tests { let slashing = ctxt.attester_slashing(&[1, 3, 5, 7, 9]); op_pool .insert_attester_slashing(slashing.clone().validate(state, spec).unwrap(), state.fork); - op_pool.prune_attester_slashings(state, state.fork); + op_pool.prune_attester_slashings(state); assert_eq!(op_pool.get_slashings(state, spec).1, vec![slashing]); } From b7ddeaec37ff254ca3b6d866a7f4a8755059ed48 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 17 Nov 2020 19:22:18 +1100 Subject: [PATCH 13/15] Fix slashings --- beacon_node/operation_pool/src/lib.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index 308761aaeac..bf094510b67 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -236,14 +236,11 @@ impl OperationPool { (proposer_slashings, attester_slashings) } - /// Prune proposer slashings for all slashed or withdrawn validators. + /// Prune proposer slashings for validators which are exited in the finalized epoch. pub fn prune_proposer_slashings(&self, head_state: &BeaconState) { prune_validator_hash_map( &mut self.proposer_slashings.write(), - |validator| { - validator.slashed - || validator.is_withdrawable_at(head_state.finalized_checkpoint.epoch) - }, + |validator| validator.exit_epoch <= head_state.finalized_checkpoint.epoch, head_state, ); } @@ -269,7 +266,7 @@ impl OperationPool { // // We cannot check the `slashed` field since the `head` is not finalized and // a fork could un-slash someone. - validator.exit_epoch <= head_state.finalized_checkpoint.epoch + validator.exit_epoch > head_state.finalized_checkpoint.epoch }) .map_or(false, |indices| !indices.is_empty()); From 906c79bb569de3da0e8bafe7f29713f22799fee0 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 17 Nov 2020 19:28:18 +1100 Subject: [PATCH 14/15] Fix fork check --- beacon_node/operation_pool/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index bf094510b67..45c16cc2fc9 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -252,7 +252,7 @@ impl OperationPool { .write() .retain(|(slashing, fork_version)| { let previous_fork_is_finalized = - head_state.finalized_checkpoint.epoch <= head_state.fork.epoch; + head_state.finalized_checkpoint.epoch >= head_state.fork.epoch; // Prune any slashings which don't match the current fork version, or the previous // fork version if it is not finalized yet. let fork_ok = (fork_version == &head_state.fork.current_version) From 93c1c835b3aad9c63abc0b1f035163e218f868c4 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 18 Nov 2020 09:58:40 +1100 Subject: [PATCH 15/15] Fix import issue after rebase --- beacon_node/beacon_chain/src/migrate.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/migrate.rs b/beacon_node/beacon_chain/src/migrate.rs index bf0cd1f7dd0..92f8efe435f 100644 --- a/beacon_node/beacon_chain/src/migrate.rs +++ b/beacon_node/beacon_chain/src/migrate.rs @@ -3,7 +3,7 @@ use crate::errors::BeaconChainError; use crate::head_tracker::{HeadTracker, SszHeadTracker}; use crate::persisted_beacon_chain::{PersistedBeaconChain, DUMMY_CANONICAL_HEAD_BLOCK_ROOT}; use parking_lot::Mutex; -use slog::{debug, info, warn, Logger}; +use slog::{debug, error, info, warn, Logger}; use std::collections::{HashMap, HashSet}; use std::mem; use std::sync::{mpsc, Arc};