diff --git a/doc/release-notes-34917.md b/doc/release-notes-34917.md new file mode 100644 index 00000000000..c723b9338c0 --- /dev/null +++ b/doc/release-notes-34917.md @@ -0,0 +1,9 @@ +RPC and Startup Option +------------ +The `bip125-replaceable` key in the wallet transaction RPCs such +as `listtransactions`, `listsinceblock`, and `gettransaction` is +marked as deprecated. Users still have the option to retrieve this +key by passing the `-deprecatedrpc=bip125` startup option. Also, +the `-walletrbf` startup option has been marked as deprecated and +will be fully removed in the next release. Using this options emits +a warning in the logs. diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index ccd82e7a7d7..74e433a35bd 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -74,7 +74,7 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const #if HAVE_SYSTEM argsman.AddArg("-walletnotify=", "Execute command when a wallet transaction changes. %s in cmd is replaced by TxID, %w is replaced by wallet name, %b is replaced by the hash of the block including the transaction (set to 'unconfirmed' if the transaction is not included) and %h is replaced by the block height (-1 if not included). %w is not currently implemented on windows. On systems where %w is supported, it should NOT be quoted because this would break shell escaping used to invoke the command.", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); #endif - argsman.AddArg("-walletrbf", strprintf("Send transactions with full-RBF opt-in enabled (RPC only, default: %u)", DEFAULT_WALLET_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); + argsman.AddArg("-walletrbf", strprintf("(DEPRECATED) Send transactions with full-RBF opt-in enabled (default: %u)", DEFAULT_WALLET_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-unsafesqlitesync", "Set SQLite synchronous=OFF to disable waiting for the database to sync to disk. This is unsafe and can cause data loss and corruption. This option is only used by tests to improve their performance (default: false)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index 038e30fceb1..f69082e1e96 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -50,15 +50,17 @@ static void WalletTxToJSON(const CWallet& wallet, const CWalletTx& wtx, UniValue entry.pushKV("timereceived", wtx.nTimeReceived); // Add opt-in RBF status - std::string rbfStatus = "no"; - if (confirms <= 0) { - RBFTransactionState rbfState = chain.isRBFOptIn(*wtx.tx); - if (rbfState == RBFTransactionState::UNKNOWN) - rbfStatus = "unknown"; - else if (rbfState == RBFTransactionState::REPLACEABLE_BIP125) - rbfStatus = "yes"; + if (chain.rpcEnableDeprecated("bip125")) { + std::string rbfStatus = "no"; + if (confirms <= 0) { + RBFTransactionState rbfState = chain.isRBFOptIn(*wtx.tx); + if (rbfState == RBFTransactionState::UNKNOWN) + rbfStatus = "unknown"; + else if (rbfState == RBFTransactionState::REPLACEABLE_BIP125) + rbfStatus = "yes"; + } + entry.pushKV("bip125-replaceable", rbfStatus); } - entry.pushKV("bip125-replaceable", rbfStatus); for (const std::pair& item : wtx.mapValue) entry.pushKV(item.first, item.second); @@ -405,7 +407,7 @@ static std::vector TransactionDescriptionString() {RPCResult::Type::NUM_TIME, "time", "The transaction time expressed in " + UNIX_EPOCH_TIME + "."}, {RPCResult::Type::NUM_TIME, "timereceived", "The time received expressed in " + UNIX_EPOCH_TIME + "."}, {RPCResult::Type::STR, "comment", /*optional=*/true, "If a comment is associated with the transaction, only present if not empty."}, - {RPCResult::Type::STR, "bip125-replaceable", "(\"yes|no|unknown\") Whether this transaction signals BIP125 replaceability or has an unconfirmed ancestor signaling BIP125 replaceability.\n" + {RPCResult::Type::STR, "bip125-replaceable", /*optional=*/true, "(\"yes|no|unknown\") (DEPRECATED) Whether this transaction signals BIP125 replaceability or has an unconfirmed ancestor signaling BIP125 replaceability.\n" "May be unknown for unconfirmed transactions not in the mempool because their unconfirmed ancestors are unknown."}, {RPCResult::Type::ARR, "parent_descs", /*optional=*/true, "Only if 'category' is 'received'. List of parent descriptors for the output script of this coin.", { {RPCResult::Type::STR, "desc", "The descriptor string."}, diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 32e59be5c25..77d0a395c93 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3103,7 +3103,11 @@ bool CWallet::LoadWalletArgs(std::shared_ptr wallet, const WalletContex wallet->m_confirm_target = args.GetIntArg("-txconfirmtarget", DEFAULT_TX_CONFIRM_TARGET); wallet->m_spend_zero_conf_change = args.GetBoolArg("-spendzeroconfchange", DEFAULT_SPEND_ZEROCONF_CHANGE); - wallet->m_signal_rbf = args.GetBoolArg("-walletrbf", DEFAULT_WALLET_RBF); + wallet->m_signal_rbf = DEFAULT_WALLET_RBF; + if (args.IsArgSet("-walletrbf")) { + warnings.push_back(_("-walletrbf is deprecated and will be fully removed in the next release.")); + wallet->m_signal_rbf = args.GetBoolArg("-walletrbf").value(); + } wallet->m_keypool_size = std::max(args.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE), int64_t{1}); wallet->m_notify_tx_changed_script = args.GetArg("-walletnotify", ""); diff --git a/test/functional/test_framework/wallet_util.py b/test/functional/test_framework/wallet_util.py index 12251530786..18095a3b14a 100755 --- a/test/functional/test_framework/wallet_util.py +++ b/test/functional/test_framework/wallet_util.py @@ -24,6 +24,8 @@ from test_framework.script_util import ( script_to_p2sh_script, ) +WALLETRBF_DEPRECATION_WARNING = "Warning: -walletrbf is deprecated and will be fully removed in the next release." + Key = namedtuple('Key', ['privkey', 'pubkey', 'p2pkh_script', diff --git a/test/functional/wallet_backwards_compatibility.py b/test/functional/wallet_backwards_compatibility.py index f00f2958d12..b2a74893306 100755 --- a/test/functional/wallet_backwards_compatibility.py +++ b/test/functional/wallet_backwards_compatibility.py @@ -28,6 +28,7 @@ from test_framework.util import ( assert_greater_than, assert_raises_rpc_error, ) +from test_framework.wallet_util import WALLETRBF_DEPRECATION_WARNING LAST_KEYPOOL_INDEX = 9 # Index of the last derived address with the keypool size of 10 @@ -349,9 +350,7 @@ class BackwardsCompatibilityTest(BitcoinTestFramework): # Restore the wallet to master load_res = node_master.restorewallet(wallet_name, backup_path) - - # There should be no warnings - assert "warnings" not in load_res + assert_equal(load_res["warnings"], [WALLETRBF_DEPRECATION_WARNING[9:]]) wallet = node_master.get_wallet_rpc(wallet_name) info = wallet.getaddressinfo(address) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index e7a333baf10..11f27ed7dbf 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -510,7 +510,7 @@ class WalletTest(BitcoinTestFramework): # Try with walletrejectlongchains # Double chain limit but require combining inputs, so we pass AttemptSelection self.stop_node(0) - extra_args = ["-walletrejectlongchains", "-limitclustercount=" + str(2 * chainlimit), "-limitancestorcount=" + str(2*chainlimit)] + extra_args = ["-deprecatedrpc=bip125", "-walletrejectlongchains", "-limitclustercount=" + str(2 * chainlimit), "-limitancestorcount=" + str(2*chainlimit)] self.start_node(0, extra_args=extra_args) # wait until the wallet has submitted all transactions to the mempool diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index 961f1a1494f..992f63ad423 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -32,6 +32,7 @@ from test_framework.util import ( find_vout_for_address, ) from test_framework.wallet import MiniWallet +from test_framework.wallet_util import WALLETRBF_DEPRECATION_WARNING WALLET_PASSPHRASE = "test" @@ -116,6 +117,8 @@ class BumpFeeTest(BitcoinTestFramework): test_feerate_checks_replaced_outputs(self, rbf_node, peer_node) test_bumpfee_with_feerate_ignores_walletincrementalrelayfee(self, rbf_node, peer_node) + self.restart_node(1, [], expected_stderr=WALLETRBF_DEPRECATION_WARNING) + def test_invalid_parameters(self, rbf_node, peer_node, dest_address): self.log.info('Test invalid parameters') rbfid = spend_one_input(rbf_node, dest_address) @@ -460,7 +463,7 @@ def test_bumpfee_with_abandoned_descendant_succeeds(self, rbf_node, rbf_node_add assert bumped_result['txid'] in rbf_node.getrawmempool() assert parent_id not in rbf_node.getrawmempool() # Cleanup - self.restart_node(1, self.extra_args[1]) + self.restart_node(1, self.extra_args[1], expected_stderr=WALLETRBF_DEPRECATION_WARNING) rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT) self.connect_nodes(1, 0) self.clear_mempool() @@ -535,11 +538,11 @@ def test_maxtxfee_fails(self, rbf_node, dest_address): # size of bumped transaction (p2wpkh, 1 input, 2 outputs): 141 vbytes # expected bump fee of 141 vbytes * 0.00200000 BTC / 1000 vbytes = 0.00002820 BTC # which exceeds maxtxfee and is expected to raise - self.restart_node(1, ['-maxtxfee=0.000025'] + self.extra_args[1]) + self.restart_node(1, ['-maxtxfee=0.000025'] + self.extra_args[1], expected_stderr=WALLETRBF_DEPRECATION_WARNING) rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT) rbfid = spend_one_input(rbf_node, dest_address) assert_raises_rpc_error(-4, "Unable to create transaction. Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)", rbf_node.bumpfee, rbfid) - self.restart_node(1, self.extra_args[1]) + self.restart_node(1, self.extra_args[1], expected_stderr=WALLETRBF_DEPRECATION_WARNING) rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT) self.connect_nodes(1, 0) self.clear_mempool() diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py index 714e6594657..d2bdd9b4295 100755 --- a/test/functional/wallet_listtransactions.py +++ b/test/functional/wallet_listtransactions.py @@ -23,7 +23,7 @@ from test_framework.util import ( assert_raises_rpc_error, find_vout_for_address, ) -from test_framework.wallet_util import get_generate_key +from test_framework.wallet_util import get_generate_key, WALLETRBF_DEPRECATION_WARNING class ListTransactionsTest(BitcoinTestFramework): @@ -31,7 +31,7 @@ class ListTransactionsTest(BitcoinTestFramework): self.num_nodes = 3 # whitelist peers to speed up tx relay / mempool sync self.noban_tx_relay = True - self.extra_args = [["-walletrbf=0"]] * self.num_nodes + self.extra_args = [["-walletrbf=0", "-deprecatedrpc=bip125"]] * self.num_nodes def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -104,6 +104,9 @@ class ListTransactionsTest(BitcoinTestFramework): self.test_op_return() self.test_from_me_status_change() + for index, _ in enumerate(self.nodes): + self.stop_node(index, expected_stderr=WALLETRBF_DEPRECATION_WARNING) + def run_rbf_opt_in_test(self): """Test the opt-in-rbf flag for sent and received transactions.""" diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index e28cfd578c5..42b2a47b3b0 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -44,7 +44,7 @@ class WalletMigrationTest(BitcoinTestFramework): self.setup_clean_chain = True self.num_nodes = 2 self.supports_cli = False - self.extra_args = [[], ["-deprecatedrpc=create_bdb"]] + self.extra_args = [["-deprecatedrpc=bip125"], ["-deprecatedrpc=create_bdb"]] def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index ba98699039d..5011deeab14 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -31,10 +31,7 @@ class WalletSendTest(BitcoinTestFramework): # whitelist peers to speed up tx relay / mempool sync self.noban_tx_relay = True self.supports_cli = False - self.extra_args = [ - ["-walletrbf=1", "-datacarriersize=16"], - ["-walletrbf=1", "-datacarriersize=16"] - ] + self.extra_args = [["-walletrbf=1", "-datacarriersize=16", "-deprecatedrpc=bip125"]] * self.num_nodes getcontext().prec = 8 # Satoshi precision for Decimal def skip_test_if_missing_module(self):