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

Improve AlreadyHave #7874

Merged
merged 1 commit into from Apr 14, 2016

Conversation

Projects
None yet
6 participants
@morcos
Member

morcos commented Apr 13, 2016

AlreadyHave is called for transactions in 3 places:

  • when an INV is received to decide whether to schedule a request of the tx
  • when a TX is received to decide whether to process it
  • when actually requesting a tx to see if it has been received since scheduling the request

In a large number of these cases, the transaction in question is new and fails each of the tests in the return statement. In particular it is very unlikely (< 10%) to hit the pcoinsTip check and then be found. But pcoinsTip must check the database to be sure it can't find the transaction. Switching the full HaveCoins check for the fast HaveCoinsInCache check will still prevent the vast majority of false negatives but will never need a database access. The downside is after the cache has been flushed there maybe be a very few more cases of requesting already confirmed transactions from peers.

@pstratem

This comment has been minimized.

Show comment
Hide comment
@pstratem

pstratem Apr 13, 2016

Contributor

utACK c6cb6f7

Contributor

pstratem commented Apr 13, 2016

utACK c6cb6f7

@laanwj laanwj added the P2P label Apr 14, 2016

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Apr 14, 2016

Member

utACK c6cb6f7

Member

sipa commented Apr 14, 2016

utACK c6cb6f7

@btcdrak

This comment has been minimized.

Show comment
Hide comment
@btcdrak
Member

btcdrak commented Apr 14, 2016

utACK c6cb6f7

@instagibbs

This comment has been minimized.

Show comment
Hide comment
@instagibbs
Member

instagibbs commented Apr 14, 2016

utACK c6cb6f7

@laanwj laanwj merged commit c6cb6f7 into bitcoin:master Apr 14, 2016

1 check passed

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

laanwj added a commit that referenced this pull request Apr 14, 2016

Merge #7874: Improve AlreadyHave
c6cb6f7 Avoid unnecessary database access for unknown transactions (Alex Morcos)

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

Merge #7874: Improve AlreadyHave
c6cb6f7 Avoid unnecessary database access for unknown transactions (Alex Morcos)

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

Merge #7874: Improve AlreadyHave
c6cb6f7 Avoid unnecessary database access for unknown transactions (Alex Morcos)

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

Merge #7874: Improve AlreadyHave
c6cb6f7 Avoid unnecessary database access for unknown transactions (Alex Morcos)

codablock added a commit to codablock/dash that referenced this pull request Oct 12, 2017

Merge #7874: Improve AlreadyHave
c6cb6f7 Avoid unnecessary database access for unknown transactions (Alex Morcos)

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

Merge #7874: Improve AlreadyHave
c6cb6f7 Avoid unnecessary database access for unknown transactions (Alex Morcos)

UdjinM6 added a commit to UdjinM6/dash that referenced this pull request Nov 8, 2017

Merge #7874: Improve AlreadyHave
c6cb6f7 Avoid unnecessary database access for unknown transactions (Alex Morcos)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment