test: use `sendall` when emptying wallet #25955

pull brunoerg wants to merge 2 commits into bitcoin:master from brunoerg:2022-08-sendall-wallet-groups changing 4 files +6 −12

brunoerg commented at 7:14 PM on August 29, 2022: contributor

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.
fanquake added the label Tests on Aug 29, 2022
kouloumos commented at 11:26 AM on August 30, 2022: contributor

Does it make sense to also change the other occurrences of this usage scenario? See git grep 'sendtoaddress.*getbalance'
brunoerg commented at 12:22 PM on August 30, 2022: contributor

Does it make sense to also change the other occurrences of this usage scenario?

Make sense, let me check other cases and if so, I will update them and change the PR title/desc.
test: use `sendall` when emptying wallet 923d24583d
brunoerg force-pushed on Aug 30, 2022
brunoerg renamed this:
~~test: use `sendall` in wallet_groups~~
test: use sendall when emptying wallet
on Aug 30, 2022
brunoerg renamed this:
~~test: use sendall when emptying wallet~~
test: use `sendall` when emptying wallet
on Aug 30, 2022
brunoerg commented at 1:02 PM on August 30, 2022: contributor

Thanks for the suggestion, @kouloumos.

Force-pushed replacing sendtoaddress to sendall for all cases where they want to empty a wallet.
in test/functional/wallet_basic.py:590 in 923d24583d outdated
```
 586 | @@ -587,7 +587,7 @@ def run_test(self):
 587 |  
 588 |          # Get all non-zero utxos together
 589 |          chain_addrs = [self.nodes[0].getnewaddress(), self.nodes[0].getnewaddress()]
 590 | -        singletxid = self.nodes[0].sendtoaddress(chain_addrs[0], self.nodes[0].getbalance(), "", "", True)
 591 | +        singletxid = self.nodes[0].sendall(recipients=[chain_addrs[0]])['txid']
```
kouloumos commented at 2:22 PM on August 30, 2022:

This might be out of scope, although it still uses the sendall to simplify the logic and makes L591-596 redundant:

# Split all non-zero utxos into two chains self.nodes[0].sendall(recipients=chain_addrs)['txid']

Note: recipients receive equal share of the unspecified amount
brunoerg commented at 7:29 PM on August 30, 2022:

Nice catch! Added this on a new commit.

ishaanam commented at 10:07 PM on August 30, 2022:

In 923d24583d826f4c6ecad30b185e0e043ea11dfc I think that this change can be removed as it is modified in the next commit anyways?
kouloumos commented at 2:22 PM on August 30, 2022: contributor

ACK 923d24583d826f4c6ecad30b185e0e043ea11dfc, this change makes sense as that's one of the intended uses with the introduction of this RPC.

Similar change in #25159.
test: simplify splitment with `sendall` in wallet_basic
recipients receive equal share of the unspecified amount
28ea4c7039
ishaanam commented at 10:14 PM on August 30, 2022: contributor

utACK 28ea4c7039710541e70ec01abefc3eb8268e06f5 I think one of the reasons behind sendall was so that it could be used to simplify functional tests.
achow101 commented at 10:39 PM on August 30, 2022: member

ACK 28ea4c7039710541e70ec01abefc3eb8268e06f5
w0xlt approved
w0xlt commented at 10:47 PM on August 30, 2022: contributor

ACK https://github.com/bitcoin/bitcoin/pull/25955/commits/28ea4c7039710541e70ec01abefc3eb8268e06f5
MarcoFalke merged this on Aug 31, 2022
MarcoFalke closed this on Aug 31, 2022
sidhujag referenced this in commit 720a7d70ee on Sep 1, 2022
bitcoin locked this on Aug 31, 2023

Contributors

Labels