Skip to content

Conversation

gavinandresen
Copy link
Contributor

This fixes a subtle bug involving block re-orgs and non-standard transactions.

Start with a block containing a non-standard transaction, and
one or more transactions spending it in the memory pool.

Then re-org away from that block to another chain that does
not contain the non-standard transaction.

Result before this fix: the dependent transactions get stuck
in the mempool without their parent, putting the mempool
in an inconsistent state.

Tested with a new unit test.

Thanks to Alex Morcos for finding the bug.

@morcos
Copy link
Contributor

morcos commented Mar 25, 2015

I tested this. ACK.
I think it has the side effect of making reorgs a little faster too!

continue;
txToRemove.push_back(it->second.ptx->GetHash());
}
}
while (!txToRemove.empty())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could use some comments saying what this block of code is intending to actually do; not obvious right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment added.

This fixes a subtle bug involving block re-orgs and non-standard transactions.

Start with a block containing a non-standard transaction, and
one or more transactions spending it in the memory pool.

Then re-org away from that block to another chain that does
not contain the non-standard transaction.

Result before this fix: the dependent transactions get stuck
in the mempool without their parent, putting the mempool
in an inconsistent state.

Tested with a new unit test.
@petertodd
Copy link
Contributor

Very much untested and rushed ACK

@laanwj
Copy link
Member

laanwj commented Apr 1, 2015

Going to test this.

@sipa
Copy link
Member

sipa commented Apr 1, 2015

Untested ACK.

@gmaxwell gmaxwell added this to the 0.10.0 milestone Apr 2, 2015
@gmaxwell
Copy link
Contributor

gmaxwell commented Apr 5, 2015

I was observed two crashes in 0.10rc1, each 24 hours apart with checkmempool=1 (and a non-default fee policy) during reorgs that appeared to be likely due to the bug this fixes. I've now run three days with this pull applied and no crashes, and there have been several reorgs during that time.

@laanwj
Copy link
Member

laanwj commented Apr 6, 2015

Tested ACK

@laanwj laanwj merged commit ad9e86d into bitcoin:master Apr 6, 2015
laanwj added a commit that referenced this pull request Apr 6, 2015
ad9e86d Keep mempool consistent during block-reorgs (Gavin Andresen)
@laanwj
Copy link
Member

laanwj commented Apr 6, 2015

Backported to 0.10 branch as 8bb4abc19ae0e76424e69a19ff6fc31e5b044333

gavinandresen added a commit that referenced this pull request Apr 6, 2015
This fixes a subtle bug involving block re-orgs and non-standard transactions.

Start with a block containing a non-standard transaction, and
one or more transactions spending it in the memory pool.

Then re-org away from that block to another chain that does
not contain the non-standard transaction.

Result before this fix: the dependent transactions get stuck
in the mempool without their parent, putting the mempool
in an inconsistent state.

Tested with a new unit test (adapted for 0.10).

Rebased-From: ad9e86d
Github-Pull: #5945
@gavinandresen gavinandresen deleted the mempool_remove_fix branch April 24, 2015 14:46
reddink pushed a commit to reddcoin-project/reddcoin-3.10 that referenced this pull request Jul 11, 2020
This fixes a subtle bug involving block re-orgs and non-standard transactions.

Start with a block containing a non-standard transaction, and
one or more transactions spending it in the memory pool.

Then re-org away from that block to another chain that does
not contain the non-standard transaction.

Result before this fix: the dependent transactions get stuck
in the mempool without their parent, putting the mempool
in an inconsistent state.

Tested with a new unit test (adapted for 0.10).

Rebased-From: ad9e86d
Github-Pull: bitcoin#5945
(cherry picked from commit 1c62e84)
reddink pushed a commit to reddcoin-project/reddcoin-3.10 that referenced this pull request Jul 14, 2020
This fixes a subtle bug involving block re-orgs and non-standard transactions.

Start with a block containing a non-standard transaction, and
one or more transactions spending it in the memory pool.

Then re-org away from that block to another chain that does
not contain the non-standard transaction.

Result before this fix: the dependent transactions get stuck
in the mempool without their parent, putting the mempool
in an inconsistent state.

Tested with a new unit test (adapted for 0.10).

Rebased-From: ad9e86d
Github-Pull: bitcoin#5945
(cherry picked from commit 1c62e84)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants