diff --git a/src/txmempool.h b/src/txmempool.h --- a/src/txmempool.h +++ b/src/txmempool.h @@ -929,6 +929,8 @@ queuedTx.get().erase(entry); } + bool isEmpty() { return queuedTx.empty(); } + void clear() { cachedInnerUsage = 0; queuedTx.clear(); diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2193,21 +2193,18 @@ return false; } - // If this block was deactivating the replay protection, then we need to - // remove transactions that are replay protected from the mempool. There is - // no easy way to do this so we'll just discard the whole mempool and then - // add the transaction of the block we just disconnected back. - if (IsReplayProtectionEnabled(config, pindexDelete) && - !IsReplayProtectionEnabled(config, pindexDelete->pprev)) { - LogPrint(BCLog::MEMPOOL, "Clearing mempool for reorg"); - - g_mempool.clear(); - // While not strictly necessary, clearing the disconnect pool is also - // beneficial so we don't try to reuse its content at the end of the - // reorg, which we know will fail. + // If this block is deactivating a fork, we move all mempool transactions + // in front of disconnectpool for reprocessing in a future + // updateMempoolForReorg call + if (GetBlockScriptFlags(config, pindexDelete) != + GetBlockScriptFlags(config, pindexDelete->pprev)) { + LogPrint(BCLog::MEMPOOL, + "%s mempool for a reorg crossing a fork boundary\n", + disconnectpool ? "Reprocessing" : "Clearing"); if (disconnectpool) { - disconnectpool->clear(); + disconnectpool->importMempool(g_mempool); } + g_mempool.clear(); } if (disconnectpool) { @@ -2474,17 +2471,21 @@ (nTime5 - nTime4) * MILLI, nTimeChainState * MICRO, nTimeChainState * MILLI / nBlocksTotal); - // If we just activated the replay protection with that block, it means - // transaction in the mempool are now invalid. As a result, we need to clear - // the mempool. - if (IsReplayProtectionEnabled(config, pindexNew) && - !IsReplayProtectionEnabled(config, pindexNew->pprev)) { - g_mempool.clear(); - } - // Remove conflicting transactions from the mempool.; g_mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight); disconnectpool.removeForBlock(blockConnecting.vtx); + + // If this block is activating a fork, we move all mempool transactions + // in front of disconnectpool for reprocessing in a future + // updateMempoolForReorg call + if (pindexNew->pprev != nullptr && + GetBlockScriptFlags(config, pindexNew) != + GetBlockScriptFlags(config, pindexNew->pprev)) { + LogPrint(BCLog::MEMPOOL, "Reprocessing mempool because new validation " + "rules are in effect\n"); + disconnectpool.importMempool(g_mempool); + } + // Update chainActive & related variables. UpdateTip(config, pindexNew); @@ -2753,9 +2754,11 @@ } } - if (fBlocksDisconnected) { - // If any blocks were disconnected, disconnectpool may be non empty. Add - // any disconnected transactions back to the mempool. + if (fBlocksDisconnected || !disconnectpool.isEmpty()) { + // If any blocks were disconnected, we need to update the mempool even + // if disconnectpool is empty. The disconnectpool may also be non-empty + // if the mempool was imported due to new validation rules being in + // effect disconnectpool.updateMempoolForReorg(config, true); } diff --git a/test/functional/abc-replay-protection.py b/test/functional/abc-mempool-coherence-on-activations.py copy from test/functional/abc-replay-protection.py copy to test/functional/abc-mempool-coherence-on-activations.py --- a/test/functional/abc-replay-protection.py +++ b/test/functional/abc-mempool-coherence-on-activations.py @@ -4,21 +4,21 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """ -This test checks activation of UAHF and the different consensus -related to this activation. -It is derived from the much more complex p2p-fullblocktest. +This test checks the mempool coherence when changing validation rulesets, +which happens on (de)activations of network upgrades. """ from test_framework.test_framework import ComparisonTestFramework from test_framework.util import assert_equal, assert_raises_rpc_error -from test_framework.comptool import TestManager, TestInstance, RejectResult +from test_framework.comptool import TestManager, TestInstance from test_framework.blocktools import * import time from test_framework.key import CECKey from test_framework.script import * -# far into the future -REPLAY_PROTECTION_START_TIME = 2000000000 +# We use the replay protection activation for this test, as it's always +# present in the client code +ACTIVATION_TIME = 2000000000 # Error due to invalid signature INVALID_SIGNATURE_ERROR = b'mandatory-script-verify-flag-failed (Signature must be zero for failed CHECK(MULTI)SIG operation)' @@ -42,13 +42,13 @@ self.tip = None self.blocks = {} self.extra_args = [['-whitelist=127.0.0.1', - "-replayprotectionactivationtime=%d" % REPLAY_PROTECTION_START_TIME]] + "-replayprotectionactivationtime=%d" % ACTIVATION_TIME]] def run_test(self): self.test = TestManager(self, self.options.tmpdir) self.test.add_all_connections(self.nodes) network_thread_start() - self.nodes[0].setmocktime(REPLAY_PROTECTION_START_TIME) + self.nodes[0].setmocktime(ACTIVATION_TIME) self.test.run() def next_block(self, number): @@ -129,7 +129,7 @@ # Now we need that block to mature so we can spend the coinbase. test = TestInstance(sync_every_block=False) - for i in range(99): + for i in range(110): block(5000 + i) test.blocks_and_transactions.append([self.tip, True]) save_spendable_output() @@ -140,12 +140,14 @@ for i in range(100): out.append(get_spendable_output()) - # Generate a key pair to test P2SH sigops count + # Generate a key pair to test private_key = CECKey() private_key.set_secretbytes(b"replayprotection") public_key = private_key.get_pubkey() - # This is a little handier to use than the version in blocktools.py + # Creates fund and spend transactions using the forkid replay + # protection mechanism. It's used to create transactions that are valid + # on only one side of the fork. def create_fund_and_spend_tx(spend, forkvalue=0): # Fund transaction script = CScript([public_key, OP_CHECKSIG]) @@ -167,141 +169,136 @@ txspend.vin[0].scriptSig = CScript([sig]) txspend.rehash() - return [txfund, txspend] + return txfund, txspend + + # Creates a chainable tx, spending a scriptPub=OP_TRUE coin into + # another + def create_chaining_tx(spend): + if isinstance(spend, CTransaction): + # After the first iteration, we start chaining txns here + tx = create_transaction( + spend, 0, b'', spend.vout[0].nValue - 1000, CScript([OP_TRUE])) + else: + tx = create_transaction( + spend.tx, spend.n, b'', 50 * COIN, CScript([OP_TRUE])) + tx.rehash() + return tx def send_transaction_to_mempool(tx): tx_id = node.sendrawtransaction(ToHex(tx)) - assert(tx_id in set(node.getrawmempool())) + assert(tx_id in node.getrawmempool()) return tx_id - # Before the fork, no replay protection required to get in the mempool. - txns = create_fund_and_spend_tx(out[0]) - send_transaction_to_mempool(txns[0]) - send_transaction_to_mempool(txns[1]) + def check_mempool_equal(txns): + assert(set(node.getrawmempool()) == set([tx.hash for tx in txns])) - # And txns get mined in a block properly. - block(1) - update_block(1, txns) - yield accepted() + # Create 2 transactions that are only valid before activation + # (pre-fork-only) + txfund0, tx_pre0 = create_fund_and_spend_tx(out[0], 0) + txfund1, tx_pre1 = create_fund_and_spend_tx(out[1], 0) - # Replay protected transactions are rejected. - replay_txns = create_fund_and_spend_tx(out[1], 0xffdead) - send_transaction_to_mempool(replay_txns[0]) - assert_raises_rpc_error(-26, RPC_INVALID_SIGNATURE_ERROR, - node.sendrawtransaction, ToHex(replay_txns[1])) + # Create 2 transactions that are only valid after activation + # (post-fork-only) + txfund2, tx_post0 = create_fund_and_spend_tx(out[2], 0xffdead) + txfund3, tx_post1 = create_fund_and_spend_tx(out[3], 0xffdead) - # And block containing them are rejected as well. - block(2) - update_block(2, replay_txns) - yield rejected(RejectResult(16, b'blk-bad-inputs')) - - # Rewind bad block - tip(1) - - # Create a block that would activate the replay protection. + # Create blocks to activate the fork. Mine all funding transactions bfork = block(5555) - bfork.nTime = REPLAY_PROTECTION_START_TIME - 1 - update_block(5555, []) + bfork.nTime = ACTIVATION_TIME - 1 + update_block(5555, [txfund0, txfund1, txfund2, txfund3]) yield accepted() for i in range(5): - block(5100 + i) + block(5200 + i) test.blocks_and_transactions.append([self.tip, True]) yield test # Check we are just before the activation time - assert_equal(node.getblockheader(node.getbestblockhash())['mediantime'], - REPLAY_PROTECTION_START_TIME - 1) - - # We are just before the fork, replay protected txns still are rejected + assert_equal( + node.getblockheader(node.getbestblockhash())['mediantime'], + ACTIVATION_TIME - 1) + + # We are just before the fork + # Pre-fork-only and always-valid txns are valid but post-fork-only txns + # are rejected + send_transaction_to_mempool(tx_pre0) + send_transaction_to_mempool(tx_pre1) + tx_chain0 = create_chaining_tx(out[4]) + tx_chain1 = create_chaining_tx(tx_chain0) + send_transaction_to_mempool(tx_chain0) + send_transaction_to_mempool(tx_chain1) assert_raises_rpc_error(-26, RPC_INVALID_SIGNATURE_ERROR, - node.sendrawtransaction, ToHex(replay_txns[1])) - - block(3) - update_block(3, replay_txns) - yield rejected(RejectResult(16, b'blk-bad-inputs')) - - # Rewind bad block - tip(5104) - - # Send some non replay protected txns in the mempool to check - # they get cleaned at activation. - txns = create_fund_and_spend_tx(out[2]) - send_transaction_to_mempool(txns[0]) - tx_id = send_transaction_to_mempool(txns[1]) + node.sendrawtransaction, ToHex(tx_post0)) + assert_raises_rpc_error(-26, RPC_INVALID_SIGNATURE_ERROR, + node.sendrawtransaction, ToHex(tx_post1)) + check_mempool_equal([tx_chain0, tx_chain1, tx_pre0, tx_pre1]) - # Activate the replay protection + # Activate the fork. Mine the first chained txn and a pre-fork-only txn block(5556) + update_block(5556, [tx_chain0, tx_pre0]) yield accepted() - # Check we just activated the replay protection - assert_equal(node.getblockheader(node.getbestblockhash())['mediantime'], - REPLAY_PROTECTION_START_TIME) - - # Non replay protected transactions are not valid anymore, - # so they should be removed from the mempool. - assert(tx_id not in set(node.getrawmempool())) - - # Good old transactions are now invalid. - send_transaction_to_mempool(txns[0]) + # Check we just activated the fork + assert_equal( + node.getblockheader(node.getbestblockhash())['mediantime'], + ACTIVATION_TIME) + + # Pre-fork-only txns were evicted from the mempool, while always-valid + # txn remain + # Evicted: tx_pre1 + check_mempool_equal([tx_chain1]) + + # Post-fork-only and always-valid txns are accepted, pre-fork-only txn + # are rejected + send_transaction_to_mempool(tx_post0) + send_transaction_to_mempool(tx_post1) + tx_chain2 = create_chaining_tx(tx_chain1) + send_transaction_to_mempool(tx_chain2) assert_raises_rpc_error(-26, RPC_INVALID_SIGNATURE_ERROR, - node.sendrawtransaction, ToHex(txns[1])) + node.sendrawtransaction, ToHex(tx_pre1)) + check_mempool_equal([tx_chain1, tx_chain2, tx_post0, tx_post1]) - # They also cannot be mined - block(4) - update_block(4, txns) - yield rejected(RejectResult(16, b'blk-bad-inputs')) - - # Rewind bad block - tip(5556) - - # The replay protected transaction is now valid - replay_tx0_id = send_transaction_to_mempool(replay_txns[0]) - replay_tx1_id = send_transaction_to_mempool(replay_txns[1]) - - # Make sure the transaction are ready to be mined. - tmpl = node.getblocktemplate() - - found_id0 = False - found_id1 = False - - for txn in tmpl['transactions']: - txid = txn['txid'] - if txid == replay_tx0_id: - found_id0 = True - elif txid == replay_tx1_id: - found_id1 = True - - assert(found_id0 and found_id1) - - # And the mempool is still in good shape. - assert(replay_tx0_id in set(node.getrawmempool())) - assert(replay_tx1_id in set(node.getrawmempool())) - - # They also can also be mined - b5 = block(5) - update_block(5, replay_txns) + # Mine the second chained txn and a post-fork-only txn + block(5557) + update_block(5557, [tx_chain1, tx_post0]) yield accepted() - # Ok, now we check if a reorg work properly accross the activation. + # So the mempool contains the 3rd chained txn and a post-fork-only txn + check_mempool_equal([tx_chain2, tx_post1]) + + # Now we check if a reorg work properly across the activation. postforkblockid = node.getbestblockhash() node.invalidateblock(postforkblockid) - assert(replay_tx0_id in set(node.getrawmempool())) - assert(replay_tx1_id in set(node.getrawmempool())) + # Transactions mined post-fork returned to the mempool + check_mempool_equal([tx_chain1, tx_chain2, tx_post0, tx_post1]) - # Deactivating replay protection. + # Deactivate the fork forkblockid = node.getbestblockhash() node.invalidateblock(forkblockid) - assert(replay_tx0_id not in set(node.getrawmempool())) - assert(replay_tx1_id not in set(node.getrawmempool())) + # Post-fork-only txns were evicted from the mempool, while txns from + # the fork activation block returned to the mempool + # Evicted: tx_post0, tx_post1 + check_mempool_equal([tx_chain0, tx_chain1, tx_chain2, tx_pre0]) - # Check that we also do it properly on deeper reorg. - node.reconsiderblock(forkblockid) + # Check that we do it properly on a reorg that deactivates and + # reactivates the fork at the same time node.reconsiderblock(postforkblockid) - node.invalidateblock(forkblockid) - assert(replay_tx0_id not in set(node.getrawmempool())) - assert(replay_tx1_id not in set(node.getrawmempool())) - + node.reconsiderblock(forkblockid) + # Send post-fork-only txn into the mempool again + send_transaction_to_mempool(tx_post1) + check_mempool_equal([tx_chain2, tx_post1]) + # Create a longer competing chain + tip(5204) + block(8) + block(9) + block(10) + yield TestInstance([[self.blocks[8], True], [self.blocks[9], True], [self.blocks[10], True]], False) + # The mempool was dumped and reevaluated post-fork, thus post-fork-only + # txn survived this reorg! + # Evicted: tx_pre0 (which had been mined in the activation block in the + # now shorter chain) + check_mempool_equal( + [tx_chain0, tx_chain1, tx_chain2, tx_post0, tx_post1]) if __name__ == '__main__': ReplayProtectionTest().main() diff --git a/test/functional/abc-replay-protection.py b/test/functional/abc-replay-protection.py --- a/test/functional/abc-replay-protection.py +++ b/test/functional/abc-replay-protection.py @@ -210,7 +210,8 @@ yield test # Check we are just before the activation time - assert_equal(node.getblockheader(node.getbestblockhash())['mediantime'], + assert_equal( + node.getblockheader(node.getbestblockhash())['mediantime'], REPLAY_PROTECTION_START_TIME - 1) # We are just before the fork, replay protected txns still are rejected @@ -235,7 +236,8 @@ yield accepted() # Check we just activated the replay protection - assert_equal(node.getblockheader(node.getbestblockhash())['mediantime'], + assert_equal( + node.getblockheader(node.getbestblockhash())['mediantime'], REPLAY_PROTECTION_START_TIME) # Non replay protected transactions are not valid anymore, @@ -292,14 +294,16 @@ # Deactivating replay protection. forkblockid = node.getbestblockhash() node.invalidateblock(forkblockid) - assert(replay_tx0_id not in set(node.getrawmempool())) + # The funding tx is not evicted from the mempool, since it's valid in + # both sides of the fork + assert(replay_tx0_id in set(node.getrawmempool())) assert(replay_tx1_id not in set(node.getrawmempool())) # Check that we also do it properly on deeper reorg. node.reconsiderblock(forkblockid) node.reconsiderblock(postforkblockid) node.invalidateblock(forkblockid) - assert(replay_tx0_id not in set(node.getrawmempool())) + assert(replay_tx0_id in set(node.getrawmempool())) assert(replay_tx1_id not in set(node.getrawmempool()))