From 07a9eccb60485e71494664cc2b1964ae06a3dcf0 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 1 Sep 2021 14:14:21 +0100 Subject: [PATCH 1/4] [net] Remove CConnman::Options.m_asmap This data member was introduced in ec45646de9e but never used. --- src/net.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/net.h b/src/net.h index a884a4521d..e7ad959fd9 100644 --- a/src/net.h +++ b/src/net.h @@ -768,7 +768,6 @@ public: bool m_use_addrman_outgoing = true; std::vector m_specified_outgoing; std::vector m_added_nodes; - std::vector m_asmap; bool m_i2p_accept_incoming; }; From bfdf4ef334a16ef6108a658bf4f8514754128c18 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Tue, 7 Sep 2021 13:31:10 +0100 Subject: [PATCH 2/4] [asmap] Remove SanityCheckASMap() from netaddress SanityCheckASMap(asmap, bits) simply calls through to SanityCheckASMap(asmap) in util/asmap. Update all callers to simply call that function. --- src/addrman.cpp | 3 ++- src/netaddress.cpp | 5 ----- src/netaddress.h | 2 -- src/test/fuzz/addrman.cpp | 2 +- src/test/fuzz/asmap.cpp | 3 ++- src/test/fuzz/net.cpp | 3 ++- 6 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 986a1a5d4b..fd0d3dc6fa 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -1028,7 +1029,7 @@ std::vector CAddrMan::DecodeAsmap(fs::path path) bits.push_back((cur_byte >> bit) & 1); } } - if (!SanityCheckASMap(bits)) { + if (!SanityCheckASMap(bits, 128)) { LogPrintf("Sanity check of asmap file %s failed\n", path); return {}; } diff --git a/src/netaddress.cpp b/src/netaddress.cpp index e7b3377475..b2f4945e3b 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -1242,8 +1242,3 @@ bool operator<(const CSubNet& a, const CSubNet& b) { return (a.network < b.network || (a.network == b.network && memcmp(a.netmask, b.netmask, 16) < 0)); } - -bool SanityCheckASMap(const std::vector& asmap) -{ - return SanityCheckASMap(asmap, 128); // For IP address lookups, the input is 128 bits -} diff --git a/src/netaddress.h b/src/netaddress.h index eb35ed3fac..cfb2edcd34 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -567,6 +567,4 @@ public: } }; -bool SanityCheckASMap(const std::vector& asmap); - #endif // BITCOIN_NETADDRESS_H diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index e95126a80f..fdbfb3b93b 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -221,7 +221,7 @@ public: [[nodiscard]] inline std::vector ConsumeAsmap(FuzzedDataProvider& fuzzed_data_provider) noexcept { std::vector asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider); - if (!SanityCheckASMap(asmap)) asmap.clear(); + if (!SanityCheckASMap(asmap, 128)) asmap.clear(); return asmap; } diff --git a/src/test/fuzz/asmap.cpp b/src/test/fuzz/asmap.cpp index 4c5bc0cbf2..d402f8632c 100644 --- a/src/test/fuzz/asmap.cpp +++ b/src/test/fuzz/asmap.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -42,7 +43,7 @@ FUZZ_TARGET(asmap) asmap.push_back((buffer[1 + i] >> j) & 1); } } - if (!SanityCheckASMap(asmap)) return; + if (!SanityCheckASMap(asmap, 128)) return; const uint8_t* addr_data = buffer.data() + 1 + asmap_size; CNetAddr net_addr; diff --git a/src/test/fuzz/net.cpp b/src/test/fuzz/net.cpp index ff0259c182..0929376045 100644 --- a/src/test/fuzz/net.cpp +++ b/src/test/fuzz/net.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -39,7 +40,7 @@ FUZZ_TARGET_INIT(net, initialize_net) }, [&] { const std::vector asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider); - if (!SanityCheckASMap(asmap)) { + if (!SanityCheckASMap(asmap, 128)) { return; } CNodeStats stats; From 9fd5618610e91e3949536c5122cf31eb58c9aa6b Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 1 Sep 2021 12:41:47 +0100 Subject: [PATCH 3/4] [asmap] Make DecodeAsmap() a utility function DecopeAsmap is a pure utility function and doesn't have any dependencies on addrman, so move it to util/asmap. Reviewer hint: use: `git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space` --- src/addrman.cpp | 28 ---------------------------- src/addrman.h | 3 --- src/init.cpp | 2 +- src/util/asmap.cpp | 38 ++++++++++++++++++++++++++++++++++++-- src/util/asmap.h | 7 ++++++- 5 files changed, 43 insertions(+), 35 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index fd0d3dc6fa..49226dde42 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -9,7 +9,6 @@ #include #include #include -#include #include #include @@ -1008,30 +1007,3 @@ CAddrInfo CAddrMan::SelectTriedCollision_() return mapInfo[id_old]; } - -std::vector CAddrMan::DecodeAsmap(fs::path path) -{ - std::vector bits; - FILE *filestr = fsbridge::fopen(path, "rb"); - CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); - if (file.IsNull()) { - LogPrintf("Failed to open asmap file from disk\n"); - return bits; - } - fseek(filestr, 0, SEEK_END); - int length = ftell(filestr); - LogPrintf("Opened asmap file %s (%d bytes) from disk\n", path, length); - fseek(filestr, 0, SEEK_SET); - uint8_t cur_byte; - for (int i = 0; i < length; ++i) { - file >> cur_byte; - for (int bit = 0; bit < 8; ++bit) { - bits.push_back((cur_byte >> bit) & 1); - } - } - if (!SanityCheckASMap(bits, 128)) { - LogPrintf("Sanity check of asmap file %s failed\n", path); - return {}; - } - return bits; -} diff --git a/src/addrman.h b/src/addrman.h index 74bfe9748b..48e0f8b871 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -149,9 +149,6 @@ static constexpr int ADDRMAN_BUCKET_SIZE{1 << ADDRMAN_BUCKET_SIZE_LOG2}; class CAddrMan { public: - // Read asmap from provided binary file - static std::vector DecodeAsmap(fs::path path); - template void Serialize(Stream& s_) const EXCLUSIVE_LOCKS_REQUIRED(!cs); diff --git a/src/init.cpp b/src/init.cpp index b744298667..1d18d80ec4 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1189,7 +1189,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) InitError(strprintf(_("Could not find asmap file %s"), asmap_path)); return false; } - asmap = CAddrMan::DecodeAsmap(asmap_path); + asmap = DecodeAsmap(asmap_path); if (asmap.size() == 0) { InitError(strprintf(_("Could not parse asmap file %s"), asmap_path)); return false; diff --git a/src/util/asmap.cpp b/src/util/asmap.cpp index bacc3690a2..5695c62012 100644 --- a/src/util/asmap.cpp +++ b/src/util/asmap.cpp @@ -2,10 +2,16 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include + +#include +#include +#include +#include + +#include #include #include -#include -#include namespace { @@ -183,3 +189,31 @@ bool SanityCheckASMap(const std::vector& asmap, int bits) } return false; // Reached EOF without RETURN instruction } + +std::vector DecodeAsmap(fs::path path) +{ + std::vector bits; + FILE *filestr = fsbridge::fopen(path, "rb"); + CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); + if (file.IsNull()) { + LogPrintf("Failed to open asmap file from disk\n"); + return bits; + } + fseek(filestr, 0, SEEK_END); + int length = ftell(filestr); + LogPrintf("Opened asmap file %s (%d bytes) from disk\n", path, length); + fseek(filestr, 0, SEEK_SET); + uint8_t cur_byte; + for (int i = 0; i < length; ++i) { + file >> cur_byte; + for (int bit = 0; bit < 8; ++bit) { + bits.push_back((cur_byte >> bit) & 1); + } + } + if (!SanityCheckASMap(bits, 128)) { + LogPrintf("Sanity check of asmap file %s failed\n", path); + return {}; + } + return bits; +} + diff --git a/src/util/asmap.h b/src/util/asmap.h index d0588bc8c3..810d70b9a1 100644 --- a/src/util/asmap.h +++ b/src/util/asmap.h @@ -5,11 +5,16 @@ #ifndef BITCOIN_UTIL_ASMAP_H #define BITCOIN_UTIL_ASMAP_H -#include +#include + +#include #include uint32_t Interpret(const std::vector &asmap, const std::vector &ip); bool SanityCheckASMap(const std::vector& asmap, int bits); +/** Read asmap from provided binary file */ +std::vector DecodeAsmap(fs::path path); + #endif // BITCOIN_UTIL_ASMAP_H From 853c4edb70f897a6a7165abaea4a303d7d448721 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 1 Sep 2021 11:24:46 +0100 Subject: [PATCH 4/4] [net] Remove asmap argument from CNode::CopyStats() This saves passing around a reference to the asmap std::vector. --- src/net.cpp | 6 +++--- src/net.h | 2 +- src/test/fuzz/net.cpp | 6 +----- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 35376b89ac..c8a6e2c763 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -552,14 +552,13 @@ Network CNode::ConnectedThroughNetwork() const #undef X #define X(name) stats.name = name -void CNode::CopyStats(CNodeStats& stats, const std::vector& asmap) +void CNode::CopyStats(CNodeStats& stats) { stats.nodeid = this->GetId(); X(nServices); X(addr); X(addrBind); stats.m_network = ConnectedThroughNetwork(); - stats.m_mapped_as = addr.GetMappedAS(asmap); if (m_tx_relay != nullptr) { LOCK(m_tx_relay->cs_filter); stats.fRelayTxes = m_tx_relay->fRelayTxes; @@ -2804,7 +2803,8 @@ void CConnman::GetNodeStats(std::vector& vstats) const vstats.reserve(vNodes.size()); for (CNode* pnode : vNodes) { vstats.emplace_back(); - pnode->CopyStats(vstats.back(), addrman.GetAsmap()); + pnode->CopyStats(vstats.back()); + vstats.back().m_mapped_as = pnode->addr.GetMappedAS(addrman.GetAsmap()); } } diff --git a/src/net.h b/src/net.h index e7ad959fd9..1479861bc2 100644 --- a/src/net.h +++ b/src/net.h @@ -652,7 +652,7 @@ public: void CloseSocketDisconnect(); - void CopyStats(CNodeStats& stats, const std::vector& asmap); + void CopyStats(CNodeStats& stats); ServiceFlags GetLocalServices() const { diff --git a/src/test/fuzz/net.cpp b/src/test/fuzz/net.cpp index 0929376045..bd1bb79d0e 100644 --- a/src/test/fuzz/net.cpp +++ b/src/test/fuzz/net.cpp @@ -39,12 +39,8 @@ FUZZ_TARGET_INIT(net, initialize_net) node.CloseSocketDisconnect(); }, [&] { - const std::vector asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider); - if (!SanityCheckASMap(asmap, 128)) { - return; - } CNodeStats stats; - node.CopyStats(stats, asmap); + node.CopyStats(stats); }, [&] { const CNode* add_ref_node = node.AddRef();