qa: Use wallet to retrieve raw transactions #15247

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:Mf1901-qaWalletRaw changing 2 files +40 −29

MarcoFalke commented at 11:45 PM on January 24, 2019: member

Instead of asking the coin database and block storage about a transaction, pull it directly from the wallet in wallet related tests.

This refactoring only makes sense in light of #15159.

<sub>This product may contain minor stylistic cleanups
MarcoFalke added the label Refactoring on Jan 24, 2019
MarcoFalke added the label Tests on Jan 24, 2019
MarcoFalke force-pushed on Jan 24, 2019
jnewbery commented at 3:35 PM on January 25, 2019: member
I think it's good to improve code style in files that you touch, but could you split this into commits:
- one that improves code style
- one that changes behaviour
doing both in a single commit makes is discouraged: do not mix any formatting fixes or code moves with actual code changes.
qa: Style-only fixes in touched files fa2198328e
MarcoFalke force-pushed on Jan 25, 2019
in test/functional/wallet_abandonconflict.py:58 in fa2320a3f0 outdated
```
  57 |  
  58 |          # Identify the 10btc outputs
  59 | -        nA = next(i for i, vout in enumerate(self.nodes[0].getrawtransaction(txA, 1)["vout"]) if vout["value"] == Decimal("10"))
  60 | -        nB = next(i for i, vout in enumerate(self.nodes[0].getrawtransaction(txB, 1)["vout"]) if vout["value"] == Decimal("10"))
  61 | -        nC = next(i for i, vout in enumerate(self.nodes[0].getrawtransaction(txC, 1)["vout"]) if vout["value"] == Decimal("10"))
  62 | +        nA = next(d["vout"] for d in self.nodes[0].gettransaction(txA)["details"] if d["amount"] == Decimal("10"))
```
jnewbery commented at 5:10 PM on January 25, 2019:

what is d for here? Would changing it to tx_out here make it clearer?

MarcoFalke commented at 6:43 PM on January 25, 2019:

d is short for details

jnewbery commented at 7:20 PM on January 25, 2019:

What is a detail in Bitcoin?

MarcoFalke commented at 7:26 PM on January 25, 2019:

It is specific to the rpc, see https://github.com/bitcoin/bitcoin/blob/f0c9e1c22b8a043983f3ba90ad910b67cf981e36/src/wallet/rpcwallet.cpp#L1684, so I am sticking to that naming scheme. I don't mind if someone improves the documentation there.

jnewbery commented at 8:06 PM on January 25, 2019:

The name details can't be changed in the RPC since it's part of the API. I'm just suggesting you could use a more variable name which is more descriptive for people reading the test.

ACK either way.

amitiuttarwar commented at 1:45 AM on January 27, 2019:

+1 descriptiveness within test :)
jnewbery commented at 6:38 PM on January 25, 2019: member

Tested ACK fa2320a3f0374045414ec5dc2fecae76224a1afe. One nit.
qa: Use wallet to retrieve raw transactions fa5278a419
MarcoFalke force-pushed on Jan 27, 2019
jnewbery commented at 4:34 PM on January 28, 2019: member

tACK fa5278a4196cf0b560f5eeb1fb5f9c5e98219ec4. Thanks!
MarcoFalke referenced this in commit 5c99bb0047 on Feb 1, 2019
MarcoFalke merged this on Feb 1, 2019
MarcoFalke closed this on Feb 1, 2019
MarcoFalke deleted the branch on Feb 1, 2019
deadalnix referenced this in commit a910f73377 on May 16, 2020
pravblockc referenced this in commit ed4727de9e on Aug 13, 2021
pravblockc referenced this in commit baa645d774 on Aug 14, 2021
pravblockc referenced this in commit 0c15fbe953 on Aug 17, 2021
pravblockc referenced this in commit 8226f22efb on Aug 17, 2021
MarcoFalke locked this on Dec 16, 2021

Contributors

Labels

Linked (view graph)

#15159 [RPC] Remove lookup to UTXO set from GetTransaction