From e9ae1d6d7da9256f7e206cc28dd158ee2d936d7b Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Mon, 21 Jul 2025 12:23:42 +0530 Subject: [PATCH 1/3] fix(supervisor): error consistency --- crates/supervisor/core/src/error.rs | 4 +--- crates/supervisor/storage/src/error.rs | 14 +++-------- .../src/providers/derivation_provider.rs | 23 ++++++++----------- .../storage/src/providers/log_provider.rs | 10 ++++---- 4 files changed, 19 insertions(+), 32 deletions(-) diff --git a/crates/supervisor/core/src/error.rs b/crates/supervisor/core/src/error.rs index e43836f34f..1e5af4437a 100644 --- a/crates/supervisor/core/src/error.rs +++ b/crates/supervisor/core/src/error.rs @@ -123,9 +123,7 @@ impl From for SpecError { Self::from(SuperchainDAError::UninitializedChainDatabase) } StorageError::ConflictError => Self::from(SuperchainDAError::ConflictingData), - StorageError::BlockOutOfOrder | StorageError::DerivedBlockOutOfOrder => { - Self::from(SuperchainDAError::OutOfOrder) - } + StorageError::BlockOutOfOrder => Self::from(SuperchainDAError::OutOfOrder), _ => Self::ErrorNotInSpec, } } diff --git a/crates/supervisor/storage/src/error.rs b/crates/supervisor/storage/src/error.rs index f27683fec8..bea913b126 100644 --- a/crates/supervisor/storage/src/error.rs +++ b/crates/supervisor/storage/src/error.rs @@ -26,10 +26,6 @@ pub enum StorageError { #[error("data not yet available")] FutureData, - /// Represents an error that occurred while initializing the database with an anchor. - #[error("invalid anchor")] - InvalidAnchor, - /// Represents an error that occurred when database is not initialized. #[error("database not initialized")] DatabaseNotInitialised, @@ -41,10 +37,6 @@ pub enum StorageError { /// Represents an error that occurred while writing to log database. #[error("latest stored block is not parent of the incoming block")] BlockOutOfOrder, - - /// Represents an error that occurred while writing to derived block database. - #[error("latest stored derived block is not parent of the incoming derived block")] - DerivedBlockOutOfOrder, } impl PartialEq for StorageError { @@ -54,9 +46,9 @@ impl PartialEq for StorageError { (Database(a), Database(b)) => a == b, (DatabaseInit(a), DatabaseInit(b)) => format!("{}", a) == format!("{}", b), (EntryNotFound(a), EntryNotFound(b)) => a == b, - (InvalidAnchor, InvalidAnchor) | - (DatabaseNotInitialised, DatabaseNotInitialised) | - (ConflictError, ConflictError) => true, + (DatabaseNotInitialised, DatabaseNotInitialised) | (ConflictError, ConflictError) => { + true + } _ => false, } } diff --git a/crates/supervisor/storage/src/providers/derivation_provider.rs b/crates/supervisor/storage/src/providers/derivation_provider.rs index 22b4768314..8ce02b966c 100644 --- a/crates/supervisor/storage/src/providers/derivation_provider.rs +++ b/crates/supervisor/storage/src/providers/derivation_provider.rs @@ -239,20 +239,17 @@ impl DerivationProvider<'_, TX> where TX: DbTxMut + DbTx, { - /// initialises the database with a derived block pair anchor. - pub(crate) fn initialise(&self, anchor: DerivedRefPair) -> Result<(), StorageError> { + /// initialises the database with a derived activation block pair. + pub(crate) fn initialise(&self, activation_pair: DerivedRefPair) -> Result<(), StorageError> { match self.get_derived_block_pair_by_number(0) { - Ok(pair) - if pair.derived.hash == anchor.derived.hash && - pair.source.hash == anchor.source.hash => - { + Ok(pair) if activation_pair == pair.clone().into() => { // Anchor matches, nothing to do Ok(()) } - Ok(_) => Err(StorageError::InvalidAnchor), + Ok(_) => Err(StorageError::ConflictError), Err(StorageError::EntryNotFound(_)) => { - self.save_source_block_internal(anchor.source)?; - self.save_derived_block_internal(anchor)?; + self.save_source_block_internal(activation_pair.source)?; + self.save_derived_block_internal(activation_pair)?; Ok(()) } Err(err) => Err(err), @@ -319,7 +316,7 @@ where incoming_derived_block_pair = %incoming_pair, "Latest stored derived block is not parent of the incoming derived block" ); - return Err(StorageError::DerivedBlockOutOfOrder); + return Err(StorageError::BlockOutOfOrder); } self.save_derived_block_internal(incoming_pair) @@ -572,7 +569,7 @@ mod tests { let wrong_derived = block_info(0, B256::from([42u8; 32]), 200); let wrong_anchor = derived_pair(source, wrong_derived); - let result = insert_pair(&db, &wrong_anchor); + let result = initialize_db(&db, &wrong_anchor); assert!(matches!(result, Err(StorageError::ConflictError))); } @@ -609,7 +606,7 @@ mod tests { let derived2 = block_info(2, wrong_parent_hash, 300); let pair2 = derived_pair(source1, derived2); let result = insert_pair(&db, &pair2); - assert!(matches!(result, Err(StorageError::DerivedBlockOutOfOrder))); + assert!(matches!(result, Err(StorageError::BlockOutOfOrder))); } #[test] @@ -624,7 +621,7 @@ mod tests { let derived2 = block_info(4, derived1.hash, 400); // should be 2, not 4 let pair2 = derived_pair(source1, derived2); let result = insert_pair(&db, &pair2); - assert!(matches!(result, Err(StorageError::DerivedBlockOutOfOrder))); + assert!(matches!(result, Err(StorageError::BlockOutOfOrder))); } #[test] diff --git a/crates/supervisor/storage/src/providers/log_provider.rs b/crates/supervisor/storage/src/providers/log_provider.rs index 450fc00b90..add349b4a8 100644 --- a/crates/supervisor/storage/src/providers/log_provider.rs +++ b/crates/supervisor/storage/src/providers/log_provider.rs @@ -43,12 +43,12 @@ impl LogProvider<'_, TX> where TX: DbTxMut + DbTx, { - pub(crate) fn initialise(&self, anchor: BlockInfo) -> Result<(), StorageError> { + pub(crate) fn initialise(&self, activation_block: BlockInfo) -> Result<(), StorageError> { match self.get_block(0) { - Ok(block) if block.hash == anchor.hash => Ok(()), - Ok(_) => Err(StorageError::InvalidAnchor), + Ok(block) if block == activation_block => Ok(()), + Ok(_) => Err(StorageError::ConflictError), Err(StorageError::EntryNotFound(_)) => { - self.store_block_logs_internal(&anchor, Vec::new()) + self.store_block_logs_internal(&activation_block, Vec::new()) } Err(err) => Err(err), @@ -363,7 +363,7 @@ mod tests { wrong_genesis.hash = B256::from([42u8; 32]); let result = initialize_db(&db, &wrong_genesis); - assert!(matches!(result, Err(StorageError::InvalidAnchor))); + assert!(matches!(result, Err(StorageError::ConflictError))); } #[test] From 30df4e0d8f233fdf8ce255693774835e76a9b329 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Mon, 21 Jul 2025 12:47:20 +0530 Subject: [PATCH 2/3] bugfix --- crates/supervisor/core/src/error.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/supervisor/core/src/error.rs b/crates/supervisor/core/src/error.rs index 1e5af4437a..c461f42a14 100644 --- a/crates/supervisor/core/src/error.rs +++ b/crates/supervisor/core/src/error.rs @@ -149,7 +149,7 @@ mod test { assert_eq!(spec_err, expected_err.into()); - let spec_err = ErrorObjectOwned::from(SpecError::from(StorageError::InvalidAnchor)); + let spec_err = ErrorObjectOwned::from(SpecError::from(StorageError::LockPoisoned)); let expected_err = SpecError::ErrorNotInSpec; assert_eq!(spec_err, expected_err.into()); @@ -171,7 +171,7 @@ mod test { fn test_supervisor_error_conversion() { // This will happen implicitly in server rpc response calls. let supervisor_err = ErrorObjectOwned::from(SupervisorError::SpecError(SpecError::from( - StorageError::InvalidAnchor, + StorageError::LockPoisoned, ))); let expected_err = SpecError::ErrorNotInSpec; From b0577b1b9453917816ae846565141490a779bb6f Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Mon, 21 Jul 2025 17:08:01 +0530 Subject: [PATCH 3/3] return ConflictErr in case of hash mismatch --- .../storage/src/providers/derivation_provider.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/supervisor/storage/src/providers/derivation_provider.rs b/crates/supervisor/storage/src/providers/derivation_provider.rs index 8ce02b966c..ec8eaacb20 100644 --- a/crates/supervisor/storage/src/providers/derivation_provider.rs +++ b/crates/supervisor/storage/src/providers/derivation_provider.rs @@ -75,9 +75,7 @@ where actual_hash = %derived_block_pair.derived.hash, "Derived block hash mismatch" ); - return Err(StorageError::EntryNotFound( - "derived block hash does not match".to_string(), - )); + return Err(StorageError::ConflictError); } Ok(derived_block_pair) @@ -144,7 +142,7 @@ where source_block_hash = %source_block_id.hash, "Source block hash mismatch" ); - return Err(StorageError::EntryNotFound("source block hash mismatch".to_string())); + return Err(StorageError::ConflictError); } while block_traversal.derived_block_numbers.is_empty() { @@ -736,7 +734,7 @@ mod tests { let wrong_hash = B256::from([123u8; 32]); let source_id = BlockNumHash { number: source1.number, hash: wrong_hash }; let result = provider.latest_derived_block_at_source(source_id); - assert!(matches!(result, Err(StorageError::EntryNotFound(_)))); + assert!(matches!(result, Err(StorageError::ConflictError))); } #[test]