test: use MiniWallet for interface_rest.py #24223

pull theStack wants to merge 2 commits into bitcoin:master from theStack:202201-test-use_MiniWallet_for_interface_rest changing 1 files +27 −26

theStack commented at 10:09 PM on January 31, 2022: contributor

This PR enables one more of the non-wallet functional tests (interface_rest.py) to be run even with the Bitcoin Core wallet disabled by using the MiniWallet instead, as proposed in #20078.

Note that the original test sent funds from one node to another and checked node's balances, but the state of a node's wallet is not relevant to any of the REST endpoints, i.e. the replacement is quite stright-forward. In an additional commit, the test is further sped up by using the good ol' immediate tx relay trick (parameter -whitelist=noban@127.0.0.1).

test: use MiniWallet for rest_interface.py

This test can now be run even with the Bitcoin Core wallet disabled.

11b9684dfd

test: speedup interface_rest.py by whitelisting peers (immediate tx relay)

By whitelisting the peers via -whitelist, the inventory is transmissioned
immediately rather than on average every 5 seconds, speeding up the test by at
least a factor of two:

before:
$ time ./interface_rest.py
...
0m14.82s real     0m01.44s user     0m01.19s system

with this commit:
$ time ./interface_rest.py
...
0m05.67s real     0m01.07s user     0m01.35s system

438e6f4c33

DrahtBot added the label Tests on Jan 31, 2022
DrahtBot commented at 8:23 AM on February 1, 2022: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #24098 (rest: Use query parameters to control resource loading by stickies-v)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
DrahtBot cross-referenced this on Feb 1, 2022 from issue rest: Use query parameters to control resource loading by stickies-v
brunoerg approved
brunoerg commented at 11:24 AM on February 1, 2022: contributor

ACK 438e6f4c3324ec48ab7daf4d4e77c3947ffc00cd No need to say I agree on using MiniWallet when possible! Code looks great!

On master (M1 - MacOS 12): test/functional/interface_rest.py 0.43s user 0.20s system 3% cpu 18.447 total

This branch (M1 - MacOS 12): test/functional/interface_rest.py 0.34s user 0.10s system 8% cpu 5.183 total

Obs: You can rebase on master to fix CI
MarcoFalke merged this on Feb 2, 2022
MarcoFalke closed this on Feb 2, 2022

in test/functional/interface_rest.py:92 in 438e6f4c33

  97 | -
  98 | -        assert_equal(self.nodes[0].getbalance(), 50)
  99 | -
 100 | -        txid = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 0.1)
 101 | +        self.log.info("Broadcast test transaction and sync nodes")
 102 | +        txid, _ = self.wallet.send_to(from_node=self.nodes[0], scriptPubKey=getnewdestination()[1], amount=int(0.1 * COIN))

MarcoFalke commented at 8:15 AM on February 2, 2022:

unrelated nit: Wondering if [1] can be replaced by a named tuple or dict or so?

theStack deleted the branch on Feb 2, 2022
sidhujag referenced this in commit 896f753b0e on Feb 3, 2022
shommel cross-referenced this on Apr 22, 2022 from issue test: Convert non-wallet tests to use our python MiniWallet by MarcoFalke
bitcoin locked this on Feb 2, 2023

Contributors

Labels

Linked (view graph)

#20078 test: Convert non-wallet tests to use our python MiniWallet

test: use MiniWallet for interface_rest.py #24223

Conflicts