refactor: move service validation to MnNetInfo, run during setting

Author: kwvg

src/evo/deterministicmns.cpp

@@ -1180,29 +1180,15 @@ void CDeterministicMNManager::CleanupCache(int nHeight)
 template <typename ProTx>
 static bool CheckService(const ProTx& proTx, TxValidationState& state)
 {
-    const auto addr{proTx.netInfo.GetPrimary()};
-    if (!addr.IsValid()) {
-        return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-ipaddr");
-    }
-    if (Params().RequireRoutableExternalIP() && !addr.IsRoutable()) {
-        return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-ipaddr");
-    }
-
-    // TODO: use real args here
-    static int mainnetDefaultPort = CreateChainParams(ArgsManager{}, CBaseChainParams::MAIN)->GetDefaultPort();
-    if (Params().NetworkIDString() == CBaseChainParams::MAIN) {
-        if (addr.GetPort() != mainnetDefaultPort) {
-            return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-ipaddr-port");
-        }
-    } else if (addr.GetPort() == mainnetDefaultPort) {
-        return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-ipaddr-port");
-    }
-
-    if (!addr.IsIPv4()) {
-        return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-ipaddr");
-    }
-
-    return true;
+    switch (proTx.netInfo.Validate()) {
+    case NetInfoStatus::BadInput:
+        return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo");
+    case NetInfoStatus::BadPort:
+        return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-port");
+    case NetInfoStatus::Success:
+        return true;
+    } // no default case, so the compiler can warn about missing cases
+    assert(false);
 }
 
 template <typename ProTx>

src/evo/netinfo.cpp

@@ -7,14 +7,54 @@
 #include <chainparams.h>
 #include <netbase.h>
 #include <span.h>
+#include <util/system.h>
+
+namespace {
+static std::unique_ptr<const CChainParams> g_main_params{nullptr};
+
+bool IsNodeOnMainnet() { return Params().NetworkIDString() == CBaseChainParams::MAIN; }
+const CChainParams& MainParams()
+{
+    // TODO: use real args here
+    if (!g_main_params) g_main_params = CreateChainParams(ArgsManager{}, CBaseChainParams::MAIN);
+    return *g_main_params;
+}
+} // anonymous namespace
+
+NetInfoStatus MnNetInfo::ValidateService(const CService& service)
+{
+    if (!service.IsValid()) {
+        return NetInfoStatus::BadInput;
+    }
+    if (!service.IsIPv4()) {
+        return NetInfoStatus::BadInput;
+    }
+    if (Params().RequireRoutableExternalIP() && !service.IsRoutable()) {
+        return NetInfoStatus::BadInput;
+    }
+
+    const auto default_port_main = MainParams().GetDefaultPort();
+    if (IsNodeOnMainnet() && service.GetPort() != default_port_main) {
+        // Must use mainnet port on mainnet
+        return NetInfoStatus::BadPort;
+    } else if (service.GetPort() == default_port_main) {
+        // Using mainnet port prohibited outside of mainnet
+        return NetInfoStatus::BadPort;
+    }
+
+    return NetInfoStatus::Success;
+}
 
 NetInfoStatus MnNetInfo::AddEntry(const std::string& input)
 {
     if (auto service = Lookup(input, /*portDefault=*/Params().GetDefaultPort(), /*fAllowLookup=*/false);
         service.has_value()) {
-        m_addr = service.value();
-        ASSERT_IF_DEBUG(m_addr != CService());
-        return NetInfoStatus::Success;
+        const auto ret = ValidateService(service.value());
+        if (ret == NetInfoStatus::Success) {
+            m_addr = service.value();
+            ASSERT_IF_DEBUG(m_addr != CService());
+        }
+        return ret;
     }
     return NetInfoStatus::BadInput;
 }

src/evo/netinfo.h

@@ -12,6 +12,7 @@ class CService;
 
 enum NetInfoStatus : uint8_t {
     BadInput,
+    BadPort,
     Success
 };
 
@@ -20,6 +21,8 @@ constexpr std::string_view NISToString(const NetInfoStatus code)
     switch (code) {
     case NetInfoStatus::BadInput:
         return "invalid address";
+    case NetInfoStatus::BadPort:
+        return "invalid port";
     case NetInfoStatus::Success:
         return "success";
     } // no default case, so the compiler can warn about missing cases
@@ -31,6 +34,9 @@ class MnNetInfo
 private:
     CService m_addr{};
 
+private:
+    static NetInfoStatus ValidateService(const CService& service);
+
 public:
     MnNetInfo() = default;
     ~MnNetInfo() = default;
@@ -47,6 +53,7 @@ class MnNetInfo
 
     const CService& GetPrimary() const { return m_addr; }
     bool IsEmpty() const { return *this == MnNetInfo(); }
+    NetInfoStatus Validate() const { return ValidateService(m_addr); }
 
     void Clear() { m_addr = CService(); }
 };

src/test/evo_netinfo_tests.cpp

@@ -16,8 +16,14 @@ const std::vector<std::pair</*input=*/std::string, /*expected_ret=*/NetInfoStatu
     {"1.1.1.1:8888", NetInfoStatus::Success},
     // Address specified, port should default to default P2P core
     {"1.1.1.1", NetInfoStatus::Success},
+    // Mainnet P2P port on non-mainnet
+    {"1.1.1.1:9999", NetInfoStatus::BadPort},
+    // Valid IPv4 formatting but invalid IPv4 address
+    {"0.0.0.0:8888", NetInfoStatus::BadInput},
     // Port greater than uint16_t max
     {"1.1.1.1:99999", NetInfoStatus::BadInput},
+    // Only IPv4 allowed
+    {"[2606:4700:4700::1111]:8888", NetInfoStatus::BadInput},
     // Domains are not allowed
     {"example.com:8888", NetInfoStatus::BadInput},
     // Incorrect IPv4 address