Utilities (logging, arguments, libraries): ranking notes
Category notes
P1 here is reserved for defects in shared primitives with node-wide consequences: silent block-data destruction, uncatchable aborts, and out-of-bounds reads. P2 covers the two foundational infrastructure chains (typed settings/args, util::Result, logging macros) plus measured cryptographic performance and user-facing configuration fixes. The P3 band is large (37 PRs) because this category legitimately collects many self-contained internal improvements of similar, modest consequence; within it I ordered correctness and secret-hygiene fixes in primitives ahead of measured performance work, then platform and logging ergonomics, then pure refactors and test-only changes.
Review order and overlapping PRs
- Settings/args chain: #31260 and #16545 both present themselves as the foundation for typed setting validation (#31260 names #16545 as a consumer, #16545 names #17580/#17783/#22978). Decide which base lands first; then #16545 -> #17580 -> #17493 -> #17581 and #16545 -> #17783 must be reviewed in that order. #17581's whole premise is void unless #17493 merges.
- Error-handling chain: #25665 is the base for #26022, #25722 and #29700; #25722 additionally needs #26022. Reviewing the dependents before #25665 wastes effort.
- Logging chain: #34778 -> #29256 -> #30342 (-> #30343). #34778 was assessed P3 while its dependent #29256 was P2; I raised #34778 so a base does not sit below the work it unblocks.
- #34038 and #35387 are competing solutions to the same problem (-loglevel silently doing nothing without -debug, plus runtime trace control via RPC). They cannot both merge; review #34038 first since #35355 and #34730 are built on it, then decide whether #35387's RPC design should replace parts of it.
- #35322 and #35355 independently replace the mutex-protected log-level map with atomic category bitmasks; this is duplicated work. Pick one implementation before reviewing the other in detail.
- #36537 and #36550 are the same secret-cleansing effort on crypto primitives by the same author (#36550 supersedes #36551); review them as one unit to keep destructor conventions consistent.
- #36105 fixes Windows wrapper exec and #36190 only adds tests for that fix; #36105 must be reviewed first.
- #36049 (streams) and #36347 (-dbcache in settings.json) are both uncatchable std::terminate aborts caused by exceptions crossing a noexcept/unwinding boundary; a reviewer looking at one should check whether the other pattern recurs elsewhere.
- #29491 and #36122 both vendor unmerged experimental libsecp256k1 modules; subtree policy for experimental upstream code should be settled once rather than per-PR.
- #26966 and #36603 each introduce a general concurrency primitive (thread pool, blocking queue); #26966 also notes overlap with #34489. Worth coordinating so the tree does not gain two parallel task-queue utilities.
Band and position changes
- #35524: P2 alone, P1 after comparison. Raised to P1 and moved up: silent destruction of existing block data during reindex on non-posix_fallocate platforms is the most consequential correctness problem on this list.
- #36049: P2 alone, P1 after comparison. Raised to P1: an uncatchable std::terminate on buffered write failure is a reliability/data-safety failure observed in practice, not a routine bug fix.
- #35281: P2 alone, P1 after comparison. Raised to P1: out-of-bounds reads in the shared JSON tokenizer are a memory-safety defect in a core parsing primitive, which this category ranks first.
- #25665: Base of the util::Result chain (#26022, #25722, #29700) but placed after the three P1 safety fixes and the settings foundations.
- #35831: Moved up: silently swallowed trailing options are a user-facing parsing bug of the same class as the other ArgsManager P2s.
- #36409: Defense-in-depth hardening ranks just below measured perf wins and user-visible config bugs within P2.
- #32966: Large feature, but its utils-relevant content is secp256k1 primitives still pending upstream, so it sits mid-band.
- #29491: Kept P2 for the validation speedup, but placed below shipped-primitive work since it rests on unmerged experimental upstream modules.
- #34778: P3 alone, P2 after comparison. Raised from P3 to P2: it is the base of #29256 and #30342 and cannot sit below the logging work it unblocks.
- #29256: Placed directly after its base #34778 so the chain is reviewed in order.
- #17493: Kept P2 but ordered after its base #17580 so the stacked args chain is reviewed in dependency order.
- #36549: Moved up: wrong allocator accounting distorts mempool and cache limits on every non-glibc platform, the broadest correctness issue in the P3 band.
- #36234: Moved up: premature rotation breaks the capacity guarantee of a widely used primitive.
- #36677: Moved up: descriptor leaks into spawned signer/notification processes are a security-hygiene issue in a shared file primitive.
- #35688: Moved up: undefined behaviour in HMAC init belongs with the other crypto-primitive correctness fixes.
- #26966: Moved up: a 7h-to-1h initial index build is the largest user-visible performance claim in this band.
- #36603: Moved up: measurable snapshot-load speedup plus a reusable concurrency primitive that #35054 builds on.
- #31868: Grouped with the other measured performance work rather than with refactors.
- #34603: Moved up: a supported-platform filesystem defect ranks above internal refactors.
- #34038: Lowered within band: the -loglevel confusion is real but competing proposal #35387 means a decision, not just review, is needed.
- #35387: Placed next to #34038 because both solve the same trace-logging configuration problem and must be decided together.
- #35355: Moved down to sit with #34038, on which it depends, and with the overlapping #35322.
- #35322: Moved down and grouped with #35355; both convert log-level state to atomic bitmasks.
- #26022: Moved down: pure error-handling ergonomics rank below correctness and performance work here, but kept ahead of its dependent #25722.
- #25722: Moved down: a behaviour-preserving refactor of wallet error plumbing, reviewable only after #25665 and #26022.
- #29700: Moved down: a pure refactor whose payoff is kernel decoupling rather than utils-side benefit.
- #36442: Moved up: it unblocks #36443 and restores round-tripping of multipath descriptor strings.
- #27260: Moved up: misleading cross-network address errors are a genuine user-facing confusion, not just an interface cleanup.
- #31672: Moved up slightly: new cross-platform thread CPU timing is reusable and feeds peer-prioritization work.
- #36122: Moved down: an explicitly draft reference implementation accompanying unmerged BIPs and unmerged secp256k1 modules.
- #34730: Moved down: cosmetic log-level consolidation that is gated on the #34038 decision.
- #33646: Moved down: the card itself notes close failures are rare, making this the least consequential logging fix.
- #35170: Moved down: test-only hardening with no active bug, below the behaviour-affecting fixes in this band.
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.