From fa16051eac9a4bc1a7e0234b65d615e655ff7cf0 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 25 Feb 2025 17:04:51 +0100 Subject: [PATCH] rpc: Allow fullrbf fee bump --- doc/release-notes-rbf-stuff.md | 9 +++++++++ src/qt/test/wallettests.cpp | 4 ++-- src/wallet/feebumper.cpp | 5 ----- src/wallet/rpc/spend.cpp | 11 ++++++----- test/functional/test_framework/messages.py | 1 + test/functional/wallet_bumpfee.py | 18 ++++++++++++++---- 6 files changed, 32 insertions(+), 16 deletions(-) create mode 100644 doc/release-notes-rbf-stuff.md diff --git a/doc/release-notes-rbf-stuff.md b/doc/release-notes-rbf-stuff.md new file mode 100644 index 00000000000..ab0c3ec50fc --- /dev/null +++ b/doc/release-notes-rbf-stuff.md @@ -0,0 +1,9 @@ +# RPC + +- The RPCs psbtbumpfee and bumpfee allow a replacement under fullrbf and no + longer require BIP-125 signalling. + +# GUI + +- A transaction's fee bump is allowed under fullrbf and no longer requires + BIP-125 signalling. diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 98dfe12f084..9adc6321767 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -300,8 +300,8 @@ void TestGUI(interfaces::Node& node, const std::shared_ptr& wallet) QVERIFY(FindTx(*transactionTableModel, txid1).isValid()); QVERIFY(FindTx(*transactionTableModel, txid2).isValid()); - // Call bumpfee. Test disabled, canceled, enabled, then failing cases. - BumpFee(transactionView, txid1, /*expectDisabled=*/true, /*expectError=*/"not BIP 125 replaceable", /*cancel=*/false); + // Call bumpfee. Test canceled fullrb bump, canceled bip-125-rbf bump, passing bump, and then failing bump. + BumpFee(transactionView, txid1, /*expectDisabled=*/false, /*expectError=*/{}, /*cancel=*/true); BumpFee(transactionView, txid2, /*expectDisabled=*/false, /*expectError=*/{}, /*cancel=*/true); BumpFee(transactionView, txid2, /*expectDisabled=*/false, /*expectError=*/{}, /*cancel=*/false); BumpFee(transactionView, txid2, /*expectDisabled=*/true, /*expectError=*/"already bumped", /*cancel=*/false); diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index b875461c9f2..f17d100e750 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -40,11 +40,6 @@ static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWallet return feebumper::Result::WALLET_ERROR; } - if (!SignalsOptInRBF(*wtx.tx)) { - errors.emplace_back(Untranslated("Transaction is not BIP 125 replaceable")); - return feebumper::Result::WALLET_ERROR; - } - if (wtx.mapValue.count("replaced_by_txid")) { errors.push_back(Untranslated(strprintf("Cannot bump transaction %s which was already bumped by transaction %s", wtx.GetHash().ToString(), wtx.mapValue.at("replaced_by_txid")))); return feebumper::Result::WALLET_ERROR; diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 0cba830e2ab..979be3e61c1 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -990,9 +990,9 @@ static RPCHelpMan bumpfee_helper(std::string method_name) const std::string incremental_fee{CFeeRate(DEFAULT_INCREMENTAL_RELAY_FEE).ToString(FeeEstimateMode::SAT_VB)}; return RPCHelpMan{method_name, - "\nBumps the fee of an opt-in-RBF transaction T, replacing it with a new transaction B.\n" + "Bumps the fee of a transaction T, replacing it with a new transaction B.\n" + std::string(want_psbt ? "Returns a PSBT instead of creating and signing a new transaction.\n" : "") + - "An opt-in RBF transaction with the given txid must be in the wallet.\n" + "A transaction with the given txid must be in the wallet.\n" "The command will pay the additional fee by reducing change outputs or adding inputs when necessary.\n" "It may add a new change output if one does not already exist.\n" "All inputs in the original transaction will be included in the replacement transaction.\n" @@ -1012,10 +1012,11 @@ static RPCHelpMan bumpfee_helper(std::string method_name) "\nSpecify a fee rate in " + CURRENCY_ATOM + "/vB instead of relying on the built-in fee estimator.\n" "Must be at least " + incremental_fee + " higher than the current transaction fee rate.\n" "WARNING: before version 0.21, fee_rate was in " + CURRENCY_UNIT + "/kvB. As of 0.21, fee_rate is in " + CURRENCY_ATOM + "/vB.\n"}, - {"replaceable", RPCArg::Type::BOOL, RPCArg::Default{true}, "Whether the new transaction should still be\n" + {"replaceable", RPCArg::Type::BOOL, RPCArg::Default{true}, + "Whether the new transaction should be\n" "marked bip-125 replaceable. If true, the sequence numbers in the transaction will\n" - "be left unchanged from the original. If false, any input sequence numbers in the\n" - "original transaction that were less than 0xfffffffe will be increased to 0xfffffffe\n" + "be set to 0xfffffffd. If false, any input sequence numbers in the\n" + "transaction will be set to 0xfffffffe\n" "so the new transaction will not be explicitly bip-125 replaceable (though it may\n" "still be replaceable in practice, for example if it has unconfirmed ancestors which\n" "are replaceable).\n"}, diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 9ebb683a9db..1ae9bbb8176 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -42,6 +42,7 @@ COIN = 100000000 # 1 btc in satoshis MAX_MONEY = 21000000 * COIN MAX_BIP125_RBF_SEQUENCE = 0xfffffffd # Sequence number that is rbf-opt-in (BIP 125) and csv-opt-out (BIP 68) +MAX_SEQUENCE_NONFINAL = 0xfffffffe # Sequence number that is csv-opt-out (BIP 68) SEQUENCE_FINAL = 0xffffffff # Sequence number that disables nLockTime if set for every input of a tx MAX_PROTOCOL_MESSAGE_LENGTH = 4000000 # Maximum length of incoming protocol messages diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index 061e9f2caa1..ca438491ee8 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -20,6 +20,7 @@ from test_framework.blocktools import ( ) from test_framework.messages import ( MAX_BIP125_RBF_SEQUENCE, + MAX_SEQUENCE_NONFINAL, ) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -374,7 +375,7 @@ def test_segwit_bumpfee_succeeds(self, rbf_node, dest_address): def test_nonrbf_bumpfee_fails(self, peer_node, dest_address): self.log.info('Test that we cannot replace a non RBF transaction') not_rbfid = peer_node.sendtoaddress(dest_address, Decimal("0.00090000")) - assert_raises_rpc_error(-4, "Transaction is not BIP 125 replaceable", peer_node.bumpfee, not_rbfid) + peer_node.bumpfee(not_rbfid) self.clear_mempool() @@ -677,11 +678,20 @@ def test_rebumping(self, rbf_node, dest_address): def test_rebumping_not_replaceable(self, rbf_node, dest_address): - self.log.info('Test that re-bumping non-replaceable fails') + self.log.info("Test that re-bumping non-replaceable passes") rbfid = spend_one_input(rbf_node, dest_address) + + def check_sequence(tx, seq_in): + tx = rbf_node.getrawtransaction(tx["txid"]) + tx = rbf_node.decoderawtransaction(tx) + seq = [i["sequence"] for i in tx["vin"]] + assert_equal(seq, [seq_in]) + bumped = rbf_node.bumpfee(rbfid, fee_rate=ECONOMICAL, replaceable=False) - assert_raises_rpc_error(-4, "Transaction is not BIP 125 replaceable", rbf_node.bumpfee, bumped["txid"], - {"fee_rate": NORMAL}) + check_sequence(bumped, MAX_SEQUENCE_NONFINAL) + bumped = rbf_node.bumpfee(bumped["txid"], {"fee_rate": NORMAL}) + check_sequence(bumped, MAX_BIP125_RBF_SEQUENCE) + self.clear_mempool()