Avoid calling CAddrMan::Connected() on block-relay-only peer addresses
Summary:
PR description:
This PR does two things:
- Block-relay-only interaction with addrman.
- Calling CAddrMan::Connected() on an address that was a block-relay-only peer causes the time we report in addr messages containing that peer to be updated; particularly now that we use anchor connections with a our block-relay-only peers, this risks leaking information about those peers. So, stop this.
- Avoiding calling CAddrMan::Good() on block-relay-only peer addresses causes the addrman logic around maintaining the new and tried table to be less good, and in particular makes it so that block-relay-only peer addresses are more likely to be evicted from the addrman (for no good reason I can think of). So, mark those addresses as good when we connect.
- Fix test-before-evict bug. There's a bug where if we get a collision in the tried table with an existing address that is one of our current peers, and the connection is long-lived enough, then SelectTriedCollisions() might return that existing peer address to us as a test-before-evict connection candidate. However, our logic for new outbound connections would later prevent us from actually making a connection; the result would be that when we get a collision with a long-lived current peer, that peer's address is likely to get evicted from the tried table. Fix this by checking to see if a test-before-evict candidate is a peer we're currently connected to, and if so, mark it as Good().
Commit description:
Connected() updates the time we serve in addr messages, so avoid leaking block-relay-only peer connections by avoiding these calls.
This is a backport of core#20187 [1/4]
https://github.com/bitcoin/bitcoin/pull/20187/commits/daf55531260833d597ee599e2d289ea1be0b1d9c
Test Plan: ninja all check-all
Reviewers: #bitcoin_abc, Fabien
Reviewed By: #bitcoin_abc, Fabien
Differential Revision: https://reviews.bitcoinabc.org/D10694