Change handle_import_statements to FatalResult (#6820)

* Changing dispute db errors to fatal

* fmt

polkadot/node/core/dispute-coordinator/src/backend.rs

@@ -21,7 +21,6 @@
 //! [`Backend`], maintaining consistency between queries and temporary writes,
 //! before any commit to the underlying storage is made.
 
-use polkadot_node_subsystem::SubsystemResult;
 use polkadot_primitives::{CandidateHash, SessionIndex};
 
 use std::collections::HashMap;
@@ -40,17 +39,17 @@ pub enum BackendWriteOp {
 /// An abstraction over backend storage for the logic of this subsystem.
 pub trait Backend {
 	/// Load the earliest session, if any.
-	fn load_earliest_session(&self) -> SubsystemResult<Option<SessionIndex>>;
+	fn load_earliest_session(&self) -> FatalResult<Option<SessionIndex>>;
 
 	/// Load the recent disputes, if any.
-	fn load_recent_disputes(&self) -> SubsystemResult<Option<RecentDisputes>>;
+	fn load_recent_disputes(&self) -> FatalResult<Option<RecentDisputes>>;
 
 	/// Load the candidate votes for the specific session-candidate pair, if any.
 	fn load_candidate_votes(
 		&self,
 		session: SessionIndex,
 		candidate_hash: &CandidateHash,
-	) -> SubsystemResult<Option<CandidateVotes>>;
+	) -> FatalResult<Option<CandidateVotes>>;
 
 	/// Atomically writes the list of operations, with later operations taking precedence over
 	/// prior.
@@ -93,7 +92,7 @@ impl<'a, B: 'a + Backend> OverlayedBackend<'a, B> {
 	}
 
 	/// Load the earliest session, if any.
-	pub fn load_earliest_session(&self) -> SubsystemResult<Option<SessionIndex>> {
+	pub fn load_earliest_session(&self) -> FatalResult<Option<SessionIndex>> {
 		if let Some(val) = self.earliest_session {
 			return Ok(Some(val))
 		}
@@ -102,7 +101,7 @@ impl<'a, B: 'a + Backend> OverlayedBackend<'a, B> {
 	}
 
 	/// Load the recent disputes, if any.
-	pub fn load_recent_disputes(&self) -> SubsystemResult<Option<RecentDisputes>> {
+	pub fn load_recent_disputes(&self) -> FatalResult<Option<RecentDisputes>> {
 		if let Some(val) = &self.recent_disputes {
 			return Ok(Some(val.clone()))
 		}
@@ -115,7 +114,7 @@ impl<'a, B: 'a + Backend> OverlayedBackend<'a, B> {
 		&self,
 		session: SessionIndex,
 		candidate_hash: &CandidateHash,
-	) -> SubsystemResult<Option<CandidateVotes>> {
+	) -> FatalResult<Option<CandidateVotes>> {
 		if let Some(val) = self.candidate_votes.get(&(session, *candidate_hash)) {
 			return Ok(val.clone())
 		}

polkadot/node/core/dispute-coordinator/src/db/v1.rs

@@ -17,7 +17,6 @@
 //! `V1` database for the dispute coordinator.
 
 use polkadot_node_primitives::DisputeStatus;
-use polkadot_node_subsystem::{SubsystemError, SubsystemResult};
 use polkadot_node_subsystem_util::database::{DBTransaction, Database};
 use polkadot_primitives::{
 	CandidateHash, CandidateReceipt, Hash, InvalidDisputeStatementKind, SessionIndex,
@@ -109,12 +108,12 @@ impl DbBackend {
 
 impl Backend for DbBackend {
 	/// Load the earliest session, if any.
-	fn load_earliest_session(&self) -> SubsystemResult<Option<SessionIndex>> {
+	fn load_earliest_session(&self) -> FatalResult<Option<SessionIndex>> {
 		load_earliest_session(&*self.inner, &self.config)
 	}
 
 	/// Load the recent disputes, if any.
-	fn load_recent_disputes(&self) -> SubsystemResult<Option<RecentDisputes>> {
+	fn load_recent_disputes(&self) -> FatalResult<Option<RecentDisputes>> {
 		load_recent_disputes(&*self.inner, &self.config)
 	}
 
@@ -123,7 +122,7 @@ impl Backend for DbBackend {
 		&self,
 		session: SessionIndex,
 		candidate_hash: &CandidateHash,
-	) -> SubsystemResult<Option<CandidateVotes>> {
+	) -> FatalResult<Option<CandidateVotes>> {
 		load_candidate_votes(&*self.inner, &self.config, session, candidate_hash)
 	}
 
@@ -287,27 +286,27 @@ pub(crate) fn load_candidate_votes(
 	config: &ColumnConfiguration,
 	session: SessionIndex,
 	candidate_hash: &CandidateHash,
-) -> SubsystemResult<Option<CandidateVotes>> {
+) -> FatalResult<Option<CandidateVotes>> {
 	load_decode(db, config.col_dispute_data, &candidate_votes_key(session, candidate_hash))
-		.map_err(|e| SubsystemError::with_origin("dispute-coordinator", e))
+		.map_err(|e| FatalError::DbReadFailed(e))
 }
 
 /// Load the earliest session, if any.
 pub(crate) fn load_earliest_session(
 	db: &dyn Database,
 	config: &ColumnConfiguration,
-) -> SubsystemResult<Option<SessionIndex>> {
+) -> FatalResult<Option<SessionIndex>> {
 	load_decode(db, config.col_dispute_data, EARLIEST_SESSION_KEY)
-		.map_err(|e| SubsystemError::with_origin("dispute-coordinator", e))
+		.map_err(|e| FatalError::DbReadFailed(e))
 }
 
 /// Load the recent disputes, if any.
 pub(crate) fn load_recent_disputes(
 	db: &dyn Database,
 	config: &ColumnConfiguration,
-) -> SubsystemResult<Option<RecentDisputes>> {
+) -> FatalResult<Option<RecentDisputes>> {
 	load_decode(db, config.col_dispute_data, RECENT_DISPUTES_KEY)
-		.map_err(|e| SubsystemError::with_origin("dispute-coordinator", e))
+		.map_err(|e| FatalError::DbReadFailed(e))
 }
 
 /// Maybe prune data in the DB based on the provided session index.
@@ -321,7 +320,7 @@ pub(crate) fn load_recent_disputes(
 pub(crate) fn note_earliest_session(
 	overlay_db: &mut OverlayedBackend<'_, impl Backend>,
 	new_earliest_session: SessionIndex,
-) -> SubsystemResult<()> {
+) -> FatalResult<()> {
 	match overlay_db.load_earliest_session()? {
 		None => {
 			// First launch - write new-earliest.

polkadot/node/core/dispute-coordinator/src/initialized.rs

@@ -694,6 +694,10 @@ impl Initialized {
 		Ok(())
 	}
 
+	// We use fatal result rather than result here. Reason being, We for example increase
+	// spam slots in this function. If then the import fails for some non fatal and
+	// unrelated reason, we should likely actually decrement previously incremented spam
+	// slots again, for non fatal errors - which is cumbersome and actually not needed
 	async fn handle_import_statements<Context>(
 		&mut self,
 		ctx: &mut Context,
@@ -702,7 +706,7 @@ impl Initialized {
 		session: SessionIndex,
 		statements: Vec<(SignedDisputeStatement, ValidatorIndex)>,
 		now: Timestamp,
-	) -> Result<ImportStatementsResult> {
+	) -> FatalResult<ImportStatementsResult> {
 		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?) or too new.