Merge pull request #2024 from mintlayer/p2p_specify_min_core_version

New hidden node option: `--p2p-min-peer-software-version`

Author: ImplOfAnImpl

Cargo.lock

@@ -31,23 +31,25 @@ Each attempt's directory has the following structure:
 - `node_log.txt` - the node's log.
 
 Some notes:
-* Currently the script requires Python 3.13 to run, though we may lift this requirement later.
-* The script can send an email when it detects an issue using the local SMTP server
+- Currently the script requires Python 3.13 to run, though we may lift this requirement later.
+- The script can send an email when it detects an issue using the local SMTP server
   (if you're on Linux, google for an SMTP Postfix tutorial to set it up).
-* Even if the script finds a problem (e.g. a checkpoint mismatch), you're still likely
+- Even if the script finds a problem (e.g. a checkpoint mismatch), you're still likely
   to end up being on the correct chain. To download the actual fork for further investigation
   you can initiate a separate full sync while using the node's option `--custom-checkpoints-csv-file`
   to override the correct checkpoints with the wrong ones.
-* Once the fork has been downloaded, you'll want to examine the contents of its chainstate db.
+- Once the fork has been downloaded, you'll want to examine the contents of its chainstate db.
   Currently we have the `chainstate-db-dumper` tool that can dump certain info about blocks
   to a CSV file (the most interesting part of it being the ids of pools that continue producing
   blocks on that fork).
-* Once the fork has been investigated you can "permanently" ban the peers that have been sending it
+- Once the fork has been investigated you can "permanently" ban the peers that have been sending it
   to you, to prevent it from being reported again and again. To do so, you can add their ip
   addresses to `permabanned_peers.txt` (one address per line, '#' starts a comment) in the script's
   working directory (it doesn't exist by default, so you'll have to create it first).
   Note that the file is checked on every iteration, so you can update it while the script is already
   running and it will come into effect when the next iteration starts.
-* The script is likely to fail if a networking error occurs, e.g. if it can't query the API server.
+- The script is likely to fail if a networking error occurs, e.g. if it can't query the API server.
   So, run it in a loop in a shell script (with some delay after each run, to prevent it from spamming
-  you with warning emails).
+  you with warning emails).
+- If you expect a split to already exist due to a hard fork (in which case reporting it would be useless),
+  use the `--min-peer-software-version` option to reject all nodes that have not been upgraded.

build-tools/fork-detection/README.md

@@ -144,6 +144,10 @@ def __init__(self, args, email_sender):
             "--rpc-password", NODE_RPC_PWD,
             "--p2p-custom-disconnection-reason-for-banning", BAN_REASON_STRING
         ]
+
+        if args.min_peer_software_version is not None:
+            self.node_cmd += ["--p2p-min-peer-software-version", args.min_peer_software_version]
+
         log.info(f"Node run command: {self.node_cmd}")
 
     def run(self):
@@ -549,6 +553,11 @@ def main():
             help=("The from address for the notification email. "
                   "If None, the --notification-email value will be used"),
             default=None)
+        parser.add_argument(
+            "--min-peer-software-version",
+            help=("The minimum peer software version, e.g. '1.2.0'. "
+                  "Peers with versions below this one will be rejected and discouraged"),
+            default=None)
         args = parser.parse_args()
 
         email_sender = EmailSender(

build-tools/fork-detection/detector.py

@@ -385,7 +385,8 @@ impl Builder {
             dns_seeds: chain_type.dns_seeds(),
             predefined_peer_addresses: chain_type.predefined_peer_addresses(),
             default_rpc_port: chain_type.default_rpc_port(),
-            software_version: SemVer::try_from(env!("CARGO_PKG_VERSION"))
+            software_version: env!("CARGO_PKG_VERSION")
+                .parse()
                 .expect("invalid CARGO_PKG_VERSION value"),
             max_block_header_size: super::MAX_BLOCK_HEADER_SIZE,
             max_block_size_with_standard_txs: super::MAX_BLOCK_TXS_SIZE,

common/src/chain/config/builder.rs

@@ -13,6 +13,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::str::FromStr;
+
 use serde::Serialize;
 use serialization::{Decode, Encode};
 
@@ -33,16 +35,10 @@ impl SemVer {
     }
 }
 
-impl From<SemVer> for String {
-    fn from(v: SemVer) -> String {
-        format!("{}.{}.{}", v.major, v.minor, v.patch)
-    }
-}
-
-impl TryFrom<&str> for SemVer {
-    type Error = &'static str;
+impl FromStr for SemVer {
+    type Err = &'static str;
 
-    fn try_from(v: &str) -> Result<SemVer, Self::Error> {
+    fn from_str(v: &str) -> Result<SemVer, Self::Err> {
         let split_version = v.split('.').collect::<Vec<_>>();
         if split_version.len() != 3 {
             return Err("Invalid version. Number of components is wrong.");
@@ -61,20 +57,22 @@ impl TryFrom<&str> for SemVer {
     }
 }
 
-impl TryFrom<String> for SemVer {
-    type Error = &'static str;
-
-    fn try_from(v: String) -> Result<SemVer, Self::Error> {
-        Self::try_from(v.as_str())
-    }
-}
-
 impl std::fmt::Display for SemVer {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(f, "{}.{}.{}", self.major, self.minor, self.patch)
     }
 }
 
+// TODO: this is redundant, but it's still used inside a macro in `regtest_chain_config_builder`.
+// Refactor the macro and remove this.
+impl TryFrom<String> for SemVer {
+    type Error = <SemVer as FromStr>::Err;
+
+    fn try_from(v: String) -> Result<SemVer, Self::Error> {
+        Self::from_str(v.as_str())
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -84,84 +82,90 @@ mod tests {
     #[test]
     fn vertest_string() {
         let version = SemVer::new(1, 2, 3);
-        assert_eq!(String::from(version), "1.2.3");
+        assert_eq!(version.to_string(), "1.2.3");
 
         let version = SemVer::new(0xff, 0xff, 0xff);
-        assert_eq!(String::from(version), "255.255.255");
+        assert_eq!(version.to_string(), "255.255.255");
 
         let version = SemVer::new(0xff, 0xff, 0xffff);
-        assert_eq!(String::from(version), "255.255.65535");
+        assert_eq!(version.to_string(), "255.255.65535");
 
         let version = SemVer::new(1, 2, 0x500);
-        assert_eq!(String::from(version), "1.2.1280");
+        assert_eq!(version.to_string(), "1.2.1280");
 
         assert_eq!(
-            SemVer::try_from(" "),
+            SemVer::from_str(" "),
             Err("Invalid version. Number of components is wrong.")
         );
 
         assert_eq!(
-            SemVer::try_from(""),
+            SemVer::from_str(""),
             Err("Invalid version. Number of components is wrong.")
         );
 
         assert_eq!(
-            SemVer::try_from("1.2"),
+            SemVer::from_str("1.2"),
             Err("Invalid version. Number of components is wrong.")
         );
 
         assert_eq!(
-            SemVer::try_from("1"),
+            SemVer::from_str("1"),
             Err("Invalid version. Number of components is wrong.")
         );
 
         let version = "hello";
         assert_eq!(
-            SemVer::try_from(version),
+            SemVer::from_str(version),
             Err("Invalid version. Number of components is wrong.")
         );
         assert_eq!(
-            SemVer::try_from(version),
+            SemVer::from_str(version),
             Err("Invalid version. Number of components is wrong.")
         );
 
-        let version = "1.2.3".to_string();
-        assert_eq!(SemVer::try_from(version.clone()), Ok(SemVer::new(1, 2, 3)));
-        assert_eq!(SemVer::try_from(version), Ok(SemVer::new(1, 2, 3)));
+        let version = "1.2.3";
+        assert_eq!(SemVer::from_str(version), Ok(SemVer::new(1, 2, 3)));
+        assert_eq!(
+            SemVer::try_from(version.to_owned()),
+            Ok(SemVer::new(1, 2, 3))
+        );
 
         let version = "255.255.255";
-        assert_eq!(SemVer::try_from(version), Ok(SemVer::new(255, 255, 255)));
-        assert_eq!(SemVer::try_from(version), Ok(SemVer::new(255, 255, 255)));
+        assert_eq!(SemVer::from_str(version), Ok(SemVer::new(255, 255, 255)));
+        assert_eq!(
+            SemVer::try_from(version.to_owned()),
+            Ok(SemVer::new(255, 255, 255))
+        );
 
-        let version = "255.255.65535".to_string();
+        let version = "255.255.65535";
+        assert_eq!(SemVer::from_str(version), Ok(SemVer::new(255, 255, 65535)));
         assert_eq!(
-            SemVer::try_from(version.clone()),
+            SemVer::try_from(version.to_owned()),
             Ok(SemVer::new(255, 255, 65535))
         );
-        assert_eq!(SemVer::try_from(version), Ok(SemVer::new(255, 255, 65535)));
 
         let version = "255.255.65536";
         assert_eq!(
-            SemVer::try_from(version),
+            SemVer::from_str(version),
             Err("Parsing SemVer component to integer failed")
         );
         assert_eq!(
-            SemVer::try_from(version.to_string()),
+            SemVer::try_from(version.to_owned()),
             Err("Parsing SemVer component to integer failed")
         );
 
         assert_eq!(
-            SemVer::try_from("1.2.a"),
+            SemVer::from_str("1.2.a"),
             Err("Parsing SemVer component to integer failed")
         );
 
         assert_eq!(
-            SemVer::try_from("1.2."),
+            SemVer::from_str("1.2."),
             Err("Parsing SemVer component to integer failed")
         );
 
         assert_eq!(
-            SemVer::try_from("1..3"),
+            SemVer::from_str("1..3"),
             Err("Parsing SemVer component to integer failed")
         );
     }

common/src/primitives/semver.rs

@@ -1071,8 +1071,7 @@ nothing
 
 Attempt to connect to a remote node (just once).
 
-For persistent connections see `add_reserved_node` should be used.
-Keep in mind that `add_reserved_node` works completely differently.
+For persistent connections consider using `add_reserved_node`.
 
 
 Parameters:
@@ -1087,7 +1086,10 @@ nothing
 
 ### Method `p2p_disconnect`
 
-Disconnect peer, given its id.
+Disconnect a peer given its id.
+
+If it was an outbound connection, the peer address will be removed from the peer database,
+and if the connection was inbound, the address will be kept.
 
 
 Parameters:

node-daemon/docs/RPC.md

@@ -29,6 +29,7 @@ fs4.workspace = true
 jsonrpsee = { workspace = true, features = ["macros"] }
 paste.workspace = true
 serde = { workspace = true, features = ["derive"] }
+serde_with.workspace = true
 thiserror.workspace = true
 tokio = { workspace = true, default-features = false }
 toml.workspace = true

node-lib/Cargo.toml

@@ -199,6 +199,7 @@ fn p2p_config(config: P2pConfigFile, options: &RunOptions) -> P2pConfigFile {
         node_type,
         force_dns_query_if_no_global_addresses_known,
         custom_disconnection_reason_for_banning,
+        min_peer_software_version,
     } = config;
 
     let networking_enabled = options.p2p_networking_enabled.or(networking_enabled);
@@ -226,6 +227,8 @@ fn p2p_config(config: P2pConfigFile, options: &RunOptions) -> P2pConfigFile {
         .p2p_custom_disconnection_reason_for_banning
         .clone()
         .or(custom_disconnection_reason_for_banning);
+    let min_peer_software_version =
+        options.p2p_min_peer_software_version.or(min_peer_software_version);
 
     P2pConfigFile {
         networking_enabled,
@@ -246,6 +249,7 @@ fn p2p_config(config: P2pConfigFile, options: &RunOptions) -> P2pConfigFile {
         node_type,
         force_dns_query_if_no_global_addresses_known,
         custom_disconnection_reason_for_banning,
+        min_peer_software_version,
     }
 }
 

node-lib/src/config_files/mod.rs

@@ -20,9 +20,9 @@ use std::{
     time::Duration,
 };
 
-use common::primitives::user_agent::mintlayer_core_user_agent;
 use serde::{Deserialize, Serialize};
 
+use common::primitives::{semver::SemVer, user_agent::mintlayer_core_user_agent};
 use p2p::{
     ban_config::BanConfig,
     config::{NodeType, P2pConfig},
@@ -61,6 +61,7 @@ impl FromStr for NodeTypeConfigFile {
 
 /// The p2p subsystem configuration.
 #[must_use]
+#[serde_with::serde_as]
 #[derive(Serialize, Deserialize, Debug, Default, Clone)]
 #[serde(deny_unknown_fields)]
 pub struct P2pConfigFile {
@@ -102,6 +103,11 @@ pub struct P2pConfigFile {
     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>,
+    /// If the peer's user agent is MintlayerCore (which is always true at the moment),
+    /// the connection will be rejected and the peer discouraged if the peer's software version
+    /// is less than the one specified.
+    #[serde_as(as = "Option<serde_with::DisplayFromStr>")]
+    pub min_peer_software_version: Option<SemVer>,
 }
 
 impl From<P2pConfigFile> for P2pConfig {
@@ -125,6 +131,7 @@ impl From<P2pConfigFile> for P2pConfig {
             node_type,
             force_dns_query_if_no_global_addresses_known,
             custom_disconnection_reason_for_banning,
+            min_peer_software_version,
         } = config_file;
 
         P2pConfig {
@@ -179,6 +186,8 @@ impl From<P2pConfigFile> for P2pConfig {
                 allow_same_ip_connections: Default::default(),
 
                 peerdb_config: Default::default(),
+
+                min_peer_software_version,
             },
             protocol_config: Default::default(),
             peer_handshake_timeout: Default::default(),