highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-09-12 05:32:22 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#35692: addrman: remove unreachable tried-collision branch

Browse Source 
bc7d905046 addrman: remove unreachable tried-collision branch (Bruno Garcia)

Pull request description:

  `ResolveCollisions_()` had a fallback for the case where a pending tried collision no longer collided because the destination tried slot became empty.

  Under current addrman invariants this cannot happen: once an entry is added to `m_tried_collisions`, the corresponding tried slot remains occupied until the collision is resolved. The only other valid outcomes are that the pending new entry disappears or becomes invalid, both of which are already handled.

  Remove the dead branch and replace the implicit assumption with assertions in `ResolveCollisions_()` and `SelectTriedCollision_()`.

  It came to my mind when taking a look at the fuzz coverage report for the addrman harness. After years (?) of fuzzing, I was trying to understand if it was a fault on the harness or a dead branch.

ACKs for top commit:
  Herb-ops:
    ACK bc7d905046
  danielabrozzoni:
    tACK bc7d905046
  stratospher:
    ACK bc7d905. `MakeTried` is the only place where we clear the tried table slot but we also refill the same slot here under the cs hold. so makes sense that during a node's runtime destination tried slot which has a previous entry/collison can't be empty (unless some id internal corruption).
  naiyoma:
    ACK bc7d905046
  mzumsande:
    Code Review ACK bc7d905046

Tree-SHA512: 57236c0f95ec1028e831aeffdc0639faf5fe083b108cb2869fa1ce237bb37ac3be96d6a49d9a6a8f4a21558d9dcfa015ab3276cc30a8826e5c65e8b0853acd45
This commit is contained in:
merge-script
2026-07-27 21:02:55 +02:00
parent a2aab6df97 bc7d905046
commit 7dea464d6b
1 changed files with 5 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
src/addrman.cpp
View File

@@ -910,7 +910,10 @@ void AddrManImpl::ResolveCollisions_()
int tried_bucket_pos = info_new.GetBucketPosition(nKey, false, tried_bucket);
if (!info_new.IsValid()) { // id_new may no longer map to a valid address
erase_collision = true;
} else if (vvTried[tried_bucket][tried_bucket_pos] != -1) { // The position in the tried bucket is not empty
} else {
// A pending tried collision implies that the destination tried slot
// remains occupied until we resolve it.
Assume(vvTried[tried_bucket][tried_bucket_pos] != -1);
// Get the to-be-evicted address that is being tested
nid_type id_old = vvTried[tried_bucket][tried_bucket_pos];
@@ -939,9 +942,6 @@ void AddrManImpl::ResolveCollisions_()
Good_(info_new, false, current_time);
erase_collision = true;
}
} else { // Collision is not actually a collision anymore
Good_(info_new, false, Now<NodeSeconds>());
erase_collision = true;
}
}
@@ -977,6 +977,7 @@ std::pair<CAddress, NodeSeconds> AddrManImpl::SelectTriedCollision_()
int tried_bucket = newInfo.GetTriedBucket(nKey, m_netgroupman);
int tried_bucket_pos = newInfo.GetBucketPosition(nKey, false, tried_bucket);
Assume(vvTried[tried_bucket][tried_bucket_pos] != -1);
const AddrInfo& info_old = mapInfo[vvTried[tried_bucket][tried_bucket_pos]];
return {info_old, info_old.m_last_try};
}