From b38a853261a2b556f1b5ac7a9b3c7cdff59ddd2b Mon Sep 17 00:00:00 2001 From: Paul Miller Date: Tue, 13 Aug 2024 16:20:34 +0000 Subject: [PATCH] Fix gh-107: dust relay fees --- src/utxo.ts | 50 ++++++++++------- test/utxo-select.test.js | 116 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 146 insertions(+), 20 deletions(-) diff --git a/src/utxo.ts b/src/utxo.ts index c23dc12..e711302 100644 --- a/src/utxo.ts +++ b/src/utxo.ts @@ -268,6 +268,7 @@ export type EstimatorOpts = TxOpts & { bip69?: boolean; // https://github.com/bitcoin/bips/blob/master/bip-0069.mediawiki network?: typeof NETWORK; dust?: number; // how much vbytes considered dust? + dustRelayFeeRate?: bigint; // fee per dust byte (DUST_RELAY_TX_FEE) createTx?: boolean; // Create tx inside selection requiredInputs?: psbt.TransactionInputUpdate[]; // these inputs always will be used allowSameUtxo?: boolean; // allow using UTXO multiple times (for test purposes) @@ -325,13 +326,10 @@ export class _Estimator { value: bigint; estimate: { weight: number; hasWitnesses: boolean }; }[]; - // https://github.com/bitcoin/bitcoin/blob/f90603ac6d24f5263649675d51233f1fce8b2ecd/src/policy/policy.cpp#L44 - // 32 + 4 + 1 + 107 + 4 // Dust used in accumExact + change address algo // - change address: can be smaller for segwit // - accumExact: ??? - private dust = 148n; // compat with coinselect - + private dust: bigint; // total dust limit (3||opts.dustRelayFeeRate * 182||opts.dust). Default: 546 constructor( inputs: psbt.TransactionInputUpdate[], private outputs: Output[], @@ -343,15 +341,34 @@ export class _Estimator { opts.feePerByte }, should be of type bigint but got ${typeof opts.feePerByte}.` ); - if (opts.dust) { - if (typeof opts.dust !== 'bigint') - throw new Error( - `Estimator: wrong dust=${ - opts.dust - }, should be of type bigint but got ${typeof opts.dust}.` - ); - this.dust = opts.dust; + // Dust stuff + // TODO: think about this more: + // - current dust filters tx which cannot be relayed by core + // - but actual dust meaning is 'can be this amount spent?' + // - dust contains full tx size. but we can use other inputs to pay for outputDust (and parially inputsDust)? + // - not sure if we can spent anything with feePerByte: 3. It will be relayed, but will it be mined? + // - for now it works exactly as bitcoin-core. But will create change/outputs which cannot be spent (reasonable). + // Number of bytes needed to create and spend a UTXO. + // https://github.com/bitcoin/bitcoin/blob/27a770b34b8f1dbb84760f442edb3e23a0c2420b/src/policy/policy.cpp#L28-L41 + const inputsDust = 32 + 4 + 1 + 107 + 4; // NOTE: can be smaller for segwit tx? + const outputDust = 34; // NOTE: 'nSize = GetSerializeSize(txout)' + const dustBytes = opts.dust === undefined ? BigInt(inputsDust + outputDust) : opts.dust; + if (typeof dustBytes !== 'bigint') { + throw new Error( + `Estimator: wrong dust=${opts.dust}, should be of type bigint but got ${typeof opts.dust}.` + ); } + // 3 sat/vb is the default minimum fee rate used to calculate dust thresholds by bitcoin core. + // 3000 sat/kvb -> 3 sat/vb. + // https://github.com/bitcoin/bitcoin/blob/27a770b34b8f1dbb84760f442edb3e23a0c2420b/src/policy/policy.h#L55 + const dustFee = opts.dustRelayFeeRate === undefined ? 3n : opts.dustRelayFeeRate; + if (typeof dustFee !== 'bigint') { + throw new Error( + `Estimator: wrong dustRelayFeeRate=${opts.dustRelayFeeRate}, should be of type bigint but got ${typeof opts.dustRelayFeeRate}.` + ); + } + // Dust uses feePerbyte by default, but we allow separate dust fee if needed + this.dust = dustBytes * dustFee; if (opts.requiredInputs !== undefined && !Array.isArray(opts.requiredInputs)) throw new Error(`Estimator: wrong required inputs=${opts.requiredInputs}`); const network = opts.network || NETWORK; @@ -446,7 +463,6 @@ export class _Estimator { // exact(biggest) will select one big utxo which is closer to targetValue+dust, if possible. // If not, it will accumulate largest utxo until value is close to targetValue+dust. accumulate(indices: number[], exact = false, skipNegative = true, all = false) { - const { feePerByte } = this.opts; // TODO: how to handle change addresses? // - cost of input // - cost of change output (if input requires change) @@ -486,11 +502,7 @@ export class _Estimator { const totalWeight = newWeight + 4 * CompactSizeLen.encode(num).length; // number of outputs can change weight fee = this.getSatoshi(totalWeight); // Best case scenario exact(biggest) -> we find biggest output, less than target+threshold - if (exact) { - const dust = this.dust * feePerByte; - // skip if added value is bigger than dust - if (amount + inputsAmount > targetAmount + fee + dust) continue; - } + if (exact && amount + inputsAmount > targetAmount + fee + this.dust) continue; // skip if added value is bigger than dust // Negative: cost of using input is more than value provided (negative) // By default 'blackjack' mode in coinselect doesn't use that, which means // it will use negative output if sorted by 'smallest' @@ -562,7 +574,7 @@ export class _Estimator { const changeFee = this.getSatoshi(changeWeight); let fee = s.fee; const change = total - this.amount - changeFee; - if (change > this.dust * this.opts.feePerByte) needChange = true; + if (change > this.dust) needChange = true; let inputs = indices; let outputs = Array.from(this.outputs); if (needChange) { diff --git a/test/utxo-select.test.js b/test/utxo-select.test.js index 9e28e9f..b051f60 100644 --- a/test/utxo-select.test.js +++ b/test/utxo-select.test.js @@ -666,6 +666,7 @@ describe('UTXO Select', () => { const FEE = 1n; const est = new btc._Estimator(inputs, [{ ...OUTPUTS[0], amount: 100n }], { feePerByte: FEE, + dustRelayFeeRate: FEE, changeAddress: '2MshuFeVGhXVdRv77UcJYvRBi2JyTNwgSR2', network: regtest, allowLegacyWitnessUtxo: true, @@ -819,6 +820,7 @@ describe('UTXO Select', () => { const t2 = (strategy, amount) => { const est = new btc._Estimator(inputs, [{ ...OUTPUTS[0], amount }], { feePerByte: FEE, + dustRelayFeeRate: FEE, changeAddress: '2MshuFeVGhXVdRv77UcJYvRBi2JyTNwgSR2', network: regtest, allowLegacyWitnessUtxo: true, @@ -972,7 +974,7 @@ describe('UTXO Select', () => { }); const t3 = (strategy) => { - const FEE = 0n // no fee to test exact amounts + const FEE = 0n; // no fee to test exact amounts const est = new btc._Estimator(inputs, [{ ...OUTPUTS[0], amount: inputsTotalAmount }], { feePerByte: FEE, changeAddress: '2MshuFeVGhXVdRv77UcJYvRBi2JyTNwgSR2', @@ -1351,6 +1353,118 @@ describe('UTXO Select', () => { }, ]); }); + // Dust (GH-107) + should('drop dust outputs', () => { + const privKey = hex.decode('0101010101010101010101010101010101010101010101010101010101010101'); + const pubKey = secp256k1.getPublicKey(privKey, true); + const spend = btc.p2wpkh(pubKey, regtest); + const txid = hex.decode('0af50a00a22f74ece24c12cd667c290d3a35d48124a69f4082700589172a3aa2'); + const utxos = [ + { + ...spend, + txid, + index: 0, + // total fee will be 22200. 22400 - 22200 = 200 is less than dust and so shouldn't be added as change. + // Default dust is 182 * 3 -> 546 + witnessUtxo: { script: spend.script, amount: 22_400n }, + }, + { + ...spend, + txid, + index: 1, + witnessUtxo: { script: spend.script, amount: 200_000n }, + }, + ]; + const outputs = [{ address: '2MvpbAgedBzJUBZWesDwdM7p3FEkBEwq3n3', amount: 200_000n }]; + const selected = btc.selectUTXO(utxos, outputs, 'default', { + changeAddress: 'bcrt1pea3850rzre54e53eh7suwmrwc66un6nmu9npd7eqrhd6g4lh8uqsxcxln8', + feePerByte: 100n, + dust: 200n, + dustRelayFeeRate: 1n, + bip69: true, + createTx: true, + network: regtest, + }); + deepStrictEqual(selected.inputs, [ + { + txid, + index: 0, + witnessUtxo: { script: spend.script, amount: 22_400n }, + sequence: 4294967295, + }, + { + txid, + index: 1, + witnessUtxo: { script: spend.script, amount: 200_000n }, + sequence: 4294967295, + }, + ]); + // No change output added + deepStrictEqual(selected.outputs, [ + { address: '2MvpbAgedBzJUBZWesDwdM7p3FEkBEwq3n3', amount: 200_000n }, + ]); + // Setting dust to 199 correctly includes input as change + const selected2 = btc.selectUTXO(utxos, outputs, 'default', { + changeAddress: 'bcrt1pea3850rzre54e53eh7suwmrwc66un6nmu9npd7eqrhd6g4lh8uqsxcxln8', + feePerByte: 100n, + dust: 66n, // 200/3 + bip69: true, + createTx: true, + network: regtest, + }); + deepStrictEqual(selected2.outputs, [ + { address: 'bcrt1pea3850rzre54e53eh7suwmrwc66un6nmu9npd7eqrhd6g4lh8uqsxcxln8', amount: 200n }, + { address: '2MvpbAgedBzJUBZWesDwdM7p3FEkBEwq3n3', amount: 200_000n }, + ]); + }); + should('add change outputs if more than dust', () => { + const privKey = hex.decode('0101010101010101010101010101010101010101010101010101010101010101'); + const pubKey = secp256k1.getPublicKey(privKey, true); + const spend = btc.p2wpkh(pubKey, regtest); + const txid = hex.decode('0af50a00a22f74ece24c12cd667c290d3a35d48124a69f4082700589172a3aa2'); + const utxos = [ + { + ...spend, + txid, + index: 0, + // total fee will be 22200. 23000 - 22200 = 800 is more than dust and so should be added as change. + witnessUtxo: { script: spend.script, amount: 23_000n }, + }, + { + ...spend, + txid, + index: 1, + witnessUtxo: { script: spend.script, amount: 200_000n }, + }, + ]; + const outputs = [{ address: '2MvpbAgedBzJUBZWesDwdM7p3FEkBEwq3n3', amount: 200_000n }]; + const selected = btc.selectUTXO(utxos, outputs, 'default', { + changeAddress: 'bcrt1pea3850rzre54e53eh7suwmrwc66un6nmu9npd7eqrhd6g4lh8uqsxcxln8', + feePerByte: 100n, + bip69: true, + createTx: true, + network: regtest, + }); + deepStrictEqual(selected.inputs, [ + { + txid, + index: 0, + witnessUtxo: { script: spend.script, amount: 23_000n }, + sequence: 4294967295, + }, + { + txid, + index: 1, + witnessUtxo: { script: spend.script, amount: 200_000n }, + sequence: 4294967295, + }, + ]); + // Change output should have been added + deepStrictEqual(selected.outputs, [ + { address: 'bcrt1pea3850rzre54e53eh7suwmrwc66un6nmu9npd7eqrhd6g4lh8uqsxcxln8', amount: 800n }, + { address: '2MvpbAgedBzJUBZWesDwdM7p3FEkBEwq3n3', amount: 200_000n }, + ]); + }); }); // ESM is broken.