fix: fix new block handling of non-tip blocks (#4431)

Description
---
Currently, the base node cannot process a block that was received out of order (aka orphaned).
The `reconcile_block` function makes the assumption that the block is always on the current tip and tries to calculate the MMR roots of the block as a sanity check. The MMR roots can only be checked by a block that is built on the current local tip. 
This changes the `reconcile_block` function to first check orphan status, and if true, it will require the full block from the peer. 

This Pr also changes how the db lock is processed. Currently, it will print out a warn followed by an error. This suppresses the error and only prints the warn. 

How Has This Been Tested?
---
Manual


Fixes: #4352

applications/tari_console_wallet/src/main.rs

@@ -100,7 +100,7 @@ fn main_inner() -> Result<(), ExitError> {
     )?;
 
     #[cfg_attr(not(all(unix, feature = "libtor")), allow(unused_mut))]
-    let mut config = ApplicationConfig::load_from(&cfg)?;
+    let config = ApplicationConfig::load_from(&cfg)?;
 
     let runtime = tokio::runtime::Builder::new_multi_thread()
         .enable_all()

base_layer/core/src/base_node/comms_interface/inbound_handlers.rs

@@ -584,7 +584,36 @@ where B: BlockchainBackend + 'static
             coinbase_output,
             kernel_excess_sigs: excess_sigs,
         } = new_block;
+        // If the block is empty, we dont have to check ask for the block, as we already have the full block available
+        // to us.
+        if excess_sigs.is_empty() {
+            let block = BlockBuilder::new(header.version)
+                .with_coinbase_utxo(coinbase_output, coinbase_kernel)
+                .with_header(header)
+                .build();
+            return Ok(Arc::new(block));
+        }
 
+        let block_hash = header.hash();
+        // We check the current tip and orphan status of the block because we cannot guarantee that mempool state is
+        // correct and the mmr root calculation is only valid if the block is building on the tip.
+        let current_meta = self.blockchain_db.get_chain_metadata().await?;
+        if header.prev_hash != *current_meta.best_block() {
+            debug!(
+                target: LOG_TARGET,
+                "Orphaned block #{}: ({}), current tip is: #{} ({}). We need to fetch the complete block from peer: \
+                 ({})",
+                header.height,
+                block_hash.to_hex(),
+                current_meta.height_of_longest_chain(),
+                current_meta.best_block().to_hex(),
+                source_peer,
+            );
+            let block = self.request_full_block_from_peer(source_peer, block_hash).await?;
+            return Ok(block);
+        }
+
+        // We know that the block is neither and orphan or a coinbase, so lets ask our mempool for the transactions
         let (known_transactions, missing_excess_sigs) = self.mempool.retrieve_by_excess_sigs(excess_sigs).await?;
         let known_transactions = known_transactions.into_iter().map(|tx| (*tx).clone()).collect();
 
@@ -599,7 +628,7 @@ where B: BlockchainBackend + 'static
                 target: LOG_TARGET,
                 "All transactions for block #{} ({}) found in mempool",
                 header.height,
-                header.hash().to_hex()
+                block_hash.to_hex()
             );
         } else {
             debug!(
@@ -623,7 +652,6 @@ where B: BlockchainBackend + 'static
             }
 
             if !not_found.is_empty() {
-                let block_hash = header.hash();
                 warn!(
                     target: LOG_TARGET,
                     "Peer {} was not able to return all transactions for block #{} ({}). {} transaction(s) not found. \
@@ -655,9 +683,14 @@ where B: BlockchainBackend + 'static
         // Perform a sanity check on the reconstructed block, if the MMR roots don't match then it's possible one or
         // more transactions in our mempool had the same excess/signature for a *different* transaction.
         // This is extremely unlikely, but still possible. In case of a mismatch, request the full block from the peer.
-        let (block, mmr_roots) = self.blockchain_db.calculate_mmr_roots(block).await?;
+        let (block, mmr_roots) = match self.blockchain_db.calculate_mmr_roots(block).await {
+            Err(_) => {
+                let block = self.request_full_block_from_peer(source_peer, block_hash).await?;
+                return Ok(block);
+            },
+            Ok(v) => v,
+        };
         if let Err(e) = helpers::check_mmr_roots(&header, &mmr_roots) {
-            let block_hash = block.hash();
             warn!(
                 target: LOG_TARGET,
                 "Reconstructed block #{} ({}) failed MMR check validation!. Requesting full block. Error: {}",

base_layer/core/src/base_node/service/service.rs

@@ -55,7 +55,7 @@ use crate::{
         StateMachineHandle,
     },
     blocks::{Block, NewBlock},
-    chain_storage::BlockchainBackend,
+    chain_storage::{BlockchainBackend, ChainStorageError},
     proto as shared_protos,
     proto::base_node as proto,
 };
@@ -297,8 +297,14 @@ where B: BlockchainBackend + 'static
         task::spawn(async move {
             let result = handle_incoming_block(inbound_nch, new_block).await;
 
-            if let Err(e) = result {
-                error!(target: LOG_TARGET, "Failed to handle incoming block message: {:?}", e);
+            match result {
+                Err(BaseNodeServiceError::CommsInterfaceError(CommsInterfaceError::ChainStorageError(
+                    ChainStorageError::AddBlockOperationLocked,
+                ))) => {
+                    // Special case, dont log this again as an error
+                },
+                Err(e) => error!(target: LOG_TARGET, "Failed to handle incoming block message: {:?}", e),
+                _ => {},
             }
         });
     }

base_layer/core/src/chain_storage/blockchain_database.rs

@@ -892,7 +892,7 @@ where B: BlockchainBackend
         if self.is_add_block_disabled() {
             warn!(
                 target: LOG_TARGET,
-                "add_block is disabled. Ignoring candidate block #{} ({})",
+                "add_block is disabled, node busy syncing. Ignoring candidate block #{} ({})",
                 block.header.height,
                 block.hash().to_hex()
             );
@@ -1230,7 +1230,7 @@ pub fn calculate_mmr_roots<T: BlockchainBackend>(db: &T, block: &Block) -> Resul
     let metadata = db.fetch_chain_metadata()?;
     if header.prev_hash != *metadata.best_block() {
         return Err(ChainStorageError::CannotCalculateNonTipMmr(format!(
-            "Block (#{}) previous hash is {} but the current tip is #{} {}",
+            "Block (#{}) is not building on tip, previous hash is {} but the current tip is #{} {}",
             header.height,
             header.prev_hash.to_hex(),
             metadata.height_of_longest_chain(),