diff --git a/packages/dash-spv/lib/merkleproofs.js b/packages/dash-spv/lib/merkleproofs.js index dca09952c35..1bba2c774e5 100644 --- a/packages/dash-spv/lib/merkleproofs.js +++ b/packages/dash-spv/lib/merkleproofs.js @@ -13,7 +13,7 @@ const merkleproofs = { if (typeof transactions[0] === 'string') { txToFilter = txToFilter.map((tx) => DashUtil.toHash(tx).toString('hex')); } - return merkleBlock.validMerkleTree + return merkleBlock.validMerkleTree() && txToFilter.filter((tx) => merkleBlock.hasTransaction(tx)).length === transactions.length; }, }; diff --git a/packages/js-dash-sdk/src/test/mocks/createAndAttachTransportMocksToClient.ts b/packages/js-dash-sdk/src/test/mocks/createAndAttachTransportMocksToClient.ts index 85d46ada220..29391bbc784 100644 --- a/packages/js-dash-sdk/src/test/mocks/createAndAttachTransportMocksToClient.ts +++ b/packages/js-dash-sdk/src/test/mocks/createAndAttachTransportMocksToClient.ts @@ -101,7 +101,12 @@ export async function createAndAttachTransportMocksToClient(client, sinon) { txStreamMock.emit(TxStreamMock.EVENTS.end); // Wait for account to resolve - await accountPromise; + const account = await accountPromise; + + // The stream emits synthetic InstantLocks. Opt this test harness into the + // wallet's trusted-verifier seam; production remains fail-closed when no + // verifier backed by quorum history is configured. + account.plugins.workers.transactionssyncworker.verifyInstantLock = async () => true; // Putting data in transport stubs transportMock.getIdentityByPublicKeyHash diff --git a/packages/wallet-lib/src/plugins/Workers/TransactionsSyncWorker/TransactionsSyncWorker.js b/packages/wallet-lib/src/plugins/Workers/TransactionsSyncWorker/TransactionsSyncWorker.js index df00b61c994..8904e8c5de1 100644 --- a/packages/wallet-lib/src/plugins/Workers/TransactionsSyncWorker/TransactionsSyncWorker.js +++ b/packages/wallet-lib/src/plugins/Workers/TransactionsSyncWorker/TransactionsSyncWorker.js @@ -3,7 +3,7 @@ const ReconnectableStream = require('@dashevo/dapi-client/lib/transport/Reconnec const Worker = require('../../Worker'); const logger = require('../../../logger'); const TransactionsReader = require('./TransactionsReader'); -const { getAddressesToSync, getTxHashesFromMerkleBlock } = require('./utils'); +const { getAddressesToSync } = require('./utils'); const EVENTS = require('../../../EVENTS'); const STATES = { @@ -65,6 +65,10 @@ class TransactionsSyncWorker extends Worker { this.blockHeightChangedHandler = null; + // InstantLocks are remote assertions. Keep them out of trusted wallet state + // unless a caller supplies a verifier backed by trusted quorum history. + this.verifyInstantLock = options.verifyInstantLock || (async () => false); + this.historicalTransactionsHandler = this.historicalTransactionsHandler.bind(this); this.historicalMerkleBlockHandler = this.historicalMerkleBlockHandler.bind(this); this.newTransactionsHandler = this.newTransactionsHandler.bind(this); @@ -427,19 +431,22 @@ class TransactionsSyncWorker extends Worker { } if (headerTime <= 0 || Number.isNaN(headerTime)) { - rejectMerkleBlock(rejectMerkleBlock(Error(`Invalid header time: ${headerTime}`))); + rejectMerkleBlock(Error(`Invalid header time: ${headerTime}`)); + return; + } + + try { + if (!merkleBlock.validMerkleTree()) { + rejectMerkleBlock(new Error(`Invalid merkle proof for block ${headerHash}`)); + return; + } + } catch (e) { + rejectMerkleBlock(e); return; } let addressesGenerated = []; if (this.historicalTransactionsToVerify.size) { - const txHashesInTheBlock = merkleBlock - .hashes.reduce((set, hashHex) => { - const hash = Buffer.from(hashHex, 'hex').reverse(); - set.add(hash.toString('hex')); - return set; - }, new Set()); - const metadata = { blockHash: headerHash, height: headerHeight, @@ -452,21 +459,21 @@ class TransactionsSyncWorker extends Worker { try { this.historicalTransactionsToVerify.forEach((tx) => { - if (!txHashesInTheBlock.has(tx.hash)) { + if (!merkleBlock.hasTransaction(tx)) { throw new Error(`Transaction ${tx.hash} was not found in merkle block ${headerHash}`); } transactionsWithMetadata.push([tx, metadata]); - this.historicalTransactionsToVerify.delete(tx.hash); }); } catch (e) { rejectMerkleBlock(e); return; } - // TODO(spv): verify transactions against the merkle block - if (transactionsWithMetadata.length) { ({ addressesGenerated } = this.importTransactions(transactionsWithMetadata)); + transactionsWithMetadata.forEach(([tx]) => { + this.historicalTransactionsToVerify.delete(tx.hash); + }); } } @@ -565,7 +572,7 @@ class TransactionsSyncWorker extends Worker { } if (headerMetadata.time <= 0 || Number.isNaN(headerMetadata.time)) { - rejectMerkleBlock(rejectMerkleBlock(Error(`Invalid header time: ${headerMetadata.time}`))); + rejectMerkleBlock(Error(`Invalid header time: ${headerMetadata.time}`)); return; } @@ -577,12 +584,18 @@ class TransactionsSyncWorker extends Worker { time: headerTime, }); + try { + if (!merkleBlock.validMerkleTree()) { + rejectMerkleBlock(new Error(`Invalid merkle proof for block ${headerHash}`)); + return; + } + } catch (e) { + rejectMerkleBlock(e); + return; + } + let $transactionsFound = 0; if (this.historicalTransactionsToVerify.size) { - const txHashesInTheBlock = getTxHashesFromMerkleBlock(merkleBlock); - - // TODO: verify merkle block in SPV - const metadata = { blockHash: headerHash, height: headerHeight, @@ -595,15 +608,15 @@ class TransactionsSyncWorker extends Worker { // Traverse through all transactions to verify and re-import ones having metadata this.historicalTransactionsToVerify.forEach((tx) => { - if (txHashesInTheBlock.has(tx.hash)) { + if (merkleBlock.hasTransaction(tx)) { transactionsWithMetadata.push([tx, metadata]); - this.historicalTransactionsToVerify.delete(tx.hash); } }); if (transactionsWithMetadata.length) { this.importTransactions(transactionsWithMetadata); transactionsWithMetadata.forEach(([tx]) => { + this.historicalTransactionsToVerify.delete(tx.hash); this.parentEvents.emit(EVENTS.CONFIRMED_TRANSACTION, tx); }); $transactionsFound = transactionsWithMetadata.length; @@ -631,11 +644,20 @@ class TransactionsSyncWorker extends Worker { * Processing is locks during the continuous sync * @param {InstantLock[]} instantLocks */ - instantLocksHandler(instantLocks) { - // TODO: perform IS locks verification - instantLocks.forEach((instantLock) => { - this.importInstantLock(instantLock); - }); + async instantLocksHandler(instantLocks) { + for (const instantLock of instantLocks) { + try { + const isVerified = await this.verifyInstantLock(instantLock); + if (!isVerified) { + this.logger.warn('Rejected unverified InstantLock', { txid: instantLock.txid }); + continue; + } + + this.importInstantLock(instantLock); + } catch (e) { + this.emitError(e); + } + } } /** diff --git a/packages/wallet-lib/src/plugins/Workers/TransactionsSyncWorker/TransactionsSyncWorker.spec.js b/packages/wallet-lib/src/plugins/Workers/TransactionsSyncWorker/TransactionsSyncWorker.spec.js index aacd3b137db..10bbc120708 100644 --- a/packages/wallet-lib/src/plugins/Workers/TransactionsSyncWorker/TransactionsSyncWorker.spec.js +++ b/packages/wallet-lib/src/plugins/Workers/TransactionsSyncWorker/TransactionsSyncWorker.spec.js @@ -41,6 +41,7 @@ describe('TransactionsSyncWorker', () => { sinon.spy(worker, 'scheduleProgressUpdate'); worker.importTransactions = sinon.stub().returns([]); + worker.importInstantLock = sinon.stub(); const transactionsReader = new EventEmitter(); transactionsReader.startHistoricalSync = sinon.spy(); @@ -781,6 +782,34 @@ describe('TransactionsSyncWorker', () => { expect(storage.scheduleStateSave).to.have.not.been.called(); expect(transactionsSyncWorker.scheduleProgressUpdate).to.have.not.been.called(); }); + + it('should reject an invalid merkle tree without mutating pending transactions', function test() { + const transaction = new Transaction().to(ADDRESSES_KEYCHAIN_1[0], 1000); + transactionsSyncWorker.historicalTransactionsToVerify.set(transaction.hash, transaction); + + const merkleBlock = mockMerkleBlock([transaction.hash]); + merkleBlock.header.merkleRoot = Buffer.alloc(32, 1); + const merkleBlockHeight = 500; + chainStore.state.headersMetadata.set(merkleBlock.header.hash, { + height: merkleBlockHeight, + time: merkleBlock.header.time, + }); + const dataEventPayload = { + merkleBlock, + acceptMerkleBlock: this.sinon.spy(), + rejectMerkleBlock: this.sinon.spy(), + }; + + transactionsSyncWorker.historicalMerkleBlockHandler(dataEventPayload); + + expect(dataEventPayload.rejectMerkleBlock).to.have.been.calledOnce(); + expect(dataEventPayload.acceptMerkleBlock).to.have.not.been.called(); + expect(transactionsSyncWorker.importTransactions).to.have.not.been.called(); + expect(transactionsSyncWorker.historicalTransactionsToVerify.has(transaction.hash)) + .to.equal(true); + expect(chainStore.updateLastSyncedBlockHeight).to.have.not.been.called(); + expect(storage.scheduleStateSave).to.have.not.been.called(); + }); }); }); @@ -1085,6 +1114,70 @@ describe('TransactionsSyncWorker', () => { }); // TODO: should reject in case merkle block verification failed + it('should reject an invalid merkle tree before confirming a transaction', function test() { + const transaction = new Transaction().to(ADDRESSES_KEYCHAIN_1[0], 1000); + transactionsSyncWorker.historicalTransactionsToVerify.set(transaction.hash, transaction); + + const merkleBlock = mockMerkleBlock([transaction.hash]); + merkleBlock.header.merkleRoot = Buffer.alloc(32, 1); + chainStore.state.headersMetadata.set(merkleBlock.header.hash, { + height: 500, + time: merkleBlock.header.time, + }); + const dataEventPayload = { + merkleBlock, + acceptMerkleBlock: this.sinon.spy(), + rejectMerkleBlock: this.sinon.spy(), + }; + + transactionsSyncWorker.newMerkleBlockHandler(dataEventPayload); + + expect(dataEventPayload.rejectMerkleBlock).to.have.been.calledOnce(); + expect(dataEventPayload.acceptMerkleBlock).to.have.not.been.called(); + expect(transactionsSyncWorker.importTransactions).to.have.not.been.called(); + expect(transactionsSyncWorker.historicalTransactionsToVerify.has(transaction.hash)) + .to.equal(true); + expect(chainStore.updateLastSyncedBlockHeight).to.have.not.been.called(); + expect(storage.scheduleStateSave).to.have.not.been.called(); + }); + }); + }); + + describe('#instantLocksHandler', () => { + beforeEach(function beforeEach() { + transactionsSyncWorker = createTransactionsSyncWorker(this.sinon); + }); + + it('should reject locks when no trusted verifier is configured', async () => { + await transactionsSyncWorker.instantLocksHandler([{ txid: 'unverified' }]); + + expect(transactionsSyncWorker.importInstantLock).to.have.not.been.called(); + }); + + it('should import only locks accepted by the trusted verifier', async function test() { + const verifiedLock = { txid: 'verified' }; + const rejectedLock = { txid: 'rejected' }; + transactionsSyncWorker.verifyInstantLock = this.sinon.stub(); + transactionsSyncWorker.verifyInstantLock.withArgs(verifiedLock).resolves(true); + transactionsSyncWorker.verifyInstantLock.withArgs(rejectedLock).resolves(false); + + await transactionsSyncWorker.instantLocksHandler([verifiedLock, rejectedLock]); + + expect(transactionsSyncWorker.verifyInstantLock).to.have.been.calledTwice(); + expect(transactionsSyncWorker.importInstantLock) + .to.have.been.calledOnceWith(verifiedLock); + }); + + it('should contain verifier errors without importing the lock', async function test() { + const verificationError = new Error('quorum state unavailable'); + transactionsSyncWorker.verifyInstantLock = this.sinon.stub().rejects(verificationError); + transactionsSyncWorker.parentEvents.on('error', () => {}); + + await transactionsSyncWorker.instantLocksHandler([{ txid: 'unverified' }]); + + expect(transactionsSyncWorker.importInstantLock).to.have.not.been.called(); + expect(transactionsSyncWorker.parentEvents.emit) + .to.have.been.calledWith('error', verificationError); }); }); }); diff --git a/packages/wallet-lib/src/test/mocks/dashcore/block.js b/packages/wallet-lib/src/test/mocks/dashcore/block.js index f2becaf7970..7b90448794d 100644 --- a/packages/wallet-lib/src/test/mocks/dashcore/block.js +++ b/packages/wallet-lib/src/test/mocks/dashcore/block.js @@ -71,15 +71,25 @@ const mockHeadersChain = (network, length, root) => { }; const mockMerkleBlock = (txHashes, prevHeader, network = 'livenet') => { - const header = prevHeader ? mockHeader(prevHeader, network) : getRoot(network); - - return new MerkleBlock({ - header, - numTransactions: txHashes.length, - hashes: txHashes - .map((hash) => Buffer.from(hash, 'hex').reverse().toString('hex')), - flags: [], - }); + const sourceHeader = prevHeader ? mockHeader(prevHeader, network) : getRoot(network); + const header = new BlockHeader(sourceHeader.toBuffer()); + const transactionHashes = txHashes.length > 0 + ? txHashes.map((hash) => Buffer.from(hash, 'hex').reverse()) + : [Buffer.alloc(32)]; + const filterMatches = txHashes.length > 0 + ? txHashes.map(() => true) + : [false]; + const merkleBlock = MerkleBlock.build(header, transactionHashes, filterMatches); + + /* eslint-disable no-underscore-dangle */ + header.merkleRoot = merkleBlock._traverseMerkleTree( + merkleBlock._calcTreeHeight(), + 0, + { hashesUsed: 0, flagBitsUsed: 0 }, + ); + /* eslint-enable no-underscore-dangle */ + + return merkleBlock; }; module.exports = { diff --git a/packages/wallet-lib/tests/integration/plugins/Workers/TransactionsSyncWorker.spec.js b/packages/wallet-lib/tests/integration/plugins/Workers/TransactionsSyncWorker.spec.js index aab8022ccec..1cf9934d35c 100644 --- a/packages/wallet-lib/tests/integration/plugins/Workers/TransactionsSyncWorker.spec.js +++ b/packages/wallet-lib/tests/integration/plugins/Workers/TransactionsSyncWorker.spec.js @@ -31,6 +31,10 @@ describe('TransactionsSyncWorker', () => { // Create worker const worker = new TransactionsSyncWorker({ executeOnStart: false, + // This integration fixture supplies synthetic locks, so explicitly + // provide the trusted-verifier seam instead of relying on production's + // fail-closed default. + verifyInstantLock: async () => true, }); // Integrate worker with wallet @@ -253,8 +257,9 @@ describe('TransactionsSyncWorker', () => { }); // TODO: also implement confirmation with instant locks - it('should send instant locks', () => { + it('should send instant locks', async () => { const sentInstantLocks = sendNewInstantLocks({ forTransactions: transactions }); + await waitOneTick(); const chainStore = wallet.storage.getDefaultChainStore(); expect(Array.from(chainStore.state.instantLocks.values()))