From b0337cfaac92939db968231cc368b56836c2cf7e Mon Sep 17 00:00:00 2001 From: Brian Pearce Date: Thu, 31 Aug 2023 14:38:10 +0200 Subject: [PATCH] fix: handle out of sync errors when returning mempool transactions (#5701) Description --- Add a new error type and gracefully recover from possible runtime panic. Motivation and Context --- Currently, there is a minimal possibility that the collection of tx's is out sync. If this occurs the base node will likely crash. How Has This Been Tested? --- CI Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify --- base_layer/core/src/mempool/error.rs | 2 ++ base_layer/core/src/mempool/mempool.rs | 2 +- .../core/src/mempool/mempool_storage.rs | 19 +++++++++++------- .../core/src/mempool/reorg_pool/reorg_pool.rs | 20 +++++++++---------- .../unconfirmed_pool/unconfirmed_pool.rs | 12 +++++++---- 5 files changed, 32 insertions(+), 23 deletions(-) diff --git a/base_layer/core/src/mempool/error.rs b/base_layer/core/src/mempool/error.rs index 534cc4a52d..bd4539ca42 100644 --- a/base_layer/core/src/mempool/error.rs +++ b/base_layer/core/src/mempool/error.rs @@ -42,4 +42,6 @@ pub enum MempoolError { BlockingTaskError(#[from] JoinError), #[error("Internal error: {0}")] InternalError(String), + #[error("Mempool indexes out of sync: transaction exists in txs_by_signature but not in tx_by_key")] + IndexOutOfSync, } diff --git a/base_layer/core/src/mempool/mempool.rs b/base_layer/core/src/mempool/mempool.rs index fdfaacbd6f..97a3be262b 100644 --- a/base_layer/core/src/mempool/mempool.rs +++ b/base_layer/core/src/mempool/mempool.rs @@ -125,7 +125,7 @@ impl Mempool { &self, excess_sigs: Vec, ) -> Result<(Vec>, Vec), MempoolError> { - self.with_read_access(move |storage| Ok(storage.retrieve_by_excess_sigs(&excess_sigs))) + self.with_read_access(move |storage| storage.retrieve_by_excess_sigs(&excess_sigs)) .await } diff --git a/base_layer/core/src/mempool/mempool_storage.rs b/base_layer/core/src/mempool/mempool_storage.rs index 92fedd052a..b7365a8227 100644 --- a/base_layer/core/src/mempool/mempool_storage.rs +++ b/base_layer/core/src/mempool/mempool_storage.rs @@ -285,14 +285,19 @@ impl MempoolStorage { Ok(results.retrieved_transactions) } - pub fn retrieve_by_excess_sigs(&self, excess_sigs: &[PrivateKey]) -> (Vec>, Vec) { - let (found_txns, remaining) = self.unconfirmed_pool.retrieve_by_excess_sigs(excess_sigs); - let (found_published_transactions, remaining) = self.reorg_pool.retrieve_by_excess_sigs(&remaining); + pub fn retrieve_by_excess_sigs( + &self, + excess_sigs: &[PrivateKey], + ) -> Result<(Vec>, Vec), MempoolError> { + let (found_txns, remaining) = self.unconfirmed_pool.retrieve_by_excess_sigs(excess_sigs)?; - ( - found_txns.into_iter().chain(found_published_transactions).collect(), - remaining, - ) + match self.reorg_pool.retrieve_by_excess_sigs(&remaining) { + Ok((found_published_transactions, remaining)) => Ok(( + found_txns.into_iter().chain(found_published_transactions).collect(), + remaining, + )), + Err(e) => Err(e), + } } /// Check if the specified excess signature is found in the Mempool. diff --git a/base_layer/core/src/mempool/reorg_pool/reorg_pool.rs b/base_layer/core/src/mempool/reorg_pool/reorg_pool.rs index 22bb919813..8770f712c1 100644 --- a/base_layer/core/src/mempool/reorg_pool/reorg_pool.rs +++ b/base_layer/core/src/mempool/reorg_pool/reorg_pool.rs @@ -32,7 +32,7 @@ use tari_utilities::hex::Hex; use crate::{ blocks::Block, - mempool::shrink_hashmap::shrink_hashmap, + mempool::{shrink_hashmap::shrink_hashmap, MempoolError}, transactions::transaction_components::Transaction, }; @@ -144,7 +144,10 @@ impl ReorgPool { result } - pub fn retrieve_by_excess_sigs(&self, excess_sigs: &[PrivateKey]) -> (Vec>, Vec) { + pub fn retrieve_by_excess_sigs( + &self, + excess_sigs: &[PrivateKey], + ) -> Result<(Vec>, Vec), MempoolError> { // Hashset used to prevent duplicates let mut found = HashSet::new(); let mut remaining = Vec::new(); @@ -158,15 +161,10 @@ impl ReorgPool { let found = found .into_iter() - .map(|id| { - self.tx_by_key - .get(id) - .expect("mempool indexes out of sync: transaction exists in txs_by_signature but not in tx_by_key") - }) - .cloned() - .collect(); - - (found, remaining) + .map(|id| self.tx_by_key.get(id).cloned().ok_or(MempoolError::IndexOutOfSync)) + .collect::, _>>()?; + + Ok((found, remaining)) } /// Check if a transaction is stored in the ReorgPool diff --git a/base_layer/core/src/mempool/unconfirmed_pool/unconfirmed_pool.rs b/base_layer/core/src/mempool/unconfirmed_pool/unconfirmed_pool.rs index abf5519f75..e16cc3881c 100644 --- a/base_layer/core/src/mempool/unconfirmed_pool/unconfirmed_pool.rs +++ b/base_layer/core/src/mempool/unconfirmed_pool/unconfirmed_pool.rs @@ -37,6 +37,7 @@ use crate::{ shrink_hashmap::shrink_hashmap, unconfirmed_pool::UnconfirmedPoolError, FeePerGramStat, + MempoolError, }, transactions::{tari_amount::MicroMinotari, transaction_components::Transaction, weight::TransactionWeight}, }; @@ -401,7 +402,10 @@ impl UnconfirmedPool { } } - pub fn retrieve_by_excess_sigs(&self, excess_sigs: &[PrivateKey]) -> (Vec>, Vec) { + pub fn retrieve_by_excess_sigs( + &self, + excess_sigs: &[PrivateKey], + ) -> Result<(Vec>, Vec), MempoolError> { // Hashset used to prevent duplicates let mut found = HashSet::new(); let mut remaining = Vec::new(); @@ -419,11 +423,11 @@ impl UnconfirmedPool { self.tx_by_key .get(&id) .map(|tx| tx.transaction.clone()) - .expect("mempool indexes out of sync: transaction exists in txs_by_signature but not in tx_by_key") + .ok_or(MempoolError::IndexOutOfSync) }) - .collect(); + .collect::, _>>()?; - (found, remaining) + Ok((found, remaining)) } fn get_all_dependent_transactions(