From a11defc506c45f22783bb8e28eefefdf6d73f2d2 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Wed, 27 Nov 2019 08:56:25 -0500 Subject: [PATCH 1/4] mempool: remove block even if mempool is empty --- lib/mempool/mempool.js | 5 ---- test/mempool-test.js | 53 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/lib/mempool/mempool.js b/lib/mempool/mempool.js index fbf3ab8ac..af5ec7720 100644 --- a/lib/mempool/mempool.js +++ b/lib/mempool/mempool.js @@ -244,11 +244,6 @@ class Mempool extends EventEmitter { */ async _removeBlock(block, txs) { - if (this.map.size === 0) { - this.tip = block.prevBlock; - return; - } - let total = 0; for (let i = 1; i < txs.length; i++) { diff --git a/test/mempool-test.js b/test/mempool-test.js index 26b627cd1..14257bf23 100644 --- a/test/mempool-test.js +++ b/test/mempool-test.js @@ -7,6 +7,7 @@ const assert = require('bsert'); const random = require('bcrypto/lib/random'); const common = require('../lib/blockchain/common'); const Block = require('../lib/primitives/block'); +const ChainEntry = require('../lib/blockchain/chainentry'); const MempoolEntry = require('../lib/mempool/mempoolentry'); const Mempool = require('../lib/mempool/mempool'); const AddrIndexer = require('../lib/mempool/addrindexer'); @@ -783,6 +784,58 @@ describe('Mempool', function() { chaincoins.addTX(tx); wallet.addTX(tx); }); + + it('should insert unconfirmed txs from removed block', async () => { + await mempool.reset(); + // Mempool is empty + assert.strictEqual(mempool.map.size, 0); + + // Create 1 TX + const coin1 = chaincoins.getCoins()[0]; + const addr = wallet.createReceive().getAddress(); + const mtx1 = new MTX(); + mtx1.addCoin(coin1); + mtx1.addOutput(addr, 90000); + chaincoins.sign(mtx1); + const tx1 = mtx1.toTX(); + chaincoins.addTX(tx1); + wallet.addTX(tx1); + + // Create 1 block (no need to actually add it to chain) + const block1 = await getMockBlock(chain, [tx1]); + const entry1 = await ChainEntry.fromBlock(block1, chain.tip); + + // Unconfirm block into mempool + await mempool._removeBlock(entry1, block1.txs); + + // Mempool should contain newly unconfirmed TX + assert(mempool.hasEntry(tx1.hash())); + + // Mempool is NOT empty + assert.strictEqual(mempool.map.size, 1); + + // Create second TX + const coin2 = chaincoins.getCoins()[0]; + const mtx2 = new MTX(); + mtx2.addCoin(coin2); + mtx2.addOutput(addr, 90000); + chaincoins.sign(mtx2); + const tx2 = mtx2.toTX(); + chaincoins.addTX(tx2); + wallet.addTX(tx2); + + // Create 1 block (no need to actually add it to chain) + const block2 = await getMockBlock(chain, [tx2]); + const entry2 = await ChainEntry.fromBlock(block2, chain.tip); + + // Unconfirm block into mempool + await mempool._removeBlock(entry2, block2.txs); + + // Mempool should contain both TXs + assert(mempool.hasEntry(tx2.hash())); + assert(mempool.hasEntry(tx1.hash())); + assert.strictEqual(mempool.map.size, 2); + }); }); describe('AddrIndexer', function () { From 2cdb03dfe1dc345ebbb8e4ee18d9b421a82b786d Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Fri, 29 Nov 2019 11:23:30 -0500 Subject: [PATCH 2/4] mempool: do contextual checks on TXs during _handleReorg() This method used to just toss out any TX that might be problematic when the chain is rewound. Instead, we can actually check the context of each TX and allow them to remain in the mempool if they are still valid. Those checks include coinbase maturity and sequence locks. --- lib/mempool/mempool.js | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/lib/mempool/mempool.js b/lib/mempool/mempool.js index af5ec7720..6563f7b8d 100644 --- a/lib/mempool/mempool.js +++ b/lib/mempool/mempool.js @@ -290,6 +290,7 @@ class Mempool extends EventEmitter { const height = this.chain.height + 1; const mtp = await this.chain.getMedianTime(this.chain.tip); const remove = []; + const lockFlags = common.lockFlags.STANDARD_LOCKTIME_FLAGS; for (const [hash, entry] of this.map) { const {tx} = entry; @@ -299,24 +300,32 @@ class Mempool extends EventEmitter { continue; } - if (tx.version > 1) { - let hasLocks = false; - - for (const {sequence} of tx.inputs) { - if (!(sequence & consensus.SEQUENCE_DISABLE_FLAG)) { - hasLocks = true; - break; - } - } + // We get SpentView here instead of CoinView because for this + // check we are only concerned with sequence locks relative to + // the height of the input coins. Since this TX is already in + // the mempool, all its input coins are "spent" anyway. + const view = await this._getSpentView(tx); - if (hasLocks) { + // Verify Sequence locks. + if (tx.version > 1) { + if (!await this.verifyLocks(tx, view, lockFlags)) { remove.push(hash); continue; } } - if (entry.coinbase) - remove.push(hash); + // In this context "coinbase" means the TX spends from a coinbase output + // and so the maturity must be checked. Actual coinbase TXs are never + // allowed in the mempool and should not be inserted in the first place. + // This exact same check is performed in TX.checkInputs() along with + // several other tests we don't need here. + if (entry.coinbase) { + for (const {prevout} of tx.inputs) { + const inputEntry = view.getEntry(prevout); + if (height - inputEntry.height < consensus.COINBASE_MATURITY) + remove.push(hash); + } + } } for (const hash of remove) { @@ -1106,6 +1115,7 @@ class Mempool extends EventEmitter { */ evictEntry(entry) { + this.logger.debug('Evicting entry %h', entry.tx.hash()); this.removeSpenders(entry); this.updateAncestors(entry, removeFee); this.removeEntry(entry); From 86f21432a04364605dfb3244c0dee7a13148e3d8 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Tue, 3 Dec 2019 12:59:40 -0500 Subject: [PATCH 3/4] test: handle reorg in mempool --- test/mempool-test.js | 360 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 308 insertions(+), 52 deletions(-) diff --git a/test/mempool-test.js b/test/mempool-test.js index 14257bf23..4a2c0fdf9 100644 --- a/test/mempool-test.js +++ b/test/mempool-test.js @@ -29,6 +29,7 @@ const {BufferSet} = require('buffer-map'); const ALL = Script.hashType.ALL; const VERIFY_NONE = common.flags.VERIFY_NONE; +const VERIFY_BODY = common.flags.VERIFY_BODY; const ONE_HASH = Buffer.alloc(32, 0x00); ONE_HASH[0] = 0x01; @@ -98,6 +99,9 @@ async function getMockBlock(chain, txs = [], cb = true) { block.time = time; block.bits = await chain.getTarget(block.time, chain.tip); + // Ensure mockblocks are unique (required for reorg testing) + block.merkleRoot = block.createMerkleRoot(); + return block; } @@ -784,58 +788,6 @@ describe('Mempool', function() { chaincoins.addTX(tx); wallet.addTX(tx); }); - - it('should insert unconfirmed txs from removed block', async () => { - await mempool.reset(); - // Mempool is empty - assert.strictEqual(mempool.map.size, 0); - - // Create 1 TX - const coin1 = chaincoins.getCoins()[0]; - const addr = wallet.createReceive().getAddress(); - const mtx1 = new MTX(); - mtx1.addCoin(coin1); - mtx1.addOutput(addr, 90000); - chaincoins.sign(mtx1); - const tx1 = mtx1.toTX(); - chaincoins.addTX(tx1); - wallet.addTX(tx1); - - // Create 1 block (no need to actually add it to chain) - const block1 = await getMockBlock(chain, [tx1]); - const entry1 = await ChainEntry.fromBlock(block1, chain.tip); - - // Unconfirm block into mempool - await mempool._removeBlock(entry1, block1.txs); - - // Mempool should contain newly unconfirmed TX - assert(mempool.hasEntry(tx1.hash())); - - // Mempool is NOT empty - assert.strictEqual(mempool.map.size, 1); - - // Create second TX - const coin2 = chaincoins.getCoins()[0]; - const mtx2 = new MTX(); - mtx2.addCoin(coin2); - mtx2.addOutput(addr, 90000); - chaincoins.sign(mtx2); - const tx2 = mtx2.toTX(); - chaincoins.addTX(tx2); - wallet.addTX(tx2); - - // Create 1 block (no need to actually add it to chain) - const block2 = await getMockBlock(chain, [tx2]); - const entry2 = await ChainEntry.fromBlock(block2, chain.tip); - - // Unconfirm block into mempool - await mempool._removeBlock(entry2, block2.txs); - - // Mempool should contain both TXs - assert(mempool.hasEntry(tx2.hash())); - assert(mempool.hasEntry(tx1.hash())); - assert.strictEqual(mempool.map.size, 2); - }); }); describe('AddrIndexer', function () { @@ -1108,4 +1060,308 @@ describe('Mempool', function() { throw err; }); }); + + describe('Mempool disconnect and reorg handling', function () { + const workers = new WorkerPool({ + enabled: true, + size: 2 + }); + + const blocks = new BlockStore({ + memory: true + }); + + const chain = new Chain({ + memory: true, + workers, + blocks + }); + + const mempool = new Mempool({ + chain, + workers, + memory: true, + indexAddress: true, + requireStandard: false // allow version 2 TXs without CSV activation + }); + + before(async () => { + await blocks.open(); + await mempool.open(); + await chain.open(); + await workers.open(); + }); + + after(async () => { + await workers.close(); + await chain.close(); + await mempool.close(); + await blocks.close(); + }); + + // Number of coins available in + // chaincoins (100k satoshi per coin). + const N = 100; + const chaincoins = new MemWallet(); + + it('should create coins in chain', async () => { + const mtx = new MTX(); + mtx.addInput(new Input()); + + for (let i = 0; i < N; i++) { + const addr = chaincoins.createReceive().getAddress(); + mtx.addOutput(addr, 100000); + } + + const cb = mtx.toTX(); + const block = await getMockBlock(chain, [cb], false); + const entry = await chain.add(block, VERIFY_BODY); + + await mempool._addBlock(entry, block.txs); + + // Add 100 blocks so we don't get + // premature spend of coinbase. + for (let i = 0; i < 100; i++) { + const block = await getMockBlock(chain); + const entry = await chain.add(block, VERIFY_BODY); + + await mempool._addBlock(entry, block.txs); + } + + chaincoins.addTX(cb); + }); + + it('should insert unconfirmed txs from removed block', async () => { + await mempool.reset(); + // Mempool is empty + assert.strictEqual(mempool.map.size, 0); + + // Create 1 TX + const coin1 = chaincoins.getCoins()[0]; + const addr = wallet.createReceive().getAddress(); + const mtx1 = new MTX(); + mtx1.addCoin(coin1); + mtx1.addOutput(addr, 90000); + chaincoins.sign(mtx1); + const tx1 = mtx1.toTX(); + chaincoins.addTX(tx1); + wallet.addTX(tx1); + + // Create 1 block (no need to actually add it to chain) + const block1 = await getMockBlock(chain, [tx1]); + const entry1 = await ChainEntry.fromBlock(block1, chain.tip); + + // Unconfirm block into mempool + await mempool._removeBlock(entry1, block1.txs); + + // Mempool should contain newly unconfirmed TX + assert(mempool.hasEntry(tx1.hash())); + + // Mempool is NOT empty + assert.strictEqual(mempool.map.size, 1); + + // Create second TX + const coin2 = chaincoins.getCoins()[0]; + const mtx2 = new MTX(); + mtx2.addCoin(coin2); + mtx2.addOutput(addr, 90000); + chaincoins.sign(mtx2); + const tx2 = mtx2.toTX(); + chaincoins.addTX(tx2); + wallet.addTX(tx2); + + // Create 1 block (no need to actually add it to chain) + const block2 = await getMockBlock(chain, [tx2]); + const entry2 = await ChainEntry.fromBlock(block2, chain.tip); + + // Unconfirm block into mempool + await mempool._removeBlock(entry2, block2.txs); + + // Mempool should contain both TXs + assert(mempool.hasEntry(tx2.hash())); + assert(mempool.hasEntry(tx1.hash())); + assert.strictEqual(mempool.map.size, 2); + + // Ensure mempool contents are valid in next block + const newBlock = await getMockBlock(chain, [tx1, tx2]); + const newEntry = await chain.add(newBlock, VERIFY_BODY); + await mempool._addBlock(newEntry, newBlock.txs); + assert.strictEqual(mempool.map.size, 0); + }); + + it('should handle reorg: coinbase spends', async () => { + // Mempool is empty + await mempool.reset(); + assert.strictEqual(mempool.map.size, 0); + + // Create a fresh coinbase tx + let cb = new MTX(); + cb.addInput(new Input()); + const addr = chaincoins.createReceive().getAddress(); + cb.addOutput(addr, 100000); + cb = cb.toTX(); + + // Add it to block and mempool + const block1 = await getMockBlock(chain, [cb], false); + const entry1 = await chain.add(block1, VERIFY_BODY); + await mempool._addBlock(entry1, block1.txs); + + // The coinbase output is a valid UTXO in the chain + assert(await chain.getCoin(cb.hash(), 0)); + + // Mempool is empty + assert.strictEqual(mempool.map.size, 0); + + // Attempt to spend the coinbase early + let spend = new MTX(); + spend.addTX(cb, 0); + spend.addOutput(addr, 90000); + chaincoins.sign(spend); + spend = spend.toTX(); + + // It's too early + await assert.rejects(async () => { + await mempool.addTX(spend, -1); + }, { + name: 'Error', + reason: 'bad-txns-premature-spend-of-coinbase' + }); + + // Add more blocks + let block2; + let entry2; + for (let i = 0; i < 99; i++) { + block2 = await getMockBlock(chain); + entry2 = await chain.add(block2, VERIFY_BODY); + + await mempool._addBlock(entry2, block2.txs); + } + + // Try again + await mempool.addTX(spend, -1); + + // Coinbase spend is in the mempool + assert.strictEqual(mempool.map.size, 1); + assert(mempool.getTX(spend.hash())); + + // Confirm coinbase spend in a block + const block3 = await getMockBlock(chain, [spend]); + const entry3 = await chain.add(block3, VERIFY_BODY); + await mempool._addBlock(entry3, block3.txs); + + // Coinbase spend has been removed from the mempool + assert.strictEqual(mempool.map.size, 0); + assert(!mempool.getTX(spend.hash())); + + // Now the block gets disconnected + await chain.disconnect(entry3); + await mempool._removeBlock(entry3, block3.txs); + + // Coinbase spend is back in the mempool + assert.strictEqual(mempool.map.size, 1); + assert(mempool.getTX(spend.hash())); + + // Now remove one more block from the chain, thus + // making the spend TX premature + await chain.disconnect(entry2); + await mempool._removeBlock(entry2, block2.txs); + + // Coinbase spend is still in the mempool + assert.strictEqual(mempool.map.size, 1); + assert(mempool.getTX(spend.hash())); + + // This is normally triggered by 'reorganize' event + await mempool._handleReorg(); + + // Premature coinbase spend has been evicted + assert.strictEqual(mempool.map.size, 0); + assert(!mempool.getTX(spend.hash())); + }); + + it('should handle reorg: BIP68 sequence locks', async () => { + // Mempool is empty + await mempool.reset(); + assert.strictEqual(mempool.map.size, 0); + + // Create a fresh UTXO + const fundCoin = chaincoins.getCoins()[0]; + let fund = new MTX(); + fund.addCoin(fundCoin); + const addr = chaincoins.createReceive().getAddress(); + fund.addOutput(addr, 90000); + chaincoins.sign(fund); + fund = fund.toTX(); + chaincoins.addTX(fund); + + // Add it to block amd mempool + const block1 = await getMockBlock(chain, [fund]); + const entry1 = await chain.add(block1, VERIFY_BODY); + await mempool._addBlock(entry1, block1.txs); + + // The fund TX output is a valid UTXO in the chain + const spendCoin = await chain.getCoin(fund.hash(), 0); + assert(spendCoin); + + // Mempool is empty + assert.strictEqual(mempool.map.size, 0); + + // Spend the coin with a sequence lock of 0x00000001 and version of 2. + // This should require the input coin to be 1 block old. + let spend = new MTX(); + spend.addCoin(spendCoin); + spend.addOutput(addr, 70000); + spend.inputs[0].sequence = 1; + spend.version = 2; + chaincoins.sign(spend); + spend = spend.toTX(); + + // Valid spend into mempool + await mempool.addTX(spend); + assert.strictEqual(mempool.map.size, 1); + assert(mempool.hasCoin(spend.hash(), 0)); + + // Confirm spend into block + const block2 = await getMockBlock(chain, [spend]); + const entry2 = await chain.add(block2, VERIFY_BODY); + await mempool._addBlock(entry2, block2.txs); + + // Spend has been removed from the mempool + assert.strictEqual(mempool.map.size, 0); + assert(!mempool.getTX(spend.hash())); + + // Now the block gets disconnected + await chain.disconnect(entry2); + await mempool._removeBlock(entry2, block2.txs); + + // Spend is back in the mempool + assert.strictEqual(mempool.map.size, 1); + assert(mempool.getTX(spend.hash())); + + // Now remove one more block from the chain, + // re-inserting the funding TX back into the mempool. + // This should make the sequence-locked spend invalid + // because its input coin is no lnger 1 block old. + await chain.disconnect(entry1); + await mempool._removeBlock(entry1, block1.txs); + + // Fund TX & spend TX are both still in the mempool + assert.strictEqual(mempool.map.size, 2); + assert(mempool.getTX(spend.hash())); + assert(mempool.getTX(fund.hash())); + + // This is normally triggered by 'reorganize' event + await mempool._handleReorg(); + + // Premature sequence lock spend has been evicted, fund TX remains + assert.strictEqual(mempool.map.size, 1); + assert(mempool.getTX(fund.hash())); + assert(!mempool.getTX(spend.hash())); + + // Ensure mempool contents are valid in next block + const newBlock = await getMockBlock(chain, [fund]); + const newEntry = await chain.add(newBlock, VERIFY_BODY); + await mempool._addBlock(newEntry, newBlock.txs); + assert.strictEqual(mempool.map.size, 0); + }); + }); }); From 0a5d105c5529f5248c7025cda0a7f1cb9b229e8d Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Thu, 2 Jan 2020 11:00:07 -0500 Subject: [PATCH 4/4] test: call mempool._handleReorg() after each _removeBlock() --- test/mempool-test.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/mempool-test.js b/test/mempool-test.js index 4a2c0fdf9..1473826a9 100644 --- a/test/mempool-test.js +++ b/test/mempool-test.js @@ -1153,6 +1153,7 @@ describe('Mempool', function() { // Unconfirm block into mempool await mempool._removeBlock(entry1, block1.txs); + await mempool._handleReorg(); // Mempool should contain newly unconfirmed TX assert(mempool.hasEntry(tx1.hash())); @@ -1176,6 +1177,7 @@ describe('Mempool', function() { // Unconfirm block into mempool await mempool._removeBlock(entry2, block2.txs); + await mempool._handleReorg(); // Mempool should contain both TXs assert(mempool.hasEntry(tx2.hash())); @@ -1256,6 +1258,7 @@ describe('Mempool', function() { // Now the block gets disconnected await chain.disconnect(entry3); await mempool._removeBlock(entry3, block3.txs); + await mempool._handleReorg(); // Coinbase spend is back in the mempool assert.strictEqual(mempool.map.size, 1); @@ -1332,6 +1335,7 @@ describe('Mempool', function() { // Now the block gets disconnected await chain.disconnect(entry2); await mempool._removeBlock(entry2, block2.txs); + await mempool._handleReorg(); // Spend is back in the mempool assert.strictEqual(mempool.map.size, 1);