diff --git a/doc/release-notes-removeprunedfunds.md b/doc/release-notes-removeprunedfunds.md new file mode 100644 index 00000000000..0e0d0ceb3c9 --- /dev/null +++ b/doc/release-notes-removeprunedfunds.md @@ -0,0 +1,6 @@ +Updated RPCs +------------ + +- The `removeprunedfunds` RPC has been deprecated and will be removed in the +next major release. In order to continue using it, `bitcoind` must be started +with the `-deprecatedrpc=removeprunedfunds` option. diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index cd998d5074b..70bc3bde9ae 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -96,6 +96,7 @@ RPCMethod removeprunedfunds() { return RPCMethod{ "removeprunedfunds", + "(DEPRECATED) This feature will be removed in the next major release. Start bitcoind with the `-deprecatedrpc=removeprunedfunds` option in order to use this.\n" "Deletes the specified transaction from the wallet. Meant for use with pruned wallets and as a companion to importprunedfunds. This will affect wallet balances.\n", { {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex-encoded id of the transaction you are deleting"}, @@ -111,6 +112,10 @@ RPCMethod removeprunedfunds() std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return UniValue::VNULL; + if (!pwallet->chain().rpcEnableDeprecated("removeprunedfunds")) { + throw JSONRPCError(RPC_METHOD_DEPRECATED, "DEPRECATION WARNING: This feature will be removed in the next major release. Start bitcoind with the `-deprecatedrpc=removeprunedfunds` option in order to use this."); + } + LOCK(pwallet->cs_wallet); Txid hash{Txid::FromUint256(ParseHashV(request.params[0], "txid"))}; diff --git a/test/functional/rpc_deprecated.py b/test/functional/rpc_deprecated.py index 2f86954f22b..dd7f6002474 100755 --- a/test/functional/rpc_deprecated.py +++ b/test/functional/rpc_deprecated.py @@ -4,6 +4,8 @@ # 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 set_test_params(self): @@ -26,7 +28,17 @@ class DeprecatedRpcTest(BitcoinTestFramework): # Please don't delete nor modify this comment self.log.info("Tests for deprecated RPC methods (if any)") - self.log.info("Currently no tests for deprecated RPC methods") + if self.is_wallet_compiled(): + self.log.info("Tests for deprecated wallet-related RPC methods (if any)") + self.nodes[0].createwallet("ancient_wallet") + wallet = self.nodes[0].get_wallet_rpc("ancient_wallet") + + self.log.info("Test removeprunedfunds deprecation") + assert_raises_rpc_error( + -32, "Start bitcoind with the `-deprecatedrpc=removeprunedfunds`", + wallet.removeprunedfunds, + "fakeargument" + ) if __name__ == '__main__': diff --git a/test/functional/wallet_importprunedfunds.py b/test/functional/wallet_importprunedfunds.py index 95e2a5b3a4f..6c1c640645f 100755 --- a/test/functional/wallet_importprunedfunds.py +++ b/test/functional/wallet_importprunedfunds.py @@ -25,6 +25,7 @@ class ImportPrunedFundsTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 + self.extra_args = [["-deprecatedrpc=removeprunedfunds"]] * 2 def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_resendwallettransactions.py b/test/functional/wallet_resendwallettransactions.py index 1b15fa17e25..b72c79d3620 100755 --- a/test/functional/wallet_resendwallettransactions.py +++ b/test/functional/wallet_resendwallettransactions.py @@ -5,8 +5,6 @@ """Test that the wallet resends transactions periodically.""" import time -from decimal import Decimal - from test_framework.blocktools import ( create_block, ) @@ -16,8 +14,6 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_raises_rpc_error, - get_fee, - try_rpc, ) # 36 hours is the upper limit of the resend timer, see CWallet::SetNextResend() @@ -83,72 +79,47 @@ class ResendWalletTransactionsTest(BitcoinTestFramework): peer_second.wait_for_broadcast([txid]) self.log.info("Chain of unconfirmed not-in-mempool txs are rebroadcast") - # This tests that the node broadcasts the parent transaction before the child transaction. - # To test that scenario, we need a method to reliably get a child transaction placed - # in mapWallet positioned before the parent. We cannot predict the position in mapWallet, - # but we can observe it using listreceivedbyaddress and other related RPCs. - # - # So we will create the child transaction, use listreceivedbyaddress to see what the - # ordering of mapWallet is, if the child is not before the parent, we will create a new - # child (via bumpfee) and remove the old child (via removeprunedfunds) until we get the - # ordering of child before parent. - child_inputs = [{"txid": txid, "vout": 0}] - child_txid = node.sendall(recipients=[addr], inputs=child_inputs)["txid"] - # Get the child tx's info for manual bumping - child_tx_info = node.gettransaction(txid=child_txid, verbose=True) - child_output_value = child_tx_info["decoded"]["vout"][0]["value"] - # Include an additional 1 vbyte buffer to handle when we have a smaller signature - additional_child_fee = get_fee(child_tx_info["decoded"]["vsize"] + 1, Decimal(0.00001100)) - while True: - txids = node.listreceivedbyaddress(minconf=0, address_filter=addr)[0]["txids"] - if txids == [child_txid, txid]: - break - # Manually bump the tx - # The inputs and the output address stay the same, just changing the amount for the new fee - child_output_value -= additional_child_fee - bumped_raw = node.createrawtransaction(inputs=child_inputs, outputs=[{addr: child_output_value}]) - bumped = node.signrawtransactionwithwallet(bumped_raw) - bumped_txid = node.decoderawtransaction(bumped["hex"])["txid"] - # Sometimes we will get a signature that is a little bit shorter than we expect which causes the - # feerate to be a bit higher, then the followup to be a bit lower. This results in a replacement - # that can't be broadcast. We can just skip that and keep grinding. - if try_rpc(-26, "insufficient fee, rejecting replacement", node.sendrawtransaction, bumped["hex"]): - continue - # The scheduler queue creates a copy of the added tx after - # send/bumpfee and re-adds it to the wallet (undoing the next - # removeprunedfunds). So empty the scheduler queue: + # We cannot predict the ordering in mapWallet of parent and child, so + # try a few times to get both. + evict_time = 0 + for _ in range(10): + child_inputs = [{"txid": txid, "vout": 0}] + child_txid = node.sendall(recipients=[addr], inputs=child_inputs)["txid"] + # Get the child tx's info for manual bumping + entry_time = node.getmempoolentry(child_txid)["time"] + + # tx must be at least 5 minutes older than the last block to be rebroadcast + block_time = entry_time + 5 * 60 + 1 + node.setmocktime(block_time) + block = create_block(int(node.getbestblockhash(), 16), height=node.getblockcount() + 1, ntime=block_time) + block.solve() + node.submitblock(block.serialize().hex()) + # Set correct m_best_block_time, which is used in ResubmitWalletTransactions node.syncwithvalidationinterfacequeue() - node.removeprunedfunds(child_txid) - child_txid = bumped_txid - entry_time = node.getmempoolentry(child_txid)["time"] - # tx must be at least 5 minutes older than the last block to be rebroadcast - block_time = entry_time + 6 * 60 - node.setmocktime(block_time) - block = create_block(int(node.getbestblockhash(), 16), height=node.getblockcount() + 1, ntime=block_time) - block.solve() - node.submitblock(block.serialize().hex()) - # Set correct m_best_block_time, which is used in ResubmitWalletTransactions - node.syncwithvalidationinterfacequeue() + evict_time = block_time + 60 * 60 * DEFAULT_MEMPOOL_EXPIRY_HOURS + 5 + # Flush out currently scheduled resubmit attempt now so that there can't be one right between eviction and check. + with node.assert_debug_log(['resubmit 2 unconfirmed transactions'], timeout=2): + node.setmocktime(evict_time) + node.mockscheduler(60) - evict_time = block_time + 60 * 60 * DEFAULT_MEMPOOL_EXPIRY_HOURS + 5 - # Flush out currently scheduled resubmit attempt now so that there can't be one right between eviction and check. - with node.assert_debug_log(['resubmit 2 unconfirmed transactions'], timeout=2): - node.setmocktime(evict_time) - node.mockscheduler(60) + # Evict these txs from the mempool + indep_send = node.send(outputs=[{node.getnewaddress(): 1}], inputs=[indep_utxo]) + node.getmempoolentry(indep_send["txid"]) + assert_raises_rpc_error(-5, "Transaction not in mempool", node.getmempoolentry, txid) + assert_raises_rpc_error(-5, "Transaction not in mempool", node.getmempoolentry, child_txid) - # Evict these txs from the mempool - indep_send = node.send(outputs=[{node.getnewaddress(): 1}], inputs=[indep_utxo]) - node.getmempoolentry(indep_send["txid"]) - assert_raises_rpc_error(-5, "Transaction not in mempool", node.getmempoolentry, txid) - assert_raises_rpc_error(-5, "Transaction not in mempool", node.getmempoolentry, child_txid) + # Rebroadcast and check that parent and child are both in the mempool + with node.assert_debug_log(['resubmit 2 unconfirmed transactions'], timeout=2): + node.setmocktime(evict_time + RESEND_TIMER_LIMIT) + node.mockscheduler(60) + node.getmempoolentry(txid) + node.getmempoolentry(child_txid) - # Rebroadcast and check that parent and child are both in the mempool - with node.assert_debug_log(['resubmit 2 unconfirmed transactions'], timeout=2): - node.setmocktime(evict_time + RESEND_TIMER_LIMIT) - node.mockscheduler(60) - node.getmempoolentry(txid) - node.getmempoolentry(child_txid) + # clear mempool + self.generate(node, 1, sync_fun=self.no_op) + parent_utxo, indep_utxo = node.listunspent()[:2] + txid = node.send(outputs=[{addr: 1}], inputs=[parent_utxo])["txid"] self.log.info("Test rebroadcast of transactions received by others") # clear mempool