p2p: periodically clear m_addr_known #20561
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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).
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). |
Member
Author
sdaftuar
force-pushed
the
2020-12-moar-addrz
branch
from
10ed228 to
65273fa
Compare
sdaftuar
changed the title
Contributor
|
I was aware of this issue, but I was never sure whether it's intended or not... Even if non-clearing somehow gave us extra security, it was probably a side-effect, because it was never documented. At most what an attacker gets here is easier noticing when we roll the filter precisely. ACK 65273fa |
Member
|
Code review ACK 65273fa |
Member
|
utACK 65273fa |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Dec 7, 2020
65273fa Clear m_addr_known before our periodic self-advertisement (Suhas Daftuar) Pull request description: We use a rolling bloom filter to track which addresses we've previously sent a peer, but after bitcoin#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. ACKs for top commit: naumenkogs: ACK 65273fa laanwj: Code review ACK 65273fa sipa: utACK 65273fa Tree-SHA512: 602c155fb6d2249b054fcb6f1c0dd17143605ceb87132286bbd90babf26d258ff6c41f9925482c17e2be41805d33f9b83926cb447f394969ffecd4bccfa0a64f
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jan 21, 2022
Summary: ``` 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). ``` Backport of [[bitcoin/bitcoin#20561 | core#20561]]. Ref T1696. Test Plan: ninja all check-all Reviewers: #bitcoin_abc, PiRK Reviewed By: #bitcoin_abc, PiRK Maniphest Tasks: T1696 Differential Revision: https://reviews.bitcoinabc.org/D10859
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.