Skip to content

Commit

Permalink
Fix gh-107: dust relay fees
Browse files Browse the repository at this point in the history
  • Loading branch information
@paulmillr
paulmillr committed Aug 13, 2024
1 parent d1bc93d commit b38a853
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 20 deletions.
50 changes: 31 additions & 19 deletions src/utxo.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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[],
Expand All @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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) {
Expand Down
116 changes: 115 additions & 1 deletion test/utxo-select.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit b38a853

Please sign in to comment.