From 7e7f72ed74f73f4e70df65fa95d1a8c77aaf0d22 Mon Sep 17 00:00:00 2001 From: Bradley Olson <34992650+BradleyOlson64@users.noreply.github.com> Date: Sat, 4 Mar 2023 10:35:04 -0800 Subject: [PATCH] Change handle_import_statements to FatalResult (#6820) * Changing dispute db errors to fatal * fmt --- .../core/dispute-coordinator/src/backend.rs | 13 ++++++------ .../core/dispute-coordinator/src/db/v1.rs | 21 +++++++++---------- .../dispute-coordinator/src/initialized.rs | 6 +++++- 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/polkadot/node/core/dispute-coordinator/src/backend.rs b/polkadot/node/core/dispute-coordinator/src/backend.rs index 9275420d0a..d49ace4925 100644 --- a/polkadot/node/core/dispute-coordinator/src/backend.rs +++ b/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>; + fn load_earliest_session(&self) -> FatalResult>; /// Load the recent disputes, if any. - fn load_recent_disputes(&self) -> SubsystemResult>; + fn load_recent_disputes(&self) -> FatalResult>; /// Load the candidate votes for the specific session-candidate pair, if any. fn load_candidate_votes( &self, session: SessionIndex, candidate_hash: &CandidateHash, - ) -> SubsystemResult>; + ) -> FatalResult>; /// 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> { + pub fn load_earliest_session(&self) -> FatalResult> { 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> { + pub fn load_recent_disputes(&self) -> FatalResult> { 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> { + ) -> FatalResult> { if let Some(val) = self.candidate_votes.get(&(session, *candidate_hash)) { return Ok(val.clone()) } diff --git a/polkadot/node/core/dispute-coordinator/src/db/v1.rs b/polkadot/node/core/dispute-coordinator/src/db/v1.rs index 441ba4e095..c0f3c9925f 100644 --- a/polkadot/node/core/dispute-coordinator/src/db/v1.rs +++ b/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> { + fn load_earliest_session(&self) -> FatalResult> { load_earliest_session(&*self.inner, &self.config) } /// Load the recent disputes, if any. - fn load_recent_disputes(&self) -> SubsystemResult> { + fn load_recent_disputes(&self) -> FatalResult> { load_recent_disputes(&*self.inner, &self.config) } @@ -123,7 +122,7 @@ impl Backend for DbBackend { &self, session: SessionIndex, candidate_hash: &CandidateHash, - ) -> SubsystemResult> { + ) -> FatalResult> { 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> { +) -> FatalResult> { 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> { +) -> FatalResult> { 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> { +) -> FatalResult> { 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. diff --git a/polkadot/node/core/dispute-coordinator/src/initialized.rs b/polkadot/node/core/dispute-coordinator/src/initialized.rs index 8304fc55ed..2e06e60d07 100644 --- a/polkadot/node/core/dispute-coordinator/src/initialized.rs +++ b/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( &mut self, ctx: &mut Context, @@ -702,7 +706,7 @@ impl Initialized { session: SessionIndex, statements: Vec<(SignedDisputeStatement, ValidatorIndex)>, now: Timestamp, - ) -> Result { + ) -> FatalResult { 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.