diff --git a/src/addrman.cpp b/src/addrman.cpp index 608d116a5e3a..4e3f33245a20 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -846,6 +846,30 @@ std::vector AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct return addresses; } +std::vector> AddrManImpl::GetEntries_(bool from_tried) const +{ + AssertLockHeld(cs); + + const int bucket_count = from_tried ? ADDRMAN_TRIED_BUCKET_COUNT : ADDRMAN_NEW_BUCKET_COUNT; + std::vector> infos; + for (int bucket = 0; bucket < bucket_count; ++bucket) { + for (int position = 0; position < ADDRMAN_BUCKET_SIZE; ++position) { + int id = GetEntry(from_tried, bucket, position); + if (id >= 0) { + AddrInfo info = mapInfo.at(id); + AddressPosition location = AddressPosition( + from_tried, + /*multiplicity_in=*/from_tried ? 1 : info.nRefCount, + bucket, + position); + infos.push_back(std::make_pair(info, location)); + } + } + } + + return infos; +} + void AddrManImpl::Connected_(const CService& addr, NodeSeconds time) { AssertLockHeld(cs); @@ -1218,6 +1242,15 @@ std::vector AddrManImpl::GetAddr(size_t max_addresses, size_t max_pct, return addresses; } +std::vector> AddrManImpl::GetEntries(bool from_tried) const +{ + LOCK(cs); + Check(); + auto addrInfos = GetEntries_(from_tried); + Check(); + return addrInfos; +} + void AddrManImpl::Connected(const CService& addr, NodeSeconds time) { LOCK(cs); @@ -1320,6 +1353,11 @@ std::vector AddrMan::GetAddr(size_t max_addresses, size_t max_pct, std return m_impl->GetAddr(max_addresses, max_pct, network, filtered); } +std::vector> AddrMan::GetEntries(bool use_tried) const +{ + return m_impl->GetEntries(use_tried); +} + void AddrMan::Connected(const CService& addr, NodeSeconds time) { m_impl->Connected(addr, time); diff --git a/src/addrman.h b/src/addrman.h index 61693baf4fc8..dd764516282d 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -40,7 +40,7 @@ class AddrManImpl; /** Default for -checkaddrman */ static constexpr int32_t DEFAULT_ADDRMAN_CONSISTENCY_CHECKS{0}; -/** Test-only struct, capturing info about an address in AddrMan */ +/** Location information for an address in AddrMan */ struct AddressPosition { // Whether the address is in the new or tried table const bool tried; @@ -183,6 +183,17 @@ class AddrMan */ std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const; + /** + * Returns an information-location pair for all addresses in the selected addrman table. + * If an address appears multiple times in the new table, an information-location pair + * is returned for each occurence. Addresses only ever appear once in the tried table. + * + * @param[in] from_tried Selects which table to return entries from. + * + * @return A vector consisting of pairs of AddrInfo and AddressPosition. + */ + std::vector> GetEntries(bool from_tried) const; + /** We have successfully connected to this peer. Calling this function * updates the CAddress's nTime, which is used in our IsTerrible() * decisions and gossiped to peers. Callers should be careful that updating diff --git a/src/addrman_impl.h b/src/addrman_impl.h index 890685260eeb..656b218b3c10 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -133,6 +133,9 @@ class AddrManImpl std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const EXCLUSIVE_LOCKS_REQUIRED(!cs); + std::vector> GetEntries(bool from_tried) const + EXCLUSIVE_LOCKS_REQUIRED(!cs); + void Connected(const CService& addr, NodeSeconds time) EXCLUSIVE_LOCKS_REQUIRED(!cs); @@ -264,6 +267,8 @@ class AddrManImpl std::vector GetAddr_(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const EXCLUSIVE_LOCKS_REQUIRED(cs); + std::vector> GetEntries_(bool from_tried) const EXCLUSIVE_LOCKS_REQUIRED(cs); + void Connected_(const CService& addr, NodeSeconds time) EXCLUSIVE_LOCKS_REQUIRED(cs); void SetServices_(const CService& addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 655860d6b700..797e91ff457e 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -458,10 +458,10 @@ UniValue RPCConvertNamedValues(std::string strMethod, const std::vector #include +#include #include #include #include @@ -1174,6 +1175,74 @@ static RPCHelpMan getaddrmaninfo() }; } +UniValue AddrmanEntryToJSON(const AddrInfo& info) +{ + UniValue ret(UniValue::VOBJ); + ret.pushKV("address", info.ToStringAddr()); + ret.pushKV("port", info.GetPort()); + ret.pushKV("services", (uint64_t)info.nServices); + ret.pushKV("time", int64_t{TicksSinceEpoch(info.nTime)}); + ret.pushKV("network", GetNetworkName(info.GetNetClass())); + ret.pushKV("source", info.source.ToStringAddr()); + ret.pushKV("source_network", GetNetworkName(info.source.GetNetClass())); + return ret; +} + +UniValue AddrmanTableToJSON(const std::vector>& tableInfos) +{ + UniValue table(UniValue::VOBJ); + for (const auto& e : tableInfos) { + AddrInfo info = e.first; + AddressPosition location = e.second; + std::ostringstream key; + key << location.bucket << "/" << location.position; + // Address manager tables have unique entries so there is no advantage + // in using UniValue::pushKV, which checks if the key already exists + // in O(N). UniValue::pushKVEnd is used instead which currently is O(1). + table.pushKVEnd(key.str(), AddrmanEntryToJSON(info)); + } + return table; +} + +static RPCHelpMan getrawaddrman() +{ + return RPCHelpMan{"getrawaddrman", + "EXPERIMENTAL warning: this call may be changed in future releases.\n" + "\nReturns information on all address manager entries for the new and tried tables.\n", + {}, + RPCResult{ + RPCResult::Type::OBJ_DYN, "", "", { + {RPCResult::Type::OBJ_DYN, "table", "buckets with addresses in the address manager table ( new, tried )", { + {RPCResult::Type::OBJ, "bucket/position", "the location in the address manager table (/)", { + {RPCResult::Type::STR, "address", "The address of the node"}, + {RPCResult::Type::NUM, "port", "The port number of the node"}, + {RPCResult::Type::STR, "network", "The network (" + Join(GetNetworkNames(), ", ") + ") of the address"}, + {RPCResult::Type::NUM, "services", "The services offered by the node"}, + {RPCResult::Type::NUM_TIME, "time", "The " + UNIX_EPOCH_TIME + " when the node was last seen"}, + {RPCResult::Type::STR, "source", "The address that relayed the address to us"}, + {RPCResult::Type::STR, "source_network", "The network (" + Join(GetNetworkNames(), ", ") + ") of the source address"}, + }} + }} + } + }, + RPCExamples{ + HelpExampleCli("getrawaddrman", "") + + HelpExampleRpc("getrawaddrman", "") + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + NodeContext& node = EnsureAnyNodeContext(request.context); + if (!node.addrman) { + throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Address manager functionality missing or disabled"); + } + + UniValue ret(UniValue::VOBJ); + ret.pushKV("new", AddrmanTableToJSON(node.addrman->GetEntries(false))); + ret.pushKV("tried", AddrmanTableToJSON(node.addrman->GetEntries(true))); + return ret; + }, + }; +} + void RegisterNetRPCCommands(CRPCTable &t) { static const CRPCCommand commands[]{ @@ -1196,6 +1265,7 @@ void RegisterNetRPCCommands(CRPCTable &t) {"hidden", &sendmsgtopeer}, {"hidden", &setmnthreadactive}, {"hidden", &getaddrmaninfo}, + {"hidden", &getrawaddrman}, }; for (const auto& c : commands) { t.appendCommand(c.name, &c); diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index 8dc0368fc371..57cb4a774c2f 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -135,6 +135,7 @@ const std::vector RPC_COMMANDS_SAFE_FOR_FUZZING{ "getnetworkinfo", "getnodeaddresses", "getpeerinfo", + "getrawaddrman", "getrawmempool", "getrawtransaction", "getrpcinfo", diff --git a/src/test/settings_tests.cpp b/src/test/settings_tests.cpp index 9bd9c7942043..5e21b03e6f51 100644 --- a/src/test/settings_tests.cpp +++ b/src/test/settings_tests.cpp @@ -35,7 +35,7 @@ inline std::ostream& operator<<(std::ostream& os, const util::SettingsValue& val inline std::ostream& operator<<(std::ostream& os, const std::pair& kv) { util::SettingsValue out(util::SettingsValue::VOBJ); - out.__pushKV(kv.first, kv.second); + out.pushKVEnd(kv.first, kv.second); os << out.write(); return os; } diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index d03924c2fe7d..16dd8a219a92 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -59,10 +59,10 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer) int TxOrphanage::EraseTx(const uint256& txid) { LOCK(m_mutex); - return _EraseTx(txid); + return EraseTxNoLock(txid); } -int TxOrphanage::_EraseTx(const uint256& txid) +int TxOrphanage::EraseTxNoLock(const uint256& txid) { AssertLockHeld(m_mutex); std::map::iterator it = m_orphans.find(txid); @@ -110,7 +110,7 @@ void TxOrphanage::EraseForPeer(NodeId peer) std::map::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid if (maybeErase->second.fromPeer == peer) { - nErased += _EraseTx(maybeErase->second.tx->GetHash()); + nErased += EraseTxNoLock(maybeErase->second.tx->GetHash()); } } if (nErased > 0) LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx from peer=%d\n", nErased, peer); @@ -132,7 +132,7 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans_size) { std::map::iterator maybeErase = iter++; if (maybeErase->second.nTimeExpire <= nNow) { - nErased += _EraseTx(maybeErase->second.tx->GetHash()); + nErased += EraseTxNoLock(maybeErase->second.tx->GetHash()); } else { nMinExpTime = std::min(maybeErase->second.nTimeExpire, nMinExpTime); } @@ -146,7 +146,7 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans_size) { // Evict a random orphan: size_t randompos = rng.randrange(m_orphan_list.size()); - _EraseTx(m_orphan_list[randompos]->first); + EraseTxNoLock(m_orphan_list[randompos]->first); ++nEvicted; } if (nEvicted > 0) LogPrint(BCLog::MEMPOOL, "orphanage overflow, removed %u tx\n", nEvicted); @@ -234,7 +234,7 @@ void TxOrphanage::EraseForBlock(const CBlock& block) if (vOrphanErase.size()) { int nErased = 0; for (const uint256& orphanHash : vOrphanErase) { - nErased += _EraseTx(orphanHash); + nErased += EraseTxNoLock(orphanHash); } LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx included or conflicted by block\n", nErased); } diff --git a/src/txorphanage.h b/src/txorphanage.h index bd68ea091627..1efb112363ac 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -104,7 +104,7 @@ class TxOrphanage { size_t m_orphan_tx_size GUARDED_BY(m_mutex){0}; /** Erase an orphan by txid */ - int _EraseTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(m_mutex); + int EraseTxNoLock(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(m_mutex); }; #endif // BITCOIN_TXORPHANAGE_H diff --git a/src/univalue/include/univalue.h b/src/univalue/include/univalue.h index 004135ef9733..94f80f9c27b9 100644 --- a/src/univalue/include/univalue.h +++ b/src/univalue/include/univalue.h @@ -87,7 +87,7 @@ class UniValue { template void push_backV(It first, It last); - void __pushKV(std::string key, UniValue val); + void pushKVEnd(std::string key, UniValue val); void pushKV(std::string key, UniValue val); void pushKVs(UniValue obj); diff --git a/src/univalue/lib/univalue.cpp b/src/univalue/lib/univalue.cpp index c3d19caae04f..656d2e820300 100644 --- a/src/univalue/lib/univalue.cpp +++ b/src/univalue/lib/univalue.cpp @@ -115,7 +115,7 @@ void UniValue::push_backV(const std::vector& vec) values.insert(values.end(), vec.begin(), vec.end()); } -void UniValue::__pushKV(std::string key, UniValue val) +void UniValue::pushKVEnd(std::string key, UniValue val) { checkType(VOBJ); @@ -131,7 +131,7 @@ void UniValue::pushKV(std::string key, UniValue val) if (findKey(key, idx)) values[idx] = std::move(val); else - __pushKV(std::move(key), std::move(val)); + pushKVEnd(std::move(key), std::move(val)); } void UniValue::pushKVs(UniValue obj) @@ -140,7 +140,7 @@ void UniValue::pushKVs(UniValue obj) obj.checkType(VOBJ); for (size_t i = 0; i < obj.keys.size(); i++) - __pushKV(std::move(obj.keys.at(i)), std::move(obj.values.at(i))); + pushKVEnd(std::move(obj.keys.at(i)), std::move(obj.values.at(i))); } void UniValue::getObjMap(std::map& kv) const diff --git a/src/univalue/test/object.cpp b/src/univalue/test/object.cpp index b2a86a3faf76..e9265224923f 100644 --- a/src/univalue/test/object.cpp +++ b/src/univalue/test/object.cpp @@ -86,7 +86,7 @@ void univalue_push_throw() UniValue j; BOOST_CHECK_THROW(j.push_back(1), std::runtime_error); BOOST_CHECK_THROW(j.push_backV({1}), std::runtime_error); - BOOST_CHECK_THROW(j.__pushKV("k", 1), std::runtime_error); + BOOST_CHECK_THROW(j.pushKVEnd("k", 1), std::runtime_error); BOOST_CHECK_THROW(j.pushKV("k", 1), std::runtime_error); BOOST_CHECK_THROW(j.pushKVs({}), std::runtime_error); } @@ -364,7 +364,7 @@ void univalue_object() obj.setObject(); UniValue uv; uv.setInt(42); - obj.__pushKV("age", uv); + obj.pushKVEnd("age", uv); BOOST_CHECK_EQUAL(obj.size(), 1); BOOST_CHECK_EQUAL(obj["age"].getValStr(), "42"); diff --git a/src/util/settings.cpp b/src/util/settings.cpp index 421b2be4623c..6be048aa720b 100644 --- a/src/util/settings.cpp +++ b/src/util/settings.cpp @@ -124,7 +124,7 @@ bool WriteSettings(const fs::path& path, { SettingsValue out(SettingsValue::VOBJ); for (const auto& value : values) { - out.__pushKV(value.first, value.second); + out.pushKVEnd(value.first, value.second); } std::ofstream file; file.open(path); diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index 8007a839f8a4..0be76d80644d 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -608,11 +608,11 @@ RPCHelpMan getaddressesbylabel() CHECK_NONFATAL(unique); // UniValue::pushKV checks if the key exists in O(N) // and since duplicate addresses are unexpected (checked with - // std::set in O(log(N))), UniValue::__pushKV is used instead, + // std::set in O(log(N))), UniValue::pushKVEnd is used instead, // which currently is O(1). UniValue value(UniValue::VOBJ); value.pushKV("purpose", _purpose); - ret.__pushKV(address, value); + ret.pushKVEnd(address, value); } }); diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index bdc03248364d..45e150d25275 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright (c) 2017-2020 The Bitcoin Core developers +# Copyright (c) 2017-present The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test RPC calls related to net. @@ -7,10 +7,12 @@ Tests correspond to code in rpc/net.cpp. """ +import time import test_framework.messages from test_framework.messages import ( MAX_PROTOCOL_MESSAGE_LENGTH, ) +from test_framework.netutil import ADDRMAN_NEW_BUCKET_COUNT, ADDRMAN_TRIED_BUCKET_COUNT, ADDRMAN_BUCKET_SIZE from test_framework.p2p import ( P2PInterface, ) @@ -74,6 +76,7 @@ def run_test(self): self.test_addpeeraddress() self.test_sendmsgtopeer() self.test_getaddrmaninfo() + self.test_getrawaddrman() def test_connection_count(self): self.log.info("Test getconnectioncount") @@ -350,9 +353,11 @@ def test_addpeeraddress(self): self.log.debug("Test that adding an address with invalid port fails") assert_raises_rpc_error(-1, "JSON integer out of range", self.nodes[0].addpeeraddress, address="1.2.3.4", port=-1) - assert_raises_rpc_error(-1, "JSON integer out of range", self.nodes[0].addpeeraddress,address="1.2.3.4", port=65536) + assert_raises_rpc_error(-1, "JSON integer out of range", self.nodes[0].addpeeraddress, address="1.2.3.4", port=65536) self.log.debug("Test that adding a valid address to the tried table succeeds") + self.addr_time = int(time.time()) + node.setmocktime(self.addr_time) assert_equal(node.addpeeraddress(address="1.2.3.4", tried=True, port=8333), {"success": True}) with node.assert_debug_log(expected_msgs=["CheckAddrman: new 0, tried 1, total 1 started"]): addrs = node.getnodeaddresses(count=0) # getnodeaddresses re-runs the addrman checks @@ -428,5 +433,114 @@ def test_getaddrmaninfo(self): assert_equal(res[net]["tried"], 0) assert_equal(res[net]["total"], 0) + def test_getrawaddrman(self): + self.log.info("Test getrawaddrman") + node = self.nodes[1] + + self.log.debug("Test that getrawaddrman is a hidden RPC") + # It is hidden from general help, but its detailed help may be called directly. + assert "getrawaddrman" not in node.help() + assert "getrawaddrman" in node.help("getrawaddrman") + + def check_addr_information(result, expected): + """Utility to compare a getrawaddrman result entry with an expected entry""" + assert_equal(result["address"], expected["address"]) + assert_equal(result["port"], expected["port"]) + assert_equal(result["services"], expected["services"]) + assert_equal(result["network"], expected["network"]) + assert_equal(result["source"], expected["source"]) + assert_equal(result["source_network"], expected["source_network"]) + assert_equal(result["time"], self.addr_time) + + def check_getrawaddrman_entries(expected): + """Utility to compare a getrawaddrman result with expected addrman contents""" + getrawaddrman = node.getrawaddrman() + getaddrmaninfo = node.getaddrmaninfo() + for (table_name, table_info) in expected.items(): + assert_equal(len(getrawaddrman[table_name]), len(table_info["entries"])) + assert_equal(len(getrawaddrman[table_name]), getaddrmaninfo["all_networks"][table_name]) + + for bucket_position in getrawaddrman[table_name].keys(): + bucket = int(bucket_position.split("/")[0]) + position = int(bucket_position.split("/")[1]) + + # bucket and position only be sanity checked here as the + # test-addrman isn't deterministic + assert 0 <= int(bucket) < table_info["bucket_count"] + assert 0 <= int(position) < ADDRMAN_BUCKET_SIZE + + entry = getrawaddrman[table_name][bucket_position] + expected_entry = list(filter(lambda e: e["address"] == entry["address"], table_info["entries"]))[0] + check_addr_information(entry, expected_entry) + + # we expect one addrman new and tried table entry, which were added in a previous test + expected = { + "new": { + "bucket_count": ADDRMAN_NEW_BUCKET_COUNT, + "entries": [ + { + "address": "2.0.0.0", + "port": 8333, + "services": 1, + "network": "ipv4", + "source": "2.0.0.0", + "source_network": "ipv4", + } + ] + }, + "tried": { + "bucket_count": ADDRMAN_TRIED_BUCKET_COUNT, + "entries": [ + { + "address": "1.2.3.4", + "port": 8333, + "services": 1, + "network": "ipv4", + "source": "1.2.3.4", + "source_network": "ipv4", + } + ] + } + } + + self.log.debug("Test that the getrawaddrman contains information about the addresses added in a previous test") + check_getrawaddrman_entries(expected) + + self.log.debug("Add one new address to each addrman table") + expected["new"]["entries"].append({ + "address": "2803:0:1234:abcd::1", + "services": 1, + "network": "ipv6", + "source": "2803:0:1234:abcd::1", + "source_network": "ipv6", + "port": -1, # set once addpeeraddress is successful + }) + expected["tried"]["entries"].append({ + "address": "nrfj6inpyf73gpkyool35hcmne5zwfmse3jl3aw23vk7chdemalyaqad.onion", + "services": 1, + "network": "onion", + "source": "nrfj6inpyf73gpkyool35hcmne5zwfmse3jl3aw23vk7chdemalyaqad.onion", + "source_network": "onion", + "port": -1, # set once addpeeraddress is successful + }) + + port = 0 + for (table_name, table_info) in expected.items(): + # There's a slight chance that the to-be-added address collides with an already + # present table entry. To avoid this, we increment the port until an address has been + # added. Incrementing the port changes the position in the new table bucket (bucket + # stays the same) and changes both the bucket and the position in the tried table. + while True: + if node.addpeeraddress(address=table_info["entries"][1]["address"], port=port, tried=table_name == "tried")["success"]: + table_info["entries"][1]["port"] = port + self.log.debug(f"Added {table_info['entries'][1]['address']} to {table_name} table") + break + else: + port += 1 + + self.log.debug("Test that the newly added addresses appear in getrawaddrman") + check_getrawaddrman_entries(expected) + + if __name__ == '__main__': NetTest().main()