Replace setInventoryKnown with a rolling bloom filter (rebase of #7100) #7133

Merged
merged 6 commits into from Dec 3, 2015

Conversation

Projects
None yet
8 participants
@sipa
Member

sipa commented Nov 30, 2015

No description provided.

gmaxwell and others added some commits Nov 26, 2015

Replace setInventoryKnown with a rolling bloom filter.
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.)
Only use filterInventoryKnown with MSG_TX inventory messages.
Previously this logic could erroneously filter a MSG_BLOCK inventory message.
Actually only use filterInventoryKnown with MSG_TX inventory messages.
Previously this logic could erroneously filter a MSG_BLOCK inventory message.
@jtimon

This comment has been minimized.

Show comment
Hide comment
@jtimon

jtimon Nov 30, 2015

Member

utACK

Member

jtimon commented Nov 30, 2015

utACK

@gmaxwell

This comment has been minimized.

Show comment
Hide comment
@gmaxwell

gmaxwell Dec 2, 2015

Member

ACK

Member

gmaxwell commented Dec 2, 2015

ACK

@@ -2342,7 +2342,7 @@ unsigned int SendBufferSize() { return 1000*GetArg("-maxsendbuffer", DEFAULT_MAX
CNode::CNode(SOCKET hSocketIn, const CAddress& addrIn, const std::string& addrNameIn, bool fInboundIn) :
ssSend(SER_NETWORK, INIT_PROTO_VERSION),
addrKnown(5000, 0.001),
- setInventoryKnown(SendBufferSize() / 1000)
+ filterInventoryKnown(50000, 0.000001)

This comment has been minimized.

@dcousens

dcousens Dec 2, 2015

Contributor

Why 50000 OOI? Should this be a named constant?

@dcousens

dcousens Dec 2, 2015

Contributor

Why 50000 OOI? Should this be a named constant?

@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Dec 2, 2015

Contributor

utACK

Contributor

dcousens commented Dec 2, 2015

utACK

@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Dec 2, 2015

Contributor

Also ACK on FP ratio of 0.000001:1

Contributor

dcousens commented Dec 2, 2015

Also ACK on FP ratio of 0.000001:1

@petertodd

This comment has been minimized.

Show comment
Hide comment
@petertodd

petertodd Dec 3, 2015

Contributor

utACK

Contributor

petertodd commented Dec 3, 2015

utACK

@laanwj laanwj merged commit aa4b0c2 into bitcoin:master Dec 3, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Dec 3, 2015

Merge pull request #7133
aa4b0c2 When not filtering blocks, getdata sends more in one test (Pieter Wuille)
d41e44c Actually only use filterInventoryKnown with MSG_TX inventory messages. (Gregory Maxwell)
b6a0da4 Only use filterInventoryKnown with MSG_TX inventory messages. (Patick Strateman)
6b84935 Rename setInventoryKnown filterInventoryKnown (Patick Strateman)
e206724 Remove mruset as it is no longer used. (Gregory Maxwell)
ec73ef3 Replace setInventoryKnown with a rolling bloom filter. (Gregory Maxwell)
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Dec 3, 2015

Member

utACK

Member

laanwj commented Dec 3, 2015

utACK

gmaxwell added a commit that referenced this pull request Dec 4, 2015

@morcos

This comment has been minimized.

Show comment
Hide comment
@morcos

morcos Dec 11, 2015

Member

posthumous utACK. nice.

Member

morcos commented Dec 11, 2015

posthumous utACK. nice.

@dgenr8 dgenr8 referenced this pull request in bitcoinxt/bitcoinxt Dec 29, 2015

Merged

Further work on thin blocks #109

@dgenr8 dgenr8 referenced this pull request in bitcoinclassic/bitcoinclassic Mar 2, 2016

Closed

Don't send full transactions with a merkleblock if already known #125

@laanwj laanwj added the P2P label Jul 18, 2016

@str4d str4d referenced this pull request in zcash/zcash Jul 14, 2017

Open

Bitcoin 0.12 P2P/Net PRs 1 #2534

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment