fix: eth66 serve/persist signature (#312)

Author: yiweichi

Cargo.lock

@@ -37,6 +37,7 @@ reth-scroll-engine-primitives = { git = "https://github.com/scroll-tech/reth.git
 # rollup-node
 rollup-node-primitives.workspace = true
 rollup-node-providers.workspace = true
+rollup-node-signer.workspace = true
 
 # scroll
 scroll-network.workspace = true

crates/engine/Cargo.toml

@@ -1,6 +1,7 @@
 use super::{payload::block_matches_attributes, EngineDriverError};
 use crate::{api::*, ForkchoiceState};
 
+use alloy_primitives::bytes::Bytes;
 use alloy_provider::Provider;
 use alloy_rpc_types_engine::{
     ExecutionData, ExecutionPayloadV1, ForkchoiceState as AlloyForkchoiceState, ForkchoiceUpdated,
@@ -13,6 +14,7 @@ use rollup_node_primitives::{
     BatchInfo, BlockInfo, ChainImport, L2BlockInfoWithL1Messages, MeteredFuture,
     ScrollPayloadAttributesWithBatchInfo, WithBlockNumber,
 };
+use rollup_node_signer::SignatureAsBytes;
 use scroll_alloy_hardforks::ScrollHardforks;
 use scroll_alloy_network::Scroll;
 use scroll_alloy_provider::ScrollEngineApi;
@@ -236,7 +238,7 @@ where
             Some(BlockImportOutcome::valid_block(
                 peer_id,
                 head,
-                Into::<Vec<u8>>::into(signature).into(),
+                Bytes::copy_from_slice(&signature.sig_as_bytes()),
             )),
             PayloadStatusEnum::Valid,
         )),

crates/engine/src/future/mod.rs

@@ -25,6 +25,7 @@ reth-tokio-util.workspace = true
 reth-scroll-chainspec.workspace = true
 reth-scroll-node.workspace = true
 reth-scroll-primitives.workspace = true
+rollup-node-primitives.workspace = true
 scroll-alloy-hardforks.workspace = true
 scroll-wire.workspace = true
 

crates/network/Cargo.toml

@@ -4,7 +4,7 @@ use super::{
     BlockImportOutcome, BlockValidation, NetworkHandleMessage, NetworkManagerEvent,
     NewBlockWithPeer, ScrollNetworkHandle,
 };
-use alloy_primitives::{FixedBytes, Signature, B256, U128};
+use alloy_primitives::{Address, FixedBytes, Signature, B256, U128};
 use futures::{FutureExt, Stream, StreamExt};
 use reth_chainspec::EthChainSpec;
 use reth_eth_wire_types::NewBlock as EthWireNewBlock;
@@ -17,6 +17,7 @@ use reth_scroll_node::ScrollNetworkPrimitives;
 use reth_scroll_primitives::ScrollBlock;
 use reth_storage_api::BlockNumReader as BlockNumReaderT;
 use reth_tokio_util::EventStream;
+use rollup_node_primitives::sig_encode_hash;
 use scroll_alloy_hardforks::ScrollHardforks;
 use scroll_wire::{
     NewBlock, ScrollWireConfig, ScrollWireEvent, ScrollWireManager, ScrollWireProtocolHandler,
@@ -59,6 +60,8 @@ pub struct ScrollNetworkManager<N, CS> {
     pub blocks_seen: LruCache<(B256, Signature)>,
     /// The constant value that must be added to the block number to get the total difficulty.
     td_constant: U128,
+    /// The authorized signer for the network.
+    authorized_signer: Option<Address>,
 }
 
 impl<CS: ScrollHardforks + EthChainSpec + Send + Sync + 'static>
@@ -72,6 +75,7 @@ impl<CS: ScrollHardforks + EthChainSpec + Send + Sync + 'static>
         scroll_wire_config: ScrollWireConfig,
         eth_wire_listener: Option<EventStream<RethNewBlockWithPeer<ScrollBlock>>>,
         td_constant: U128,
+        authorized_signer: Option<Address>,
     ) -> Self {
         // Create the scroll-wire protocol handler.
         let (scroll_wire_handler, events) = ScrollWireProtocolHandler::new(scroll_wire_config);
@@ -105,6 +109,7 @@ impl<CS: ScrollHardforks + EthChainSpec + Send + Sync + 'static>
             blocks_seen,
             eth_wire_listener,
             td_constant,
+            authorized_signer,
         }
     }
 }
@@ -124,6 +129,7 @@ impl<
         events: UnboundedReceiver<ScrollWireEvent>,
         eth_wire_listener: Option<EventStream<RethNewBlockWithPeer<ScrollBlock>>>,
         td_constant: U128,
+        authorized_signer: Option<Address>,
     ) -> Self {
         // Create the channel for sending messages to the network manager from the network handle.
         let (to_manager_tx, from_handle_rx) = mpsc::unbounded_channel();
@@ -143,6 +149,7 @@ impl<
             blocks_seen,
             eth_wire_listener,
             td_constant,
+            authorized_signer,
         }
     }
 
@@ -169,13 +176,37 @@ impl<
             .filter_map(|(peer_id, blocks)| (!blocks.contains(&hash)).then_some(*peer_id))
             .collect();
 
-        let eth_wire_new_block = {
-            let td = compute_td(self.td_constant, block.block.header.number);
-            let mut eth_wire_block = block.block.clone();
-            eth_wire_block.header.extra_data = block.signature.clone().into();
-            EthWireNewBlock { block: eth_wire_block, td }
+        // TODO: remove this once we deprecate l2geth.
+        // Determine if we should announce via eth wire
+        let should_announce_eth_wire = if let Some(authorized_signer) = self.authorized_signer {
+            // Only announce if the block signature matches the authorized signer
+            let sig_hash = sig_encode_hash(&block.block.header);
+            if let Ok(signature) = Signature::from_raw(&block.signature) {
+                if let Ok(recovered_signer) =
+                    reth_primitives_traits::crypto::secp256k1::recover_signer(&signature, sig_hash)
+                {
+                    authorized_signer == recovered_signer
+                } else {
+                    false
+                }
+            } else {
+                false
+            }
+        } else {
+            // If no authorized signer is set, always announce
+            true
         };
-        self.inner_network_handle().eth_wire_announce_block(eth_wire_new_block, hash);
+
+        // Announce via eth wire if allowed
+        if should_announce_eth_wire {
+            let eth_wire_new_block = {
+                let td = compute_td(self.td_constant, block.block.header.number);
+                let mut eth_wire_block = block.block.clone();
+                eth_wire_block.header.extra_data = block.signature.clone().into();
+                EthWireNewBlock { block: eth_wire_block, td }
+            };
+            self.inner_network_handle().eth_wire_announce_block(eth_wire_new_block, hash);
+        }
 
         // Announce block to the filtered set of peers
         for peer_id in peers {

crates/network/src/manager.rs

@@ -246,12 +246,17 @@ impl ScrollRollupNodeConfig {
             .network_args
             .enable_eth_scroll_wire_bridge
             .then_some(ctx.network.eth_wire_block_listener().await?);
+
+        // TODO: remove this once we deprecate l2geth.
+        let authorized_signer = self.network_args.effective_signer(chain_spec.chain().named());
+
         let scroll_network_manager = ScrollNetworkManager::from_parts(
             chain_spec.clone(),
             ctx.network.clone(),
             events,
             eth_wire_listener,
             td_constant(chain_spec.chain().named()),
+            authorized_signer,
         );
 
         // On startup we replay the latest batch of blocks from the database as such we set the safe

crates/node/src/args.rs

@@ -10,10 +10,11 @@ use reth_network::{
 use reth_node_api::TxTy;
 use reth_node_builder::{components::NetworkBuilder, BuilderContext, FullNodeTypes};
 use reth_node_types::NodeTypes;
-use reth_primitives_traits::BlockHeader;
+use reth_primitives_traits::{BlockHeader, Header};
 use reth_scroll_chainspec::ScrollChainSpec;
 use reth_scroll_primitives::ScrollPrimitives;
 use reth_transaction_pool::{PoolTransaction, TransactionPool};
+use rollup_node_primitives::sig_encode_hash;
 use rollup_node_signer::SignatureAsBytes;
 use scroll_alloy_hardforks::ScrollHardforks;
 use scroll_db::{Database, DatabaseOperations};
@@ -165,13 +166,14 @@ impl<H: BlockHeader, ChainSpec: EthChainSpec + ScrollHardforks + Debug + Send +
         if !self.chain_spec.is_euclid_v2_active_at_timestamp(header.timestamp()) {
             return header;
         }
+
         // TODO: remove this once we deprecated l2geth
         // Validate and process signature
-
         if let Err(err) = self.validate_and_store_signature(&mut header, self.signer) {
             debug!(
                 target: "scroll::network::response_header_transform",
-                "Header signature persistence failed, header hash: {:?}, error: {}",
+                "Header signature persistence failed, block number: {:?}, header hash: {:?}, error: {}",
+                header.number(),
                 header.hash_slow(), err
             );
         }
@@ -190,7 +192,7 @@ impl<ChainSpec: ScrollHardforks + Debug + Send + Sync> ScrollHeaderTransform<Cha
         let signature = parse_65b_signature(&signature_bytes)?;
 
         // Recover and verify signer
-        recover_and_verify_signer(&signature, header.hash_slow(), authorized_signer)?;
+        recover_and_verify_signer(&signature, header, authorized_signer)?;
 
         // Store signature in database
         persist_signature(self.db.clone(), header.hash_slow(), signature);
@@ -231,15 +233,17 @@ impl<H: BlockHeader, ChainSpec: EthChainSpec + ScrollHardforks + Debug + Send +
         }
 
         // read the signature from the rollup node.
-        let block_hash = header.hash_slow();
+        let hash = header.hash_slow();
+
         let signature = self
             .db
-            .get_signature(block_hash)
+            .get_signature(hash)
             .await
             .inspect_err(|e| {
                 warn!(target: "scroll::network::request_header_transform",
-                    "Failed to get block signature from database, header hash: {:?}, error: {}",
-                    header.hash_slow(),
+                    "Failed to get block signature from database, block number: {:?}, header hash: {:?}, error: {}",
+                    header.number(),
+                    hash,
                     HeaderTransformError::DatabaseError(e.to_string())
                 )
             })
@@ -249,32 +253,42 @@ impl<H: BlockHeader, ChainSpec: EthChainSpec + ScrollHardforks + Debug + Send +
         // If we have a signature in the database and it matches configured signer then add it
         // to the extra data field
         if let Some(sig) = signature {
-            if let Err(err) = recover_and_verify_signer(&sig, header.hash_slow(), self.signer) {
+            if let Err(err) = recover_and_verify_signer(&sig, &header, self.signer) {
                 warn!(
                 target: "scroll::network::request_header_transform",
-                    "Found invalid signature(different from the hardcoded signer) for header hash: {:?}, sig: {:?}, error: {}",
-                    header.hash_slow(),
+                    "Found invalid signature(different from the hardcoded signer={:?}) for block number: {:?}, header hash: {:?}, sig: {:?}, error: {}",
+                    self.signer,
+                    header.number(),
+                    hash,
                     sig.to_string(),
                     err
                 );
             } else {
                 *header.extra_data_mut() = sig.sig_as_bytes().into();
             }
+        } else {
+            debug!(
+                target: "scroll::network::request_header_transform",
+                "No signature found in database for block number: {:?}, header hash: {:?}",
+                header.number(),
+                hash,
+            );
         }
 
         header
     }
 }
 
 /// Recover signer from signature and verify authorization.
-fn recover_and_verify_signer(
+fn recover_and_verify_signer<H: BlockHeader>(
     signature: &Signature,
-    hash: B256,
+    header: &H,
     authorized_signer: Option<Address>,
 ) -> Result<Address, HeaderTransformError> {
+    let hash = sig_encode_hash(&header_to_alloy(header));
+
     // Recover signer from signature
-    let signer = signature
-        .recover_address_from_prehash(&hash)
+    let signer = reth_primitives_traits::crypto::secp256k1::recover_signer(signature, hash)
         .map_err(|_| HeaderTransformError::RecoveryFailed)?;
 
     // Verify signer is authorized
@@ -310,3 +324,30 @@ fn persist_signature(db: Arc<Database>, hash: B256, signature: Signature) {
         }
     });
 }
+
+/// Convert a generic `BlockHeader` to `alloy_consensus::Header`
+fn header_to_alloy<H: BlockHeader>(header: &H) -> Header {
+    Header {
+        parent_hash: header.parent_hash(),
+        ommers_hash: header.ommers_hash(),
+        beneficiary: header.beneficiary(),
+        state_root: header.state_root(),
+        transactions_root: header.transactions_root(),
+        receipts_root: header.receipts_root(),
+        logs_bloom: header.logs_bloom(),
+        difficulty: header.difficulty(),
+        number: header.number(),
+        gas_limit: header.gas_limit(),
+        gas_used: header.gas_used(),
+        timestamp: header.timestamp(),
+        extra_data: header.extra_data().clone(),
+        mix_hash: header.mix_hash().unwrap_or_default(),
+        nonce: header.nonce().unwrap_or_default(),
+        base_fee_per_gas: header.base_fee_per_gas(),
+        withdrawals_root: header.withdrawals_root(),
+        blob_gas_used: header.blob_gas_used(),
+        excess_blob_gas: header.excess_blob_gas(),
+        parent_beacon_block_root: header.parent_beacon_block_root(),
+        requests_hash: header.requests_hash(),
+    }
+}

crates/node/src/builder/network.rs

@@ -746,6 +746,7 @@ async fn can_bridge_blocks() {
         scroll_wire_config,
         None,
         Default::default(),
+        None,
     )
     .await;
     let scroll_network_handle = scroll_network.handle();