#36326 kernel: Use typed errors for fatal and flush error notifications

ReviewabilityReviewsAgreementSize
#36326 arejula27 kernel: Use typed errors for fatal and flush error notificationsWaiting on author0Mild358 + 96 tests
  • Remove application-level translation machinery dependencies from libbitcoinkernel
  • Replace string-based fatal and flush error notifications with typed errors across the kernel boundary
  • Move error string formatting and translation into the node layer
  • Ready for review
  • Concept approval without detailed review (sedited, yuvicc)
  • Objection that replacing C API callback messages with error codes drops details (ryanofsky, yuvicc)
  • Unaddressed suggestions on C API dynamic error details and debug assertions (Yudis-bit)
  • 358 lines added or modified outside tests
  • 96 lines added or modified in tests
  • 89 lines removed in total
  • 15 files, 6 commits
Kernel (libbitcoinkernel)#8 of 18
P2 · unblocks #27587
  • P2 because it decouples libbitcoinkernel from application-layer translation dependencies
  • Directly advances tracking issue #27587 by removing translation.h from the kernel
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Replaces string-based fatal and flush error notifications in the kernel interface with typed data structures. It introduces kernel::FlushError as an enum and kernel::FatalError as a std::variant of context-bearing structs, moving string generation and translation into the node layer. It also updates the kernel C API callbacks to pass numeric error codes rather than formatted message strings.

Problem: kernel::Notifications::fatalError() and flushError() take bilingual_str arguments, which pulls util/translation.h and application-level translation machinery into libbitcoinkernel where UI string formatting does not belong.

Evidence of importance:

  • Part of tracking issue #27587 ('Bitcoin Kernel Library Project Tracking'), specifically the task to 'Remove translation.h usage from the kernel'.
  • Commit cdd4757706 notes that std::visit keeps the node's message table exhaustive when rendering kernel::FatalError.

Scope: Changes the C API btck_NotificationInterfaceCallbacks signatures for flush_error and fatal_error by replacing (const char* message, size_t message_len) with enum error codes.

Risk of a mistake: medium (validation). Touches fatal error handling and abort logic across block storage and chainstate validation paths; a bug could mishandle disk write failures or corrupt shutdown behavior.

Changed since the previous assessment: Added node::KernelNotifications::abort() for non-kernel aborts, dropped FailedToStartIndexes from the kernel fatal error set since init.cpp is application code, and structured snapshot coins DB rename failures into a typed CoinsDbRenameFailed struct.

Reviewability

Paused · Waiting on author

Open objections from Yudis-bit, yuvicc with no author reply for 21 days; review would repeat what has been asked.

Waiting on (from the thread read):

  • author: reply to yuvicc's review comment on C API callbacks losing error details 2026-10-09
  • author: respond to Yudis-bit's suggestions on Assume(false) in FatalErrorString and C API error notification format 2026-10-11

Agreement

Mild

objection that needs a change was answered, no reply from the objector (ryanofsky: btck_NotifyFatalError and btck_NotifyFlushError lose all error details because messages are replaced by numeric codes, l); objection that needs an answer has none yet (yuvicc)

Thread notes: Added participant Yudis-bit and two suggestions from c:6107685559 proposing Assume(false) in FatalErrorString and providing both code and formatted message in btck_NotifyFatalError.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
ryanofsky (member)objection, approachHas removing translation.h been discussed somewhere, and what is the overarching strategy for kernel error messages?accepted · author explained background and scope, and ryanofsky apologized for initial reaction and agreed with the PR's directionansweredyes2026-09-24: 'Has this been discussed somewhere? Do we want to have no ability for the kernel to show detailed translated messages when something fails in the kernel'
Status from: 2026-09-25: ryanofsky: 'Thanks, and I want apologize because my last comment was just based on an initial reaction and not very constructive. I '
ryanofsky (member)objection, approachRemoving translation.h provides no significant benefit and replacing it with structural error representations imposes costs on developers and users through code complexity or vague error messages.accepted · author clarified error set size and message preservation in the node; ryanofsky accepted the structured error approach and found the changes reasonableansweredyes2026-09-24: 'I don't see significant benefits to removing it and don't think it should be a goal to remove.'
Status from: 2026-09-25: ryanofsky: 'looking at actual changes in this PR they seem reasonable, and like something that could be built on to fix other proble'
ryanofsky (member)objection, interfacebtck_NotifyFatalError and btck_NotifyFlushError lose all error details because messages are replaced by numeric codes, leaving kernel applications without a clean way to extract and present error information to users.answered · author updated PR description explaining the C API limitationchangedyes2026-09-25: 'The one thing I would change is that it seems like btck_NotifyFatalError and btck_NotifyFlushError are losing all details about errors that occur because messages are being replaced by codes. This see'
Status from: 2026-09-26: arejula27: 'I have updated the PR description, hope it is what you wanted.'
yuvicc (contributor)objection, interfaceReducing fatal error variants to error codes loses error details, preventing C API callback consumers from reconstructing the underlying cause.openansweredno2026-10-09: 'I think with this change this method reduces each variant to an error code losing the error details (block hash, paths, snapshot recovery info). For e.g. `SystemErrorWhileSavingBlock` now reports onl'
Yudis-bit (none)suggestion, safetyAdd Assume(false) before the fallthrough return {} in FatalErrorString in validation.cpp to catch accidental use of the other variants in debug builds.openansweredno2026-10-11: 'Having an Assume(false) before the fallthrough return {}; would catch any accidental use of those variants in FatalError(notifications, state, ...) in debug builds, preventing BlockValidationState::Error() from silently absorbing an empty reject reason.'
Yudis-bit (none)suggestion, interfacePass both the error code and the formatted message in btck_NotifyFatalError, mirroring btck_NotifyWarningSet, to preserve dynamic error details for C API consumers.openansweredno2026-10-11: 'For btck_NotifyFatalError, passing both the code and the formatted message would mirror btck_NotifyWarningSet, which already supplies both btck_Warning warning and (const char* message, size_t message_len). That preserves programmatic dispatch for C consumers without dropping dynamic details like paths or block hashes.'

Support:

  • sedited (member) (Concept ACK, 2026-09-24): Concept ACK :) [not substantive]
  • yuvicc (contributor) (Concept ACK, 2026-10-09): I haven’t looked into this in depth yet. I’ll try to review it more closely alongside https://github.com/bitcoin/bitcoin/pull/35646. Meanwhile, I've left some nits. [not substantive]

Participants:

  • ryanofsky (member, objection, 2 statements 2026-09-24 to 2026-09-25): agreed with structured error approach but objected to C API callbacks losing error message details
  • sedited (member, support, 1 statements 2026-09-24 to 2026-09-24): Concept ACK
  • yuvicc (contributor, objection, 7 statements 2026-10-09 to 2026-10-09): Concept ACK, but objected that C API callbacks lose error details
  • Yudis-bit (none, neutral, 1 statements 2026-10-11 to 2026-10-11): suggested Assume(false) in FatalErrorString and passing both code and formatted message in btck_NotifyFatalError

Files

100 lines under test/bench/ci.

  • src/test/kernel_error_tests.cpp +91/-0
  • src/kernel/error.h +90/-0
  • src/validation.cpp +38/-47
  • src/node/kernel_notifications.cpp +80/-3
  • src/kernel/bitcoinkernel.cpp +40/-4
  • src/kernel/bitcoinkernel.h +37/-4
  • src/kernel/bitcoinkernel_wrapper.h +28/-4
  • src/node/blockstorage.cpp +11/-10
  • src/validation.h +12/-3
  • src/node/kernel_notifications.h +10/-2
  • src/kernel/notifications_interface.h +7/-2
  • src/bitcoin-chainstate.cpp +4/-4
  • src/test/kernel/test_kernel.cpp +4/-4
  • src/init.cpp +1/-2
  • src/test/CMakeLists.txt +1/-0

Card

The short text the ranking pass compares across PRs.

Refactors kernel fatal and flush error notifications to use typed representations instead of translated bilingual_str objects. Defines kernel::FlushError as an enum and kernel::FatalError as a variant of context structs, moving user-facing string construction and translation out of the kernel and into the node application layer. Kernel C API callbacks are modified to emit numeric error codes rather than strings. This is a step toward decoupling libbitcoinkernel from util/translation.h as tracked in #27587.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-12T01:22, confidence high, input hash 70b4fecbd71aa556
thread read through 2026-10-11 (13 statements), head 8a5934f7d2
labels: Validation

Categories

Kernel (libbitcoinkernel)

P2 · unblocks #27587

Replaces string-based error notifications across the kernel boundary with typed data structures to eliminate node translation dependencies. As noted in the description, this directly advances tracking issue #27587 by removing `translation.h` usage from libbitcoinkernel, which represents core boundary decoupling work for the project.

Membership: Modifies src/kernel/ and the C API (src/kernel/bitcoinkernel.cpp) to decouple error notifications from translation machinery.

Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 2