Kernel (libbitcoinkernel): ranking notes

Category notes

The category splits into two clusters that dominate review time: logging isolation (#34775, #33847, #29256, #30342, #36584) and fatal-error propagation across the boundary (#29700, #36326, #35646, #36318). Within each cluster the band differences reflect which PR makes the decision and which follow it. Only two PRs reach P1: the boundary deadlock fix and the storage lock that blocks external readers; both concern whether outside consumers can safely use the library at all.

Review order and overlapping PRs

  • Direction conflict: #34775 restricts libbitcoinkernel to a single global logging connection and moves level config onto that handle, while #33847/#30342 attach connections to contexts with per-connection levels and per-context routing. These cannot both land as written; review #34775 first (it fixes a concrete deadlock) but settle the one-connection-vs-per-context question before merging either side.
  • Logging chain order matters: #29256 (context-aware log macros) and #33847 (connection/context API hooks) are both prerequisites of #30342 (per-context delivery); #33847 is explicitly framed as avoiding a later breaking API change, so it should be decided before downstream consumers and bindings adapt.
  • Three overlapping takes on fatal-error reporting across the kernel boundary: #29700 (util::Result/FlushResult returns), #36326 (typed FatalError/FlushError notifications, removes translation.h), and #35646 (RFC moving runtime errors out of BlockValidationState via util::Expected). They redefine the same error surface; review #29700 first as the broadest restructuring, then #36326, and treat #35646 as the concept decision that may redirect both.
  • #36318 (BLOCK_INCONCLUSIVE / always-set invalid result) and #35646 both change ValidationState semantics exposed through the C API; coordinate so the kernel error enum is not reshaped twice.
  • #32427 is as much a block-storage/validation change as a kernel change (new WAL format, checksums, flush semantics); per the category note it likely needs splitting or dual review with validation reviewers.
  • #36353 is a documentation-only mitigation for a real correctness hazard (btck_Block caching fChecked across different consensus parameters); a reviewer should decide whether the API should instead prevent the bypass, which would supersede the doc patch.

Band and position changes

  • #34775: P2 alone, P1 after comparison. Raised to P1 and moved up five places: a deadlock risk when consumer callbacks log is a reliability defect exactly at the kernel boundary, and the single-connection decision also gates the per-context logging series.
  • #32427: P2 alone, P1 after comparison. Raised to P1: LevelDB's exclusive process lock is the concrete reason external consumers (Electrs/Esplora) cannot read block/header data from a running node, which is the category's defining use case.
  • #29256: Moved up five places: it is the macro-level prerequisite for both #33847 and #30342, so it inherits their importance and must be reviewed before them.
  • #36353: Moved up two bands' worth of positions relative to its score because it warns of a silent validation bypass (cached fChecked across parameter sets), a correctness hazard for consumers rather than ordinary doc polish.
  • #33820: P3 alone, P4 after comparison. Lowered to P4: removing two redundant accessors is cosmetic API polish with no consumer asking for it, which the category explicitly places at P3 or below; it is the weakest claim on review time here.

About

This pass by claude-opus-5 on 2026-11-01 saw every PR in the category at once and checked the bands given to each PR alone against each other, ordered the PRs, and noted chains and overlaps. Back to the category.