From 2fcdfd8170ef14596b922a3c7dd2494263f23842 Mon Sep 17 00:00:00 2001 From: Rakan Al-Huneiti Date: Tue, 17 Sep 2024 15:17:54 +0300 Subject: [PATCH] Complete test implementation (#1124) * Complete test implementation * Add comment * Wait for proof * Sort before checking range * Send shutdown message to da queue * Cleanup * Remove docker from prover test config * Restore prefixes * Update bin/citrea/tests/bitcoin_e2e/tests/prover_test.rs Co-authored-by: jfldde <168934971+jfldde@users.noreply.github.com> * Restore inscribes_queue * Use finality depth to generate 1 finalized DA height * Unwrap or else * Update crates/prover/src/da_block_handler.rs * Mention L1 height in log * Remove unnecessary sort --------- Co-authored-by: jfldde <168934971+jfldde@users.noreply.github.com> --- bin/citrea/src/rollup/bitcoin.rs | 2 +- bin/citrea/tests/bitcoin_e2e/test_case.rs | 58 +++++----- .../tests/bitcoin_e2e/tests/mempool_accept.rs | 2 +- .../tests/bitcoin_e2e/tests/prover_test.rs | 106 ++++++++++++----- .../tests/sequencer_commitments.rs | 6 +- .../tests/bitcoin_e2e/tests/sequencer_test.rs | 2 +- .../tests/bitcoin_e2e/tests/sync_test.rs | 2 +- crates/bitcoin-da/src/service.rs | 109 ++++++++++-------- crates/fullnode/src/da_block_handler.rs | 3 +- crates/prover/src/da_block_handler.rs | 7 +- crates/sequencer/src/sequencer.rs | 2 +- .../adapters/mock-da/src/service.rs | 6 +- .../rollup-interface/src/node/services/da.rs | 2 +- 13 files changed, 185 insertions(+), 122 deletions(-) diff --git a/bin/citrea/src/rollup/bitcoin.rs b/bin/citrea/src/rollup/bitcoin.rs index 121a40b06..5614924cf 100644 --- a/bin/citrea/src/rollup/bitcoin.rs +++ b/bin/citrea/src/rollup/bitcoin.rs @@ -119,7 +119,7 @@ impl RollupBlueprint for BitcoinRollup { rollup_config: &FullNodeConfig, require_wallet_check: bool, ) -> Result, anyhow::Error> { - let (tx, rx) = unbounded_channel::>(); + let (tx, rx) = unbounded_channel::>>(); let bitcoin_service = if require_wallet_check { BitcoinService::new_with_wallet_check( diff --git a/bin/citrea/tests/bitcoin_e2e/test_case.rs b/bin/citrea/tests/bitcoin_e2e/test_case.rs index 0496b9f3a..b299a3412 100644 --- a/bin/citrea/tests/bitcoin_e2e/test_case.rs +++ b/bin/citrea/tests/bitcoin_e2e/test_case.rs @@ -3,7 +3,6 @@ use std::panic::{self}; use std::path::{Path, PathBuf}; -use std::sync::Arc; use std::time::Duration; use anyhow::{bail, Context}; @@ -11,8 +10,8 @@ use async_trait::async_trait; use bitcoin_da::service::BitcoinServiceConfig; use bitcoincore_rpc::RpcApi; use citrea_sequencer::SequencerConfig; +use futures::FutureExt; use sov_stf_runner::{ProverConfig, RpcConfig, RunnerConfig, StorageConfig}; -use tokio::task; use super::config::{ default_rollup_config, BitcoinConfig, FullFullNodeConfig, FullProverConfig, @@ -29,12 +28,12 @@ use crate::bitcoin_e2e::utils::{get_default_genesis_path, get_workspace_root}; /// It creates a test framework with the associated configs, spawns required nodes, connects them, /// runs the test case, and performs cleanup afterwards. The `run` method handles any panics that /// might occur during test execution and takes care of cleaning up and stopping the child processes. -pub struct TestCaseRunner(Arc); +pub struct TestCaseRunner(T); impl TestCaseRunner { /// Creates a new TestCaseRunner with the given test case. pub fn new(test_case: T) -> Self { - Self(Arc::new(test_case)) + Self(test_case) } pub async fn setup(&self, f: &mut TestFramework) -> Result<()> { @@ -60,7 +59,7 @@ impl TestCaseRunner { } /// Internal method to set up connect the nodes, wait for the nodes to be ready and run the test. - async fn run_test_case(&self, f: &mut TestFramework) -> Result<()> { + async fn run_test_case(&mut self, f: &mut TestFramework) -> Result<()> { f.bitcoin_nodes.connect_nodes().await?; if let Some(sequencer) = &f.sequencer { @@ -72,34 +71,29 @@ impl TestCaseRunner { /// Executes the test case, handling any panics and performing cleanup. /// - /// This method spawns a blocking task to run the test, sets up the framework, - /// executes the test, and ensures cleanup is performed even if a panic occurs. - pub async fn run(self) -> Result<()> { - let result = task::spawn_blocking(move || { - let mut framework = None; - let result = panic::catch_unwind(panic::AssertUnwindSafe(|| { - futures::executor::block_on(async { - framework = Some(TestFramework::new(Self::generate_test_config()?).await?); - self.setup(framework.as_mut().unwrap()).await?; - self.run_test_case(framework.as_mut().unwrap()).await - }) - })); - - // Always attempt to stop the framework, even if a panic occurred - if let Some(mut f) = framework { - let _ = futures::executor::block_on(f.stop()); - - if result.is_err() { - if let Err(e) = f.dump_log() { - eprintln!("{e}") - } + /// This sets up the framework, executes the test, and ensures cleanup is performed even if a panic occurs. + pub async fn run(mut self) -> Result<()> { + let result = panic::AssertUnwindSafe(async { + let mut framework = TestFramework::new(Self::generate_test_config()?).await?; + self.setup(&mut framework).await?; + + let test_result = self.run_test_case(&mut framework).await; + + if test_result.is_err() { + if let Err(e) = framework.dump_log() { + eprintln!("Error dumping log: {}", e); } } - result + framework.stop().await?; + + test_result }) - .await - .expect("Task panicked"); + .catch_unwind() + .await; + + // Additional test cleanup + self.0.cleanup().await?; match result { Ok(Ok(())) => Ok(()), @@ -309,7 +303,11 @@ pub trait TestCase: Send + Sync + 'static { /// /// # Arguments /// * `framework` - A reference to the TestFramework instance - async fn run_test(&self, framework: &TestFramework) -> Result<()>; + async fn run_test(&mut self, framework: &TestFramework) -> Result<()>; + + async fn cleanup(&self) -> Result<()> { + Ok(()) + } } fn create_dirs(base_dir: &Path) -> Result<[PathBuf; 6]> { diff --git a/bin/citrea/tests/bitcoin_e2e/tests/mempool_accept.rs b/bin/citrea/tests/bitcoin_e2e/tests/mempool_accept.rs index 76d47d22e..5301ded54 100644 --- a/bin/citrea/tests/bitcoin_e2e/tests/mempool_accept.rs +++ b/bin/citrea/tests/bitcoin_e2e/tests/mempool_accept.rs @@ -23,7 +23,7 @@ impl TestCase for MempoolAcceptTest { } } - async fn run_test(&self, f: &TestFramework) -> Result<()> { + async fn run_test(&mut self, f: &TestFramework) -> Result<()> { let sequencer = f.sequencer.as_ref().unwrap(); let da = f.bitcoin_nodes.get(0).expect("DA not running."); diff --git a/bin/citrea/tests/bitcoin_e2e/tests/prover_test.rs b/bin/citrea/tests/bitcoin_e2e/tests/prover_test.rs index 14c4bcc73..88926145d 100644 --- a/bin/citrea/tests/bitcoin_e2e/tests/prover_test.rs +++ b/bin/citrea/tests/bitcoin_e2e/tests/prover_test.rs @@ -3,13 +3,13 @@ use std::time::Duration; use anyhow::bail; use async_trait::async_trait; -use bitcoin::secp256k1::generate_keypair; -use bitcoin_da::service::{BitcoinService, BitcoinServiceConfig, FINALITY_DEPTH}; +use bitcoin_da::service::{BitcoinService, BitcoinServiceConfig, TxidWrapper, FINALITY_DEPTH}; use bitcoin_da::spec::RollupParams; use bitcoincore_rpc::RpcApi; use citrea_primitives::{REVEAL_BATCH_PROOF_PREFIX, REVEAL_LIGHT_CLIENT_PREFIX}; -use hex::ToHex; use sov_rollup_interface::da::{DaData, SequencerCommitment}; +use sov_rollup_interface::services::da::SenderWithNotifier; +use tokio::sync::mpsc::UnboundedSender; use crate::bitcoin_e2e::config::{SequencerConfig, TestCaseConfig}; use crate::bitcoin_e2e::framework::TestFramework; @@ -42,7 +42,7 @@ impl TestCase for BasicProverTest { } } - async fn run_test(&self, f: &TestFramework) -> Result<()> { + async fn run_test(&mut self, f: &TestFramework) -> Result<()> { let Some(sequencer) = &f.sequencer else { bail!("Sequencer not running. Set TestCaseConfig with_sequencer to true") }; @@ -111,7 +111,10 @@ impl TestCase for BasicProverTest { } } -struct SkipPreprovenCommitmentsTest; +#[derive(Default)] +struct SkipPreprovenCommitmentsTest { + tx: Option>>>, +} #[async_trait] impl TestCase for SkipPreprovenCommitmentsTest { @@ -119,7 +122,6 @@ impl TestCase for SkipPreprovenCommitmentsTest { TestCaseConfig { with_prover: true, with_full_node: true, - docker: true, ..Default::default() } } @@ -132,7 +134,7 @@ impl TestCase for SkipPreprovenCommitmentsTest { } } - async fn run_test(&self, f: &TestFramework) -> Result<()> { + async fn run_test(&mut self, f: &TestFramework) -> Result<()> { let Some(sequencer) = &f.sequencer else { bail!("Sequencer not running. Set TestCaseConfig with_sequencer to true") }; @@ -151,11 +153,6 @@ impl TestCase for SkipPreprovenCommitmentsTest { let _initial_height = f.initial_da_height; - let (secret_key, _public_key) = generate_keypair(&mut rand::thread_rng()); - let secret_key = secret_key.secret_bytes().encode_hex(); - // let key_pair = Keypair::from_secret_key(&secp, &secret_key); - // let mut buf = [0u8; constants::SECRET_KEY_SIZE * 2]; - // to_hex(&self.secret_bytes(), &mut buf).expect("fixed-size hex serialization"); let da_config = &f.bitcoin_nodes.get(0).unwrap().config; let bitcoin_da_service_config = BitcoinServiceConfig { node_url: format!( @@ -166,10 +163,21 @@ impl TestCase for SkipPreprovenCommitmentsTest { node_username: da_config.rpc_user.clone(), node_password: da_config.rpc_password.clone(), network: bitcoin::Network::Regtest, - da_private_key: Some(secret_key), + da_private_key: Some( + // This is the private key used by the sequencer. + // This is because the prover has a check to make sure that the commitment was + // submitted by the sequencer and NOT any other key. Which means that arbitrary keys + // CANNOT submit preproven commitments. + // Using the sequencer DA private key means that we simulate the fact that the sequencer + // somehow resubmitted the same commitment. + "045FFC81A3C1FDB3AF1359DBF2D114B0B3EFBF7F29CC9C5DA01267AA39D2C78D".to_owned(), + ), tx_backup_dir: get_tx_backup_dir(), }; let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); + // Keep sender for cleanup + self.tx = Some(tx.clone()); + let bitcoin_da_service = Arc::new( BitcoinService::new_with_wallet_check( bitcoin_da_service_config, @@ -177,7 +185,7 @@ impl TestCase for SkipPreprovenCommitmentsTest { reveal_light_client_prefix: REVEAL_LIGHT_CLIENT_PREFIX.to_vec(), reveal_batch_prover_prefix: REVEAL_BATCH_PROOF_PREFIX.to_vec(), }, - tx, + tx.clone(), ) .await .unwrap(), @@ -190,26 +198,31 @@ impl TestCase for SkipPreprovenCommitmentsTest { let seq_height0 = sequencer.client.eth_block_number().await; assert_eq!(seq_height0, 0); - for _ in 0..10 { + let min_soft_confirmations_per_commitment = + sequencer.min_soft_confirmations_per_commitment(); + + for _ in 0..min_soft_confirmations_per_commitment { sequencer.client.send_publish_batch_request().await; } - da.generate(1 + FINALITY_DEPTH, None).await?; + da.generate(FINALITY_DEPTH, None).await?; // Wait for blob inscribe tx to be in mempool da.wait_mempool_len(1, None).await?; - da.generate(1 + FINALITY_DEPTH, None).await?; + da.generate(FINALITY_DEPTH, None).await?; let finalized_height = da.get_finalized_height().await?; - println!("FINALIZED HEIGHT: {}", finalized_height); prover .wait_for_l1_height(finalized_height, Some(Duration::from_secs(300))) .await; - da.generate(5, None).await?; + da.generate(FINALITY_DEPTH, None).await?; let proofs = full_node - .wait_for_zkproofs(finalized_height + 5, Some(Duration::from_secs(120))) + .wait_for_zkproofs( + finalized_height + FINALITY_DEPTH, + Some(Duration::from_secs(120)), + ) .await .unwrap(); @@ -220,13 +233,21 @@ impl TestCase for SkipPreprovenCommitmentsTest { .preproven_commitments .is_empty()); + // Make sure the mempool is mined. + da.wait_mempool_len(0, None).await?; + // Fetch the commitment created from the previous L1 range - let commitments: Vec = sequencer + let commitments: Vec = full_node .client .ledger_get_sequencer_commitments_on_slot_by_number(finalized_height) .await - .unwrap() - .unwrap() + .unwrap_or_else(|_| { + panic!( + "Failed to get sequencer commitments at {}", + finalized_height + ) + }) + .unwrap_or_else(|| panic!("No sequencer commitments found at {}", finalized_height)) .into_iter() .map(|response| SequencerCommitment { merkle_root: response.merkle_root, @@ -235,6 +256,7 @@ impl TestCase for SkipPreprovenCommitmentsTest { }) .collect(); + // Send the same commitment that was already proven. let fee_sat_per_vbyte = bitcoin_da_service.get_fee_rate().await.unwrap(); bitcoin_da_service .send_transaction_with_fee_rate( @@ -245,12 +267,35 @@ impl TestCase for SkipPreprovenCommitmentsTest { .await .unwrap(); - da.generate(5, None).await?; + // Wait for the duplicate commitment transaction to be accepted. + da.wait_mempool_len(2, None).await?; + + // Trigger a new commitment. + for _ in 0..min_soft_confirmations_per_commitment { + sequencer.client.send_publish_batch_request().await; + } + + // Wait for the sequencer commitment to be submitted & accepted. + da.wait_mempool_len(4, None).await?; + + da.generate(FINALITY_DEPTH, None).await?; + + let finalized_height = da.get_finalized_height().await?; prover - .wait_for_l1_height(FINALITY_DEPTH, Some(Duration::from_secs(300))) + .wait_for_l1_height(finalized_height, Some(Duration::from_secs(300))) .await; + da.generate(FINALITY_DEPTH, None).await?; + + let proofs = full_node + .wait_for_zkproofs( + finalized_height + FINALITY_DEPTH, + Some(Duration::from_secs(120)), + ) + .await + .unwrap(); + assert_eq!( proofs .first() @@ -263,6 +308,14 @@ impl TestCase for SkipPreprovenCommitmentsTest { Ok(()) } + + async fn cleanup(&self) -> Result<()> { + // Send shutdown message to da queue + if let Some(tx) = &self.tx { + tx.send(None).unwrap(); + } + Ok(()) + } } #[tokio::test] @@ -270,10 +323,9 @@ async fn basic_prover_test() -> Result<()> { TestCaseRunner::new(BasicProverTest).run().await } -#[ignore] #[tokio::test] async fn prover_skips_preproven_commitments_test() -> Result<()> { - TestCaseRunner::new(SkipPreprovenCommitmentsTest) + TestCaseRunner::new(SkipPreprovenCommitmentsTest::default()) .run() .await } diff --git a/bin/citrea/tests/bitcoin_e2e/tests/sequencer_commitments.rs b/bin/citrea/tests/bitcoin_e2e/tests/sequencer_commitments.rs index e771ce088..6080061ab 100644 --- a/bin/citrea/tests/bitcoin_e2e/tests/sequencer_commitments.rs +++ b/bin/citrea/tests/bitcoin_e2e/tests/sequencer_commitments.rs @@ -35,7 +35,7 @@ impl TestCase for LedgerGetCommitmentsProverTest { } } - async fn run_test(&self, f: &TestFramework) -> Result<()> { + async fn run_test(&mut self, f: &TestFramework) -> Result<()> { let sequencer = f.sequencer.as_ref().unwrap(); let da = f.bitcoin_nodes.get(0).expect("DA not running."); let prover = f.prover.as_ref().unwrap(); @@ -113,7 +113,7 @@ impl TestCase for LedgerGetCommitmentsTest { } } - async fn run_test(&self, f: &TestFramework) -> Result<()> { + async fn run_test(&mut self, f: &TestFramework) -> Result<()> { let sequencer = f.sequencer.as_ref().unwrap(); let da = f.bitcoin_nodes.get(0).expect("DA not running."); let full_node = f.full_node.as_ref().unwrap(); @@ -181,7 +181,7 @@ impl TestCase for SequencerSendCommitmentsToDaTest { } } - async fn run_test(&self, f: &TestFramework) -> Result<()> { + async fn run_test(&mut self, f: &TestFramework) -> Result<()> { let sequencer = f.sequencer.as_ref().unwrap(); let da = f.bitcoin_nodes.get(0).expect("DA not running."); diff --git a/bin/citrea/tests/bitcoin_e2e/tests/sequencer_test.rs b/bin/citrea/tests/bitcoin_e2e/tests/sequencer_test.rs index eb15693e2..da49aa706 100644 --- a/bin/citrea/tests/bitcoin_e2e/tests/sequencer_test.rs +++ b/bin/citrea/tests/bitcoin_e2e/tests/sequencer_test.rs @@ -11,7 +11,7 @@ struct BasicSequencerTest; #[async_trait] impl TestCase for BasicSequencerTest { - async fn run_test(&self, f: &TestFramework) -> Result<()> { + async fn run_test(&mut self, f: &TestFramework) -> Result<()> { let Some(sequencer) = &f.sequencer else { anyhow::bail!("Sequencer not running. Set TestCaseConfig with_sequencer to true") }; diff --git a/bin/citrea/tests/bitcoin_e2e/tests/sync_test.rs b/bin/citrea/tests/bitcoin_e2e/tests/sync_test.rs index 5a594df14..c99554d26 100644 --- a/bin/citrea/tests/bitcoin_e2e/tests/sync_test.rs +++ b/bin/citrea/tests/bitcoin_e2e/tests/sync_test.rs @@ -21,7 +21,7 @@ impl TestCase for BasicSyncTest { } } - async fn run_test(&self, f: &TestFramework) -> Result<()> { + async fn run_test(&mut self, f: &TestFramework) -> Result<()> { let (Some(da0), Some(da1)) = (f.bitcoin_nodes.get(0), f.bitcoin_nodes.get(1)) else { bail!("bitcoind not running. Test should run with two da nodes") }; diff --git a/crates/bitcoin-da/src/service.rs b/crates/bitcoin-da/src/service.rs index e0587d020..9d3ee8876 100644 --- a/crates/bitcoin-da/src/service.rs +++ b/crates/bitcoin-da/src/service.rs @@ -62,7 +62,7 @@ pub struct BitcoinService { da_private_key: Option, reveal_light_client_prefix: Vec, reveal_batch_prover_prefix: Vec, - inscribes_queue: UnboundedSender>, + inscribes_queue: UnboundedSender>>, tx_backup_dir: PathBuf, } @@ -92,7 +92,7 @@ impl BitcoinService { pub async fn new_with_wallet_check( config: BitcoinServiceConfig, chain_params: RollupParams, - tx: UnboundedSender>, + tx: UnboundedSender>>, ) -> Result { let client = Client::new( &config.node_url, @@ -137,7 +137,7 @@ impl BitcoinService { pub async fn new_without_wallet_check( config: BitcoinServiceConfig, chain_params: RollupParams, - tx: UnboundedSender>, + tx: UnboundedSender>>, ) -> Result { let client = Client::new( &config.node_url, @@ -171,7 +171,7 @@ impl BitcoinService { pub fn spawn_da_queue( self: Arc, - mut rx: UnboundedReceiver>, + mut rx: UnboundedReceiver>>, ) { // This is a queue of inscribe requests tokio::task::spawn_blocking(|| { @@ -191,50 +191,61 @@ impl BitcoinService { trace!("BitcoinDA queue is initialized. Waiting for the first request..."); // We execute commit and reveal txs one by one to chain them - while let Some(request) = rx.recv().await { - trace!("A new request is received"); - let prev = prev_utxo.take(); - loop { - // Build and send tx with retries: - let fee_sat_per_vbyte = match self.get_fee_rate().await { - Ok(rate) => rate, - Err(e) => { - error!(?e, "Failed to call get_fee_rate. Retrying..."); - tokio::time::sleep(Duration::from_secs(1)).await; - continue; - } - }; - match self - .send_transaction_with_fee_rate( - prev.clone(), - request.da_data.clone(), - fee_sat_per_vbyte, - ) - .await - { - Ok(tx) => { - let tx_id = TxidWrapper(tx.id); - info!(%tx.id, "Sent tx to BitcoinDA"); - prev_utxo = Some(UTXO { - tx_id: tx.id, - vout: 0, - script_pubkey: tx.tx.output[0].script_pubkey.to_hex_string(), - address: None, - amount: tx.tx.output[0].value.to_sat(), - confirmations: 0, - spendable: true, - solvable: true, - }); - - let _ = request.notify.send(Ok(tx_id)); - } - Err(e) => { - error!(?e, "Failed to send transaction to DA layer"); - tokio::time::sleep(Duration::from_secs(1)).await; - continue; + while let Some(request_opt) = rx.recv().await { + match request_opt { + Some(request) => { + trace!("A new request is received"); + let prev = prev_utxo.take(); + loop { + // Build and send tx with retries: + let fee_sat_per_vbyte = match self.get_fee_rate().await { + Ok(rate) => rate, + Err(e) => { + error!(?e, "Failed to call get_fee_rate. Retrying..."); + tokio::time::sleep(Duration::from_secs(1)).await; + continue; + } + }; + match self + .send_transaction_with_fee_rate( + prev.clone(), + request.da_data.clone(), + fee_sat_per_vbyte, + ) + .await + { + Ok(tx) => { + let tx_id = TxidWrapper(tx.id); + info!(%tx.id, "Sent tx to BitcoinDA"); + prev_utxo = Some(UTXO { + tx_id: tx.id, + vout: 0, + script_pubkey: tx.tx.output[0] + .script_pubkey + .to_hex_string(), + address: None, + amount: tx.tx.output[0].value.to_sat(), + confirmations: 0, + spendable: true, + solvable: true, + }); + + let _ = request.notify.send(Ok(tx_id)); + } + Err(e) => { + error!(?e, "Failed to send transaction to DA layer"); + tokio::time::sleep(Duration::from_secs(1)).await; + continue; + } + } + break; } } - break; + + None => { + info!("Shutdown signal received. Stopping BitcoinDA queue."); + break; + } } } @@ -858,16 +869,16 @@ impl DaService for BitcoinService { ) -> Result<::TransactionId, Self::Error> { let queue = self.get_send_transaction_queue(); let (tx, rx) = oneshot_channel(); - queue.send(SenderWithNotifier { + queue.send(Some(SenderWithNotifier { da_data, notify: tx, - })?; + }))?; rx.await? } fn get_send_transaction_queue( &self, - ) -> UnboundedSender> { + ) -> UnboundedSender>> { self.inscribes_queue.clone() } diff --git a/crates/fullnode/src/da_block_handler.rs b/crates/fullnode/src/da_block_handler.rs index 365ea9b7a..b9441f876 100644 --- a/crates/fullnode/src/da_block_handler.rs +++ b/crates/fullnode/src/da_block_handler.rs @@ -242,9 +242,10 @@ where let end_l2_height = sequencer_commitment.l2_end_block_number; tracing::info!( - "Processing sequencer commitment. L2 Range = {}-{}.", + "Processing sequencer commitment for L2 Range = {}-{} at L1 height {}.", start_l2_height, end_l2_height, + l1_block.header().height(), ); // Traverse each item's field of vector of transactions, put them in merkle tree diff --git a/crates/prover/src/da_block_handler.rs b/crates/prover/src/da_block_handler.rs index 3eb47675d..034abd17d 100644 --- a/crates/prover/src/da_block_handler.rs +++ b/crates/prover/src/da_block_handler.rs @@ -182,6 +182,10 @@ where l1_block.header().height(), ); + // Make sure all sequencer commitments are stored in ascending order. + // We sort before checking ranges to prevent substraction errors. + sequencer_commitments.sort(); + // If the L2 range does not exist, we break off the local loop getting back to // the outer loop / select to make room for other tasks to run. // We retry the L1 block there as well. @@ -197,9 +201,6 @@ where let should_prove = self.prover_config.proof_sampling_number == 0 || rand::thread_rng().gen_range(0..self.prover_config.proof_sampling_number) == 0; - // Make sure all sequencer commitments are stored in ascending order. - sequencer_commitments.sort(); - let (sequencer_commitments, preproven_commitments) = filter_out_proven_commitments(&self.ledger_db, &sequencer_commitments)?; diff --git a/crates/sequencer/src/sequencer.rs b/crates/sequencer/src/sequencer.rs index 9cd2ce007..27c6b3d84 100644 --- a/crates/sequencer/src/sequencer.rs +++ b/crates/sequencer/src/sequencer.rs @@ -692,7 +692,7 @@ where let request = SenderWithNotifier { da_data, notify }; self.da_service .get_send_transaction_queue() - .send(request) + .send(Some(request)) .map_err(|_| anyhow!("Bitcoin service already stopped!"))?; info!( diff --git a/crates/sovereign-sdk/adapters/mock-da/src/service.rs b/crates/sovereign-sdk/adapters/mock-da/src/service.rs index 4b72d09aa..ab97c65a9 100644 --- a/crates/sovereign-sdk/adapters/mock-da/src/service.rs +++ b/crates/sovereign-sdk/adapters/mock-da/src/service.rs @@ -442,11 +442,11 @@ impl DaService for MockDaService { fn get_send_transaction_queue( &self, - ) -> UnboundedSender> { - let (tx, mut rx) = unbounded_channel::>(); + ) -> UnboundedSender>> { + let (tx, mut rx) = unbounded_channel::>>(); let this = self.clone(); tokio::spawn(async move { - while let Some(req) = rx.recv().await { + while let Some(Some(req)) = rx.recv().await { let res = this.send_transaction(req.da_data).await; let _ = req.notify.send(res); } diff --git a/crates/sovereign-sdk/rollup-interface/src/node/services/da.rs b/crates/sovereign-sdk/rollup-interface/src/node/services/da.rs index 752230cf4..9a1052e36 100644 --- a/crates/sovereign-sdk/rollup-interface/src/node/services/da.rs +++ b/crates/sovereign-sdk/rollup-interface/src/node/services/da.rs @@ -140,7 +140,7 @@ pub trait DaService: Send + Sync + 'static { /// A tx part of the queue to send transactions in order fn get_send_transaction_queue( &self, - ) -> UnboundedSender> { + ) -> UnboundedSender>> { unimplemented!() }