From 5f340b0b47bbf6245cd834dd1564dd8bac57f9bd Mon Sep 17 00:00:00 2001 From: Lily Johnson Date: Thu, 31 Oct 2024 10:38:13 +0700 Subject: [PATCH] remove expect --- crates/astria-sequencer/src/app/mod.rs | 6 +-- .../src/mempool/benchmarks.rs | 2 +- crates/astria-sequencer/src/mempool/mod.rs | 37 ++++------------ .../src/mempool/transactions_container.rs | 42 ++++++++++++------- 4 files changed, 36 insertions(+), 51 deletions(-) diff --git a/crates/astria-sequencer/src/app/mod.rs b/crates/astria-sequencer/src/app/mod.rs index 99d13a115c..9cc5115110 100644 --- a/crates/astria-sequencer/src/app/mod.rs +++ b/crates/astria-sequencer/src/app/mod.rs @@ -582,11 +582,7 @@ impl App { let mut current_tx_group = Group::BundleableGeneral; // get copy of transactions to execute from mempool - let pending_txs = self - .mempool - .builder_queue(&self.state) - .await - .expect("failed to fetch pending transactions"); + let pending_txs = self.mempool.builder_queue(&self.state).await; let mut unused_count = pending_txs.len(); for (tx_hash, tx) in pending_txs { diff --git a/crates/astria-sequencer/src/mempool/benchmarks.rs b/crates/astria-sequencer/src/mempool/benchmarks.rs index 3caaa920d7..7d2bcbc3cd 100644 --- a/crates/astria-sequencer/src/mempool/benchmarks.rs +++ b/crates/astria-sequencer/src/mempool/benchmarks.rs @@ -206,7 +206,7 @@ fn builder_queue(bencher: divan::Bencher) { .with_inputs(|| init_mempool::()) .bench_values(move |mempool| { runtime.block_on(async { - mempool.builder_queue(&mock_state).await.unwrap(); + mempool.builder_queue(&mock_state).await; }); }); } diff --git a/crates/astria-sequencer/src/mempool/mod.rs b/crates/astria-sequencer/src/mempool/mod.rs index 68579620f4..cfdb1d6b93 100644 --- a/crates/astria-sequencer/src/mempool/mod.rs +++ b/crates/astria-sequencer/src/mempool/mod.rs @@ -294,7 +294,7 @@ impl Mempool { pub(crate) async fn builder_queue( &self, state: &S, - ) -> Result)>> { + ) -> Vec<([u8; 32], Arc)> { self.pending.read().await.builder_queue(state).await } @@ -649,10 +649,7 @@ mod tests { // grab building queue, should return transactions [1,2] since [0] was below and [4] is // gapped - let builder_queue = mempool - .builder_queue(&mock_state) - .await - .expect("failed to get builder queue"); + let builder_queue = mempool.builder_queue(&mock_state).await; // see contains first two transactions that should be pending assert_eq!(builder_queue[0].1.nonce(), 1, "nonce should be one"); @@ -682,10 +679,7 @@ mod tests { assert_eq!(mempool.len().await, 1); // see transaction [4] properly promoted - let mut builder_queue = mempool - .builder_queue(&mock_state) - .await - .expect("failed to get builder queue"); + let mut builder_queue = mempool.builder_queue(&mock_state).await; let (_, returned_tx) = builder_queue.pop().expect("should return last transaction"); assert_eq!(returned_tx.nonce(), 4, "nonce should be four"); } @@ -730,10 +724,7 @@ mod tests { 1, ); - let builder_queue = mempool - .builder_queue(&mock_state) - .await - .expect("failed to get builder queue"); + let builder_queue = mempool.builder_queue(&mock_state).await; assert_eq!( builder_queue.len(), 1, @@ -752,10 +743,7 @@ mod tests { mempool.run_maintenance(&mock_state, false).await; // see builder queue now contains them - let builder_queue = mempool - .builder_queue(&mock_state) - .await - .expect("failed to get builder queue"); + let builder_queue = mempool.builder_queue(&mock_state).await; assert_eq!( builder_queue.len(), 3, @@ -804,10 +792,7 @@ mod tests { 1, ); - let builder_queue = mempool - .builder_queue(&mock_state) - .await - .expect("failed to get builder queue"); + let builder_queue = mempool.builder_queue(&mock_state).await; assert_eq!( builder_queue.len(), 4, @@ -824,10 +809,7 @@ mod tests { mempool.run_maintenance(&mock_state, false).await; // see builder queue now contains single transactions - let builder_queue = mempool - .builder_queue(&mock_state) - .await - .expect("failed to get builder queue"); + let builder_queue = mempool.builder_queue(&mock_state).await; assert_eq!( builder_queue.len(), 1, @@ -847,10 +829,7 @@ mod tests { mempool.run_maintenance(&mock_state, false).await; - let builder_queue = mempool - .builder_queue(&mock_state) - .await - .expect("failed to get builder queue"); + let builder_queue = mempool.builder_queue(&mock_state).await; assert_eq!( builder_queue.len(), 3, diff --git a/crates/astria-sequencer/src/mempool/transactions_container.rs b/crates/astria-sequencer/src/mempool/transactions_container.rs index 95d17b00c1..3f9f501549 100644 --- a/crates/astria-sequencer/src/mempool/transactions_container.rs +++ b/crates/astria-sequencer/src/mempool/transactions_container.rs @@ -20,7 +20,6 @@ use astria_core::{ use astria_eyre::eyre::{ eyre, Result, - WrapErr as _, }; use tokio::time::{ Duration, @@ -773,7 +772,7 @@ impl PendingTransactions { pub(super) async fn builder_queue( &self, state: &S, - ) -> Result)>> { + ) -> Vec<([u8; 32], Arc)> { // Used to hold the values in Vec for sorting. struct QueueEntry { tx: Arc, @@ -784,10 +783,27 @@ impl PendingTransactions { let mut queue = Vec::with_capacity(self.len()); // Add all transactions to the queue. for (address, account_txs) in &self.txs { - let current_account_nonce = state - .get_account_nonce(address) - .await - .wrap_err("failed to fetch account nonce for builder queue")?; + let current_account_nonce = match state.get_account_nonce(address).await { + Ok(nonce) => nonce, + Err(error) => { + error!(address = %telemetry::display::base64(address), "failed to fetch account nonce for builder queue: {error:#}"); + // use the lowest nonce in pending as the current nonce, this should be the + // same value as the nonce returned by the state. the pending queue shouldn't + // be empty, if it is we can continue to the next account + if let Some(nonce) = account_txs + .txs() + .values() + .map(TimemarkedTransaction::nonce) + .min() + { + nonce + } else { + error!(address = %telemetry::display::base64(address), "pending queue is empty during builder queue step"); + continue; + } + } + }; + for ttx in account_txs.txs.values() { let priority = match ttx.priority(current_account_nonce) { Ok(priority) => priority, @@ -811,11 +827,11 @@ impl PendingTransactions { // Sort the queue and return the relevant data. Note that the sorted queue will be ordered // from lowest to highest priority, so we need to reverse the order before returning. queue.sort_unstable_by_key(|entry| entry.priority); - Ok(queue + queue .into_iter() .rev() .map(|entry| (entry.tx_hash, entry.tx)) - .collect()) + .collect() } } @@ -2011,10 +2027,7 @@ mod tests { ); // get builder queue - let builder_queue = pending_txs - .builder_queue(&mock_state) - .await - .expect("building builders queue should work"); + let builder_queue = pending_txs.builder_queue(&mock_state).await; assert_eq!( builder_queue.len(), 3, @@ -2291,10 +2304,7 @@ mod tests { // get the builder queue // note: the account nonces are set to zero when not initialized in the mock state - let builder_queue = pending_txs - .builder_queue(&mock_state) - .await - .expect("building builders queue should work"); + let builder_queue = pending_txs.builder_queue(&mock_state).await; // check that the transactions are in the expected order let (first_tx_hash, _) = builder_queue[0];