← Watch feed
Merge bitcoin/bitcoin#36083: test: cover getrawtransaction on a stale block via txindex
Public commit record
100/100 · Strong
What the developer wrote
Authored by merge-script
Merge bitcoin/bitcoin#36083: test: cover getrawtransaction on a stale block via txindex
793e22eed586306d41e05811f749da6cbe29f3de test: cover getrawtransaction on a stale block via txindex (Íñigo Aréjula Aísa)
a84fbf9dfd2cc173499fdc6b2267246cf9403292 test: name the getrawtransaction test nodes (Íñigo Aréjula Aísa)
Pull request description:
<!--
*** Please remove the following help text before submitting: ***
Pull requests may be closed immediately if they:
- do not have a rationale and clear improvement
- do not adhere to doc/AI_POLICY.md
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui first.
See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Contributors should note which tests cover
modified code. If no tests exist for a region of modified code, new tests
should accompany the change.
* Bug fixes are most welcome when they come with steps to reproduce or an
explanation of the potential issue as well as reasoning for the way the bug
was fixed.
* Features are welcome, but might be rejected due to design or scope issues.
If a feature is based on a lot of dependencies, contributors should first
consider building the system outside of Bitcoin Core, if possible.
* Refactoring changes are only accepted if they are required for a feature or
bug fix or otherwise improve developer experience significantly. For example,
most "code style" refactoring changes require a thorough explanation why they
are useful, what downsides they have and why they *significantly* improve
developer experience or avoid serious programming bugs. Note that code style
is often a subjective matter. Unless they are explicitly mentioned to be
preferred in the [developer notes](/doc/developer-notes.md), stylistic code
changes are usually rejected.
-->
<!--
Bitcoin Core has a thorough review process and even the most trivial change
needs to pass a lot of eyes and requires non-zero or even substantial time
effort to review. There is a huge lack of active reviewers on the project, so
patches often sit for a long time.
-->
This checks the `txindex` as the only possible source for a transaction in a stale block. **The response of `getrawtransaction `is checked to report that block as stale: confirmations 0, no time, no blocktime**.
It uses one transaction that cannot come back to the mempool once the block is disconnected: the block's coinbase which is asserted to be out of the mempool.
I didn't find an existing test that reaches the index this way.
ACKs for top commit:
l0rinc:
ACK 793e22eed586306d41e05811f749da6cbe29f3de
sedited:
ACK 793e22eed586306d41e05811f749da6cbe29f3de
Tree-SHA512: ec1df499f4ddaf30e5667c6ec9e8f94c54635040485f16d7b911785d2b9fdc52e9a59f3865f1707d250032c8c2b89d5f988c76d0cccb684366f683a849bd71a9
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode✓ Mentions testing or verification✓ Links an issue, advisory, or supporting reference
What changed, and why it matters
This commit only adds a new functional test to Bitcoin Core. It checks that the getrawtransaction RPC can retrieve a stale block's coinbase transaction via the optional txindex, and that the response correctly shows the block is no longer in the active chain (confirmations 0, no time/blocktime). There is no change to production code, no bug fix, and no security patch.
Recommended action
No security action needed. This is a test-only improvement. Reviewers may optionally run the updated functional test to confirm coverage.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The diff modifies test/functional/rpc_rawtransaction.py to add coverage for getrawtransaction on a transaction that exists only in a stale block, accessed through -txindex. It refactors node iteration from numeric indices to (node, txindex_enabled) tuples, names nodes more explicitly, and adds assertions that a stale coinbase can be fetched without a blockhash when txindex is enabled, returning confirmations=0 and absent time/blocktime. No C++/Python production code is changed.
Changed components
test/functional/rpc_rawtransaction.pyInspect captured patch +41 / −34
### test/functional/rpc_rawtransaction.py
@@ -101,78 +101,85 @@ def getrawtransaction_tests(self):
" blockchain transaction queries. Use gettransaction for wallet transactions."
)
- for n in [0, 2]:
- self.log.info(f"Test getrawtransaction {'with' if n == 0 else 'without'} -txindex")
+ for node, txindex_enabled in [(self.nodes[0], True), (self.nodes[2], False)]:
+ self.log.info(f"Test getrawtransaction {'with' if txindex_enabled else 'without'} -txindex")
- if n == 0:
- sync_txindex(self, self.nodes[n])
+ if txindex_enabled:
+ sync_txindex(self, node)
# With -txindex.
# 1. valid parameters - only supply txid
- assert_equal(self.nodes[n].getrawtransaction(txId), tx['hex'])
+ assert_equal(node.getrawtransaction(txId), tx['hex'])
# 2. valid parameters - supply txid and 0 for non-verbose
- assert_equal(self.nodes[n].getrawtransaction(txId, 0), tx['hex'])
+ assert_equal(node.getrawtransaction(txId, 0), tx['hex'])
# 3. valid parameters - supply txid and False for non-verbose
- assert_equal(self.nodes[n].getrawtransaction(txId, False), tx['hex'])
+ assert_equal(node.getrawtransaction(txId, False), tx['hex'])
# 4. valid parameters - supply txid and 1 for verbose.
# We only check the "hex" field of the output so we don't need to update this test every time the output format changes.
- assert_equal(self.nodes[n].getrawtransaction(txId, 1)["hex"], tx['hex'])
- assert_equal(self.nodes[n].getrawtransaction(txId, 2)["hex"], tx['hex'])
+ assert_equal(node.getrawtransaction(txId, 1)["hex"], tx['hex'])
+ assert_equal(node.getrawtransaction(txId, 2)["hex"], tx['hex'])
# 5. valid parameters - supply txid and True for non-verbose
- assert_equal(self.nodes[n].getrawtransaction(txId, True)["hex"], tx['hex'])
+ assert_equal(node.getrawtransaction(txId, True)["hex"], tx['hex'])
else:
# Without -txindex, expect to raise.
for verbose in [None, 0, False, 1, True]:
- assert_raises_rpc_error(-5, err_msg, self.nodes[n].getrawtransaction, txId, verbose)
+ assert_raises_rpc_error(-5, err_msg, node.getrawtransaction, txId, verbose)
# 6. invalid parameters - supply txid and invalid boolean values (strings) for verbose
for value in ["True", "False"]:
- assert_raises_rpc_error(-3, "not of expected type number", self.nodes[n].getrawtransaction, txid=txId, verbose=value)
- assert_raises_rpc_error(-3, "not of expected type number", self.nodes[n].getrawtransaction, txid=txId, verbosity=value)
+ assert_raises_rpc_error(-3, "not of expected type number", node.getrawtransaction, txid=txId, verbose=value)
+ assert_raises_rpc_error(-3, "not of expected type number", node.getrawtransaction, txid=txId, verbosity=value)
# 7. invalid parameters - supply txid and empty array
- assert_raises_rpc_error(-3, "not of expected type number", self.nodes[n].getrawtransaction, txId, [])
+ assert_raises_rpc_error(-3, "not of expected type number", node.getrawtransaction, txId, [])
# 8. invalid parameters - supply txid and empty dict
- assert_raises_rpc_error(-3, "not of expected type number", self.nodes[n].getrawtransaction, txId, {})
+ assert_raises_rpc_error(-3, "not of expected type number", node.getrawtransaction, txId, {})
# Make a tx by sending, then generate 2 blocks; block1 has the tx in it
tx = self.wallet.send_self_transfer(from_node=self.nodes[2])['txid']
block1, block2 = self.generate(self.nodes[2], 2)
- for n in [0, 2]:
- self.log.info(f"Test getrawtransaction {'with' if n == 0 else 'without'} -txindex, with blockhash")
+ stale_coinbase = self.nodes[0].getblock(block1)['tx'][0]
+ for node, txindex_enabled in [(self.nodes[0], True), (self.nodes[2], False)]:
+ self.log.info(f"Test getrawtransaction {'with' if txindex_enabled else 'without'} -txindex, with blockhash")
# We should be able to get the raw transaction by providing the correct block
- gottx = self.nodes[n].getrawtransaction(txid=tx, verbose=True, blockhash=block1)
+ gottx = node.getrawtransaction(txid=tx, verbose=True, blockhash=block1)
assert_equal(gottx['txid'], tx)
assert_equal(gottx['in_active_chain'], True)
- if n == 0:
+ if txindex_enabled:
self.log.info("Test getrawtransaction with -txindex, without blockhash: 'in_active_chain' should be absent")
for v in [1,2]:
- gottx = self.nodes[n].getrawtransaction(txid=tx, verbosity=v)
+ gottx = node.getrawtransaction(txid=tx, verbosity=v)
assert_equal(gottx['txid'], tx)
assert 'in_active_chain' not in gottx
else:
self.log.info("Test getrawtransaction without -txindex, without blockhash: expect the call to raise")
- assert_raises_rpc_error(-5, err_msg, self.nodes[n].getrawtransaction, txid=tx, verbose=True)
+ assert_raises_rpc_error(-5, err_msg, node.getrawtransaction, txid=tx, verbose=True)
# We should not get the tx if we provide an unrelated block
- assert_raises_rpc_error(-5, "No such transaction found", self.nodes[n].getrawtransaction, txid=tx, blockhash=block2)
+ assert_raises_rpc_error(-5, "No such transaction found", node.getrawtransaction, txid=tx, blockhash=block2)
# An invalid block hash should raise the correct errors
- assert_raises_rpc_error(-3, "JSON value of type bool is not of expected type string", self.nodes[n].getrawtransaction, txid=tx, blockhash=True)
- assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 6, for 'foobar')", self.nodes[n].getrawtransaction, txid=tx, blockhash="foobar")
- assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 8, for 'abcd1234')", self.nodes[n].getrawtransaction, txid=tx, blockhash="abcd1234")
+ assert_raises_rpc_error(-3, "JSON value of type bool is not of expected type string", node.getrawtransaction, txid=tx, blockhash=True)
+ assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 6, for 'foobar')", node.getrawtransaction, txid=tx, blockhash="foobar")
+ assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 8, for 'abcd1234')", node.getrawtransaction, txid=tx, blockhash="abcd1234")
foo = "ZZZ0000000000000000000000000000000000000000000000000000000000000"
- assert_raises_rpc_error(-8, f"parameter 3 must be hexadecimal string (not '{foo}')", self.nodes[n].getrawtransaction, txid=tx, blockhash=foo)
+ assert_raises_rpc_error(-8, f"parameter 3 must be hexadecimal string (not '{foo}')", node.getrawtransaction, txid=tx, blockhash=foo)
bar = "0000000000000000000000000000000000000000000000000000000000000000"
- assert_raises_rpc_error(-5, "Block hash not found", self.nodes[n].getrawtransaction, txid=tx, blockhash=bar)
+ assert_raises_rpc_error(-5, "Block hash not found", node.getrawtransaction, txid=tx, blockhash=bar)
# Undo the blocks and verify that "in_active_chain" is false.
- self.nodes[n].invalidateblock(block1)
- gottx = self.nodes[n].getrawtransaction(txid=tx, verbose=True, blockhash=block1)
+ node.invalidateblock(block1)
+ gottx = node.getrawtransaction(txid=tx, verbose=True, blockhash=block1)
assert_equal(gottx['in_active_chain'], False)
- self.nodes[n].reconsiderblock(block1)
- assert_equal(self.nodes[n].getbestblockhash(), block2)
+ self.log.info(f"Test getrawtransaction {'with' if txindex_enabled else 'without'} -txindex on a stale block, without blockhash")
+ if txindex_enabled:
+ gottx = node.getrawtransaction(txid=stale_coinbase, verbose=True)
+ assert_equal({key: gottx[key] for key in gottx.keys() & {'blockhash', 'confirmations', 'time', 'blocktime'}}, {'blockhash': block1, 'confirmations': 0})
+ else:
+ assert_raises_rpc_error(-5, err_msg, node.getrawtransaction, txid=stale_coinbase, verbose=True)
+ node.reconsiderblock(block1)
+ assert_equal(node.getbestblockhash(), block2)
self.log.info("Test getrawtransaction on genesis block coinbase returns an error")
block = self.nodes[0].getblock(self.nodes[0].getblockhash(0))
@@ -210,9 +217,9 @@ def getrawtransaction_verbosity_tests(self):
'type',
]
# node 0 & 2 with verbosity 1 & 2
- for n, v in product([0, 2], [1, 2]):
- self.log.info(f"Test getrawtransaction_verbosity {v} {'with' if n == 0 else 'without'} -txindex, with blockhash")
- gottx = self.nodes[n].getrawtransaction(txid=tx, verbosity=v, blockhash=block1)
+ for (node, txindex_enabled), v in product([(self.nodes[0], True), (self.nodes[2], False)], [1, 2]):
+ self.log.info(f"Test getrawtransaction_verbosity {v} {'with' if txindex_enabled else 'without'} -txindex, with blockhash")
+ gottx = node.getrawtransaction(txid=tx, verbosity=v, blockhash=block1)
missing_fields = set(fields).difference(gottx.keys())
if missing_fields:
raise AssertionError(f"fields {', '.join(missing_fields)} are not in transaction")Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.