Skip to content
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

Fix removing of orphan transactions #5985

Merged
merged 1 commit into from Jun 10, 2015
Merged

Fix removing of orphan transactions #5985

merged 1 commit into from Jun 10, 2015

Conversation

@morcos
Copy link
Member

morcos commented Apr 8, 2015

The transaction thats being accepted can't be an orphan otherwise it would have previously been accepted, so doesn't need to be added to the erase queue. We also don't want to erase orphans that still have missing inputs.

@dgenr8
Copy link
Contributor

dgenr8 commented Apr 13, 2015

utACK

@@ -4028,6 +4027,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
}
// too-little-fee orphan

This comment has been minimized.

Copy link
@dgenr8

dgenr8 Apr 13, 2015

Contributor

This (pre-existing) comment is too specific to be correct.

@@ -3979,7 +3979,6 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
mempool.check(pcoinsTip);
RelayTransaction(tx);
vWorkQueue.push_back(inv.hash);
vEraseQueue.push_back(inv.hash);

This comment has been minimized.

Copy link
@sipa

sipa Apr 27, 2015

Member

Why can't we mark a tx for removal already? It was accepted into the memory pool, so alls its inputs must be satisfied?

This comment has been minimized.

Copy link
@morcos

morcos Apr 27, 2015

Author Member

This tx isn't in the orphan map. We're just now receiving it off the network and accepting it to the mempool, so it never entered the orphan map. If it was a duplicate transaction that was an orphan the first time it was received, it would have previously been accepted to the mempool at the time its inputs were satisfied, and wouldn't be accepted here.

It doesn't hurt to add tx to vEraseQueue, but its unnecessary and confuses the logic.

@laanwj laanwj added the Bug label Apr 29, 2015
We don't want to erase orphans that still have missing inputs, they should still be tracked as orphans.  Also, the transaction thats being accepted can't be an orphan otherwise it would have previously been accepted, so doesn't need to be added to the erase queue.
@morcos morcos force-pushed the morcos:fix-orphans branch from 3588633 to 14d4eef May 12, 2015
@morcos
Copy link
Member Author

morcos commented May 12, 2015

Fixed comment nit and clarified commit message

@mrbandrews
Copy link
Contributor

mrbandrews commented May 27, 2015

Tested ACK

@laanwj
Copy link
Member

laanwj commented Jun 10, 2015

utACK

@laanwj laanwj merged commit 14d4eef into bitcoin:master Jun 10, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
laanwj added a commit that referenced this pull request Jun 10, 2015
14d4eef Fix removing of orphan transactions (Alex Morcos)
laanwj added a commit that referenced this pull request Jun 10, 2015
We don't want to erase orphans that still have missing inputs, they should still be tracked as orphans.  Also, the transaction thats being accepted can't be an orphan otherwise it would have previously been accepted, so doesn't need to be added to the erase queue.

Github-Pull: #5985
Rebased-From: 14d4eef
@laanwj
Copy link
Member

laanwj commented Jun 10, 2015

Cherry-picked to 0.11 branch as 37b4e42

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.