From 9aef8398d681b680cada2ac2bfc0718f1b0c7865 Mon Sep 17 00:00:00 2001 From: Kenny Joseph Date: Thu, 3 Oct 2024 17:29:43 -0400 Subject: [PATCH 1/6] add deep dive into string of replacements --- packages/bitcore-node/src/services/pruning.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/bitcore-node/src/services/pruning.ts b/packages/bitcore-node/src/services/pruning.ts index 1fcbc6ba997..ce4f3f09c11 100644 --- a/packages/bitcore-node/src/services/pruning.ts +++ b/packages/bitcore-node/src/services/pruning.ts @@ -21,6 +21,7 @@ const args = parseArgv([], [ { arg: 'MEMPOOL_AGE', type: 'int' }, { arg: 'OLD_INTERVAL_HRS', type: 'float' }, { arg: 'INV_INTERVAL_MINS', type: 'float' }, + { arg: 'INV_MATURE_LEN', type: 'int' }, { arg: 'DESCENDANT_LIMIT', type: 'int' }, { arg: 'VERBOSE', type: 'bool' } ]); @@ -33,6 +34,7 @@ const CHAIN = args.CHAIN || PRUNING_CHAIN; const NETWORK = args.NETWORK || PRUNING_NETWORK; const OLD_INTERVAL_HRS = args.OLD_INTERVAL_HRS || Number(PRUNING_OLD_INTERVAL_HRS) || 12; const INV_INTERVAL_MINS = args.INV_INTERVAL_MINS || Number(PRUNING_INV_INTERVAL_MINS) || 10; +const INV_MATURE_LEN = args.INV_MATURE_LEN || 3; // using || means INV_MATURE_LEN needs to be >0 const MEMPOOL_AGE = args.MEMPOOL_AGE || Number(PRUNING_MEMPOOL_AGE) || 7; const DESCENDANT_LIMIT = args.DESCENDANT_LIMIT || Number(PRUNING_DESCENDANT_LIMIT) || 10; const VERBOSE = Boolean(args.VERBOSE ?? false); @@ -266,8 +268,12 @@ export class PruningService { async invalidateTx(chain: string, network: string, tx: ITransaction) { const tipHeight = await this.rpcs[`${chain}:${network}`].getBlockHeight(); - const rTx = await this.transactionModel.collection.findOne({ chain, network, txid: tx.replacedByTxid }); - const isMature = rTx?.blockHeight! > SpentHeightIndicators.pending && tipHeight - rTx?.blockHeight! > 3; + let rTx = await this.transactionModel.collection.findOne({ chain, network, txid: tx.replacedByTxid }); + while (rTx?.replacedByTxid && rTx?.blockHeight! < 0) { + // replacement tx has also been replaced + rTx = await this.transactionModel.collection.findOne({ chain, network, txid: rTx.replacedByTxid }); + } + const isMature = rTx?.blockHeight! > SpentHeightIndicators.pending && tipHeight - rTx?.blockHeight! > INV_MATURE_LEN; if (isMature) { try { logger.info(`${args.DRY ? 'DRY RUN - ' : ''}Invalidating ${tx.txid} with replacement => ${tx.replacedByTxid}`); From 535f58df93ecf4eb732b7a68368a2fa1b80621bc Mon Sep 17 00:00:00 2001 From: Kenny Joseph Date: Fri, 4 Oct 2024 15:49:59 -0400 Subject: [PATCH 2/6] guard against circular replacements & edge cases --- packages/bitcore-node/src/services/pruning.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/bitcore-node/src/services/pruning.ts b/packages/bitcore-node/src/services/pruning.ts index ce4f3f09c11..25d0ef57941 100644 --- a/packages/bitcore-node/src/services/pruning.ts +++ b/packages/bitcore-node/src/services/pruning.ts @@ -246,6 +246,7 @@ export class PruningService { invalidCount++; } } else { + // Check if the parent tx was replaced since the sync process marks immediate replacements as replaced, but not descendants const vins = await this.coinModel.collection.find({ chain, network, spentTxid: vout.mintTxid }).toArray(); const vinTxs = await this.transactionModel.collection.find({ chain, network, txid: { $in: vins.map(vin => vin.mintTxid) } }).toArray(); for (const tx of vinTxs) { @@ -267,12 +268,22 @@ export class PruningService { } async invalidateTx(chain: string, network: string, tx: ITransaction) { - const tipHeight = await this.rpcs[`${chain}:${network}`].getBlockHeight(); + if (tx.blockHeight! >= 0) { + // This means that downstream coins are still pending when they should be marked as confirmed. + // This indicates a bug in the sync process. + logger.warn(`Tx ${tx.txid} is already mined`); + return false; + } let rTx = await this.transactionModel.collection.findOne({ chain, network, txid: tx.replacedByTxid }); - while (rTx?.replacedByTxid && rTx?.blockHeight! < 0) { + while (rTx?.replacedByTxid && rTx?.blockHeight! < 0 && rTx?.txid !== tx.txid) { // replacement tx has also been replaced + // Note: rTx.txid === tx.txid may happen if tx.replacedByTxid => rTx.txid and rTx.replacedByTxid => tx.txid. + // This might happen if tx was rebroadcast _after_ being marked as replaced by rTx, thus marking rTx as replaced by tx. + // Without this check, we could end up in an infinite loop where the two txs keep finding each other as unconfirmed replacements. rTx = await this.transactionModel.collection.findOne({ chain, network, txid: rTx.replacedByTxid }); } + // Re-org protection + const tipHeight = await this.rpcs[`${chain}:${network}`].getBlockHeight(); const isMature = rTx?.blockHeight! > SpentHeightIndicators.pending && tipHeight - rTx?.blockHeight! > INV_MATURE_LEN; if (isMature) { try { From 603570765ae612be23cb429e61faacf3ad394a4e Mon Sep 17 00:00:00 2001 From: Kenny Joseph Date: Tue, 8 Oct 2024 09:02:12 -0400 Subject: [PATCH 3/6] added jsdocs --- packages/bitcore-node/src/services/pruning.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/bitcore-node/src/services/pruning.ts b/packages/bitcore-node/src/services/pruning.ts index 25d0ef57941..447870221bd 100644 --- a/packages/bitcore-node/src/services/pruning.ts +++ b/packages/bitcore-node/src/services/pruning.ts @@ -267,6 +267,13 @@ export class PruningService { } } + /** + * Invalidate a transaction and its descendants + * @param {string} chain + * @param {string} network + * @param {ITransaction} tx Transaction object with replacedByTxid + * @returns + */ async invalidateTx(chain: string, network: string, tx: ITransaction) { if (tx.blockHeight! >= 0) { // This means that downstream coins are still pending when they should be marked as confirmed. @@ -274,6 +281,10 @@ export class PruningService { logger.warn(`Tx ${tx.txid} is already mined`); return false; } + if (!tx.replacedByTxid) { + logger.warn(`Given tx has no replacedByTxid: ${tx.txid}`); + return false; + } let rTx = await this.transactionModel.collection.findOne({ chain, network, txid: tx.replacedByTxid }); while (rTx?.replacedByTxid && rTx?.blockHeight! < 0 && rTx?.txid !== tx.txid) { // replacement tx has also been replaced From 582dc25f321dc24367bd5861324815076b525c30 Mon Sep 17 00:00:00 2001 From: Kenny Joseph Date: Tue, 8 Oct 2024 11:09:46 -0400 Subject: [PATCH 4/6] add test cases for pruning invalid --- .../services/pruning.integration.ts | 357 +++++++++++------- 1 file changed, 211 insertions(+), 146 deletions(-) diff --git a/packages/bitcore-node/test/integration/services/pruning.integration.ts b/packages/bitcore-node/test/integration/services/pruning.integration.ts index d8c11abe293..2ebdea73118 100644 --- a/packages/bitcore-node/test/integration/services/pruning.integration.ts +++ b/packages/bitcore-node/test/integration/services/pruning.integration.ts @@ -4,11 +4,14 @@ import { MongoBound } from '../../../src/models/base'; import { CoinStorage, ICoin } from '../../../src/models/coin'; import { IBtcTransaction, TransactionStorage } from '../../../src/models/transaction'; import { PruningService } from '../../../src/services/pruning'; -const Pruning = new PruningService({ transactionModel: TransactionStorage, coinModel: CoinStorage }); import '../../../src/utils/polyfills'; import { resetDatabase } from '../../helpers'; import { intAfterHelper, intBeforeHelper } from '../../helpers/integration'; import { RPC } from '../../../src/rpc'; +import { SpentHeightIndicators } from '../../../src/types/Coin'; +import logger from '../../../src/logger'; + +const Pruning = new PruningService({ transactionModel: TransactionStorage, coinModel: CoinStorage }); describe('Pruning Service', function() { const suite = this; @@ -25,6 +28,9 @@ describe('Pruning Service', function() { Pruning.rpcs['BTC:mainnet'] = new RPC('user', 'pw', 'host', 'port'); sandbox.stub(Pruning.rpcs['BTC:mainnet'], 'getBlockHeight').resolves(1240); process.env.DRYRUN = 'false'; + sandbox.spy(logger, 'info'); + sandbox.spy(logger, 'warn'); + sandbox.spy(logger, 'error'); }); afterEach(() => { process.env.DRYRUN = undefined; @@ -41,7 +47,7 @@ describe('Pruning Service', function() { const invalidTx = { chain: 'BTC', network: 'mainnet', - blockHeight: -3, + blockHeight: SpentHeightIndicators.conflicting, txid: 'invalidCoin', replacedByTxid: 'replacementTx' } as MongoBound; @@ -49,32 +55,32 @@ describe('Pruning Service', function() { const invalidCoin = { chain: 'BTC', network: 'mainnet', - mintHeight: -1, + mintHeight: SpentHeightIndicators.pending, mintTxid: 'invalidCoin', - spentHeight: -1, + spentHeight: SpentHeightIndicators.pending, spentTxid: 'spentInMempool' } as ICoin; const mempoolCoin = { chain: 'BTC', network: 'mainnet', - mintHeight: -1, + mintHeight: SpentHeightIndicators.pending, mintTxid: 'spentInMempool', - spentHeight: -1, + spentHeight: SpentHeightIndicators.pending, spentTxid: 'spentInMempoolAgain' } as ICoin; const mempoolCoin2 = { chain: 'BTC', network: 'mainnet', - mintHeight: -1, + mintHeight: SpentHeightIndicators.pending, mintTxid: 'spentInMempoolAgain' } as ICoin; const oldMempoolTx = { chain: 'BTC', network: 'mainnet', - blockHeight: -1, + blockHeight: SpentHeightIndicators.pending, txid: 'oldMempoolTx', blockTimeNormalized: new Date(Date.now() - 30 * 24 * 60 * 60 * 1000) } as MongoBound; @@ -82,7 +88,7 @@ describe('Pruning Service', function() { const oldMempoolTxOutput = { chain: 'BTC', network: 'mainnet', - mintHeight: -1, + mintHeight: SpentHeightIndicators.pending, mintTxid: 'oldMempoolTx', spentTxid: 'oldMempoolTx2' } as ICoin; @@ -90,7 +96,7 @@ describe('Pruning Service', function() { const oldMempoolTx2Output = { // output that spends oldMempoolTxOutput chain: 'BTC', network: 'mainnet', - mintHeight: -1, + mintHeight: SpentHeightIndicators.pending, mintTxid: 'oldMempoolTx2', spentTxid: '' } as ICoin; @@ -100,7 +106,7 @@ describe('Pruning Service', function() { network: 'mainnet', mintHeight: 1234, mintTxid: 'parentTx', - spentHeight: -1, + spentHeight: SpentHeightIndicators.pending, spentTxid: 'oldMempoolTx' // this output is an input for oldMempoolTx } as ICoin; @@ -109,7 +115,7 @@ describe('Pruning Service', function() { network: 'mainnet', mintHeight: 1234, mintTxid: 'parentTx', - spentHeight: -1, + spentHeight: SpentHeightIndicators.pending, spentTxid: 'imaginaryTx' // another imaginary tx spent this output. This is here to make sure we don't mark this output as unspent } as ICoin; @@ -135,150 +141,209 @@ describe('Pruning Service', function() { await CoinStorage.collection.insertOne({ ...oldMempoolTx2Output, mintTxid: modTxid(oldMempoolTx2Output.mintTxid, i) }); } - it('should detect coins that should be invalid but are not', async () => { - await insertBadCoins(); - const { chain, network } = invalidCoin; - await Pruning.processAllInvalidTxs(chain, network); - const shouldBeInvalid = await CoinStorage.collection - .find({ chain, network, mintTxid: { $in: [invalidCoin.mintTxid, mempoolCoin.mintTxid, mempoolCoin2.mintTxid] } }) - .toArray(); - for (const coin of shouldBeInvalid) { - expect(coin.mintHeight).eq(-3); - } - expect(shouldBeInvalid.length).eq(3); - }); + describe('processAllInvalidTxs', function() { + it('should detect coins that should be invalid but are not', async () => { + await insertBadCoins(); + const { chain, network } = invalidCoin; + await Pruning.processAllInvalidTxs(chain, network); + const shouldBeInvalid = await CoinStorage.collection + .find({ chain, network, mintTxid: { $in: [invalidCoin.mintTxid, mempoolCoin.mintTxid, mempoolCoin2.mintTxid] } }) + .toArray(); + for (const coin of shouldBeInvalid) { + expect(coin.mintHeight).eq(SpentHeightIndicators.conflicting); + } + expect(shouldBeInvalid.length).eq(3); + }); - it('should mark detected coins as invalid', async () => { - await insertBadCoins(); - const { chain, network } = invalidCoin; - await Pruning.processAllInvalidTxs(chain, network); - const shouldBeInvalid = await CoinStorage.collection - .find({ chain, network, mintTxid: { $in: [invalidCoin.mintTxid, mempoolCoin.mintTxid, mempoolCoin2.mintTxid] } }) - .toArray(); - for (const coin of shouldBeInvalid) { - expect(coin.mintHeight).eq(-3); - } - expect(shouldBeInvalid.length).eq(3); - }); + it('should mark detected coins as invalid', async () => { + await insertBadCoins(); + const { chain, network } = invalidCoin; + await Pruning.processAllInvalidTxs(chain, network); + const shouldBeInvalid = await CoinStorage.collection + .find({ chain, network, mintTxid: { $in: [invalidCoin.mintTxid, mempoolCoin.mintTxid, mempoolCoin2.mintTxid] } }) + .toArray(); + for (const coin of shouldBeInvalid) { + expect(coin.mintHeight).eq(SpentHeightIndicators.conflicting); + } + expect(shouldBeInvalid.length).eq(3); + }); - it('should remove old transactions', async () => { - sandbox.stub(RPC.prototype, 'getTransaction').resolves(null); - await insertOldTx(); - const { chain, network } = oldMempoolTx; + it('should not invalidate a mined tx', async function() { + await insertBadCoins(); + const { chain, network } = invalidCoin; + await Pruning.invalidateTx(chain, network, replacementTx); + const shouldBeInvalidStill = await CoinStorage.collection + .find({ chain, network, mintTxid: { $in: [invalidCoin.mintTxid, mempoolCoin.mintTxid, mempoolCoin2.mintTxid] } }) + .toArray(); + expect(shouldBeInvalidStill.every(coin => coin.mintHeight === SpentHeightIndicators.pending)).to.equal(true); + expect(shouldBeInvalidStill.length).eq(3); + const msg = `Tx ${replacementTx.txid} is already mined`; + expect((logger.warn as any).args.find((args: any) => args[0] === msg)).to.exist; + }); - const count = await TransactionStorage.collection.countDocuments({ - chain, - network, - blockHeight: -1, - blockTimeNormalized: { $lt: new Date() } + it('should not go into an infinite loop if there is a circular replacedByTxid reference', async function() { + await insertBadCoins(); + // invalidTx replaced by replacementTx, replacementTx replaced by invalidTx + await TransactionStorage.collection.updateOne({ + txid: replacementTx.txid + }, { + $set: { + replacedByTxid: invalidTx.txid + } + }); + const { chain, network } = invalidCoin; + await Pruning.processAllInvalidTxs(chain, network); + const shouldBeInvalid = await CoinStorage.collection + .find({ chain, network, mintTxid: { $in: [invalidCoin.mintTxid, mempoolCoin.mintTxid, mempoolCoin2.mintTxid] } }) + .toArray(); + expect(shouldBeInvalid.every(coin => coin.mintHeight === SpentHeightIndicators.conflicting)).to.equal(true); + expect(shouldBeInvalid.length).eq(3); }); - expect(count).eq(1); - await Pruning.processOldMempoolTxs(chain, network, 29); - - const shouldBeExpiredTx = await TransactionStorage.collection - .find({ chain, network, txid: { $in: [oldMempoolTx.txid] } }) - .toArray(); - const shouldBeExpiredCoins = await CoinStorage.collection - .find({ chain, network, mintTxid: { $in: [oldMempoolTxOutput.mintTxid, oldMempoolTx2Output.mintTxid] } }) - .toArray(); - const parentTxOutputs = await CoinStorage.collection - .find({ chain, network, mintTxid: parentTxOutput1.mintTxid }) - .toArray(); - - expect(shouldBeExpiredTx.length).eq(1); - expect(shouldBeExpiredTx.every(tx => tx.blockHeight === -5)).to.equal(true); - expect(shouldBeExpiredCoins.length).eq(2); - expect(shouldBeExpiredCoins.every(coin => coin.mintHeight === -5)).to.equal(true); - expect(parentTxOutputs.length).eq(2); - expect(parentTxOutputs.filter(coin => coin.spentHeight === -2).length).to.equal(1); - }); - it('should skip removing transactions still in mempool', async () => { - const rpcStub = sandbox.stub(RPC.prototype, 'getTransaction') - rpcStub.onCall(0).resolves(null); - rpcStub.onCall(1).resolves({}); - rpcStub.onCall(2).resolves(null); - await insertOldTx(0); - await insertOldTx(1); - await insertOldTx(2); - const { chain, network } = oldMempoolTx; - - const count = await TransactionStorage.collection.countDocuments({ - chain, - network, - blockHeight: -1, - blockTimeNormalized: { $lt: new Date() } + it('should not go into an infinite loop if there is a circular, unconfirmed replacedByTxid reference', async function() { + await insertBadCoins(); + // invalidTx replaced by replacementTx, replacementTx replaced by invalidTx + // but replacementTx is still unconfirmed + await TransactionStorage.collection.updateOne({ + txid: replacementTx.txid + }, { + $set: { + replacedByTxid: invalidTx.txid, + blockHeight: SpentHeightIndicators.pending + } + }); + const { chain, network } = invalidCoin; + await Pruning.processAllInvalidTxs(chain, network); + const shouldBePendingStill = await CoinStorage.collection + .find({ chain, network, mintTxid: { $in: [invalidCoin.mintTxid, mempoolCoin.mintTxid, mempoolCoin2.mintTxid] } }) + .toArray(); + expect(shouldBePendingStill.every(coin => coin.mintHeight === SpentHeightIndicators.pending)).to.equal(true); + expect(shouldBePendingStill.length).eq(3); + const msg = `Skipping invalidation of ${invalidTx.txid} with immature replacement => ${invalidTx.replacedByTxid}`; + expect((logger.info as any).args.find((args: any) => args[0] === msg)).to.exist; }); - expect(count).eq(3); - await Pruning.processOldMempoolTxs(chain, network, 29); - - const processedTxs = await TransactionStorage.collection - .find({ chain, network, txid: { $in: [modTxid(oldMempoolTx.txid, 0), modTxid(oldMempoolTx.txid, 1), modTxid(oldMempoolTx.txid, 2)] } }) - .toArray(); - const processedCoins = await CoinStorage.collection - .find({ chain, network, mintTxid: { $in: [modTxid(oldMempoolTxOutput.mintTxid, 0), modTxid(oldMempoolTx2Output.mintTxid, 0), modTxid(oldMempoolTxOutput.mintTxid, 1), modTxid(oldMempoolTx2Output.mintTxid, 1), modTxid(oldMempoolTxOutput.mintTxid, 2), modTxid(oldMempoolTx2Output.mintTxid, 2)] } }) - .toArray(); - - expect(processedTxs.length).eq(3); - expect(processedTxs.filter(tx => tx.blockHeight === -5).length).eq(2); - expect(processedTxs.filter(tx => tx.blockHeight === -1).length).eq(1); // still in mempool - expect(processedCoins.length).eq(6); - expect(processedCoins.filter(coin => coin.mintHeight === -5).length).eq(4); - expect(processedCoins.filter(coin => coin.mintHeight === -1).length).eq(2); // still in mempool }); - it('should skip removing transactions on rpc error', async () => { - const rpcStub = sandbox.stub(RPC.prototype, 'getTransaction') - rpcStub.onCall(0).rejects({ code: -1, message: 'hahaha' }); - await insertOldTx(); - const { chain, network } = oldMempoolTx; - - const count = await TransactionStorage.collection.countDocuments({ - chain, - network, - blockHeight: -1, - blockTimeNormalized: { $lt: new Date() } + describe('processOldMempoolTxs', function() { + it('should remove old transactions', async () => { + sandbox.stub(RPC.prototype, 'getTransaction').resolves(null); + await insertOldTx(); + const { chain, network } = oldMempoolTx; + + const count = await TransactionStorage.collection.countDocuments({ + chain, + network, + blockHeight: -1, + blockTimeNormalized: { $lt: new Date() } + }); + expect(count).eq(1); + await Pruning.processOldMempoolTxs(chain, network, 29); + + const shouldBeExpiredTx = await TransactionStorage.collection + .find({ chain, network, txid: { $in: [oldMempoolTx.txid] } }) + .toArray(); + const shouldBeExpiredCoins = await CoinStorage.collection + .find({ chain, network, mintTxid: { $in: [oldMempoolTxOutput.mintTxid, oldMempoolTx2Output.mintTxid] } }) + .toArray(); + const parentTxOutputs = await CoinStorage.collection + .find({ chain, network, mintTxid: parentTxOutput1.mintTxid }) + .toArray(); + + expect(shouldBeExpiredTx.length).eq(1); + expect(shouldBeExpiredTx.every(tx => tx.blockHeight === SpentHeightIndicators.expired)).to.equal(true); + expect(shouldBeExpiredCoins.length).eq(2); + expect(shouldBeExpiredCoins.every(coin => coin.mintHeight === SpentHeightIndicators.expired)).to.equal(true); + expect(parentTxOutputs.length).eq(2); + expect(parentTxOutputs.filter(coin => coin.spentHeight === SpentHeightIndicators.unspent).length).to.equal(1); + }); + + it('should skip removing transactions still in mempool', async () => { + const rpcStub = sandbox.stub(RPC.prototype, 'getTransaction') + rpcStub.onCall(0).resolves(null); + rpcStub.onCall(1).resolves({}); + rpcStub.onCall(2).resolves(null); + await insertOldTx(0); + await insertOldTx(1); + await insertOldTx(2); + const { chain, network } = oldMempoolTx; + + const count = await TransactionStorage.collection.countDocuments({ + chain, + network, + blockHeight: SpentHeightIndicators.pending, + blockTimeNormalized: { $lt: new Date() } + }); + expect(count).eq(3); + await Pruning.processOldMempoolTxs(chain, network, 29); + + const processedTxs = await TransactionStorage.collection + .find({ chain, network, txid: { $in: [modTxid(oldMempoolTx.txid, 0), modTxid(oldMempoolTx.txid, 1), modTxid(oldMempoolTx.txid, 2)] } }) + .toArray(); + const processedCoins = await CoinStorage.collection + .find({ chain, network, mintTxid: { $in: [modTxid(oldMempoolTxOutput.mintTxid, 0), modTxid(oldMempoolTx2Output.mintTxid, 0), modTxid(oldMempoolTxOutput.mintTxid, 1), modTxid(oldMempoolTx2Output.mintTxid, 1), modTxid(oldMempoolTxOutput.mintTxid, 2), modTxid(oldMempoolTx2Output.mintTxid, 2)] } }) + .toArray(); + + expect(processedTxs.length).eq(3); + expect(processedTxs.filter(tx => tx.blockHeight === SpentHeightIndicators.expired).length).eq(2); + expect(processedTxs.filter(tx => tx.blockHeight === SpentHeightIndicators.pending).length).eq(1); // still in mempool + expect(processedCoins.length).eq(6); + expect(processedCoins.filter(coin => coin.mintHeight === SpentHeightIndicators.expired).length).eq(4); + expect(processedCoins.filter(coin => coin.mintHeight === SpentHeightIndicators.pending).length).eq(2); // still in mempool + }); + + it('should skip removing transactions on rpc error', async () => { + const rpcStub = sandbox.stub(RPC.prototype, 'getTransaction') + rpcStub.onCall(0).rejects({ code: -1, message: 'hahaha' }); + await insertOldTx(); + const { chain, network } = oldMempoolTx; + + const count = await TransactionStorage.collection.countDocuments({ + chain, + network, + blockHeight: SpentHeightIndicators.pending, + blockTimeNormalized: { $lt: new Date() } + }); + expect(count).eq(1); + await Pruning.processOldMempoolTxs(chain, network, 29); + + const shouldBeGoneTx = await TransactionStorage.collection + .find({ chain, network, txid: { $in: [oldMempoolTx.txid] } }) + .toArray(); + const shouldBeGoneCoins = await CoinStorage.collection + .find({ chain, network, mintTxid: { $in: [oldMempoolTxOutput.mintTxid, oldMempoolTx2Output.mintTxid] } }) + .toArray(); + + expect(shouldBeGoneTx.length).eq(1); + expect(shouldBeGoneCoins.length).eq(2); }); - expect(count).eq(1); - await Pruning.processOldMempoolTxs(chain, network, 29); - - const shouldBeGoneTx = await TransactionStorage.collection - .find({ chain, network, txid: { $in: [oldMempoolTx.txid] } }) - .toArray(); - const shouldBeGoneCoins = await CoinStorage.collection - .find({ chain, network, mintTxid: { $in: [oldMempoolTxOutput.mintTxid, oldMempoolTx2Output.mintTxid] } }) - .toArray(); - - expect(shouldBeGoneTx.length).eq(1); - expect(shouldBeGoneCoins.length).eq(2); - }); - it('should skip removing transactions if coin has >0 confs', async () => { - const rpcStub = sandbox.stub(RPC.prototype, 'getTransaction') - rpcStub.onCall(0).rejects({ code: -5, message: 'already exists' }); - const oldMempoolTx2OutputHeight = oldMempoolTx2Output.mintHeight; - oldMempoolTx2Output.mintHeight = 1; - await insertOldTx(); - oldMempoolTx2Output.mintHeight = oldMempoolTx2OutputHeight; // reset - const { chain, network } = oldMempoolTx; - - const count = await TransactionStorage.collection.countDocuments({ - chain, - network, - blockHeight: -1, - blockTimeNormalized: { $lt: new Date() } + it('should skip removing transactions if coin has >0 confs', async () => { + const rpcStub = sandbox.stub(RPC.prototype, 'getTransaction') + rpcStub.onCall(0).rejects({ code: -5, message: 'already exists' }); + const oldMempoolTx2OutputHeight = oldMempoolTx2Output.mintHeight; + oldMempoolTx2Output.mintHeight = 1; + await insertOldTx(); + oldMempoolTx2Output.mintHeight = oldMempoolTx2OutputHeight; // reset + const { chain, network } = oldMempoolTx; + + const count = await TransactionStorage.collection.countDocuments({ + chain, + network, + blockHeight: SpentHeightIndicators.pending, + blockTimeNormalized: { $lt: new Date() } + }); + expect(count).eq(1); + await Pruning.processOldMempoolTxs(chain, network, 29); + + const shouldBeGoneTx = await TransactionStorage.collection + .find({ chain, network, txid: { $in: [oldMempoolTx.txid] } }) + .toArray(); + const shouldBeGoneCoins = await CoinStorage.collection + .find({ chain, network, mintTxid: { $in: [oldMempoolTxOutput.mintTxid, oldMempoolTx2Output.mintTxid] } }) + .toArray(); + + expect(shouldBeGoneTx.length).eq(1); + expect(shouldBeGoneCoins.length).eq(2); }); - expect(count).eq(1); - await Pruning.processOldMempoolTxs(chain, network, 29); - - const shouldBeGoneTx = await TransactionStorage.collection - .find({ chain, network, txid: { $in: [oldMempoolTx.txid] } }) - .toArray(); - const shouldBeGoneCoins = await CoinStorage.collection - .find({ chain, network, mintTxid: { $in: [oldMempoolTxOutput.mintTxid, oldMempoolTx2Output.mintTxid] } }) - .toArray(); - - expect(shouldBeGoneTx.length).eq(1); - expect(shouldBeGoneCoins.length).eq(2); }); }); From 7e0c866ef9f6c6a04be702f55f95b69df055028d Mon Sep 17 00:00:00 2001 From: Kenny Joseph Date: Wed, 9 Oct 2024 16:57:31 -0400 Subject: [PATCH 5/6] prevent loop anywhere in the chain --- packages/bitcore-node/src/services/pruning.ts | 4 ++- .../services/pruning.integration.ts | 30 +++++++++++++++---- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/packages/bitcore-node/src/services/pruning.ts b/packages/bitcore-node/src/services/pruning.ts index 447870221bd..4a3e39a8c9e 100644 --- a/packages/bitcore-node/src/services/pruning.ts +++ b/packages/bitcore-node/src/services/pruning.ts @@ -286,11 +286,13 @@ export class PruningService { return false; } let rTx = await this.transactionModel.collection.findOne({ chain, network, txid: tx.replacedByTxid }); - while (rTx?.replacedByTxid && rTx?.blockHeight! < 0 && rTx?.txid !== tx.txid) { + let txids = [tx.txid]; + while (rTx?.replacedByTxid && rTx?.blockHeight! < 0 && !txids.includes(rTx?.txid)) { // replacement tx has also been replaced // Note: rTx.txid === tx.txid may happen if tx.replacedByTxid => rTx.txid and rTx.replacedByTxid => tx.txid. // This might happen if tx was rebroadcast _after_ being marked as replaced by rTx, thus marking rTx as replaced by tx. // Without this check, we could end up in an infinite loop where the two txs keep finding each other as unconfirmed replacements. + txids.push(rTx.txid); rTx = await this.transactionModel.collection.findOne({ chain, network, txid: rTx.replacedByTxid }); } // Re-org protection diff --git a/packages/bitcore-node/test/integration/services/pruning.integration.ts b/packages/bitcore-node/test/integration/services/pruning.integration.ts index 2ebdea73118..d14d87183fe 100644 --- a/packages/bitcore-node/test/integration/services/pruning.integration.ts +++ b/packages/bitcore-node/test/integration/services/pruning.integration.ts @@ -1,3 +1,4 @@ +import { ObjectId } from 'bson'; import { expect } from 'chai'; import sinon from 'sinon'; import { MongoBound } from '../../../src/models/base'; @@ -183,14 +184,25 @@ describe('Pruning Service', function() { it('should not go into an infinite loop if there is a circular replacedByTxid reference', async function() { await insertBadCoins(); - // invalidTx replaced by replacementTx, replacementTx replaced by invalidTx + console.log('point1'); + await TransactionStorage.collection.insertOne({ + ...replacementTx, + _id: new ObjectId(), + txid: 'replacementTx2', + blockHeight: SpentHeightIndicators.pending, + replacedByTxid: replacementTx.txid + }); + console.log('point2'); await TransactionStorage.collection.updateOne({ txid: replacementTx.txid }, { $set: { - replacedByTxid: invalidTx.txid + replacedByTxid: 'replacementTx2' } }); + console.log('point3'); + // at this point, invalidTx => replacementTx => replacementTx2 => replacementTx + const { chain, network } = invalidCoin; await Pruning.processAllInvalidTxs(chain, network); const shouldBeInvalid = await CoinStorage.collection @@ -202,16 +214,24 @@ describe('Pruning Service', function() { it('should not go into an infinite loop if there is a circular, unconfirmed replacedByTxid reference', async function() { await insertBadCoins(); - // invalidTx replaced by replacementTx, replacementTx replaced by invalidTx - // but replacementTx is still unconfirmed + await TransactionStorage.collection.insertOne({ + ...replacementTx, + _id: new ObjectId(), + txid: 'replacementTx2', + blockHeight: SpentHeightIndicators.pending, + replacedByTxid: replacementTx.txid + }); await TransactionStorage.collection.updateOne({ txid: replacementTx.txid }, { $set: { - replacedByTxid: invalidTx.txid, + replacedByTxid: 'replacementTx2', blockHeight: SpentHeightIndicators.pending } }); + // at this point, invalidTx => replacementTx => replacementTx2 => replacementTx + // but replacementTx is still unconfirmed + const { chain, network } = invalidCoin; await Pruning.processAllInvalidTxs(chain, network); const shouldBePendingStill = await CoinStorage.collection From aad859ecbefb3351fc1f44207994062e830420f3 Mon Sep 17 00:00:00 2001 From: Kenny Joseph Date: Thu, 10 Oct 2024 16:14:37 -0400 Subject: [PATCH 6/6] rm extraneous conole.logs --- .../test/integration/services/pruning.integration.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/bitcore-node/test/integration/services/pruning.integration.ts b/packages/bitcore-node/test/integration/services/pruning.integration.ts index d14d87183fe..05e1e13063c 100644 --- a/packages/bitcore-node/test/integration/services/pruning.integration.ts +++ b/packages/bitcore-node/test/integration/services/pruning.integration.ts @@ -184,7 +184,6 @@ describe('Pruning Service', function() { it('should not go into an infinite loop if there is a circular replacedByTxid reference', async function() { await insertBadCoins(); - console.log('point1'); await TransactionStorage.collection.insertOne({ ...replacementTx, _id: new ObjectId(), @@ -192,7 +191,6 @@ describe('Pruning Service', function() { blockHeight: SpentHeightIndicators.pending, replacedByTxid: replacementTx.txid }); - console.log('point2'); await TransactionStorage.collection.updateOne({ txid: replacementTx.txid }, { @@ -200,7 +198,6 @@ describe('Pruning Service', function() { replacedByTxid: 'replacementTx2' } }); - console.log('point3'); // at this point, invalidTx => replacementTx => replacementTx2 => replacementTx const { chain, network } = invalidCoin;