From 0ea7a3793cba1227b5a6d95ae73a6bc0b071f056 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Fri, 4 Jul 2025 16:29:06 +0530 Subject: [PATCH 01/27] fix(supervisor/core): derivation update failure --- .../core/src/chain_processor/chain.rs | 14 ++- .../core/src/chain_processor/task.rs | 113 ++++++++++------- crates/supervisor/core/src/supervisor.rs | 2 +- crates/supervisor/core/src/syncnode/error.rs | 6 +- crates/supervisor/core/src/syncnode/mod.rs | 5 +- crates/supervisor/core/src/syncnode/node.rs | 17 ++- .../supervisor/core/src/syncnode/resetter.rs | 114 +++++++----------- crates/supervisor/core/src/syncnode/task.rs | 10 +- crates/supervisor/core/src/syncnode/traits.rs | 36 +++++- 9 files changed, 189 insertions(+), 128 deletions(-) diff --git a/crates/supervisor/core/src/chain_processor/chain.rs b/crates/supervisor/core/src/chain_processor/chain.rs index 153498d57f..03998c678d 100644 --- a/crates/supervisor/core/src/chain_processor/chain.rs +++ b/crates/supervisor/core/src/chain_processor/chain.rs @@ -117,7 +117,10 @@ mod tests { use super::*; use crate::{ event::ChainEvent, - syncnode::{ManagedNodeApiProvider, ManagedNodeError, NodeSubscriber, ReceiptProvider}, + syncnode::{ + ManagedNodeController, ManagedNodeDataProvider, ManagedNodeError, NodeSubscriber, + ReceiptProvider, + }, }; use alloy_primitives::B256; use alloy_rpc_types_eth::BlockNumHash; @@ -165,7 +168,7 @@ mod tests { } #[async_trait] - impl ManagedNodeApiProvider for MockNode { + impl ManagedNodeDataProvider for MockNode { async fn output_v0_at_timestamp( &self, _timestamp: u64, @@ -186,7 +189,10 @@ mod tests { ) -> Result { Ok(BlockInfo::default()) } + } + #[async_trait] + impl ManagedNodeController for MockNode { async fn update_finalized( &self, _finalized_block_id: BlockNumHash, @@ -208,6 +214,10 @@ mod tests { ) -> Result<(), ManagedNodeError> { Ok(()) } + + async fn reset(&self) -> Result<(), ManagedNodeError> { + Ok(()) + } } mock!( diff --git a/crates/supervisor/core/src/chain_processor/task.rs b/crates/supervisor/core/src/chain_processor/task.rs index 7972fb676a..c851f4f878 100644 --- a/crates/supervisor/core/src/chain_processor/task.rs +++ b/crates/supervisor/core/src/chain_processor/task.rs @@ -1,9 +1,10 @@ use super::Metrics; use crate::{ChainProcessorError, LogIndexer, event::ChainEvent, syncnode::ManagedNodeProvider}; use alloy_primitives::ChainId; +use jsonrpsee::core::error; use kona_interop::{BlockReplacement, DerivedRefPair}; use kona_protocol::BlockInfo; -use kona_supervisor_storage::{DerivationStorageWriter, HeadRefStorageWriter, LogStorageWriter}; +use kona_supervisor_storage::{DerivationStorageWriter, HeadRefStorageWriter, LogStorageWriter, StorageError}; use std::{fmt::Debug, sync::Arc}; use tokio::sync::mpsc; use tokio_util::sync::CancellationToken; @@ -295,8 +296,28 @@ where block_number = derived_ref_pair.derived.number, "Processing local safe derived block pair" ); - self.state_manager.save_derived_block_pair(derived_ref_pair)?; - Ok(derived_ref_pair.derived) + match self.state_manager.save_derived_block_pair(derived_ref_pair) { + Ok(_) => Ok(derived_ref_pair.derived), + Err(StorageError::BlockOutOfOrder) => { + error!( + target: "chain_processor", + chain_id = self.chain_id, + block_number = derived_ref_pair.derived.number, + "Block out of order detected, resetting managed node" + ); + + if let Err(err) = self.managed_node.reset().await { + error!( + target: "chain_processor", + chain_id = self.chain_id, + %err, + "Failed to reset managed node after block out of order" + ); + } + Err(StorageError::BlockOutOfOrder.into()) + } + Err(err) => Err(err.into()), + } } async fn handle_unsafe_event( @@ -352,7 +373,10 @@ mod tests { use super::*; use crate::{ event::ChainEvent, - syncnode::{ManagedNodeApiProvider, ManagedNodeError, NodeSubscriber, ReceiptProvider}, + syncnode::{ + ManagedNodeController, ManagedNodeDataProvider, ManagedNodeError, NodeSubscriber, + ReceiptProvider, + }, }; use alloy_primitives::B256; use alloy_rpc_types_eth::BlockNumHash; @@ -377,46 +401,51 @@ mod tests { &self, _event_tx: mpsc::Sender, ) -> Result<(), ManagedNodeError>; - } + } - #[async_trait] - impl ReceiptProvider for Node { - async fn fetch_receipts(&self, _block_hash: B256) -> Result; - } + #[async_trait] + impl ReceiptProvider for Node { + async fn fetch_receipts(&self, _block_hash: B256) -> Result; + } - #[async_trait] - impl ManagedNodeApiProvider for Node { - async fn output_v0_at_timestamp( - &self, - _timestamp: u64, - ) -> Result; - - async fn pending_output_v0_at_timestamp( - &self, - _timestamp: u64, - ) -> Result; - - async fn l2_block_ref_by_timestamp( - &self, - _timestamp: u64, - ) -> Result; - - async fn update_finalized( - &self, - _finalized_block_id: BlockNumHash, - ) -> Result<(), ManagedNodeError>; - - async fn update_cross_unsafe( - &self, - cross_unsafe_block_id: BlockNumHash, - ) -> Result<(), ManagedNodeError>; - - async fn update_cross_safe( - &self, - source_block_id: BlockNumHash, - derived_block_id: BlockNumHash, - ) -> Result<(), ManagedNodeError>; - } + #[async_trait] + impl ManagedNodeDataProvider for Node { + async fn output_v0_at_timestamp( + &self, + _timestamp: u64, + ) -> Result; + + async fn pending_output_v0_at_timestamp( + &self, + _timestamp: u64, + ) -> Result; + + async fn l2_block_ref_by_timestamp( + &self, + _timestamp: u64, + ) -> Result; + } + + #[async_trait] + impl ManagedNodeController for Node { + async fn update_finalized( + &self, + _finalized_block_id: BlockNumHash, + ) -> Result<(), ManagedNodeError>; + + async fn update_cross_unsafe( + &self, + cross_unsafe_block_id: BlockNumHash, + ) -> Result<(), ManagedNodeError>; + + async fn update_cross_safe( + &self, + source_block_id: BlockNumHash, + derived_block_id: BlockNumHash, + ) -> Result<(), ManagedNodeError>; + + async fn reset(&self) -> Result<(), ManagedNodeError>; + } ); mock!( diff --git a/crates/supervisor/core/src/supervisor.rs b/crates/supervisor/core/src/supervisor.rs index 5f2c2aa149..2be40314eb 100644 --- a/crates/supervisor/core/src/supervisor.rs +++ b/crates/supervisor/core/src/supervisor.rs @@ -28,7 +28,7 @@ use crate::{ event::ChainEvent, l1_watcher::L1Watcher, safety_checker::{CrossSafePromoter, CrossUnsafePromoter}, - syncnode::{Client, ManagedNode, ManagedNodeApiProvider, ManagedNodeClient}, + syncnode::{Client, ManagedNode, ManagedNodeClient, ManagedNodeDataProvider}, }; /// Defines the service for the Supervisor core logic. diff --git a/crates/supervisor/core/src/syncnode/error.rs b/crates/supervisor/core/src/syncnode/error.rs index 0020d61bf2..51fa921811 100644 --- a/crates/supervisor/core/src/syncnode/error.rs +++ b/crates/supervisor/core/src/syncnode/error.rs @@ -21,9 +21,9 @@ pub enum ManagedNodeError { #[error("failed to authenticate: {0}")] Authentication(#[from] AuthenticationError), - /// Database provider is not set for the managed node. - #[error("database not initialised for managed node")] - DatabaseNotInitialised, + /// Represents an error that occurred while fetching data from the storage. + #[error(transparent)] + StorageError(#[from] StorageError), } impl PartialEq for ManagedNodeError { diff --git a/crates/supervisor/core/src/syncnode/mod.rs b/crates/supervisor/core/src/syncnode/mod.rs index 2c01baa247..e73f319816 100644 --- a/crates/supervisor/core/src/syncnode/mod.rs +++ b/crates/supervisor/core/src/syncnode/mod.rs @@ -8,7 +8,10 @@ mod error; pub use error::{AuthenticationError, ManagedEventTaskError, ManagedNodeError, SubscriptionError}; mod traits; -pub use traits::{ManagedNodeApiProvider, ManagedNodeProvider, NodeSubscriber, ReceiptProvider}; +pub use traits::{ + ManagedNodeController, ManagedNodeDataProvider, ManagedNodeProvider, NodeSubscriber, + ReceiptProvider, +}; mod client; pub use client::{Client, ClientConfig, ManagedNodeClient}; diff --git a/crates/supervisor/core/src/syncnode/node.rs b/crates/supervisor/core/src/syncnode/node.rs index 3645d317dc..654b4e7aa2 100644 --- a/crates/supervisor/core/src/syncnode/node.rs +++ b/crates/supervisor/core/src/syncnode/node.rs @@ -17,8 +17,8 @@ use tokio_util::sync::CancellationToken; use tracing::{error, info, warn}; use super::{ - ManagedNodeApiProvider, ManagedNodeClient, ManagedNodeError, NodeSubscriber, ReceiptProvider, - SubscriptionError, resetter::Resetter, task::ManagedEventTask, + ManagedNodeClient, ManagedNodeController, ManagedNodeDataProvider, ManagedNodeError, + NodeSubscriber, ReceiptProvider, SubscriptionError, resetter::Resetter, task::ManagedEventTask, }; use crate::event::ChainEvent; @@ -178,7 +178,7 @@ where } #[async_trait] -impl ManagedNodeApiProvider for ManagedNode +impl ManagedNodeDataProvider for ManagedNode where DB: LogStorageReader + DerivationStorageReader + HeadRefStorageReader + Send + Sync + 'static, C: ManagedNodeClient + Send + Sync + 'static, @@ -200,7 +200,14 @@ where ) -> Result { self.client.l2_block_ref_by_timestamp(timestamp).await } +} +#[async_trait] +impl ManagedNodeController for ManagedNode +where + DB: LogStorageReader + DerivationStorageReader + HeadRefStorageReader + Send + Sync + 'static, + C: ManagedNodeClient + Send + Sync + 'static, +{ async fn update_finalized( &self, finalized_block_id: BlockNumHash, @@ -222,4 +229,8 @@ where ) -> Result<(), ManagedNodeError> { self.client.update_cross_safe(source_block_id, derived_block_id).await } + + async fn reset(&self) -> Result<(), ManagedNodeError> { + self.resetter.reset().await + } } diff --git a/crates/supervisor/core/src/syncnode/resetter.rs b/crates/supervisor/core/src/syncnode/resetter.rs index 0e037f59cc..9c530a425a 100644 --- a/crates/supervisor/core/src/syncnode/resetter.rs +++ b/crates/supervisor/core/src/syncnode/resetter.rs @@ -1,4 +1,4 @@ -use super::ManagedNodeClient; +use super::{ManagedNodeClient, ManagedNodeError}; use kona_interop::SafetyLevel; use kona_supervisor_storage::HeadRefStorageReader; use std::sync::Arc; @@ -23,67 +23,49 @@ where } /// Resets the node using the latest super head. - pub(crate) async fn reset(&self) { + pub(crate) async fn reset(&self) -> Result<(), ManagedNodeError> { let _guard = self.reset_guard.lock().await; info!(target: "resetter", "Resetting the node"); - let unsafe_ref = match self.db_provider.get_safety_head_ref(SafetyLevel::LocalUnsafe) { - Ok(val) => val, - Err(err) => { - error!(target: "resetter", %err, "Failed to get unsafe head ref"); - return; - } - }; - - let cross_unsafe_ref = match self.db_provider.get_safety_head_ref(SafetyLevel::CrossUnsafe) - { - Ok(val) => val, - Err(err) => { + let unsafe_ref = + self.db_provider.get_safety_head_ref(SafetyLevel::LocalUnsafe).inspect_err(|err| { + error!(target: "resetter", %err, "Failed to get local unsafe head ref"); + })?; + + let cross_unsafe_ref = + self.db_provider.get_safety_head_ref(SafetyLevel::CrossUnsafe).inspect_err(|err| { error!(target: "resetter", %err, "Failed to get cross unsafe head ref"); - return; - } - }; + })?; - let local_safe_ref = match self.db_provider.get_safety_head_ref(SafetyLevel::LocalSafe) { - Ok(val) => val, - Err(err) => { + let local_safe_ref = + self.db_provider.get_safety_head_ref(SafetyLevel::LocalSafe).inspect_err(|err| { error!(target: "resetter", %err, "Failed to get local safe head ref"); - return; - } - }; - - let safe_ref = match self.db_provider.get_safety_head_ref(SafetyLevel::CrossSafe) { - Ok(val) => val, - Err(err) => { - error!(target: "resetter", %err, "Failed to get safe head ref"); - return; - } - }; - - let finalised_ref = match self.db_provider.get_safety_head_ref(SafetyLevel::Finalized) { - Ok(val) => val, - Err(err) => { + })?; + + let safe_ref = + self.db_provider.get_safety_head_ref(SafetyLevel::CrossSafe).inspect_err(|err| { + error!(target: "resetter", %err, "Failed to get cross safe head ref"); + })?; + + let finalised_ref = + self.db_provider.get_safety_head_ref(SafetyLevel::Finalized).inspect_err(|err| { error!(target: "resetter", %err, "Failed to get finalised head ref"); - return; - } - }; + })?; - let node_safe_ref = match self.client.block_ref_by_number(local_safe_ref.number).await { - Ok(block) => block, - Err(err) => { + let node_safe_ref = + self.client.block_ref_by_number(local_safe_ref.number).await.inspect_err(|err| { // todo: it's possible that supervisor is ahead of the op-node // in this case we should handle the error gracefully error!(target: "resetter", %err, "Failed to get block by number"); - return; - } - }; + })?; // check with consistency with the op-node if node_safe_ref.hash != local_safe_ref.hash { // todo: handle this case error!(target: "resetter", "Local safe ref hash does not match node safe ref hash"); - return; + // returning ok here for now since this case should be handled + return Ok(()); } info!(target: "resetter", @@ -95,8 +77,7 @@ where "Resetting managed node with latest information", ); - if let Err(err) = self - .client + self.client .reset( unsafe_ref.id(), cross_unsafe_ref.id(), @@ -105,16 +86,18 @@ where finalised_ref.id(), ) .await - { - error!(target: "resetter", %err, "Failed to reset managed node"); - } + .inspect_err(|err| { + error!(target: "resetter", %err, "Failed to reset managed node"); + })?; + + Ok(()) } } #[cfg(test)] mod tests { use super::*; - use crate::syncnode::ManagedNodeError; + use crate::syncnode::{AuthenticationError, ManagedNodeError}; use alloy_eips::BlockNumHash; use alloy_primitives::{B256, ChainId}; use async_trait::async_trait; @@ -197,8 +180,7 @@ mod tests { let resetter = Resetter::new(Arc::new(client), Arc::new(db)); - resetter.reset().await; - // You can assert logs or side effects if needed + assert!(resetter.reset().await.is_ok()); } #[tokio::test] @@ -212,8 +194,7 @@ mod tests { let resetter = Resetter::new(Arc::new(client), Arc::new(db)); - resetter.reset().await; - // Should return early, no panic + assert!(resetter.reset().await.is_err()); } #[tokio::test] @@ -238,18 +219,17 @@ mod tests { .returning(move |_| Ok(super_head.finalized)); let mut client = MockClient::new(); - client - .expect_block_ref_by_number() - .returning(|_| Err(ManagedNodeError::DatabaseNotInitialised)); + client.expect_block_ref_by_number().returning(|_| { + Err(ManagedNodeError::Authentication(AuthenticationError::InvalidHeader)) + }); let resetter = Resetter::new(Arc::new(client), Arc::new(db)); - resetter.reset().await; - // Should return early, no panic + assert!(resetter.reset().await.is_err()); } #[tokio::test] - async fn test_reset_consistency_error() { + async fn test_reset_inconsistency() { let super_head = make_super_head(); let mut db = MockDb::new(); @@ -277,8 +257,7 @@ mod tests { let resetter = Resetter::new(Arc::new(client), Arc::new(db)); - resetter.reset().await; - // Should return early, no panic + assert!(resetter.reset().await.is_ok()); } #[tokio::test] @@ -304,13 +283,12 @@ mod tests { let mut client = MockClient::new(); client.expect_block_ref_by_number().returning(move |_| Ok(super_head.local_safe)); - client - .expect_reset() - .returning(|_, _, _, _, _| Err(ManagedNodeError::DatabaseNotInitialised)); + client.expect_reset().returning(|_, _, _, _, _| { + Err(ManagedNodeError::Authentication(AuthenticationError::InvalidJwt)) + }); let resetter = Resetter::new(Arc::new(client), Arc::new(db)); - resetter.reset().await; - // Should log error, no panic + assert!(resetter.reset().await.is_err()); } } diff --git a/crates/supervisor/core/src/syncnode/task.rs b/crates/supervisor/core/src/syncnode/task.rs index c8bb1dab40..1207171450 100644 --- a/crates/supervisor/core/src/syncnode/task.rs +++ b/crates/supervisor/core/src/syncnode/task.rs @@ -53,7 +53,9 @@ where // Process each field of the event if it's present if let Some(reset_id) = &event.reset { info!(target: "managed_event_task", %reset_id, "Reset event received"); - self.resetter.reset().await; + if let Err(err) = self.resetter.reset().await { + error!(target: "managed_event_task", %err, "Failed to reset node"); + } } if let Some(unsafe_block) = &event.unsafe_block { @@ -175,8 +177,10 @@ where } if !self.is_node_consistent(derived_ref_pair).await? { - self.resetter.reset().await; - return Ok(()); + if let Err(err) = self.resetter.reset().await { + error!(target: "managed_event_task", %err, "Failed to reset node after inconsistency check"); + } + return Ok(()) } let block_info = BlockInfo { diff --git a/crates/supervisor/core/src/syncnode/traits.rs b/crates/supervisor/core/src/syncnode/traits.rs index 1e415c3fac..35465a43ea 100644 --- a/crates/supervisor/core/src/syncnode/traits.rs +++ b/crates/supervisor/core/src/syncnode/traits.rs @@ -46,10 +46,10 @@ pub trait ReceiptProvider: Send + Sync + Debug { async fn fetch_receipts(&self, block_hash: B256) -> Result; } -/// [`ManagedNodeApiProvider`] abstracts the managed node APIs that supervisor uses to fetch info -/// from the managed node. +/// [`ManagedNodeDataProvider`] abstracts the managed node data APIs that supervisor uses to fetch +/// info from the managed node. #[async_trait] -pub trait ManagedNodeApiProvider: Send + Sync + Debug { +pub trait ManagedNodeDataProvider: Send + Sync + Debug { /// Fetch the output v0 at a given timestamp. /// /// # Arguments @@ -84,7 +84,12 @@ pub trait ManagedNodeApiProvider: Send + Sync + Debug { &self, timestamp: u64, ) -> Result; +} +/// [`ManagedNodeController`] abstracts the managed node control APIs that supervisor uses to +/// control the managed node state. +#[async_trait] +pub trait ManagedNodeController: Send + Sync + Debug { /// Update the finalized block head using the given [`BlockNumHash`]. /// /// # Arguments @@ -125,6 +130,15 @@ pub trait ManagedNodeApiProvider: Send + Sync + Debug { source_block_id: BlockNumHash, derived_block_id: BlockNumHash, ) -> Result<(), ManagedNodeError>; + + /// Reset the managed node based on the supervisor's state. + /// This is typically used to reset the node's state + /// when the supervisor detects a misalignment + /// + /// # Returns + /// * `Ok(())` on success + /// * `Err(ManagedNodeError)` if the reset fails + async fn reset(&self) -> Result<(), ManagedNodeError>; } /// Composite trait for any node that provides: @@ -136,12 +150,24 @@ pub trait ManagedNodeApiProvider: Send + Sync + Debug { /// within the supervisor context. #[async_trait] pub trait ManagedNodeProvider: - NodeSubscriber + ReceiptProvider + ManagedNodeApiProvider + Send + Sync + Debug + NodeSubscriber + + ReceiptProvider + + ManagedNodeDataProvider + + ManagedNodeController + + Send + + Sync + + Debug { } #[async_trait] impl ManagedNodeProvider for T where - T: NodeSubscriber + ReceiptProvider + ManagedNodeApiProvider + Send + Sync + Debug + T: NodeSubscriber + + ReceiptProvider + + ManagedNodeDataProvider + + ManagedNodeController + + Send + + Sync + + Debug { } From dac9db9b3c8b3fca0c8d2b1c8d51e55a2e81c6d4 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Mon, 7 Jul 2025 13:49:01 +0530 Subject: [PATCH 02/27] derivation storage idempotancy --- .../core/src/chain_processor/task.rs | 50 +++++++++++--- crates/supervisor/core/src/syncnode/node.rs | 16 +---- crates/supervisor/core/src/syncnode/task.rs | 58 +++------------- .../src/providers/derivation_provider.rs | 67 +++++++++++++++++++ 4 files changed, 121 insertions(+), 70 deletions(-) diff --git a/crates/supervisor/core/src/chain_processor/task.rs b/crates/supervisor/core/src/chain_processor/task.rs index 34323bfe0d..a626a68e8a 100644 --- a/crates/supervisor/core/src/chain_processor/task.rs +++ b/crates/supervisor/core/src/chain_processor/task.rs @@ -1,10 +1,11 @@ use super::Metrics; use crate::{ChainProcessorError, LogIndexer, event::ChainEvent, syncnode::ManagedNodeProvider}; use alloy_primitives::ChainId; -use jsonrpsee::core::error; use kona_interop::{BlockReplacement, DerivedRefPair}; use kona_protocol::BlockInfo; -use kona_supervisor_storage::{DerivationStorageWriter, HeadRefStorageWriter, LogStorageWriter, StorageError}; +use kona_supervisor_storage::{ + DerivationStorageWriter, HeadRefStorageWriter, LogStorageWriter, StorageError, +}; use std::{fmt::Debug, sync::Arc}; use tokio::sync::mpsc; use tokio_util::sync::CancellationToken; @@ -176,7 +177,7 @@ where .await; } ChainEvent::DerivationOriginUpdate { origin } => { - let _ = self.handle_derivation_origin_update(origin).inspect_err(|err| { + let _ = self.handle_derivation_origin_update(origin).await.inspect_err(|err| { error!( target: "chain_processor", chain_id = self.chain_id, @@ -271,7 +272,7 @@ where Ok(finalized_derived_block) } - fn handle_derivation_origin_update( + async fn handle_derivation_origin_update( &self, origin: BlockInfo, ) -> Result<(), ChainProcessorError> { @@ -281,9 +282,31 @@ where block_number = origin.number, "Processing derivation origin update" ); - self.state_manager.update_current_l1(origin)?; - self.state_manager.save_source_block(origin)?; - Ok(()) + match self.state_manager.save_source_block(origin) { + Ok(_) => { + self.state_manager.update_current_l1(origin)?; + Ok(()) + } + Err(StorageError::BlockOutOfOrder) => { + error!( + target: "chain_processor", + chain_id = self.chain_id, + block_number = origin.number, + "Source block out of order detected, resetting managed node" + ); + + if let Err(err) = self.managed_node.reset().await { + error!( + target: "chain_processor", + chain_id = self.chain_id, + %err, + "Failed to reset managed node after block out of order" + ); + } + Err(StorageError::BlockOutOfOrder.into()) + } + Err(err) => Err(err.into()), + } } async fn handle_safe_event( @@ -305,7 +328,7 @@ where block_number = derived_ref_pair.derived.number, "Block out of order detected, resetting managed node" ); - + if let Err(err) = self.managed_node.reset().await { error!( target: "chain_processor", @@ -316,7 +339,16 @@ where } Err(StorageError::BlockOutOfOrder.into()) } - Err(err) => Err(err.into()), + Err(err) => { + error!( + target: "chain_processor", + chain_id = self.chain_id, + block_number = derived_ref_pair.derived.number, + %err, + "Failed to save derived block pair" + ); + Err(err.into()) + } } } diff --git a/crates/supervisor/core/src/syncnode/node.rs b/crates/supervisor/core/src/syncnode/node.rs index 654b4e7aa2..1a8ac0b54a 100644 --- a/crates/supervisor/core/src/syncnode/node.rs +++ b/crates/supervisor/core/src/syncnode/node.rs @@ -29,8 +29,6 @@ use crate::event::ChainEvent; pub struct ManagedNode { /// The attached web socket client client: Arc, - /// The database provider for fetching information - db_provider: Arc, /// Shared L1 provider for fetching receipts l1_provider: RootProvider, /// Resetter for handling node resets @@ -53,16 +51,9 @@ where cancel_token: CancellationToken, l1_provider: RootProvider, ) -> Self { - let resetter = Arc::new(Resetter::new(client.clone(), db_provider.clone())); - - Self { - client, - db_provider, - resetter, - cancel_token, - task_handle: Mutex::new(None), - l1_provider, - } + let resetter = Arc::new(Resetter::new(client.clone(), db_provider)); + + Self { client, resetter, cancel_token, task_handle: Mutex::new(None), l1_provider } } /// Returns the [`ChainId`] of the [`ManagedNode`]. @@ -106,7 +97,6 @@ where let task = ManagedEventTask::new( self.client.clone(), self.l1_provider.clone(), - self.db_provider.clone(), self.resetter.clone(), event_tx, ); diff --git a/crates/supervisor/core/src/syncnode/task.rs b/crates/supervisor/core/src/syncnode/task.rs index 960fad6d3b..dd5bd52760 100644 --- a/crates/supervisor/core/src/syncnode/task.rs +++ b/crates/supervisor/core/src/syncnode/task.rs @@ -17,8 +17,6 @@ pub(super) struct ManagedEventTask { client: Arc, /// The URL of the L1 RPC endpoint to use for fetching L1 data l1_provider: RootProvider, - /// The database provider for fetching information - db_provider: Arc, /// The resetter for handling node resets resetter: Arc>, /// The channel to send the events to which require further processing e.g. db updates @@ -34,11 +32,10 @@ where pub(super) const fn new( client: Arc, l1_provider: RootProvider, - db_provider: Arc, resetter: Arc>, event_tx: mpsc::Sender, ) -> Self { - Self { client, l1_provider, db_provider, resetter, event_tx } + Self { client, l1_provider, resetter, event_tx } } /// Processes a managed event received from the subscription. @@ -70,10 +67,8 @@ where } if let Some(derived_ref_pair) = &event.derivation_update { - info!(target: "managed_event_task", %derived_ref_pair, "Derivation update received"); - if event.derivation_origin_update.is_none() { - info!(target: "managed_event_task", "Derivation update received without origin update"); + info!(target: "managed_event_task", %event, "Derivation update received without origin update"); if let Err(err) = self .event_tx @@ -173,13 +168,6 @@ where })? } - if !self.is_node_consistent(derived_ref_pair).await? { - if let Err(err) = self.resetter.reset().await { - error!(target: "managed_event_task", %err, "Failed to reset node after inconsistency check"); - } - return Ok(()) - } - let block_info = BlockInfo { hash: block.header.hash, number: block.header.number, @@ -195,32 +183,6 @@ where info!(target: "managed_event_task", "Sent next L1 block to managed node using provide_l1"); Ok(()) } - - async fn is_node_consistent( - &self, - derived_ref_pair: &DerivedRefPair, - ) -> Result { - let derived_block = derived_ref_pair.derived; - let derivation_state = self.db_provider.latest_derivation_state() - .inspect_err(|err| error!(target: "managed_event_task", %err, "Failed to get latest derived block pair"))?; - - if derivation_state.derived.number < derived_block.number { - // this could happen since the events are being processed in async - // this case should be handled at the processing stage - return Ok(true); - } - - if derivation_state.derived != derived_block { - error!( - target: "managed_event_task", - incoming_pair = %derived_ref_pair, - %derivation_state, - "Node is inconsistent with the supervisor state" - ); - return Ok(false); - } - Ok(true) - } } #[cfg(test)] @@ -313,8 +275,8 @@ mod tests { let asserter = Asserter::new(); let transport = MockTransport::new(asserter.clone()); let provider = RootProvider::::new(RpcClient::new(transport, false)); - let resetter = Arc::new(Resetter::new(client.clone(), db.clone())); - let task = ManagedEventTask::new(client, provider, db, resetter, tx); + let resetter = Arc::new(Resetter::new(client.clone(), db)); + let task = ManagedEventTask::new(client, provider, resetter, tx); task.handle_managed_event(Some(managed_event)).await; @@ -363,8 +325,8 @@ mod tests { let asserter = Asserter::new(); let transport = MockTransport::new(asserter.clone()); let provider = RootProvider::::new(RpcClient::new(transport, false)); - let resetter = Arc::new(Resetter::new(client.clone(), db.clone())); - let task = ManagedEventTask::new(client, provider, db, resetter, tx); + let resetter = Arc::new(Resetter::new(client.clone(), db)); + let task = ManagedEventTask::new(client, provider, resetter, tx); task.handle_managed_event(Some(managed_event)).await; @@ -410,8 +372,8 @@ mod tests { let asserter = Asserter::new(); let transport = MockTransport::new(asserter.clone()); let provider = RootProvider::::new(RpcClient::new(transport, false)); - let resetter = Arc::new(Resetter::new(client.clone(), db.clone())); - let task = ManagedEventTask::new(client, provider, db, resetter, tx); + let resetter = Arc::new(Resetter::new(client.clone(), db)); + let task = ManagedEventTask::new(client, provider, resetter, tx); task.handle_managed_event(Some(managed_event)).await; @@ -493,8 +455,8 @@ mod tests { let asserter = Asserter::new(); let transport = MockTransport::new(asserter.clone()); let provider = RootProvider::::new(RpcClient::new(transport, false)); - let resetter = Arc::new(Resetter::new(client.clone(), db.clone())); - let task = ManagedEventTask::new(client, provider, db, resetter, tx); + let resetter = Arc::new(Resetter::new(client.clone(), db)); + let task = ManagedEventTask::new(client, provider, resetter, tx); // push the value that we expect on next call asserter.push(MockResponse::Success(serde_json::from_str(next_block).unwrap())); diff --git a/crates/supervisor/storage/src/providers/derivation_provider.rs b/crates/supervisor/storage/src/providers/derivation_provider.rs index 10e4fdad45..5b69718cea 100644 --- a/crates/supervisor/storage/src/providers/derivation_provider.rs +++ b/crates/supervisor/storage/src/providers/derivation_provider.rs @@ -215,6 +215,12 @@ where Ok(block_traversal) } + /// Gets the source block for the given source block number. + fn get_source_block(&self, source_block_number: u64) -> Result { + let block_traversal = self.get_block_traversal(source_block_number)?; + Ok(block_traversal.source.into()) + } + /// Gets the latest source block, even if it has no derived blocks. pub(crate) fn latest_source_block(&self) -> Result { let block = self.latest_source_block_traversal().inspect_err(|err| { @@ -267,6 +273,37 @@ where Err(e) => return Err(e), }; + // If the incoming derived block is not newer than the latest stored derived block, + // we do not save it, check if it is consistent with the saved state. + // If it is not consistent, we return an error. + if latest_derivation_state.derived.number >= incoming_pair.derived.number { + let stored_pair = self + .get_derived_block_pair_by_number(incoming_pair.derived.number) + .inspect_err(|err| { + error!( + target: "supervisor_storage", + incoming_derived_block_pair = %incoming_pair, + %err, + "Failed to get derived block pair" + ); + })?; + + if incoming_pair == stored_pair.into() { + return Ok(()); + } else { + error!( + target: "supervisor_storage", + latest_derived_block_pair = %latest_derivation_state, + incoming_derived_block_pair = %incoming_pair, + "Incoming derived block is not consistent with the latest stored derived block" + ); + return Err(StorageError::ConflictError( + "incoming derived block is not consistent with the stored derived block" + .to_string(), + )); + } + } + // Latest source block must be same as the incoming source block if latest_derivation_state.source != incoming_pair.source { warn!( @@ -365,6 +402,36 @@ where return Ok(()); } + // If the incoming source block is not newer than the latest source block, + // we do not save it, check if it is consistent with the saved state. + // If it is not consistent, we return an error. + if latest_source_block.number > incoming_source.number { + let source_block = + self.get_source_block(incoming_source.number).inspect_err(|err| { + error!( + target: "supervisor_storage", + incoming_source = %incoming_source, + %err, + "Failed to get source block" + ); + })?; + + if source_block == incoming_source { + return Ok(()); + } else { + error!( + target: "supervisor_storage", + latest_source_block = %latest_source_block, + incoming_source = %incoming_source, + "Incoming source block is not consistent with the latest source block" + ); + return Err(StorageError::ConflictError( + "incoming source block is not consistent with the stored source block" + .to_string(), + )); + } + } + if !latest_source_block.is_parent_of(&incoming_source) { error!( target: "supervisor_storage", From 34339a2429ec417cd3367bef2e451e9bf2614085 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Mon, 7 Jul 2025 13:49:15 +0530 Subject: [PATCH 03/27] docs updated --- crates/supervisor/storage/src/traits.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/crates/supervisor/storage/src/traits.rs b/crates/supervisor/storage/src/traits.rs index fa713e7cb2..4bff3d090d 100644 --- a/crates/supervisor/storage/src/traits.rs +++ b/crates/supervisor/storage/src/traits.rs @@ -61,7 +61,14 @@ pub trait DerivationStorageReader: Debug { /// Implementations are expected to provide persistent and thread-safe access to block data. pub trait DerivationStorageWriter: Debug { /// Saves a [`DerivedRefPair`] to the storage. - /// This method is append only and does not overwrite existing pairs. + /// + /// This method is **append-only**: it does not overwrite existing pairs. + /// - If a pair with the same block number already exists and is identical to the incoming pair, + /// the request is silently ignored (idempotent). + /// - If a pair with the same block number exists but differs from the incoming pair, + /// an error is returned to indicate a data inconsistency. + /// - If the pair is new and consistent, it is appended to the storage. + /// /// Ensures that the latest stored pair is the parent of the incoming pair before saving. /// /// # Arguments @@ -72,8 +79,17 @@ pub trait DerivationStorageWriter: Debug { /// * `Err(StorageError)` if there is an issue saving the pair. fn save_derived_block(&self, incoming_pair: DerivedRefPair) -> Result<(), StorageError>; - /// Saves the latest incoming source block to the storage. + /// Saves the latest incoming source [`BlockInfo`] to the storage. + /// + /// This method is **append-only**: it does not overwrite existing source blocks. + /// - If a source block with the same number already exists and is identical to the incoming block, + /// the request is silently ignored (idempotent). + /// - If a source block with the same number exists but differs from the incoming block, + /// an error is returned to indicate a data inconsistency. + /// - If the block is new and consistent, it is appended to the storage. /// + /// Ensures that the latest stored source block is the parent of the incoming block before saving. + /// /// # Arguments /// * `source` - The source block to save. /// From 457b19df330a4f864226b52251e1b1bdb2efece2 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Mon, 7 Jul 2025 14:21:08 +0530 Subject: [PATCH 04/27] lint and test fix --- .../src/providers/derivation_provider.rs | 73 ++++++++++++++++--- crates/supervisor/storage/src/traits.rs | 17 +++-- 2 files changed, 72 insertions(+), 18 deletions(-) diff --git a/crates/supervisor/storage/src/providers/derivation_provider.rs b/crates/supervisor/storage/src/providers/derivation_provider.rs index 5b69718cea..01c32aa32a 100644 --- a/crates/supervisor/storage/src/providers/derivation_provider.rs +++ b/crates/supervisor/storage/src/providers/derivation_provider.rs @@ -634,7 +634,7 @@ mod tests { } #[test] - fn duplicate_derived_block_number_should_fail() { + fn duplicate_derived_block_number_should_pass() { let db = setup_db(); let source1 = block_info(100, B256::from([100u8; 32]), 200); @@ -644,7 +644,25 @@ mod tests { // Try to insert the same derived block again let result = insert_pair(&db, &pair1); - assert!(matches!(result, Err(StorageError::DerivedBlockOutOfOrder))); + assert!(result.is_ok(), "Should allow inserting the same derived block again"); + } + + #[test] + fn save_old_block_should_pass() { + let db = setup_db(); + + let source1 = block_info(100, B256::from([100u8; 32]), 200); + let derived1 = block_info(1, genesis_block().hash, 200); + let pair1 = derived_pair(source1, derived1); + assert!(initialize_db(&db, &pair1).is_ok()); + + let derived2 = block_info(2, derived1.hash, 300); + let pair2 = derived_pair(source1, derived2); + assert!(insert_pair(&db, &pair2).is_ok()); + + // Try to insert a block with a lower number than the latest + let result = insert_pair(&db, &pair1); + assert!(result.is_ok(), "Should allow inserting an old derived block"); } #[test] @@ -664,7 +682,7 @@ mod tests { let derived_non_monotonic = block_info(1, derived2.hash, 400); let pair_non_monotonic = derived_pair(source1, derived_non_monotonic); let result = insert_pair(&db, &pair_non_monotonic); - assert!(matches!(result, Err(StorageError::DerivedBlockOutOfOrder))); + assert!(matches!(result, Err(StorageError::ConflictError(_)))); } #[test] @@ -842,20 +860,55 @@ mod tests { } #[test] - fn save_source_block_lower_number_should_fail() { + fn save_source_invalid_parent_should_fail() { let db = setup_db(); - let derived0 = block_info(10, B256::from([10u8; 32]), 200); - let pair1 = derived_pair(genesis_block(), derived0); + let source0 = block_info(10, B256::from([10u8; 32]), 200); + let derived0 = genesis_block(); + let pair1 = derived_pair(source0, derived0); assert!(initialize_db(&db, &pair1).is_ok()); - let source1 = block_info(1, genesis_block().hash, 400); + let source1 = block_info(11, B256::from([1u8; 32]), 200); + let result = insert_source_block(&db, &source1); + assert!( + matches!(result, Err(StorageError::BlockOutOfOrder)), + "Should fail with BlockOutOfOrder error" + ); + } + + #[test] + fn save_source_block_lower_number_should_pass() { + let db = setup_db(); + + let source0 = block_info(10, B256::from([10u8; 32]), 200); + let derived0 = genesis_block(); + let pair1 = derived_pair(source0, derived0); + assert!(initialize_db(&db, &pair1).is_ok()); + + let source1 = block_info(11, source0.hash, 400); assert!(insert_source_block(&db, &source1).is_ok()); - let source2 = block_info(0, source1.hash, 400); // Try to save a block with a lower number - let result = insert_source_block(&db, &source2); - assert!(matches!(result, Err(StorageError::BlockOutOfOrder))); + let result = insert_source_block(&db, &source0); + assert!(result.is_ok(), "Should allow saving a old source block"); + } + + #[test] + fn save_inconsistent_source_block_lower_number_should_fail() { + let db = setup_db(); + + let source0 = block_info(10, B256::from([10u8; 32]), 200); + let derived0 = genesis_block(); + let pair1 = derived_pair(source0, derived0); + assert!(initialize_db(&db, &pair1).is_ok()); + + let source1 = block_info(11, source0.hash, 400); + assert!(insert_source_block(&db, &source1).is_ok()); + + let old_source = block_info(source0.number, B256::from([1u8; 32]), 400); + // Try to save a block with a lower number + let result = insert_source_block(&db, &old_source); + assert!(matches!(result, Err(StorageError::ConflictError(_)))); } #[test] diff --git a/crates/supervisor/storage/src/traits.rs b/crates/supervisor/storage/src/traits.rs index 4bff3d090d..3e4376a7eb 100644 --- a/crates/supervisor/storage/src/traits.rs +++ b/crates/supervisor/storage/src/traits.rs @@ -65,8 +65,8 @@ pub trait DerivationStorageWriter: Debug { /// This method is **append-only**: it does not overwrite existing pairs. /// - If a pair with the same block number already exists and is identical to the incoming pair, /// the request is silently ignored (idempotent). - /// - If a pair with the same block number exists but differs from the incoming pair, - /// an error is returned to indicate a data inconsistency. + /// - If a pair with the same block number exists but differs from the incoming pair, an error + /// is returned to indicate a data inconsistency. /// - If the pair is new and consistent, it is appended to the storage. /// /// Ensures that the latest stored pair is the parent of the incoming pair before saving. @@ -82,14 +82,15 @@ pub trait DerivationStorageWriter: Debug { /// Saves the latest incoming source [`BlockInfo`] to the storage. /// /// This method is **append-only**: it does not overwrite existing source blocks. - /// - If a source block with the same number already exists and is identical to the incoming block, - /// the request is silently ignored (idempotent). - /// - If a source block with the same number exists but differs from the incoming block, - /// an error is returned to indicate a data inconsistency. + /// - If a source block with the same number already exists and is identical to the incoming + /// block, the request is silently ignored (idempotent). + /// - If a source block with the same number exists but differs from the incoming block, an + /// error is returned to indicate a data inconsistency. /// - If the block is new and consistent, it is appended to the storage. /// - /// Ensures that the latest stored source block is the parent of the incoming block before saving. - /// + /// Ensures that the latest stored source block is the parent of the incoming block before + /// saving. + /// /// # Arguments /// * `source` - The source block to save. /// From 1df0198144c7b811e9daa1f2635e7e7c279c2567 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Mon, 7 Jul 2025 16:11:23 +0530 Subject: [PATCH 05/27] chore(supervisor): remove l1 cache --- .../core/src/chain_processor/chain.rs | 5 - .../core/src/chain_processor/task.rs | 14 +- .../supervisor/core/src/syncnode/resetter.rs | 123 ++++-------------- crates/supervisor/core/src/syncnode/task.rs | 1 - crates/supervisor/storage/src/chaindb.rs | 81 +----------- crates/supervisor/storage/src/traits.rs | 19 +-- 6 files changed, 28 insertions(+), 215 deletions(-) diff --git a/crates/supervisor/core/src/chain_processor/chain.rs b/crates/supervisor/core/src/chain_processor/chain.rs index e3fa5e7464..ed2f763977 100644 --- a/crates/supervisor/core/src/chain_processor/chain.rs +++ b/crates/supervisor/core/src/chain_processor/chain.rs @@ -245,11 +245,6 @@ mod tests { } impl HeadRefStorageWriter for Db { - fn update_current_l1( - &self, - block_info: BlockInfo, - ) -> Result<(), StorageError>; - fn update_finalized_using_source( &self, block_info: BlockInfo, diff --git a/crates/supervisor/core/src/chain_processor/task.rs b/crates/supervisor/core/src/chain_processor/task.rs index a626a68e8a..79715efde7 100644 --- a/crates/supervisor/core/src/chain_processor/task.rs +++ b/crates/supervisor/core/src/chain_processor/task.rs @@ -283,10 +283,7 @@ where "Processing derivation origin update" ); match self.state_manager.save_source_block(origin) { - Ok(_) => { - self.state_manager.update_current_l1(origin)?; - Ok(()) - } + Ok(_) => Ok(()), Err(StorageError::BlockOutOfOrder) => { error!( target: "chain_processor", @@ -505,11 +502,6 @@ mod tests { } impl HeadRefStorageWriter for Db { - fn update_current_l1( - &self, - block_info: BlockInfo, - ) -> Result<(), StorageError>; - fn update_finalized_using_source( &self, block_info: BlockInfo, @@ -620,10 +612,6 @@ mod tests { assert_eq!(block_info, origin_clone); Ok(()) }); - mockdb.expect_update_current_l1().returning(move |block_info: BlockInfo| { - assert_eq!(block_info, origin_clone); - Ok(()) - }); let writer = Arc::new(mockdb); let managed_node = Arc::new(mocknode); diff --git a/crates/supervisor/core/src/syncnode/resetter.rs b/crates/supervisor/core/src/syncnode/resetter.rs index 9c530a425a..fd59b32240 100644 --- a/crates/supervisor/core/src/syncnode/resetter.rs +++ b/crates/supervisor/core/src/syncnode/resetter.rs @@ -1,6 +1,6 @@ use super::{ManagedNodeClient, ManagedNodeError}; -use kona_interop::SafetyLevel; use kona_supervisor_storage::HeadRefStorageReader; +use kona_supervisor_types::SuperHead; use std::sync::Arc; use tokio::sync::Mutex; use tracing::{error, info}; @@ -28,40 +28,22 @@ where info!(target: "resetter", "Resetting the node"); - let unsafe_ref = - self.db_provider.get_safety_head_ref(SafetyLevel::LocalUnsafe).inspect_err(|err| { - error!(target: "resetter", %err, "Failed to get local unsafe head ref"); - })?; - - let cross_unsafe_ref = - self.db_provider.get_safety_head_ref(SafetyLevel::CrossUnsafe).inspect_err(|err| { - error!(target: "resetter", %err, "Failed to get cross unsafe head ref"); - })?; - - let local_safe_ref = - self.db_provider.get_safety_head_ref(SafetyLevel::LocalSafe).inspect_err(|err| { - error!(target: "resetter", %err, "Failed to get local safe head ref"); - })?; - - let safe_ref = - self.db_provider.get_safety_head_ref(SafetyLevel::CrossSafe).inspect_err(|err| { - error!(target: "resetter", %err, "Failed to get cross safe head ref"); - })?; + let super_head = self.db_provider.get_super_head().inspect_err(|err| { + error!(target: "resetter", %err, "Failed to get super head"); + })?; - let finalised_ref = - self.db_provider.get_safety_head_ref(SafetyLevel::Finalized).inspect_err(|err| { - error!(target: "resetter", %err, "Failed to get finalised head ref"); - })?; + let SuperHead { local_unsafe, cross_unsafe, local_safe, cross_safe, finalized, .. } = + super_head; let node_safe_ref = - self.client.block_ref_by_number(local_safe_ref.number).await.inspect_err(|err| { + self.client.block_ref_by_number(local_safe.number).await.inspect_err(|err| { // todo: it's possible that supervisor is ahead of the op-node // in this case we should handle the error gracefully error!(target: "resetter", %err, "Failed to get block by number"); })?; // check with consistency with the op-node - if node_safe_ref.hash != local_safe_ref.hash { + if node_safe_ref.hash != local_safe.hash { // todo: handle this case error!(target: "resetter", "Local safe ref hash does not match node safe ref hash"); // returning ok here for now since this case should be handled @@ -69,21 +51,21 @@ where } info!(target: "resetter", - %unsafe_ref, - %cross_unsafe_ref, - %local_safe_ref, - %safe_ref, - %finalised_ref, + %local_unsafe, + %cross_unsafe, + %local_safe, + %cross_safe, + %finalized, "Resetting managed node with latest information", ); self.client .reset( - unsafe_ref.id(), - cross_unsafe_ref.id(), - local_safe_ref.id(), - safe_ref.id(), - finalised_ref.id(), + local_unsafe.id(), + cross_unsafe.id(), + local_safe.id(), + cross_safe.id(), + finalized.id(), ) .await .inspect_err(|err| { @@ -114,7 +96,6 @@ mod tests { pub Db {} impl HeadRefStorageReader for Db { - fn get_current_l1(&self) -> Result; fn get_safety_head_ref(&self, level: SafetyLevel) -> Result; fn get_super_head(&self) -> Result; } @@ -157,21 +138,7 @@ mod tests { let super_head = make_super_head(); let mut db = MockDb::new(); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::LocalUnsafe) - .returning(move |_| Ok(super_head.local_unsafe)); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::CrossUnsafe) - .returning(move |_| Ok(super_head.cross_unsafe)); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::LocalSafe) - .returning(move |_| Ok(super_head.local_safe)); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::CrossSafe) - .returning(move |_| Ok(super_head.cross_safe)); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::Finalized) - .returning(move |_| Ok(super_head.finalized)); + db.expect_get_super_head().returning(move || Ok(super_head.clone())); let mut client = MockClient::new(); client.expect_block_ref_by_number().returning(move |_| Ok(super_head.local_safe)); @@ -186,9 +153,7 @@ mod tests { #[tokio::test] async fn test_reset_db_error() { let mut db = MockDb::new(); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::LocalUnsafe) - .returning(move |_| Err(StorageError::DatabaseNotInitialised)); + db.expect_get_super_head().returning(|| Err(StorageError::DatabaseNotInitialised)); let client = MockClient::new(); @@ -202,21 +167,7 @@ mod tests { let super_head = make_super_head(); let mut db = MockDb::new(); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::LocalUnsafe) - .returning(move |_| Ok(super_head.local_unsafe)); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::CrossUnsafe) - .returning(move |_| Ok(super_head.cross_unsafe)); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::LocalSafe) - .returning(move |_| Ok(super_head.local_safe)); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::CrossSafe) - .returning(move |_| Ok(super_head.cross_safe)); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::Finalized) - .returning(move |_| Ok(super_head.finalized)); + db.expect_get_super_head().returning(move || Ok(super_head.clone())); let mut client = MockClient::new(); client.expect_block_ref_by_number().returning(|_| { @@ -233,21 +184,7 @@ mod tests { let super_head = make_super_head(); let mut db = MockDb::new(); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::LocalUnsafe) - .returning(move |_| Ok(super_head.local_unsafe)); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::CrossUnsafe) - .returning(move |_| Ok(super_head.cross_unsafe)); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::LocalSafe) - .returning(move |_| Ok(super_head.local_safe)); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::CrossSafe) - .returning(move |_| Ok(super_head.cross_safe)); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::Finalized) - .returning(move |_| Ok(super_head.finalized)); + db.expect_get_super_head().returning(move || Ok(super_head.clone())); let mut client = MockClient::new(); // Return a block that does not match local_safe @@ -265,21 +202,7 @@ mod tests { let super_head = make_super_head(); let mut db = MockDb::new(); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::LocalUnsafe) - .returning(move |_| Ok(super_head.local_unsafe)); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::CrossUnsafe) - .returning(move |_| Ok(super_head.cross_unsafe)); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::LocalSafe) - .returning(move |_| Ok(super_head.local_safe)); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::CrossSafe) - .returning(move |_| Ok(super_head.cross_safe)); - db.expect_get_safety_head_ref() - .withf(|level| *level == SafetyLevel::Finalized) - .returning(move |_| Ok(super_head.finalized)); + db.expect_get_super_head().returning(move || Ok(super_head.clone())); let mut client = MockClient::new(); client.expect_block_ref_by_number().returning(move |_| Ok(super_head.local_safe)); diff --git a/crates/supervisor/core/src/syncnode/task.rs b/crates/supervisor/core/src/syncnode/task.rs index dd5bd52760..9dbf7ca495 100644 --- a/crates/supervisor/core/src/syncnode/task.rs +++ b/crates/supervisor/core/src/syncnode/task.rs @@ -218,7 +218,6 @@ mod tests { } impl HeadRefStorageReader for Db { - fn get_current_l1(&self) -> Result; fn get_safety_head_ref(&self, level: SafetyLevel) -> Result; fn get_super_head(&self) -> Result; } diff --git a/crates/supervisor/storage/src/chaindb.rs b/crates/supervisor/storage/src/chaindb.rs index d0af74be62..4f3a52956b 100644 --- a/crates/supervisor/storage/src/chaindb.rs +++ b/crates/supervisor/storage/src/chaindb.rs @@ -22,8 +22,8 @@ use reth_db::{ mdbx::{DatabaseArguments, init_db_for}, }; use reth_db_api::database::Database; -use std::{path::Path, sync::RwLock}; -use tracing::{error, warn}; +use std::path::Path; +use tracing::warn; /// Manages the database environment for a single chain. /// Provides transactional access to data via providers. @@ -33,17 +33,13 @@ pub struct ChainDb { metrics_enabled: Option, env: DatabaseEnv, - - /// Current L1 block reference, used for tracking the latest L1 block processed. - /// In-memory only, not persisted. - current_l1: RwLock>, } impl ChainDb { /// Creates or opens a database environment at the given path. pub fn new(chain_id: ChainId, path: &Path) -> Result { let env = init_db_for::<_, crate::models::Tables>(path, DatabaseArguments::default())?; - Ok(Self { chain_id, metrics_enabled: None, env, current_l1: RwLock::new(None) }) + Ok(Self { chain_id, metrics_enabled: None, env }) } /// Enables metrics on the database environment. @@ -187,19 +183,6 @@ impl LogStorageWriter for ChainDb { } impl HeadRefStorageReader for ChainDb { - fn get_current_l1(&self) -> Result { - self.observe_call( - "get_current_l1", - || { - let guard = self.current_l1.read().map_err(|err| { - error!(target: "supervisor_storage", %err, "Failed to acquire read lock on current_l1"); - StorageError::LockPoisoned - })?; - guard.as_ref().cloned().ok_or(StorageError::FutureData) - } - ) - } - fn get_safety_head_ref(&self, safety_level: SafetyLevel) -> Result { self.observe_call("get_safety_head_ref", || { self.env.view(|tx| SafetyHeadRefProvider::new(tx).get_safety_head_ref(safety_level)) @@ -233,35 +216,6 @@ impl HeadRefStorageReader for ChainDb { } impl HeadRefStorageWriter for ChainDb { - fn update_current_l1(&self, block: BlockInfo) -> Result<(), StorageError> { - self.observe_call( - "update_current_l1", - || { - let mut guard = self - .current_l1 - .write() - .map_err(|err| { - error!(target: "supervisor_storage", %err, "Failed to acquire write lock on current_l1" ); - StorageError::LockPoisoned - })?; - - // Check if the new block number is greater than the current L1 block - if let Some(ref current) = *guard { - if block.number <= current.number { - error!(target: "supervisor_storage", - current_block_number = current.number, - new_block_number = block.number, - "New L1 block number is not greater than current L1 block number", - ); - return Err(StorageError::BlockOutOfOrder); - } - } - *guard = Some(block); - Ok(()) - }, - ) - } - fn update_finalized_using_source( &self, finalized_source_block: BlockInfo, @@ -577,35 +531,6 @@ mod tests { ); } - #[test] - fn test_update_and_get_current_l1() { - let tmp_dir = tempfile::TempDir::new().unwrap(); - let db_path = tmp_dir.path().join("chaindb_current_l1"); - let db = ChainDb::new(1, &db_path).unwrap(); - - let block1 = BlockInfo { number: 10, ..Default::default() }; - let block2 = BlockInfo { number: 20, ..Default::default() }; - - // Initially, get_current_l1 should return FutureData error - let err = db.get_current_l1().unwrap_err(); - assert!(matches!(err, StorageError::FutureData)); - - // Update current_l1 with block1 - db.update_current_l1(block1).unwrap(); - let got = db.get_current_l1().unwrap(); - assert_eq!(got, block1); - - // Update with a higher block number - db.update_current_l1(block2).unwrap(); - let got = db.get_current_l1().unwrap(); - assert_eq!(got, block2); - - // Update with a lower block number should error - let block3 = BlockInfo { number: 15, ..Default::default() }; - let err = db.update_current_l1(block3).unwrap_err(); - assert!(matches!(err, StorageError::BlockOutOfOrder)); - } - #[test] fn test_update_current_cross_unsafe() { let tmp_dir = tempfile::TempDir::new().unwrap(); diff --git a/crates/supervisor/storage/src/traits.rs b/crates/supervisor/storage/src/traits.rs index 3e4376a7eb..38f2adf4fe 100644 --- a/crates/supervisor/storage/src/traits.rs +++ b/crates/supervisor/storage/src/traits.rs @@ -17,7 +17,7 @@ pub trait DerivationStorageReader: Debug { /// Gets the source [`BlockInfo`] for a given derived block [`BlockNumHash`]. /// /// NOTE: [`LocalUnsafe`] block is not pushed to L1 yet, hence it cannot be part of derivation - /// storage. For reading latest L1 block in memory use [`HeadRefStorageReader::get_current_l1`]. + /// storage. /// /// # Arguments /// * `derived_block_id` - The identifier (number and hash) of the derived (L2) block. @@ -185,13 +185,6 @@ impl LogStorage for T {} /// Implementations are expected to provide persistent and thread-safe access to safety head /// references. pub trait HeadRefStorageReader: Debug { - /// Retrieves the current L1 block reference from the storage. - /// - /// # Returns - /// * `Ok(BlockInfo)` containing the current L1 block reference. - /// * `Err(StorageError)` if there is an issue retrieving the reference. - fn get_current_l1(&self) -> Result; - /// Retrieves the current [`BlockInfo`] for a given [`SafetyLevel`]. /// /// # Arguments @@ -218,16 +211,6 @@ pub trait HeadRefStorageReader: Debug { /// Implementations are expected to provide persistent and thread-safe access to safety head /// references. pub trait HeadRefStorageWriter: Debug { - /// Updates the current L1 block reference in the storage. - /// - /// # Arguments - /// * `block` - The new [`BlockInfo`] to set as the current L1 block reference. - /// - /// # Returns - /// * `Ok(())` if the reference was successfully updated. - /// * `Err(StorageError)` if there is an issue updating the reference. - fn update_current_l1(&self, block: BlockInfo) -> Result<(), StorageError>; - /// Updates the finalized head reference using a finalized source(l1) block. /// /// # Arguments From fc34dd33b0a5321eaa15f9b9e0f39ae1cf7741de Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Mon, 7 Jul 2025 17:03:12 +0530 Subject: [PATCH 06/27] lintfix --- crates/supervisor/core/src/syncnode/resetter.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/supervisor/core/src/syncnode/resetter.rs b/crates/supervisor/core/src/syncnode/resetter.rs index fd59b32240..fa571b8613 100644 --- a/crates/supervisor/core/src/syncnode/resetter.rs +++ b/crates/supervisor/core/src/syncnode/resetter.rs @@ -138,7 +138,7 @@ mod tests { let super_head = make_super_head(); let mut db = MockDb::new(); - db.expect_get_super_head().returning(move || Ok(super_head.clone())); + db.expect_get_super_head().returning(move || Ok(super_head)); let mut client = MockClient::new(); client.expect_block_ref_by_number().returning(move |_| Ok(super_head.local_safe)); @@ -167,7 +167,7 @@ mod tests { let super_head = make_super_head(); let mut db = MockDb::new(); - db.expect_get_super_head().returning(move || Ok(super_head.clone())); + db.expect_get_super_head().returning(move || Ok(super_head)); let mut client = MockClient::new(); client.expect_block_ref_by_number().returning(|_| { @@ -184,7 +184,7 @@ mod tests { let super_head = make_super_head(); let mut db = MockDb::new(); - db.expect_get_super_head().returning(move || Ok(super_head.clone())); + db.expect_get_super_head().returning(move || Ok(super_head)); let mut client = MockClient::new(); // Return a block that does not match local_safe @@ -202,7 +202,7 @@ mod tests { let super_head = make_super_head(); let mut db = MockDb::new(); - db.expect_get_super_head().returning(move || Ok(super_head.clone())); + db.expect_get_super_head().returning(move || Ok(super_head)); let mut client = MockClient::new(); client.expect_block_ref_by_number().returning(move |_| Ok(super_head.local_safe)); From 42fa9dbd2f7c3bcd2760ab7b35b4ba0e14fa6eeb Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Tue, 8 Jul 2025 15:20:29 +0530 Subject: [PATCH 07/27] feat(supervisor): preinterop db support --- .../core/src/chain_processor/chain.rs | 11 ++- .../core/src/chain_processor/task.rs | 26 ++++-- .../core/src/config/rollup_config_set.rs | 4 +- crates/supervisor/core/src/supervisor.rs | 6 +- crates/supervisor/storage/src/chaindb.rs | 5 +- .../src/providers/derivation_provider.rs | 90 +++++++------------ .../storage/src/providers/log_provider.rs | 22 ++--- 7 files changed, 76 insertions(+), 88 deletions(-) diff --git a/crates/supervisor/core/src/chain_processor/chain.rs b/crates/supervisor/core/src/chain_processor/chain.rs index ed2f763977..e874c3f5ef 100644 --- a/crates/supervisor/core/src/chain_processor/chain.rs +++ b/crates/supervisor/core/src/chain_processor/chain.rs @@ -1,5 +1,5 @@ use super::{ChainProcessorError, ChainProcessorTask}; -use crate::{event::ChainEvent, syncnode::ManagedNodeProvider}; +use crate::{config::RollupConfig, event::ChainEvent, syncnode::ManagedNodeProvider}; use alloy_primitives::ChainId; use kona_supervisor_storage::{DerivationStorageWriter, HeadRefStorageWriter, LogStorageWriter}; use std::sync::Arc; @@ -16,6 +16,9 @@ use tracing::warn; // chain processor will support multiple managed nodes in the future. #[derive(Debug)] pub struct ChainProcessor { + // The rollup configuration for the chain + rollup_config: RollupConfig, + // The chainId that this processor is associated with chain_id: ChainId, @@ -45,6 +48,7 @@ where { /// Creates a new instance of [`ChainProcessor`]. pub fn new( + rollup_config: RollupConfig, chain_id: ChainId, managed_node: Arc

, state_manager: Arc, @@ -52,6 +56,7 @@ where ) -> Self { // todo: validate chain_id against managed_node Self { + rollup_config, chain_id, event_tx: None, metrics_enabled: None, @@ -93,6 +98,7 @@ where self.managed_node.start_subscription(event_tx.clone()).await?; let mut task = ChainProcessorTask::new( + self.rollup_config.clone(), self.chain_id, self.managed_node.clone(), self.state_manager.clone(), @@ -268,8 +274,9 @@ mod tests { let storage = Arc::new(MockDb::new()); let cancel_token = CancellationToken::new(); + let rollup_config = RollupConfig::default(); let mut processor = - ChainProcessor::new(1, Arc::clone(&mock_node), Arc::clone(&storage), cancel_token); + ChainProcessor::new(rollup_config, 1, Arc::clone(&mock_node), Arc::clone(&storage), cancel_token); assert!(processor.start().await.is_ok()); diff --git a/crates/supervisor/core/src/chain_processor/task.rs b/crates/supervisor/core/src/chain_processor/task.rs index 79715efde7..5989defeab 100644 --- a/crates/supervisor/core/src/chain_processor/task.rs +++ b/crates/supervisor/core/src/chain_processor/task.rs @@ -1,5 +1,5 @@ use super::Metrics; -use crate::{ChainProcessorError, LogIndexer, event::ChainEvent, syncnode::ManagedNodeProvider}; +use crate::{config::RollupConfig, event::ChainEvent, syncnode::ManagedNodeProvider, ChainProcessorError, LogIndexer}; use alloy_primitives::ChainId; use kona_interop::{BlockReplacement, DerivedRefPair}; use kona_protocol::BlockInfo; @@ -15,6 +15,7 @@ use tracing::{debug, error, info}; /// It listens for events emitted by the managed node and handles them accordingly. #[derive(Debug)] pub struct ChainProcessorTask { + rollup_config: RollupConfig, chain_id: ChainId, metrics_enabled: Option, @@ -37,6 +38,7 @@ where { /// Creates a new [`ChainProcessorTask`]. pub fn new( + rollup_config: RollupConfig, chain_id: u64, managed_node: Arc

, state_manager: Arc, @@ -45,6 +47,7 @@ where ) -> Self { let log_indexer = LogIndexer::new(managed_node.clone(), state_manager.clone()); Self { + rollup_config, chain_id, metrics_enabled: None, cancel_token, @@ -539,7 +542,8 @@ mod tests { let cancel_token = CancellationToken::new(); let (tx, rx) = mpsc::channel(10); - let task = ChainProcessorTask::new(1, managed_node, writer, cancel_token.clone(), rx); + let rollup_config = RollupConfig::default(); + let task = ChainProcessorTask::new(rollup_config, 1, managed_node, writer, cancel_token.clone(), rx); tx.send(ChainEvent::UnsafeBlock { block }).await.unwrap(); @@ -584,7 +588,8 @@ mod tests { let cancel_token = CancellationToken::new(); let (tx, rx) = mpsc::channel(10); - let task = ChainProcessorTask::new(1, managed_node, writer, cancel_token.clone(), rx); + let rollup_config = RollupConfig::default(); + let task = ChainProcessorTask::new(rollup_config, 1, managed_node, writer, cancel_token.clone(), rx); // Send unsafe block event tx.send(ChainEvent::DerivedBlock { derived_ref_pair: block_pair }).await.unwrap(); @@ -619,7 +624,8 @@ mod tests { let cancel_token = CancellationToken::new(); let (tx, rx) = mpsc::channel(10); - let task = ChainProcessorTask::new(1, managed_node, writer, cancel_token.clone(), rx); + let rollup_config = RollupConfig::default(); + let task = ChainProcessorTask::new(rollup_config, 1, managed_node, writer, cancel_token.clone(), rx); // Send derivation origin update event tx.send(ChainEvent::DerivationOriginUpdate { origin }).await.unwrap(); @@ -665,7 +671,8 @@ mod tests { let cancel_token = CancellationToken::new(); let (tx, rx) = mpsc::channel(10); - let task = ChainProcessorTask::new(1, managed_node, writer, cancel_token.clone(), rx); + let rollup_config = RollupConfig::default(); + let task = ChainProcessorTask::new(rollup_config, 1, managed_node, writer, cancel_token.clone(), rx); // Send FinalizedSourceUpdate event tx.send(ChainEvent::FinalizedSourceUpdate { finalized_source_block }).await.unwrap(); @@ -702,7 +709,8 @@ mod tests { let cancel_token = CancellationToken::new(); let (tx, rx) = mpsc::channel(10); - let task = ChainProcessorTask::new(1, managed_node, writer, cancel_token.clone(), rx); + let rollup_config = RollupConfig::default(); + let task = ChainProcessorTask::new(rollup_config, 1, managed_node, writer, cancel_token.clone(), rx); // Send FinalizedSourceUpdate event tx.send(ChainEvent::FinalizedSourceUpdate { finalized_source_block }).await.unwrap(); @@ -736,7 +744,8 @@ mod tests { let cancel_token = CancellationToken::new(); let (tx, rx) = mpsc::channel(10); - let task = ChainProcessorTask::new(1, managed_node, writer, cancel_token.clone(), rx); + let rollup_config = RollupConfig::default(); + let task = ChainProcessorTask::new(rollup_config, 1, managed_node, writer, cancel_token.clone(), rx); // Send derivation origin update event tx.send(ChainEvent::CrossUnsafeUpdate { block }).await.unwrap(); @@ -773,7 +782,8 @@ mod tests { let cancel_token = CancellationToken::new(); let (tx, rx) = mpsc::channel(10); - let task = ChainProcessorTask::new(1, managed_node, writer, cancel_token.clone(), rx); + let rollup_config = RollupConfig::default(); + let task = ChainProcessorTask::new(rollup_config, 1, managed_node, writer, cancel_token.clone(), rx); // Send derivation origin update event tx.send(ChainEvent::CrossSafeUpdate { diff --git a/crates/supervisor/core/src/config/rollup_config_set.rs b/crates/supervisor/core/src/config/rollup_config_set.rs index 3483840f8a..3ce6437484 100644 --- a/crates/supervisor/core/src/config/rollup_config_set.rs +++ b/crates/supervisor/core/src/config/rollup_config_set.rs @@ -7,7 +7,7 @@ use std::collections::HashMap; use crate::SupervisorError; /// Genesis provides the genesis information relevant for Interop. -#[derive(Debug, Clone)] +#[derive(Debug, Default, Clone)] pub struct Genesis { /// The L1 [`BlockInfo`] that the rollup starts after. pub l1: BlockInfo, @@ -36,7 +36,7 @@ impl Genesis { } /// RollupConfig contains the configuration for the Optimism rollup. -#[derive(Debug, Clone)] +#[derive(Debug, Default, Clone)] pub struct RollupConfig { /// Genesis anchor information for the rollup. pub genesis: Genesis, diff --git a/crates/supervisor/core/src/supervisor.rs b/crates/supervisor/core/src/supervisor.rs index 2be40314eb..c3f306337f 100644 --- a/crates/supervisor/core/src/supervisor.rs +++ b/crates/supervisor/core/src/supervisor.rs @@ -159,9 +159,13 @@ impl Supervisor { chain_id )))?; + let rollup_config = self.config.rollup_config_set.get(*chain_id).ok_or( + SupervisorError::Initialise(format!("no rollup config found for chain {}", chain_id)) + )?; + // initialise chain processor for the chain. let mut processor = - ChainProcessor::new(*chain_id, managed_node.clone(), db, self.cancel_token.clone()); + ChainProcessor::new(rollup_config.clone(), *chain_id, managed_node.clone(), db, self.cancel_token.clone()); // todo: enable metrics only if configured processor = processor.with_metrics(); diff --git a/crates/supervisor/storage/src/chaindb.rs b/crates/supervisor/storage/src/chaindb.rs index 4f3a52956b..06b991b21a 100644 --- a/crates/supervisor/storage/src/chaindb.rs +++ b/crates/supervisor/storage/src/chaindb.rs @@ -71,8 +71,8 @@ impl ChainDb { /// initialises the database with a given anchor derived block pair. pub fn initialise(&self, anchor: DerivedRefPair) -> Result<(), StorageError> { self.env.update(|tx| { - DerivationProvider::new(tx).initialise(anchor)?; - LogProvider::new(tx).initialise(anchor.derived)?; + DerivationProvider::new(tx).save_derived_block_pair(anchor)?; + LogProvider::new(tx).store_block_logs(&anchor.derived, Vec::new())?; let sp = SafetyHeadRefProvider::new(tx); // todo: cross check if we can consider following safety head ref update @@ -110,7 +110,6 @@ impl DerivationStorageReader for ChainDb { } impl DerivationStorageWriter for ChainDb { - // Todo: better name save_derived_block_pair fn save_derived_block(&self, incoming_pair: DerivedRefPair) -> Result<(), StorageError> { self.observe_call("save_derived_block_pair", || { self.env.update(|ctx| { diff --git a/crates/supervisor/storage/src/providers/derivation_provider.rs b/crates/supervisor/storage/src/providers/derivation_provider.rs index 01c32aa32a..03db4d21d2 100644 --- a/crates/supervisor/storage/src/providers/derivation_provider.rs +++ b/crates/supervisor/storage/src/providers/derivation_provider.rs @@ -239,26 +239,6 @@ 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> { - match self.get_derived_block_pair_by_number(0) { - Ok(pair) - if pair.derived.hash == anchor.derived.hash && - pair.source.hash == anchor.source.hash => - { - // Anchor matches, nothing to do - Ok(()) - } - Ok(_) => Err(StorageError::InvalidAnchor), - Err(StorageError::EntryNotFound(_)) => { - self.save_source_block_internal(anchor.source)?; - self.save_derived_block_pair_internal(anchor)?; - Ok(()) - } - Err(err) => Err(err), - } - } - /// Saves a [`StoredDerivedBlockPair`] to [`DerivedBlocks`](`crate::models::DerivedBlocks`) /// table and [`SourceBlockTraversal`] to [`BlockTraversal`](`crate::models::BlockTraversal`) /// table in the database. @@ -269,7 +249,12 @@ where // todo: use cursor to get the last block(performance improvement) let latest_derivation_state = match self.latest_derivation_state() { Ok(pair) => pair, - Err(StorageError::EntryNotFound(_)) => return Err(StorageError::DatabaseNotInitialised), + Err(StorageError::EntryNotFound(_)) => { + // in this case the database is not initialised, so we need to save the source block first + self.save_source_block_internal(incoming_pair.source)?; + self.save_derived_block_pair_internal(incoming_pair)?; + return Ok(()); + }, Err(e) => return Err(e), }; @@ -499,17 +484,6 @@ mod tests { .expect("Failed to init database") } - /// Helper to initialize database in a new transaction, committing if successful. - fn initialize_db(db: &DatabaseEnv, pair: &DerivedRefPair) -> Result<(), StorageError> { - let tx = db.tx_mut().expect("Could not get mutable tx"); - let provider = DerivationProvider::new(&tx); - let res = provider.initialise(*pair); - if res.is_ok() { - tx.commit().expect("Failed to commit transaction"); - } - res - } - /// Helper to insert a pair in a new transaction, committing if successful. fn insert_pair(db: &DatabaseEnv, pair: &DerivedRefPair) -> Result<(), StorageError> { let tx = db.tx_mut().expect("Could not get mutable tx"); @@ -541,7 +515,7 @@ mod tests { let anchor = derived_pair(source, derived); // Should succeed and insert the anchor - assert!(initialize_db(&db, &anchor).is_ok()); + assert!(insert_pair(&db, &anchor).is_ok()); // Check that the anchor is present let tx = db.tx().expect("Could not get tx"); @@ -559,9 +533,9 @@ mod tests { let anchor = derived_pair(source, genesis_block()); // First initialise - assert!(initialize_db(&db, &anchor).is_ok()); + assert!(insert_pair(&db, &anchor).is_ok()); // Second initialise with the same anchor should succeed (idempotent) - assert!(initialize_db(&db, &anchor).is_ok()); + assert!(insert_pair(&db, &anchor).is_ok()); } #[test] @@ -572,14 +546,14 @@ mod tests { let anchor = derived_pair(source, genesis_block()); // Insert the genesis - assert!(initialize_db(&db, &anchor).is_ok()); + assert!(insert_pair(&db, &anchor).is_ok()); // Try to initialise with a different anchor (different hash) - let wrong_derived = block_info(1, B256::from([42u8; 32]), 200); + let wrong_derived = block_info(0, B256::from([42u8; 32]), 200); let wrong_anchor = derived_pair(source, wrong_derived); - let result = initialize_db(&db, &wrong_anchor); - assert!(matches!(result, Err(StorageError::InvalidAnchor))); + let result = insert_pair(&db, &wrong_anchor); + assert!(matches!(result, Err(StorageError::ConflictError(_)))); } #[test] @@ -589,7 +563,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); let derived2 = block_info(2, derived1.hash, 300); let pair2 = derived_pair(source1, derived2); @@ -609,7 +583,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); let wrong_parent_hash = B256::from([99u8; 32]); let derived2 = block_info(2, wrong_parent_hash, 300); @@ -625,7 +599,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); let derived2 = block_info(4, derived1.hash, 400); // should be 2, not 4 let pair2 = derived_pair(source1, derived2); @@ -640,7 +614,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); // Try to insert the same derived block again let result = insert_pair(&db, &pair1); @@ -654,7 +628,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); let derived2 = block_info(2, derived1.hash, 300); let pair2 = derived_pair(source1, derived2); @@ -672,7 +646,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); let derived2 = block_info(2, derived1.hash, 300); let pair2 = derived_pair(source1, derived2); @@ -692,7 +666,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); let derived2 = block_info(2, derived1.hash, 300); let pair2 = derived_pair(source1, derived2); @@ -737,7 +711,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); // Use correct number but wrong hash let tx = db.tx().expect("Could not get tx"); @@ -755,7 +729,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); let derived2 = block_info(2, derived1.hash, 300); let pair2 = derived_pair(source1, derived2); @@ -775,7 +749,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); let source2 = block_info(101, source1.hash, 300); let derived2 = block_info(2, derived1.hash, 300); @@ -811,7 +785,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); let tx = db.tx().expect("Could not get tx"); let provider = DerivationProvider::new(&tx); @@ -839,7 +813,7 @@ mod tests { let derived0 = block_info(10, B256::from([10u8; 32]), 200); let pair1 = derived_pair(genesis_block(), derived0); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); let source1 = block_info(1, genesis_block().hash, 200); assert!(insert_source_block(&db, &source1).is_ok()); @@ -851,7 +825,7 @@ mod tests { let derived0 = block_info(10, B256::from([10u8; 32]), 200); let pair1 = derived_pair(genesis_block(), derived0); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); let source1 = block_info(1, genesis_block().hash, 200); assert!(insert_source_block(&db, &source1).is_ok()); @@ -866,7 +840,7 @@ mod tests { let source0 = block_info(10, B256::from([10u8; 32]), 200); let derived0 = genesis_block(); let pair1 = derived_pair(source0, derived0); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); let source1 = block_info(11, B256::from([1u8; 32]), 200); let result = insert_source_block(&db, &source1); @@ -883,7 +857,7 @@ mod tests { let source0 = block_info(10, B256::from([10u8; 32]), 200); let derived0 = genesis_block(); let pair1 = derived_pair(source0, derived0); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); let source1 = block_info(11, source0.hash, 400); assert!(insert_source_block(&db, &source1).is_ok()); @@ -900,7 +874,7 @@ mod tests { let source0 = block_info(10, B256::from([10u8; 32]), 200); let derived0 = genesis_block(); let pair1 = derived_pair(source0, derived0); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); let source1 = block_info(11, source0.hash, 400); assert!(insert_source_block(&db, &source1).is_ok()); @@ -917,7 +891,7 @@ mod tests { let derived0 = block_info(10, B256::from([10u8; 32]), 200); let pair1 = derived_pair(genesis_block(), derived0); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); let source1 = block_info(2, genesis_block().hash, 400); // Try to skip a block @@ -931,7 +905,7 @@ mod tests { let derived0 = block_info(10, B256::from([10u8; 32]), 200); let pair1 = derived_pair(genesis_block(), derived0); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); let source1 = block_info(1, genesis_block().hash, 200); let source2 = block_info(2, source1.hash, 400); @@ -945,7 +919,7 @@ mod tests { let derived0 = block_info(10, B256::from([10u8; 32]), 200); let pair1 = derived_pair(genesis_block(), derived0); - assert!(initialize_db(&db, &pair1).is_ok()); + assert!(insert_pair(&db, &pair1).is_ok()); let source1 = block_info(1, genesis_block().hash, 200); assert!(insert_source_block(&db, &source1).is_ok()); diff --git a/crates/supervisor/storage/src/providers/log_provider.rs b/crates/supervisor/storage/src/providers/log_provider.rs index a3b165786e..3e128153fa 100644 --- a/crates/supervisor/storage/src/providers/log_provider.rs +++ b/crates/supervisor/storage/src/providers/log_provider.rs @@ -43,18 +43,6 @@ impl LogProvider<'_, TX> where TX: DbTxMut + DbTx, { - pub(crate) fn initialise(&self, anchor: BlockInfo) -> Result<(), StorageError> { - match self.get_block(0) { - Ok(block) if block.hash == anchor.hash => Ok(()), - Ok(_) => Err(StorageError::InvalidAnchor), - Err(StorageError::EntryNotFound(_)) => { - self.store_block_logs_internal(&anchor, Vec::new()) - } - - Err(err) => Err(err), - } - } - pub(crate) fn store_block_logs( &self, block: &BlockInfo, @@ -64,10 +52,16 @@ where let latest_block = match self.get_latest_block() { Ok(block) => block, - Err(StorageError::EntryNotFound(_)) => return Err(StorageError::DatabaseNotInitialised), + Err(StorageError::EntryNotFound(_)) => { + // If no blocks are found, this is the first block being stored. + self.store_block_logs_internal(block, logs)?; + return Ok(()); + }, Err(e) => return Err(e), }; + // todo: handle idempotency + if !latest_block.is_parent_of(block) { warn!( target: "supervisor_storage", @@ -283,7 +277,7 @@ mod tests { fn initialize_db(db: &DatabaseEnv, block: &BlockInfo) -> Result<(), StorageError> { let tx = db.tx_mut().expect("Could not get mutable tx"); let provider = LogProvider::new(&tx); - let res = provider.initialise(*block); + let res = provider.store_block_logs(block, Vec::new()); if res.is_ok() { tx.commit().expect("Failed to commit transaction"); } else { From db55affadbf8245f21c36917234269a1d77cfd99 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Tue, 8 Jul 2025 15:45:21 +0530 Subject: [PATCH 08/27] reset pre-interop api integrated --- crates/supervisor/core/src/syncnode/client.rs | 18 ++++++++++++++++++ .../supervisor/core/src/syncnode/resetter.rs | 1 + crates/supervisor/core/src/syncnode/task.rs | 1 + crates/supervisor/rpc/src/jsonrpsee.rs | 4 ++++ 4 files changed, 24 insertions(+) diff --git a/crates/supervisor/core/src/syncnode/client.rs b/crates/supervisor/core/src/syncnode/client.rs index b922ff4036..d4e907104b 100644 --- a/crates/supervisor/core/src/syncnode/client.rs +++ b/crates/supervisor/core/src/syncnode/client.rs @@ -47,6 +47,9 @@ pub trait ManagedNodeClient: Debug { /// Fetches the [`BlockInfo`] by block number. async fn block_ref_by_number(&self, block_number: u64) -> Result; + /// Resets the managed node to the pre-interop state. + async fn reset_pre_interop(&self) -> Result<(), ManagedNodeError>; + /// Resets the node state with the provided block IDs. async fn reset( &self, @@ -312,6 +315,21 @@ impl ManagedNodeClient for Client { Ok(block_info) } + async fn reset_pre_interop(&self) -> Result<(), ManagedNodeError> { + let client = self.get_ws_client().await?; + observe_metrics_for_result_async!( + Metrics::MANAGED_NODE_RPC_REQUESTS_SUCCESS_TOTAL, + Metrics::MANAGED_NODE_RPC_REQUESTS_ERROR_TOTAL, + Metrics::MANAGED_NODE_RPC_REQUEST_DURATION_SECONDS, + "reset_pre_interop", + async { + ManagedModeApiClient::reset_pre_interop(client.as_ref()).await + }, + "node" => self.config.url.clone() + )?; + Ok(()) + } + async fn reset( &self, unsafe_id: BlockNumHash, diff --git a/crates/supervisor/core/src/syncnode/resetter.rs b/crates/supervisor/core/src/syncnode/resetter.rs index fa571b8613..13f330b7fb 100644 --- a/crates/supervisor/core/src/syncnode/resetter.rs +++ b/crates/supervisor/core/src/syncnode/resetter.rs @@ -114,6 +114,7 @@ mod tests { async fn pending_output_v0_at_timestamp(&self, timestamp: u64) -> Result; async fn l2_block_ref_by_timestamp(&self, timestamp: u64) -> Result; async fn block_ref_by_number(&self, block_number: u64) -> Result; + async fn reset_pre_interop(&self) -> Result<(), ManagedNodeError>; async fn reset(&self, unsafe_id: BlockNumHash, cross_unsafe_id: BlockNumHash, local_safe_id: BlockNumHash, cross_safe_id: BlockNumHash, finalised_id: BlockNumHash) -> Result<(), ManagedNodeError>; async fn provide_l1(&self, block_info: BlockInfo) -> Result<(), ManagedNodeError>; async fn update_finalized(&self, finalized_block_id: BlockNumHash) -> Result<(), ManagedNodeError>; diff --git a/crates/supervisor/core/src/syncnode/task.rs b/crates/supervisor/core/src/syncnode/task.rs index 9dbf7ca495..9d1772542c 100644 --- a/crates/supervisor/core/src/syncnode/task.rs +++ b/crates/supervisor/core/src/syncnode/task.rs @@ -236,6 +236,7 @@ mod tests { async fn pending_output_v0_at_timestamp(&self, timestamp: u64) -> Result; async fn l2_block_ref_by_timestamp(&self, timestamp: u64) -> Result; async fn block_ref_by_number(&self, block_number: u64) -> Result; + async fn reset_pre_interop(&self) -> Result<(), ManagedNodeError>; async fn reset(&self, unsafe_id: BlockNumHash, cross_unsafe_id: BlockNumHash, local_safe_id: BlockNumHash, cross_safe_id: BlockNumHash, finalised_id: BlockNumHash) -> Result<(), ManagedNodeError>; async fn provide_l1(&self, block_info: BlockInfo) -> Result<(), ManagedNodeError>; async fn update_finalized(&self, finalized_block_id: BlockNumHash) -> Result<(), ManagedNodeError>; diff --git a/crates/supervisor/rpc/src/jsonrpsee.rs b/crates/supervisor/rpc/src/jsonrpsee.rs index ada988367e..de776cdb3f 100644 --- a/crates/supervisor/rpc/src/jsonrpsee.rs +++ b/crates/supervisor/rpc/src/jsonrpsee.rs @@ -170,6 +170,10 @@ pub trait ManagedModeApi { #[method(name = "anchorPoint")] async fn anchor_point(&self) -> RpcResult; + /// Reset the managed node to the pre-interop state + #[method(name = "resetPreInterop")] + async fn reset_pre_interop(&self) -> RpcResult<()>; + /// Reset the managed node to the specified block heads #[method(name = "reset")] async fn reset( From 916492697aa05ce8bbde7bb19083611f2f672750 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Tue, 8 Jul 2025 16:09:16 +0530 Subject: [PATCH 09/27] pluggedin db initialisation error --- .../core/src/chain_processor/chain.rs | 9 ++- .../core/src/chain_processor/task.rs | 68 ++++++++++++++++--- crates/supervisor/core/src/supervisor.rs | 16 +++-- crates/supervisor/storage/src/chaindb.rs | 35 ++++++++++ .../src/providers/derivation_provider.rs | 22 +++++- .../storage/src/providers/log_provider.rs | 15 +++- 6 files changed, 145 insertions(+), 20 deletions(-) diff --git a/crates/supervisor/core/src/chain_processor/chain.rs b/crates/supervisor/core/src/chain_processor/chain.rs index e874c3f5ef..2a498b75ea 100644 --- a/crates/supervisor/core/src/chain_processor/chain.rs +++ b/crates/supervisor/core/src/chain_processor/chain.rs @@ -275,8 +275,13 @@ mod tests { let cancel_token = CancellationToken::new(); let rollup_config = RollupConfig::default(); - let mut processor = - ChainProcessor::new(rollup_config, 1, Arc::clone(&mock_node), Arc::clone(&storage), cancel_token); + let mut processor = ChainProcessor::new( + rollup_config, + 1, + Arc::clone(&mock_node), + Arc::clone(&storage), + cancel_token, + ); assert!(processor.start().await.is_ok()); diff --git a/crates/supervisor/core/src/chain_processor/task.rs b/crates/supervisor/core/src/chain_processor/task.rs index 5989defeab..bb63f28184 100644 --- a/crates/supervisor/core/src/chain_processor/task.rs +++ b/crates/supervisor/core/src/chain_processor/task.rs @@ -1,5 +1,8 @@ use super::Metrics; -use crate::{config::RollupConfig, event::ChainEvent, syncnode::ManagedNodeProvider, ChainProcessorError, LogIndexer}; +use crate::{ + ChainProcessorError, LogIndexer, config::RollupConfig, event::ChainEvent, + syncnode::ManagedNodeProvider, +}; use alloy_primitives::ChainId; use kona_interop::{BlockReplacement, DerivedRefPair}; use kona_protocol::BlockInfo; @@ -543,7 +546,14 @@ mod tests { let (tx, rx) = mpsc::channel(10); let rollup_config = RollupConfig::default(); - let task = ChainProcessorTask::new(rollup_config, 1, managed_node, writer, cancel_token.clone(), rx); + let task = ChainProcessorTask::new( + rollup_config, + 1, + managed_node, + writer, + cancel_token.clone(), + rx, + ); tx.send(ChainEvent::UnsafeBlock { block }).await.unwrap(); @@ -589,7 +599,14 @@ mod tests { let (tx, rx) = mpsc::channel(10); let rollup_config = RollupConfig::default(); - let task = ChainProcessorTask::new(rollup_config, 1, managed_node, writer, cancel_token.clone(), rx); + let task = ChainProcessorTask::new( + rollup_config, + 1, + managed_node, + writer, + cancel_token.clone(), + rx, + ); // Send unsafe block event tx.send(ChainEvent::DerivedBlock { derived_ref_pair: block_pair }).await.unwrap(); @@ -625,7 +642,14 @@ mod tests { let (tx, rx) = mpsc::channel(10); let rollup_config = RollupConfig::default(); - let task = ChainProcessorTask::new(rollup_config, 1, managed_node, writer, cancel_token.clone(), rx); + let task = ChainProcessorTask::new( + rollup_config, + 1, + managed_node, + writer, + cancel_token.clone(), + rx, + ); // Send derivation origin update event tx.send(ChainEvent::DerivationOriginUpdate { origin }).await.unwrap(); @@ -672,7 +696,14 @@ mod tests { let (tx, rx) = mpsc::channel(10); let rollup_config = RollupConfig::default(); - let task = ChainProcessorTask::new(rollup_config, 1, managed_node, writer, cancel_token.clone(), rx); + let task = ChainProcessorTask::new( + rollup_config, + 1, + managed_node, + writer, + cancel_token.clone(), + rx, + ); // Send FinalizedSourceUpdate event tx.send(ChainEvent::FinalizedSourceUpdate { finalized_source_block }).await.unwrap(); @@ -710,7 +741,14 @@ mod tests { let (tx, rx) = mpsc::channel(10); let rollup_config = RollupConfig::default(); - let task = ChainProcessorTask::new(rollup_config, 1, managed_node, writer, cancel_token.clone(), rx); + let task = ChainProcessorTask::new( + rollup_config, + 1, + managed_node, + writer, + cancel_token.clone(), + rx, + ); // Send FinalizedSourceUpdate event tx.send(ChainEvent::FinalizedSourceUpdate { finalized_source_block }).await.unwrap(); @@ -745,7 +783,14 @@ mod tests { let (tx, rx) = mpsc::channel(10); let rollup_config = RollupConfig::default(); - let task = ChainProcessorTask::new(rollup_config, 1, managed_node, writer, cancel_token.clone(), rx); + let task = ChainProcessorTask::new( + rollup_config, + 1, + managed_node, + writer, + cancel_token.clone(), + rx, + ); // Send derivation origin update event tx.send(ChainEvent::CrossUnsafeUpdate { block }).await.unwrap(); @@ -783,7 +828,14 @@ mod tests { let (tx, rx) = mpsc::channel(10); let rollup_config = RollupConfig::default(); - let task = ChainProcessorTask::new(rollup_config, 1, managed_node, writer, cancel_token.clone(), rx); + let task = ChainProcessorTask::new( + rollup_config, + 1, + managed_node, + writer, + cancel_token.clone(), + rx, + ); // Send derivation origin update event tx.send(ChainEvent::CrossSafeUpdate { diff --git a/crates/supervisor/core/src/supervisor.rs b/crates/supervisor/core/src/supervisor.rs index c3f306337f..ab13e9552a 100644 --- a/crates/supervisor/core/src/supervisor.rs +++ b/crates/supervisor/core/src/supervisor.rs @@ -159,13 +159,19 @@ impl Supervisor { chain_id )))?; - let rollup_config = self.config.rollup_config_set.get(*chain_id).ok_or( - SupervisorError::Initialise(format!("no rollup config found for chain {}", chain_id)) - )?; + let rollup_config = + self.config.rollup_config_set.get(*chain_id).ok_or(SupervisorError::Initialise( + format!("no rollup config found for chain {}", chain_id), + ))?; // initialise chain processor for the chain. - let mut processor = - ChainProcessor::new(rollup_config.clone(), *chain_id, managed_node.clone(), db, self.cancel_token.clone()); + let mut processor = ChainProcessor::new( + rollup_config.clone(), + *chain_id, + managed_node.clone(), + db, + self.cancel_token.clone(), + ); // todo: enable metrics only if configured processor = processor.with_metrics(); diff --git a/crates/supervisor/storage/src/chaindb.rs b/crates/supervisor/storage/src/chaindb.rs index 06b991b21a..ced0bc5956 100644 --- a/crates/supervisor/storage/src/chaindb.rs +++ b/crates/supervisor/storage/src/chaindb.rs @@ -84,6 +84,7 @@ impl ChainDb { } } +// todo: make sure all get method return DatabaseNotInitialised error if db is not initialised impl DerivationStorageReader for ChainDb { fn derived_to_source(&self, derived_block_id: BlockNumHash) -> Result { self.observe_call("derived_to_source", || { @@ -142,6 +143,7 @@ impl DerivationStorageWriter for ChainDb { } } +// todo: make sure all get method return DatabaseNotInitialised error if db is not initialised impl LogStorageReader for ChainDb { fn get_latest_block(&self) -> Result { self.observe_call("get_latest_block", || { @@ -446,6 +448,39 @@ mod tests { assert_eq!(log, logs[1], "Block by log should match stored block"); } + #[test] + fn test_super_head_empty() { + let tmp_dir = TempDir::new().expect("create temp dir"); + let db_path = tmp_dir.path().join("chaindb_super_head_empty"); + let db = ChainDb::new(1, &db_path).expect("create db"); + + // Get super head when no blocks are stored + let err = db.get_super_head().unwrap_err(); + assert!(matches!(err, StorageError::DatabaseNotInitialised)); + } + + #[test] + fn test_latest_derivation_state_empty() { + let tmp_dir = TempDir::new().expect("create temp dir"); + let db_path = tmp_dir.path().join("chaindb_latest_derivation_empty"); + let db = ChainDb::new(1, &db_path).expect("create db"); + + // Get latest derivation state when no blocks are stored + let err = db.latest_derivation_state().unwrap_err(); + assert!(matches!(err, StorageError::DatabaseNotInitialised)); + } + + #[test] + fn test_get_latest_block_empty() { + let tmp_dir = TempDir::new().expect("create temp dir"); + let db_path = tmp_dir.path().join("chaindb_latest_block_empty"); + let db = ChainDb::new(1, &db_path).expect("create db"); + + // Get latest block when no blocks are stored + let err = db.get_latest_block().unwrap_err(); + assert!(matches!(err, StorageError::DatabaseNotInitialised)); + } + #[test] fn test_derivation_storage() { let tmp_dir = TempDir::new().expect("create temp dir"); diff --git a/crates/supervisor/storage/src/providers/derivation_provider.rs b/crates/supervisor/storage/src/providers/derivation_provider.rs index 03db4d21d2..4c17cd87d3 100644 --- a/crates/supervisor/storage/src/providers/derivation_provider.rs +++ b/crates/supervisor/storage/src/providers/derivation_provider.rs @@ -189,7 +189,7 @@ where target: "supervisor_storage", "No blocks found in storage" ); - StorageError::EntryNotFound("no blocks found".to_string()) + StorageError::DatabaseNotInitialised })?; let latest_source_block = self.latest_source_block().inspect_err(|err| { @@ -250,11 +250,12 @@ where let latest_derivation_state = match self.latest_derivation_state() { Ok(pair) => pair, Err(StorageError::EntryNotFound(_)) => { - // in this case the database is not initialised, so we need to save the source block first + // in this case the database is not initialised, so we need to save the source block + // first self.save_source_block_internal(incoming_pair.source)?; self.save_derived_block_pair_internal(incoming_pair)?; return Ok(()); - }, + } Err(e) => return Err(e), }; @@ -742,6 +743,21 @@ mod tests { assert_eq!(latest, pair2); } + #[test] + fn test_latest_derivation_state_enpty_storage() { + let db = setup_db(); + + let tx = db.tx().expect("Could not get tx"); + let provider = DerivationProvider::new(&tx); + + let result = provider.latest_derivation_state(); + print!("{:?}", result); + assert!( + matches!(result, Err(StorageError::DatabaseNotInitialised)), + "Should return DatabaseNotInitialised error when no derivation state exists" + ); + } + #[test] fn test_latest_derivation_state_empty_source() { let db = setup_db(); diff --git a/crates/supervisor/storage/src/providers/log_provider.rs b/crates/supervisor/storage/src/providers/log_provider.rs index 3e128153fa..c775af6e37 100644 --- a/crates/supervisor/storage/src/providers/log_provider.rs +++ b/crates/supervisor/storage/src/providers/log_provider.rs @@ -56,7 +56,7 @@ where // If no blocks are found, this is the first block being stored. self.store_block_logs_internal(block, logs)?; return Ok(()); - }, + } Err(e) => return Err(e), }; @@ -138,7 +138,7 @@ where let (_, block) = result.ok_or_else(|| { warn!(target: "supervisor_storage", "No blocks found in storage"); - StorageError::EntryNotFound("no blocks found".to_string()) + StorageError::DatabaseNotInitialised })?; Ok(block.into()) } @@ -344,6 +344,17 @@ mod tests { assert!(matches!(result, Err(StorageError::InvalidAnchor))); } + #[test] + fn test_get_latest_block_empty() { + let db = setup_db(); + + let tx = db.tx().expect("Failed to start RO tx"); + let log_reader = LogProvider::new(&tx); + + let result = log_reader.get_latest_block(); + assert!(matches!(result, Err(StorageError::DatabaseNotInitialised))); + } + #[test] fn test_storage_read_write_success() { let db = setup_db(); From 743d186e71f73835adccf9e5b47ab495eb269b72 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Tue, 8 Jul 2025 16:28:10 +0530 Subject: [PATCH 10/27] resetter refactored --- .../supervisor/core/src/syncnode/resetter.rs | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/crates/supervisor/core/src/syncnode/resetter.rs b/crates/supervisor/core/src/syncnode/resetter.rs index 13f330b7fb..877022426c 100644 --- a/crates/supervisor/core/src/syncnode/resetter.rs +++ b/crates/supervisor/core/src/syncnode/resetter.rs @@ -1,5 +1,5 @@ use super::{ManagedNodeClient, ManagedNodeError}; -use kona_supervisor_storage::HeadRefStorageReader; +use kona_supervisor_storage::{HeadRefStorageReader, StorageError}; use kona_supervisor_types::SuperHead; use std::sync::Arc; use tokio::sync::Mutex; @@ -28,9 +28,30 @@ where info!(target: "resetter", "Resetting the node"); - let super_head = self.db_provider.get_super_head().inspect_err(|err| { - error!(target: "resetter", %err, "Failed to get super head"); + match self.db_provider.get_super_head() { + Ok(super_head) => { + info!(target: "resetter", "Super head fetched successfully: {:?}", super_head); + self.reset_post_interop(super_head).await + } + Err(StorageError::DatabaseNotInitialised) => self.reset_pre_interop().await, + Err(err) => { + error!(target: "resetter", %err, "Failed to get super head from storage"); + return Err(ManagedNodeError::StorageError(err)); + } + } + } + + async fn reset_pre_interop(&self) -> Result<(), ManagedNodeError> { + info!(target: "resetter", "Resetting the node to pre-interop state"); + + self.client.reset_pre_interop().await.inspect_err(|err| { + error!(target: "resetter", %err, "Failed to reset managed node to pre-interop state"); })?; + Ok(()) + } + + async fn reset_post_interop(&self, super_head: SuperHead) -> Result<(), ManagedNodeError> { + info!(target: "resetter", "Resetting the node to post-interop state"); let SuperHead { local_unsafe, cross_unsafe, local_safe, cross_safe, finalized, .. } = super_head; @@ -71,7 +92,6 @@ where .inspect_err(|err| { error!(target: "resetter", %err, "Failed to reset managed node"); })?; - Ok(()) } } From c593bd8f4605b148d96d3708352ad4fdb070d0dc Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Tue, 8 Jul 2025 17:32:20 +0530 Subject: [PATCH 11/27] refactor --- .../core/src/chain_processor/chain.rs | 10 +++ .../core/src/chain_processor/task.rs | 14 ++- .../supervisor/core/src/logindexer/indexer.rs | 4 + crates/supervisor/core/src/supervisor.rs | 8 +- crates/supervisor/storage/src/chaindb.rs | 71 ++++++++++----- crates/supervisor/storage/src/metrics.rs | 8 +- .../src/providers/derivation_provider.rs | 88 ++++++++++++------- .../storage/src/providers/log_provider.rs | 22 +++-- crates/supervisor/storage/src/traits.rs | 25 ++++++ 9 files changed, 181 insertions(+), 69 deletions(-) diff --git a/crates/supervisor/core/src/chain_processor/chain.rs b/crates/supervisor/core/src/chain_processor/chain.rs index 2a498b75ea..5d8aff89b5 100644 --- a/crates/supervisor/core/src/chain_processor/chain.rs +++ b/crates/supervisor/core/src/chain_processor/chain.rs @@ -231,6 +231,11 @@ mod tests { pub Db {} impl LogStorageWriter for Db { + fn initialise_log_storage( + &self, + block: BlockInfo, + ) -> Result<(), StorageError>; + fn store_block_logs( &self, block: &BlockInfo, @@ -239,6 +244,11 @@ mod tests { } impl DerivationStorageWriter for Db { + fn initialise_derivation_storage( + &self, + incoming_pair: DerivedRefPair, + ) -> Result<(), StorageError>; + fn save_derived_block( &self, incoming_pair: DerivedRefPair, diff --git a/crates/supervisor/core/src/chain_processor/task.rs b/crates/supervisor/core/src/chain_processor/task.rs index bb63f28184..815ca5a8e0 100644 --- a/crates/supervisor/core/src/chain_processor/task.rs +++ b/crates/supervisor/core/src/chain_processor/task.rs @@ -18,7 +18,7 @@ use tracing::{debug, error, info}; /// It listens for events emitted by the managed node and handles them accordingly. #[derive(Debug)] pub struct ChainProcessorTask { - rollup_config: RollupConfig, + _rollup_config: RollupConfig, chain_id: ChainId, metrics_enabled: Option, @@ -50,7 +50,7 @@ where ) -> Self { let log_indexer = LogIndexer::new(managed_node.clone(), state_manager.clone()); Self { - rollup_config, + _rollup_config: rollup_config, chain_id, metrics_enabled: None, cancel_token, @@ -488,6 +488,11 @@ mod tests { pub Db {} impl LogStorageWriter for Db { + fn initialise_log_storage( + &self, + block: BlockInfo, + ) -> Result<(), StorageError>; + fn store_block_logs( &self, block: &BlockInfo, @@ -496,6 +501,11 @@ mod tests { } impl DerivationStorageWriter for Db { + fn initialise_derivation_storage( + &self, + incoming_pair: DerivedRefPair, + ) -> Result<(), StorageError>; + fn save_derived_block( &self, incoming_pair: DerivedRefPair, diff --git a/crates/supervisor/core/src/logindexer/indexer.rs b/crates/supervisor/core/src/logindexer/indexer.rs index 0d2c2f31db..b0a825ff33 100644 --- a/crates/supervisor/core/src/logindexer/indexer.rs +++ b/crates/supervisor/core/src/logindexer/indexer.rs @@ -139,6 +139,10 @@ mod tests { } impl LogStorageWriter for MockLogStorage { + fn initialise_log_storage(&self, _block: BlockInfo) -> Result<(), StorageError> { + Ok(()) + } + fn store_block_logs(&self, block: &BlockInfo, logs: Vec) -> Result<(), StorageError> { self.blocks.lock().unwrap().push(*block); self.logs.lock().unwrap().extend(logs); diff --git a/crates/supervisor/core/src/supervisor.rs b/crates/supervisor/core/src/supervisor.rs index ab13e9552a..038683573c 100644 --- a/crates/supervisor/core/src/supervisor.rs +++ b/crates/supervisor/core/src/supervisor.rs @@ -11,8 +11,8 @@ use kona_interop::{ }; use kona_protocol::BlockInfo; use kona_supervisor_storage::{ - ChainDb, ChainDbFactory, DerivationStorageReader, FinalizedL1Storage, HeadRefStorageReader, - LogStorageReader, + ChainDb, ChainDbFactory, DerivationStorageReader, DerivationStorageWriter, FinalizedL1Storage, + HeadRefStorageReader, LogStorageReader, LogStorageWriter, }; use kona_supervisor_types::{SuperHead, parse_access_list}; use op_alloy_rpc_types::SuperchainDAError; @@ -142,7 +142,9 @@ impl Supervisor { for (chain_id, config) in self.config.rollup_config_set.rollups.iter() { // Initialise the database for each chain. let db = self.database_factory.get_or_create_db(*chain_id)?; - db.initialise(config.genesis.get_anchor())?; + let anchor = config.genesis.get_anchor(); + db.initialise_log_storage(anchor.derived)?; + db.initialise_derivation_storage(anchor)?; info!(target: "supervisor_service", chain_id, "Database initialized successfully"); } Ok(()) diff --git a/crates/supervisor/storage/src/chaindb.rs b/crates/supervisor/storage/src/chaindb.rs index ced0bc5956..211f75fbe2 100644 --- a/crates/supervisor/storage/src/chaindb.rs +++ b/crates/supervisor/storage/src/chaindb.rs @@ -67,21 +67,6 @@ impl ChainDb { f() } } - - /// initialises the database with a given anchor derived block pair. - pub fn initialise(&self, anchor: DerivedRefPair) -> Result<(), StorageError> { - self.env.update(|tx| { - DerivationProvider::new(tx).save_derived_block_pair(anchor)?; - LogProvider::new(tx).store_block_logs(&anchor.derived, Vec::new())?; - - let sp = SafetyHeadRefProvider::new(tx); - // todo: cross check if we can consider following safety head ref update - sp.update_safety_head_ref(SafetyLevel::LocalUnsafe, &anchor.derived)?; - sp.update_safety_head_ref(SafetyLevel::CrossUnsafe, &anchor.derived)?; - sp.update_safety_head_ref(SafetyLevel::LocalSafe, &anchor.derived)?; - sp.update_safety_head_ref(SafetyLevel::CrossSafe, &anchor.derived) - })? - } } // todo: make sure all get method return DatabaseNotInitialised error if db is not initialised @@ -104,15 +89,30 @@ impl DerivationStorageReader for ChainDb { } fn latest_derivation_state(&self) -> Result { - self.observe_call("latest_derived_block_pair", || { + self.observe_call("latest_derivation_state", || { self.env.view(|tx| DerivationProvider::new(tx).latest_derivation_state()) })? } } impl DerivationStorageWriter for ChainDb { + fn initialise_derivation_storage( + &self, + incoming_pair: DerivedRefPair, + ) -> Result<(), StorageError> { + self.observe_call("initialise_derivation_storage", || { + self.env.update(|ctx| { + DerivationProvider::new(ctx).initialise(incoming_pair)?; + SafetyHeadRefProvider::new(ctx) + .update_safety_head_ref(SafetyLevel::LocalSafe, &incoming_pair.derived)?; + SafetyHeadRefProvider::new(ctx) + .update_safety_head_ref(SafetyLevel::CrossSafe, &incoming_pair.derived) + }) + })? + } + fn save_derived_block(&self, incoming_pair: DerivedRefPair) -> Result<(), StorageError> { - self.observe_call("save_derived_block_pair", || { + self.observe_call("save_derived_block", || { self.env.update(|ctx| { let derived_block = incoming_pair.derived; let block = LogProvider::new(ctx).get_block(derived_block.number).map_err( @@ -137,7 +137,7 @@ impl DerivationStorageWriter for ChainDb { } fn save_source_block(&self, incoming_source: BlockInfo) -> Result<(), StorageError> { - self.observe_call("save_block_traversal", || { + self.observe_call("save_source_block", || { self.env.update(|ctx| DerivationProvider::new(ctx).save_source_block(incoming_source)) })? } @@ -171,6 +171,18 @@ impl LogStorageReader for ChainDb { } impl LogStorageWriter for ChainDb { + fn initialise_log_storage(&self, block: BlockInfo) -> Result<(), StorageError> { + self.observe_call("initialise_log_storage", || { + self.env.update(|ctx| { + LogProvider::new(ctx).initialise(block)?; + SafetyHeadRefProvider::new(ctx) + .update_safety_head_ref(SafetyLevel::LocalUnsafe, &block)?; + SafetyHeadRefProvider::new(ctx) + .update_safety_head_ref(SafetyLevel::CrossUnsafe, &block) + }) + })? + } + fn store_block_logs(&self, block: &BlockInfo, logs: Vec) -> Result<(), StorageError> { self.observe_call("store_block_logs", || { self.env.update(|ctx| { @@ -421,7 +433,8 @@ mod tests { }, }; - db.initialise(anchor).expect("initialise db"); + db.initialise_log_storage(anchor.derived).expect("initialise log storage"); + db.initialise_derivation_storage(anchor).expect("initialise derivation storage"); let block = BlockInfo { hash: B256::from([4u8; 32]), @@ -519,7 +532,8 @@ mod tests { }; // Initialise the database with the anchor derived block pair - db.initialise(anchor).expect("initialise db with anchor"); + db.initialise_log_storage(anchor.derived).expect("initialise log storage"); + db.initialise_derivation_storage(anchor).expect("initialise derivation storage"); // Save derived block pair - should error conflict let err = db.save_derived_block(derived_pair).unwrap_err(); @@ -585,7 +599,9 @@ mod tests { timestamp: 1, }; - db.initialise(DerivedRefPair { source, derived: block1 }).unwrap(); + db.initialise_log_storage(block1).expect("initialise log storage"); + db.initialise_derivation_storage(DerivedRefPair { source, derived: block1 }) + .expect("initialise derivation storage"); // should error as block2 must be child of block1 let err = db.update_current_cross_unsafe(&block2).expect_err("should return an error"); @@ -625,7 +641,9 @@ mod tests { timestamp: 1, }; - db.initialise(DerivedRefPair { source, derived: block1 }).unwrap(); + db.initialise_log_storage(block1).expect("initialise log storage"); + db.initialise_derivation_storage(DerivedRefPair { source, derived: block1 }) + .expect("initialise derivation storage"); // should error as block2 must be child of block1 let err = db.update_current_cross_safe(&block2).expect_err("should return an error"); @@ -674,7 +692,10 @@ mod tests { timestamp: 9101, }; - assert!(db.initialise(DerivedRefPair { source: source1, derived: derived1 }).is_ok()); + db.initialise_log_storage(derived1).expect("initialise log storage"); + db.initialise_derivation_storage(DerivedRefPair { source: source1, derived: derived1 }) + .expect("initialise derivation storage"); + assert!(db.save_source_block(source2).is_ok()); // Retrieve latest source block @@ -702,7 +723,9 @@ mod tests { timestamp: 1234, }, }; - assert!(db.initialise(anchor).is_ok()); + + db.initialise_log_storage(anchor.derived).expect("initialise log storage"); + db.initialise_derivation_storage(anchor).expect("initialise derivation storage"); let source1 = BlockInfo { hash: B256::from([2u8; 32]), diff --git a/crates/supervisor/storage/src/metrics.rs b/crates/supervisor/storage/src/metrics.rs index f2f4d5c7fc..edcdc39db8 100644 --- a/crates/supervisor/storage/src/metrics.rs +++ b/crates/supervisor/storage/src/metrics.rs @@ -14,11 +14,13 @@ impl Metrics { "kona_supervisor_storage_duration_seconds"; // List all your ChainDb method names here - const METHODS: [&'static str; 18] = [ + const METHODS: [&'static str; 20] = [ "derived_to_source", "latest_derived_block_at_source", - "latest_derived_block_pair", - "save_derived_block_pair", + "latest_derivation_state", + "initialise_derivation_storage", + "save_derived_block", + "save_source_block", "get_latest_block", "get_block", "get_log", diff --git a/crates/supervisor/storage/src/providers/derivation_provider.rs b/crates/supervisor/storage/src/providers/derivation_provider.rs index 4c17cd87d3..1cab540682 100644 --- a/crates/supervisor/storage/src/providers/derivation_provider.rs +++ b/crates/supervisor/storage/src/providers/derivation_provider.rs @@ -239,6 +239,26 @@ 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> { + match self.get_derived_block_pair_by_number(0) { + Ok(pair) + if pair.derived.hash == anchor.derived.hash && + pair.source.hash == anchor.source.hash => + { + // Anchor matches, nothing to do + Ok(()) + } + Ok(_) => Err(StorageError::InvalidAnchor), + Err(StorageError::EntryNotFound(_)) => { + self.save_source_block_internal(anchor.source)?; + self.save_derived_block_pair_internal(anchor)?; + Ok(()) + } + Err(err) => Err(err), + } + } + /// Saves a [`StoredDerivedBlockPair`] to [`DerivedBlocks`](`crate::models::DerivedBlocks`) /// table and [`SourceBlockTraversal`] to [`BlockTraversal`](`crate::models::BlockTraversal`) /// table in the database. @@ -249,13 +269,7 @@ where // todo: use cursor to get the last block(performance improvement) let latest_derivation_state = match self.latest_derivation_state() { Ok(pair) => pair, - Err(StorageError::EntryNotFound(_)) => { - // in this case the database is not initialised, so we need to save the source block - // first - self.save_source_block_internal(incoming_pair.source)?; - self.save_derived_block_pair_internal(incoming_pair)?; - return Ok(()); - } + Err(StorageError::EntryNotFound(_)) => return Err(StorageError::DatabaseNotInitialised), Err(e) => return Err(e), }; @@ -485,6 +499,17 @@ mod tests { .expect("Failed to init database") } + /// Helper to initialize database in a new transaction, committing if successful. + fn initialize_db(db: &DatabaseEnv, pair: &DerivedRefPair) -> Result<(), StorageError> { + let tx = db.tx_mut().expect("Could not get mutable tx"); + let provider = DerivationProvider::new(&tx); + let res = provider.initialise(*pair); + if res.is_ok() { + tx.commit().expect("Failed to commit transaction"); + } + res + } + /// Helper to insert a pair in a new transaction, committing if successful. fn insert_pair(db: &DatabaseEnv, pair: &DerivedRefPair) -> Result<(), StorageError> { let tx = db.tx_mut().expect("Could not get mutable tx"); @@ -516,7 +541,7 @@ mod tests { let anchor = derived_pair(source, derived); // Should succeed and insert the anchor - assert!(insert_pair(&db, &anchor).is_ok()); + assert!(initialize_db(&db, &anchor).is_ok()); // Check that the anchor is present let tx = db.tx().expect("Could not get tx"); @@ -534,7 +559,7 @@ mod tests { let anchor = derived_pair(source, genesis_block()); // First initialise - assert!(insert_pair(&db, &anchor).is_ok()); + assert!(initialize_db(&db, &anchor).is_ok()); // Second initialise with the same anchor should succeed (idempotent) assert!(insert_pair(&db, &anchor).is_ok()); } @@ -547,7 +572,7 @@ mod tests { let anchor = derived_pair(source, genesis_block()); // Insert the genesis - assert!(insert_pair(&db, &anchor).is_ok()); + assert!(initialize_db(&db, &anchor).is_ok()); // Try to initialise with a different anchor (different hash) let wrong_derived = block_info(0, B256::from([42u8; 32]), 200); @@ -564,7 +589,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); let derived2 = block_info(2, derived1.hash, 300); let pair2 = derived_pair(source1, derived2); @@ -584,7 +609,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); let wrong_parent_hash = B256::from([99u8; 32]); let derived2 = block_info(2, wrong_parent_hash, 300); @@ -600,7 +625,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); let derived2 = block_info(4, derived1.hash, 400); // should be 2, not 4 let pair2 = derived_pair(source1, derived2); @@ -615,7 +640,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); // Try to insert the same derived block again let result = insert_pair(&db, &pair1); @@ -629,7 +654,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); let derived2 = block_info(2, derived1.hash, 300); let pair2 = derived_pair(source1, derived2); @@ -647,7 +672,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); let derived2 = block_info(2, derived1.hash, 300); let pair2 = derived_pair(source1, derived2); @@ -667,7 +692,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); let derived2 = block_info(2, derived1.hash, 300); let pair2 = derived_pair(source1, derived2); @@ -712,7 +737,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); // Use correct number but wrong hash let tx = db.tx().expect("Could not get tx"); @@ -730,7 +755,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); let derived2 = block_info(2, derived1.hash, 300); let pair2 = derived_pair(source1, derived2); @@ -765,7 +790,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); let source2 = block_info(101, source1.hash, 300); let derived2 = block_info(2, derived1.hash, 300); @@ -791,7 +816,10 @@ mod tests { let tx = db.tx().expect("Could not get tx"); let provider = DerivationProvider::new(&tx); - assert!(matches!(provider.latest_derivation_state(), Err(StorageError::EntryNotFound(_)))); + assert!(matches!( + provider.latest_derivation_state(), + Err(StorageError::DatabaseNotInitialised) + )); } #[test] @@ -801,7 +829,7 @@ mod tests { let source1 = block_info(100, B256::from([100u8; 32]), 200); let derived1 = block_info(1, genesis_block().hash, 200); let pair1 = derived_pair(source1, derived1); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); let tx = db.tx().expect("Could not get tx"); let provider = DerivationProvider::new(&tx); @@ -829,7 +857,7 @@ mod tests { let derived0 = block_info(10, B256::from([10u8; 32]), 200); let pair1 = derived_pair(genesis_block(), derived0); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); let source1 = block_info(1, genesis_block().hash, 200); assert!(insert_source_block(&db, &source1).is_ok()); @@ -841,7 +869,7 @@ mod tests { let derived0 = block_info(10, B256::from([10u8; 32]), 200); let pair1 = derived_pair(genesis_block(), derived0); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); let source1 = block_info(1, genesis_block().hash, 200); assert!(insert_source_block(&db, &source1).is_ok()); @@ -856,7 +884,7 @@ mod tests { let source0 = block_info(10, B256::from([10u8; 32]), 200); let derived0 = genesis_block(); let pair1 = derived_pair(source0, derived0); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); let source1 = block_info(11, B256::from([1u8; 32]), 200); let result = insert_source_block(&db, &source1); @@ -873,7 +901,7 @@ mod tests { let source0 = block_info(10, B256::from([10u8; 32]), 200); let derived0 = genesis_block(); let pair1 = derived_pair(source0, derived0); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); let source1 = block_info(11, source0.hash, 400); assert!(insert_source_block(&db, &source1).is_ok()); @@ -890,7 +918,7 @@ mod tests { let source0 = block_info(10, B256::from([10u8; 32]), 200); let derived0 = genesis_block(); let pair1 = derived_pair(source0, derived0); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); let source1 = block_info(11, source0.hash, 400); assert!(insert_source_block(&db, &source1).is_ok()); @@ -907,7 +935,7 @@ mod tests { let derived0 = block_info(10, B256::from([10u8; 32]), 200); let pair1 = derived_pair(genesis_block(), derived0); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); let source1 = block_info(2, genesis_block().hash, 400); // Try to skip a block @@ -921,7 +949,7 @@ mod tests { let derived0 = block_info(10, B256::from([10u8; 32]), 200); let pair1 = derived_pair(genesis_block(), derived0); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); let source1 = block_info(1, genesis_block().hash, 200); let source2 = block_info(2, source1.hash, 400); @@ -935,7 +963,7 @@ mod tests { let derived0 = block_info(10, B256::from([10u8; 32]), 200); let pair1 = derived_pair(genesis_block(), derived0); - assert!(insert_pair(&db, &pair1).is_ok()); + assert!(initialize_db(&db, &pair1).is_ok()); let source1 = block_info(1, genesis_block().hash, 200); assert!(insert_source_block(&db, &source1).is_ok()); diff --git a/crates/supervisor/storage/src/providers/log_provider.rs b/crates/supervisor/storage/src/providers/log_provider.rs index c775af6e37..1fd07bba61 100644 --- a/crates/supervisor/storage/src/providers/log_provider.rs +++ b/crates/supervisor/storage/src/providers/log_provider.rs @@ -43,6 +43,18 @@ impl LogProvider<'_, TX> where TX: DbTxMut + DbTx, { + pub(crate) fn initialise(&self, anchor: BlockInfo) -> Result<(), StorageError> { + match self.get_block(0) { + Ok(block) if block.hash == anchor.hash => Ok(()), + Ok(_) => Err(StorageError::InvalidAnchor), + Err(StorageError::EntryNotFound(_)) => { + self.store_block_logs_internal(&anchor, Vec::new()) + } + + Err(err) => Err(err), + } + } + pub(crate) fn store_block_logs( &self, block: &BlockInfo, @@ -52,11 +64,7 @@ where let latest_block = match self.get_latest_block() { Ok(block) => block, - Err(StorageError::EntryNotFound(_)) => { - // If no blocks are found, this is the first block being stored. - self.store_block_logs_internal(block, logs)?; - return Ok(()); - } + Err(StorageError::EntryNotFound(_)) => return Err(StorageError::DatabaseNotInitialised), Err(e) => return Err(e), }; @@ -277,7 +285,7 @@ mod tests { fn initialize_db(db: &DatabaseEnv, block: &BlockInfo) -> Result<(), StorageError> { let tx = db.tx_mut().expect("Could not get mutable tx"); let provider = LogProvider::new(&tx); - let res = provider.store_block_logs(block, Vec::new()); + let res = provider.initialise(*block); if res.is_ok() { tx.commit().expect("Failed to commit transaction"); } else { @@ -416,7 +424,7 @@ mod tests { let log_reader = LogProvider::new(&tx); let result = log_reader.get_latest_block(); - assert!(matches!(result, Err(StorageError::EntryNotFound(_)))); + assert!(matches!(result, Err(StorageError::DatabaseNotInitialised))); // Initialize with genesis block let genesis = genesis_block(); diff --git a/crates/supervisor/storage/src/traits.rs b/crates/supervisor/storage/src/traits.rs index 38f2adf4fe..106380f197 100644 --- a/crates/supervisor/storage/src/traits.rs +++ b/crates/supervisor/storage/src/traits.rs @@ -60,6 +60,20 @@ pub trait DerivationStorageReader: Debug { /// /// Implementations are expected to provide persistent and thread-safe access to block data. pub trait DerivationStorageWriter: Debug { + /// Initializes the derivation storage with a given [`DerivedRefPair`]. + /// This method is typically called once to set up the storage with the initial pair. + /// + /// # Arguments + /// * `incoming_pair` - The derived block pair to initialize the storage with. + /// + /// # Returns + /// * `Ok(())` if the storage was successfully initialized. + /// * `Err(StorageError)` if there is an issue initializing the storage. + fn initialise_derivation_storage( + &self, + incoming_pair: DerivedRefPair, + ) -> Result<(), StorageError>; + /// Saves a [`DerivedRefPair`] to the storage. /// /// This method is **append-only**: it does not overwrite existing pairs. @@ -155,6 +169,17 @@ pub trait LogStorageReader: Debug { /// /// Implementations are expected to provide persistent and thread-safe access to block logs. pub trait LogStorageWriter: Send + Sync + Debug { + /// Initializes the log storage with a given [`BlockInfo`]. + /// This method is typically called once to set up the storage with the initial block. + /// + /// # Arguments + /// * `block` - The [`BlockInfo`] to initialize the storage with. + /// + /// # Returns + /// * `Ok(())` if the storage was successfully initialized. + /// * `Err(StorageError)` if there is an issue initializing the storage. + fn initialise_log_storage(&self, block: BlockInfo) -> Result<(), StorageError>; + /// Stores [`BlockInfo`] and [`Log`]s in the storage. /// This method is append-only and does not overwrite existing logs. /// Ensures that the latest stored block is the parent of the incoming block before saving. From 84839820aa30662e3cc206a8d13886e5092cdea7 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Tue, 8 Jul 2025 22:02:22 +0530 Subject: [PATCH 12/27] pre-interop support added --- .../core/src/chain_processor/task.rs | 80 +++++++++++++------ .../core/src/config/rollup_config_set.rs | 34 +++++++- crates/supervisor/core/src/supervisor.rs | 10 ++- 3 files changed, 92 insertions(+), 32 deletions(-) diff --git a/crates/supervisor/core/src/chain_processor/task.rs b/crates/supervisor/core/src/chain_processor/task.rs index 815ca5a8e0..d22cb8e8bb 100644 --- a/crates/supervisor/core/src/chain_processor/task.rs +++ b/crates/supervisor/core/src/chain_processor/task.rs @@ -18,7 +18,7 @@ use tracing::{debug, error, info}; /// It listens for events emitted by the managed node and handles them accordingly. #[derive(Debug)] pub struct ChainProcessorTask { - _rollup_config: RollupConfig, + rollup_config: RollupConfig, chain_id: ChainId, metrics_enabled: Option, @@ -50,7 +50,7 @@ where ) -> Self { let log_indexer = LogIndexer::new(managed_node.clone(), state_manager.clone()); Self { - _rollup_config: rollup_config, + rollup_config, chain_id, metrics_enabled: None, cancel_token, @@ -322,37 +322,52 @@ where block_number = derived_ref_pair.derived.number, "Processing local safe derived block pair" ); - match self.state_manager.save_derived_block(derived_ref_pair) { - Ok(_) => Ok(derived_ref_pair.derived), - Err(StorageError::BlockOutOfOrder) => { - error!( - target: "chain_processor", - chain_id = self.chain_id, - block_number = derived_ref_pair.derived.number, - "Block out of order detected, resetting managed node" - ); - if let Err(err) = self.managed_node.reset().await { + if self.rollup_config.is_interop_activation_block(derived_ref_pair.derived) { + info!( + target: "chain_processor", + chain_id = self.chain_id, + block_number = derived_ref_pair.derived.number, + "Initialising derivation storage for interop activation block" + ); + self.state_manager.initialise_derivation_storage(derived_ref_pair)?; + return Ok(derived_ref_pair.derived); + } + + if self.rollup_config.is_post_interop(derived_ref_pair.derived.timestamp) { + match self.state_manager.save_derived_block(derived_ref_pair) { + Ok(_) => return Ok(derived_ref_pair.derived), + Err(StorageError::BlockOutOfOrder) => { + error!( + target: "chain_processor", + chain_id = self.chain_id, + block_number = derived_ref_pair.derived.number, + "Block out of order detected, resetting managed node" + ); + + if let Err(err) = self.managed_node.reset().await { + error!( + target: "chain_processor", + chain_id = self.chain_id, + %err, + "Failed to reset managed node after block out of order" + ); + } + return Err(StorageError::BlockOutOfOrder.into()); + } + Err(err) => { error!( target: "chain_processor", chain_id = self.chain_id, + block_number = derived_ref_pair.derived.number, %err, - "Failed to reset managed node after block out of order" + "Failed to save derived block pair" ); + return Err(err.into()); } - Err(StorageError::BlockOutOfOrder.into()) - } - Err(err) => { - error!( - target: "chain_processor", - chain_id = self.chain_id, - block_number = derived_ref_pair.derived.number, - %err, - "Failed to save derived block pair" - ); - Err(err.into()) } } + Ok(derived_ref_pair.derived) } async fn handle_unsafe_event( @@ -366,7 +381,22 @@ where "Processing unsafe block" ); - self.log_indexer.process_and_store_logs(&block).await?; + if self.rollup_config.is_interop_activation_block(block) { + info!( + target: "chain_processor", + chain_id = self.chain_id, + block_number = block.number, + "Initialising log storage for interop activation block" + ); + self.state_manager.initialise_log_storage(block)?; + return Ok(block); + } + + if self.rollup_config.is_post_interop(block.timestamp) { + self.log_indexer.process_and_store_logs(&block).await?; + return Ok(block); + } + Ok(block) } diff --git a/crates/supervisor/core/src/config/rollup_config_set.rs b/crates/supervisor/core/src/config/rollup_config_set.rs index 3ce6437484..c06594430d 100644 --- a/crates/supervisor/core/src/config/rollup_config_set.rs +++ b/crates/supervisor/core/src/config/rollup_config_set.rs @@ -29,8 +29,8 @@ impl Genesis { } } - /// Returns the genesis anchor as a [`DerivedRefPair`]. - pub const fn get_anchor(&self) -> DerivedRefPair { + /// Returns the genesis as a [`DerivedRefPair`]. + pub const fn get_derived_pair(&self) -> DerivedRefPair { DerivedRefPair { derived: self.l2, source: self.l1 } } } @@ -73,15 +73,36 @@ impl RollupConfig { }) } + /// Returns `true` if the timestamp is at or after the interop activation time. + /// + /// Interop activates at [`interop_time`](Self::interop_time). This function checks whether the + /// provided timestamp is or after interop time + /// + /// Returns `false` if `interop_time` is not configured or is 0. + fn is_interop(&self, timestamp: u64) -> bool { + self.interop_time.is_some_and(|t| t != 0 && timestamp >= t) + } + /// Returns `true` if the timestamp is strictly after the interop activation block. /// /// Interop activates at [`interop_time`](Self::interop_time). This function checks whether the - /// current block timestamp is *after* that activation, skipping the activation block + /// provided timestamp is *after* that activation, skipping the activation block /// itself. /// /// Returns `false` if `interop_time` is not configured. pub fn is_post_interop(&self, timestamp: u64) -> bool { - self.interop_time.is_some_and(|t| timestamp.saturating_sub(self.block_time) >= t) + self.is_interop(timestamp.saturating_sub(self.block_time)) + } + + /// Returns `true` if the timestamp is of an interop activation block. + /// + /// An interop activation block is defined as the block that is right after the + /// interop activation time. + /// + /// Returns `false` if `interop_time` is not configured. + pub fn is_interop_activation_block(&self, block: BlockInfo) -> bool { + self.is_interop(block.timestamp) && + !self.is_interop(block.timestamp.saturating_sub(self.block_time)) } } @@ -119,6 +140,11 @@ impl RollupConfigSet { pub fn is_interop_enabled(&self, chain_id: ChainId, timestamp: u64) -> bool { self.get(chain_id).map(|cfg| cfg.is_post_interop(timestamp)).unwrap_or(false) // if config not found, return false } + + /// returns whether the given block is an interop activation block for the specified chain. + pub fn is_interop_activation_block(&self, chain_id: ChainId, block: BlockInfo) -> bool { + self.get(chain_id).map(|cfg| cfg.is_interop_activation_block(block)).unwrap_or(false) + } } #[cfg(test)] diff --git a/crates/supervisor/core/src/supervisor.rs b/crates/supervisor/core/src/supervisor.rs index 038683573c..5d19105dfb 100644 --- a/crates/supervisor/core/src/supervisor.rs +++ b/crates/supervisor/core/src/supervisor.rs @@ -142,9 +142,13 @@ impl Supervisor { for (chain_id, config) in self.config.rollup_config_set.rollups.iter() { // Initialise the database for each chain. let db = self.database_factory.get_or_create_db(*chain_id)?; - let anchor = config.genesis.get_anchor(); - db.initialise_log_storage(anchor.derived)?; - db.initialise_derivation_storage(anchor)?; + let interop_time = config.interop_time; + let derived_pair = config.genesis.get_derived_pair(); + if config.is_interop_activation_block(derived_pair.derived) { + info!(target: "supervisor_service", chain_id, interop_time, %derived_pair, "Initialising database for interop activation block"); + db.initialise_log_storage(derived_pair.derived)?; + db.initialise_derivation_storage(derived_pair)?; + } info!(target: "supervisor_service", chain_id, "Database initialized successfully"); } Ok(()) From 7851f48447b4a765ae65b81fd05bee253a0eea31 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Wed, 9 Jul 2025 09:50:52 +0530 Subject: [PATCH 13/27] interop offset updated --- tests/devnets/simple-supervisor.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/devnets/simple-supervisor.yaml b/tests/devnets/simple-supervisor.yaml index 5d265146ba..e5a0721b75 100644 --- a/tests/devnets/simple-supervisor.yaml +++ b/tests/devnets/simple-supervisor.yaml @@ -9,12 +9,12 @@ optimism_package: type: op-geth cl: type: op-node - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/op-node:v1.13.3 + image: op-node:local log_level: debug network_params: network: "kurtosis" network_id: "2151908" - interop_time_offset: 0 + interop_time_offset: 1 holocene_time_offset: 0 isthmus_time_offset: 0 fjord_time_offset: 0 @@ -32,12 +32,12 @@ optimism_package: type: op-geth cl: type: op-node - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/op-node:v1.13.3 + image: op-node:local log_level: debug network_params: network: "kurtosis" network_id: "2151909" - interop_time_offset: 0 + interop_time_offset: 1 holocene_time_offset: 0 isthmus_time_offset: 0 fjord_time_offset: 0 From dfb77ffcd4c57ed4c8526d9d86a00c41dc768d56 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Wed, 9 Jul 2025 14:49:52 +0530 Subject: [PATCH 14/27] fix metrics --- crates/supervisor/storage/src/chaindb.rs | 47 +++++++++++++----------- crates/supervisor/storage/src/metrics.rs | 11 +++--- 2 files changed, 30 insertions(+), 28 deletions(-) diff --git a/crates/supervisor/storage/src/chaindb.rs b/crates/supervisor/storage/src/chaindb.rs index 211f75fbe2..2a7554b9dc 100644 --- a/crates/supervisor/storage/src/chaindb.rs +++ b/crates/supervisor/storage/src/chaindb.rs @@ -233,28 +233,31 @@ impl HeadRefStorageWriter for ChainDb { &self, finalized_source_block: BlockInfo, ) -> Result { - self.env.update(|tx| { - let sp = SafetyHeadRefProvider::new(tx); - let safe = sp.get_safety_head_ref(SafetyLevel::CrossSafe)?; - - let dp = DerivationProvider::new(tx); - let safe_block_pair = dp.get_derived_block_pair(safe.id())?; - - if finalized_source_block.number >= safe_block_pair.source.number { - // this could happen during initial sync - warn!( - target: "supervisor_storage", - l1_finalized_block_number = finalized_source_block.number, - safe_source_block_number = safe_block_pair.source.number, - "L1 finalized block is greater than safe block", - ); - sp.update_safety_head_ref(SafetyLevel::Finalized, &safe)?; - return Ok(safe); - } - - let latest_derived = dp.latest_derived_block_at_source(finalized_source_block.id())?; - sp.update_safety_head_ref(SafetyLevel::Finalized, &latest_derived)?; - Ok(latest_derived) + self.observe_call("update_finalized_using_source", || { + self.env.update(|tx| { + let sp = SafetyHeadRefProvider::new(tx); + let safe = sp.get_safety_head_ref(SafetyLevel::CrossSafe)?; + + let dp = DerivationProvider::new(tx); + let safe_block_pair = dp.get_derived_block_pair(safe.id())?; + + if finalized_source_block.number >= safe_block_pair.source.number { + // this could happen during initial sync + warn!( + target: "supervisor_storage", + l1_finalized_block_number = finalized_source_block.number, + safe_source_block_number = safe_block_pair.source.number, + "L1 finalized block is greater than safe block", + ); + sp.update_safety_head_ref(SafetyLevel::Finalized, &safe)?; + return Ok(safe); + } + + let latest_derived = + dp.latest_derived_block_at_source(finalized_source_block.id())?; + sp.update_safety_head_ref(SafetyLevel::Finalized, &latest_derived)?; + Ok(latest_derived) + }) })? } diff --git a/crates/supervisor/storage/src/metrics.rs b/crates/supervisor/storage/src/metrics.rs index edcdc39db8..06f7759f81 100644 --- a/crates/supervisor/storage/src/metrics.rs +++ b/crates/supervisor/storage/src/metrics.rs @@ -14,7 +14,7 @@ impl Metrics { "kona_supervisor_storage_duration_seconds"; // List all your ChainDb method names here - const METHODS: [&'static str; 20] = [ + const METHODS: [&'static str; 19] = [ "derived_to_source", "latest_derived_block_at_source", "latest_derivation_state", @@ -25,16 +25,15 @@ impl Metrics { "get_block", "get_log", "get_logs", + "initialise_log_storage", "store_block_logs", - "get_current_l1", "get_safety_head_ref", "get_super_head", - "update_current_l1", - "update_safety_head_ref", - "update_finalized_l1", - "get_finalized_l1", + "update_finalized_using_source", "update_current_cross_unsafe", "update_current_cross_safe", + "update_finalized_l1", + "get_finalized_l1", // Add more as needed ]; From de52623c7eeac8c875f29cdb9b434dc23576b21c Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Wed, 9 Jul 2025 15:05:55 +0530 Subject: [PATCH 15/27] log storage idempotancy --- .../src/providers/derivation_provider.rs | 2 +- .../storage/src/providers/log_provider.rs | 68 ++++++++++++++++++- 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/crates/supervisor/storage/src/providers/derivation_provider.rs b/crates/supervisor/storage/src/providers/derivation_provider.rs index 1cab540682..214f4cb70e 100644 --- a/crates/supervisor/storage/src/providers/derivation_provider.rs +++ b/crates/supervisor/storage/src/providers/derivation_provider.rs @@ -769,7 +769,7 @@ mod tests { } #[test] - fn test_latest_derivation_state_enpty_storage() { + fn test_latest_derivation_state_empty_storage() { let db = setup_db(); let tx = db.tx().expect("Could not get tx"); diff --git a/crates/supervisor/storage/src/providers/log_provider.rs b/crates/supervisor/storage/src/providers/log_provider.rs index 1fd07bba61..c58e6e8861 100644 --- a/crates/supervisor/storage/src/providers/log_provider.rs +++ b/crates/supervisor/storage/src/providers/log_provider.rs @@ -68,7 +68,23 @@ where Err(e) => return Err(e), }; - // todo: handle idempotency + if latest_block.number >= block.number { + // If the latest block is ahead of the incoming block, it means + // the incoming block is old block, check if it is same as the stored block. + let stored_block = self.get_block(block.number)?; + if stored_block == *block { + return Ok(()); + } + error!( + target: "supervisor_storage", + %stored_block, + incoming_block = %block, + "Incoming log block is not consistent with the stored log block", + ); + return Err(StorageError::ConflictError( + "incoming log block is not consistent with the stored log block".to_string(), + )) + } if !latest_block.is_parent_of(block) { warn!( @@ -466,4 +482,54 @@ mod tests { let result = insert_block_logs(&db, &block2, logs2); assert!(matches!(result, Err(StorageError::BlockOutOfOrder))); } + + #[test] + fn store_block_logs_skips_if_block_already_exists() { + let db = setup_db(); + let genesis = genesis_block(); + initialize_db(&db, &genesis).expect("Failed to initialize DB with genesis block"); + + let block1 = sample_block_info(1, genesis.hash); + let logs1 = vec![sample_log(0, false)]; + + // Store block1 for the first time + assert!(insert_block_logs(&db, &block1, logs1.clone()).is_ok()); + + // Try storing the same block again (should skip and succeed) + assert!(insert_block_logs(&db, &block1, logs1.clone()).is_ok()); + + // Try storing genesis block again (should skip and succeed) + assert!(insert_block_logs(&db, &genesis, Vec::new()).is_ok()); + + // Check that the logs are still present and correct + let tx = db.tx().expect("Failed to start RO tx"); + let log_reader = LogProvider::new(&tx); + let logs = log_reader.get_logs(block1.number).expect("Should get logs"); + assert_eq!(logs, logs1); + } + + #[test] + fn store_block_logs_returns_conflict_if_block_exists_with_different_data() { + let db = setup_db(); + let genesis = genesis_block(); + initialize_db(&db, &genesis).expect("Failed to initialize DB with genesis block"); + + let block1 = sample_block_info(1, genesis.hash); + let logs1 = vec![sample_log(0, false)]; + assert!(insert_block_logs(&db, &block1, logs1.clone()).is_ok()); + + // Try storing block1 again with a different hash (simulate conflict) + let mut block1_conflict = block1; + block1_conflict.hash = B256::from([0x22; 32]); + let logs1_conflict = vec![sample_log(0, false)]; + + let result = insert_block_logs(&db, &block1_conflict, logs1_conflict); + assert!(matches!(result, Err(StorageError::ConflictError(_)))); + + // Try storing genesis block again with a different hash (simulate conflict) + let mut genesis_conflict = genesis; + genesis_conflict.hash = B256::from([0x33; 32]); + let result = insert_block_logs(&db, &genesis_conflict, Vec::new()); + assert!(matches!(result, Err(StorageError::ConflictError(_)))); + } } From 5e46799ecc25ac70a26c15c396e046f63df07286 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Wed, 9 Jul 2025 15:06:10 +0530 Subject: [PATCH 16/27] lintfix --- crates/supervisor/storage/src/providers/log_provider.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/supervisor/storage/src/providers/log_provider.rs b/crates/supervisor/storage/src/providers/log_provider.rs index c58e6e8861..309ee750e9 100644 --- a/crates/supervisor/storage/src/providers/log_provider.rs +++ b/crates/supervisor/storage/src/providers/log_provider.rs @@ -497,7 +497,7 @@ mod tests { // Try storing the same block again (should skip and succeed) assert!(insert_block_logs(&db, &block1, logs1.clone()).is_ok()); - + // Try storing genesis block again (should skip and succeed) assert!(insert_block_logs(&db, &genesis, Vec::new()).is_ok()); @@ -525,7 +525,7 @@ mod tests { let result = insert_block_logs(&db, &block1_conflict, logs1_conflict); assert!(matches!(result, Err(StorageError::ConflictError(_)))); - + // Try storing genesis block again with a different hash (simulate conflict) let mut genesis_conflict = genesis; genesis_conflict.hash = B256::from([0x33; 32]); From 04fe5daa54c57f2f453169e5d651a1757cc06773 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Wed, 9 Jul 2025 15:14:09 +0530 Subject: [PATCH 17/27] linfix --- crates/supervisor/storage/src/providers/log_provider.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/supervisor/storage/src/providers/log_provider.rs b/crates/supervisor/storage/src/providers/log_provider.rs index 309ee750e9..92e81d51c1 100644 --- a/crates/supervisor/storage/src/providers/log_provider.rs +++ b/crates/supervisor/storage/src/providers/log_provider.rs @@ -516,7 +516,7 @@ mod tests { let block1 = sample_block_info(1, genesis.hash); let logs1 = vec![sample_log(0, false)]; - assert!(insert_block_logs(&db, &block1, logs1.clone()).is_ok()); + assert!(insert_block_logs(&db, &block1, logs1).is_ok()); // Try storing block1 again with a different hash (simulate conflict) let mut block1_conflict = block1; From 9af9d1a129ee6b132645acdbbf6aae4e03c55835 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Mon, 14 Jul 2025 15:22:14 +0530 Subject: [PATCH 18/27] minor improvements --- .../core/src/safety_checker/task.rs | 25 +++++++++++++++---- .../src/providers/head_ref_provider.rs | 1 - 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/crates/supervisor/core/src/safety_checker/task.rs b/crates/supervisor/core/src/safety_checker/task.rs index adae55f6e9..ec5e61b4c9 100644 --- a/crates/supervisor/core/src/safety_checker/task.rs +++ b/crates/supervisor/core/src/safety_checker/task.rs @@ -6,7 +6,7 @@ use crate::{ use alloy_primitives::ChainId; use derive_more::Constructor; use kona_protocol::BlockInfo; -use kona_supervisor_storage::CrossChainSafetyProvider; +use kona_supervisor_storage::{CrossChainSafetyProvider, StorageError}; use std::{sync::Arc, time::Duration}; use tokio::sync::mpsc; use tokio_util::sync::CancellationToken; @@ -114,10 +114,25 @@ where // Finds the next block that is eligible for promotion at the configured target level. fn find_next_promotable_block(&self) -> Result { - let current_head = - self.provider.get_safety_head_ref(self.chain_id, self.promoter.target_level())?; - let upper_head = - self.provider.get_safety_head_ref(self.chain_id, self.promoter.lower_bound_level())?; + let current_head = self.provider + .get_safety_head_ref(self.chain_id, self.promoter.target_level()) + .map_err(|err| { + if matches!(err, StorageError::EntryNotFound(_)) { + CrossSafetyError::NoBlockToPromote + } else { + err.into() + } + })?; + + let upper_head = self.provider + .get_safety_head_ref(self.chain_id, self.promoter.lower_bound_level()) + .map_err(|err| { + if matches!(err, StorageError::EntryNotFound(_)) { + CrossSafetyError::NoBlockToPromote + } else { + err.into() + } + })?; if current_head.number >= upper_head.number { return Err(CrossSafetyError::NoBlockToPromote); diff --git a/crates/supervisor/storage/src/providers/head_ref_provider.rs b/crates/supervisor/storage/src/providers/head_ref_provider.rs index 8e80dc7d3b..e7d2d4a2ae 100644 --- a/crates/supervisor/storage/src/providers/head_ref_provider.rs +++ b/crates/supervisor/storage/src/providers/head_ref_provider.rs @@ -34,7 +34,6 @@ where ); })?; let block_ref = result.ok_or_else(|| { - warn!(target: "supervisor_storage", %safety_level, "No head reference found"); StorageError::EntryNotFound("no head reference found".to_string()) })?; Ok(block_ref.into()) From 290cd7cf8dd343ba16047cca91f0ac7c674a7903 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Mon, 14 Jul 2025 16:17:28 +0530 Subject: [PATCH 19/27] test cases fixes --- crates/supervisor/core/src/syncnode/resetter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/supervisor/core/src/syncnode/resetter.rs b/crates/supervisor/core/src/syncnode/resetter.rs index fca8d02215..4c27c9b810 100644 --- a/crates/supervisor/core/src/syncnode/resetter.rs +++ b/crates/supervisor/core/src/syncnode/resetter.rs @@ -228,7 +228,7 @@ mod tests { #[tokio::test] async fn test_reset_db_error() { let mut db = MockDb::new(); - db.expect_get_super_head().returning(|| Err(StorageError::DatabaseNotInitialised)); + db.expect_get_super_head().returning(|| Err(StorageError::LockPoisoned)); let client = MockClient::new(); From 7897656c9fe8e6c53d39c67ee2250ad30718b518 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Mon, 14 Jul 2025 16:32:45 +0530 Subject: [PATCH 20/27] op-node image updated --- tests/devnets/simple-supervisor.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/devnets/simple-supervisor.yaml b/tests/devnets/simple-supervisor.yaml index e5a0721b75..4d408061bf 100644 --- a/tests/devnets/simple-supervisor.yaml +++ b/tests/devnets/simple-supervisor.yaml @@ -9,7 +9,7 @@ optimism_package: type: op-geth cl: type: op-node - image: op-node:local + image: us-docker.pkg.dev/oplabs-tools-artifacts/images/op-node:develop log_level: debug network_params: network: "kurtosis" @@ -32,7 +32,7 @@ optimism_package: type: op-geth cl: type: op-node - image: op-node:local + image: us-docker.pkg.dev/oplabs-tools-artifacts/images/op-node:develop log_level: debug network_params: network: "kurtosis" From 9742f09c95a188b83ae0a7a8759997832f133ea4 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Mon, 14 Jul 2025 18:02:25 +0530 Subject: [PATCH 21/27] added test cases --- .../core/src/chain_processor/task.rs | 201 +++++++++++++++++- 1 file changed, 196 insertions(+), 5 deletions(-) diff --git a/crates/supervisor/core/src/chain_processor/task.rs b/crates/supervisor/core/src/chain_processor/task.rs index a963a433e8..a82e9d4782 100644 --- a/crates/supervisor/core/src/chain_processor/task.rs +++ b/crates/supervisor/core/src/chain_processor/task.rs @@ -441,6 +441,7 @@ where mod tests { use super::*; use crate::{ + config::Genesis, event::ChainEvent, syncnode::{ BlockProvider, ManagedNodeController, ManagedNodeDataProvider, ManagedNodeError, @@ -577,13 +578,60 @@ mod tests { } ); + fn genesis() -> Genesis { + let l2 = BlockInfo::new(B256::from([1u8; 32]), 0, B256::ZERO, 50); + let l1 = BlockInfo::new(B256::from([2u8; 32]), 10, B256::ZERO, 1000); + Genesis::new(l1, l2) + } + + fn get_rollup_config(interop_time: u64) -> RollupConfig { + RollupConfig::new(genesis(), 2, Some(interop_time)) + } + #[tokio::test] - async fn test_handle_unsafe_event_triggers() { + async fn test_handle_unsafe_event_pre_interop() { + let mockdb = MockDb::new(); + let mocknode = MockNode::new(); + + // Send unsafe block event + let block = BlockInfo::new(B256::ZERO, 123, B256::ZERO, 10); + + let writer = Arc::new(mockdb); + let managed_node = Arc::new(mocknode); + + let cancel_token = CancellationToken::new(); + let (tx, rx) = mpsc::channel(10); + + let rollup_config = get_rollup_config(1000); + + let task = ChainProcessorTask::new( + rollup_config, + 1, + managed_node, + writer, + cancel_token.clone(), + rx, + ); + + tx.send(ChainEvent::UnsafeBlock { block }).await.unwrap(); + + let task_handle = tokio::spawn(task.run()); + + // Give it time to process + tokio::time::sleep(Duration::from_millis(50)).await; + + // Stop the task + cancel_token.cancel(); + task_handle.await.unwrap(); + } + + #[tokio::test] + async fn test_handle_unsafe_event_post_interop() { let mut mockdb = MockDb::new(); let mut mocknode = MockNode::new(); // Send unsafe block event - let block = BlockInfo::new(B256::ZERO, 123, B256::ZERO, 0); + let block = BlockInfo::new(B256::ZERO, 123, B256::ZERO, 1003); mockdb.expect_store_block_logs().returning(move |_block, _log| Ok(())); mocknode.expect_fetch_receipts().returning(move |block_hash| { @@ -597,7 +645,8 @@ mod tests { let cancel_token = CancellationToken::new(); let (tx, rx) = mpsc::channel(10); - let rollup_config = RollupConfig::default(); + let rollup_config = get_rollup_config(1000); + let task = ChainProcessorTask::new( rollup_config, 1, @@ -620,7 +669,45 @@ mod tests { } #[tokio::test] - async fn test_handle_derived_event_triggers() { + async fn test_handle_unsafe_event_interop_activation() { + let mut mockdb = MockDb::new(); + let mocknode = MockNode::new(); + + // Block that triggers interop activation + let block = BlockInfo::new(B256::ZERO, 123, B256::ZERO, 1001); // Use timestamp/number that triggers activation + + let rollup_config = get_rollup_config(1000); + + mockdb.expect_initialise_log_storage().returning(move |b| { + assert_eq!(b, block); + Ok(()) + }); + + let writer = Arc::new(mockdb); + let managed_node = Arc::new(mocknode); + + let cancel_token = CancellationToken::new(); + let (tx, rx) = mpsc::channel(10); + + let task = ChainProcessorTask::new( + rollup_config, + 1, + managed_node, + writer, + cancel_token.clone(), + rx, + ); + + tx.send(ChainEvent::UnsafeBlock { block }).await.unwrap(); + + let task_handle = tokio::spawn(task.run()); + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + cancel_token.cancel(); + task_handle.await.unwrap(); + } + + #[tokio::test] + async fn test_handle_derived_event_pre_interop() { let block_pair = DerivedRefPair { source: BlockInfo { number: 123, @@ -632,8 +719,57 @@ mod tests { number: 1234, hash: B256::ZERO, parent_hash: B256::ZERO, + timestamp: 999, + }, + }; + + let mockdb = MockDb::new(); + let mocknode = MockNode::new(); + + let writer = Arc::new(mockdb); + let managed_node = Arc::new(mocknode); + + let cancel_token = CancellationToken::new(); + let (tx, rx) = mpsc::channel(10); + + let rollup_config = get_rollup_config(1000); + let task = ChainProcessorTask::new( + rollup_config, + 1, + managed_node, + writer, + cancel_token.clone(), + rx, + ); + + // Send unsafe block event + tx.send(ChainEvent::DerivedBlock { derived_ref_pair: block_pair }).await.unwrap(); + + let task_handle = tokio::spawn(task.run()); + + // Give it time to process + tokio::time::sleep(Duration::from_millis(50)).await; + + // Stop the task + cancel_token.cancel(); + task_handle.await.unwrap(); + } + + #[tokio::test] + async fn test_handle_derived_event_post_interop() { + let block_pair = DerivedRefPair { + source: BlockInfo { + number: 123, + hash: B256::ZERO, + parent_hash: B256::ZERO, timestamp: 0, }, + derived: BlockInfo { + number: 1234, + hash: B256::ZERO, + parent_hash: B256::ZERO, + timestamp: 1003, + }, }; let mut mockdb = MockDb::new(); @@ -650,7 +786,62 @@ mod tests { let cancel_token = CancellationToken::new(); let (tx, rx) = mpsc::channel(10); - let rollup_config = RollupConfig::default(); + let rollup_config = get_rollup_config(1000); + let task = ChainProcessorTask::new( + rollup_config, + 1, + managed_node, + writer, + cancel_token.clone(), + rx, + ); + + // Send unsafe block event + tx.send(ChainEvent::DerivedBlock { derived_ref_pair: block_pair }).await.unwrap(); + + let task_handle = tokio::spawn(task.run()); + + // Give it time to process + tokio::time::sleep(Duration::from_millis(50)).await; + + // Stop the task + cancel_token.cancel(); + task_handle.await.unwrap(); + } + + #[tokio::test] + async fn test_handle_derived_event_interop_activation() { + let block_pair = DerivedRefPair { + source: BlockInfo { + number: 123, + hash: B256::ZERO, + parent_hash: B256::ZERO, + timestamp: 0, + }, + derived: BlockInfo { + number: 1234, + hash: B256::ZERO, + parent_hash: B256::ZERO, + timestamp: 1001, + }, + }; + + let mut mockdb = MockDb::new(); + let mocknode = MockNode::new(); + + mockdb.expect_initialise_derivation_storage().returning(move |_pair: DerivedRefPair| { + assert_eq!(_pair, block_pair); + Ok(()) + }); + + let writer = Arc::new(mockdb); + let managed_node = Arc::new(mocknode); + + let cancel_token = CancellationToken::new(); + let (tx, rx) = mpsc::channel(10); + + let rollup_config = get_rollup_config(1000); + let task = ChainProcessorTask::new( rollup_config, 1, From 2ad4707394fa78354d1e8a1aa71d9a229a99711b Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Mon, 14 Jul 2025 23:44:51 +0530 Subject: [PATCH 22/27] corner cases + bugfixes --- .../core/src/config/rollup_config_set.rs | 16 +++++++++++++--- .../supervisor/core/src/safety_checker/task.rs | 4 ++-- crates/supervisor/core/src/supervisor.rs | 2 +- crates/supervisor/core/src/syncnode/resetter.rs | 5 ++++- .../storage/src/providers/head_ref_provider.rs | 3 +-- 5 files changed, 21 insertions(+), 9 deletions(-) diff --git a/crates/supervisor/core/src/config/rollup_config_set.rs b/crates/supervisor/core/src/config/rollup_config_set.rs index 5c9d9c06e2..c9f7ea7a94 100644 --- a/crates/supervisor/core/src/config/rollup_config_set.rs +++ b/crates/supervisor/core/src/config/rollup_config_set.rs @@ -78,9 +78,9 @@ impl RollupConfig { /// Interop activates at [`interop_time`](Self::interop_time). This function checks whether the /// provided timestamp is or after interop time /// - /// Returns `false` if `interop_time` is not configured or is 0. - fn is_interop(&self, timestamp: u64) -> bool { - self.interop_time.is_some_and(|t| t != 0 && timestamp >= t) + /// Returns `false` if `interop_time` is not configured. + pub fn is_interop(&self, timestamp: u64) -> bool { + self.interop_time.is_some_and(|t| timestamp >= t) } /// Returns `true` if the timestamp is strictly after the interop activation block. @@ -178,4 +178,14 @@ mod tests { // Unknown chain_id returns false assert!(!set.is_interop_enabled(ChainId::from(999u64), 200)); } + + #[test] + fn test_rollup_config_is_interop_interop_time_zero() { + // Interop time is 100, block_time is 10 + let rollup_config = + RollupConfig::new(Genesis::new(dummy_blockinfo(0), dummy_blockinfo(0)), 2, Some(0)); + + assert!(rollup_config.is_interop(0)); + assert!(rollup_config.is_interop(1000)); + } } diff --git a/crates/supervisor/core/src/safety_checker/task.rs b/crates/supervisor/core/src/safety_checker/task.rs index 42d2aeb2de..5446130220 100644 --- a/crates/supervisor/core/src/safety_checker/task.rs +++ b/crates/supervisor/core/src/safety_checker/task.rs @@ -118,7 +118,7 @@ where .provider .get_safety_head_ref(self.chain_id, self.promoter.target_level()) .map_err(|err| { - if matches!(err, StorageError::EntryNotFound(_)) { + if matches!(err, StorageError::FutureData) { CrossSafetyError::NoBlockToPromote } else { err.into() @@ -129,7 +129,7 @@ where .provider .get_safety_head_ref(self.chain_id, self.promoter.lower_bound_level()) .map_err(|err| { - if matches!(err, StorageError::EntryNotFound(_)) { + if matches!(err, StorageError::FutureData) { CrossSafetyError::NoBlockToPromote } else { err.into() diff --git a/crates/supervisor/core/src/supervisor.rs b/crates/supervisor/core/src/supervisor.rs index 598f3c12c1..cfa5ddede3 100644 --- a/crates/supervisor/core/src/supervisor.rs +++ b/crates/supervisor/core/src/supervisor.rs @@ -145,7 +145,7 @@ impl Supervisor { let db = self.database_factory.get_or_create_db(*chain_id)?; let interop_time = config.interop_time; let derived_pair = config.genesis.get_derived_pair(); - if config.is_interop_activation_block(derived_pair.derived) { + if config.is_interop(derived_pair.derived.timestamp) { info!(target: "supervisor_service", chain_id, interop_time, %derived_pair, "Initialising database for interop activation block"); db.initialise_log_storage(derived_pair.derived)?; db.initialise_derivation_storage(derived_pair)?; diff --git a/crates/supervisor/core/src/syncnode/resetter.rs b/crates/supervisor/core/src/syncnode/resetter.rs index 4c27c9b810..722dd12476 100644 --- a/crates/supervisor/core/src/syncnode/resetter.rs +++ b/crates/supervisor/core/src/syncnode/resetter.rs @@ -33,7 +33,10 @@ where match self.get_latest_valid_super_head().await { Ok(block) => block, // todo: require refactor and corner case handling - Err(ManagedNodeError::StorageError(StorageError::DatabaseNotInitialised)) => { + Err( + ManagedNodeError::StorageError(StorageError::DatabaseNotInitialised) | + ManagedNodeError::StorageError(StorageError::FutureData), + ) => { self.reset_pre_interop().await?; return Ok(()); } diff --git a/crates/supervisor/storage/src/providers/head_ref_provider.rs b/crates/supervisor/storage/src/providers/head_ref_provider.rs index eca8fe3b76..565c53544a 100644 --- a/crates/supervisor/storage/src/providers/head_ref_provider.rs +++ b/crates/supervisor/storage/src/providers/head_ref_provider.rs @@ -33,8 +33,7 @@ where "Failed to seek head reference" ); })?; - let block_ref = result - .ok_or_else(|| StorageError::EntryNotFound("no head reference found".to_string()))?; + let block_ref = result.ok_or_else(|| StorageError::FutureData)?; Ok(block_ref.into()) } } From 98f22bcc32ce475cb59f1f924706621b55400460 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Tue, 15 Jul 2025 14:39:49 +0530 Subject: [PATCH 23/27] managed node spec changes added --- crates/supervisor/core/src/logindexer/indexer.rs | 2 +- crates/supervisor/core/src/syncnode/client.rs | 2 +- crates/supervisor/core/src/syncnode/error.rs | 2 +- crates/supervisor/rpc/src/jsonrpsee.rs | 4 ++-- tests/devnets/simple-supervisor.yaml | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/supervisor/core/src/logindexer/indexer.rs b/crates/supervisor/core/src/logindexer/indexer.rs index 02eac64c06..42302e676c 100644 --- a/crates/supervisor/core/src/logindexer/indexer.rs +++ b/crates/supervisor/core/src/logindexer/indexer.rs @@ -272,7 +272,7 @@ mod tests { let mut mock_provider = MockBlockProvider::new(); mock_provider.expect_fetch_receipts().withf(move |hash| *hash == block_hash).returning( |_| { - Err(ManagedNodeError::Client(ClientError::Authentication( + Err(ManagedNodeError::ClientError(ClientError::Authentication( AuthenticationError::InvalidHeader, ))) }, diff --git a/crates/supervisor/core/src/syncnode/client.rs b/crates/supervisor/core/src/syncnode/client.rs index 980ead0fad..9fc0b731f7 100644 --- a/crates/supervisor/core/src/syncnode/client.rs +++ b/crates/supervisor/core/src/syncnode/client.rs @@ -305,7 +305,7 @@ impl ManagedNodeClient for Client { Metrics::MANAGED_NODE_RPC_REQUEST_DURATION_SECONDS, "block_ref_by_number", async { - ManagedModeApiClient::block_ref_by_number(client.as_ref(), block_number).await + ManagedModeApiClient::l2_block_ref_by_number(client.as_ref(), block_number).await }, "node" => self.config.url.clone() )?; diff --git a/crates/supervisor/core/src/syncnode/error.rs b/crates/supervisor/core/src/syncnode/error.rs index 6aab59f503..01d1233f45 100644 --- a/crates/supervisor/core/src/syncnode/error.rs +++ b/crates/supervisor/core/src/syncnode/error.rs @@ -7,7 +7,7 @@ use thiserror::Error; pub enum ManagedNodeError { /// Represents an error that occurred while starting the managed node. #[error(transparent)] - Client(#[from] ClientError), + ClientError(#[from] ClientError), /// Represents an error that occurred while subscribing to the managed node. #[error("subscription error: {0}")] diff --git a/crates/supervisor/rpc/src/jsonrpsee.rs b/crates/supervisor/rpc/src/jsonrpsee.rs index 58a3249d54..a2a3722ab8 100644 --- a/crates/supervisor/rpc/src/jsonrpsee.rs +++ b/crates/supervisor/rpc/src/jsonrpsee.rs @@ -193,8 +193,8 @@ pub trait ManagedModeApi { async fn fetch_receipts(&self, block_hash: BlockHash) -> RpcResult; /// Get block infor for a given block number - #[method(name = "blockRefByNumber")] - async fn block_ref_by_number(&self, number: u64) -> RpcResult; + #[method(name = "l2BlockRefByNumber")] + async fn l2_block_ref_by_number(&self, number: u64) -> RpcResult; /// Get the chain id #[method(name = "chainID")] diff --git a/tests/devnets/simple-supervisor.yaml b/tests/devnets/simple-supervisor.yaml index 4d408061bf..090fe86e95 100644 --- a/tests/devnets/simple-supervisor.yaml +++ b/tests/devnets/simple-supervisor.yaml @@ -14,7 +14,7 @@ optimism_package: network_params: network: "kurtosis" network_id: "2151908" - interop_time_offset: 1 + interop_time_offset: 0 holocene_time_offset: 0 isthmus_time_offset: 0 fjord_time_offset: 0 @@ -37,7 +37,7 @@ optimism_package: network_params: network: "kurtosis" network_id: "2151909" - interop_time_offset: 1 + interop_time_offset: 0 holocene_time_offset: 0 isthmus_time_offset: 0 fjord_time_offset: 0 From 6302763ca3702385e1141f6c72f339856ccd82d1 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Tue, 15 Jul 2025 15:42:56 +0530 Subject: [PATCH 24/27] revert reset on FutureErr to let finalized head sync --- crates/supervisor/core/src/syncnode/resetter.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/supervisor/core/src/syncnode/resetter.rs b/crates/supervisor/core/src/syncnode/resetter.rs index 722dd12476..ba062dbd0c 100644 --- a/crates/supervisor/core/src/syncnode/resetter.rs +++ b/crates/supervisor/core/src/syncnode/resetter.rs @@ -34,8 +34,7 @@ where Ok(block) => block, // todo: require refactor and corner case handling Err( - ManagedNodeError::StorageError(StorageError::DatabaseNotInitialised) | - ManagedNodeError::StorageError(StorageError::FutureData), + ManagedNodeError::StorageError(StorageError::DatabaseNotInitialised) ) => { self.reset_pre_interop().await?; return Ok(()); From eec5897917184caf780546ee4e29cf532683c131 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Tue, 15 Jul 2025 15:44:03 +0530 Subject: [PATCH 25/27] lintfix --- crates/supervisor/core/src/syncnode/resetter.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/supervisor/core/src/syncnode/resetter.rs b/crates/supervisor/core/src/syncnode/resetter.rs index ba062dbd0c..4c27c9b810 100644 --- a/crates/supervisor/core/src/syncnode/resetter.rs +++ b/crates/supervisor/core/src/syncnode/resetter.rs @@ -33,9 +33,7 @@ where match self.get_latest_valid_super_head().await { Ok(block) => block, // todo: require refactor and corner case handling - Err( - ManagedNodeError::StorageError(StorageError::DatabaseNotInitialised) - ) => { + Err(ManagedNodeError::StorageError(StorageError::DatabaseNotInitialised)) => { self.reset_pre_interop().await?; return Ok(()); } From aaaba11c2a3ba929943dfcdc48bf6b7e42367d48 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Tue, 15 Jul 2025 16:36:35 +0530 Subject: [PATCH 26/27] review fixes --- .../core/src/chain_processor/task.rs | 33 ++++++++++--------- .../core/src/config/rollup_config_set.rs | 8 ++--- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/crates/supervisor/core/src/chain_processor/task.rs b/crates/supervisor/core/src/chain_processor/task.rs index a82e9d4782..55009ed227 100644 --- a/crates/supervisor/core/src/chain_processor/task.rs +++ b/crates/supervisor/core/src/chain_processor/task.rs @@ -327,17 +327,6 @@ where "Processing local safe derived block pair" ); - if self.rollup_config.is_interop_activation_block(derived_ref_pair.derived) { - info!( - target: "chain_processor", - chain_id = self.chain_id, - block_number = derived_ref_pair.derived.number, - "Initialising derivation storage for interop activation block" - ); - self.state_manager.initialise_derivation_storage(derived_ref_pair)?; - return Ok(derived_ref_pair.derived); - } - if self.rollup_config.is_post_interop(derived_ref_pair.derived.timestamp) { match self.state_manager.save_derived_block(derived_ref_pair) { Ok(_) => return Ok(derived_ref_pair.derived), @@ -371,6 +360,18 @@ where } } } + + if self.rollup_config.is_interop_activation_block(derived_ref_pair.derived) { + info!( + target: "chain_processor", + chain_id = self.chain_id, + block_number = derived_ref_pair.derived.number, + "Initialising derivation storage for interop activation block" + ); + self.state_manager.initialise_derivation_storage(derived_ref_pair)?; + return Ok(derived_ref_pair.derived); + } + Ok(derived_ref_pair.derived) } @@ -385,6 +386,11 @@ where "Processing unsafe block" ); + if self.rollup_config.is_post_interop(block.timestamp) { + self.log_indexer.clone().sync_logs(block); + return Ok(block); + } + if self.rollup_config.is_interop_activation_block(block) { info!( target: "chain_processor", @@ -396,11 +402,6 @@ where return Ok(block); } - if self.rollup_config.is_post_interop(block.timestamp) { - self.log_indexer.clone().sync_logs(block); - return Ok(block); - } - Ok(block) } diff --git a/crates/supervisor/core/src/config/rollup_config_set.rs b/crates/supervisor/core/src/config/rollup_config_set.rs index c9f7ea7a94..8f6dfd6da5 100644 --- a/crates/supervisor/core/src/config/rollup_config_set.rs +++ b/crates/supervisor/core/src/config/rollup_config_set.rs @@ -76,7 +76,7 @@ impl RollupConfig { /// Returns `true` if the timestamp is at or after the interop activation time. /// /// Interop activates at [`interop_time`](Self::interop_time). This function checks whether the - /// provided timestamp is or after interop time + /// provided timestamp is before or after interop timestamp. /// /// Returns `false` if `interop_time` is not configured. pub fn is_interop(&self, timestamp: u64) -> bool { @@ -94,7 +94,7 @@ impl RollupConfig { self.is_interop(timestamp.saturating_sub(self.block_time)) } - /// Returns `true` if the timestamp is of an interop activation block. + /// Returns `true` if given block is the interop activation block. /// /// An interop activation block is defined as the block that is right after the /// interop activation time. @@ -136,12 +136,12 @@ impl RollupConfigSet { Ok(()) } - /// returns whether interop is enabled for a chain at given timestamp + /// Returns `true` if interop is enabled for the chain at given timestamp. pub fn is_interop_enabled(&self, chain_id: ChainId, timestamp: u64) -> bool { self.get(chain_id).map(|cfg| cfg.is_post_interop(timestamp)).unwrap_or(false) // if config not found, return false } - /// returns whether the given block is an interop activation block for the specified chain. + /// Returns `true` if given block is the interop activation block for the specified chain. pub fn is_interop_activation_block(&self, chain_id: ChainId, block: BlockInfo) -> bool { self.get(chain_id).map(|cfg| cfg.is_interop_activation_block(block)).unwrap_or(false) } From 7471cf2341e12ecebe4c6b8b46fdc83745e8a930 Mon Sep 17 00:00:00 2001 From: Arun Dhyani Date: Tue, 15 Jul 2025 18:16:28 +0530 Subject: [PATCH 27/27] added more test cases --- .../core/src/chain_processor/task.rs | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/crates/supervisor/core/src/chain_processor/task.rs b/crates/supervisor/core/src/chain_processor/task.rs index 55009ed227..a2209cdb31 100644 --- a/crates/supervisor/core/src/chain_processor/task.rs +++ b/crates/supervisor/core/src/chain_processor/task.rs @@ -865,6 +865,109 @@ mod tests { task_handle.await.unwrap(); } + #[tokio::test] + async fn test_handle_derived_event_block_out_of_order_triggers_reset() { + let block_pair = DerivedRefPair { + source: BlockInfo { + number: 123, + hash: B256::ZERO, + parent_hash: B256::ZERO, + timestamp: 0, + }, + derived: BlockInfo { + number: 1234, + hash: B256::ZERO, + parent_hash: B256::ZERO, + timestamp: 1003, // post-interop + }, + }; + + let mut mockdb = MockDb::new(); + let mut mocknode = MockNode::new(); + + // Simulate BlockOutOfOrder error + mockdb + .expect_save_derived_block() + .returning(move |_pair: DerivedRefPair| Err(StorageError::BlockOutOfOrder)); + + // Expect reset to be called + mocknode.expect_reset().returning(|| Ok(())); + + let writer = Arc::new(mockdb); + let managed_node = Arc::new(mocknode); + + let cancel_token = CancellationToken::new(); + let (tx, rx) = mpsc::channel(10); + + let rollup_config = get_rollup_config(1000); + let task = ChainProcessorTask::new( + rollup_config, + 1, + managed_node, + writer, + cancel_token.clone(), + rx, + ); + + tx.send(ChainEvent::DerivedBlock { derived_ref_pair: block_pair }).await.unwrap(); + + let task_handle = tokio::spawn(task.run()); + + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + cancel_token.cancel(); + task_handle.await.unwrap(); + } + + #[tokio::test] + async fn test_handle_derived_event_other_error() { + let block_pair = DerivedRefPair { + source: BlockInfo { + number: 123, + hash: B256::ZERO, + parent_hash: B256::ZERO, + timestamp: 0, + }, + derived: BlockInfo { + number: 1234, + hash: B256::ZERO, + parent_hash: B256::ZERO, + timestamp: 1003, // post-interop + }, + }; + + let mut mockdb = MockDb::new(); + let mocknode = MockNode::new(); + + // Simulate a different error + mockdb + .expect_save_derived_block() + .returning(move |_pair: DerivedRefPair| Err(StorageError::DatabaseNotInitialised)); + + let writer = Arc::new(mockdb); + let managed_node = Arc::new(mocknode); + + let cancel_token = CancellationToken::new(); + let (tx, rx) = mpsc::channel(10); + + let rollup_config = get_rollup_config(1000); + let task = ChainProcessorTask::new( + rollup_config, + 1, + managed_node, + writer, + cancel_token.clone(), + rx, + ); + + tx.send(ChainEvent::DerivedBlock { derived_ref_pair: block_pair }).await.unwrap(); + + let task_handle = tokio::spawn(task.run()); + + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + cancel_token.cancel(); + task_handle.await.unwrap(); + } + #[tokio::test] async fn test_handle_derivation_origin_update_triggers() { let origin =