From b3965122b4de38903ba955ee6bec320fcd91db47 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 26 Aug 2022 16:55:22 -0400 Subject: [PATCH 1/2] Merge bitcoin/bitcoin#25922: wallet: trigger MaybeResendWalletTxs() every minute 5ef8c2c9fc4ebce6cbfea6a55a89a0ab7ee98a1a test: fix typo for MaybeResendWalletTxs (stickies-v) fbba4a131647c991afc53b6a3dfb9721f5c430b2 wallet: trigger MaybeResendWalletTxs() every minute (stickies-v) Pull request description: ResendWalletTransactions() only executes every [12-36h (24h average)](https://github.com/bitcoin/bitcoin/blob/1420547ec30a24fc82ba3ae5ac18374e8e5af5e5/src/wallet/wallet.cpp#L1947). Triggering it every second is excessive, once per minute should be plenty. The goal of this PR is to reduce the amount of (unnecessary) schedule executions by ~60x without meaningfully altering transaction rebroadcast logic/assumptions which would require more significant review. ACKs for top commit: achow101: ACK 5ef8c2c9fc4ebce6cbfea6a55a89a0ab7ee98a1a 1440000bytes: ACK https://github.com/bitcoin/bitcoin/pull/25922/commits/5ef8c2c9fc4ebce6cbfea6a55a89a0ab7ee98a1a Tree-SHA512: 4a077e3579b289c11c347eaa0d3601ef2dbb9fee66ab918d56b4a0c2e08222560a0e6be295297a74831836e001a997ecc143adb0c132faaba96a669dac1cd9e6 --- src/wallet/load.cpp | 2 +- test/functional/wallet_resendwallettransactions.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 6f5b661efd728..a1e51d76b6748 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -141,7 +141,7 @@ void StartWallets(CScheduler& scheduler, const ArgsManager& args) if (args.GetBoolArg("-flushwallet", DEFAULT_FLUSHWALLET)) { scheduler.scheduleEvery(MaybeCompactWalletDB, std::chrono::milliseconds{500}); } - scheduler.scheduleEvery(MaybeResendWalletTxs, std::chrono::milliseconds{1000}); + scheduler.scheduleEvery(MaybeResendWalletTxs, 1min); } void FlushWallets() diff --git a/test/functional/wallet_resendwallettransactions.py b/test/functional/wallet_resendwallettransactions.py index f5eae33288c04..82bccd1eccd0d 100755 --- a/test/functional/wallet_resendwallettransactions.py +++ b/test/functional/wallet_resendwallettransactions.py @@ -28,11 +28,11 @@ def run_test(self): self.log.info("Create a new transaction and wait until it's broadcast") txid = node.sendtoaddress(node.getnewaddress(), 1) - # Wallet rebroadcast is first scheduled 1 sec after startup (see + # Wallet rebroadcast is first scheduled 1 min sec after startup (see # nNextResend in ResendWalletTransactions()). Tell scheduler to call - # MaybeResendWalletTxn now to initialize nNextResend before the first + # MaybeResendWalletTxs now to initialize nNextResend before the first # setmocktime call below. - node.mockscheduler(1) + node.mockscheduler(60) # Can take a few seconds due to transaction trickling def wait_p2p(): @@ -61,7 +61,7 @@ def wait_p2p(): twelve_hrs = 12 * 60 * 60 two_min = 2 * 60 node.setmocktime(self.mocktime + twelve_hrs - two_min) - node.mockscheduler(1) # Tell scheduler to call MaybeResendWalletTxn now + node.mockscheduler(1) # Tell scheduler to call MaybeResendWalletTxs now assert_equal(int(txid, 16) in peer_second.get_invs(), False) self.log.info("Bump time & check that transaction is rebroadcast") @@ -69,7 +69,7 @@ def wait_p2p(): # but can range from 12-36. So bump 36 hours to be sure. with node.assert_debug_log(['ResendWalletTransactions: resubmit 1 unconfirmed transactions']): node.setmocktime(self.mocktime + 36 * 60 * 60) - # Tell scheduler to call MaybeResendWalletTxn now. + # Tell scheduler to call MaybeResendWalletTxs now. node.mockscheduler(1) # Give some time for trickle to occur node.setmocktime(self.mocktime + 36 * 60 * 60 + 600) From 4d61fb7936a3dc49bbe290351c8e2f55cb3ac475 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Wed, 31 Aug 2022 09:03:42 +0200 Subject: [PATCH 2/2] Merge bitcoin/bitcoin#25955: test: use `sendall` when emptying wallet 28ea4c7039710541e70ec01abefc3eb8268e06f5 test: simplify splitment with `sendall` in wallet_basic (brunoerg) 923d24583d826f4c6ecad30b185e0e043ea11dfc test: use `sendall` when emptying wallet (brunoerg) Pull request description: In some tests they have used `sendtoaddress` in order to empty a wallet. With the addition of `sendall`, it makes sense to use it for that. ACKs for top commit: achow101: ACK 28ea4c7039710541e70ec01abefc3eb8268e06f5 ishaanam: utACK 28ea4c7039710541e70ec01abefc3eb8268e06f5 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25955/commits/28ea4c7039710541e70ec01abefc3eb8268e06f5 Tree-SHA512: 903136d7df5c65d3c02310d5a84241c9fd11070f69d932b4e188b8ad45c38ab5bc1bd5a9242b3e52d2576665ead14be0a03971a9ad8c00431fed442eba4ca48f --- test/functional/rpc_fundrawtransaction.py | 4 ++-- test/functional/wallet_basic.py | 10 ++-------- test/functional/wallet_groups.py | 2 +- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index eeac900658be2..5f1ccfeb7c5f3 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -569,7 +569,7 @@ def test_many_inputs_fee(self): self.log.info("Test fundrawtxn fee with many inputs") # Empty node1, send some small coins from node0 to node1. - self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), self.nodes[1].getbalance(), "", "", True) + self.nodes[1].sendall(recipients=[self.nodes[0].getnewaddress()]) self.generate(self.nodes[1], 1) for _ in range(20): @@ -595,7 +595,7 @@ def test_many_inputs_send(self): self.log.info("Test fundrawtxn sign+send with many inputs") # Again, empty node1, send some small coins from node0 to node1. - self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), self.nodes[1].getbalance(), "", "", True) + self.nodes[1].sendall(recipients=[self.nodes[0].getnewaddress()]) self.generate(self.nodes[1], 1) for _ in range(20): diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 79eb44fa25134..632d6b8376533 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -622,15 +622,9 @@ def run_test(self): # ==Check that wallet prefers to use coins that don't exceed mempool limits ===== - # Get all non-zero utxos together + # Get all non-zero utxos together and split into two chains chain_addrs = [self.nodes[0].getnewaddress(), self.nodes[0].getnewaddress()] - singletxid = self.nodes[0].sendtoaddress(chain_addrs[0], self.nodes[0].getbalance(), "", "", True) - self.generate(self.nodes[0], 1, sync_fun=self.no_op) - node0_balance = self.nodes[0].getbalance() - # Split into two chains - rawtx = self.nodes[0].createrawtransaction([{"txid": singletxid, "vout": 0}], {chain_addrs[0]: node0_balance // 2 - Decimal('0.01'), chain_addrs[1]: node0_balance // 2 - Decimal('0.01')}) - signedtx = self.nodes[0].signrawtransactionwithwallet(rawtx) - singletxid = self.nodes[0].sendrawtransaction(hexstring=signedtx["hex"], maxfeerate=0) + self.nodes[0].sendall(recipients=chain_addrs) self.generate(self.nodes[0], 1, sync_fun=self.no_op) # Make a long chain of unconfirmed payments without hitting mempool limit diff --git a/test/functional/wallet_groups.py b/test/functional/wallet_groups.py index 3eb6a8fa3c761..60dcdb488da7b 100755 --- a/test/functional/wallet_groups.py +++ b/test/functional/wallet_groups.py @@ -150,7 +150,7 @@ def run_test(self): assert_equal(2, len(tx6["vout"])) # Empty out node2's wallet - self.nodes[2].sendtoaddress(address=self.nodes[0].getnewaddress(), amount=self.nodes[2].getbalance(), subtractfeefromamount=True) + self.nodes[2].sendall(recipients=[self.nodes[0].getnewaddress()]) self.sync_all() self.generate(self.nodes[0], 1)