address feedback

Author: frisitano

crates/chain-orchestrator/src/consolidation.rs

@@ -30,11 +30,7 @@ pub(crate) async fn reconcile_batch<L2P: Provider<Scroll>>(
             };
 
             // Check if the block matches the derived attributes.
-            if block_matches_attributes(
-                &attributes.attributes,
-                &current_block,
-                current_block.parent_hash,
-            ) {
+            if block_matches_attributes(&attributes.attributes, &current_block) {
                 // Extract the block info with L1 messages.
                 let block_info: L2BlockInfoWithL1Messages = (&current_block).into();
 

crates/chain-orchestrator/src/lib.rs

@@ -255,6 +255,7 @@ impl<
         match event {
             SignerEvent::SignedBlock { block, signature } => {
                 let tx = self.database.tx_mut().await?;
+                tx.set_l2_head_block_number(block.header.number).await?;
                 tx.insert_signature(block.hash_slow(), signature).await?;
                 tx.commit().await?;
                 self.network.handle().announce_block(block.clone(), signature);
@@ -295,7 +296,6 @@ impl<
                     let tx = self.database.tx_mut().await?;
                     let block_info: L2BlockInfoWithL1Messages = (&block).into();
                     tx.update_l1_messages_from_l2_blocks(vec![block_info.clone()]).await?;
-                    tx.set_l2_head_block_number(block_info.block_info.number).await?;
                     tx.commit().await?;
                     self.signer
                         .as_mut()
@@ -1066,18 +1066,14 @@ impl<
             }
         }
 
-        // Persist the L1 message to L2 block mappings for reorg awareness, the block signature and
-        // handle the valid block import if we are in a synced state and the result is valid.
+        // Persist the L1 message to L2 block mappings for reorg awareness, update the l2 head block
+        // number and handle the valid block import if we are in a synced state and the
+        // result is valid.
         if self.sync_state.is_synced() && result.is_valid() {
             let blocks = chain.iter().map(|block| block.into()).collect::<Vec<_>>();
             let tx = self.database.tx_mut().await?;
             tx.update_l1_messages_from_l2_blocks(blocks).await?;
-            tx.commit().await?;
-
-            // Persist the signature for the block and notify the network manager of a successful
-            // import.
-            let tx = self.database.tx_mut().await?;
-            tx.insert_signature(chain_head_hash, block_with_peer.signature).await?;
+            tx.set_l2_head_block_number(block_with_peer.block.header.number).await?;
             tx.commit().await?;
 
             self.network.handle().block_import_outcome(BlockImportOutcome::valid_block(

crates/database/db/src/db.rs

@@ -940,7 +940,7 @@ mod test {
 
         // Retrieve and verify the head block info.
         let tx = db.tx().await.unwrap();
-        let head_block_info = tx.get_l2_head_block_number().await.unwrap().unwrap();
+        let head_block_info = tx.get_l2_head_block_number().await.unwrap();
 
         assert_eq!(head_block_info, block_info.number);
     }

crates/database/db/src/operations.rs

@@ -544,15 +544,17 @@ pub trait DatabaseReadOperations: ReadConnectionProvider + Sync {
     }
 
     /// Get the latest L2 head block info.
-    async fn get_l2_head_block_number(&self) -> Result<Option<u64>, DatabaseError> {
+    async fn get_l2_head_block_number(&self) -> Result<u64, DatabaseError> {
         Ok(models::metadata::Entity::find()
             .filter(models::metadata::Column::Key.eq("l2_head_block"))
             .select_only()
             .column(models::metadata::Column::Value)
             .into_tuple::<String>()
             .one(self.get_connection())
-            .await
-            .map(|x| x.and_then(|x| serde_json::from_str(&x).ok()))?)
+            .await?
+            .expect("l2_head_block should always be set")
+            .parse::<u64>()
+            .expect("l2_head_block should always be a valid u64"))
     }
 
     /// Get an iterator over all [`BatchCommitData`]s in the database.

crates/database/migration/src/m20251005_160938_add_initial_l1_block_numbers.rs

@@ -14,7 +14,8 @@ impl MigrationTrait for Migration {
             INSERT INTO metadata (key, value)
             VALUES 
                 ('l1_finalized_block', '0'),
-                ('l1_latest_block', '0')
+                ('l1_latest_block', '0'),
+                ('l2_head_block', '0')
             ON CONFLICT(key) DO NOTHING;
             "#,
         )
@@ -29,7 +30,7 @@ impl MigrationTrait for Migration {
         db.execute_unprepared(
             r#"
             DELETE FROM metadata 
-            WHERE key IN ('l1_finalized_block', 'l1_latest_block');
+            WHERE key IN ('l1_finalized_block', 'l1_latest_block', 'l2_head_block');
             "#,
         )
         .await?;

crates/engine/src/payload.rs

@@ -1,30 +1,16 @@
-use alloy_primitives::{B256, B64};
+use alloy_primitives::B64;
 use reth_primitives_traits::{AlloyBlockHeader, Block, BlockBody};
 use scroll_alloy_rpc_types_engine::ScrollPayloadAttributes;
 
 use tracing::debug;
 
 /// Returns true if the [`Block`] matches the [`ScrollPayloadAttributes`]:
-///    - parent hash matches the parent hash of the [`Block`].
 ///    - all transactions match.
 ///    - timestamps are equal.
 ///    - `prev_randaos` are equal.
 ///    - `block_data_hint` matches the block data if present.
-pub fn block_matches_attributes<B: Block>(
-    attributes: &ScrollPayloadAttributes,
-    block: &B,
-    parent_hash: B256,
-) -> bool {
+pub fn block_matches_attributes<B: Block>(attributes: &ScrollPayloadAttributes, block: &B) -> bool {
     let header = block.header();
-    if header.parent_hash() != parent_hash {
-        debug!(
-            target: "scroll::engine::driver",
-            expected = ?parent_hash,
-            got = ?header.parent_hash(),
-            "reorg: mismatch in parent hash"
-        );
-        return false;
-    }
 
     let payload_transactions = &block.body().encoded_2718_transactions();
     let matching_transactions =
@@ -109,7 +95,7 @@ mod tests {
 
     use alloy_consensus::Header;
     use alloy_eips::Encodable2718;
-    use alloy_primitives::{Bytes, U256};
+    use alloy_primitives::{Bytes, B256, U256};
     use arbitrary::{Arbitrary, Unstructured};
     use reth_scroll_primitives::ScrollBlock;
     use reth_testing_utils::{generators, generators::Rng};
@@ -154,7 +140,7 @@ mod tests {
             body: alloy_consensus::BlockBody { transactions, ..Default::default() },
         };
 
-        assert!(block_matches_attributes(&attributes, &block, parent_hash));
+        assert!(block_matches_attributes(&attributes, &block));
 
         Ok(())
     }
@@ -185,7 +171,7 @@ mod tests {
             body: alloy_consensus::BlockBody { transactions, ..Default::default() },
         };
 
-        assert!(!block_matches_attributes(&attributes, &block, parent_hash));
+        assert!(!block_matches_attributes(&attributes, &block));
 
         Ok(())
     }

crates/node/src/args.rs

@@ -266,34 +266,23 @@ impl ScrollRollupNodeConfig {
         let tx = db.tx_mut().await?;
         let (_startup_safe_block, l1_start_block_number) =
             tx.prepare_on_startup(chain_spec.genesis_hash()).await?;
+        let l2_head_block_number = tx.get_l2_head_block_number().await?;
+        tx.purge_l1_message_to_l2_block_mappings(Some(l2_head_block_number + 1)).await?;
         tx.commit().await?;
-        // if let Some(block_info) = startup_safe_block {
-        //     fcs.update(None, Some(block_info), Some(block_info))?;
-        // } else {
-        //     fcs.update(
-        //         None,
-        //         Some(BlockInfo {
-        //             hash: genesis_hash_from_chain_spec(chain_spec.clone()).unwrap(),
-        //             number: 0,
-        //         }),
-        //         None,
-        //     )?;
-        // }
 
         // Update the head block info if available and ahead of finalized.
-        if let Some(latest_block_number) = db.tx().await?.get_l2_head_block_number().await? {
-            if latest_block_number > fcs.finalized_block_info().number {
-                let block = l2_provider
-                    .get_block(latest_block_number.into())
-                    .full()
-                    .await?
-                    .expect("latest block from db should exist")
-                    .into_consensus()
-                    .map_transactions(|tx| tx.inner.into_inner());
-                let block_info: BlockInfo = (&block).into();
-
-                fcs.update(Some(block_info), None, None)?;
-            }
+        let l2_head_block_number = db.tx().await?.get_l2_head_block_number().await?;
+        if l2_head_block_number > fcs.finalized_block_info().number {
+            let block = l2_provider
+                .get_block(l2_head_block_number.into())
+                .full()
+                .await?
+                .expect("latest block from db should exist")
+                .into_consensus()
+                .map_transactions(|tx| tx.inner.into_inner());
+            let block_info: BlockInfo = (&block).into();
+
+            fcs.update(Some(block_info), None, None)?;
         }
 
         let chain_spec = Arc::new(chain_spec.clone());

crates/node/tests/e2e.rs

@@ -1080,10 +1080,9 @@ async fn shutdown_consolidates_most_recent_batch_on_startup() -> eyre::Result<()
 }
 
 /// Test that when the rollup node manager is shutdown, it restarts with the head set to the latest
-/// sequenced block stored in database.
+/// signed block stored in database.
 #[tokio::test]
-async fn graceful_shutdown_sets_fcs_to_latest_sequenced_block_in_db_on_start_up() -> eyre::Result<()>
-{
+async fn graceful_shutdown_sets_fcs_to_latest_signed_block_in_db_on_start_up() -> eyre::Result<()> {
     reth_tracing::init_test_tracing();
     let chain_spec = (*SCROLL_DEV).clone();
 
@@ -1152,7 +1151,7 @@ async fn graceful_shutdown_sets_fcs_to_latest_sequenced_block_in_db_on_start_up(
         handle.build_block();
         let block_number = loop {
             let _ = rnm.poll_unpin(&mut Context::from_waker(noop_waker_ref()));
-            if let Poll::Ready(Some(ChainOrchestratorEvent::BlockSequenced(block))) =
+            if let Poll::Ready(Some(ChainOrchestratorEvent::SignedBlock { block, signature: _ })) =
                 rnm_events.poll_next_unpin(&mut Context::from_waker(noop_waker_ref()))
             {
                 break block.header.number

crates/sequencer/src/lib.rs

@@ -236,6 +236,17 @@ impl<SMP, CS> Stream for Sequencer<SMP, CS> {
         let this = self.get_mut();
         this.waker.register(cx.waker());
 
+        // If there is an inflight payload building job, poll it.
+        if let Some(payload_building_job) = this.payload_building_job.as_mut() {
+            match payload_building_job.future.as_mut().poll(cx) {
+                Poll::Ready(payload_id) => {
+                    this.payload_building_job = None;
+                    return Poll::Ready(Some(SequencerEvent::PayloadReady(payload_id)));
+                }
+                Poll::Pending => {}
+            }
+        }
+
         // Poll the trigger to see if it's time to build a new block.
         if let Some(trigger) = this.trigger.as_mut() {
             match trigger.poll_tick(cx) {
@@ -250,17 +261,6 @@ impl<SMP, CS> Stream for Sequencer<SMP, CS> {
             }
         }
 
-        // If there is an inflight payload building job, poll it.
-        if let Some(payload_building_job) = this.payload_building_job.as_mut() {
-            match payload_building_job.future.as_mut().poll(cx) {
-                Poll::Ready(payload_id) => {
-                    this.payload_building_job = None;
-                    return Poll::Ready(Some(SequencerEvent::PayloadReady(payload_id)));
-                }
-                Poll::Pending => {}
-            }
-        }
-
         Poll::Pending
     }
 }