Replace setInventoryKnown with a rolling bloom filter #4997
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
Mruset setInventoryKnown was reduced to a remarkably small 1000 entries as a side effect of sendbuffer size reductions in 2012. This removes setInventoryKnown filtering from merkleBlock responses because false positives there are especially unattractive and also because I'm not sure if there aren't race conditions around the relay pool that would cause some transactions there to be suppressed. (Also, ProcessGetData was accessing setInventoryKnown without taking the required lock.)
Previously this logic could erroneously filter a MSG_BLOCK inventory message.
Previously this logic could erroneously filter a MSG_BLOCK inventory message.
str4d
added
C-upstream-port
nuttycom
approved these changes
Feb 19, 2021
Contributor
nuttycom
left a comment
nuttycom
left a comment
There was a problem hiding this comment.
utACK, verified against bitcoin/bitcoin#7133
Contributor
Author
|
Not merging this until after the 4.3.0 release. |
Contributor
Author
|
But in the meantime: @zkbot try |
Contributor
zkbot
added a commit
that referenced
this pull request
Feb 22, 2021
Replace setInventoryKnown with a rolling bloom filter Cherry-picked from bitcoin/bitcoin#7133. - Excluding for last commit, which needs bitcoin/bitcoin#7129. Part of #2074.
Contributor
|
☀️ Test successful - pr-try |
Contributor
Author
|
There have been; I will backport them in a separate PR. @zkbot r+ |
Contributor
|
📌 Commit 227933f has been approved by |
Contributor
Contributor
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 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.
Cherry-picked from bitcoin/bitcoin#7133.
Part of #2074.