p2p: periodically clear m_addr_known #20561

pull sdaftuar wants to merge 1 commits into bitcoin:master from sdaftuar:2020-12-moar-addrz changing 1 files +9 −0

sdaftuar commented at 6:39 PM on December 3, 2020: member

We use a rolling bloom filter to track which addresses we've previously sent a peer, but after #7125 we no longer clear it every day before our own announcement. This looks to me like an oversight which has the effect of reducing the frequency with which we actually self-announce our own address, so this reintroduces resetting that filter.

Clear m_addr_known before our periodic self-advertisement

This behavior was apparently inadvertently broken in 5400ef6; without this
change our daily self-announcements frequently go unsent, because our
address is still in the peer's rolling bloom filter (for potentially many
days, depending on addr traffic).

65273fa0e7

DrahtBot added the label P2P on Dec 3, 2020
sipa commented at 6:49 PM on December 3, 2020: member

Concept ACK on resetting on relay.

Your second commit seems to be identical to #19763 (unclear what the status is there, but there was some interesting discussion earlier).
sdaftuar commented at 6:51 PM on December 3, 2020: member

@sipa Thanks, I missed that PR. I'll drop my second commit and let the discussion continue in #19763.
sdaftuar force-pushed on Dec 3, 2020
sdaftuar renamed this:
~~p2p: periodically clear m_addr_known, and choose new peers to receive relayed addresses~~
p2p: periodically clear m_addr_known
on Dec 3, 2020
naumenkogs commented at 1:42 AM on December 4, 2020: member

I was aware of this issue, but I was never sure whether it's intended or not... I guess your reasoning makes sense, especially considering stuff happened in #7125.

Even if non-clearing somehow gave us extra security, it was probably a side-effect, because it was never documented. I also tried to break AddrMan many times, and but that never has anything to do with the freshness of m_addr_known.

At most what an attacker gets here is easier noticing when we roll the filter precisely. But I don't think timing this activity gives any significant advantage to an attacker.

ACK 65273fa0e74f0c11dfbf0645dd962bdc779ea558
MarcoFalke deleted a comment on Dec 4, 2020
laanwj commented at 10:52 PM on December 7, 2020: member

Code review ACK 65273fa0e74f0c11dfbf0645dd962bdc779ea558
sipa commented at 10:56 PM on December 7, 2020: member

utACK 65273fa0e74f0c11dfbf0645dd962bdc779ea558
laanwj merged this on Dec 7, 2020
laanwj closed this on Dec 7, 2020
sidhujag referenced this in commit bf48f3125c on Dec 7, 2020
Fabcien referenced this in commit e146f16463 on Jan 21, 2022
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#7125 Replace global trickle node with random delays