Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wallet: add BIP125 RBF bump fee #1170

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
0a3ecf2
mempool: Allow double spend of RBF transaction
pinheadmz Jul 3, 2019
bb8b4b0
mempool: Reject RBF replacements with insufficient fees
pinheadmz Jul 5, 2019
d62f1b3
mempool: Reject RBF replacement edge cases
pinheadmz Jul 5, 2019
70ef286
mempool: replace by fee
pinheadmz Jul 5, 2019
f3cd659
mempool: clarify eviction of RBF conflicts
pinheadmz Oct 14, 2019
bf43d9a
mempool: improve readability and documentation of RBF logic
pinheadmz Aug 11, 2023
caabf10
mempool: allow replacement of tx spending unconfirmed utxo
pinheadmz Aug 19, 2023
5e0cd19
mempool: restore original isDoubleSpend() implementation
pinheadmz Aug 19, 2023
6f04c0d
mempool: fix RBF Rule #2
pinheadmz Aug 19, 2023
6ae1752
mempool: check input index when comparing replacement to conflict
pinheadmz Aug 21, 2023
24408b9
mempool: allow replacement of tx spending unconfirmed utxo
pinheadmz Aug 11, 2023
f316989
mempool: allow RBF by default
pinheadmz Aug 1, 2023
f83e39d
wallet: make all txs replaceable by default, optionally opt out
pinheadmz Aug 1, 2023
f99a7c2
test: add utility forEvent()
pinheadmz Aug 1, 2023
b8afe61
wallet: implement and test bumpTXFee()
pinheadmz Aug 3, 2023
e83563a
cli/http: add bwallet-cli bump <txid> <rate> <sign> <passphrase>
pinheadmz Aug 11, 2023
9a1135a
wallet: do not replace TX with more than one change address
pinheadmz Sep 21, 2023
d5d302c
wallet: do not accept too-low RBF replacement fee rate
pinheadmz Sep 21, 2023
4d97480
wallet rbf: add new in/out if necessary
pinheadmz Sep 21, 2023
5f472ad
wallet rbf: do not violate rule 6
pinheadmz Sep 22, 2023
14e5743
wallet rbf: add new in/out when change output is too low for RBF fee
pinheadmz Sep 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
wallet rbf: add new in/out if necessary
  • Loading branch information
pinheadmz committed Sep 22, 2023
commit 4d974807de28d208ee49e57df4943a8fda750f21
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 set the change value.
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
62 changes: 39 additions & 23 deletions lib/wallet/wallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -1143,7 +1143,8 @@
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,36 +1377,51 @@
}
}

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 more inputs (and maybe a change output) 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's fee (rule #4).
await this.fund(
mtx,
{
bumpFee: true, // set bump fee mode to ignore existing output values
rate, // rate in s/kvB for the rule 4 fee
depth: 1 // replacements can not add new unconfirmed coins
});
}

if (!sign)
return mtx.toTX();

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

View check run for this annotation

Codecov / codecov/patch

lib/wallet/wallet.js#L1419

Added line #L1419 was not covered by tests

await this.sign(mtx, passphrase);

if (!mtx.isSigned())
throw new Error('Replacement TX could not be fully signed.');

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

View check run for this annotation

Codecov / codecov/patch

lib/wallet/wallet.js#L1424

Added line #L1424 was not covered by tests

const ntx = mtx.toTX();

Expand All @@ -1420,7 +1436,7 @@
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() >= 2000 && 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]);
});
});
Loading