Skip to content

Commit

Permalink
wallet rbf: add new in/out if necessary
Browse files Browse the repository at this point in the history
  • Loading branch information
pinheadmz committed Sep 21, 2023
1 parent d5d302c commit 368195a
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 25 deletions.
16 changes: 15 additions & 1 deletion lib/primitives/mtx.js
Original file line number Diff line number Diff line change
Expand Up @@ -1227,11 +1227,25 @@ class MTX extends TX {
async fund(coins, options) {
assert(options, 'Options are required.');
assert(options.changeAddress, 'Change address is required.');
assert(this.inputs.length === 0, 'TX is already funded.');
if (!options.bumpFee)
assert(this.inputs.length === 0, 'TX is already funded.');

// Select necessary coins.
const select = await this.selectCoins(coins, options);

// Bump Fee mode:
// We want the coin selector to ignore the values of
// all existing inputs and outputs, but still consider their size.
// Inside the coin selector this is done by making a copy of the TX
// and then setting the total output value to 0
// (input value is already ignored).
// The coin selector will add coins to cover the fee on the entire TX
// including paying for any inputs it adds.
// Now that coin selection is done, we add back in the fee paid by
// the original existing inputs and outputs so we can create change.
if (options.bumpFee)
select.fee += this.getFee();

// Add coins to transaction.
for (const coin of select.chosen)
this.addCoin(coin);
Expand Down
8 changes: 7 additions & 1 deletion lib/wallet/coinselector.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ class CoinSelector {
this.changeAddress = null;
this.inputs = new BufferMap();
this.useSelectEstimate = false;
this.bumpFee = false;

// Needed for size estimation.
this.getAccount = null;
Expand Down Expand Up @@ -158,6 +159,11 @@ class CoinSelector {
this.useSelectEstimate = options.useSelectEstimate;
}

if (options.bumpFee != null) {
assert(typeof options.bumpFee === 'boolean');
this.bumpFee = options.bumpFee;
}

if (options.changeAddress) {
const addr = options.changeAddress;
if (typeof addr === 'string') {
Expand Down Expand Up @@ -209,7 +215,7 @@ class CoinSelector {

async init(coins) {
this.coins = coins.slice();
this.outputValue = this.tx.getOutputValue();
this.outputValue = this.bumpFee ? 0 : this.tx.getOutputValue();
this.chosen = [];
this.change = 0;
this.fee = CoinSelector.MIN_FEE;
Expand Down
55 changes: 32 additions & 23 deletions lib/wallet/wallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -1143,7 +1143,8 @@ class Wallet extends EventEmitter {
rate: rate,
maxFee: options.maxFee,
useSelectEstimate: options.useSelectEstimate,
getAccount: this.getAccountByAddress.bind(this)
getAccount: this.getAccountByAddress.bind(this),
bumpFee: options.bumpFee
});

assert(mtx.getFee() <= CoinSelector.MAX_FEE, 'TX exceeds MAX_FEE.');
Expand Down Expand Up @@ -1376,27 +1377,35 @@ class Wallet extends EventEmitter {
}
}

if (!change)
throw new Error('Transaction has no change output.');

// Start by reducing absolute fee to 0
change.value += oldFee;
if (mtx.getFee() !== 0)
throw new Error('Arithmetic error for change.');

// Pay for replacement fees: BIP 125 rule #3
change.value -= oldFee;

// Pay for our own current fee: BIP 125 rule #4
change.value -= currentFee;

if (change.value < 0)
throw new Error('Change output insufficient for fee.');

// If change output is below dust, give it all to fee
if (change.isDust()) {
mtx.outputs.splice(mtx.changeIndex, 1);
mtx.changeIndex = -1;
if (change) {
// Start by reducing absolute fee to 0
change.value += oldFee;
if (mtx.getFee() !== 0)
throw new Error('Arithmetic error for change.');

Check warning on line 1384 in lib/wallet/wallet.js

View check run for this annotation

Codecov / codecov/patch

lib/wallet/wallet.js#L1384

Added line #L1384 was not covered by tests

// Pay for replacement fees: BIP 125 rule #3
change.value -= oldFee;

// Pay for our own current fee: BIP 125 rule #4
change.value -= currentFee;

// We need to add more inputs.
// This will obviously also fail the dust test next
// and conveniently remove the change output for us.
if (change.value < 0)
throw new Error('Change value insufficient to bump fee.');

Check warning on line 1396 in lib/wallet/wallet.js

View check run for this annotation

Codecov / codecov/patch

lib/wallet/wallet.js#L1396

Added line #L1396 was not covered by tests

// If change output is below dust,
// give it all to fee (remove change output)
if (change.isDust()) {
mtx.outputs.splice(mtx.changeIndex, 1);
mtx.changeIndex = -1;

Check warning on line 1402 in lib/wallet/wallet.js

View check run for this annotation

Codecov / codecov/patch

lib/wallet/wallet.js#L1401-L1402

Added lines #L1401 - L1402 were not covered by tests
}
} else {
// We need to add another input + change to increase the fee
// Since the original inputs and outputs already paid for
// their own fee (rule #3) all we have to do is pay for this new TX fee.
await this.fund(mtx, {rate, bumpFee: true});
}

if (!sign)
Expand All @@ -1420,7 +1429,7 @@ class Wallet extends EventEmitter {
await this.wdb.send(ntx);
}

return ntx;
return mtx;
}

/**
Expand Down
31 changes: 31 additions & 0 deletions test/wallet-rbf-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,4 +186,35 @@ describe('Wallet RBF', function () {
});
await node.rpc.generateToAddress([1, aliceReceive]);
});

it('should bump a tx with no change by adding new in/out pair', async () => {
const coins = await alice.getCoins();
let coin;
for (coin of coins) {
if (!coin.coinbase)
break;
}
const mtx = new MTX();
mtx.addCoin(coin);
mtx.addOutput(bobReceive, coin.value - 200);
mtx.inputs[0].sequence = 0xfffffffd;
await alice.sign(mtx);
const tx = mtx.toTX();
assert.strictEqual(tx.inputs.length, 1);
assert.strictEqual(tx.outputs.length, 1);
await alice.wdb.addTX(tx);
await alice.wdb.send(tx);
await forEvent(node.mempool, 'tx');

const rtx = await alice.bumpTXFee(tx.hash(), 2000 /* satoshis per kvB */, true, null);
assert.strictEqual(rtx.inputs.length, 2);
assert.strictEqual(rtx.outputs.length, 2);
assert(rtx.getRate() > 1999 && rtx.getRate() < 3000);

await forEvent(node.mempool, 'tx');
assert(!node.mempool.hasEntry(tx.hash()));
assert(node.mempool.hasEntry(rtx.hash()));

await node.rpc.generateToAddress([1, aliceReceive]);
});
});

0 comments on commit 368195a

Please sign in to comment.