test: use miniwallet in test_rpc function in feature

Shubhankar-Gambhir commented at 10:03 AM on September 21, 2021: contributor

This PR aims to further increase MiniWallet usage in the functional test feature_rbf.py by using it in the test_rpc(...) . Sub test needs wallet to get a utxo for creating raw txn, and to get address for output, address can be hardcoded and mini wallet can be used for utxo. fund raw transaction is a wallet rpc and should be tested only when bitcoin core is compiled with wallet

Shubhankar-Gambhir commented at 10:30 AM on September 21, 2021: contributor

<details> <summary> successfully ran test with changes proposed on utxo helper in [#22998](/bitcoin-bitcoin/22998/) </summary>

2021-09-21T10:18:35.765000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_q9cpp4nw
2021-09-21T10:18:42.487000Z TestFramework (INFO): Running test simple doublespend...
2021-09-21T10:18:42.504000Z TestFramework (INFO): Running test doublespend chain...
2021-09-21T10:18:42.553000Z TestFramework (INFO): Running test doublespend tree...
2021-09-21T10:18:43.002000Z TestFramework (INFO): Running test replacement feeperkb...
2021-09-21T10:18:43.062000Z TestFramework (INFO): Running test spends of conflicting outputs...
2021-09-21T10:18:43.073000Z TestFramework (INFO): Running test new unconfirmed inputs...
2021-09-21T10:18:43.081000Z TestFramework (INFO): Running test too many replacements...
2021-09-21T10:18:43.147000Z TestFramework (INFO): Running test opt-in...
2021-09-21T10:18:43.159000Z TestFramework (INFO): Running test RPC...
2021-09-21T10:18:43.728000Z TestFramework (INFO): Running test prioritised transactions...
2021-09-21T10:18:43.817000Z TestFramework (INFO): Running test no inherited signaling...
2021-09-21T10:18:43.823000Z TestFramework (INFO): Check that the child tx can still be replaced (via a tx that also replaces the parent)
2021-09-21T10:18:43.825000Z TestFramework (INFO): Running test replacement relay fee...
2021-09-21T10:18:43.828000Z TestFramework (INFO): Passed
2021-09-21T10:18:43.878000Z TestFramework (INFO): Stopping nodes
2021-09-21T10:18:47.246000Z TestFramework (INFO): Cleaning up /tmp/bitcoin_func_test_q9cpp4nw on exit
2021-09-21T10:18:47.246000Z TestFramework (INFO): Tests successful

</details>

DrahtBot added the label Tests on Sep 21, 2021

theStack commented at 11:46 AM on September 21, 2021: member

Concept ACK, thanks for working on this!

This is a nice complementary PR to #22998: as soon as one the two PRs gets merged, the following part can be removed in the other one (if I'm not mistaken):

    def skip_test_if_missing_module(self):
        self.skip_if_no_wallet()

Shubhankar-Gambhir force-pushed on Sep 21, 2021

Shubhankar-Gambhir commented at 3:39 PM on September 21, 2021: contributor

As #22998 is merged, the test can run without compiled wallet, so removed the corresponding patch as mentioned by @theStack , got succesful result (same as before) on running test with bitcoin core compiled without wallet.

Shubhankar-Gambhir force-pushed on Sep 22, 2021

Shubhankar-Gambhir closed this on Sep 26, 2021

Shubhankar-Gambhir force-pushed on Sep 26, 2021

Shubhankar-Gambhir reopened this on Sep 26, 2021

in test/functional/feature_rbf.py:547 in 91428914ac outdated

 541 | @@ -545,24 +542,25 @@ def test_prioritised_transactions(self):
 542 |          assert tx2b_txid in self.nodes[0].getrawmempool()
 543 |  
 544 |      def test_rpc(self):
 545 | -        us0 = self.nodes[0].listunspent()[0]
 546 | +        us0 = self.wallet.get_utxo()
 547 |          ins = [us0]
 548 | -        outs = {self.nodes[0].getnewaddress(): Decimal(1.0000000)}
 549 | +        outs = {"bcrt1qxcm2aqvzejqyd44jujp5qyz6sya035j6st07g0": Decimal(1.0000000)}

MarcoFalke commented at 8:20 AM on September 26, 2021:

can this use ADDRESS_BCRT1_UNSPENDABLE?

Shubhankar-Gambhir commented at 5:54 AM on September 27, 2021:

Yes, I think it can (I have succesfully run the test locally), but would there be any benifit of using it in place of the hardcode address present currently?

MarcoFalke commented at 6:20 AM on September 27, 2021:

Obviously a nit, but using the undependable address clarifies that the test isn't going to spend this output

Shubhankar-Gambhir commented at 6:40 AM on September 27, 2021:

Agreed, thanks for clarifying. I have included it now.

Shubhankar-Gambhir force-pushed on Sep 27, 2021

Shubhankar-Gambhir force-pushed on Sep 29, 2021

test: use miniwallet in test_rpc() function in feature_rbf.py 74c0d81b46

Shubhankar-Gambhir force-pushed on Sep 29, 2021

Shubhankar-Gambhir commented at 5:01 PM on September 29, 2021: contributor

Have used init_wallet to remove test failures.

laanwj commented at 1:34 PM on September 30, 2021: member

Code review ACK 74c0d81b46150923f9af9631f4e3ecf80213cca2

theStack approved

theStack commented at 7:00 PM on October 2, 2021: member

tACK 74c0d81b46150923f9af9631f4e3ecf80213cca2

Ran feature_rbf.py locally twice, once with and once without wallet compiled into bitcoind, both tests passed.

MarcoFalke merged this on Oct 4, 2021

MarcoFalke closed this on Oct 4, 2021

sidhujag referenced this in commit 3a30259f1d on Oct 4, 2021

DrahtBot locked this on Oct 30, 2022

test: use miniwallet in test_rpc function in feature_rbf.py #23052