#35728 rpc: Properly throw on internal I/O errors in GetTransaction
https://github.com/bitcoin/bitcoin/pull/35728 · · +108/-41 in 10 files, 3 commits · labels: RPC/REST/ZMQ, Needs rebase · draft
Goal
- Prevent RPC and REST from falsely reporting confirmed transactions as missing when disk reads fail
- Alert users and callers to internal storage faults instead of misleading them into thinking a tx is absent
This PR refactors `TxIndex::FindTx` and `node::GetTransaction` to return `util::Expected<CTransactionRef, std::string>` instead of flattening failures to `nullptr` or `false`. RPC and REST callers (`getrawtransaction`, `gettxoutproof`, `ProcessPSBT`, `rest_tx`) are updated to throw internal errors on disk or deserialization failures rather than reporting the transaction as not found.
Problem: When reading a transaction from disk fails due to an I/O or deserialization error, `GetTransaction` returns `nullptr`, making callers believe the transaction does not exist. Downstream services and wallets can misinterpret node I/O corruption as an unconfirmed or nonexistent transaction.
Category: RPC / REST / ZMQ (#23 of 52)
P3 · bug fix
- P3 because callers are no longer misled into thinking confirmed transactions do not exist on read errors
- Ensures RPC clients detect storage or index faults instead of assuming transaction absence
Fixes an RPC error-reporting flaw where I/O errors are conflated with missing transactions, avoiding misinforming RPC consumers during disk corruption.
Membership: Changes error handling and return codes for getrawtransaction, gettxoutproof, and REST endpoints.
Factors: security/stability 1, bug 1, performance 0, user value 1, leverage 0
Category: Indexes (#6 of 7)
P3 · cleanup
- P3 because it cleanly separates missing index records from disk read or deserialization failures
- Stops callers from mistaking index corruption or storage faults for absent keys
Improves txindex query interface by clearly signaling disk read failures rather than conflating them with missing tx entries.
Membership: Modifies TxIndex::FindTx interface in src/index/txindex.cpp to return util::Expected.
Factors: security/stability 1, bug 1, performance 0, user value 0, leverage 1
Reviewability: Stale: Needs rebase
- Needs rebase due to merge conflicts, and the author has been silent for two months
The PR has merge conflicts with base branches, is in draft state, and the author has been inactive on the PR for over 60 days.
Author status: silent since 2026-07-16
Open concerns:
- yuvicc pointed out that for pruned blocks, `getrawtransaction <txid> <blockhash>` returns 'I/O error reading block data' instead of 'Block not available'.
Agreement: Mild
- Concept approval for separating missing transactions from internal read errors (yuvicc, musaHaruna)
- Unaddressed objection: pruned blocks may return an I/O error instead of block not available (yuvicc)
Mild: yuvicc noted pruned blocks mistakenly report an I/O error instead of 'Block not available', open with no author reply.
Reviewers support distinguishing I/O errors from missing transactions, but yuvicc noted a regression where pruned blocks report an I/O error, which remains unanswered by the author.
- yuvicc provided Concept ACK but flagged incorrect pruned block error handling
- musaHaruna gave Concept ACK and suggested minor functional test improvements
Objections:
| Reviewer | Kind | Harm | Status | Blocking | Author replied | Quote |
|---|---|---|---|---|---|---|
| yuvicc | correctness | In pruned mode, requesting a pruned block returns 'I/O error reading block data' instead of 'Block not available' | open | no | no | 2026-07-16: 'In pruned mode when user calls getrawtransaction <txid> <blockhash> for the block that is pruned we return "I/O error reading block data" instead we should return "Block not available" which was done previously?' |
Support:
- yuvicc: Distinguishing “transaction not found” from internal read failures in TxIndex::FindTx/GetTransaction makes sense.
- musaHaruna: Concept ACK on separating expected outcomes ("transaction not found") from unexpected failures (I/O errors).
Participants: yuvicc (objection), musaHaruna (support)
State derived from the lists: nonblocking objection open (yuvicc)
Review verdicts (DrahtBot): 0
- Concept ACK: yuvicc, musaHaruna
Files
75 lines under test/bench/ci.
- test/functional/feature_io_errors.py +54/-0
- src/rpc/rawtransaction.cpp +10/-11
- src/test/txindex_tests.cpp +7/-13
- src/node/transaction.cpp +10/-4
- src/index/txindex.cpp +7/-6
- src/index/txindex.h +4/-3
- src/node/transaction.h +5/-2
- src/rest.cpp +5/-1
- src/rpc/txoutproof.cpp +5/-1
- test/functional/test_runner.py +1/-0
Card
This draft PR updates GetTransaction and TxIndex::FindTx to return Expected types, differentiating internal I/O failures from missing transactions across RPC and REST endpoints. Currently, an I/O error reading block data falsely reports that a transaction does not exist. Both reviewers agree with the concept, but yuvicc noted an unresolved regression where pruned blocks return an internal I/O error instead of 'Block not available'. The PR has merge conflicts and has been waiting on the author for over two months.