classify validation errors by category, MEDIUM_BAN on hard-fork errors

Author: redsh4de

binaries/cuprated/src/blockchain/error.rs

@@ -2,7 +2,7 @@
 
 use cuprate_blockchain::BlockchainError;
 use cuprate_consensus::ExtendedConsensusError;
-use cuprate_consensus_rules::ConsensusError;
+use cuprate_consensus_rules::{blocks::BlockError, hard_forks::HardForkError, ConsensusError};
 use cuprate_txpool::TxPoolError;
 use cuprate_types::TxConversionError;
 
@@ -20,17 +20,21 @@ macro_rules! impl_internal_from {
 /// A validation failure - the peer should be banned.
 #[derive(Debug, thiserror::Error)]
 pub enum BlockValidationError {
-    /// The block failed consensus validation.
+    /// Invalid hard-fork rules.
     #[error(transparent)]
-    Consensus(ExtendedConsensusError),
+    HardFork(HardForkError),
+
+    /// Any other consensus rule violation.
+    #[error(transparent)]
+    Other(ExtendedConsensusError),
 }
 
 /// An error from the blockchain manager's internal handlers.
 #[derive(Debug, thiserror::Error)]
 pub enum BlockManagerError {
     /// The peer sent us an invalid block; ban them.
     #[error(transparent)]
-    Validation(#[from] BlockValidationError),
+    Validation(BlockValidationError),
 
     /// A node-side failure.
     #[error(transparent)]
@@ -39,16 +43,25 @@ pub enum BlockManagerError {
 
 impl From<ExtendedConsensusError> for BlockManagerError {
     fn from(e: ExtendedConsensusError) -> Self {
-        if let ExtendedConsensusError::DBErr(e) = e {
-            return Self::Internal(e);
+        match e {
+            ExtendedConsensusError::DBErr(e) => Self::Internal(e),
+            ExtendedConsensusError::ConErr(ConsensusError::Block(BlockError::HardForkError(e))) => {
+                Self::Validation(BlockValidationError::HardFork(e))
+            }
+
+            ExtendedConsensusError::ConErr(_)
+            | ExtendedConsensusError::TxsIncludedWithBlockIncorrect
+            | ExtendedConsensusError::OneOrMoreBatchVerificationStatementsInvalid
+            | ExtendedConsensusError::NoBlocksToVerify => {
+                Self::Validation(BlockValidationError::Other(e))
+            }
         }
-        BlockValidationError::Consensus(e).into()
     }
 }
 
 impl From<ConsensusError> for BlockManagerError {
     fn from(e: ConsensusError) -> Self {
-        BlockValidationError::Consensus(e.into()).into()
+        ExtendedConsensusError::ConErr(e).into()
     }
 }
 

binaries/cuprated/src/blockchain/manager/handler.rs

@@ -23,7 +23,11 @@ use cuprate_consensus::{
 use cuprate_consensus_context::{BlockchainContext, NewBlockData};
 use cuprate_fast_sync::{block_to_verified_block_information, fast_sync_stop_height};
 use cuprate_helper::cast::usize_to_u64;
-use cuprate_p2p::{block_downloader::BlockBatch, constants::LONG_BAN, BroadcastRequest};
+use cuprate_p2p::{
+    block_downloader::BlockBatch,
+    constants::{LONG_BAN, MEDIUM_BAN},
+    BroadcastRequest,
+};
 use cuprate_txpool::service::interface::TxpoolWriteRequest;
 use cuprate_types::{
     blockchain::{BlockchainReadRequest, BlockchainResponse, BlockchainWriteRequest},
@@ -246,15 +250,20 @@ impl super::BlockchainManager {
         {
             Ok(v) => v,
             Err(BlockManagerError::Internal(e)) => return Err(e),
-            Err(BlockManagerError::Validation(_)) => {
-                batch.peer_handle.ban_peer(LONG_BAN);
+            Err(BlockManagerError::Validation(e)) => {
+                let duration = match e {
+                    BlockValidationError::HardFork(_) => MEDIUM_BAN,
+                    BlockValidationError::Other(_) => LONG_BAN,
+                };
+                batch.peer_handle.ban_peer(duration);
                 self.stop_current_block_downloader.notify_waiters();
                 return Ok(());
             }
         };
 
         for (block, txs) in prepped_blocks {
             let hash = block.block_hash;
+            let block_version = block.hf_version.as_u8();
             let verified_block = match verify_prepped_main_chain_block(
                 block,
                 txs,
@@ -268,12 +277,27 @@ impl super::BlockchainManager {
                 Ok(block) => block,
                 Err(BlockManagerError::Internal(e)) => return Err(e),
                 Err(BlockManagerError::Validation(e)) => {
-                    warn!(
-                        "Failed to verify block: {}, error {}, banning peer.",
-                        hex::encode(hash),
-                        e
-                    );
-                    batch.peer_handle.ban_peer(LONG_BAN);
+                    let duration = match e {
+                        BlockValidationError::HardFork(e) => {
+                            warn!(
+                                "Failed to verify block: {}, error {} (block v{}, current v{}), banning peer.",
+                                hex::encode(hash),
+                                e,
+                                block_version,
+                                self.blockchain_context_service.blockchain_context().current_hf.as_u8()
+                            );
+                            MEDIUM_BAN
+                        }
+                        BlockValidationError::Other(e) => {
+                            warn!(
+                                "Failed to verify block: {}, error {}, banning peer.",
+                                hex::encode(hash),
+                                e
+                            );
+                            LONG_BAN
+                        }
+                    };
+                    batch.peer_handle.ban_peer(duration);
                     self.stop_current_block_downloader.notify_waiters();
                     return Ok(());
                 }
@@ -335,6 +359,7 @@ impl super::BlockchainManager {
 
         while let Some((block, txs)) = blocks.next() {
             let hash = block.hash();
+            let block_version = block.header.hardfork_version;
 
             // async blocks work as try blocks.
             let res = async {
@@ -355,12 +380,27 @@ impl super::BlockchainManager {
             match res {
                 Err(BlockManagerError::Internal(e)) => return Err(e),
                 Err(BlockManagerError::Validation(e)) => {
-                    warn!(
-                        "Failed to verify block: {}, error {}, banning peer.",
-                        hex::encode(hash),
-                        e
-                    );
-                    batch.peer_handle.ban_peer(LONG_BAN);
+                    let duration = match e {
+                        BlockValidationError::HardFork(e) => {
+                            warn!(
+                                "Failed to verify block: {}, error {} (block v{}, current v{}), banning peer.",
+                                hex::encode(hash),
+                                e,
+                                block_version,
+                                self.blockchain_context_service.blockchain_context().current_hf.as_u8()
+                            );
+                            MEDIUM_BAN
+                        }
+                        BlockValidationError::Other(e) => {
+                            warn!(
+                                "Failed to verify block: {}, error {}, banning peer.",
+                                hex::encode(hash),
+                                e
+                            );
+                            LONG_BAN
+                        }
+                    };
+                    batch.peer_handle.ban_peer(duration);
                     self.stop_current_block_downloader.notify_waiters();
                     return Ok(());
                 }

binaries/cuprated/src/p2p/request_handler.rs

@@ -30,6 +30,7 @@ use cuprate_helper::{
 };
 use cuprate_p2p::constants::{
     LONG_BAN, MAX_BLOCKS_IDS_IN_CHAIN_ENTRY, MAX_BLOCK_BATCH_LEN, MAX_TRANSACTION_BLOB_SIZE,
+    MEDIUM_BAN,
 };
 use cuprate_p2p_core::{
     client::{InternalPeerID, PeerInformation},
@@ -46,7 +47,7 @@ use cuprate_wire::protocol::{
 };
 
 use crate::{
-    blockchain::{interface::BlockchainManagerHandle, IncomingBlockError},
+    blockchain::{interface::BlockchainManagerHandle, BlockValidationError, IncomingBlockError},
     p2p::CrossNetworkInternalPeerId,
     txpool::{IncomingTxError, IncomingTxHandler, IncomingTxs},
 };
@@ -355,6 +356,7 @@ async fn new_fluffy_block<A: NetZoneAddress>(
     }
 
     let block_hash = block.hash();
+    let block_version = block.header.hardfork_version;
     let res = blockchain_manager
         .handle_incoming_block(
             block,
@@ -381,21 +383,33 @@ async fn new_fluffy_block<A: NetZoneAddress>(
             // Manager has exited (likely shutdown); drop silently.
             Ok(ProtocolResponse::NA)
         }
-        Err(IncomingBlockError::Internal(e)) => {
-            tracing::error!("Failed to handle incoming block: {e}");
-            Ok(ProtocolResponse::NA)
-        }
-        Err(e) => {
-            if matches!(e, IncomingBlockError::Validation(_)) {
+        Err(IncomingBlockError::Validation(e)) => match e {
+            BlockValidationError::HardFork(e) => {
+                tracing::warn!(
+                    "Failed to verify block: {}, error {} (block v{}, current v{}), banning peer.",
+                    hex::encode(block_hash),
+                    e,
+                    block_version,
+                    context.current_hf.as_u8()
+                );
+                peer_information.handle.ban_peer(MEDIUM_BAN);
+                Err(e.into())
+            }
+            BlockValidationError::Other(e) => {
                 tracing::warn!(
                     "Failed to verify block: {}, error {}, banning peer.",
                     hex::encode(block_hash),
                     e
                 );
                 peer_information.handle.ban_peer(LONG_BAN);
+                Err(e.into())
             }
-            Err(e.into())
+        },
+        Err(IncomingBlockError::Internal(e)) => {
+            tracing::error!("Failed to handle incoming block: {e}");
+            Ok(ProtocolResponse::NA)
         }
+        Err(e) => Err(e.into()),
     }
 }