Add node option to specify custom disconnection reason when a peer is banned, and use it in the fork detector

Author: ImplOfAnImpl

build-tools/fork-detection/detector.py

@@ -81,6 +81,14 @@
 
 DEFAULT_BAN_DURATION_HOURS = 12
 
+# Custom disconnection reason to send to peers when banning them. We don't want the default
+# "Your address is banned" to be sent, because it sounds like the peer's node is faulty.
+# We also don't want to be too specific, e.g. the peer doesn't need to know thar something
+# called "fork detector" is running somewhere. So we choose a reason that is somewhat vague.
+# Also note that since we ban peers when networking is already disabled, the peer can only
+# get this message when attempting an outbound connectiion to the detector's node.
+BAN_REASON_STRING = "Cannot accept connections at this moment"
+
 # We use Queue.shutdown which is only available since Python v3.13
 MIN_PYTHON_VERSION_MAJOR = 3
 MIN_PYTHON_VERSION_MINOR = 13
@@ -89,7 +97,6 @@
 PERMABAN_DURATION_DAYS = 30
 PERMABAN_DURATION_SECS = 3600 * 24 * PERMABAN_DURATION_DAYS
 
-
 class Handler():
     def __init__(self, args, email_sender):
         CONSOLE_PRINTER.set_status("Initializing")
@@ -135,6 +142,7 @@ def __init__(self, args, email_sender):
             "--rpc-bind-address", args.node_rpc_bind_address,
             "--rpc-username", NODE_RPC_USER,
             "--rpc-password", NODE_RPC_PWD,
+            "--p2p-custom-disconnection-reason-for-banning", BAN_REASON_STRING
         ]
         log.info(f"Node run command: {self.node_cmd}")
 
@@ -315,10 +323,8 @@ def on_attempt_completion():
 
             peer_ips_to_ban = self.get_node_peer_ip_addrs_to_ban()
 
-            # Before banning, force the disconnection of all peers by disabling networking,
-            # to avoid sending them the "scary" disconnection reason "Your address is banned"
-            # (though nodes may still see this reason if they try connecting to our node
-            # during the next attempt).
+            # Before banning, disable networking; this will disconnect all peers and prevent them
+            # from reconnecting again.
             self.node_rpc_client.enable_networking(False)
             # Give the node some time to actually disconnect all peers.
             time.sleep(2)

dns-server/src/main.rs

@@ -84,6 +84,7 @@ async fn run(options: DnsServerRunOptions) -> anyhow::Result<Never> {
         sync_stalling_timeout: Default::default(),
         peer_manager_config: Default::default(),
         protocol_config: Default::default(),
+        custom_disconnection_reason_for_banning: Default::default(),
     });
 
     let transport = p2p::make_p2p_transport();

node-lib/src/config_files/mod.rs

@@ -196,6 +196,7 @@ fn p2p_config(config: P2pConfigFile, options: &RunOptions) -> P2pConfigFile {
         sync_stalling_timeout,
         node_type,
         force_dns_query_if_no_global_addresses_known,
+        custom_disconnection_reason_for_banning,
     } = config;
 
     let networking_enabled = options.p2p_networking_enabled.or(networking_enabled);
@@ -219,6 +220,10 @@ fn p2p_config(config: P2pConfigFile, options: &RunOptions) -> P2pConfigFile {
     let force_dns_query_if_no_global_addresses_known = options
         .p2p_force_dns_query_if_no_global_addresses_known
         .or(force_dns_query_if_no_global_addresses_known);
+    let custom_disconnection_reason_for_banning = options
+        .p2p_custom_disconnection_reason_for_banning
+        .clone()
+        .or(custom_disconnection_reason_for_banning);
 
     P2pConfigFile {
         networking_enabled,
@@ -238,6 +243,7 @@ fn p2p_config(config: P2pConfigFile, options: &RunOptions) -> P2pConfigFile {
         sync_stalling_timeout,
         node_type,
         force_dns_query_if_no_global_addresses_known,
+        custom_disconnection_reason_for_banning,
     }
 }
 

node-lib/src/config_files/p2p.rs

@@ -100,6 +100,8 @@ pub struct P2pConfigFile {
     /// If true, the node will perform an early dns query if the peer db doesn't contain
     /// any global addresses at startup.
     pub force_dns_query_if_no_global_addresses_known: Option<bool>,
+    /// If set, this text will be sent to banned peers as part of the DisconnectionReason.
+    pub custom_disconnection_reason_for_banning: Option<String>,
 }
 
 impl From<P2pConfigFile> for P2pConfig {
@@ -122,6 +124,7 @@ impl From<P2pConfigFile> for P2pConfig {
             sync_stalling_timeout,
             node_type,
             force_dns_query_if_no_global_addresses_known,
+            custom_disconnection_reason_for_banning,
         } = config_file;
 
         P2pConfig {
@@ -179,6 +182,7 @@ impl From<P2pConfigFile> for P2pConfig {
             },
             protocol_config: Default::default(),
             peer_handshake_timeout: Default::default(),
+            custom_disconnection_reason_for_banning,
         }
     }
 }

node-lib/src/options.rs

@@ -318,6 +318,10 @@ pub struct RunOptions {
     #[arg(hide = true)]
     pub p2p_force_dns_query_if_no_global_addresses_known: Option<bool>,
 
+    /// If specified, this text will be sent to banned peers as part of the DisconnectionReason.
+    #[clap(long, hide = true)]
+    pub p2p_custom_disconnection_reason_for_banning: Option<String>,
+
     /// A maximum tip age in seconds.
     ///
     /// The initial block download is finished if the difference between the current time and the
@@ -426,6 +430,7 @@ mod tests {
             p2p_sync_stalling_timeout: Default::default(),
             p2p_max_clock_diff: Default::default(),
             p2p_force_dns_query_if_no_global_addresses_known: Default::default(),
+            p2p_custom_disconnection_reason_for_banning: Default::default(),
             max_tip_age: Default::default(),
             rpc_bind_address: Default::default(),
             rpc_enabled: Default::default(),

node-lib/tests/cli.rs

@@ -114,6 +114,7 @@ fn read_config_override_values() {
     let p2p_sync_stalling_timeout = NonZeroU64::new(37).unwrap();
     let p2p_max_clock_diff = 15;
     let p2p_force_dns_query_if_no_global_addresses_known = true;
+    let p2p_custom_disconnection_reason_for_banning = "foo".to_owned();
     let rpc_bind_address = "127.0.0.1:5432".parse().unwrap();
     let backend_type = StorageBackendConfigFile::InMemory;
     let node_type = NodeTypeConfigFile::FullNode;
@@ -152,6 +153,9 @@ fn read_config_override_values() {
         p2p_force_dns_query_if_no_global_addresses_known: Some(
             p2p_force_dns_query_if_no_global_addresses_known,
         ),
+        p2p_custom_disconnection_reason_for_banning: (Some(
+            p2p_custom_disconnection_reason_for_banning.clone(),
+        )),
         max_tip_age: Some(max_tip_age),
         rpc_bind_address: Some(rpc_bind_address),
         rpc_enabled: Some(true),
@@ -273,6 +277,10 @@ fn read_config_override_values() {
         config.p2p.as_ref().unwrap().force_dns_query_if_no_global_addresses_known,
         Some(p2p_force_dns_query_if_no_global_addresses_known)
     );
+    assert_eq!(
+        config.p2p.as_ref().unwrap().custom_disconnection_reason_for_banning,
+        Some(p2p_custom_disconnection_reason_for_banning)
+    );
 
     assert_eq!(
         config.rpc.clone().unwrap().bind_address,

p2p/backend-test-suite/src/block_announcement.rs

@@ -193,6 +193,7 @@ where
         sync_stalling_timeout: Default::default(),
         peer_manager_config: Default::default(),
         protocol_config: Default::default(),
+        custom_disconnection_reason_for_banning: Default::default(),
     });
     let shutdown = Arc::new(SeqCstAtomicBool::new(false));
     let (shutdown_sender_1, shutdown_receiver) = oneshot::channel();

p2p/src/config.rs

@@ -107,6 +107,8 @@ pub struct P2pConfig {
     pub peer_manager_config: PeerManagerConfig,
     /// Various limits related to the protocol; these should only be overridden in tests.
     pub protocol_config: ProtocolConfig,
+    /// If set, this text will be sent to banned peers as part of the DisconnectionReason.
+    pub custom_disconnection_reason_for_banning: Option<String>,
 }
 
 impl P2pConfig {

p2p/src/disconnection_reason.rs

@@ -19,6 +19,7 @@ use networking::error::{MessageCodecError, NetworkingError};
 use p2p_types::services::Services;
 
 use crate::{
+    config::P2pConfig,
     error::{ConnectionValidationError, P2pError},
     protocol::MIN_SUPPORTED_PROTOCOL_VERSION,
 };
@@ -107,13 +108,24 @@ pub enum DisconnectionReason {
     // Another possible reason for message decoding failure.
     #[display("Your message size {actual_size} exceeded the maximum size {max_size}")]
     MessageTooLarge { actual_size: usize, max_size: usize },
+
+    #[display("{_0}")]
+    CustomMessage(String),
 }
 
 impl DisconnectionReason {
-    pub fn from_result<T>(res: &crate::Result<T>) -> Option<Self> {
+    pub fn from_result<T>(res: &crate::Result<T>, p2p_config: &P2pConfig) -> Option<Self> {
         match res {
             Ok(_) => None,
-            Err(err) => Self::from_error(err),
+            Err(err) => Self::from_error(err, p2p_config),
+        }
+    }
+
+    pub fn ban_reason(p2p_config: &P2pConfig) -> Self {
+        if let Some(custom_reason) = p2p_config.custom_disconnection_reason_for_banning.as_ref() {
+            Self::CustomMessage(custom_reason.clone())
+        } else {
+            Self::AddressBanned
         }
     }
 
@@ -163,7 +175,7 @@ impl DisconnectionReason {
         }
     }
 
-    pub fn from_error(err: &P2pError) -> Option<Self> {
+    pub fn from_error(err: &P2pError, p2p_config: &P2pConfig) -> Option<Self> {
         match err {
             P2pError::NetworkingError(err) => Self::from_networking_error(err),
             P2pError::ProtocolError(_)
@@ -203,7 +215,7 @@ impl DisconnectionReason {
                     Some(Self::TooManyInboundPeersAndCannotEvictAnyone)
                 }
                 ConnectionValidationError::AddressBanned { address: _ } => {
-                    Some(Self::AddressBanned)
+                    Some(Self::ban_reason(p2p_config))
                 }
                 ConnectionValidationError::AddressDiscouraged { address: _ } => {
                     Some(Self::AddressDiscouraged)

p2p/src/net/default_backend/peer.rs

@@ -229,7 +229,7 @@ where
         })();
 
         self.maybe_send_will_disconnect_for_protocol_version(
-            DisconnectionReason::from_result(&result),
+            DisconnectionReason::from_result(&result, &self.p2p_config),
             peer_protocol_version,
         )
         .await?;
@@ -498,7 +498,7 @@ where
                         log::info!("Connection closed for peer {}, reason: {err:?}", self.peer_id);
 
                         let err = P2pError::NetworkingError(err);
-                        self.maybe_send_will_disconnect(DisconnectionReason::from_error(&err)).await?;
+                        self.maybe_send_will_disconnect(DisconnectionReason::from_error(&err, &self.p2p_config)).await?;
 
                         let ban_score = err.ban_score();
                         if ban_score > 0 {