From bf194c920cf768d1339d41aef1441a78e2f5fcbe Mon Sep 17 00:00:00 2001 From: Pol Espinasa Date: Tue, 4 Mar 2025 12:11:12 +0100 Subject: [PATCH 1/2] wallet, rpc: deprecate settxfee and paytxfee --- src/wallet/init.cpp | 2 +- src/wallet/rpc/spend.cpp | 7 ++++++- src/wallet/wallet.cpp | 2 ++ test/functional/rpc_deprecated.py | 11 ++++++++++- test/functional/test_framework/test_framework.py | 4 ++-- test/functional/wallet_basic.py | 2 +- test/functional/wallet_bumpfee.py | 1 + test/functional/wallet_create_tx.py | 3 ++- test/functional/wallet_fundrawtransaction.py | 3 +++ test/functional/wallet_multiwallet.py | 2 +- test/functional/wallet_txn_clone.py | 3 +++ 11 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index a0374310552..34b7caa7e5f 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -66,7 +66,7 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const CURRENCY_UNIT, FormatMoney(DEFAULT_TRANSACTION_MAXFEE)), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-mintxfee=", strprintf("Fee rates (in %s/kvB) smaller than this are considered zero fee for transaction creation (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_TRANSACTION_MINFEE)), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); - argsman.AddArg("-paytxfee=", strprintf("Fee rate (in %s/kvB) to add to transactions you send (default: %s)", + argsman.AddArg("-paytxfee=", strprintf("(DEPRECATED) Fee rate (in %s/kvB) to add to transactions you send (default: %s)", CURRENCY_UNIT, FormatMoney(CFeeRate{DEFAULT_PAY_TX_FEE}.GetFeePerK())), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); #ifdef ENABLE_EXTERNAL_SIGNER argsman.AddArg("-signer=", "External signing tool, see doc/external-signer.md", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 0cba830e2ab..8e6747e3630 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -418,7 +418,7 @@ RPCHelpMan sendmany() RPCHelpMan settxfee() { return RPCHelpMan{"settxfee", - "\nSet the transaction fee rate in " + CURRENCY_UNIT + "/kvB for this wallet. Overrides the global -paytxfee command line parameter.\n" + "\n(DEPRECATED) Set the transaction fee rate in " + CURRENCY_UNIT + "/kvB for this wallet. Overrides the global -paytxfee command line parameter.\n" "Can be deactivated by passing 0 as the fee. In that case automatic fee selection will be used by default.\n", { {"amount", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "The transaction fee rate in " + CURRENCY_UNIT + "/kvB"}, @@ -437,6 +437,11 @@ RPCHelpMan settxfee() LOCK(pwallet->cs_wallet); + if (!pwallet->chain().rpcEnableDeprecated("settxfee")) { + throw JSONRPCError(RPC_METHOD_DEPRECATED, "settxfee is deprecated and will be fully removed in v31.0." + "\nTo use settxfee restart bitcoind with -deprecatedrpc=settxfee."); + } + CAmount nAmount = AmountFromValue(request.params[0]); CFeeRate tx_fee_rate(nAmount, 1000); CFeeRate max_tx_fee_rate(pwallet->m_default_max_tx_fee, 1000); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 09eda0c28e4..b7f5f787d95 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3181,6 +3181,8 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri } if (args.IsArgSet("-paytxfee")) { + warnings.push_back(_("-paytxfee is deprecated and will be fully removed in v31.0.")); + std::optional pay_tx_fee = ParseMoney(args.GetArg("-paytxfee", "")); if (!pay_tx_fee) { error = AmountErrMsg("paytxfee", args.GetArg("-paytxfee", "")); diff --git a/test/functional/rpc_deprecated.py b/test/functional/rpc_deprecated.py index f4036abdad6..4edf93a096f 100755 --- a/test/functional/rpc_deprecated.py +++ b/test/functional/rpc_deprecated.py @@ -4,13 +4,21 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test deprecation of RPC calls.""" from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_raises_rpc_error class DeprecatedRpcTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True self.extra_args = [[]] + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + def run_test(self): # This test should be used to verify the errors of the currently # deprecated RPC methods (without the -deprecatedrpc flag) until @@ -23,7 +31,8 @@ class DeprecatedRpcTest(BitcoinTestFramework): # at least one other functional test that still tests the RPCs # functionality using the respective -deprecatedrpc flag. - self.log.info("Currently no tests for deprecated RPC methods") + self.log.info("Test settxfee RPC") + assert_raises_rpc_error(-32, 'settxfee is deprecated and will be fully removed in v31.0.', self.nodes[0].rpc.settxfee, 0.01) if __name__ == '__main__': DeprecatedRpcTest(__file__).main() diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 7596fca7b9a..9cf822462f6 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -598,9 +598,9 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): # Wait for nodes to stop node.wait_until_stopped() - def restart_node(self, i, extra_args=None, clear_addrman=False): + def restart_node(self, i, extra_args=None, clear_addrman=False, *, expected_stderr=''): """Stop and start a test node""" - self.stop_node(i) + self.stop_node(i, expected_stderr=expected_stderr) if clear_addrman: peers_dat = self.nodes[i].chain_path / "peers.dat" os.remove(peers_dat) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index c968e4333a0..b9c975d3d27 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -35,7 +35,7 @@ class WalletTest(BitcoinTestFramework): # whitelist peers to speed up tx relay / mempool sync self.noban_tx_relay = True self.extra_args = [[ - "-dustrelayfee=0", "-walletrejectlongchains=0" + "-dustrelayfee=0", "-walletrejectlongchains=0", "-deprecatedrpc=settxfee" ]] * self.num_nodes self.setup_clean_chain = True self.supports_cli = False diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index 061e9f2caa1..01bbfca5b1b 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -61,6 +61,7 @@ class BumpFeeTest(BitcoinTestFramework): "-walletrbf={}".format(i), "-mintxfee=0.00002", "-addresstype=bech32", + "-deprecatedrpc=settxfee" ] for i in range(self.num_nodes)] def skip_test_if_missing_module(self): diff --git a/test/functional/wallet_create_tx.py b/test/functional/wallet_create_tx.py index 6deb262c9aa..85c6626a98a 100755 --- a/test/functional/wallet_create_tx.py +++ b/test/functional/wallet_create_tx.py @@ -23,6 +23,7 @@ class CreateTxWalletTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 + self.extra_args = [["-deprecatedrpc=settxfee"]] def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -71,7 +72,7 @@ class CreateTxWalletTest(BitcoinTestFramework): ) self.log.info('Check maxtxfee in combination with settxfee') - self.restart_node(0) + self.restart_node(0, expected_stderr='Warning: -paytxfee is deprecated and will be fully removed in v31.0.') self.nodes[0].settxfee(0.01) assert_raises_rpc_error( -6, diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index 827f27b431c..2674eec7dde 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -44,6 +44,9 @@ class RawTransactionsTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 4 + self.extra_args = [[ + "-deprecatedrpc=settxfee" + ] for i in range(self.num_nodes)] self.setup_clean_chain = True # whitelist peers to speed up tx relay / mempool sync self.noban_tx_relay = True diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index e241edd3fa6..16d11a786c8 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -46,7 +46,7 @@ class MultiWalletTest(BitcoinTestFramework): self.setup_clean_chain = True self.num_nodes = 2 self.rpc_timeout = 120 - self.extra_args = [["-nowallet"], []] + self.extra_args = [["-nowallet", "-deprecatedrpc=settxfee"], []] def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_txn_clone.py b/test/functional/wallet_txn_clone.py index a2dd223aed6..edd9bf66529 100755 --- a/test/functional/wallet_txn_clone.py +++ b/test/functional/wallet_txn_clone.py @@ -18,6 +18,9 @@ class TxnMallTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 3 self.supports_cli = False + self.extra_args = [[ + "-deprecatedrpc=settxfee" + ] for i in range(self.num_nodes)] def skip_test_if_missing_module(self): self.skip_if_no_wallet() From 2f2ab47bf74f4da37aad75a186cb0bb16e8af579 Mon Sep 17 00:00:00 2001 From: Pol Espinasa Date: Tue, 4 Mar 2025 12:10:52 +0100 Subject: [PATCH 2/2] Release notes --- doc/release-notes-31278.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 doc/release-notes-31278.md diff --git a/doc/release-notes-31278.md b/doc/release-notes-31278.md new file mode 100644 index 00000000000..1faa9577f55 --- /dev/null +++ b/doc/release-notes-31278.md @@ -0,0 +1,3 @@ +RPC and Startup Option +--- +The `-paytxfee` startup option and the `settxfee` RPC are now deprecated and will be removed in Bitcoin Core 31.0. They allowed the user to set a static fee rate for wallet transactions, which could potentially lead to overpaying or underpaying. Users should instead rely on fee estimation or specify a fee rate per transaction using the `fee_rate` argument in RPCs such as `fundrawtransaction`, `sendtoaddress`, `send`, `sendall`, and `sendmany`. (#31278) \ No newline at end of file