From 2170476b18dd289b84a8f5e5204209267bb308e8 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Mon, 28 Nov 2022 13:50:50 +0000 Subject: [PATCH 1/4] RollingSession: add fn contains Signed-off-by: Andrei Sandu --- .../src/rolling_session_window.rs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index beac31292b7d..4ebfad405b5b 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -294,6 +294,11 @@ impl RollingSessionWindow { self.earliest_session + (self.session_info.len() as SessionIndex).saturating_sub(1) } + /// Returns `true` if `session_index` is contained in the window. + pub fn contains(&self, session_index: SessionIndex) -> bool { + session_index >= self.earliest_session() && session_index <= self.latest_session() + } + async fn earliest_non_finalized_block_session( sender: &mut Sender, ) -> Result @@ -783,6 +788,21 @@ mod tests { cache_session_info_test(1, 2, Some(window), 2, None); } + #[test] + fn cache_session_window_contains() { + let window = RollingSessionWindow { + earliest_session: 10, + session_info: vec![dummy_session_info(1)], + window_size: SESSION_WINDOW_SIZE, + db_params: Some(dummy_db_params()), + }; + + assert!(!window.contains(0)); + assert!(!window.contains(10 + SESSION_WINDOW_SIZE.get())); + assert!(!window.contains(11)); + assert!(!window.contains(10 + SESSION_WINDOW_SIZE.get() - 1)); + } + #[test] fn cache_session_info_first_late() { cache_session_info_test( From 1e2fa600bd13eb02b705b8ad3f0fb369763ad214 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Mon, 28 Nov 2022 13:52:30 +0000 Subject: [PATCH 2/4] handle_import_statements fix ancient dispute check Signed-off-by: Andrei Sandu --- node/core/dispute-coordinator/src/initialized.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/node/core/dispute-coordinator/src/initialized.rs b/node/core/dispute-coordinator/src/initialized.rs index ab9faca39868..bf385b802e8c 100644 --- a/node/core/dispute-coordinator/src/initialized.rs +++ b/node/core/dispute-coordinator/src/initialized.rs @@ -27,7 +27,7 @@ use sc_keystore::LocalKeystore; use polkadot_node_primitives::{ CandidateVotes, DisputeMessage, DisputeMessageCheckError, DisputeStatus, - SignedDisputeStatement, Timestamp, DISPUTE_WINDOW, + SignedDisputeStatement, Timestamp, }; use polkadot_node_subsystem::{ messages::{ @@ -299,7 +299,7 @@ impl Initialized { self.highest_session = session; - db::v1::note_current_session(overlay_db, session)?; + db::v1::note_earliest_session(overlay_db, new_window_start)?; self.spam_slots.prune_old(new_window_start); } }, @@ -708,7 +708,7 @@ impl Initialized { now: Timestamp, ) -> Result { gum::trace!(target: LOG_TARGET, ?statements, "In handle import statements"); - if session + DISPUTE_WINDOW.get() < self.highest_session { + if !self.rolling_session_window.contains(session) { // It is not valid to participate in an ancient dispute (spam?). return Ok(ImportStatementsResult::InvalidImport) } From d090ac865a3219b4a651d9c7b9b751ab4d20d83d Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Mon, 28 Nov 2022 13:54:01 +0000 Subject: [PATCH 3/4] work with earliest session instead of latest Signed-off-by: Andrei Sandu --- node/core/dispute-coordinator/src/db/v1.rs | 24 +++++++++++----------- node/core/dispute-coordinator/src/lib.rs | 4 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/node/core/dispute-coordinator/src/db/v1.rs b/node/core/dispute-coordinator/src/db/v1.rs index bb1456a59745..ab571108af37 100644 --- a/node/core/dispute-coordinator/src/db/v1.rs +++ b/node/core/dispute-coordinator/src/db/v1.rs @@ -32,7 +32,7 @@ use crate::{ backend::{Backend, BackendWriteOp, OverlayedBackend}, error::{FatalError, FatalResult}, metrics::Metrics, - DISPUTE_WINDOW, LOG_TARGET, + LOG_TARGET, }; const RECENT_DISPUTES_KEY: &[u8; 15] = b"recent-disputes"; @@ -318,25 +318,24 @@ pub(crate) fn load_recent_disputes( /// /// If one or more ancient sessions are pruned, all metadata on candidates within the ancient /// session will be deleted. -pub(crate) fn note_current_session( +pub(crate) fn note_earliest_session( overlay_db: &mut OverlayedBackend<'_, impl Backend>, - current_session: SessionIndex, + new_earliest_session: SessionIndex, ) -> SubsystemResult<()> { - let new_earliest = current_session.saturating_sub(DISPUTE_WINDOW.get()); match overlay_db.load_earliest_session()? { None => { // First launch - write new-earliest. - overlay_db.write_earliest_session(new_earliest); + overlay_db.write_earliest_session(new_earliest_session); }, - Some(prev_earliest) if new_earliest > prev_earliest => { + Some(prev_earliest) if new_earliest_session > prev_earliest => { // Prune all data in the outdated sessions. - overlay_db.write_earliest_session(new_earliest); + overlay_db.write_earliest_session(new_earliest_session); // Clear recent disputes metadata. { let mut recent_disputes = overlay_db.load_recent_disputes()?.unwrap_or_default(); - let lower_bound = (new_earliest, CandidateHash(Hash::repeat_byte(0x00))); + let lower_bound = (new_earliest_session, CandidateHash(Hash::repeat_byte(0x00))); let new_recent_disputes = recent_disputes.split_off(&lower_bound); // Any remanining disputes are considered ancient and must be pruned. @@ -373,6 +372,7 @@ mod tests { use super::*; use ::test_helpers::{dummy_candidate_receipt, dummy_hash}; + use polkadot_node_primitives::DISPUTE_WINDOW; use polkadot_primitives::v2::{Hash, Id as ParaId}; fn make_db() -> DbBackend { @@ -422,7 +422,7 @@ mod tests { let mut overlay_db = OverlayedBackend::new(&backend); gum::trace!(target: LOG_TARGET, ?current_session, "Noting current session"); - note_current_session(&mut overlay_db, current_session).unwrap(); + note_earliest_session(&mut overlay_db, earliest_session).unwrap(); let write_ops = overlay_db.into_write_ops(); backend.write(write_ops).unwrap(); @@ -442,7 +442,7 @@ mod tests { let current_session = current_session + 1; let earliest_session = earliest_session + 1; - note_current_session(&mut overlay_db, current_session).unwrap(); + note_earliest_session(&mut overlay_db, earliest_session).unwrap(); let write_ops = overlay_db.into_write_ops(); backend.write(write_ops).unwrap(); @@ -599,7 +599,7 @@ mod tests { } #[test] - fn note_current_session_prunes_old() { + fn note_earliest_session_prunes_old() { let mut backend = make_db(); let hash_a = CandidateHash(Hash::repeat_byte(0x0a)); @@ -648,7 +648,7 @@ mod tests { backend.write(write_ops).unwrap(); let mut overlay_db = OverlayedBackend::new(&backend); - note_current_session(&mut overlay_db, current_session).unwrap(); + note_earliest_session(&mut overlay_db, new_earliest_session).unwrap(); assert_eq!(overlay_db.load_earliest_session().unwrap(), Some(new_earliest_session)); diff --git a/node/core/dispute-coordinator/src/lib.rs b/node/core/dispute-coordinator/src/lib.rs index 09d6c621b999..e7ac66ce2ece 100644 --- a/node/core/dispute-coordinator/src/lib.rs +++ b/node/core/dispute-coordinator/src/lib.rs @@ -30,7 +30,7 @@ use futures::FutureExt; use sc_keystore::LocalKeystore; -use polkadot_node_primitives::{CandidateVotes, DISPUTE_WINDOW}; +use polkadot_node_primitives::CandidateVotes; use polkadot_node_subsystem::{ overseer, ActivatedLeaf, FromOrchestra, OverseerSignal, SpawnedSubsystem, SubsystemError, }; @@ -272,7 +272,7 @@ impl DisputeCoordinatorSubsystem { ChainScraper, )> { // Prune obsolete disputes: - db::v1::note_current_session(overlay_db, rolling_session_window.latest_session())?; + db::v1::note_earliest_session(overlay_db, rolling_session_window.earliest_session())?; let active_disputes = match overlay_db.load_recent_disputes() { Ok(Some(disputes)) => From 42ad824759f6b28b7dca9771565983cd6c2beae4 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Mon, 28 Nov 2022 15:30:48 +0000 Subject: [PATCH 4/4] update comment Signed-off-by: Andrei Sandu --- node/core/dispute-coordinator/src/initialized.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/core/dispute-coordinator/src/initialized.rs b/node/core/dispute-coordinator/src/initialized.rs index bf385b802e8c..4a2076a225be 100644 --- a/node/core/dispute-coordinator/src/initialized.rs +++ b/node/core/dispute-coordinator/src/initialized.rs @@ -709,7 +709,7 @@ impl Initialized { ) -> Result { gum::trace!(target: LOG_TARGET, ?statements, "In handle import statements"); if !self.rolling_session_window.contains(session) { - // It is not valid to participate in an ancient dispute (spam?). + // It is not valid to participate in an ancient dispute (spam?) or too new. return Ok(ImportStatementsResult::InvalidImport) }