diff --git a/src/addrman.cpp b/src/addrman.cpp index cdfd079fcdf..19a4f4fa631 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -757,16 +757,14 @@ std::pair AddrManImpl::Select_(bool new_only, std::option // Iterate over the positions of that bucket, starting at the initial one, // and looping around. - int i; + int i, position, node_id; for (i = 0; i < ADDRMAN_BUCKET_SIZE; ++i) { - int position = (initial_position + i) % ADDRMAN_BUCKET_SIZE; - int node_id = GetEntry(search_tried, bucket, position); + position = (initial_position + i) % ADDRMAN_BUCKET_SIZE; + node_id = GetEntry(search_tried, bucket, position); if (node_id != -1) { if (network.has_value()) { const auto it{mapInfo.find(node_id)}; - assert(it != mapInfo.end()); - const auto info{it->second}; - if (info.GetNetwork() == *network) break; + if (Assume(it != mapInfo.end()) && it->second.GetNetwork() == *network) break; } else { break; } @@ -777,9 +775,7 @@ std::pair AddrManImpl::Select_(bool new_only, std::option if (i == ADDRMAN_BUCKET_SIZE) continue; // Find the entry to return. - int position = (initial_position + i) % ADDRMAN_BUCKET_SIZE; - int nId = GetEntry(search_tried, bucket, position); - const auto it_found{mapInfo.find(nId)}; + const auto it_found{mapInfo.find(node_id)}; assert(it_found != mapInfo.end()); const AddrInfo& info{it_found->second}; @@ -798,15 +794,17 @@ int AddrManImpl::GetEntry(bool use_tried, size_t bucket, size_t position) const { AssertLockHeld(cs); - assert(position < ADDRMAN_BUCKET_SIZE); - if (use_tried) { - assert(bucket < ADDRMAN_TRIED_BUCKET_COUNT); - return vvTried[bucket][position]; + if (Assume(position < ADDRMAN_BUCKET_SIZE) && Assume(bucket < ADDRMAN_TRIED_BUCKET_COUNT)) { + return vvTried[bucket][position]; + } } else { - assert(bucket < ADDRMAN_NEW_BUCKET_COUNT); - return vvNew[bucket][position]; + if (Assume(position < ADDRMAN_BUCKET_SIZE) && Assume(bucket < ADDRMAN_NEW_BUCKET_COUNT)) { + return vvNew[bucket][position]; + } } + + return -1; } std::vector AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct, std::optional network) const diff --git a/src/addrman.h b/src/addrman.h index 6284b80a52e..f41687dcffc 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -148,8 +148,10 @@ public: * * @param[in] new_only Whether to only select addresses from the new table. Passing `true` returns * an address from the new table or an empty pair. Passing `false` will return an - * address from either the new or tried table (it does not guarantee a tried entry). - * @param[in] network Select only addresses of this network (nullopt = all) + * empty pair or an address from either the new or tried table (it does not + * guarantee a tried entry). + * @param[in] network Select only addresses of this network (nullopt = all). Passing a network may + * slow down the search. * @return CAddress The record for the selected peer. * seconds The last time we attempted to connect to that peer. */ diff --git a/src/addrman_impl.h b/src/addrman_impl.h index 7aead2812ba..9aff408e342 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -255,7 +255,7 @@ private: /** Helper to generalize looking up an addrman entry from either table. * - * @return int The nid of the entry or -1 if the addrman position is empty. + * @return int The nid of the entry. If the addrman position is empty or not found, returns -1. * */ int GetEntry(bool use_tried, size_t bucket, size_t position) const EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 328e7f81a07..c02322ec7db 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -239,8 +239,9 @@ BOOST_AUTO_TEST_CASE(addrman_select_by_network) // ensure that both new and tried table are selected from bool new_selected{false}; bool tried_selected{false}; + int counter = 256; - while (!new_selected || !tried_selected) { + while (--counter > 0 && (!new_selected || !tried_selected)) { const CAddress selected{addrman->Select(/*new_only=*/false, NET_I2P).first}; BOOST_REQUIRE(selected == i2p_addr || selected == i2p_addr2); if (selected == i2p_addr) { @@ -249,6 +250,9 @@ BOOST_AUTO_TEST_CASE(addrman_select_by_network) new_selected = true; } } + + BOOST_CHECK(new_selected); + BOOST_CHECK(tried_selected); } BOOST_AUTO_TEST_CASE(addrman_select_special)