diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..c1e3f39 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,32 @@ +name: Tests + +on: + pull_request: + push: + branches: [master] + +permissions: + contents: read + +jobs: + test: + name: Unit tests and lint (Node ${{ matrix.node }}) + runs-on: ubuntu-latest + timeout-minutes: 15 + strategy: + fail-fast: false + matrix: + node: [24.22.1, 26.12.1] + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7 + with: + node-version: ${{ matrix.node }} + cache: npm + - run: npm ci + - run: npx --no-install standard + - run: npm run unit + env: + NODE_OPTIONS: --require=./test/network-fence.cjs diff --git a/package-lock.json b/package-lock.json index aab7fca..f33a94a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,9 +12,9 @@ "@bitcoinerlab/secp256k1": "^1.2.0", "axios": "^1.18.1", "bip32": "^5.0.1", - "bip39": "^3.1.0", + "bip39": "^3.1.2", "bitcoin-ops": "^1.4.1", - "bitcoinjs-lib": "^7.0.1", + "bitcoinjs-lib": "^7.0.5", "bn.js": "^5.2.4", "bs58": "^6.0.0", "ecpair": "^3.0.1", @@ -1069,22 +1069,22 @@ } }, "node_modules/bip174": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/bip174/-/bip174-3.0.0.tgz", - "integrity": "sha512-N3vz3rqikLEu0d6yQL8GTrSkpYb35NQKWMR7Hlza0lOj6ZOlvQ3Xr7N9Y+JPebaCVoEUHdBeBSuLxcHr71r+Lw==", + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/bip174/-/bip174-3.0.1.tgz", + "integrity": "sha512-avOnOoLOwdjw1MebWKh3bHqWOH558Q9VpqmRKUv2S63vjw0pnX7Y+yRihxIibgP55twhc7+YfC0IAH43IF5FqQ==", "license": "MIT", "dependencies": { - "uint8array-tools": "^0.0.9", - "varuint-bitcoin": "^2.0.0" + "uint8array-tools": "^0.0.10", + "varuint-bitcoin": "^2.0.1" }, "engines": { "node": ">=18.0.0" } }, "node_modules/bip174/node_modules/uint8array-tools": { - "version": "0.0.9", - "resolved": "https://registry.npmjs.org/uint8array-tools/-/uint8array-tools-0.0.9.tgz", - "integrity": "sha512-9vqDWmoSXOoi+K14zNaf6LBV51Q8MayF0/IiQs3GlygIKUYtog603e6virExkjjFosfJUBI4LhbQK1iq8IG11A==", + "version": "0.0.10", + "resolved": "https://registry.npmjs.org/uint8array-tools/-/uint8array-tools-0.0.10.tgz", + "integrity": "sha512-sSbrZqlr04w4p8KKpHw0ME/R4JzQaadum+BWhMlk2M7LrDyXWTfJOco3d1jNMdQNxgk5DDm88R3IWYH0nR+tzQ==", "license": "MIT", "engines": { "node": ">=14.0.0" @@ -1107,9 +1107,9 @@ } }, "node_modules/bip39": { - "version": "3.1.0", - "resolved": "https://registry.npmjs.org/bip39/-/bip39-3.1.0.tgz", - "integrity": "sha512-c9kiwdk45Do5GL0vJMe7tS95VjCii65mYAH7DfWl3uW8AVzXKQVUm64i3hzVybBDMp9r7j9iNxR85+ul8MdN/A==", + "version": "3.1.2", + "resolved": "https://registry.npmjs.org/bip39/-/bip39-3.1.2.tgz", + "integrity": "sha512-inxKLEH+Weos8f5rBYNKuOrpOIcNWtKnrLur9x72s4Cb8w4Mm0jJktUPkev7+1pQuivxFqkiut9+AcrwunLHxA==", "license": "ISC", "dependencies": { "@noble/hashes": "^1.2.0" @@ -1122,18 +1122,18 @@ "license": "MIT" }, "node_modules/bitcoinjs-lib": { - "version": "7.0.1", - "resolved": "https://registry.npmjs.org/bitcoinjs-lib/-/bitcoinjs-lib-7.0.1.tgz", - "integrity": "sha512-vwEmpL5Tpj0I0RBdNkcDMXePoaYSTeKY6mL6/l5esbnTs+jGdPDuLp4NY1hSh6Zk5wSgePygZ4Wx5JJao30Pww==", + "version": "7.0.5", + "resolved": "https://registry.npmjs.org/bitcoinjs-lib/-/bitcoinjs-lib-7.0.5.tgz", + "integrity": "sha512-xH24P1ile3QtyChzX1ITitcgpfZgDzG986gy1glbGXFsvcSnRX0P6p4m3nuTQfJ+j97va8py0leEK7xG8oxMqQ==", "license": "MIT", "dependencies": { "@noble/hashes": "^1.2.0", "bech32": "^2.0.0", - "bip174": "^3.0.0", + "bip174": "^3.0.1", "bs58check": "^4.0.0", - "uint8array-tools": "^0.0.9", + "uint8array-tools": "^0.0.10", "valibot": "^1.2.0", - "varuint-bitcoin": "^2.0.0" + "varuint-bitcoin": "^2.0.1" }, "engines": { "node": ">=18.0.0" @@ -1146,9 +1146,9 @@ "license": "MIT" }, "node_modules/bitcoinjs-lib/node_modules/uint8array-tools": { - "version": "0.0.9", - "resolved": "https://registry.npmjs.org/uint8array-tools/-/uint8array-tools-0.0.9.tgz", - "integrity": "sha512-9vqDWmoSXOoi+K14zNaf6LBV51Q8MayF0/IiQs3GlygIKUYtog603e6virExkjjFosfJUBI4LhbQK1iq8IG11A==", + "version": "0.0.10", + "resolved": "https://registry.npmjs.org/uint8array-tools/-/uint8array-tools-0.0.10.tgz", + "integrity": "sha512-sSbrZqlr04w4p8KKpHw0ME/R4JzQaadum+BWhMlk2M7LrDyXWTfJOco3d1jNMdQNxgk5DDm88R3IWYH0nR+tzQ==", "license": "MIT", "engines": { "node": ">=14.0.0" @@ -6846,12 +6846,21 @@ } }, "node_modules/varuint-bitcoin": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/varuint-bitcoin/-/varuint-bitcoin-2.0.0.tgz", - "integrity": "sha512-6QZbU/rHO2ZQYpWFDALCDSRsXbAs1VOEmXAxtbtjLtKuMJ/FQ8YbhfxlaiKv5nklci0M6lZtlZyxo9Q+qNnyog==", + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/varuint-bitcoin/-/varuint-bitcoin-2.0.1.tgz", + "integrity": "sha512-TTl5qSPJ3FzjuTHo0Ex9WUE3jTxnUsQHR38vIDhHApoVstzJK4gE20WqmTWOT5oo7ilvOSNejXt4CJrfDcqm5A==", "license": "MIT", "dependencies": { - "uint8array-tools": "^0.0.8" + "uint8array-tools": "^0.0.10" + } + }, + "node_modules/varuint-bitcoin/node_modules/uint8array-tools": { + "version": "0.0.10", + "resolved": "https://registry.npmjs.org/uint8array-tools/-/uint8array-tools-0.0.10.tgz", + "integrity": "sha512-sSbrZqlr04w4p8KKpHw0ME/R4JzQaadum+BWhMlk2M7LrDyXWTfJOco3d1jNMdQNxgk5DDm88R3IWYH0nR+tzQ==", + "license": "MIT", + "engines": { + "node": ">=14.0.0" } }, "node_modules/version-guard": { diff --git a/package.json b/package.json index 1a8a1ad..6c0569e 100644 --- a/package.json +++ b/package.json @@ -50,9 +50,9 @@ "@bitcoinerlab/secp256k1": "^1.2.0", "axios": "^1.18.1", "bip32": "^5.0.1", - "bip39": "^3.1.0", + "bip39": "^3.1.2", "bitcoin-ops": "^1.4.1", - "bitcoinjs-lib": "^7.0.1", + "bitcoinjs-lib": "^7.0.5", "bn.js": "^5.2.4", "bs58": "^6.0.0", "ecpair": "^3.0.1", diff --git a/test/network-fence.cjs b/test/network-fence.cjs new file mode 100644 index 0000000..356a02f --- /dev/null +++ b/test/network-fence.cjs @@ -0,0 +1,20 @@ +'use strict' + +// Tests use synthetic transaction data and must not access public networks. +const net = require('node:net') +const dns = require('node:dns') +const dgram = require('node:dgram') +const blocked = () => { throw new Error('Offline test network fence') } + +net.Socket.prototype.connect = blocked +dgram.Socket.prototype.send = blocked +for (const name of Object.keys(dns)) { + if (/^(lookup|resolve|reverse)/.test(name) && typeof dns[name] === 'function') { + dns[name] = blocked + } +} +for (const name of Object.keys(dns.promises)) { + if (/^(lookup|resolve|reverse)/.test(name) && typeof dns.promises[name] === 'function') { + dns.promises[name] = async () => blocked() + } +} diff --git a/test/psbt-fee-boundaries.test.js b/test/psbt-fee-boundaries.test.js new file mode 100644 index 0000000..a7a91e4 --- /dev/null +++ b/test/psbt-fee-boundaries.test.js @@ -0,0 +1,118 @@ +const tape = require('tape') +const syscoin = require('..') +const bitcoin = syscoin.utils.bitcoinjs +const { SYSCOIN_TX_VERSION_ALLOCATION_BURN_TO_SYSCOIN } = require('syscointx-js').utils + +// Fixed, synthetic transactions only; no private keys or network access. +const script = Buffer.from([0x51]) +const previous = new bitcoin.Transaction() +previous.addInput(Buffer.alloc(32, 7), 0) +previous.addOutput(script, 5000n) + +function makePsbt (options = {}) { + const input = { + hash: options.hash || previous.getId(), + index: options.index === undefined ? 0 : options.index + } + if (options.witness) { + input.witnessUtxo = { + script: options.script || script, + value: options.value === undefined ? 5000n : options.value + } + } + if (options.nonWitness) input.nonWitnessUtxo = previous.toBuffer() + if (options.finalized !== false) input.finalScriptSig = Buffer.from([0]) + const psbt = new bitcoin.Psbt() + if (options.version !== undefined) psbt.setVersion(options.version) + psbt.addInput(input) + psbt.addOutput({ script, value: options.output === undefined ? 4000n : options.output }) + return psbt +} + +function asImported (psbt) { + return syscoin.utils.importPsbtFromJson(syscoin.utils.exportPsbtToJson(psbt)).psbt +} + +tape('PSBT fee calculation accepts trustworthy previous-output metadata', (t) => { + for (const [description, options] of [ + ['witness', { witness: true }], + ['non-witness', { nonWitness: true }], + ['matching witness and non-witness', { witness: true, nonWitness: true }] + ]) { + for (const psbt of [makePsbt(options), asImported(makePsbt(options))]) { + t.equal(psbt.getFee(), 1000n, description + ' computes the exact fee') + t.ok(psbt.getFeeRate() >= 0, description + ' computes a fee rate') + t.doesNotThrow(() => psbt.extractTransaction(), description + ' extracts normally') + } + } + t.end() +}) + +tape('PSBT fee calculation rejects incomplete or inconsistent metadata', (t) => { + const cases = [ + ['missing UTXO', {}], + ['wrong previous transaction', { nonWitness: true, hash: Buffer.alloc(32, 8) }], + ['both forms with wrong previous transaction', { witness: true, nonWitness: true, hash: Buffer.alloc(32, 8) }], + ['missing previous output', { nonWitness: true, index: 1 }], + ['conflicting amounts', { witness: true, nonWitness: true, value: 4999n }], + ['conflicting scripts', { witness: true, nonWitness: true, script: Buffer.from([0x52]) }] + ] + for (const [description, options] of cases) { + for (const psbt of [makePsbt(options), asImported(makePsbt(options))]) { + t.throws(() => psbt.getFee(), description + ' cannot produce a fee') + t.throws(() => psbt.getFeeRate(), description + ' cannot produce a fee rate') + t.throws(() => psbt.extractTransaction(), description + ' cannot pass normal extraction') + t.throws(() => psbt.extractTransaction(true), description + ' cannot pass extraction with the fee limit disabled') + } + } + t.end() +}) + +tape('normal extraction retains the maximum fee-rate safeguard', (t) => { + const psbt = makePsbt({ witness: true }) + psbt.setMaximumFeeRate(1) + t.throws(() => psbt.extractTransaction(), /setMaximumFeeRate/, 'high fees are rejected by default') + t.doesNotThrow(() => psbt.extractTransaction(true), 'explicit fee-limit opt-out remains available') + t.end() +}) + +tape('PSBT fee calculations do not reuse stale metadata', (t) => { + const psbt = makePsbt({ witness: true, nonWitness: true }) + t.equal(psbt.getFee(), 1000n, 'matching metadata initially computes correctly') + t.ok(psbt.getFeeRate() > 0, 'initial fee rate is available') + psbt.data.inputs[0].witnessUtxo.value = 4999n + t.throws(() => psbt.getFee(), 'a later inconsistency invalidates the cached fee') + t.throws(() => psbt.getFeeRate(), 'a later inconsistency invalidates the cached rate') + t.throws(() => psbt.extractTransaction(), 'a cached rate cannot bypass extraction checks') + const nonWitness = makePsbt({ nonWitness: true }) + t.equal(nonWitness.getFee(), 1000n, 'non-witness metadata initially computes correctly') + const replacement = previous.clone() + replacement.outs[0].value = 4999n + nonWitness.data.inputs[0].nonWitnessUtxo = replacement.toBuffer() + t.throws(() => nonWitness.getFee(), 'replaced previous-transaction bytes are revalidated') + t.throws(() => nonWitness.extractTransaction(), 'replacement cannot reuse cached extraction') + t.end() +}) + +tape('unsigned preview neither requires UTXOs nor caches an unverified fee', (t) => { + const psbt = makePsbt({ finalized: false }) + const preview = psbt.extractTransaction(true, true) + t.equal(preview.outs[0].value, 4000n, 'preview exposes the transaction output') + psbt.updateInput(0, { + witnessUtxo: { script, value: 5000n }, + finalScriptSig: Buffer.from([0]) + }) + t.equal(psbt.getFee(), 1000n, 'later fee uses subsequently verified input data') + t.doesNotThrow(() => psbt.extractTransaction(), 'finalized transaction extracts normally') + const finalized = psbt.extractTransaction() + t.ok(finalized.ins[0].script.length > 0, 'normal extraction contains the finalized script') + t.equal(psbt.extractTransaction(true, true).ins[0].script.length, 0, 'preview remains unsigned after normal extraction') + t.end() +}) + +tape('Syscoin allocation burns retain negative native-fee semantics', (t) => { + const psbt = makePsbt({ witness: true, output: 6000n, version: SYSCOIN_TX_VERSION_ALLOCATION_BURN_TO_SYSCOIN }) + t.equal(psbt.getFee(), -1000n, 'native output can exceed input for an allocation burn') + t.doesNotThrow(() => psbt.extractTransaction(), 'allocation burn still extracts') + t.end() +}) diff --git a/utils.js b/utils.js index 8be67b9..df13d88 100644 --- a/utils.js +++ b/utils.js @@ -1792,41 +1792,8 @@ function scriptWitnessToWitnessStack (buffer) { return readVector() } -function addNonWitnessTxCache (cache, input, inputIndex) { - cache.__NON_WITNESS_UTXO_BUF_CACHE[inputIndex] = input.nonWitnessUtxo - const tx = bjs.Transaction.fromBuffer(input.nonWitnessUtxo) - cache.__NON_WITNESS_UTXO_TX_CACHE[inputIndex] = tx - const self = cache - const selfIndex = inputIndex - delete input.nonWitnessUtxo - Object.defineProperty(input, 'nonWitnessUtxo', { - enumerable: true, - get () { - const buf = self.__NON_WITNESS_UTXO_BUF_CACHE[selfIndex] - const txCache = self.__NON_WITNESS_UTXO_TX_CACHE[selfIndex] - if (buf !== undefined) { - return buf - } else { - const newBuf = txCache.toBuffer() - self.__NON_WITNESS_UTXO_BUF_CACHE[selfIndex] = newBuf - return newBuf - } - }, - set (data) { - self.__NON_WITNESS_UTXO_BUF_CACHE[selfIndex] = data - } - }) -} - -function nonWitnessUtxoTxFromCache (cache, input, inputIndex) { - const c = cache.__NON_WITNESS_UTXO_TX_CACHE - if (!c[inputIndex]) { - addNonWitnessTxCache(cache, input, inputIndex) - } - return c[inputIndex] -} - -// override of psbt.js inputFinalizeGetAmts without fee < 0 check +// Match bitcoinjs prevout validation without its fee < 0 restriction: +// Syscoin allocation burns can legitimately produce more native SYS than they consume. function inputFinalizeGetAmts (inputs, tx, cache, mustFinalize) { let inputAmount = 0n inputs.forEach((input, idx) => { @@ -1836,14 +1803,26 @@ function inputFinalizeGetAmts (inputs, tx, cache, mustFinalize) { input.finalScriptWitness ) } - if (input.witnessUtxo) { - inputAmount += input.witnessUtxo.value - } else if (input.nonWitnessUtxo) { - const nwTx = nonWitnessUtxoTxFromCache(cache, input, idx) - const vout = tx.ins[idx].index - const out = nwTx.outs[vout] - inputAmount += out.value + let prevout + if (input.nonWitnessUtxo) { + // Parse the current bytes each time so stale PSBT caches cannot hide edits. + const nwTx = bjs.Transaction.fromBuffer(input.nonWitnessUtxo) + const txInput = cache.__TX.ins[idx] + if (!Buffer.from(txInput.hash).equals(Buffer.from(nwTx.getHash()))) { + throw new Error(`Non-witness UTXO hash for input #${idx} does not match its prevout`) + } + prevout = nwTx.outs[txInput.index] + if (!prevout) throw new Error(`Non-witness UTXO for input #${idx} has no referenced output`) + if (input.witnessUtxo && + (input.witnessUtxo.value !== prevout.value || + !Buffer.from(input.witnessUtxo.script).equals(Buffer.from(prevout.script)))) { + throw new Error(`Witness UTXO for input #${idx} does not match its non-witness UTXO`) + } + } else { + prevout = input.witnessUtxo } + if (!prevout) throw new Error(`Need a UTXO input item for input #${idx}`) + inputAmount += prevout.value }) const outputAmount = tx.outs.reduce((total, o) => total + o.value, 0n) const fee = inputAmount - outputAmount @@ -1862,7 +1841,7 @@ function isFinalized (input) { } function checkFees (psbt, cache, opts) { - const feeRate = cache.__FEE_RATE || psbt.getFeeRate() + const feeRate = psbt.getFeeRate() const vsize = cache.__EXTRACTED_TX.virtualSize() const satoshis = feeRate * vsize if (feeRate >= opts.maximumFeeRate) { @@ -1878,17 +1857,9 @@ function checkFees (psbt, cache, opts) { function getTxCacheValue (key, name, inputs, c) { if (!inputs.every(isFinalized)) { throw new Error(`PSBT must be finalized to calculate ${name}`) } - if (key === '__FEE_RATE' && c.__FEE_RATE) return c.__FEE_RATE - if (key === '__FEE' && c.__FEE) return c.__FEE - let tx - let mustFinalize = true - if (c.__EXTRACTED_TX) { - tx = c.__EXTRACTED_TX - mustFinalize = false - } else { - tx = c.__TX.clone() - } - inputFinalizeGetAmts(inputs, tx, c, mustFinalize) + // Input metadata can change after a previous calculation; revalidate it. + const tx = c.__TX.clone() + inputFinalizeGetAmts(inputs, tx, c, true) if (key === '__FEE_RATE') return c.__FEE_RATE else if (key === '__FEE') return c.__FEE } @@ -1911,12 +1882,14 @@ class SPSBT extends bjs.Psbt { extractTransaction (disableFeeCheck, disableFinalize) { if (!disableFinalize && !this.data.inputs.every(isFinalized)) throw new Error('Not finalized') const c = this.__CACHE + // Unsigned display does not need UTXOs. Do not turn its incomplete data + // into a cached fee or a cached finalized transaction. + if (disableFeeCheck && disableFinalize) return c.__TX.clone() if (!disableFeeCheck) { checkFees(this, c, this.opts) + return c.__EXTRACTED_TX } - if (c.__EXTRACTED_TX) return c.__EXTRACTED_TX const tx = c.__TX.clone() - // When disableFinalize is true (unsigned preview), do not inject final scripts/witness inputFinalizeGetAmts(this.data.inputs, tx, c, !disableFinalize) return tx }