From c00e365a0eeeb2c2e46c91b51d62c794d31a2605 Mon Sep 17 00:00:00 2001 From: Nodari Chkuaselidze Date: Thu, 23 Nov 2023 14:23:24 +0400 Subject: [PATCH 1/3] txdb: cover open reorg. --- lib/wallet/txdb.js | 15 ++- lib/wallet/walletdb.js | 3 +- test/wallet-auction-test.js | 192 +++++++++++++++++++++++++++++++----- 3 files changed, 184 insertions(+), 26 deletions(-) diff --git a/lib/wallet/txdb.js b/lib/wallet/txdb.js index 80528ba4b..d1fc2ca61 100644 --- a/lib/wallet/txdb.js +++ b/lib/wallet/txdb.js @@ -784,7 +784,6 @@ class TXDB { /** * Add transaction without a batch. - * @private * @param {TX} tx * @param {BlockMeta} [block] * @returns {Promise} @@ -1278,6 +1277,8 @@ class TXDB { // Commit the new state. The balance has updated. const balance = await this.updateBalance(b, state); + this.unindexOpens(b, tx); + await b.write(); this.unlockTX(tx); @@ -1533,6 +1534,14 @@ class TXDB { if (tx.isCoinbase()) return this.removeRecursive(wtx); + // On unconfirm, if we already have OPEN txs in the pending list we + // remove transaction and it's descendants instead of storing them in + // the pending list. This follows the mempool behaviour where the first + // entries in the mempool will be the ones left, instead of txs coming + // from the block. This ensures consistency with the double open rules. + if (await this.isDoubleOpen(tx)) + return this.removeRecursive(wtx); + return this.disconnect(wtx, wtx.getBlock()); } @@ -1650,6 +1659,10 @@ class TXDB { await this.saveCredit(b, credit, path); } + // Unconfirm will also index OPENs as the transaction is now part of the + // wallet pending transactions. + this.indexOpens(b, tx); + // Undo name state. await this.undoNameState(b, tx); diff --git a/lib/wallet/walletdb.js b/lib/wallet/walletdb.js index 135eb09ba..14db3d864 100644 --- a/lib/wallet/walletdb.js +++ b/lib/wallet/walletdb.js @@ -2215,7 +2215,7 @@ class WalletDB extends EventEmitter { /** * Revert TXDB to an older state. * @param {Number} target - * @returns {Promise} + * @returns {Promise} */ async revert(target) { @@ -2241,6 +2241,7 @@ class WalletDB extends EventEmitter { }); this.logger.info('Rolled back %d WalletDB transactions.', total); + return total; } /** diff --git a/test/wallet-auction-test.js b/test/wallet-auction-test.js index 69c09a69b..834be5c2e 100644 --- a/test/wallet-auction-test.js +++ b/test/wallet-auction-test.js @@ -12,8 +12,10 @@ const Network = require('../lib/protocol/network'); const rules = require('../lib/covenants/rules'); const Address = require('../lib/primitives/address'); const Output = require('../lib/primitives/output'); +const Coin = require('../lib/primitives/coin'); const Covenant = require('../lib/primitives/covenant'); const {Resource} = require('../lib/dns/resource'); +const {forEvent} = require('./util/common'); const network = Network.get('regtest'); const NAME1 = rules.grindName(10, 2, network); @@ -56,7 +58,7 @@ const wdb = new WalletDB({ }); describe('Wallet Auction', function() { - let winner, openAuctionMTX, openAuctionMTX2; + let wallet; before(async () => { // Open @@ -67,15 +69,19 @@ describe('Wallet Auction', function() { await wdb.connect(); // Set up wallet - winner = await wdb.create(); + wallet = await wdb.create(); chain.on('connect', async (entry, block) => { await wdb.addBlock(entry, block.txs); }); + chain.on('disconnect', async (entry) => { + await wdb.removeBlock(entry); + }); + // Generate blocks to roll out name and fund wallet - let winnerAddr = await winner.createReceive(); + let winnerAddr = await wallet.createReceive(); winnerAddr = winnerAddr.getAddress().toString(network); - for (let i = 0; i < 4; i++) { + for (let i = 0; i < 10; i++) { const block = await cpu.mineBlock(null, winnerAddr); await chain.add(block); } @@ -90,17 +96,44 @@ describe('Wallet Auction', function() { }); describe('Duplicate OPENs', function() { + // Prepare several OPEN txs to mine them on the network. + // Because they don't have any height, we can reuse them whenever + // we want. + const OPENS1 = 4; + const openTXs = []; + + // block/mempool/confirm indexes + let openIndex = 0; + const insertIndexes = []; + it('should open auction', async () => { - openAuctionMTX = await winner.createOpen(NAME1); - await winner.sign(openAuctionMTX); - const tx = openAuctionMTX.toTX(); - await wdb.addTX(tx); + for (let i = 0; i < OPENS1; i++) { + const open = await wallet.createOpen(NAME1); + await wallet.sign(open); + + assert.strictEqual(open.inputs.length, 1); + // make sure we don't double spend. + wallet.lockCoin(open.inputs[0].prevout); + openTXs.push(open); + } + + // This one will not get confirmed, but will be forever erased. + insertIndexes[0] = openIndex; + const openMTX = openTXs[openIndex++]; + const tx = openMTX.toTX(); + const addResult = await wdb.addTX(tx); + assert.strictEqual(addResult.size, 1); + assert.ok(addResult.has(wallet.wid)); + + const pending = await wallet.getPending(); + assert.strictEqual(pending.length, 1); + assert.bufferEqual(pending[0].hash, tx.hash()); }); it('should fail to create duplicate open', async () => { let err; try { - await winner.createOpen(NAME1); + await wallet.createOpen(NAME1); } catch (e) { err = e; } @@ -109,20 +142,48 @@ describe('Wallet Auction', function() { assert.strictEqual(err.message, `Already sent an open for: ${NAME1}.`); }); - it('should mine 1 block', async () => { + it('should not accept own duplicate open', async () => { + const pendingBefore = await wallet.getPending(); + assert.strictEqual(pendingBefore.length, 1); + assert.bufferEqual(pendingBefore[0].hash, openTXs[insertIndexes[0]].hash()); + + const openMTX = openTXs[openIndex]; + const result = await wdb.addTX(openMTX.toTX()); + assert.strictEqual(result, null); + + const pendingAfter = await wallet.getPending(); + assert.strictEqual(pendingAfter.length, 1); + assert.bufferEqual(pendingAfter[0].hash, openTXs[insertIndexes[0]].hash()); + }); + + it('should mine 1 block with different OPEN tx', async () => { const job = await cpu.createJob(); - job.addTX(openAuctionMTX.toTX(), openAuctionMTX.view); + + const removeEvents = forEvent(wdb, 'remove tx'); + + insertIndexes[1] = openIndex; + const openMTX = openTXs[openIndex++]; + + const [tx, view] = openMTX.commit(); + job.addTX(tx, view); job.refresh(); const block = await job.mineAsync(); - assert(await chain.add(block)); + + const removedTXs = await removeEvents; + assert.strictEqual(removedTXs.length, 1); + const removedTX = removedTXs[0].values[1]; + assert.bufferEqual(removedTX.hash(), openTXs[0].hash()); + + const pending = await wallet.getPending(); + assert.strictEqual(pending.length, 0); }); it('should fail to re-open auction during OPEN phase', async () => { let err; try { - await winner.createOpen(NAME1); + await wallet.createOpen(NAME1); } catch (e) { err = e; } @@ -140,12 +201,12 @@ describe('Wallet Auction', function() { }); it('should fail to send bid to null address', async () => { - const mtx = await winner.makeBid(NAME1, 1000, 2000, 0); + const mtx = await wallet.makeBid(NAME1, 1000, 2000, 0); mtx.outputs[0].address = new Address(); - await winner.fill(mtx); - await winner.finalize(mtx); + await wallet.fill(mtx); + await wallet.finalize(mtx); - const fn = async () => await winner.sendMTX(mtx); + const fn = async () => await wallet.sendMTX(mtx); await assert.rejects(fn, {message: 'Cannot send to null address.'}); }); @@ -153,7 +214,7 @@ describe('Wallet Auction', function() { it('should fail to re-open auction during BIDDING phase', async () => { let err; try { - await winner.createOpen(NAME1); + await wallet.createOpen(NAME1); } catch (e) { err = e; } @@ -171,16 +232,16 @@ describe('Wallet Auction', function() { }); it('should open auction (again)', async () => { - openAuctionMTX2 = await winner.createOpen(NAME1); - await winner.sign(openAuctionMTX2); - const tx = openAuctionMTX2.toTX(); - await wdb.addTX(tx); + // This one will be inserted and THEN confirmed. + insertIndexes[2] = openIndex; + const mtx = openTXs[openIndex++]; + await wdb.addTX(mtx.toTX()); }); it('should fail to create duplicate open (again)', async () => { let err; try { - await winner.createOpen(NAME1); + await wallet.createOpen(NAME1); } catch (e) { err = e; } @@ -191,7 +252,8 @@ describe('Wallet Auction', function() { it('should confirm OPEN transaction', async () => { const job = await cpu.createJob(); - job.addTX(openAuctionMTX2.toTX(), openAuctionMTX2.view); + const [tx, view] = openTXs[insertIndexes[2]].commit(); + job.addTX(tx, view); job.refresh(); const block = await job.mineAsync(); @@ -211,6 +273,88 @@ describe('Wallet Auction', function() { state = ns.state(chain.height, network); assert.strictEqual(state, states.BIDDING); }); + + it('should create TX spending change of the OPEN', async () => { + // Last OPEN and spending change will be used for the test in + // the pending index test. + const lastOpenMTX = openTXs[insertIndexes[2]]; + const change = lastOpenMTX.outputs[1]; + assert.notStrictEqual(change.value, 0); + + // does not matter where this goes. + const spendMTX = wallet.makeTX([{ + value: 1e5, + address: change.address + }]); + + const coin = Coin.fromTX(lastOpenMTX.toTX(), 1, wdb.height); + await spendMTX.fund([coin], { + changeAddress: await wallet.changeAddress() + }); + + // We don't mine this transaction and reuse this to make sure + // double opens are properly removed. + await wallet.sign(spendMTX); + const added = await wdb.addTX(spendMTX.toTX()); + assert.strictEqual(added.size, 1); + }); + + it('should mine enough blocks to expire auction (again)', async () => { + for (let i = 0; i < biddingPeriod + revealPeriod; i++) { + const block = await cpu.mineBlock(); + assert(block); + assert(await chain.add(block)); + } + }); + + it('should insert OPEN into the block', async () => { + // This makes sure the confirmed/mined TX does not get removed + const job = await cpu.createJob(); + + insertIndexes[3] = openIndex; + const openMTX = openTXs[openIndex++]; + let countRemoves = 0; + + wdb.on('remove tx', () => { + countRemoves++; + }); + + const [tx, view] = openMTX.commit(); + job.addTX(tx, view); + job.refresh(); + + const block = await job.mineAsync(); + assert(await chain.add(block)); + + assert.strictEqual(countRemoves, 0); + }); + + it('should revert the two auctions and only leave one open', async () => { + const pendingBefore = await wallet.getPending(); + assert.strictEqual(pendingBefore.length, 1); + + await wdb.rollback(biddingPeriod + revealPeriod + 2); + const pendingAfter = await wallet.getPending(); + + // first OPEN and tx spending from first OPEN should get removed. + // This mimics the behaviour of the mempool where OPENs from the block + // will end up getting removed, if there's OPEN sitting there. + assert.strictEqual(pendingAfter.length, 1); + + const secondTX = await wallet.getTX(openTXs[insertIndexes[2]].hash()); + assert.strictEqual(secondTX, null); + }); + + it('should resync and recover', async () => { + for (let i = wdb.height; i <= chain.tip.height; i++) { + const entry = await chain.getEntryByHeight(i); + const block = await chain.getBlock(entry.hash); + await wdb.addBlock(entry, block.txs); + } + + const secondTX = await wallet.getTX(openTXs[insertIndexes[2]].hash()); + assert.notStrictEqual(secondTX, null); + }); }); describe('Batch TXs', function() { From c74d528f3a89e1c56606fb061dac48d5ed26ff65 Mon Sep 17 00:00:00 2001 From: Nodari Chkuaselidze Date: Fri, 24 Nov 2023 09:27:55 +0400 Subject: [PATCH 2/3] txdb: avoid migration and remove tx after checking height. --- lib/wallet/txdb.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/wallet/txdb.js b/lib/wallet/txdb.js index d1fc2ca61..d5f53d98e 100644 --- a/lib/wallet/txdb.js +++ b/lib/wallet/txdb.js @@ -879,6 +879,11 @@ class TXDB { if (!hash) continue; + const wtx = await this.getTX(hash); + + if (wtx.height !== -1) + return; + await this.remove(hash); } } From a1c72c2d6894333291b8593cc3950ec985c48b11 Mon Sep 17 00:00:00 2001 From: Nodari Chkuaselidze Date: Fri, 24 Nov 2023 15:11:48 +0400 Subject: [PATCH 3/3] wallet: watch open earlier. This allows to track double OPEN issue from the outside of the wallet. Should reduce number of stale OPEN txs. --- lib/wallet/records.js | 4 ++ lib/wallet/txdb.js | 29 +++++++++++-- lib/wallet/walletdb.js | 27 ++++++++++++ test/mempool-invalidation-test.js | 4 +- test/wallet-auction-test.js | 70 +++++++++++++++++++++++-------- 5 files changed, 110 insertions(+), 24 deletions(-) diff --git a/lib/wallet/records.js b/lib/wallet/records.js index a81ccb8ab..a5d8d0836 100644 --- a/lib/wallet/records.js +++ b/lib/wallet/records.js @@ -422,6 +422,10 @@ class MapRecord extends bio.Struct { return this.wids.delete(wid); } + has(wid) { + return this.wids.has(wid); + } + write(bw) { bw.writeU32(this.wids.size); diff --git a/lib/wallet/txdb.js b/lib/wallet/txdb.js index d5f53d98e..a1e53ab4f 100644 --- a/lib/wallet/txdb.js +++ b/lib/wallet/txdb.js @@ -1045,6 +1045,7 @@ class TXDB { } await this.saveCredit(b, credit, path); + await this.watchOpensEarly(b, output); } // Handle names. @@ -1852,12 +1853,33 @@ class TXDB { } } + /** + * Start tracking OPENs right away. + * This does not check if the name is owned by the wallet. + * @private + * @param {Batch} b + * @param {Output} tx + * @param {Path} path + * @returns {Promise} + */ + + async watchOpensEarly(b, output) { + const {covenant} = output; + + if (!covenant.isOpen()) + return; + + const nameHash = covenant.getHash(0); + + if (!await this.wdb.hasNameMap(nameHash, this.wid)) + await this.addNameMap(b, nameHash); + } + /** * Handle incoming covenant. * @param {Object} b * @param {TX} tx - * @param {Number} i - * @param {Path} path + * @param {CoinView} view * @param {Number} height * @returns {Promise} updated */ @@ -1916,8 +1938,7 @@ class TXDB { case types.OPEN: { if (!path) { // Are we "watching" this name? - const map = await this.wdb.getNameMap(nameHash); - if (!map || !map.wids.has(this.wid)) + if (!await this.wdb.hasNameMap(nameHash, this.wid)) break; const name = covenant.get(2); diff --git a/lib/wallet/walletdb.js b/lib/wallet/walletdb.js index 14db3d864..32503c0f3 100644 --- a/lib/wallet/walletdb.js +++ b/lib/wallet/walletdb.js @@ -1925,6 +1925,22 @@ class WalletDB extends EventEmitter { return MapRecord.decode(data); } + /** + * Does wdb have wallet map. + * @param {Buffer} key + * @param {Number} wid + * @returns {Promise} + */ + + async hasMap(key, wid) { + const map = await this.getMap(key); + + if (!map) + return false; + + return map.has(wid); + } + /** * Add wid to a wallet map. * @param {Wallet} wallet @@ -2116,6 +2132,17 @@ class WalletDB extends EventEmitter { return this.getMap(layout.N.encode(nameHash)); } + /** + * Has wid in the wallet map. + * @param {Buffer} nameHash + * @param {Number} wid + * @returns {Promise} + */ + + async hasNameMap(nameHash, wid) { + return this.hasMap(layout.N.encode(nameHash), wid); + } + /** * Add wid to a wallet map. * @param {Wallet} wallet diff --git a/test/mempool-invalidation-test.js b/test/mempool-invalidation-test.js index 85c27bffe..67ec153e8 100644 --- a/test/mempool-invalidation-test.js +++ b/test/mempool-invalidation-test.js @@ -137,8 +137,8 @@ describe('Mempool Invalidation', function() { assert.strictEqual(await getNameState(name), states.OPENING); assert.strictEqual(node.mempool.map.size, 0); - // we don't want coins to get stuck in the wallet. - wallet2.abandon(memopen.hash()); + const pending = await wallet2.getPending(); + assert.strictEqual(pending.length, 0); }); it('should invalidate bids', async () => { diff --git a/test/wallet-auction-test.js b/test/wallet-auction-test.js index 834be5c2e..267e111dd 100644 --- a/test/wallet-auction-test.js +++ b/test/wallet-auction-test.js @@ -18,7 +18,6 @@ const {Resource} = require('../lib/dns/resource'); const {forEvent} = require('./util/common'); const network = Network.get('regtest'); -const NAME1 = rules.grindName(10, 2, network); const { treeInterval, biddingPeriod, @@ -58,7 +57,10 @@ const wdb = new WalletDB({ }); describe('Wallet Auction', function() { - let wallet; + let wallet, wallet2; + + const name1 = rules.grindName(10, 2, network); + const name2 = rules.grindName(10, 2, network); before(async () => { // Open @@ -70,6 +72,7 @@ describe('Wallet Auction', function() { // Set up wallet wallet = await wdb.create(); + wallet2 = await wdb.create(); chain.on('connect', async (entry, block) => { await wdb.addBlock(entry, block.txs); }); @@ -79,10 +82,16 @@ describe('Wallet Auction', function() { }); // Generate blocks to roll out name and fund wallet - let winnerAddr = await wallet.createReceive(); - winnerAddr = winnerAddr.getAddress().toString(network); - for (let i = 0; i < 10; i++) { - const block = await cpu.mineBlock(null, winnerAddr); + let walletAddr = await wallet.createReceive(); + walletAddr = walletAddr.getAddress().toString(network); + for (let i = 0; i < 5; i++) { + const block = await cpu.mineBlock(null, walletAddr); + await chain.add(block); + } + + walletAddr = (await wallet2.createReceive()).getAddress().toString(network); + for (let i = 0; i < 5; i++) { + const block = await cpu.mineBlock(null, walletAddr); await chain.add(block); } }); @@ -108,7 +117,7 @@ describe('Wallet Auction', function() { it('should open auction', async () => { for (let i = 0; i < OPENS1; i++) { - const open = await wallet.createOpen(NAME1); + const open = await wallet.createOpen(name1); await wallet.sign(open); assert.strictEqual(open.inputs.length, 1); @@ -133,13 +142,13 @@ describe('Wallet Auction', function() { it('should fail to create duplicate open', async () => { let err; try { - await wallet.createOpen(NAME1); + await wallet.createOpen(name1); } catch (e) { err = e; } assert(err); - assert.strictEqual(err.message, `Already sent an open for: ${NAME1}.`); + assert.strictEqual(err.message, `Already sent an open for: ${name1}.`); }); it('should not accept own duplicate open', async () => { @@ -183,13 +192,13 @@ describe('Wallet Auction', function() { it('should fail to re-open auction during OPEN phase', async () => { let err; try { - await wallet.createOpen(NAME1); + await wallet.createOpen(name1); } catch (e) { err = e; } assert(err); - assert.strictEqual(err.message, `Name is already opening: ${NAME1}.`); + assert.strictEqual(err.message, `Name is already opening: ${name1}.`); }); it('should mine enough blocks to enter BIDDING phase', async () => { @@ -201,7 +210,7 @@ describe('Wallet Auction', function() { }); it('should fail to send bid to null address', async () => { - const mtx = await wallet.makeBid(NAME1, 1000, 2000, 0); + const mtx = await wallet.makeBid(name1, 1000, 2000, 0); mtx.outputs[0].address = new Address(); await wallet.fill(mtx); await wallet.finalize(mtx); @@ -214,13 +223,13 @@ describe('Wallet Auction', function() { it('should fail to re-open auction during BIDDING phase', async () => { let err; try { - await wallet.createOpen(NAME1); + await wallet.createOpen(name1); } catch (e) { err = e; } assert(err); - assert.strictEqual(err.message, `Name is not available: ${NAME1}.`); + assert.strictEqual(err.message, `Name is not available: ${name1}.`); }); it('should mine enough blocks to expire auction', async () => { @@ -241,13 +250,13 @@ describe('Wallet Auction', function() { it('should fail to create duplicate open (again)', async () => { let err; try { - await wallet.createOpen(NAME1); + await wallet.createOpen(name1); } catch (e) { err = e; } assert(err); - assert.strictEqual(err.message, `Already sent an open for: ${NAME1}.`); + assert.strictEqual(err.message, `Already sent an open for: ${name1}.`); }); it('should confirm OPEN transaction', async () => { @@ -259,7 +268,7 @@ describe('Wallet Auction', function() { const block = await job.mineAsync(); assert(await chain.add(block)); - let ns = await chain.db.getNameStateByName(NAME1); + let ns = await chain.db.getNameStateByName(name1); let state = ns.state(chain.height, network); assert.strictEqual(state, states.OPENING); @@ -269,7 +278,7 @@ describe('Wallet Auction', function() { assert(await chain.add(block)); } - ns = await chain.db.getNameStateByName(NAME1); + ns = await chain.db.getNameStateByName(name1); state = ns.state(chain.height, network); assert.strictEqual(state, states.BIDDING); }); @@ -355,6 +364,31 @@ describe('Wallet Auction', function() { const secondTX = await wallet.getTX(openTXs[insertIndexes[2]].hash()); assert.notStrictEqual(secondTX, null); }); + + it('should handle foreign double open after sending open', async () => { + const open = await wallet.createOpen(name2); + await wallet.sign(open); + + const open2 = await wallet2.createOpen(name2); + await wallet2.sign(open2); + + // try to open. + await wdb.addTX(open.toTX()); + const pending1 = await wallet.getPending(); + assert.strictEqual(pending1.length, 1); + assert.bufferEqual(pending1[0].hash, open.hash()); + + const job = await cpu.createJob(); + const [tx, view] = open2.commit(); + job.addTX(tx, view); + job.refresh(); + + const block = await job.mineAsync(); + assert(await chain.add(block)); + + const pending1after = await wallet.getPending(); + assert.strictEqual(pending1after.length, 0); + }); }); describe('Batch TXs', function() {