New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

limitfreerelay edge case bugfix #6842

Merged
merged 1 commit into from Jan 29, 2016

Conversation

Projects
None yet
4 participants
@ptschip
Contributor

ptschip commented Oct 17, 2015

Currently if a new incoming transaction will cause -limitfreerelay
to be exceeded then it will still be accepted into the memory pool and the byte counter
updated only after the fact.

What I've been seeing during this attack is that dFreeCount will often get up to 140 to 149KB then the next spam transaction of 15KB will still slip through to bring the total up past the 150KB limit ( limitfreerelay default set at 15), whereas I think it shouldn't be accepted if it's going to exceed the limit. This only allows the attacker to slip through large transactions past the limit, which could be any size up to the transaction size maximum.

@ptschip ptschip changed the title from limitfreerelay edge case bugfix: to limitfreerelay edge case bugfix Oct 17, 2015

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Oct 17, 2015

Member

Concept ACK but this will create yet another merge conflict with #6349.

Member

MarcoFalke commented Oct 17, 2015

Concept ACK but this will create yet another merge conflict with #6349.

@laanwj laanwj added the Mempool label Oct 19, 2015

@ptschip

This comment has been minimized.

Show comment
Hide comment
@ptschip

ptschip Oct 21, 2015

Contributor

You're right, #6349 needs to be merged first and then I'll rebase.

Contributor

ptschip commented Oct 21, 2015

You're right, #6349 needs to be merged first and then I'll rebase.

limitfreerelay edge case bugfix:
If a new transaction will cause limitfreerelay
to be exceeded it should not be accepted
into the memory pool and the byte counter
should be updated only after the fact.
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jan 28, 2016

Member

@gmaxwell @sipa oops, did we completely forget about this?

utACK ptschip@2dfeaa1

Member

laanwj commented Jan 28, 2016

@gmaxwell @sipa oops, did we completely forget about this?

utACK ptschip@2dfeaa1

@laanwj laanwj added this to the 0.12.0 milestone Jan 28, 2016

@sdaftuar

This comment has been minimized.

Show comment
Hide comment
@sdaftuar

sdaftuar Jan 28, 2016

Member

No strong opinion on this change; since it arguably makes the behavior more intuitive I think it's fine.

But I think there's no need to include this in 0.12, as this is a pretty small effect, and I'm not sure I'd call the current behavior a "bug" -- just different semantics (bytes allowed now are at the expense of bytes in the future).

Member

sdaftuar commented Jan 28, 2016

No strong opinion on this change; since it arguably makes the behavior more intuitive I think it's fine.

But I think there's no need to include this in 0.12, as this is a pretty small effect, and I'm not sure I'd call the current behavior a "bug" -- just different semantics (bytes allowed now are at the expense of bytes in the future).

@laanwj laanwj removed this from the 0.12.0 milestone Jan 28, 2016

@laanwj laanwj merged commit 2dfeaa1 into bitcoin:master Jan 29, 2016

1 check passed

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

laanwj added a commit that referenced this pull request Jan 29, 2016

Merge #6842: limitfreerelay edge case bugfix
2dfeaa1 limitfreerelay edge case bugfix: (ptschip)

@ptschip ptschip deleted the ptschip:limitfreerelay_edgecase branch Mar 7, 2016

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge #6842: limitfreerelay edge case bugfix
2dfeaa1 limitfreerelay edge case bugfix: (ptschip)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #6842: limitfreerelay edge case bugfix
2dfeaa1 limitfreerelay edge case bugfix: (ptschip)

codablock added a commit to codablock/dash that referenced this pull request Dec 9, 2017

Merge #6842: limitfreerelay edge case bugfix
2dfeaa1 limitfreerelay edge case bugfix: (ptschip)

codablock added a commit to codablock/dash that referenced this pull request Dec 9, 2017

Merge #6842: limitfreerelay edge case bugfix
2dfeaa1 limitfreerelay edge case bugfix: (ptschip)

codablock added a commit to codablock/dash that referenced this pull request Dec 11, 2017

Merge #6842: limitfreerelay edge case bugfix
2dfeaa1 limitfreerelay edge case bugfix: (ptschip)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment