interfaces: Avoid node crashes from invalid Chain IPC calls #36520

pull ryanofsky wants to merge 3 commits into bitcoin:master from ryanofsky:pr/chainsafe changing 3 files +120 −9

ryanofsky commented at 5:39 AM on October 14, 2026: contributor

Problem: A program using the Chain IPC interface from #29409 can crash bitcoin-node with a single bad call: asking for a block hash above the tip, leaving the transaction argument of broadcastTransaction or checkChainLimits unset, or returning an error from a notification handler. Separately, findAncestorByHash reports success when the descendant block is unknown (#36472). The Chain methods were written for in-process C++ callers like the wallet, which follow documented preconditions, so they assert or dereference null instead of returning errors.

Solution: Make these Chain methods in src/node/interfaces.cpp return errors for invalid arguments, catch and log IPC errors from notification handlers, and fix the findAncestorByHash check. Three small commits, each with unit tests in src/test/interfaces_tests.cpp. Valid calls behave the same as before. Details are in the commit messages.

This PR does not depend on #29409, but the fixes matter if it is merged and interfaces::Chain is exposed over IPC. The problems were reported by zaidmstrr, willcl-ark and xyzconstant while reviewing #29409, and by enirox001 in #36472. The findAncestorByHash fix is the same one previously proposed by Romansage6 and Yudis-bit in #36473, #36484 and #36490.

interfaces: Fix findAncestorByHash returning true for unknown block

Chain::findAncestorByHash returned true when block_hash was unknown and
ancestor_hash was known, because the check clearing the ancestor pointer
only ran when both hashes were found. Clear the ancestor whenever the
block is not found.

Fixes #36472, reported by enirox001 while testing the IPC Chain interface
from #29409. The same fix was previously proposed in #36473, #36484 and
#36490 by Romansage6 and Yudis-bit.

This change was written with Claude Opus 5.5 (1M context).

03e46542e6

interfaces: Return errors instead of crashing on invalid Chain arguments

Chain methods were written for in-process C++ callers, which are expected
to follow documented preconditions. Over IPC, non-C++ clients can pass any
value, including null pointers for omitted Cap'n Proto fields, and an
invalid argument should not be able to crash the node.

- getBlockHash throws std::out_of_range instead of asserting when the
  height is not in the active chain. libmultiprocess sends the exception
  to the IPC client as an error.
- broadcastTransaction returns an error for a null transaction or an out
  of range broadcast method, instead of dereferencing null or silently
  doing nothing.
- checkChainLimits returns an error for a null transaction instead of
  dereferencing it.

These were reported by zaidmstrr and willcl-ark while reviewing #29409.

This change was written with Claude Opus 5.5 (1M context).

42fcc504e2

interfaces: Catch IPC exceptions thrown by Chain notification handlers

Chain::Notifications methods are called from the validation interface
scheduler thread, where an uncaught exception terminates the node.
A handler implemented by an IPC client throws ipc::Exception on the node
side if the client returns an error or disconnects. Catch and log these
exceptions so a misbehaving client cannot crash the node. Exceptions from
in-process handlers are still not caught, since they indicate bugs.

Reported by xyzconstant while reviewing #29409.

This change was written with Claude Opus 5.5 (1M context).

b67d871f95

DrahtBot added the label IPC on Oct 14, 2026
DrahtBot commented at 5:39 AM on October 14, 2026: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



External sites
- Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36520
- Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36520.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36520#author-ralph


Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers

Concept ACK enirox001

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #36074 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36074.svg"></sub> (scripted-diff: [test] Add util/check.h includes for assertions by maflcko)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.



LLM Linter (✨ experimental)

Possible places where comparison-specific test macros should replace generic comparisons:
- src/test/interfaces_tests.cpp: BOOST_CHECK_THROW(chain->getBlockHash(tip->nHeight + 1), std::out_of_range) → Prefer BOOST_CHECK_EXCEPTION with a predicate checking the exact “not in the active chain” message.
- src/test/interfaces_tests.cpp: BOOST_CHECK_THROW(chain->getBlockHash(-1), std::out_of_range) → Prefer BOOST_CHECK_EXCEPTION with a predicate checking the exact “Block height -1 is not in the active chain” message.
<sup>2026-10-14 05:39:57</sup>
enirox001 commented at 10:43 AM on October 14, 2026: contributor

Concept ACK

I like this approach, which includes the specific fixes while also making the chain interface more defensive.

These methods were originally being called in process where the callers are expected to follow the preconditions, but once they are exposed over IPC, it makes sense to be more defensive around invalid inputs. Ideally an unexpected/bad IPC call should result in an error rather than crashing the nod

Type	Reviewers
Concept ACK	enirox001

Contributors

Labels

Linked (view graph)

#29409 ipc: Expose Chain interface to IPC clients #36472 interfaces: `findAncestorByHash` returns true for an unknown descendant #36473 interfaces: fix findAncestorByHash false positive #36484 interfaces: fix findAncestorByHash returning true for unknown descendant #36490 interfaces: fix findAncestorByHash returning true for unknown block

interfaces: Avoid node crashes from invalid Chain IPC calls #36520

External sites

Reviews

Conflicts

LLM Linter (✨ experimental)