fix: ban peer when merkle roots mismatch

base_layer/core/src/base_node/state_machine_service/states/block_sync.rs

@@ -58,6 +58,7 @@ impl BlockSync {
         shared: &mut BaseNodeStateMachine<B>,
     ) -> StateEvent {
         let mut synchronizer = BlockSynchronizer::new(
+            shared.config.block_sync_config.clone(),
             shared.db.clone(),
             shared.connectivity.clone(),
             self.sync_peer.take(),

base_layer/core/src/base_node/sync/block_sync/error.rs

@@ -20,7 +20,7 @@
 //  WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
 //  USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-use crate::{chain_storage::ChainStorageError, proof_of_work::PowError, validation::ValidationError};
+use crate::{chain_storage::ChainStorageError, validation::ValidationError};
 use tari_comms::{
     connectivity::ConnectivityError,
     protocol::rpc::{RpcError, RpcStatus},
@@ -42,10 +42,8 @@ pub enum BlockSyncError {
     ConnectivityError(#[from] ConnectivityError),
     #[error("No sync peers available")]
     NoSyncPeers,
-    #[error("Error fetching PoW: {0}")]
-    PowError(#[from] PowError),
-    //#[error("Expected to find header at height {0} however the header did not exist")]
-    // ExpectedHeaderNotFound(u64),
     #[error("Block validation failed: {0}")]
     ValidationError(#[from] ValidationError),
+    #[error("Failed to ban peer: {0}")]
+    FailedToBan(ConnectivityError),
 }

base_layer/core/src/base_node/sync/block_sync/synchronizer.rs

@@ -22,7 +22,10 @@
 
 use super::error::BlockSyncError;
 use crate::{
-    base_node::sync::{hooks::Hooks, rpc},
+    base_node::{
+        sync::{hooks::Hooks, rpc},
+        BlockSyncConfig,
+    },
     chain_storage::{async_db::AsyncBlockchainDb, BlockchainBackend, ChainBlock},
     proto::base_node::SyncBlocksRequest,
     tari_utilities::{hex::Hex, Hashable},
@@ -47,6 +50,7 @@ use tokio::task;
 const LOG_TARGET: &str = "c::bn::block_sync";
 
 pub struct BlockSynchronizer<B> {
+    config: BlockSyncConfig,
     db: AsyncBlockchainDb<B>,
     connectivity: ConnectivityRequester,
     sync_peer: Option<PeerConnection>,
@@ -56,12 +60,14 @@ pub struct BlockSynchronizer<B> {
 
 impl<B: BlockchainBackend + 'static> BlockSynchronizer<B> {
     pub fn new(
+        config: BlockSyncConfig,
         db: AsyncBlockchainDb<B>,
         connectivity: ConnectivityRequester,
         sync_peer: Option<PeerConnection>,
         block_validator: Arc<dyn CandidateBlockBodyValidation<B>>,
     ) -> Self {
         Self {
+            config,
             db,
             connectivity,
             sync_peer,
@@ -87,10 +93,17 @@ impl<B: BlockchainBackend + 'static> BlockSynchronizer<B> {
             target: LOG_TARGET,
             "Attempting to synchronize blocks with `{}`", node_id
         );
-        self.attempt_block_sync(peer_conn).await?;
-
-        self.db.cleanup_orphans().await?;
-        Ok(())
+        match self.attempt_block_sync(peer_conn).await {
+            Ok(_) => {
+                self.db.cleanup_orphans().await?;
+                Ok(())
+            },
+            Err(err @ BlockSyncError::ValidationError(_)) | Err(err @ BlockSyncError::ReceivedInvalidBlockBody(_)) => {
+                self.ban_peer(node_id, &err).await?;
+                Err(err)
+            },
+            Err(err) => Err(err),
+        }
     }
 
     async fn get_next_sync_peer(&mut self) -> Result<PeerConnection, BlockSyncError> {
@@ -267,4 +280,21 @@ impl<B: BlockchainBackend + 'static> BlockSynchronizer<B> {
         .await
         .expect("block validator panicked")
     }
+
+    async fn ban_peer<T: ToString>(&mut self, node_id: NodeId, reason: T) -> Result<(), BlockSyncError> {
+        let reason = reason.to_string();
+        if self.config.sync_peers.contains(&node_id) {
+            debug!(
+                target: LOG_TARGET,
+                "Not banning peer that is allowlisted for sync. Ban reason = {}", reason
+            );
+            return Ok(());
+        }
+        warn!(target: LOG_TARGET, "Banned sync peer because {}", reason);
+        self.connectivity
+            .ban_peer_until(node_id, self.config.ban_period, reason)
+            .await
+            .map_err(BlockSyncError::FailedToBan)?;
+        Ok(())
+    }
 }

base_layer/core/src/base_node/sync/config.rs

@@ -25,9 +25,6 @@ use tari_comms::peer_manager::NodeId;
 
 #[derive(Debug, Clone)]
 pub struct BlockSyncConfig {
-    pub max_sync_peers: usize,
-    pub num_tip_hashes: usize,
-    pub num_proof_headers: usize,
     pub ban_period: Duration,
     pub short_ban_period: Duration,
     pub sync_peers: Vec<NodeId>,
@@ -36,9 +33,6 @@ pub struct BlockSyncConfig {
 impl Default for BlockSyncConfig {
     fn default() -> Self {
         Self {
-            max_sync_peers: 10,
-            num_tip_hashes: 500,
-            num_proof_headers: 100,
             ban_period: Duration::from_secs(30 * 60),
             short_ban_period: Duration::from_secs(60),
             sync_peers: Default::default(),

base_layer/core/src/base_node/sync/header_sync/synchronizer.rs

@@ -110,28 +110,28 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> {
                 Ok(()) => return Ok(peer_conn),
                 // Try another peer
                 Err(err @ BlockHeaderSyncError::NotInSync) => {
-                    debug!(target: LOG_TARGET, "{}", err);
+                    warn!(target: LOG_TARGET, "{}", err);
                 },
 
                 Err(err @ BlockHeaderSyncError::RpcError(RpcError::HandshakeError(RpcHandshakeError::TimedOut))) => {
-                    debug!(target: LOG_TARGET, "{}", err);
+                    warn!(target: LOG_TARGET, "{}", err);
                     self.ban_peer_short(node_id, BanReason::RpcNegotiationTimedOut).await?;
                 },
                 Err(BlockHeaderSyncError::ValidationFailed(err)) => {
-                    debug!(target: LOG_TARGET, "Block header validation failed: {}", err);
+                    warn!(target: LOG_TARGET, "Block header validation failed: {}", err);
                     self.ban_peer_long(node_id, err.into()).await?;
                 },
                 Err(BlockHeaderSyncError::ChainSplitNotFound(peer)) => {
-                    debug!(target: LOG_TARGET, "Chain split not found for peer {}.", peer);
+                    warn!(target: LOG_TARGET, "Chain split not found for peer {}.", peer);
                     self.ban_peer_long(peer, BanReason::ChainSplitNotFound).await?;
                 },
                 Err(err @ BlockHeaderSyncError::InvalidBlockHeight { .. }) => {
-                    debug!(target: LOG_TARGET, "{}", err);
+                    warn!(target: LOG_TARGET, "{}", err);
                     self.ban_peer_long(node_id, BanReason::GeneralHeaderSyncFailure(err))
                         .await?;
                 },
                 Err(err) => {
-                    debug!(
+                    error!(
                         target: LOG_TARGET,
                         "Failed to synchronize headers from peer `{}`: {}", node_id, err
                     );