Change handle_import_statements to FatalResult

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())
  }

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.

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.