From 069583dafb7cea1ccb60d3ce0a93e9c90e56c972 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sat, 17 May 2025 14:49:04 +0000 Subject: [PATCH 1/8] evo: expand error codes for `netInfo` validation --- src/evo/deterministicmns.cpp | 11 +++++++++-- src/evo/netinfo.cpp | 6 +++--- src/evo/netinfo.h | 15 ++++++++++++++- src/test/evo_netinfo_tests.cpp | 6 +++--- 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index ea789de75b59..f758a34c1849 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -1205,12 +1205,19 @@ template static bool CheckService(const ProTx& proTx, TxValidationState& state) { switch (proTx.netInfo.Validate()) { - case NetInfoStatus::BadInput: - return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo"); + case NetInfoStatus::BadAddress: + return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-addr"); case NetInfoStatus::BadPort: return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-port"); + case NetInfoStatus::BadType: + return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-addr-type"); + case NetInfoStatus::NotRoutable: + return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-addr-unroutable"); case NetInfoStatus::Success: return true; + // Shouldn't be possible during self-checks + case NetInfoStatus::BadInput: + assert(false); } // no default case, so the compiler can warn about missing cases assert(false); } diff --git a/src/evo/netinfo.cpp b/src/evo/netinfo.cpp index 38e767d69ca7..53aca4de7bc8 100644 --- a/src/evo/netinfo.cpp +++ b/src/evo/netinfo.cpp @@ -27,13 +27,13 @@ const CChainParams& MainParams() NetInfoStatus MnNetInfo::ValidateService(const CService& service) { if (!service.IsValid()) { - return NetInfoStatus::BadInput; + return NetInfoStatus::BadAddress; } if (!service.IsIPv4()) { - return NetInfoStatus::BadInput; + return NetInfoStatus::BadType; } if (Params().RequireRoutableExternalIP() && !service.IsRoutable()) { - return NetInfoStatus::BadInput; + return NetInfoStatus::NotRoutable; } if (IsNodeOnMainnet() != (service.GetPort() == MainParams().GetDefaultPort())) { diff --git a/src/evo/netinfo.h b/src/evo/netinfo.h index a264d76b8e0d..07cf6e023632 100644 --- a/src/evo/netinfo.h +++ b/src/evo/netinfo.h @@ -11,18 +11,31 @@ class CService; enum class NetInfoStatus : uint8_t { + // Managing entries BadInput, + + // Validation + BadAddress, BadPort, + BadType, + NotRoutable, + Success }; constexpr std::string_view NISToString(const NetInfoStatus code) { switch (code) { - case NetInfoStatus::BadInput: + case NetInfoStatus::BadAddress: return "invalid address"; + case NetInfoStatus::BadInput: + return "invalid input"; case NetInfoStatus::BadPort: return "invalid port"; + case NetInfoStatus::BadType: + return "invalid address type"; + case NetInfoStatus::NotRoutable: + return "unroutable address"; case NetInfoStatus::Success: return "success"; } // no default case, so the compiler can warn about missing cases diff --git a/src/test/evo_netinfo_tests.cpp b/src/test/evo_netinfo_tests.cpp index 2c8952b234f0..2e942b4b5955 100644 --- a/src/test/evo_netinfo_tests.cpp +++ b/src/test/evo_netinfo_tests.cpp @@ -21,13 +21,13 @@ const std::vector Date: Tue, 27 May 2025 16:00:44 +0000 Subject: [PATCH 2/8] evo: introduce type-flexible `NetInfoEntry` to allow non-`CService` data --- src/evo/netinfo.cpp | 120 ++++++++++++++++++++++++++++++++- src/evo/netinfo.h | 71 +++++++++++++++++++ src/test/evo_netinfo_tests.cpp | 83 +++++++++++++++++++++++ 3 files changed, 272 insertions(+), 2 deletions(-) diff --git a/src/evo/netinfo.cpp b/src/evo/netinfo.cpp index 53aca4de7bc8..5bd2990ebd2f 100644 --- a/src/evo/netinfo.cpp +++ b/src/evo/netinfo.cpp @@ -13,6 +13,7 @@ namespace { static std::unique_ptr g_main_params{nullptr}; static std::once_flag g_main_params_flag; +static const CService empty_service{}; bool IsNodeOnMainnet() { return Params().NetworkIDString() == CBaseChainParams::MAIN; } const CChainParams& MainParams() @@ -24,6 +25,121 @@ const CChainParams& MainParams() } } // anonymous namespace +bool NetInfoEntry::operator==(const NetInfoEntry& rhs) const +{ + if (m_type != rhs.m_type) return false; + return std::visit( + [](auto&& lhs, auto&& rhs) -> bool { + if constexpr (std::is_same_v) { + return lhs == rhs; + } + return false; + }, + m_data, rhs.m_data); +} + +bool NetInfoEntry::operator<(const NetInfoEntry& rhs) const +{ + if (m_type != rhs.m_type) return m_type < rhs.m_type; + return std::visit( + [](auto&& lhs, auto&& rhs) -> bool { + using T1 = std::decay_t; + using T2 = std::decay_t; + if constexpr (std::is_same_v) { + // Both the same type, compare as usual + return lhs < rhs; + } + // If lhs is monostate, it less than rhs; otherwise rhs is greater + return std::is_same_v; + }, + m_data, rhs.m_data); +} + +std::optional> NetInfoEntry::GetAddrPort() const +{ + if (const auto* data_ptr{std::get_if(&m_data)}; m_type == NetInfoType::Service && data_ptr) { + ASSERT_IF_DEBUG(data_ptr->IsValid()); + return *data_ptr; + } + return std::nullopt; +} + +uint16_t NetInfoEntry::GetPort() const +{ + return std::visit( + [](auto&& input) -> uint16_t { + using T1 = std::decay_t; + if constexpr (std::is_same_v) { + return input.GetPort(); + } + return 0; + }, + m_data); +} + +// NetInfoEntry is a dumb object that doesn't enforce validation rules, that is the responsibility of +// types that utilize NetInfoEntry (MnNetInfo and others). IsTriviallyValid() is there to check if a +// NetInfoEntry object is properly constructed. +bool NetInfoEntry::IsTriviallyValid() const +{ + if (m_type == NetInfoType::Invalid) return false; + return std::visit( + [this](auto&& input) -> bool { + using T1 = std::decay_t; + static_assert(std::is_same_v || std::is_same_v, "Unexpected type"); + if constexpr (std::is_same_v) { + // Empty underlying data isn't a valid entry + return false; + } else if constexpr (std::is_same_v) { + // Type code should be truthful as it decides what underlying type is used when (de)serializing + if (m_type != NetInfoType::Service) return false; + // Underlying data must meet surface-level validity checks for its type + if (!input.IsValid()) return false; + } + return true; + }, + m_data); +} + +std::string NetInfoEntry::ToString() const +{ + return std::visit( + [](auto&& input) -> std::string { + using T1 = std::decay_t; + if constexpr (std::is_same_v) { + return strprintf("CService(addr=%s, port=%u)", input.ToStringAddr(), input.GetPort()); + } + return "[invalid entry]"; + }, + m_data); +} + +std::string NetInfoEntry::ToStringAddr() const +{ + return std::visit( + [](auto&& input) -> std::string { + using T1 = std::decay_t; + if constexpr (std::is_same_v) { + return input.ToStringAddr(); + } + return "[invalid entry]"; + }, + m_data); +} + +std::string NetInfoEntry::ToStringAddrPort() const +{ + return std::visit( + [](auto&& input) -> std::string { + using T1 = std::decay_t; + if constexpr (std::is_same_v) { + return input.ToStringAddrPort(); + } + return "[invalid entry]"; + }, + m_data); +} + NetInfoStatus MnNetInfo::ValidateService(const CService& service) { if (!service.IsValid()) { @@ -52,7 +168,7 @@ NetInfoStatus MnNetInfo::AddEntry(const std::string& input) const auto ret = ValidateService(service.value()); if (ret == NetInfoStatus::Success) { m_addr = service.value(); - ASSERT_IF_DEBUG(m_addr != CService()); + ASSERT_IF_DEBUG(m_addr != empty_service); } return ret; } @@ -63,7 +179,7 @@ CServiceList MnNetInfo::GetEntries() const { CServiceList ret; if (!IsEmpty()) { - ASSERT_IF_DEBUG(m_addr != CService()); + ASSERT_IF_DEBUG(m_addr != empty_service); ret.push_back(m_addr); } // If MnNetInfo is empty, we probably don't expect any entries to show up, so diff --git a/src/evo/netinfo.h b/src/evo/netinfo.h index 07cf6e023632..c6fe3884a0d0 100644 --- a/src/evo/netinfo.h +++ b/src/evo/netinfo.h @@ -8,6 +8,8 @@ #include #include +#include + class CService; enum class NetInfoStatus : uint8_t { @@ -42,6 +44,75 @@ constexpr std::string_view NISToString(const NetInfoStatus code) assert(false); } +class NetInfoEntry +{ +public: + enum NetInfoType : uint8_t { + Service = 0x01, + Invalid = 0xff + }; + +private: + uint8_t m_type{NetInfoType::Invalid}; + std::variant m_data{std::monostate{}}; + +public: + NetInfoEntry() = default; + NetInfoEntry(const CService& service) + { + if (!service.IsValid()) return; + m_type = NetInfoType::Service; + m_data = service; + } + + ~NetInfoEntry() = default; + + bool operator<(const NetInfoEntry& rhs) const; + bool operator==(const NetInfoEntry& rhs) const; + bool operator!=(const NetInfoEntry& rhs) const { return !(*this == rhs); } + + template + void Serialize(Stream& s) const + { + if (const auto* data_ptr{std::get_if(&m_data)}; + m_type == NetInfoType::Service && data_ptr && data_ptr->IsValid()) { + s << m_type << *data_ptr; + } else { + s << NetInfoType::Invalid; + } + } + + template + void Unserialize(Stream& s) + { + s >> m_type; + if (m_type == NetInfoType::Service) { + m_data = CService{}; + try { + CService& service{std::get(m_data)}; + s >> service; + if (!service.IsValid()) { Clear(); } // Invalid CService, mark as invalid + } catch (const std::ios_base::failure&) { Clear(); } // Deser failed, mark as invalid + } else { Clear(); } // Invalid type code, mark as invalid + } + + void Clear() + { + m_type = NetInfoType::Invalid; + m_data = std::monostate{}; + } + + std::optional> GetAddrPort() const; + uint16_t GetPort() const; + bool IsEmpty() const { return *this == NetInfoEntry{}; } + bool IsTriviallyValid() const; + std::string ToString() const; + std::string ToStringAddr() const; + std::string ToStringAddrPort() const; +}; + +template<> struct is_serializable_enum : std::true_type {}; + using CServiceList = std::vector>; class MnNetInfo diff --git a/src/test/evo_netinfo_tests.cpp b/src/test/evo_netinfo_tests.cpp index 2e942b4b5955..d001d8331a7e 100644 --- a/src/test/evo_netinfo_tests.cpp +++ b/src/test/evo_netinfo_tests.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -55,4 +56,86 @@ BOOST_AUTO_TEST_CASE(mnnetinfo_rules) } } +BOOST_AUTO_TEST_CASE(netinfo_ser) +{ + { + // An empty object should only store one byte to denote it is invalid + CDataStream ds(SER_DISK, CLIENT_VERSION); + NetInfoEntry entry{}; + ds << entry; + BOOST_CHECK_EQUAL(ds.size(), sizeof(uint8_t)); + } + + { + // Reading a nonsense byte should return an empty object + CDataStream ds(SER_DISK, CLIENT_VERSION); + NetInfoEntry entry{}; + ds << 0xfe; + ds >> entry; + BOOST_CHECK(entry.IsEmpty() && !entry.IsTriviallyValid()); + } + + { + // Reading an invalid CService should fail trivial validation and return an empty object + CDataStream ds(SER_DISK, CLIENT_VERSION); + NetInfoEntry entry{}; + ds << NetInfoEntry::NetInfoType::Service << CService{}; + ds >> entry; + BOOST_CHECK(entry.IsEmpty() && !entry.IsTriviallyValid()); + } + + { + // Reading an unrecognized type should fail trivial validation and return an empty object + CDataStream ds(SER_DISK, CLIENT_VERSION); + NetInfoEntry entry{}; + ds << NetInfoEntry::NetInfoType::Service << uint256{}; + ds >> entry; + BOOST_CHECK(entry.IsEmpty() && !entry.IsTriviallyValid()); + } + + { + // A valid CService should be constructable, readable and pass validation + CDataStream ds(SER_DISK, CLIENT_VERSION); + CService service{LookupNumeric("1.1.1.1", Params().GetDefaultPort())}; + BOOST_CHECK(service.IsValid()); + NetInfoEntry entry{service}, entry2{}; + ds << NetInfoEntry::NetInfoType::Service << service; + ds >> entry2; + BOOST_CHECK(entry == entry2); + BOOST_CHECK(!entry.IsEmpty() && entry.IsTriviallyValid()); + BOOST_CHECK(entry.GetAddrPort().value() == service); + } +} + +BOOST_AUTO_TEST_CASE(netinfo_retvals) +{ + uint16_t p2p_port{Params().GetDefaultPort()}; + CService service{LookupNumeric("1.1.1.1", p2p_port)}, service2{LookupNumeric("1.1.1.2", p2p_port)}; + NetInfoEntry entry{service}, entry2{service2}, entry_empty{}; + + // Check that values are correctly recorded and pass trivial validation + BOOST_CHECK(service.IsValid()); + BOOST_CHECK(!entry.IsEmpty() && entry.IsTriviallyValid()); + BOOST_CHECK(entry.GetAddrPort().value() == service); + BOOST_CHECK(!entry2.IsEmpty() && entry2.IsTriviallyValid()); + BOOST_CHECK(entry2.GetAddrPort().value() == service2); + + // Check that dispatch returns the expected values + BOOST_CHECK_EQUAL(entry.GetPort(), service.GetPort()); + BOOST_CHECK_EQUAL(entry.ToString(), strprintf("CService(addr=%s, port=%u)", service.ToStringAddr(), service.GetPort())); + BOOST_CHECK_EQUAL(entry.ToStringAddr(), service.ToStringAddr()); + BOOST_CHECK_EQUAL(entry.ToStringAddrPort(), service.ToStringAddrPort()); + BOOST_CHECK_EQUAL(service < service2, entry < entry2); + + // Check that empty/invalid entries return error messages + BOOST_CHECK_EQUAL(entry_empty.GetPort(), 0); + BOOST_CHECK_EQUAL(entry_empty.ToString(), "[invalid entry]"); + BOOST_CHECK_EQUAL(entry_empty.ToStringAddr(), "[invalid entry]"); + BOOST_CHECK_EQUAL(entry_empty.ToStringAddrPort(), "[invalid entry]"); + + // The invalid entry type code is 0xff (highest possible value) and therefore will return as greater + // in comparison to any valid entry + BOOST_CHECK(entry < entry_empty); +} + BOOST_AUTO_TEST_SUITE_END() From b0a634ebf374d35abc63656fa07fe09c91deae3f Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Fri, 9 May 2025 23:30:15 +0000 Subject: [PATCH 3/8] evo: ensure the ADDRV2 serialization is always used in `NetInfoEntry` --- src/evo/netinfo.h | 7 +++++-- src/test/evo_netinfo_tests.cpp | 24 +++++++++++++++++++++++- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/evo/netinfo.h b/src/evo/netinfo.h index c6fe3884a0d0..96af71f2f13d 100644 --- a/src/evo/netinfo.h +++ b/src/evo/netinfo.h @@ -7,6 +7,7 @@ #include #include +#include #include @@ -72,8 +73,9 @@ class NetInfoEntry bool operator!=(const NetInfoEntry& rhs) const { return !(*this == rhs); } template - void Serialize(Stream& s) const + void Serialize(Stream& s_) const { + OverrideStream s(&s_, /*nType=*/0, s_.GetVersion() | ADDRV2_FORMAT); if (const auto* data_ptr{std::get_if(&m_data)}; m_type == NetInfoType::Service && data_ptr && data_ptr->IsValid()) { s << m_type << *data_ptr; @@ -83,8 +85,9 @@ class NetInfoEntry } template - void Unserialize(Stream& s) + void Unserialize(Stream& s_) { + OverrideStream s(&s_, /*nType=*/0, s_.GetVersion() | ADDRV2_FORMAT); s >> m_type; if (m_type == NetInfoType::Service) { m_data = CService{}; diff --git a/src/test/evo_netinfo_tests.cpp b/src/test/evo_netinfo_tests.cpp index d001d8331a7e..cea818a05b81 100644 --- a/src/test/evo_netinfo_tests.cpp +++ b/src/test/evo_netinfo_tests.cpp @@ -95,7 +95,7 @@ BOOST_AUTO_TEST_CASE(netinfo_ser) { // A valid CService should be constructable, readable and pass validation - CDataStream ds(SER_DISK, CLIENT_VERSION); + CDataStream ds(SER_DISK, CLIENT_VERSION | ADDRV2_FORMAT); CService service{LookupNumeric("1.1.1.1", Params().GetDefaultPort())}; BOOST_CHECK(service.IsValid()); NetInfoEntry entry{service}, entry2{}; @@ -105,6 +105,28 @@ BOOST_AUTO_TEST_CASE(netinfo_ser) BOOST_CHECK(!entry.IsEmpty() && entry.IsTriviallyValid()); BOOST_CHECK(entry.GetAddrPort().value() == service); } + + { + // NetInfoEntry should be able to read and write ADDRV2 addresses + CService service{}; + service.SetSpecial("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion"); + BOOST_CHECK(service.IsValid() && service.IsTor()); + + CDataStream ds(SER_DISK, CLIENT_VERSION | ADDRV2_FORMAT); + ds << NetInfoEntry::NetInfoType::Service << service; + ds.SetVersion(CLIENT_VERSION); // Drop the explicit format flag + + NetInfoEntry entry{}; + ds >> entry; + BOOST_CHECK(!entry.IsEmpty() && entry.IsTriviallyValid()); + BOOST_CHECK(entry.GetAddrPort().value() == service); + ds.clear(); + + NetInfoEntry entry2{}; + ds << entry; + ds >> entry2; + BOOST_CHECK(entry == entry2 && entry2.GetAddrPort().value() == service); + } } BOOST_AUTO_TEST_CASE(netinfo_retvals) From 6286c9bd7126df467679f726fee256651d7ff854 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 15 May 2025 12:33:06 +0000 Subject: [PATCH 4/8] evo: change internal type of `MnNetInfo` to `NetInfoEntry` --- src/evo/deterministicmns.cpp | 2 ++ src/evo/netinfo.cpp | 33 +++++++++++++++++++------- src/evo/netinfo.h | 33 +++++++++++++++++++++----- src/test/evo_netinfo_tests.cpp | 43 ++++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 15 deletions(-) diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index f758a34c1849..41c5856d1641 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -1213,6 +1213,8 @@ static bool CheckService(const ProTx& proTx, TxValidationState& state) return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-addr-type"); case NetInfoStatus::NotRoutable: return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-addr-unroutable"); + case NetInfoStatus::Malformed: + return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-bad"); case NetInfoStatus::Success: return true; // Shouldn't be possible during self-checks diff --git a/src/evo/netinfo.cpp b/src/evo/netinfo.cpp index 5bd2990ebd2f..f845cf218992 100644 --- a/src/evo/netinfo.cpp +++ b/src/evo/netinfo.cpp @@ -163,12 +163,11 @@ NetInfoStatus MnNetInfo::ValidateService(const CService& service) NetInfoStatus MnNetInfo::AddEntry(const std::string& input) { - if (auto service = Lookup(input, /*portDefault=*/Params().GetDefaultPort(), /*fAllowLookup=*/false); - service.has_value()) { - const auto ret = ValidateService(service.value()); + if (auto service_opt{Lookup(input, /*portDefault=*/Params().GetDefaultPort(), /*fAllowLookup=*/false)}) { + const auto ret{ValidateService(*service_opt)}; if (ret == NetInfoStatus::Success) { - m_addr = service.value(); - ASSERT_IF_DEBUG(m_addr != empty_service); + m_addr = NetInfoEntry{*service_opt}; + ASSERT_IF_DEBUG(GetPrimary() != empty_service); } return ret; } @@ -179,18 +178,34 @@ CServiceList MnNetInfo::GetEntries() const { CServiceList ret; if (!IsEmpty()) { - ASSERT_IF_DEBUG(m_addr != empty_service); - ret.push_back(m_addr); + ASSERT_IF_DEBUG(GetPrimary() != empty_service); + ret.push_back(GetPrimary()); } // If MnNetInfo is empty, we probably don't expect any entries to show up, so // we return a blank set instead. return ret; } +const CService& MnNetInfo::GetPrimary() const +{ + if (const auto& service_opt{m_addr.GetAddrPort()}) { + return *service_opt; + } + return empty_service; +} + +NetInfoStatus MnNetInfo::Validate() const +{ + if (!m_addr.IsTriviallyValid()) { + return NetInfoStatus::Malformed; + } + return ValidateService(GetPrimary()); +} + std::string MnNetInfo::ToString() const { // Extra padding to account for padding done by the calling function. return strprintf("MnNetInfo()\n" - " CService(addr=%s, port=%u)\n", - m_addr.ToStringAddr(), m_addr.GetPort()); + " %s\n", + m_addr.ToString()); } diff --git a/src/evo/netinfo.h b/src/evo/netinfo.h index 96af71f2f13d..a4c296479c39 100644 --- a/src/evo/netinfo.h +++ b/src/evo/netinfo.h @@ -22,6 +22,7 @@ enum class NetInfoStatus : uint8_t { BadPort, BadType, NotRoutable, + Malformed, Success }; @@ -39,6 +40,8 @@ constexpr std::string_view NISToString(const NetInfoStatus code) return "invalid address type"; case NetInfoStatus::NotRoutable: return "unroutable address"; + case NetInfoStatus::Malformed: + return "malformed"; case NetInfoStatus::Success: return "success"; } // no default case, so the compiler can warn about missing cases @@ -121,7 +124,7 @@ using CServiceList = std::vector>; class MnNetInfo { private: - CService m_addr{}; + NetInfoEntry m_addr{}; private: static NetInfoStatus ValidateService(const CService& service); @@ -133,20 +136,38 @@ class MnNetInfo bool operator==(const MnNetInfo& rhs) const { return m_addr == rhs.m_addr; } bool operator!=(const MnNetInfo& rhs) const { return !(*this == rhs); } - SERIALIZE_METHODS(MnNetInfo, obj) + template + void Serialize(Stream& s) const + { + if (const auto& service{m_addr.GetAddrPort()}; service.has_value()) { + s << service->get(); + } else { + s << CService{}; + } + } + + void Serialize(CSizeComputer& s) const + { + s.seek(::GetSerializeSize(CService{}, s.GetVersion())); + } + + template + void Unserialize(Stream& s) { - READWRITE(obj.m_addr); + CService service; + s >> service; + m_addr = NetInfoEntry{service}; } NetInfoStatus AddEntry(const std::string& service); CServiceList GetEntries() const; - const CService& GetPrimary() const { return m_addr; } + const CService& GetPrimary() const; bool IsEmpty() const { return *this == MnNetInfo(); } - NetInfoStatus Validate() const { return ValidateService(m_addr); } + NetInfoStatus Validate() const; std::string ToString() const; - void Clear() { m_addr = CService(); } + void Clear() { m_addr.Clear(); } }; #endif // BITCOIN_EVO_NETINFO_H diff --git a/src/test/evo_netinfo_tests.cpp b/src/test/evo_netinfo_tests.cpp index cea818a05b81..c16a7725c81b 100644 --- a/src/test/evo_netinfo_tests.cpp +++ b/src/test/evo_netinfo_tests.cpp @@ -49,8 +49,11 @@ BOOST_AUTO_TEST_CASE(mnnetinfo_rules) MnNetInfo netInfo; BOOST_CHECK_EQUAL(netInfo.AddEntry(input), expected_ret); if (expected_ret != NetInfoStatus::Success) { + // An empty MnNetInfo is considered malformed + BOOST_CHECK_EQUAL(netInfo.Validate(), NetInfoStatus::Malformed); BOOST_CHECK(netInfo.GetEntries().empty()); } else { + BOOST_CHECK_EQUAL(netInfo.Validate(), NetInfoStatus::Success); ValidateGetEntries(netInfo.GetEntries(), /*expected_size=*/1); } } @@ -160,4 +163,44 @@ BOOST_AUTO_TEST_CASE(netinfo_retvals) BOOST_CHECK(entry < entry_empty); } +bool CheckIfSerSame(const CService& lhs, const MnNetInfo& rhs) +{ + CHashWriter ss_lhs(SER_GETHASH, 0), ss_rhs(SER_GETHASH, 0); + ss_lhs << lhs; + ss_rhs << rhs; + return ss_lhs.GetSHA256() == ss_rhs.GetSHA256(); +} + +BOOST_AUTO_TEST_CASE(cservice_compatible) +{ + // Empty values should be the same + CService service; + MnNetInfo netInfo; + BOOST_CHECK(CheckIfSerSame(service, netInfo)); + + // Valid IPv4 address, valid port + service = LookupNumeric("1.1.1.1", 9999); + netInfo.Clear(); + BOOST_CHECK_EQUAL(netInfo.AddEntry("1.1.1.1:9999"), NetInfoStatus::Success); + BOOST_CHECK(CheckIfSerSame(service, netInfo)); + + // Valid IPv4 address, default P2P port implied + service = LookupNumeric("1.1.1.1", Params().GetDefaultPort()); + netInfo.Clear(); + BOOST_CHECK_EQUAL(netInfo.AddEntry("1.1.1.1"), NetInfoStatus::Success); + BOOST_CHECK(CheckIfSerSame(service, netInfo)); + + // Lookup() failure (domains not allowed), MnNetInfo should remain empty if Lookup() failed + service = CService(); + netInfo.Clear(); + BOOST_CHECK_EQUAL(netInfo.AddEntry("example.com"), NetInfoStatus::BadInput); + BOOST_CHECK(CheckIfSerSame(service, netInfo)); + + // Validation failure (non-IPv4 not allowed), MnNetInfo should remain empty if ValidateService() failed + service = CService(); + netInfo.Clear(); + BOOST_CHECK_EQUAL(netInfo.AddEntry("[2606:4700:4700::1111]:9999"), NetInfoStatus::BadType); + BOOST_CHECK(CheckIfSerSame(service, netInfo)); +} + BOOST_AUTO_TEST_SUITE_END() From 06cf4eeade7dbf03c0d069727a62a2092ba6f094 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 22 May 2025 15:58:57 +0000 Subject: [PATCH 5/8] evo: return `MnNetInfo::GetEntries()` with `NetInfoEntry` --- src/evo/deterministicmns.cpp | 114 +++++++++++++++++++++++---------- src/evo/deterministicmns.h | 1 + src/evo/netinfo.cpp | 11 ++-- src/evo/netinfo.h | 4 +- src/rpc/masternode.cpp | 2 +- src/test/evo_netinfo_tests.cpp | 5 +- src/txmempool.cpp | 16 ++--- src/txmempool.h | 3 +- 8 files changed, 103 insertions(+), 53 deletions(-) diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index 41c5856d1641..b9e78a27ec62 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -470,11 +470,18 @@ void CDeterministicMNList::AddMN(const CDeterministicMNCPtr& dmn, bool fBumpTota throw(std::runtime_error(strprintf("%s: Can't add a masternode %s with a duplicate collateralOutpoint=%s", __func__, dmn->proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort()))); } - for (const CService& entry : dmn->pdmnState->netInfo.GetEntries()) { - if (!AddUniqueProperty(*dmn, entry)) { + for (const NetInfoEntry& entry : dmn->pdmnState->netInfo.GetEntries()) { + if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) { + const CService& service{service_opt.value()}; + if (!AddUniqueProperty(*dmn, service)) { + mnUniquePropertyMap = mnUniquePropertyMapSaved; + throw std::runtime_error(strprintf("%s: Can't add a masternode %s with a duplicate address=%s", + __func__, dmn->proTxHash.ToString(), service.ToStringAddrPort())); + } + } else { mnUniquePropertyMap = mnUniquePropertyMapSaved; - throw(std::runtime_error(strprintf("%s: Can't add a masternode %s with a duplicate address=%s", __func__, - dmn->proTxHash.ToString(), entry.ToStringAddrPort()))); + throw std::runtime_error( + strprintf("%s: Can't add a masternode %s with invalid address", __func__, dmn->proTxHash.ToString())); } } if (!AddUniqueProperty(*dmn, dmn->pdmnState->keyIDOwner)) { @@ -513,28 +520,40 @@ void CDeterministicMNList::UpdateMN(const CDeterministicMN& oldDmn, const std::s // Using this temporary map as a checkpoint to roll back to in case of any issues. decltype(mnUniquePropertyMap) mnUniquePropertyMapSaved = mnUniquePropertyMap; - const auto updateNetInfo = [&]() { - if (oldState->netInfo != pdmnState->netInfo) { + auto updateNetInfo = [this](const CDeterministicMN& dmn, const MnNetInfo& oldInfo, + const MnNetInfo& newInfo) -> std::string { + if (oldInfo != newInfo) { // We track each individual entry in netInfo as opposed to netInfo itself (preventing us from // using UpdateUniqueProperty()), so we need to successfully purge all old entries and insert // new entries to successfully update. - for (const CService& old_entry : oldState->netInfo.GetEntries()) { - if (!DeleteUniqueProperty(*dmn, old_entry)) { - return strprintf("internal error"); // This shouldn't be possible + for (const NetInfoEntry& old_entry : oldInfo.GetEntries()) { + if (const auto& service_opt{old_entry.GetAddrPort()}) { + const CService& service{service_opt.value()}; + if (!DeleteUniqueProperty(dmn, service)) { + return "internal error"; // This shouldn't be possible + } + } else { + return "invalid address"; } } - for (const CService& new_entry : pdmnState->netInfo.GetEntries()) { - if (!AddUniqueProperty(*dmn, new_entry)) { - return strprintf("duplicate (%s)", new_entry.ToStringAddrPort()); + for (const NetInfoEntry& new_entry : newInfo.GetEntries()) { + if (const auto& service_opt{new_entry.GetAddrPort()}) { + const CService& service{service_opt.value()}; + if (!AddUniqueProperty(dmn, service)) { + return strprintf("duplicate (%s)", service.ToStringAddrPort()); + } + } else { + return "invalid address"; } } } - return strprintf(""); - }(); - if (!updateNetInfo.empty()) { + return ""; + }; + + if (auto err = updateNetInfo(*dmn, oldState->netInfo, pdmnState->netInfo); !err.empty()) { mnUniquePropertyMap = mnUniquePropertyMapSaved; throw(std::runtime_error(strprintf("%s: Can't update masternode %s with addresses, reason=%s", __func__, - oldDmn.proTxHash.ToString(), updateNetInfo))); + oldDmn.proTxHash.ToString(), err))); } if (!UpdateUniqueProperty(*dmn, oldState->keyIDOwner, pdmnState->keyIDOwner)) { mnUniquePropertyMap = mnUniquePropertyMapSaved; @@ -591,11 +610,18 @@ void CDeterministicMNList::RemoveMN(const uint256& proTxHash) throw(std::runtime_error(strprintf("%s: Can't delete a masternode %s with a collateralOutpoint=%s", __func__, proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort()))); } - for (const CService& entry : dmn->pdmnState->netInfo.GetEntries()) { - if (!DeleteUniqueProperty(*dmn, entry)) { + for (const NetInfoEntry& entry : dmn->pdmnState->netInfo.GetEntries()) { + if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) { + const CService& service{service_opt.value()}; + if (!DeleteUniqueProperty(*dmn, service)) { + mnUniquePropertyMap = mnUniquePropertyMapSaved; + throw std::runtime_error(strprintf("%s: Can't delete a masternode %s with an address=%s", __func__, + proTxHash.ToString(), service.ToStringAddrPort())); + } + } else { mnUniquePropertyMap = mnUniquePropertyMapSaved; - throw(std::runtime_error(strprintf("%s: Can't delete a masternode %s with an address=%s", __func__, - proTxHash.ToString(), entry.ToStringAddrPort()))); + throw std::runtime_error(strprintf("%s: Can't delete a masternode %s with invalid address", __func__, + dmn->proTxHash.ToString())); } } if (!DeleteUniqueProperty(*dmn, dmn->pdmnState->keyIDOwner)) { @@ -811,9 +837,14 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::no } } - for (const CService& entry : proTx.netInfo.GetEntries()) { - if (newList.HasUniqueProperty(entry)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry"); + for (const NetInfoEntry& entry : proTx.netInfo.GetEntries()) { + if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) { + const CService& service{service_opt.value()}; + if (newList.HasUniqueProperty(service)) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry"); + } + } else { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-netinfo-entry"); } } if (newList.HasUniqueProperty(proTx.keyIDOwner) || newList.HasUniqueProperty(proTx.pubKeyOperator)) { @@ -842,10 +873,15 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::no return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); } - for (const CService& entry : opt_proTx->netInfo.GetEntries()) { - if (newList.HasUniqueProperty(entry) && - newList.GetUniquePropertyMN(entry)->proTxHash != opt_proTx->proTxHash) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry"); + for (const NetInfoEntry& entry : opt_proTx->netInfo.GetEntries()) { + if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) { + const CService& service{service_opt.value()}; + if (newList.HasUniqueProperty(service) && + newList.GetUniquePropertyMN(service)->proTxHash != opt_proTx->proTxHash) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry"); + } + } else { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-netinfo-entry"); } } @@ -1382,10 +1418,15 @@ bool CheckProRegTx(CDeterministicMNManager& dmnman, const CTransaction& tx, gsl: auto mnList = dmnman.GetListForBlock(pindexPrev); // only allow reusing of addresses when it's for the same collateral (which replaces the old MN) - for (const CService& entry : opt_ptx->netInfo.GetEntries()) { - if (mnList.HasUniqueProperty(entry) && - mnList.GetUniquePropertyMN(entry)->collateralOutpoint != collateralOutpoint) { - return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-netinfo-entry"); + for (const NetInfoEntry& entry : opt_ptx->netInfo.GetEntries()) { + if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) { + const CService& service{service_opt.value()}; + if (mnList.HasUniqueProperty(service) && + mnList.GetUniquePropertyMN(service)->collateralOutpoint != collateralOutpoint) { + return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-netinfo-entry"); + } + } else { + return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-entry"); } } @@ -1455,9 +1496,14 @@ bool CheckProUpServTx(CDeterministicMNManager& dmnman, const CTransaction& tx, g } // don't allow updating to addresses already used by other MNs - for (const CService& entry : opt_ptx->netInfo.GetEntries()) { - if (mnList.HasUniqueProperty(entry) && mnList.GetUniquePropertyMN(entry)->proTxHash != opt_ptx->proTxHash) { - return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-netinfo-entry"); + for (const NetInfoEntry& entry : opt_ptx->netInfo.GetEntries()) { + if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) { + const CService& service{service_opt.value()}; + if (mnList.HasUniqueProperty(service) && mnList.GetUniquePropertyMN(service)->proTxHash != opt_ptx->proTxHash) { + return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-netinfo-entry"); + } + } else { + return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-entry"); } } diff --git a/src/evo/deterministicmns.h b/src/evo/deterministicmns.h index 386d98a23063..bb94a6aa4da0 100644 --- a/src/evo/deterministicmns.h +++ b/src/evo/deterministicmns.h @@ -398,6 +398,7 @@ class CDeterministicMNList static_assert(!std::is_same_v, name>, "GetUniquePropertyHash cannot be templated against " #name) DMNL_NO_TEMPLATE(CBLSPublicKey); DMNL_NO_TEMPLATE(MnNetInfo); + DMNL_NO_TEMPLATE(NetInfoEntry); #undef DMNL_NO_TEMPLATE return ::SerializeHash(v); } diff --git a/src/evo/netinfo.cpp b/src/evo/netinfo.cpp index f845cf218992..a3aeb2f31ad7 100644 --- a/src/evo/netinfo.cpp +++ b/src/evo/netinfo.cpp @@ -167,23 +167,22 @@ NetInfoStatus MnNetInfo::AddEntry(const std::string& input) const auto ret{ValidateService(*service_opt)}; if (ret == NetInfoStatus::Success) { m_addr = NetInfoEntry{*service_opt}; - ASSERT_IF_DEBUG(GetPrimary() != empty_service); + ASSERT_IF_DEBUG(m_addr.GetAddrPort().has_value()); } return ret; } return NetInfoStatus::BadInput; } -CServiceList MnNetInfo::GetEntries() const +NetInfoList MnNetInfo::GetEntries() const { - CServiceList ret; if (!IsEmpty()) { - ASSERT_IF_DEBUG(GetPrimary() != empty_service); - ret.push_back(GetPrimary()); + ASSERT_IF_DEBUG(m_addr.GetAddrPort().has_value()); + return {m_addr}; } // If MnNetInfo is empty, we probably don't expect any entries to show up, so // we return a blank set instead. - return ret; + return {}; } const CService& MnNetInfo::GetPrimary() const diff --git a/src/evo/netinfo.h b/src/evo/netinfo.h index a4c296479c39..7da23572201b 100644 --- a/src/evo/netinfo.h +++ b/src/evo/netinfo.h @@ -119,7 +119,7 @@ class NetInfoEntry template<> struct is_serializable_enum : std::true_type {}; -using CServiceList = std::vector>; +using NetInfoList = std::vector>; class MnNetInfo { @@ -160,7 +160,7 @@ class MnNetInfo } NetInfoStatus AddEntry(const std::string& service); - CServiceList GetEntries() const; + NetInfoList GetEntries() const; const CService& GetPrimary() const; bool IsEmpty() const { return *this == MnNetInfo(); } diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index 601ad845b4b1..7f98c03b3af7 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -568,7 +568,7 @@ static RPCHelpMan masternodelist_helper(bool is_composite) std::string strAddress{}; if (strMode == "addr" || strMode == "full" || strMode == "info" || strMode == "json" || strMode == "recent" || strMode == "evo") { - for (const CService& entry : dmn.pdmnState->netInfo.GetEntries()) { + for (const NetInfoEntry& entry : dmn.pdmnState->netInfo.GetEntries()) { strAddress += entry.ToStringAddrPort() + " "; } if (!strAddress.empty()) strAddress.pop_back(); // Remove trailing space diff --git a/src/test/evo_netinfo_tests.cpp b/src/test/evo_netinfo_tests.cpp index c16a7725c81b..fc6768498e8c 100644 --- a/src/test/evo_netinfo_tests.cpp +++ b/src/test/evo_netinfo_tests.cpp @@ -37,9 +37,12 @@ const std::vector(tx)); mapProTxRefs.emplace(proTx.proTxHash, tx_hash); - for (const CService& entry : proTx.netInfo.GetEntries()) { + for (const NetInfoEntry& entry : proTx.netInfo.GetEntries()) { mapProTxAddresses.emplace(entry, tx_hash); } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { @@ -795,7 +795,7 @@ void CTxMemPool::removeUncheckedProTx(const CTransaction& tx) if (!proTx.collateralOutpoint.IsNull()) { eraseProTxRef(tx_hash, proTx.collateralOutpoint.hash); } - for (const CService& entry : proTx.netInfo.GetEntries()) { + for (const NetInfoEntry& entry : proTx.netInfo.GetEntries()) { mapProTxAddresses.erase(entry); } mapProTxPubKeyIDs.erase(proTx.keyIDOwner); @@ -805,7 +805,7 @@ void CTxMemPool::removeUncheckedProTx(const CTransaction& tx) } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { auto proTx = *Assert(GetTxPayload(tx)); eraseProTxRef(proTx.proTxHash, tx_hash); - for (const CService& entry : proTx.netInfo.GetEntries()) { + for (const NetInfoEntry& entry : proTx.netInfo.GetEntries()) { mapProTxAddresses.erase(entry); } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { @@ -1034,7 +1034,7 @@ void CTxMemPool::removeProTxConflicts(const CTransaction &tx) } auto& proTx = *opt_proTx; - for (const CService& entry : proTx.netInfo.GetEntries()) { + for (const NetInfoEntry& entry : proTx.netInfo.GetEntries()) { if (mapProTxAddresses.count(entry)) { uint256 conflictHash = mapProTxAddresses[entry]; if (conflictHash != tx_hash && mapTx.count(conflictHash)) { @@ -1056,7 +1056,7 @@ void CTxMemPool::removeProTxConflicts(const CTransaction &tx) return; } - for (const CService& entry : opt_proTx->netInfo.GetEntries()) { + for (const NetInfoEntry& entry : opt_proTx->netInfo.GetEntries()) { if (mapProTxAddresses.count(entry)) { uint256 conflictHash = mapProTxAddresses[entry]; if (conflictHash != tx_hash && mapTx.count(conflictHash)) { @@ -1394,7 +1394,7 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { return true; // i.e. can't decode payload == conflict } auto& proTx = *opt_proTx; - for (const CService& entry : proTx.netInfo.GetEntries()) { + for (const NetInfoEntry& entry : proTx.netInfo.GetEntries()) { if (mapProTxAddresses.count(entry)) { return true; } @@ -1419,7 +1419,7 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { LogPrint(BCLog::MEMPOOL, "%s: ERROR: Invalid transaction payload, tx: %s\n", __func__, tx_hash.ToString()); return true; // i.e. can't decode payload == conflict } - for (const CService& entry : opt_proTx->netInfo.GetEntries()) { + for (const NetInfoEntry& entry : opt_proTx->netInfo.GetEntries()) { auto it = mapProTxAddresses.find(entry); if (it != mapProTxAddresses.end() && it->second != opt_proTx->proTxHash) { return true; diff --git a/src/txmempool.h b/src/txmempool.h index 9bdc667dc4d0..6bb254eb2205 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -525,7 +526,7 @@ class CTxMemPool std::map> mapSpentInserted; std::multimap mapProTxRefs; // proTxHash -> transaction (all TXs that refer to an existing proTx) - std::map mapProTxAddresses; + std::map mapProTxAddresses; std::map mapProTxPubKeyIDs; std::map mapProTxBlsPubKeyHashes; std::map mapProTxCollaterals; From c69184bbfd2366492486702dd20a903c5ae41ec1 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sat, 3 May 2025 16:22:01 +0000 Subject: [PATCH 6/8] evo: utilize `NetInfoEntry::IsTriviallyValid()` in ProTx trivial checks Also, ProRegTx allows an empty netInfo, ProUpServTx does not, so we should check that ProUpServTx::netInfo isn't empty. --- src/evo/providertx.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/evo/providertx.cpp b/src/evo/providertx.cpp index cb869fc2b88c..c2684fc0758b 100644 --- a/src/evo/providertx.cpp +++ b/src/evo/providertx.cpp @@ -36,6 +36,11 @@ bool CProRegTx::IsTriviallyValid(bool is_basic_scheme_active, TxValidationState& if (!scriptPayout.IsPayToPublicKeyHash() && !scriptPayout.IsPayToScriptHash()) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-payee"); } + for (const NetInfoEntry& entry : netInfo.GetEntries()) { + if (!entry.IsTriviallyValid()) { + return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-bad"); + } + } CTxDestination payoutDest; if (!ExtractDestination(scriptPayout, payoutDest)) { @@ -105,6 +110,14 @@ bool CProUpServTx::IsTriviallyValid(bool is_basic_scheme_active, TxValidationSta if (nVersion < ProTxVersion::BasicBLS && nType == MnType::Evo) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-evo-version"); } + if (netInfo.IsEmpty()) { + return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-empty"); + } + for (const NetInfoEntry& entry : netInfo.GetEntries()) { + if (!entry.IsTriviallyValid()) { + return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-bad"); + } + } return true; } From e0a1c643804a4f2eec5f3108e54bb2d821c9f3ac Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Fri, 9 May 2025 17:40:30 +0000 Subject: [PATCH 7/8] evo: prohibit overwriting entry in `MnNetInfo` --- src/evo/deterministicmns.cpp | 1 + src/evo/netinfo.cpp | 3 +++ src/evo/netinfo.h | 3 +++ src/test/evo_netinfo_tests.cpp | 8 ++++++++ 4 files changed, 15 insertions(+) diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index b9e78a27ec62..8e98517e54fc 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -1255,6 +1255,7 @@ static bool CheckService(const ProTx& proTx, TxValidationState& state) return true; // Shouldn't be possible during self-checks case NetInfoStatus::BadInput: + case NetInfoStatus::MaxLimit: assert(false); } // no default case, so the compiler can warn about missing cases assert(false); diff --git a/src/evo/netinfo.cpp b/src/evo/netinfo.cpp index a3aeb2f31ad7..8a3a1fe9f542 100644 --- a/src/evo/netinfo.cpp +++ b/src/evo/netinfo.cpp @@ -163,6 +163,9 @@ NetInfoStatus MnNetInfo::ValidateService(const CService& service) NetInfoStatus MnNetInfo::AddEntry(const std::string& input) { + if (!IsEmpty()) { + return NetInfoStatus::MaxLimit; + } if (auto service_opt{Lookup(input, /*portDefault=*/Params().GetDefaultPort(), /*fAllowLookup=*/false)}) { const auto ret{ValidateService(*service_opt)}; if (ret == NetInfoStatus::Success) { diff --git a/src/evo/netinfo.h b/src/evo/netinfo.h index 7da23572201b..e09de3890ec9 100644 --- a/src/evo/netinfo.h +++ b/src/evo/netinfo.h @@ -16,6 +16,7 @@ class CService; enum class NetInfoStatus : uint8_t { // Managing entries BadInput, + MaxLimit, // Validation BadAddress, @@ -42,6 +43,8 @@ constexpr std::string_view NISToString(const NetInfoStatus code) return "unroutable address"; case NetInfoStatus::Malformed: return "malformed"; + case NetInfoStatus::MaxLimit: + return "too many entries"; case NetInfoStatus::Success: return "success"; } // no default case, so the compiler can warn about missing cases diff --git a/src/test/evo_netinfo_tests.cpp b/src/test/evo_netinfo_tests.cpp index fc6768498e8c..cb8f61bd6460 100644 --- a/src/test/evo_netinfo_tests.cpp +++ b/src/test/evo_netinfo_tests.cpp @@ -60,6 +60,14 @@ BOOST_AUTO_TEST_CASE(mnnetinfo_rules) ValidateGetEntries(netInfo.GetEntries(), /*expected_size=*/1); } } + + { + // MnNetInfo only stores one value, overwriting prohibited + MnNetInfo netInfo; + BOOST_CHECK_EQUAL(netInfo.AddEntry("1.1.1.1:9999"), NetInfoStatus::Success); + BOOST_CHECK_EQUAL(netInfo.AddEntry("1.1.1.2:9999"), NetInfoStatus::MaxLimit); + ValidateGetEntries(netInfo.GetEntries(), /*expected_size=*/1); + } } BOOST_AUTO_TEST_CASE(netinfo_ser) From 3a72f2f864dffba51234470fe6d38ef37a1924a8 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 22 May 2025 17:44:35 +0000 Subject: [PATCH 8/8] evo: fast-fail `MnNetInfo::AddEntry()` if invalid characters found --- src/evo/netinfo.cpp | 18 +++++++++++++++++- src/test/evo_netinfo_tests.cpp | 4 ++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/evo/netinfo.cpp b/src/evo/netinfo.cpp index 8a3a1fe9f542..b25818b6aecf 100644 --- a/src/evo/netinfo.cpp +++ b/src/evo/netinfo.cpp @@ -15,6 +15,8 @@ static std::unique_ptr g_main_params{nullptr}; static std::once_flag g_main_params_flag; static const CService empty_service{}; +static constexpr std::string_view SAFE_CHARS_IPV4{"1234567890."}; + bool IsNodeOnMainnet() { return Params().NetworkIDString() == CBaseChainParams::MAIN; } const CChainParams& MainParams() { @@ -23,6 +25,11 @@ const CChainParams& MainParams() [&]() { g_main_params = CreateChainParams(ArgsManager{}, CBaseChainParams::MAIN); }); return *Assert(g_main_params); } + +bool MatchCharsFilter(std::string_view input, std::string_view filter) +{ + return std::all_of(input.begin(), input.end(), [&filter](char c) { return filter.find(c) != std::string_view::npos; }); +} } // anonymous namespace bool NetInfoEntry::operator==(const NetInfoEntry& rhs) const @@ -166,7 +173,16 @@ NetInfoStatus MnNetInfo::AddEntry(const std::string& input) if (!IsEmpty()) { return NetInfoStatus::MaxLimit; } - if (auto service_opt{Lookup(input, /*portDefault=*/Params().GetDefaultPort(), /*fAllowLookup=*/false)}) { + + std::string addr; + uint16_t port{Params().GetDefaultPort()}; + SplitHostPort(input, port, addr); + // Contains invalid characters, unlikely to pass Lookup(), fast-fail + if (!MatchCharsFilter(addr, SAFE_CHARS_IPV4)) { + return NetInfoStatus::BadInput; + } + + if (auto service_opt{Lookup(addr, /*portDefault=*/port, /*fAllowLookup=*/false)}) { const auto ret{ValidateService(*service_opt)}; if (ret == NetInfoStatus::Success) { m_addr = NetInfoEntry{*service_opt}; diff --git a/src/test/evo_netinfo_tests.cpp b/src/test/evo_netinfo_tests.cpp index cb8f61bd6460..234f149813d5 100644 --- a/src/test/evo_netinfo_tests.cpp +++ b/src/test/evo_netinfo_tests.cpp @@ -28,7 +28,7 @@ const std::vector