darkfid/task/garbage_collect: handle unreferenced sled trees

bin/darkfid/src/registry/mod.rs

@@ -215,6 +215,9 @@ impl DarkfiMinersRegistry {
     /// Create a registry record for provided wallet config. If the
     /// record already exists return its template, otherwise create its
     /// current template based on provided validator state.
+    ///
+    /// Note: Always remember to purge new trees from the database if
+    /// not needed.
     async fn create_template(
         &self,
         validator: &ValidatorPtr,
@@ -242,9 +245,6 @@ impl DarkfiMinersRegistry {
         )
         .await;
 
-        // Drop new trees opened by the forks' overlay
-        extended_fork.overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
-
         // Check result
         let block_template = result?;
 
@@ -344,6 +344,9 @@ impl DarkfiMinersRegistry {
 
     /// Refresh outdated jobs in the provided registry maps based on
     /// provided validator state.
+    ///
+    /// Note: Always remember to purge new trees from the database if
+    /// not needed.
     pub async fn refresh_jobs(
         &self,
         block_templates: &mut HashMap<String, BlockTemplate>,
@@ -426,9 +429,6 @@ impl DarkfiMinersRegistry {
             )
             .await;
 
-            // Drop new trees opened by the forks' overlay
-            extended_fork.overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
-
             // Check result
             *block_template = result?;
 

bin/darkfid/src/task/garbage_collect.rs

@@ -26,6 +26,11 @@ use crate::DarkfiNodePtr;
 pub async fn garbage_collect_task(node: DarkfiNodePtr) -> Result<()> {
     info!(target: "darkfid::task::garbage_collect_task", "Starting garbage collection task...");
 
+    // Purge all unreferenced contract trees from the database
+    if let Err(e) = node.validator.consensus.purge_unreferenced_trees().await {
+        error!(target: "darkfid::task::garbage_collect_task", "Purging unreferenced contract trees from the database failed: {e}");
+    }
+
     // Grab all current unproposed transactions.  We verify them in batches,
     // to not load them all in memory.
     let (mut last_checked, mut txs) =
@@ -40,6 +45,12 @@ pub async fn garbage_collect_task(node: DarkfiNodePtr) -> Result<()> {
             }
         };
 
+    // Check if we have transactions to process
+    if txs.is_empty() {
+        info!(target: "darkfid::task::garbage_collect_task", "Garbage collection finished successfully!");
+        return Ok(())
+    }
+
     while !txs.is_empty() {
         // Verify each one against current forks
         for tx in txs {
@@ -158,6 +169,11 @@ pub async fn garbage_collect_task(node: DarkfiNodePtr) -> Result<()> {
         };
     }
 
+    // Purge all unreferenced contract trees from the database again
+    if let Err(e) = node.validator.consensus.purge_unreferenced_trees().await {
+        error!(target: "darkfid::task::garbage_collect_task", "Purging unreferenced contract trees from the database failed: {e}");
+    }
+
     info!(target: "darkfid::task::garbage_collect_task", "Garbage collection finished successfully!");
     Ok(())
 }

bin/darkfid/src/task/unknown_proposal.rs

@@ -228,13 +228,15 @@ async fn handle_unknown_proposal(
     false
 }
 
-/// Auxiliary function to handle a potential reorg.
-/// We first find our last common block with the peer,
-/// then grab the header sequence from that block until
-/// the proposal and check if it ranks higher than our
+/// Auxiliary function to handle a potential reorg. We first find our
+/// last common block with the peer, then grab the header sequence from
+/// that block until the proposal and check if it ranks higher than our
 /// current best ranking fork, to perform a reorg.
-/// Returns a boolean flag indicate if we should ban the
-/// channel.
+///
+/// Returns a boolean flag indicate if we should ban the channel.
+///
+/// Note: Always remember to purge new trees from the database if not
+/// needed.
 async fn handle_reorg(
     validator: &ValidatorPtr,
     p2p: &P2pPtr,
@@ -501,7 +503,6 @@ async fn handle_reorg(
         Ok(i) => i,
         Err(e) => {
             error!(target: "darkfid::task::handle_reorg", "Retrieving state inverse diffs failed: {e}");
-            peer_fork.purge_new_trees();
             return false
         }
     };
@@ -510,7 +511,6 @@ async fn handle_reorg(
             peer_fork.overlay.lock().unwrap().overlay.lock().unwrap().add_diff(inverse_diff)
         {
             error!(target: "darkfid::task::handle_reorg", "Applying inverse diff failed: {e}");
-            peer_fork.purge_new_trees();
             return false
         }
     }
@@ -522,7 +522,6 @@ async fn handle_reorg(
         Ok(d) => d,
         Err(e) => {
             error!(target: "darkfid::task::handle_reorg", "Generate full inverse diff failed: {e}");
-            peer_fork.purge_new_trees();
             return false
         }
     };
@@ -545,7 +544,6 @@ async fn handle_reorg(
         let request = ForkProposalsRequest { headers: batch.clone(), fork_header: proposal.hash };
         if let Err(e) = channel.send(&request).await {
             debug!(target: "darkfid::task::handle_reorg", "Channel send failed: {e}");
-            peer_fork.purge_new_trees();
             return true
         };
 
@@ -557,7 +555,6 @@ async fn handle_reorg(
             Ok(r) => r,
             Err(e) => {
                 debug!(target: "darkfid::task::handle_reorg", "Asking peer for proposals sequence failed: {e}");
-                peer_fork.purge_new_trees();
                 return true
             }
         };
@@ -566,7 +563,6 @@ async fn handle_reorg(
         // Response sequence must be the same length as the one requested
         if response.proposals.len() != batch.len() {
             debug!(target: "darkfid::task::handle_reorg", "Peer responded with a different proposals sequence length");
-            peer_fork.purge_new_trees();
             return true
         }
 
@@ -577,7 +573,6 @@ async fn handle_reorg(
             // Validate its the proposal we requested
             if peer_proposal.hash != batch[peer_proposal_index] {
                 error!(target: "darkfid::task::handle_reorg", "Peer responded with a differend proposal: {} - {}", batch[peer_proposal_index], peer_proposal.hash);
-                peer_fork.purge_new_trees();
                 return true
             }
 
@@ -592,7 +587,6 @@ async fn handle_reorg(
             // Append proposal
             if let Err(e) = peer_fork.append_proposal(peer_proposal).await {
                 error!(target: "darkfid::task::handle_reorg", "Appending proposal failed: {e}");
-                peer_fork.purge_new_trees();
                 return true
             }
         }
@@ -613,7 +607,6 @@ async fn handle_reorg(
     // Append trigger proposal
     if let Err(e) = peer_fork.append_proposal(proposal).await {
         error!(target: "darkfid::task::handle_reorg", "Appending proposal failed: {e}");
-        peer_fork.purge_new_trees();
         return true
     }
 
@@ -623,7 +616,6 @@ async fn handle_reorg(
         Ok(i) => i,
         Err(e) => {
             debug!(target: "darkfid::task::handle_reorg", "Retrieving best fork index failed: {e}");
-            peer_fork.purge_new_trees();
             return false
         }
     };
@@ -633,7 +625,6 @@ async fn handle_reorg(
             peer_fork.hashes_rank <= best_fork.hashes_rank)
     {
         info!(target: "darkfid::task::handle_reorg", "Peer fork ranks lower than our current best fork, skipping...");
-        peer_fork.purge_new_trees();
         drop(forks);
         return true
     }
@@ -650,7 +641,6 @@ async fn handle_reorg(
         .apply_diff(&peer_fork.diffs.remove(0))
     {
         error!(target: "darkfid::task::handle_reorg", "Applying full inverse diff failed: {e}");
-        peer_fork.purge_new_trees();
         return false
     };
     *validator.consensus.module.write().await = module;

src/validator/consensus.rs

@@ -16,14 +16,14 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-use std::collections::{HashMap, HashSet};
+use std::collections::{BTreeSet, HashMap};
 
 use darkfi_sdk::{crypto::MerkleTree, tx::TransactionHash};
-use darkfi_serial::{async_trait, SerialDecodable, SerialEncodable};
+use darkfi_serial::{async_trait, deserialize, SerialDecodable, SerialEncodable};
 use num_bigint::BigUint;
-use sled_overlay::database::SledDbOverlayStateDiff;
+use sled_overlay::{database::SledDbOverlayStateDiff, sled::IVec};
 use smol::lock::RwLock;
-use tracing::{debug, error, info, warn};
+use tracing::{debug, info, warn};
 
 use crate::{
     blockchain::{
@@ -500,12 +500,15 @@ impl Consensus {
         Ok((last.block.header.height, last.hash))
     }
 
-    /// Auxiliary function to purge current forks and reset the ones starting
-    /// with the provided prefix, excluding provided confirmed fork.
-    /// Additionally, remove confirmed transactions from the forks mempools,
-    /// along with the unporposed transactions sled trees.
-    /// This function assumes that the prefix blocks have already been appended
-    /// to canonical chain from the confirmed fork.
+    /// Auxiliary function to purge current forks and reset the ones
+    /// starting with the provided prefix, excluding provided confirmed
+    /// fork. Additionally, remove confirmed transactions from the
+    /// forks mempools. This function assumes that the prefix blocks
+    /// have already been appended to canonical chain from the
+    /// confirmed fork.
+    ///
+    /// Note: Always remember to purge new trees from the database if
+    /// not needed.
     pub async fn reset_forks(
         &self,
         prefix: &[HeaderHash],
@@ -517,40 +520,18 @@ impl Consensus {
 
         // Find all the forks that start with the provided prefix,
         // excluding confirmed fork index, and remove their prefixed
-        // proposals, and their corresponding diffs.
-        // If the fork is not starting with the provided prefix,
-        // drop it. Additionally, keep track of all the referenced
-        // trees in overlays that are valid.
+        // proposals, and their corresponding diffs. If the fork is not
+        // starting with the provided prefix, drop it.
         let excess = prefix.len();
         let prefix_last_index = excess - 1;
         let prefix_last = prefix.last().unwrap();
         let mut keep = vec![true; forks.len()];
-        let mut referenced_trees = HashSet::new();
-        let mut referenced_txs = HashSet::new();
         let confirmed_txs_hashes: Vec<TransactionHash> =
             confirmed_txs.iter().map(|tx| tx.hash()).collect();
         for (index, fork) in forks.iter_mut().enumerate() {
             if &index == confirmed_fork_index {
-                // Store its tree references
-                let fork_overlay = fork.overlay.lock().unwrap();
-                let overlay = fork_overlay.overlay.lock().unwrap();
-                for tree in &overlay.state.initial_tree_names {
-                    referenced_trees.insert(tree.clone());
-                }
-                for tree in &overlay.state.new_tree_names {
-                    referenced_trees.insert(tree.clone());
-                }
-                for tree in overlay.state.dropped_trees.keys() {
-                    referenced_trees.insert(tree.clone());
-                }
                 // Remove confirmed proposals txs from fork's mempool
                 fork.mempool.retain(|tx| !confirmed_txs_hashes.contains(tx));
-                // Store its txs references
-                for tx in &fork.mempool {
-                    referenced_txs.insert(*tx);
-                }
-                drop(overlay);
-                drop(fork_overlay);
                 continue
             }
 
@@ -564,10 +545,6 @@ impl Consensus {
 
             // Remove confirmed proposals txs from fork's mempool
             fork.mempool.retain(|tx| !confirmed_txs_hashes.contains(tx));
-            // Store its txs references
-            for tx in &fork.mempool {
-                referenced_txs.insert(*tx);
-            }
 
             // Remove the commited differences
             let rest_proposals = fork.proposals.split_off(excess);
@@ -578,59 +555,6 @@ impl Consensus {
             for diff in diffs.iter_mut() {
                 fork.overlay.lock().unwrap().overlay.lock().unwrap().remove_diff(diff);
             }
-
-            // Store its tree references
-            let fork_overlay = fork.overlay.lock().unwrap();
-            let overlay = fork_overlay.overlay.lock().unwrap();
-            for tree in &overlay.state.initial_tree_names {
-                referenced_trees.insert(tree.clone());
-            }
-            for tree in &overlay.state.new_tree_names {
-                referenced_trees.insert(tree.clone());
-            }
-            for tree in overlay.state.dropped_trees.keys() {
-                referenced_trees.insert(tree.clone());
-            }
-            drop(overlay);
-            drop(fork_overlay);
-        }
-
-        // Find the trees and pending txs that are no longer referenced by valid forks
-        let mut dropped_trees = HashSet::new();
-        let mut dropped_txs = HashSet::new();
-        for (index, fork) in forks.iter_mut().enumerate() {
-            if keep[index] {
-                continue
-            }
-            for tx in &fork.mempool {
-                if !referenced_txs.contains(tx) {
-                    dropped_txs.insert(*tx);
-                }
-            }
-            let fork_overlay = fork.overlay.lock().unwrap();
-            let overlay = fork_overlay.overlay.lock().unwrap();
-            for tree in &overlay.state.initial_tree_names {
-                if !referenced_trees.contains(tree) {
-                    dropped_trees.insert(tree.clone());
-                }
-            }
-            for tree in &overlay.state.new_tree_names {
-                if !referenced_trees.contains(tree) {
-                    dropped_trees.insert(tree.clone());
-                }
-            }
-            for tree in overlay.state.dropped_trees.keys() {
-                if !referenced_trees.contains(tree) {
-                    dropped_trees.insert(tree.clone());
-                }
-            }
-            drop(overlay);
-            drop(fork_overlay);
-        }
-
-        // Drop unreferenced trees from the database
-        for tree in dropped_trees {
-            self.blockchain.sled_db.drop_tree(tree)?;
         }
 
         // Drop invalid forks
@@ -640,9 +564,6 @@ impl Consensus {
         // Remove confirmed proposals txs from the unporposed txs sled tree
         self.blockchain.remove_pending_txs_hashes(&confirmed_txs_hashes)?;
 
-        // Remove unreferenced txs from the unporposed txs sled tree
-        self.blockchain.remove_pending_txs_hashes(&Vec::from_iter(dropped_txs))?;
-
         // Drop forks lock
         drop(forks);
 
@@ -700,6 +621,50 @@ impl Consensus {
 
         Ok(())
     }
+
+    /// Auxiliary function to purge all unreferenced contract trees
+    /// from the database.
+    pub async fn purge_unreferenced_trees(&self) -> Result<()> {
+        // Grab a lock over current forks
+        let lock = self.forks.read().await;
+
+        // Keep track of referenced trees
+        let mut referenced_trees = BTreeSet::new();
+
+        // Check if we have forks
+        if lock.is_empty() {
+            // If no forks exist, build a new one so we retrieve the
+            // native/protected trees references.
+            let fork = Fork::new(self.blockchain.clone(), self.module.read().await.clone()).await?;
+            fork.referenced_trees(&mut referenced_trees);
+        } else {
+            // Iterate over current forks to retrieve referenced trees
+            for fork in lock.iter() {
+                fork.referenced_trees(&mut referenced_trees);
+            }
+        }
+
+        // Retrieve current database trees
+        let current_trees = self.blockchain.sled_db.tree_names();
+
+        // Iterate over current database trees and drop unreferenced
+        // contracts ones.
+        for tree in current_trees {
+            // Check if its referenced
+            if referenced_trees.contains(&tree) {
+                continue
+            }
+
+            // Check if its a contract tree pointer
+            let Ok(tree) = deserialize::<[u8; 32]>(&tree) else { continue };
+
+            // Drop it
+            debug!(target: "validator::consensus::purge_unreferenced_trees", "Dropping unreferenced tree: {}", blake3::Hash::from(tree));
+            self.blockchain.sled_db.drop_tree(tree)?;
+        }
+
+        Ok(())
+    }
 }
 
 /// This struct represents a block proposal, used for consensus.
@@ -726,9 +691,10 @@ impl From<Proposal> for BlockInfo {
 
 /// Struct representing a forked blockchain state.
 ///
-/// An overlay over the original blockchain is used, containing all pending to-write
-/// records. Additionally, each fork keeps a vector of valid pending transactions hashes,
-/// in order of receival, and the proposals hashes sequence, for validations.
+/// An overlay over the original blockchain is used, containing all
+/// pending to-write records. Additionally, each fork keeps a vector of
+/// valid pending transactions hashes, in order of receival, and the
+/// proposals hashes sequence, for validations.
 #[derive(Clone)]
 pub struct Fork {
     /// Canonical (confirmed) blockchain
@@ -828,7 +794,8 @@ impl Fork {
     /// Auxiliary function to retrieve unproposed valid transactions,
     /// along with their total gas used and total paid fees.
     ///
-    /// Note: Always remember to purge new trees from the overlay if not needed.
+    /// Note: Always remember to purge new trees from the database if
+    /// not needed.
     pub async fn unproposed_txs(
         &self,
         verifying_block_height: u32,
@@ -885,7 +852,6 @@ impl Fork {
                 Ok(gas_values) => gas_values,
                 Err(e) => {
                     debug!(target: "validator::consensus::unproposed_txs", "Transaction verification failed: {e}");
-                    self.overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
                     self.overlay.lock().unwrap().revert_to_checkpoint()?;
                     continue
                 }
@@ -903,7 +869,6 @@ impl Fork {
                     target: "validator::consensus::unproposed_txs",
                     "Retrieving transaction {tx} would exceed configured unproposed transaction gas limit: {accumulated_gas_usage} - {BLOCK_GAS_LIMIT}"
                 );
-                self.overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
                 self.overlay.lock().unwrap().revert_to_checkpoint()?;
                 break
             }
@@ -919,9 +884,10 @@ impl Fork {
         Ok((unproposed_txs, total_gas_used, total_gas_paid))
     }
 
-    /// Auxiliary function to create a full clone using BlockchainOverlay::full_clone.
-    /// Changes to this copy don't affect original fork overlay records, since underlying
-    /// overlay pointer have been updated to the cloned one.
+    /// Auxiliary function to create a full clone using
+    /// BlockchainOverlay::full_clone. Changes to this copy don't
+    /// affect original fork overlay records, since underlying overlay
+    /// pointer have been updated to the cloned one.
     pub fn full_clone(&self) -> Result<Self> {
         let blockchain = self.blockchain.clone();
         let overlay = self.overlay.lock().unwrap().full_clone()?;
@@ -966,11 +932,31 @@ impl Fork {
         Ok(())
     }
 
-    /// Auxiliary function to purge all new trees from the fork
-    /// overlay.
-    pub fn purge_new_trees(&self) {
-        if let Err(e) = self.overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees() {
-            error!(target: "validator::consensus::fork::purge_new_trees", "Purging new trees in the overlay failed: {e}");
+    /// Auxiliary function to retrieve all referenced trees from the
+    /// fork overlay and insert them to provided `BTreeSet`.
+    pub fn referenced_trees(&self, trees: &mut BTreeSet<IVec>) {
+        // Grab its current overlay
+        let fork_overlay = self.overlay.lock().unwrap();
+        let overlay = fork_overlay.overlay.lock().unwrap();
+
+        // Retrieve its initial trees
+        for initial_tree in &overlay.state.initial_tree_names {
+            trees.insert(initial_tree.clone());
+        }
+
+        // Retrieve its new trees
+        for new_tree in &overlay.state.new_tree_names {
+            trees.insert(new_tree.clone());
+        }
+
+        // Retrieve its dropped trees
+        for dropped_tree in overlay.state.dropped_trees.keys() {
+            trees.insert(dropped_tree.clone());
+        }
+
+        // Retrieve its protected trees
+        for protected_tree in &overlay.state.protected_tree_names {
+            trees.insert(protected_tree.clone());
         }
     }
 }

src/validator/mod.rs

@@ -140,10 +140,13 @@ impl Validator {
         Ok(state)
     }
 
-    /// Auxiliary function to compute provided transaction's required fee,
-    /// against current best fork.
-    /// The function takes a boolean called `verify_fee` to overwrite
-    /// the nodes configured `verify_fees` flag.
+    /// Auxiliary function to compute provided transaction's required
+    /// fee, against current best fork. The function takes a boolean
+    /// called `verify_fee` to overwrite the nodes configured
+    /// `verify_fees` flag.
+    ///
+    /// Note: Always remember to purge new trees from the database if
+    /// not needed.
     pub async fn calculate_fee(&self, tx: &Transaction, verify_fee: bool) -> Result<u64> {
         // Grab the best fork to verify against
         let forks = self.consensus.forks.read().await;
@@ -171,14 +174,14 @@ impl Validator {
         )
         .await?;
 
-        // Purge new trees
-        fork.overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
-
         Ok(compute_fee(&verify_result.total_gas_used()))
     }
 
-    /// The node retrieves a transaction, validates its state transition,
-    /// and appends it to the pending txs store.
+    /// The node retrieves a transaction, validates its state
+    /// transition, and appends it to the pending txs store.
+    ///
+    /// Note: Always remember to purge new trees from the database if
+    /// not needed.
     pub async fn append_tx(&self, tx: &Transaction, write: bool) -> Result<()> {
         let tx_hash = tx.hash();
 
@@ -218,9 +221,6 @@ impl Validator {
             )
             .await;
 
-            // Purge new trees
-            fork_clone.overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
-
             // Handle response
             match verify_result {
                 Ok(_) => {}
@@ -253,7 +253,11 @@ impl Validator {
         Ok(())
     }
 
-    /// The node removes invalid transactions from the pending txs store.
+    /// The node removes invalid transactions from the pending txs
+    /// store.
+    ///
+    /// Note: Always remember to purge new trees from the database if
+    /// not needed.
     pub async fn purge_pending_txs(&self) -> Result<()> {
         info!(target: "validator::purge_pending_txs", "Removing invalid transactions from pending transactions store...");
 
@@ -292,9 +296,6 @@ impl Validator {
                 )
                 .await;
 
-                // Purge new trees
-                fork_clone.overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
-
                 // Handle response
                 match verify_result {
                     Ok(_) => {
@@ -419,13 +420,17 @@ impl Validator {
         Ok(confirmed_blocks)
     }
 
-    /// Apply provided set of [`BlockInfo`] without doing formal verification.
-    /// A set of [`HeaderHash`] is also provided, to verify that the provided
-    /// block hash matches the expected header one.
-    /// Note: this function should only be used for blocks received using a
-    /// checkpoint, since in that case we enforce the node to follow the sequence,
-    /// assuming all its blocks are valid. Additionally, it will update
-    /// any forks to a single empty one, holding the updated module.
+    /// Apply provided set of [`BlockInfo`] without doing formal
+    /// verification. A set of [`HeaderHash`] is also provided, to
+    /// verify that the provided block hash matches the expected header
+    /// one.
+    ///
+    /// Note: this function should only be used for blocks received
+    /// using a checkpoint, since in that case we enforce the node to
+    /// follow the sequence, assuming all its blocks are valid.
+    /// Additionally, it will update any forks to a single empty one,
+    /// holding the updated module. Always remember to purge new trees
+    /// from the database if not needed.
     pub async fn add_checkpoint_blocks(
         &self,
         blocks: &[BlockInfo],
@@ -466,7 +471,6 @@ impl Validator {
                 Err(Error::BlockAlreadyExists(_)) => continue,
                 Err(e) => {
                     error!(target: "validator::add_checkpoint_blocks", "Erroneous block found in set: {e}");
-                    overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
                     return Err(Error::BlockIsInvalid(block.hash().as_string()))
                 }
             };
@@ -530,9 +534,12 @@ impl Validator {
         Ok(())
     }
 
-    /// Validate a set of [`BlockInfo`] in sequence and apply them if all are valid.
-    /// Note: this function should only be used in tests when we don't want to
-    /// perform consensus logic.
+    /// Validate a set of [`BlockInfo`] in sequence and apply them if
+    /// all are valid.
+    ///
+    /// Note: this function should only be used in tests when we don't
+    /// want to perform consensus logic and always remember to purge
+    /// new trees from the database if not needed.
     pub async fn add_test_blocks(&self, blocks: &[BlockInfo]) -> Result<()> {
         debug!(target: "validator::add_test_blocks", "Instantiating BlockchainOverlay");
         let overlay = BlockchainOverlay::new(&self.blockchain)?;
@@ -568,7 +575,6 @@ impl Validator {
                 }
                 Err(e) => {
                     error!(target: "validator::add_test_blocks", "Erroneous block found in set: {e}");
-                    overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
                     return Err(Error::BlockIsInvalid(block.hash().as_string()))
                 }
             };
@@ -632,14 +638,18 @@ impl Validator {
         Ok(())
     }
 
-    /// Validate a set of [`Transaction`] in sequence and apply them if all are valid.
-    /// In case any of the transactions fail, they will be returned to the caller.
-    /// The function takes a boolean called `write` which tells it to actually write
-    /// the state transitions to the database, and a boolean called `verify_fees` to
+    /// Validate a set of [`Transaction`] in sequence and apply them if
+    /// all are valid. In case any of the transactions fail, they will
+    /// be returned to the caller. The function takes a boolean called
+    /// `write` which tells it to actually write the state transitions
+    /// to the database, and a boolean called `verify_fees` to
     /// overwrite the nodes configured `verify_fees` flag.
     ///
-    /// Returns the total gas used and total paid fees for the given transactions.
-    /// Note: this function should only be used in tests.
+    /// Returns the total gas used and total paid fees for the given
+    /// transactions.
+    ///
+    /// Note: This function should only be used in tests and always
+    /// remember to purge new trees from the database if not needed.
     pub async fn add_test_transactions(
         &self,
         txs: &[Transaction],
@@ -665,16 +675,13 @@ impl Validator {
         let lock = overlay.lock().unwrap();
         let mut overlay = lock.overlay.lock().unwrap();
 
-        if let Err(e) = verify_result {
-            overlay.purge_new_trees()?;
-            return Err(e)
-        }
-
-        let gas_values = verify_result.unwrap();
+        let gas_values = match verify_result {
+            Ok(v) => v,
+            Err(e) => return Err(e),
+        };
 
         if !write {
             debug!(target: "validator::add_transactions", "Skipping apply of state updates because write=false");
-            overlay.purge_new_trees()?;
             return Ok(gas_values)
         }
 
@@ -683,11 +690,13 @@ impl Validator {
         Ok(gas_values)
     }
 
-    /// Validate a producer `Transaction` and apply it if valid.
-    /// In case the transactions fail, ir will be returned to the caller.
-    /// The function takes a boolean called `write` which tells it to actually write
-    /// the state transitions to the database.
-    /// This should be only used for test purposes.
+    /// Validate a producer `Transaction` and apply it if valid. In
+    /// case the transactions fail, ir will be returned to the caller.
+    /// The function takes a boolean called `write` which tells it to
+    /// actually write the state transitions to the database.
+    ///
+    /// Note: This function should only be used in tests and always
+    /// remember to purge new trees from the database if not needed.
     pub async fn add_test_producer_transaction(
         &self,
         tx: &Transaction,
@@ -717,13 +726,11 @@ impl Validator {
         let mut overlay = lock.overlay.lock().unwrap();
         if !erroneous_txs.is_empty() {
             warn!(target: "validator::add_test_producer_transaction", "Erroneous transactions found in set");
-            overlay.purge_new_trees()?;
             return Err(TxVerifyFailed::ErroneousTxs(erroneous_txs).into())
         }
 
         if !write {
             debug!(target: "validator::add_test_producer_transaction", "Skipping apply of state updates because write=false");
-            overlay.purge_new_trees()?;
             return Ok(())
         }
 
@@ -735,6 +742,9 @@ impl Validator {
     /// Retrieve all existing blocks and try to apply them
     /// to an in memory overlay to verify their correctness.
     /// Be careful as this will try to load everything in memory.
+    ///
+    /// Note: Always remember to purge new trees from the database if
+    /// not needed.
     pub async fn validate_blockchain(
         &self,
         pow_target: u32,
@@ -789,7 +799,6 @@ impl Validator {
                 verify_block(&overlay, &diffs, &module, &block, &previous, self.verify_fees).await
             {
                 error!(target: "validator::validate_blockchain", "Erroneous block found in set: {e}");
-                overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
                 return Err(Error::BlockIsInvalid(block.hash().as_string()))
             };
 

src/validator/verification.rs

@@ -51,7 +51,11 @@ use crate::{
     Error, Result,
 };
 
-/// Verify given genesis [`BlockInfo`], and apply it to the provided overlay.
+/// Verify given genesis [`BlockInfo`], and apply it to the provided
+/// overlay.
+///
+/// Note: Always remember to purge new trees from the database if not
+/// needed.
 pub async fn verify_genesis_block(
     overlay: &BlockchainOverlayPtr,
     diffs: &[SledDbOverlayStateDiff],
@@ -105,7 +109,6 @@ pub async fn verify_genesis_block(
             target: "validator::verification::verify_genesis_block",
             "[VALIDATOR] Erroneous transactions found in set",
         );
-        overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
         return Err(e)
     }
 
@@ -206,6 +209,9 @@ pub fn validate_blockchain(
 }
 
 /// Verify given [`BlockInfo`], and apply it to the provided overlay.
+///
+/// Note: Always remember to purge new trees from the database if not
+/// needed.
 pub async fn verify_block(
     overlay: &BlockchainOverlayPtr,
     diffs: &[SledDbOverlayStateDiff],
@@ -233,7 +239,7 @@ pub async fn verify_block(
     // Verify transactions, exluding producer(last) one
     let mut tree = MerkleTree::new(1);
     let txs = &block.txs[..block.txs.len() - 1];
-    let e = verify_transactions(
+    if let Err(e) = verify_transactions(
         overlay,
         block.header.height,
         module.target,
@@ -241,13 +247,12 @@ pub async fn verify_block(
         &mut tree,
         verify_fees,
     )
-    .await;
-    if let Err(e) = e {
+    .await
+    {
         warn!(
             target: "validator::verification::verify_block",
             "[VALIDATOR] Erroneous transactions found in set",
         );
-        overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
         return Err(e)
     }
 
@@ -287,7 +292,11 @@ pub async fn verify_block(
     Ok(())
 }
 
-/// Verify given checkpoint [`BlockInfo`], and apply it to the provided overlay.
+/// Verify given checkpoint [`BlockInfo`], and apply it to the provided
+/// overlay.
+///
+/// Note: Always remember to purge new trees from the database if not
+/// needed.
 pub async fn verify_checkpoint_block(
     overlay: &BlockchainOverlayPtr,
     diffs: &[SledDbOverlayStateDiff],
@@ -317,13 +326,13 @@ pub async fn verify_checkpoint_block(
     // Apply transactions, excluding producer(last) one
     let mut tree = MerkleTree::new(1);
     let txs = &block.txs[..block.txs.len() - 1];
-    let e = apply_transactions(overlay, block.header.height, block_target, txs, &mut tree).await;
-    if let Err(e) = e {
+    if let Err(e) =
+        apply_transactions(overlay, block.header.height, block_target, txs, &mut tree).await
+    {
         warn!(
             target: "validator::verification::verify_checkpoint_block",
             "[VALIDATOR] Erroneous transactions found in set",
         );
-        overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
         return Err(e)
     }
 
@@ -953,12 +962,15 @@ pub async fn apply_transaction(
     Ok(())
 }
 
-/// Verify a set of [`Transaction`] in sequence and apply them if all are valid.
+/// Verify a set of [`Transaction`] in sequence and apply them if all
+/// are valid. In case any of the transactions fail, they will be
+/// returned to the caller as an error. If all transactions are valid,
+/// the function will return the total gas used and total paid fees
+/// from all the transactions. Additionally, their hash is appended to
+/// the provided Merkle tree.
 ///
-/// In case any of the transactions fail, they will be returned to the caller as an error.
-/// If all transactions are valid, the function will return the total gas used and total
-/// paid fees from all the transactions. Additionally, their hash is appended to the provided
-/// Merkle tree.
+/// Note: Always remember to purge new trees from the database if not
+/// needed.
 pub async fn verify_transactions(
     overlay: &BlockchainOverlayPtr,
     verifying_block_height: u32,
@@ -1007,7 +1019,6 @@ pub async fn verify_transactions(
             Err(e) => {
                 warn!(target: "validator::verification::verify_transactions", "Transaction verification failed: {e}");
                 erroneous_txs.push(tx.clone());
-                overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
                 overlay.lock().unwrap().revert_to_checkpoint()?;
                 continue
             }
@@ -1027,7 +1038,6 @@ pub async fn verify_transactions(
                 tx.hash()
             );
             erroneous_txs.push(tx.clone());
-            overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
             overlay.lock().unwrap().revert_to_checkpoint()?;
             break
         }
@@ -1087,6 +1097,9 @@ async fn apply_transactions(
 ///     1. Proposal hash matches the actual block one
 ///     2. Block is valid
 /// Additional validity rules can be applied.
+///
+/// Note: Always remember to purge new trees from the database if not
+/// needed.
 pub async fn verify_proposal(
     consensus: &Consensus,
     proposal: &Proposal,
@@ -1120,7 +1133,6 @@ pub async fn verify_proposal(
     .await
     {
         error!(target: "validator::verification::verify_proposal", "Erroneous proposal block found: {e}");
-        fork.overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
         return Err(Error::BlockIsInvalid(proposal.hash.as_string()))
     };
 
@@ -1133,6 +1145,9 @@ pub async fn verify_proposal(
 ///     1. Proposal hash matches the actual block one
 ///     2. Block is valid
 /// Additional validity rules can be applied.
+///
+/// Note: Always remember to purge new trees from the database if not
+/// needed.
 pub async fn verify_fork_proposal(
     fork: &mut Fork,
     proposal: &Proposal,
@@ -1163,7 +1178,6 @@ pub async fn verify_fork_proposal(
     .await
     {
         error!(target: "validator::verification::verify_fork_proposal", "Erroneous proposal block found: {e}");
-        fork.overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
         return Err(Error::BlockIsInvalid(proposal.hash.as_string()))
     };