dispute-coordinator: fix earliest session checks for pruning and impo…

…rt (#6358) * RollingSession: add fn contains Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * handle_import_statements fix ancient dispute check Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * work with earliest session instead of latest Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * update comment Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
paritytech · Nov 28, 2022 · 1b1bc62 · 1b1bc62
1 parent 7538b76
commit 1b1bc62
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 18 deletions.
diff --git 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/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,8 +708,8 @@ impl Initialized {
 		now: Timestamp,
 	) -> Result<ImportStatementsResult> {
 		gum::trace!(target: LOG_TARGET, ?statements, "In handle import statements");
-		if session + DISPUTE_WINDOW.get() < self.highest_session {
-			// It is not valid to participate in an ancient dispute (spam?).
+		if !self.rolling_session_window.contains(session) {
+			// It is not valid to participate in an ancient dispute (spam?) or too new.
 			return Ok(ImportStatementsResult::InvalidImport)
 		}
 

diff --git 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)) =>

diff --git 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>(
 		sender: &mut Sender,
 	) -> Result<u32, SessionsUnavailable>
@@ -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(