(+) #35524 winterrdog validation: Avoid rewriting the genesis block during index recovery P1 · bug fix Needs rebase 0 Crickets 40 Prevent reindexing from silently destroying block data on systems without native posix_fallocate Avoid rewriting the genesis block and zeroing subsequent blocks during block index recovery P1 after comparing with the other PRs here (assessed alone as P2): 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. P2 because preallocating within existing file boundaries could overwrite or truncate existing data Fixes zero-fill fallback and Windows file allocation to preserve existing file contents No review verdicts recorded No substantive review comments either way yet Suggestion to widen offset type for safety was adopted (thomasbuilds) Author tested an alternative approach and found it did not fix the bug alone (sedited) 40 lines added or modified outside tests 0 lines added or modified in tests 19 lines removed in total 2 files, 2 commits (+) #36049 l0rinc streams: avoid termination on buffered write failure P1 · bug fix Waiting on author 1 Strong 19 + 36 tests Prevent the node from abruptly crashing when disk writes fail during block storage Enable clean error reporting and graceful shutdown instead of sudden process aborts P1 after comparing with the other PRs here (assessed alone as P2): 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. P2 because it stops stream write failures from triggering an uncatchable process abort Allows callers to safely catch and handle write errors during buffered I/O Reviewable immediately with nothing known that would invalidate a review Support from a contributor who ran into the same bug while working in this area (w0xlt) Asked why storage write errors should be caught instead of crashing, answered by author (andrewtoth) Suggested finding a wholesale pattern or linter check for destructor exceptions (maflcko) 19 lines added or modified outside tests 36 lines added or modified in tests 5 lines removed in total 3 files, 3 commits (+) #35281 ferminquant univalue: respect token end pointer P1 · bug fix Ready 0 (+1) Blocked 31 + 3 tests Prevent out-of-bounds reads when parsing JSON inputs that lack null termination Avoid potential node crashes when processing truncated or bounded JSON requests P1 after comparing with the other PRs here (assessed alone as P2): 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. P2 because it fixes memory-safety flaws in core JSON token parsing primitives Ensuring parsers respect explicit buffer boundaries provides durable value across the codebase Unaddressed objection: tests fail to properly isolate bounded inputs with trailing data (maflcko) Concept approval because it fixes two separate out-of-bounds reads (sedited) Verified by testing added coverage for fractions and exponents (carloantinarella) 31 lines added or modified outside tests 3 lines added or modified in tests 30 lines removed in total 10 files, 1 commits (+) #16545 ryanofsky refactor: Implement missing error checking for ArgsManager flags P2 · unblocks #17580 Ready 0 (+1) -1 Mild 308 + 542 tests Validate command-line argument types and reject duplicate non-list config settings during startup Prevent configuration parsing footguns and silent bugs caused by unexpected type coercion P2 because it unblocks multiple pull requests migrating node options to strict typed flags Prevents argument footguns like bad type coercion and silently ignored duplicate configuration settings General support for typed argument validation, but one answered objection awaits reviewer response Unaddressed objection: resetting empty values to defaults alters existing configuration semantics (ajtowns) Support with local testing (hebasto, promag) Concept approval for incremental argument typing and cleaner error handling (hodlinator, laanwj, l0rinc) 308 lines added or modified outside tests 542 lines added or modified in tests 69 lines removed in total 9 files, 6 commits (+) #31260 ryanofsky scripted-diff: Type-safe settings retrieval P2 · cleanup Ready 0 Mild 2,845 + 882 tests Prevent typos and type mismatches in configuration settings across the codebase at compile time Keep setting help text and fallback defaults consistent across registration and retrieval sites P2 because it prevents silent configuration bugs repository-wide at compile time P2 because it unblocks planned runtime setting validation work Broad concept support for compile-time checking of settings (l0rinc, hodlinator, jeanpablojp) Unaddressed objection: help text omitted for BlockFilterIndexSetting (jeanpablojp) Unaddressed objection: nulls in settings.json dropped instead of erroring (jeanpablojp) Unaddressed objection: check-doc linter misses some setting calls (jeanpablojp) Open suggestion: remove GetPtr() now that dependency merged (hodlinator) 2,845 lines added or modified outside tests 882 lines added or modified in tests 1,122 lines removed in total 100 files, 12 commits (+) #27409 ryanofsky Make GUI and CLI tools use the same datadir P2 · bug fix Author silent 383d 0 Blocked 455 + 10 tests Ensure GUI and CLI tools default to the same datadir Prevent CLI tools from silently using the wrong datadir, re-downloading blocks, or missing wallets P2 because it prevents silent datadir divergence, duplicate block syncs, and missing wallets Fixes a configuration flaw where CLI tools ignore custom datadirs selected via the GUI Review #27302 first Wait for author update: needs rebase, author noted PR is in-progress, and author has been silent for 349 days No review verdicts recorded Unaddressed objection: Windows shell CSIDL retrieval fails in functional tests (pinheadmz) Clarified concern about bitcoin.conf placement in GUI-selected datadir (luke-jr) 455 lines added or modified outside tests 10 lines added or modified in tests 103 lines removed in total 13 files, 3 commits (+) #25665 ryanofsky refactor: Add util::Result failure types and ability to merge result values P2 · unblocks #25722 Ready 0 (+8) Strong 551 + 211 tests Allow functions to return structured failure codes and multiple messages without ad-hoc tuples Enable callers to distinguish between error types like recoverable corruption versus fatal errors P2 because it provides shared error-handling infrastructure that replaces ad-hoc tuples and out-parameters Directly unblocks downstream error-handling cleanups across wallet and kernel functions Ranking pass: Base of the util::Result chain (#26022 #25722 #29700 Ready to review Nothing known that would invalidate a review now Substantive support across multiple revisions (hernanmarino, stickies-v, sedited, laanwj, achow101) Verified by code review and testing (hernanmarino) Open minor suggestion to drop an obsolete CI workaround commit (maflcko) Several participants suggest closing and reopening in a fresh PR due to thread length (maflcko, sedited) 551 lines added or modified outside tests 211 lines added or modified in tests 170 lines removed in total 14 files, 7 commits (+) #36188 furszy crypto: plug hardware optimized SHA256 into libsecp256k1's context P2 · speedup Ready 2 Strong 224 + 237 tests Speed up elliptic curve signing by enabling hardware-accelerated SHA256 in libsecp256k1 Brings signing performance gains to users running on CPUs with dedicated SHA extensions P2 because it speeds up cryptographic signing operations on CPUs with hardware SHA extensions ECDSA signing drops by roughly 13% to 15% and Schnorr signing drops by 3% to 7% Verification gains are negligible, keeping the performance benefits focused on signing workloads Ready for review; no known blockers or pending changes to invalidate review Broad concept support across several contributors and maintainers Verified by differential testing across 20,000 cryptographic operation rounds (fametrano) Reviewed and approved after addressing context separation and startup questions (theStack, w0xlt) Unanswered suggestion to clone the signing context to avoid a second self-test (fametrano) 224 lines added or modified outside tests 237 lines added or modified in tests 150 lines removed in total 44 files, 11 commits (+) #34083 theuni Add initial vectorized chacha20 implementation for 2-3x speedup P2 · speedup Waiting on author 0 Blocked 434 Relieve network thread CPU bottlenecks caused by ChaCha20 encryption during BIP324 v2 transport Speed up data stream encryption and decryption by 2-3x during high-bandwidth peer-to-peer activity P2 because ChaCha20 encryption consumes substantial network thread CPU time during BIP324 transport Delivers a 2x speedup on x86-64 and 3x on ARM NEON for core cryptographic primitives Relieves processing bottlenecks during initial block download and high-bandwidth peer sync Waiting on author to address open objections and CI lint failures before further review is worthwhile Support because benchmark results show good cryptographic speedups (ajtowns) Concept approval without stated reasons (jonatack) Unaddressed objection: circular include dependency fails CI linting (l0rinc) Unaddressed objection: 7 percent slowdown on 64-byte inputs and slower IBD on Umbrel (l0rinc) Design questions remain open regarding SIMD vector layout and block processing (sedited) 434 lines added or modified outside tests 0 lines added or modified in tests 5 lines removed in total 5 files, 3 commits (+) #35831 pablomartin4btc argsman, cli: Allow options after non-option arguments (GNU-style) P2 · bug fix Author silent 80d 1 Positive 103 + 182 tests Parse command-line options that appear after commands or positional arguments Prevent trailing options like -rpcwallet from being silently swallowed or misparsed P2 because it fixes a shared footgun where trailing options were silently ignored Adds standard '--' argument termination across common parsing logic Ranking pass: Moved up: silently swallowed trailing options are a user-facing parsing bug of the same class as the other ArgsManager P2s. Support without stated reasons (w0xlt) Previous objections on parsing edge cases and bitcoin-tx handling were fixed (w0xlt, vicjuma) 103 lines added or modified outside tests 182 lines added or modified in tests 38 lines removed in total 7 files, 3 commits (+) #17783 ryanofsky common: Disallow calling IsArgSet() on ALLOW_LIST options P2 · bug fix Review #16545 0 Positive 392 + 793 tests Prevent subtle configuration and networking bugs caused by negated list options Ensure disabled list arguments are not mistakenly treated as active during node startup P2 because it eliminates an error-prone argument check that causes subtle node initialization bugs Fixes improper startup failures when combining negated networking flags with other options Concept approval without stated reasons (promag) No objections raised 392 lines added or modified outside tests 793 lines added or modified in tests 186 lines removed in total 14 files, 17 commits (+) #36409 fametrano sign: use fresh randomness as BIP340 auxiliary data P2 · fund safety Ready 1 Strong 6 + 11 tests Add defense-in-depth protection against side-channel and fault attacks during Taproot signing Comply with BIP340 recommendations by supplying fresh randomness as auxiliary data P2 because it hardens Taproot signing against side-channel leakage and fault injection attacks Provides durable security hardening for wallet key signing operations per BIP340 guidance Ranking pass: Defense-in-depth hardening ranks just below measured perf wins and user-visible config bugs within P2. Strong support with no open objections (Yudis-bit) Supports fresh randomness for side-channel defense while keeping signing fast (Yudis-bit) Verified test vector reproduction and deterministic fuzzing with optional fixed data (Yudis-bit) 6 lines added or modified outside tests 11 lines added or modified in tests 4 lines removed in total 4 files, 1 commits (+) #32966 Eunovo Silent Payments: Receiving P2 · new feature Needs rebase 0 Neutral 15,289 + 3,665 tests Add BIP 352 Silent Payments receiving capabilities to the Bitcoin Core wallet Allow users to share a single static address without sacrificing privacy from address reuse P2 because it adds extensive BIP 352 cryptographic primitives to the libsecp256k1 module Provides shared elliptic curve calculations and test vectors used across silent payments Ranking pass: Large feature, but its utils-relevant content is secp256k1 primitives still pending upstream, so it sits mid-band. Review the logic now as the required rebase is mechanical No review verdicts recorded Prior objections are largely settled, but no reviewers have formally endorsed the PR yet Agreed to fix missed block sync payments and fragile test checksums on rebase (bitsagarob, rkrux) Addressed earlier issues with change outputs, block filters, and immutable wallet flags (Sjors) Scoped key tweaking methods specifically to BIP 352 to avoid generic footguns (josibake) 15,289 lines added or modified outside tests 3,665 lines added or modified in tests 165 lines removed in total 94 files, 41 commits (+) #29491 fjahr [EXPERIMENTAL] Schnorr batch verification for blocks P2 · speedup CI failing 0 Strong 2,452 + 276 tests Speed up block validation and initial sync by batch verifying Taproot Schnorr signatures Reduce CPU overhead during block connection by verifying signature cache misses simultaneously P2 because batch verification in libsecp256k1 delivers up to 2x or greater speedups for Schnorr signatures Adds unmerged cryptographic primitives that require careful algorithmic verification Ranking pass: Kept P2 for the validation speedup, but placed below shipped-primitive work since it rests on unmerged experimental upstream modules. CI is failing, so verify whether the failure is related to this PR before doing a deep review No review verdicts recorded Substantive support after earlier performance and lock contention issues were resolved (Sjors, Eunovo) Support highlighted speedups on Taproot blocks and batch tweak verification readiness (Sjors, Eunovo) Open request to document how batch verification and signature collection works (Sjors) 2,452 lines added or modified outside tests 276 lines added or modified in tests 172 lines removed in total 56 files, 11 commits (+) #36536 fametrano log: escape C1 control characters in messages P2 · bug fix Ready 3 Strong 7 + 6 tests Prevent untrusted log output from driving or corrupting terminal displays Protect node operators viewing logs in VTE-based terminals from control sequence injection P2 because it stops untrusted log content from injecting terminal control sequences into VTE terminals Prevents terminal display corruption or escape sequence manipulation when viewing logs Strong support with substantive testing on Linux x86_64 (cyb3ralbert) Verified that C1 characters in paths are escaped in debug.log and unit tests pass (cyb3ralbert) Minor test and comment suggestions accepted by author if a re-push is requested (cyb3ralbert) Approval without stated reasons (jpk68) 7 lines added or modified outside tests 6 lines added or modified in tests 4 lines removed in total 2 files, 1 commits (+) #34778 ryanofsky logging: rewrite macros to enforce restrictions at compile-time, improve efficiency and usability P2 · cleanup Ready 0 -1 Blocked 164 + 116 tests Clean up logging macros and enforce correct category arguments at compile time with clear error messages Skip message formatting overhead when logging is fully disabled, benefiting fuzzing and kernel uses P2 after comparing with the other PRs here (assessed alone as P3): Raised from P3 to P2: it is the base of #29256 #30342 P3 because it improves macro maintainability and catches incorrect logging usage at compile time Eliminates unnecessary string formatting overhead when logging is completely turned off Concept objection to reintroducing generic LogPrint conflicting with existing work (ajtowns) Objection that exposing LogPrint risks broken rate limiting and hurts greppability (maflcko) Unaddressed objection: invalid LogDebug flags compile but trigger runtime assertion aborts (cyb3ralbert) Unanswered question on whether omitting category prefixes in -loglevelalways is intentional (cyb3ralbert) 164 lines added or modified outside tests 116 lines added or modified in tests 125 lines removed in total 7 files, 6 commits (+) #29256 ryanofsky log, refactor: Allow log macros to accept context arguments P2 · new feature Review #34778 0 -2 Disputed 217 + 207 tests Let logging macros target specific logger instances, custom formatters, and categories Allow kernel library users to isolate log output instead of forcing all callers into one global stream Skip unnecessary formatting work when log messages are disabled P2 because it upgrades node-wide logging infrastructure with context arguments and compile checks Avoids formatting overhead across the node when logging is disabled Ranking pass: Placed directly after its base #34778 Disputed with an active Concept NACK against instanced logging and context arguments (ajtowns) Thinks multi-instance logging adds unnecessary complexity and call-site churn without benefit (ajtowns) Concept support to enable separate kernel log instances like parallel chainstate managers (sedited) Concept support for separating logging facility from severity (vasild, jonatack) Requested changes to split commits were answered by author and await re-review (l0rinc) 217 lines added or modified outside tests 207 lines added or modified in tests 127 lines removed in total 7 files, 10 commits (+) #17580 ryanofsky refactor: Add ALLOW_LIST flags and enforce usage in CheckArgFlags P2 · cleanup Review #16545 0 Strong 378 + 767 tests Prevent subtle argument parsing bugs where multi-value list options are mistakenly accessed as single values Fix inconsistent handling of empty arguments like -blockfilterindex so they are rejected reliably P2 because it eliminates error-prone getter usage on list arguments and fixes parsing anomalies It establishes argument checking infrastructure that unblocks follow-up config validation work Review #16545 PR is stacked on unmerged #16545 Concept approval with review feedback on runtime versus compile-time checks (ajtowns) Concept approval without stated reasons (hebasto, promag) All review suggestions and questions have been addressed or resolved 378 lines added or modified outside tests 767 lines added or modified in tests 176 lines removed in total 14 files, 13 commits (+) #17493 ryanofsky util: Forbid ambiguous multiple assignments in config file P2 · bug fix Review #16545 0 -1 Mild 389 + 808 tests Fail on startup when a single-value setting is assigned multiple times in a config file Prevent node operators from unknowingly running with silently discarded configuration settings P2 because node operators avoid running with silently discarded configuration settings It unblocks downstream cleanup of reverse precedence handling in settings merging Ranking pass: Kept P2 but ordered after its base #17580 Concept approval because intentionally repeatable settings get a new flag (JeremyRubin) Concern that existing configs in the wild will cause nodes to fail to restart on upgrade (ajtowns) Approval from an account with no project history and no stated reason (Khailand69) 389 lines added or modified outside tests 808 lines added or modified in tests 182 lines removed in total 15 files, 16 commits (+) #36549 sipa WIP: improve MallocUsage and test in CI P3 · bug fix Ready 0 Crickets 94 + 296 tests Accurately calculate dynamic memory usage across different operating systems and allocators Prevents cache and mempool limits from relying on outdated glibc allocation assumptions P3 because it fixes outdated memory accounting heuristics that overestimated allocation sizes Ensures mempool and cache sizing track real memory across glibc, musl, macOS, and Windows Ranking pass: Moved up: wrong allocator accounting distorts mempool and cache limits on every non-glibc platform, the broadest correctness issue in the P3 band. No review verdicts recorded No reviews or comments yet No substantive discussion has taken place on the pull request 94 lines added or modified outside tests 296 lines added or modified in tests 21 lines removed in total 6 files, 4 commits (+) #36234 JeremyRubin util: avoid redundant rolling Bloom insertions P3 · bug fix Ready 0 Crickets 22 + 52 tests Prevent rolling Bloom filters from prematurely expiring unrelated entries when duplicate items are inserted Preserve configured capacity guarantees when items are announced repeatedly P3 because repeated insertions prematurely rotate the filter and expire unrelated entries before capacity Fixes an edge case where duplicate entries degrade filter retention guarantees Ranking pass: Moved up: premature rotation breaks the capacity guarantee of a widely used primitive. No review verdicts recorded No substantive reviews yet Asked whether this targets worrying real-world usage patterns or general correctness (instagibbs) Author demonstrated that repeated announcements can trivially wipe the known-transaction filter 22 lines added or modified outside tests 52 lines added or modified in tests 3 lines removed in total 3 files, 1 commits (+) #36677 SirECMG util: Make files opened with fdbridge::fopen noninheritable P3 · bug fix Ready 0 Strong 47 + 41 tests Prevent file descriptor leaks into child processes like external signers or notification scripts Ensure files opened across platforms are marked non-inheritable by default P3 because it hardens shared file opening primitives against descriptor leaks into child processes Worthwhile security hygiene for external tool execution but deferrable without active failures Ranking pass: Moved up: descriptor leaks into spawned signer/notification processes are a security-hygiene issue in a shared file primitive. No review verdicts recorded Substantive support with all prior objections resolved Verified on Windows that opened files lack inheritance flags (ValeraFinebits) Initial race condition concern in multithreaded file opening was addressed (hebasto) 47 lines added or modified outside tests 41 lines added or modified in tests 4 lines removed in total 2 files, 2 commits (+) #35688 l0rinc crypto: accept empty HMAC keys P3 · bug fix Author silent 115d 1 Positive 4 + 5 tests Avoid undefined behavior and UBSan warnings when initializing HMAC with empty keys Remove defensive workarounds in fuzz targets for empty byte vectors P3 because it fixes minor undefined behavior in crypto utilities without direct user-facing impact Resolves UBSan warnings on zero-length keys and cleans up defensive checks in fuzz targets Ranking pass: Moved up: undefined behaviour in HMAC init belongs with the other crypto-primitive correctness fixes. Author has been silent for 77 days, so responses to review feedback may be delayed Concept approval without stated reasons (sedited) Resolved concern on memcpy semantics and safety by switching to std::copy (maflcko) 4 lines added or modified outside tests 5 lines added or modified in tests 22 lines removed in total 5 files, 2 commits (+) #26966 furszy index: initial sync speedup, parallelize process P3 · new feature Author silent 272d 0 (+1) -1 Disputed 569 + 462 tests Speed up initial index synchronization for block filter and transaction indexes Allow node runners to process block batches across multiple worker threads during initial sync P3 because it introduces a generic thread pool utility that other node subsystems can reuse Serves as a helpful concurrent building block rather than solving an urgent deficiency Ranking pass: Moved up: a 7h-to-1h initial index build is the largest user-visible performance claim in this band. Needs rebase Author is splitting out the thread pool into #33689 before updating this PR Verified significant block filter index speedups on SSD (Sjors, pinheadmz, mzumsande) Unaddressed objection: txindex is slower on SSD and parallel sync adds too much complexity (l0rinc) Unaddressed objection: LevelDB write contention slows txindex sync on SSD by ~7% (andrewtoth) Concept and approach approval for the parallel processing design (ryanofsky, sedited, w0xlt) Unanswered question on how parallel indexing interacts with AssumeUTXO (maflcko) 569 lines added or modified outside tests 462 lines added or modified in tests 153 lines removed in total 14 files, 11 commits (+) #36347 0xShadowX node: don't abort on invalid -dbcache in settings.json P3 · bug fix Ready 2 Strong 2 + 12 tests Prevent node crashes at startup when settings.json contains an invalid dbcache value Ensure the node exits with a clear error message instead of aborting P3 because it fixes an unexpected startup crash from malformed configuration values Node operators receive a helpful error message instead of an abrupt termination Support with testing confirming the node exits with an error instead of aborting (8144225309) Suggested adding a test case for valid integer settings, which the author added (Hamza1610) Additional approval lacked rationale from an account with no project history (Hamza1610) 2 lines added or modified outside tests 12 lines added or modified in tests 2 lines removed in total 3 files, 1 commits (+) #36717 Snan0M init: reject -port=65535 when the onion port has to be derived from it P3 · bug fix Ready 0 Crickets 14 + 11 tests Prevent integer overflow on startup when deriving the default Tor onion service port from -port=65535 Stop the node from unintentionally binding the onion service to an arbitrary ephemeral port P3 because it fixes an integer overflow bug in startup argument validation Prevents unintended binding to arbitrary OS-assigned ports in an edge case configuration No review verdicts recorded No substantive discussion or review comments yet No support or objections recorded 14 lines added or modified outside tests 11 lines added or modified in tests 4 lines removed in total 2 files, 1 commits (+) #36603 ajtowns Improve loadtxoutset performance P3 · new feature Ready 0 Mild 1,110 + 213 tests Speed up assumeutxo snapshot loading during node startup Reduce memory overhead and redundant disk flushes when importing UTXO snapshots P3 because it introduces a general thread-safe blocking queue primitive Provides a reusable concurrency utility for the codebase beyond snapshot loading Ranking pass: Moved up: measurable snapshot-load speedup plus a reusable concurrency primitive that #35054 No review verdicts recorded Unaddressed objection: adds assumeutxo complexity to consensus paths (l0rinc) Questioned whether speeding up snapshot loading justifies code complexity (l0rinc) Author defended the approach for fast node startup with no further reply yet (l0rinc) 1,110 lines added or modified outside tests 213 lines added or modified in tests 472 lines removed in total 27 files, 12 commits (+) #31868 l0rinc [IBD] specialize block serialization P3 · speedup Author silent 124d 0 Mild 196 + 38 tests Speed up block serialization and Initial Block Download by reducing overhead in primitive serialization Avoid unnecessary buffering and endianness conversions during single-byte writes and size computation P3 because microbenchmarks show 10-25% faster block serialization latency and a modest ~2% IBD gain Practical impact may be limited because production stream writes are already buffered Ranking pass: Grouped with the other measured performance work rather than with refactors. Needs rebase Author silent for 86 days No review verdicts recorded Unaddressed objection: single-byte writes in sha256 add complexity without clear benefit (sedited) Questioned whether existing stream buffering already negates the real-world IBD speedup (maflcko) Support for faster block serialization, noting potential value for future kernel users (sedited, theuni) 196 lines added or modified outside tests 38 lines added or modified in tests 98 lines removed in total 11 files, 5 commits (+) #35763 kevkevinpal util: write JSON atomically in WriteJson P3 · bug fix Ready 1 (+1) Strong 28 + 45 tests Prevent banlist and settings files from being corrupted or truncated if a crash occurs during a write Ensure settings and ban updates are replaced atomically rather than written directly in place P3 because atomic updates prevent corrupted or empty banlist files following unexpected shutdowns The benefit is limited to protecting local state during rare crash or write-interruption scenarios Strong support with all earlier concerns on API safety, temp files, and tests resolved Tested on FreeBSD and approved the self-contained atomic write implementation (winterrdog) Approved noting that the new test coverage is useful (Herb-ops) 28 lines added or modified outside tests 45 lines added or modified in tests 24 lines removed in total 7 files, 1 commits (+) #34603 achow101 wallet: Fix detection of symlinks on Windows P3 · platform fix Author silent 159d 0 Strong 30 + 27 tests Detect symlinks properly on Windows builds Prevents wallets from unintentionally traversing symlinked directories or misclassifying symlinks P3 because it provides a platform-specific filesystem helper to detect symlinks on Windows It acts as a straightforward workaround for GCC libstdc++ limitations primarily serving wallet checks Ranking pass: Moved up: a supported-platform filesystem defect ranks above internal refactors. Author has been silent for 121 days so reviews may wait long for a response Concept approval with testing across MinGW, MSVC, and clang-cl on Windows 11 (molnard) Approval without detailed rationale (sedited) Resolved objection regarding redundant filesystem checks (luke-jr) 30 lines added or modified outside tests 27 lines added or modified in tests 40 lines removed in total 5 files, 4 commits (+) #36105 ryanofsky bitcoin wrapper: Fix Windows exec so wrapper waits for child process P3 · platform fix Ready 1 Strong 15 + 14 tests Make the Windows bitcoin wrapper wait for child processes and return their exit codes Allow scripts, external callers, and test suites to reliably check child process outcomes Re-enable skipped functional tests on Windows P3 because the process execution and error category fixes in the utility helper are low leverage The utility is currently only used by the wrapper executable Tested on Linux and Windows with the MSVCRT workaround dropped (cyb3ralbert) Concept approval without stated reasons (hebasto, hodlinator) All discussion questions and suggestions addressed 15 lines added or modified outside tests 14 lines added or modified in tests 21 lines removed in total 7 files, 3 commits (+) #34038 ajtowns logging: replace -loglevel with -trace, expose trace logging via RPC P3 · user request Ready 0 (+2) -1 Disputed 172 + 94 tests Fix silent failures when configuring trace logging and let node operators manage trace logs via RPC P3 because it prevents -loglevel silently failing when -debug is omitted P3 because confusing logging options degrade ergonomics but are noncritical and easily deferrable Ranking pass: Lowered within band: the -loglevel confusion is real but competing proposal #35387 Approach objection: three interacting flags are confusing and break RPC compatibility (ryanofsky) Approach objection: setting trace as default while keeping specific categories at debug is impossible (ryanofsky) Prefers alternative proposal in #35387 Supports parallel flags as an easier startup interface despite unwieldy RPC interactions (stickies-v) Tested and supports parallel flag design (sedited) 172 lines added or modified outside tests 94 lines added or modified in tests 133 lines removed in total 16 files, 3 commits (+) #35387 ryanofsky logging: make trace logging easily usable P3 · new feature Waiting on author 0 Mild 188 + 102 tests Allow loglevel configuration standalone without requiring paired debug flags to take effect Inspect and adjust category log levels at runtime via RPC without restarting the node P3 because decoupling loglevel from debug flags simplifies configuration Aids live node debugging by letting operators change log verbosity on the fly Ranking pass: Placed next to #34038 Author silent for 93 days, so reviews may wait a long time for a response No review verdicts recorded Support for an elegant approach and intuitive interface (stickies-v) Nonblocking objection: maintaining two parallel active logging RPC methods adds maintenance burden (stickies-v) Unaddressed question asking if breaking debug interactions was intended (cyb3ralbert) 188 lines added or modified outside tests 102 lines added or modified in tests 41 lines removed in total 11 files, 2 commits (+) #35355 ajtowns Use atomics for determining whether trace logging is enabled P3 · speedup Review #34038 0 Crickets 297 + 116 tests Eliminate lock and lookup overhead when debug or trace logging is disabled Speed up hot multi-threaded execution paths across the node Simplify how operators configure trace logging categories P3 because it removes mutex overhead for disabled log checks on hot paths Performance gains are incremental and mostly relevant under high concurrency Ranking pass: Moved down to sit with #34038 #35322 No review verdicts recorded No reviews or comments yet 297 lines added or modified outside tests 116 lines added or modified in tests 347 lines removed in total 24 files, 14 commits (+) #35322 ryanofsky logging: streamline Logger state and drop redundant methods P3 · cleanup Ready 0 Neutral 87 + 93 tests Simplify internal logging state and eliminate mutex locks during log checks Make configuring and inspecting log levels faster and less error-prone P3 because it simplifies internal logging state and removes locking overhead during log checks The improvements are internal code cleanup with modest performance benefit for logging Ranking pass: Moved down and grouped with #35355 No review verdicts recorded No reviewers have formally endorsed the changes yet Prior objections on commit structure and category behavior changes were addressed (l0rinc) Awaiting follow-up review on the updated commits and tests (l0rinc) 87 lines added or modified outside tests 93 lines added or modified in tests 185 lines removed in total 11 files, 13 commits (+) #30342 ryanofsky kernel, logging: Deliver each context's log output to its own logging connection P3 · cleanup Review #29256 0 -1 Mild 832 + 437 tests Prevent interleaved log streams when running multiple chainstate contexts or kernel library instances Allow kernel consumers and multi-context tests to cleanly separate log output by context P3 because it decouples general logging primitives from mandatory global singletons Provides modular logging sinks without altering behavior for standalone executables Concept approval without stated reasons (sedited) Thinks review cost and merge conflicts outweigh current user demand, favoring simpler logging (stickies-v) Earlier objection about cumbersome logger plumbing was resolved with per-file enforcement (stickies-v) 832 lines added or modified outside tests 437 lines added or modified in tests 744 lines removed in total 79 files, 18 commits (+) #26022 ryanofsky Add util::ResultPtr class P3 · cleanup Review #25665 0 Mild 596 + 246 tests Simplifies error-handling wrappers for pointers by removing the need for double dereferencing Prevents bugs where error checks mistakenly evaluate to true even when the returned pointer is null P3 because it provides a safer and cleaner result-handling pattern for pointer types across the codebase Reduces subtle null-pointer checking pitfalls without altering user-facing behavior Ranking pass: Moved down: pure error-handling ergonomics rank below correctness and performance work here, but kept ahead of its dependent #25722 No review verdicts recorded Unaddressed objection: possible inverted boolean condition in wallet interfaces, no author reply (l0rinc) 596 lines added or modified outside tests 246 lines added or modified in tests 199 lines removed in total 19 files, 10 commits (+) #25722 ryanofsky refactor: Use util::Result class for wallet loading P3 · cleanup Review #25665 0 Strong 1,156 + 280 tests Standardize error reporting and propagation across wallet loading, creation, and migration Eliminate cumbersome mutable out-parameters for error statuses and warning lists across the stack P3 because it enhances general result utilities to support structured error types and warnings Benefits maintainers with cleaner error propagation across subsystems without urgent timing Ranking pass: Moved down: a behaviour-preserving refactor of wallet error plumbing, reviewable only after #25665 #26022 No review verdicts recorded Substantive support praising the expanded result utility as powerful yet simple to use (martinus) Resolved objections regarding compile errors and ambiguous boolean type conversions (martinus, sedited) 1,156 lines added or modified outside tests 280 lines added or modified in tests 783 lines removed in total 44 files, 24 commits (+) #29700 ryanofsky kernel, refactor: return error status on all fatal errors P3 · cleanup Review #25665 0 Blocked 1,334 + 464 tests Return error status directly from validation and storage routines rather than triggering node shutdowns Lets libbitcoinkernel callers handle fatal errors programmatically instead of via global callbacks P3 because it broadens utility result primitives to support detailed status codes and flush outcomes The primary motivation is decoupling the kernel rather than providing standalone utility benefits Ranking pass: Moved down: a pure refactor whose payoff is kernel decoupling rather than utils-side benefit. No review verdicts recorded Unaddressed objection: Result-style error handling hurts performance and defies C++ idioms (purpleKarrot) Concern about fragile nested result unwrapping and missed interruptions was answered (maflcko) Inconsistency from ignored flush returns mutating validation state was resolved in #35621 (optout21) Approval without stated reason from an account with no project history (Graysonbarton) 1,334 lines added or modified outside tests 464 lines added or modified in tests 676 lines removed in total 56 files, 23 commits (+) #36442 achow101 descriptor: Represent multipath descriptors in a single Descriptor object P3 · cleanup Waiting on author 0 Blocked 642 + 163 tests Preserve multipath descriptor strings during parsing instead of immediately splitting them Enable RPCs like getdescriptorinfo to return the original unexpanded multipath descriptor P3 because it improves type safety and derivation path parsing across shared BIP32 utilities The changes primarily serve descriptor-layer functionality rather than standalone utility needs Ranking pass: Moved up: it unblocks #36443 Review the logic now; conflicts with master are mechanical and will need a rebase Unaddressed objection: migratewallet marks all imported key origin steps as hardened (fametrano) Suggested adding a release note for changed display formatting of mixed hardening characters (fametrano) Concept approval without stated reasons (w0xlt, Sjors) 642 lines added or modified outside tests 163 lines added or modified in tests 712 lines removed in total 39 files, 8 commits (+) #27260 portlandhodl Enhanced error messages for invalid network prefix during address parsing. P3 · cleanup Author silent 68d 0 Mild 221 + 369 tests Give users clear, network-aware error messages when passing an address formatted for another network Avoid confusing fallback errors like generic Base58 failures on valid Bech32 addresses P3 because it provides a cleaner, structured error interface across shared address parsing helpers Removes prefix-guessing heuristics during decoding, offering a minor maintenance improvement Ranking pass: Moved up: misleading cross-network address errors are a genuine user-facing confusion, not just an interface cleanup. Ready to review now Conflicts with master, but the rebase is mechanical Broad concept support for making address decoding error messages more user friendly (Sjors, rkrux, jonatack) Tested and confirmed improved address decoding errors (Sjors) Unaddressed objection: bundling large refactors with behavior changes impedes review (l0rinc) Open objections on std::string_view lifetime risks and redundant decoding passes (l0rinc) 221 lines added or modified outside tests 369 lines added or modified in tests 352 lines removed in total 11 files, 8 commits (+) #35558 davidgumberg p2p: Prefill compact blocks P3 · new feature Needs rebase 0 (+2) Strong 548 + 83 tests Speed up block propagation by prefilling missing compact block transactions within TCP window limits Eliminate round-trips to retrieve missing transactions, lowering stale block and orphan risks network-wide P3 because it adds cross-platform socket inspection to query TCP window and queue sizes Provides new socket capabilities across multiple OSes but is specialized to block relay Broad support for the concept and backwards compatibility (0xB10C, josibake, johnnyasantoss) Verified on live nodes with telemetry confirming higher immediate reconstruction (m4ycon) Verified against compact block unit and functional test suites (s00ly) Past objections on use-after-free, locking, and unnecessary TCP syscalls were resolved (andrewtoth) 548 lines added or modified outside tests 83 lines added or modified in tests 146 lines removed in total 16 files, 13 commits (+) #36210 pablomartin4btc streams: Include the OS error when AutoFile I/O fails P3 · bug fix Ready 0 Positive 14 + 104 tests Include specific operating system error details when file stream operations fail Helps users and callers diagnose root causes such as full disks, invalid seeks, or permission errors P3 because callers previously had no way to distinguish out-of-space or permission errors from generic failures Improves diagnostic value across stream operations with low risk and self-contained changes Concept approval without stated reasons (w0xlt) Resolved objection: description and comments reworded to follow project AI policy (maflcko) 14 lines added or modified outside tests 104 lines added or modified in tests 6 lines removed in total 3 files, 2 commits (+) #31672 vasild rpc: add cpu_load to getpeerinfo P3 · new feature Author silent 186d 1 (+1) -1 Blocked 159 + 1 tests Help node operators identify resource-intensive or abusive peers Track and expose message processing CPU usage per peer in getpeerinfo and -netinfo Lay groundwork for future peer-prioritization and anti-DoS measures P3 because cross-platform thread CPU timing utilities are incremental internal infrastructure Utility benefit is limited to supporting exploratory diagnostics Ranking pass: Moved up slightly: new cross-platform thread CPU timing is reusable and feeds peer-prioritization work. Author silent for 180 days; review may wait a long time for a response Wide concept support for peer CPU tracking to combat DoS (jonatack, laanwj, mzumsande, hodlinator) Verified in testing on signet IBD and -netinfo (jonatack, BrandonOdiwuor, yuvicc) Unaddressed objection: metrics are non-actionable, flame graphs or wall time are better (ajtowns) Nonblocking objection: exploratory metrics should not go into production getpeerinfo (sipa) Suggested using a decaying average instead of connection-wide average (sipa) 159 lines added or modified outside tests 1 lines added or modified in tests 4 lines removed in total 8 files, 2 commits (+) #36033 maflcko [wip,nomerge,rfc] build: Require C++23 compiler P3 · cleanup Needs rebase 0 Positive 67 + 34 tests Eliminate accumulating in-tree polyfills and workarounds by raising compiler requirements to C++23 Unblock downstream dependencies and future C++23 language features across the project P3 because it cleans up in-tree utility polyfills in favor of standard library implementations Affects internal utility primitives without user-visible impact CI is failing, check whether the failure is from this PR before deep review Concept approval without stated reasons (l0rinc) Informational observation on include heuristics with no objections raised (hebasto) 67 lines added or modified outside tests 34 lines added or modified in tests 409 lines removed in total 56 files, 9 commits (+) #36122 fjahr BIP460: CISA for Taproot key path spends P3 · new feature Waiting on author 0 Strong 7,680 + 3,565 tests Allow cross-input signature aggregation for Taproot key path spends to reduce transaction size and fees Lower fee overhead for multi-party collaborative transactions like CoinJoin and Payjoin P3 because it vendors experimental half- and full-aggregation modules from libsecp256k1 It provides the cryptographic primitives necessary to test the proposed aggregation BIPs Ranking pass: Moved down: an explicitly draft reference implementation accompanying unmerged BIPs and unmerged secp256k1 modules. Worth reviewing now, but check if CI failures are caused by this PR before a deep review No review verdicts recorded Substantive support after testing consensus logic against BIP460 vectors and edge cases (fametrano) Objections regarding wallet signing and PSBT input grouping were addressed in code (BarneyChambers) Open suggestion to update vendored consensus test vectors to match the latest BIP copy (fametrano) 7,680 lines added or modified outside tests 3,565 lines added or modified in tests 219 lines removed in total 111 files, 22 commits (+) #36038 fanquake util: remove usage of `strerror_r` P3 · cleanup Waiting on author 0 Crickets 9 + 1 tests Simplify non-Windows system error formatting and remove platform-specific buffer branching Eliminate custom build-time checks needed to handle GNU versus POSIX error string variants P3 because it is an internal refactoring that simplifies error formatting without user-facing impact Reduces maintenance overhead by delegating platform differences to the standard library No review verdicts recorded No substantive approvals or rejections yet Suggested postponing until PR 36042 due to missing changes in GCC 12 libstdc++ (hebasto) Suggested adjusting header includes between Windows and other platforms (hebasto) 9 lines added or modified outside tests 1 lines added or modified in tests 33 lines removed in total 5 files, 1 commits (+) #36087 maflcko util: Add and use AssertUnreachable P3 · cleanup Author silent 61d 0 Mild 368 + 422 tests Eliminate compiler return-type warnings caused by unreachable code assertions Prevent assertion ambiguity and undefined behavior in unreachable code paths P3 because it clears GCC return-type warnings and resolves ambiguous debug assertion behavior It also protects against undefined behavior hazards in unreachable code paths Worth reviewing now because the rebase conflict is mechanical No review verdicts recorded Objection pending code update: standalone consensus library build breaks (jeanpablojp) Resolved prior concerns regarding linter regex matching and commit ordering (jeanpablojp) 368 lines added or modified outside tests 422 lines added or modified in tests 546 lines removed in total 312 files, 7 commits (+) #36183 kevkevinpal span: diagnose dangling views from MakeByteSpan/MakeUCharSpan P3 · cleanup Waiting on author 0 Blocked 23 Warn developers at compile time when non-owning views bind to temporary containers Prevents accidental dangling memory bugs before code reaches runtime P3 because compile-time checks on utility helpers prevent subtle dangling pointer bugs Improves internal safety across the codebase but is reasonably deferrable Blocked by an unaddressed objection that dangling views still go undiagnosed (maflcko) Concept approval without stated reasons (jeanpablojp) 23 lines added or modified outside tests 0 lines added or modified in tests 1 lines removed in total 2 files, 1 commits (+) #34730 ajtowns util/log: Combine the warning/error log levels into a single alert level P3 · cleanup Waiting on author 1 Mild 48 + 15 tests Simplify log interpretation by removing the confusing distinction between warning and error levels Clarify logging for node operators since node shutdown already makes actual failures obvious P3 because it eliminates a recurring source of confusion across the codebase with minimal disruption Provides modest internal cleanup value by streamlining alert logging conventions for developers Ranking pass: Moved down: cosmetic log-level consolidation that is gated on the #34038 Review #34038 Waiting on author response to an open objection Code review ACK because it aligns logging macros with their actual usage (ryanofsky) Nonblocking objection: losing distinct tags makes separating actual from potential issues harder (ryanofsky) 48 lines added or modified outside tests 15 lines added or modified in tests 57 lines removed in total 7 files, 2 commits (+) #33646 cedwies log: check fclose() results and report safely in logging.cpp P3 · bug fix Waiting on author 0 Positive 29 Report errors when closing log files during log rotation, shrink, or shutdown directly to stderr Avoid unnoticed loss of diagnostics from filesystem writeback failures P3 because log file close failures are rare and the benefit is limited to avoiding silent diagnostic loss Safely reports writeback errors directly to stderr without re-entering the logger Ranking pass: Moved down: the card itself notes close failures are rare, making this the least consequential logging fix. Author silent for 85 days with open review comments No review verdicts recorded Support without stated reasons (sedited) Unaddressed suggestions to drop noexcept and remove an extra variable (sedited) Unanswered question on whether the commit message is stale (sedited) 29 lines added or modified outside tests 0 lines added or modified in tests 8 lines removed in total 1 files, 1 commits (+) #35170 optout21 test: Better test coverage for ParseHDKeypath() P3 · test coverage Ready 1 (+3) Strong 0 + 120 tests Ensure BIP32 keypath parser tests verify parsed keypath outputs rather than only checking boolean success Catch bugs where a broken parser signals success but corrupts or clears the parsed output vector P3 because it strengthens regression tests for utility functions without fixing an active bug Closes a testing blind spot where an invalid parser producing empty paths still passed unit tests Protects BIP32 keypath handling correctness for developers and maintainers while remaining deferrable Ranking pass: Moved down: test-only hardening with no active bug, below the behaviour-affecting fixes in this band. Substantive support reviewing test coverage and round-trip assertions (haishmg) Approvals across multiple revisions and rebases (brunoerg, w0xlt) Dropped a contested function rename after reviewer feedback (sedited, achow101) Approved after ensuring test coverage is not temporarily reduced in commit history (Sjors) 0 lines added or modified outside tests 120 lines added or modified in tests 96 lines removed in total 1 files, 4 commits (+) #36190 Bortlesboat test: Check bitcoin wrapper child exit status on windows P3 · test coverage Review #36105 0 (+2) Strong 15 + 55 tests Verify that the Windows wrapper correctly forwards child exit statuses and reports spawn failures Prevent regressions where child exit code -1 is conflated with process launch errors P3 because it tests Windows child process execution utilities against edge cases Validates that process execution properly distinguishes spawn failures from child exit states Review #36105 The PR branch is stacked on open PR #36105 Strong support with no open objections Verified by testing on Linux and Windows runners across both CRTs (cyb3ralbert) Finds the new tests well written and useful additions to test coverage (ryanofsky) 15 lines added or modified outside tests 55 lines added or modified in tests 20 lines removed in total 7 files, 5 commits (+) #17581 ryanofsky refactor: Remove settings merge reverse precedence code P4 · cleanup Review #16545 0 Crickets 390 + 811 tests Make settings precedence consistent across all sources by removing legacy reverse-precedence handling Simplifies settings interpretation without altering user-facing behavior P4 because cleans up obsolete settings parsing special-casing without changing behavior Offers marginal internal code simplification for maintainers No review verdicts recorded No substantive comments or reviews from other participants yet Only update is a rebase note from the author 390 lines added or modified outside tests 811 lines added or modified in tests 202 lines removed in total 17 files, 18 commits (+) #35315 maflcko refactor: Use NodeClock::time_point in more places P4 · cleanup Author silent 103d 3 (+2) Strong 148 + 35 tests Standardize absolute time tracking across P2P and networking code using strongly typed time points Eliminate type confusion and second-level truncation that causes arbitrary tie-breaking in peer eviction P4 because it adds minor shared utility helpers and standard sentinels in util/time.h Needs rebase and author response after 92 days of inactivity Strong support with all objections resolved (ryanofsky, stickies-v) Strong support for type safety and clearer time representation (ryanofsky, stickies-v) Verified by running unit and P2P functional test suites (seduless, jeanpablojp) Rebase requested (sedited) 148 lines added or modified outside tests 35 lines added or modified in tests 181 lines removed in total 24 files, 10 commits (+) #36451 maflcko refactor: Use NotNull for CreateNewBlock and pimpls P4 · cleanup CI failing 0 Strong 30 + 13 tests Clarify pointer contracts by enforcing non-null semantics for block creation and internal implementations Eliminate confusing null checks and dead error-handling branches in tests and callers P4 because expressing pointer non-null contracts and expanding unit tests is internal code cleanup Cleans dead branches in coverage and mutation testing without affecting runtime safety or external behavior CI is failing so check whether the failure belongs to this PR before a deep review Concept approval because it adds useful clarity to block template creation (Sjors) Concept approval without stated reasons (sedited, stickies-v) 30 lines added or modified outside tests 13 lines added or modified in tests 34 lines removed in total 20 files, 3 commits (+) #19460 ryanofsky multiprocess: Add bitcoin-wallet -ipcconnect option P4 · cleanup Review #10102 0 Blocked 4,183 + 197 tests Allow the standalone wallet tool to connect to a running node over IPC instead of running strictly offline Prepare the wallet tool for future online actions like querying chain state and syncing balances P4 because utility modifications only support IPC connection glue It provides no general-purpose utility or logging improvements Review #10102 Stacked on top of open multiprocess PR #10102 No review verdicts recorded Unaddressed objection: combining CLI tool and daemon behavior adds work and risks user footguns (ajtowns) Answered objection: offline operation remains fully supported without requiring a node (jonasschnelli) Approvals from accounts with no project history provided no rationale 4,183 lines added or modified outside tests 197 lines added or modified in tests 644 lines removed in total 149 files, 26 commits (+) #36696 purpleKarrot CTransaction: Add constructor taking inputs and outputs P4 · cleanup Ready 0 Strong 49 + 38 tests Reduce boilerplate when creating test transactions declaratively Avoid requiring an intermediate mutable transaction just to build immutable test fixtures P4 because container conversion helpers only clean up minor boilerplate Provides no broader architectural benefit or safety improvements to utilities No review verdicts recorded Support because adding a witness argument removes workarounds in test PRs (alexanderwiederin) Objection on moving elements out of spans was resolved with an updated commit (alexanderwiederin) Prefers simpler std::vector over generic ranges but deferred to other reviewers (maflcko) 49 lines added or modified outside tests 38 lines added or modified in tests 59 lines removed in total 4 files, 3 commits (+) #36373 LarryRuane init: log resolved datadir path P4 · user request Ready 0 Crickets 6 Log the resolved canonical data directory path at node startup Helps developers and node operators verify the actual underlying storage device used in saved debug logs P4 because it only adds an informational startup log line for resolved storage paths Provides minor diagnostic convenience when analyzing logs without affecting node behavior or stability Ready to review Reviewers can help decide whether to log the resolved path unconditionally or only when symlinks differ No review verdicts recorded No substantive review or approvals yet Suggested avoiding a second log line when the resolved path matches the configured directory (l0rinc) Asked whether this change is part of broader symlink improvements (l0rinc) 6 lines added or modified outside tests 0 lines added or modified in tests 1 lines removed in total 1 files, 1 commits