test: get and decode tx with a single `gettransaction` RPC call #23287

Rather than subsequently calling gettransaction and decoderawtransaction to get the decoded information for a specific tx-id, we can simply use the verbose version of gettransaction, which returns this in a 'decoded' key. I.e.

node.decoderawtransaction(node.gettransaction(txid)['hex'])

can simply be replaced by:

node.gettransaction(txid=txid, verbose=True)['decoded']

Rationale: shorter code, shorter test logs, less RPC calls.

Tested ACK c045207fb4a96f3cbdf7841ecab5c36323ea01a3. Ran the tests and reviewed the changes.

Concept ACK

I think mempool_packages.py (line 68 and 69) could be also included here, couldn't it?

Code review ACK c045207fb4a96f3cbdf7841ecab5c36323ea01a3. Didn't check if same approach could be applied in other places.

Concept ACK

This PR simplifies the code and RPC calls around getting a transaction and decoding it. I was able to successfully compile the PR on Ubuntu 20.04 without any errors. Nice catch, @theStack!

I think mempool_packages.py (line 68 and 69) could be also included here, couldn't it?

I think this is a valid suggestion. I tried to implement this myself while testing, and I was able to successfully do so in the following way:

# We need the wtxids to check P2P announcements
-            fulltx = self.nodes[0].getrawtransaction(txid)
-            witnesstx = self.nodes[0].decoderawtransaction(fulltx, True)
+            witnesstx = self.nodes[0].gettransaction(txid=txid, verbose=True)['decoded']
             witness_chain.append(witnesstx['hash'])

tACK c045207fb4a96f3cbdf7841ecab5c36323ea01a3

I think mempool_packages.py (line 68 and 69) could be also included here, couldn't it?

I think this is a valid suggestion. I tried to implement this myself while testing, and I was able to successfully do so in the following way:

# We need the wtxids to check P2P announcements
-            fulltx = self.nodes[0].getrawtransaction(txid)
-            witnesstx = self.nodes[0].decoderawtransaction(fulltx, True)
+            witnesstx = self.nodes[0].gettransaction(txid=txid, verbose=True)['decoded']
             witness_chain.append(witnesstx['hash'])

Great! I am gonna check if there are other places it could be applied

Tested ACK c045207

Verified that the output dictionary produced for a specific tx-id in both the master and PR are the same. There’s 1 more instance where this change can be applied:

In test/functional/rpc_rawtransaction.py:

• L464-465
• L474-475
• L507-508
• L515-516

Thanks for all the reviews! Force-pushed with the substitution also done in mempool_packages.py, as suggested by @brunoerg and @shaavan (https://github.com/bitcoin/bitcoin/pull/23287#issuecomment-944615929, #23287#pullrequestreview-781363806). @stratospher: Note that I intentionally didn't change anything in rpc_rawtransaction.py, as I think this test's focus are RPCs that cope with raw transactions (like decoderawtransaction).

tested ACK 130ee48

@stratospher: Note that I intentionally didn't change anything in rpc_rawtransaction.py, as I think this test's focus are RPCs that cope with raw transactions (like decoderawtransaction). @theStack, Thanks for clarifying. That makes total sense!

Tested ACK 130ee48 on Ubuntu 20.04.

ACK 130ee481082d2612d452d7d69131ade935b225b5

tACK 130ee481082d2612d452d7d69131ade935b225b5