Wallet

PRPriorityReviewabilityReviewsAgreementSize
(+)#35984 furszy sign: skip signing SIGHASH_SINGLE inputs with no corresponding output
  • Prevent wallet users from signing unsafe SIGHASH_SINGLE inputs without a matching output
  • Protects users from fund theft if tricked into signing a malicious PSBT
P1 · fund safety
  • P2 because it prevents a fund-safety vulnerability during PSBT signing
  • Protects users from transaction output redirection when handling untrusted PSBTs
  • Eliminates an unsafe signing footgun across all wallet signing paths
Ready
  • Ready for review with passing CI and no open objections
0 (+1)
Strong
  • Strong support with no opposition to fixing the signing footgun
  • Concept approval for addressing the issue quickly (l0rinc)
  • Reviewed and approved an earlier revision (achow101)
6 + 31 tests
  • 6 lines added or modified outside tests
  • 31 lines added or modified in tests
  • 8 lines removed in total
  • 2 files, 1 commits
(+)#36245 kriss39 wallet: start descriptor top up at range_start instead of index 0
  • Prevent wallet balance inconsistencies across reloads when importing descriptors with non-zero range starts
  • Avoid massive derivation delays and memory exhaustion when importing descriptors with high start indices
P1 · bug fix
  • P1 after comparing with the other PRs here (assessed alone as P2): Promoted to P1: imported funds vanish on wallet reload and derivation blows up to O(M) on high range_start imports.
  • P2 because it fixes inconsistent balances where transactions vanish after reloading an imported wallet
  • Eliminates severe latency and memory exhaustion when importing descriptors starting at high indices
Ready
  • Ready for review
  • Clean patch with passing CI and functional test coverage
0
  • No review verdicts recorded
Crickets
  • No reviews or comments yet
6 + 37 tests
  • 6 lines added or modified outside tests
  • 37 lines added or modified in tests
  • 3 lines removed in total
  • 2 files, 1 commits
(+)#27865 achow101 wallet: Track no-longer-spendable TXOs separately
  • Stop scanning historical spent outputs during wallet balance checks and coin selection preparation
  • Eliminate transaction creation latency for wallets with extensive histories or high activity
P2 · speedup
  • P2 because it resolves a major performance bottleneck in wallets with long transaction histories
  • Reduces balance check and coin selection latency by 250x to 500x on large wallets
  • Crucial for high-frequency wallet operators such as payment processors, exchanges, and Lightning nodes
Ready
  • Ready for review
  • Clean merge status, passing CI, and recently updated with review feedback
0 (+1)
Blocked
  • Strong support for massive performance gains, with benchmarks showing 250x to 500x speedups (remyers, w0xlt)
  • Concept approval without detailed reviews (murchandamus, jonatack, rkrux)
  • Multiple early state synchronization and assertion crash bugs were fixed (w0xlt)
  • Unaddressed objection: crashes in getbalances, reorg balance bugs, and db rewrites on corrupt loads (jeanpablojp)
389 + 160 tests
  • 389 lines added or modified outside tests
  • 160 lines added or modified in tests
  • 162 lines removed in total
  • 18 files, 19 commits
(+)#35989 shuv-amp wallet: fix crash on importdescriptors with a range ending at 2^31-1
  • Prevent node crashes and wallet corruption when importing descriptors with large ranges or keypools
  • Return clean RPC errors instead of triggering assertion failures and aborting bitcoind
P2 · bug fix
  • P2 because it fixes an assertion abort and database corruption caused by extreme descriptor ranges
  • Prevents persisting inverted range records to disk before terminating the node
Needs rebase
  • Needs rebase due to merge conflicts with master
2
Strong
  • Strong consensus on preventing node aborts with no open objections
  • Verified by reproducing crashes on master and testing clean error handling (jeanpablojp, molnard)
  • Verified watch-only wallet behavior and node stability (kriss39)
  • Agreed that repairing pre-existing corrupted wallet records is follow-up work (molnard)
15 + 50 tests
  • 15 lines added or modified outside tests
  • 50 lines added or modified in tests
  • 2 lines removed in total
  • 4 files, 3 commits
(+)#36230 achow101 wallet: Improve `HasWalletDescriptor` performance and other canonical descriptor string followups
  • Fix a noticeable lookup performance regression when handling complex descriptors like Miniscript
  • Avoid repeatedly allocating and computing canonical descriptor strings during wallet checks
P2 · speedup
  • P2 because it fixes a significant performance regression in descriptor comparison checks
  • Halves descriptor lookup time for wallets holding complex descriptors like Miniscript
  • Prevents the performance regression from persisting into release
  • Ranking pass: Perf regression flagged for 32.x backport, so placed ahead of feature work in this band.
Ready
  • Ready for review, with passing CI and prompt author updates
0 (+1)
Positive
  • Broad support for caching to eliminate repeated descriptor generation
  • Verified by testing a 50% speedup on Miniscript wallet tests (rkrux)
  • Verified test runtime dropped substantially on Alpine (hebasto)
  • Suggested making cached descriptors const to prevent post-construction mutations (polespinasa, davidgumberg)
65 + 21 tests
  • 65 lines added or modified outside tests
  • 21 lines added or modified in tests
  • 30 lines removed in total
  • 9 files, 7 commits
(+)#34400 Eunovo wallet: parallel fast rescan (approx 8x speed up with 8 threads)
  • Speed up wallet rescans using parallel block filter checks across multi-core systems
  • Greatly reduce wait times during wallet recovery and descriptor imports
P2 · speedup
  • P2 because wallet rescan is a frequent pain point during wallet recovery and descriptor imports
  • Empirically validated 4x to 8x speedup delivers major performance gains on multi-core machines
Needs rebase
  • Needs a rebase after prerequisite #34681 merged
0
Strong
  • Broad support with benchmarks confirming 4x to 8x speedups across multiple setups
  • Verified 6x speedup on an AMD Ryzen setup (ismaelsadeeq)
  • Verified 3x speedup on an HDD setup (l0rinc)
  • Strong concept approval after scanning refactor concerns were addressed (rkrux)
+1451/-388
  • 18 files
  • 15 commits
(+)#29278 ismaelsadeeq Wallet: Add `maxfeerate` wallet startup option
  • Cap transaction feerates independently from total fee limits on wallet transactions
  • Prevent accidental high feerates on small transactions while still allowing large transactions
P2 · fund safety
  • P2 because it fixes an issue where total fee limits were ambiguously used as feerate limits
  • Protects users from accidental high feerates on small transactions without blocking large transactions
  • Ranking pass: Valuable guardrail rather than an active defect, so below the measured performance and privkey-exposure work.
Ready
  • Ready for review, with clean code, passing CI, and all prior review comments addressed
1 (+4)Strong
  • Broad agreement on decoupling total fee limits from feerate caps
  • Support for independently constraining fee totals and feerates (murchandamus)
  • Reviewed and approved across iterations (josibake, achow101, furszy)
  • Approved following fixes to unconfirmed input bump fee handling (polespinasa)
119 + 184 tests
  • 119 lines added or modified outside tests
  • 184 lines added or modified in tests
  • 79 lines removed in total
  • 30 files, 9 commits
(+)#35377 achow101 wallet: Allow importing of descriptors without private keys when the wallet has the private keys
  • Allow importing public descriptors into a wallet if it already holds the matching private keys
  • Removes the safety hazard and friction of exposing plaintext private keys during import
P2 · fund safety
  • P2 because it removes the need to handle and expose plaintext private keys to import descriptors
  • Resolves a long-standing fund safety issue affecting multisig and coordinator workflows
Ready
  • Review #34861 first
  • Ready for review with passing CI and isolated descriptor and wallet changes
0
Strong
  • Strong support for eliminating manual private key exposure during descriptor imports
  • Concept approval because users do not have to deal with private keys manually (rkrux)
  • Concept approval pending review of base PR #34861 (polespinasa)
  • Concept approval with an alternative key resolution branch explored (Sjors)
143 + 68 tests
  • 143 lines added or modified outside tests
  • 68 lines added or modified in tests
  • 283 lines removed in total
  • 18 files, 16 commits
(+)#35998 achow101 wallet: Handle or explicitly ignore `WalletBatch` write failures
  • Prevent wallet in-memory state from desynchronizing with disk when database writes fail
  • Avoid silent data loss or corruption during key encryption, passphrase updates, and transactions
P2 · fund safety
  • P2 because it addresses systematic data integrity and state synchronization risks in wallet persistence
  • Eliminates silent failure modes when updating passphrases or encrypting keys
Needs rebase
  • Needs rebase due to merge conflicts with active wallet PRs
  • Review #35752 first
0
Strong
  • Concept approval because it ensures consistency between in-memory and disk state (rkrux, l0rinc)
  • Concern about partial in-memory updates on write failure versus treating corruption as fatal (jeanpablojp)
277 + 265 tests
  • 277 lines added or modified outside tests
  • 265 lines added or modified in tests
  • 230 lines removed in total
  • 19 files, 25 commits
(+)#35294 achow101 wallet: Update tx chain state during loading during AttachChain instead of before
  • Prevent node crashes caused by assertion failures when loading a wallet while blocks are actively syncing
P2 · bug fix
  • P2 because it fixes an assertion failure crash when loading a wallet during active block connection
  • Ensures transaction state is evaluated against a coherent chain tip rather than a moving target
Ready
  • Ready to review; the patch is small, cleanly rebased, and all feedback has been addressed
0 (+2)
Strong
  • Strong consensus that deferring state updates until chain attachment fixes the crash bug
  • Verified the reproduction test fails before the patch and passes after (shuv-amp)
  • Approved following Antithesis testing (dergoegge)
  • Verified race window specifics and confirmed safety for offline wallet tooling (pablomartin4btc)
13 + 81 tests
  • 13 lines added or modified outside tests
  • 81 lines added or modified in tests
  • 13 lines removed in total
  • 4 files, 2 commits
(+)#34193 furszy wallet: make migration more robust against failures
  • Prevent legacy wallet migration from crashing or leaving wallets in a half-converted state on errors
  • Ensure the original database remains intact until SQLite migration succeeds so recovery is automated
P2 · bug fix
  • P2 because it prevents users from being stranded with corrupt or interrupted wallet conversions
  • Protects legacy wallet users during migration by keeping an automated recovery path on write errors
Needs rebase
  • Needs rebase due to merge conflicts with master before review is worthwhile
0 (+1)
Strong
  • Strong support because keeping original database files until migration finishes improves safety (ryanofsky)
  • Cleaned up temporary SQLite files and error returns per feedback (achow101, ryanofsky)
  • Feedback addressed and previous push received a code review ACK (ryanofsky)
+187/-106
  • 5 files
  • 8 commits
(+)#35901 pablomartin4btc wallet: Fix ScanForWalletTransactions missing tx when look-ahead pool expands mid-block
  • Prevent descriptor wallets from silently missing transactions and showing wrong balances during rescans
  • Ensures funds are found when look-ahead keypools expand mid-block during descriptor imports or recovery
P2 · bug fix
  • P2 because it fixes a silent bug where block rescans miss transactions and compute incorrect balances
  • Affects users recovering funds or importing descriptors when transactions order unfavorably mid-block
Needs rebase
  • Wait for rebase and revisions as PR has merge conflicts with master
  • Author is evaluating an alternative algorithm proposal before pushing updates
0 (+1)
Disputed
  • Reviewers agree the bug exists, but proved the prefix re-scan approach does not fully solve it
  • Unaddressed objection: receives past the expansion point remain missed, shorting the balance (jeanpablojp)
  • Nonblocking objection: child spends after the expansion point are missed, leaving outputs unspent (molnard)
  • Concept approval despite double-scan performance considerations (achow101)
35 + 251 tests
  • 35 lines added or modified outside tests
  • 251 lines added or modified in tests
  • 2 lines removed in total
  • 3 files, 2 commits
(+)#35492 OSINTv96 wallet: fail dump on incomplete writes
  • Prevent wallet dumps from reporting success if writing fails or disk space runs out
  • Delete partial dump files on failure so users do not mistake them for valid backups
P2 · bug fix
  • P2 because silent dump failures could mislead users into trusting truncated backups
  • Ensuring dump integrity prevents accidental key loss during wallet recovery
  • Ranking pass: Backup integrity (truncated dumps trusted as valid) kept just under the fund-safety cluster it neighbours.
Ready
  • Ready for review: clean CI, no conflicts, and all reviewer feedback has been addressed
1Strong
  • Strong support because durability guarantees prevent crash-related corruption (pablomartin4btc)
  • Earlier suggestions on commit handling and error paths were fully incorporated (sedited, maflcko)
54 + 19 tests
  • 54 lines added or modified outside tests
  • 19 lines added or modified in tests
  • 43 lines removed in total
  • 2 files, 2 commits
(+)#32966 Eunovo Silent Payments: Receiving
  • Enables Bitcoin Core wallet to receive BIP 352 Silent Payments
  • Lets recipients publish a single static address without compromising privacy through address reuse
P2 · new feature
  • P2 because it provides a high-demand privacy upgrade that prevents address reuse
  • Lays the foundational receiving architecture for Bitcoin Core silent payments support
  • Ranking pass: Biggest privacy feature here, but benefit is still uncertain (upstream libsecp dependency), so ranked below ready correctness and perf work.
Needs rebase
  • Needs rebase, has merge conflicts, and author has been silent for months
  • Review #35301 first and wait for upstream libsecp256k1 support
0
  • No review verdicts recorded
Strong
  • Strong concept consensus on adding silent payment receiving to the wallet
  • Iteratively tested wallet creation and rescanning across revisions (Sjors)
  • Architectural feedback on key derivation and descriptor structures (josibake)
  • Contributed review feedback on tests (rkrux)
15,289 + 3,665 tests
  • 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
(+)#35302 Eunovo Silent Payments: Sending (take 2)
  • Allow wallet users to send BIP 352 silent payments
  • Improve on-chain privacy and eliminate address reuse by generating unique unlinked taproot outputs
P2 · new feature
  • P2 because it brings a major privacy improvement for wallet users
  • Unblocks silent payments receiving support and the broader silent payments roadmap
  • Ranking pass: Sending builds on receiving and on #35301; discounted for the same uncertainty as #32966.
Needs rebase
  • Review #35301 first
  • Needs rebase due to merge conflicts with master
0
Positive
  • Concept approval (rkrux)
  • Feedback on script handling and spend RPCs was resolved (theStack)
  • Suggestions on functional tests and bumpfee test coverage were addressed (rkrux)
1,487 + 7,258 tests
  • 1,487 lines added or modified outside tests
  • 7,258 lines added or modified in tests
  • 114 lines removed in total
  • 47 files, 18 commits
(+)#36114 Sjors wallet: harden external signer psbt processing, revamp mock
  • Prevent buggy or compromised hardware wallets from tampering with outputs or signing with unsafe sighash types
  • Improve test coverage for complex external signer workflows including multisig and misbehaving signers
P2 · fund safety
  • P2 because it prevents external signers from silently dropping or altering outputs and using unsafe sighash
  • Protects hardware wallet users from potential fund loss caused by malicious or buggy signer firmware
  • Significantly expands functional test capabilities for advanced multi-party signing setups
Waiting on author
  • Waiting on author to address a demonstrated bypass where finalized responses evade the sighash check
0
Positive
  • Concept approval with detailed testing of edge cases (jeanpablojp)
  • Unaddressed finding: finalized responses bypass the unsafe sighash check, no author reply (jeanpablojp)
40 + 206 tests
  • 40 lines added or modified outside tests
  • 206 lines added or modified in tests
  • 66 lines removed in total
  • 5 files, 4 commits
(+)#35742 yashbhutwala descriptors: check duplicate keys in all multipath Miniscript branches
  • Reject invalid multipath Miniscript descriptors with duplicate keys across any branch
  • Prevents wallets from importing invalid descriptors that cause reload failures after restart
P2 · bug fix
  • P2 because importing an invalid descriptor can break wallet reloads on restart
  • Prevents wallets from accepting invalid Miniscript spending policies on change paths
  • Ranking pass: Invalid multipath descriptors that break wallet reload; grouped with the other descriptor-correctness P2 items.
Needs rebase
  • Needs rebase due to merge conflicts with master
  • Author has not addressed requested commit restructuring and extra tests
0
Positive
  • Strong concept support for fixing the BIP 389 sanity check violation
  • Verified bug reproduction on master and fix on branch (Zeegaths)
  • Concept approval noting alignment with BIP 389 (vicjuma)
  • Requested split commits, simpler error extraction, and broader test coverage (l0rinc)
18 + 21 tests
  • 18 lines added or modified outside tests
  • 21 lines added or modified in tests
  • 3 lines removed in total
  • 2 files, 1 commits
(+)#33034 achow101 wallet: Store transactions in a separate sqlite table
  • Store transactions in a dedicated SQLite table instead of generic key-value records
  • Allow deterministic in-order loading to simplify wallet logic like tracking spent outputs
P2 · unblocks #27865
  • P2 because it directly unblocks tracking spent transaction outputs in #27865
  • Provides deterministic in-order loading via SQL queries instead of arbitrary record loads
  • Significantly improves maintainability by removing bespoke composite serialization
CI failing
0
Strong
  • Strong concept support with no objections
  • Concept approval for using relational tables over complex serialization workarounds (rkrux)
  • Concept approval without stated reasons (w0xlt)
1,045 + 10 tests
  • 1,045 lines added or modified outside tests
  • 10 lines added or modified in tests
  • 256 lines removed in total
  • 15 files, 27 commits
(+)#33033 achow101 wallet, sqlite: Encapsulate SQLite statements in a RAII class
  • Prevent manual resource handling mistakes in the wallet database backend
  • Lays groundwork for more complex database queries and upcoming schema updates
P2 · cleanup
  • P2 after comparing with the other PRs here (assessed alone as P3): Raised to P2: prerequisite for #33034, which itself unblocks #27865; it cannot sit below them.
  • P3 because this is an internal database refactoring with no direct user-facing change
  • Improves memory safety and code hygiene in the wallet database layer
  • Unblocks upcoming schema work such as storing transactions in a separate table
Ready
  • Ready for review: merges cleanly, CI is passing, and prior feedback has been addressed
0 (+3)
Strong
  • Strong consensus on using automatic resource management for database statements
  • Concept approval without detailed objection (rkrux)
  • Approved after verifying error handling and lifetime semantics (vasild)
  • Code review approval on recent revisions (pablomartin4btc)
230
  • 230 lines added or modified outside tests
  • 0 lines added or modified in tests
  • 212 lines removed in total
  • 2 files, 10 commits
(+)#32895 achow101 wallet: Prepare for future upgrades by recording versions of last client to open and decrypt
  • Track which client capabilities last opened and decrypted a wallet
  • Lets future versions safely detect downgrades and re-run upgrades without full wallet rescans
P2 · migration
  • P2 because recording client capabilities must be deployed well before future releases can rely on them
  • Unblocks future automatic wallet upgrades without risking data inconsistency across release cycles
Ready
  • Ready to review
  • Code is clean, CI is passing, and all previous reviewer feedback has been addressed
1 (+3)
Strong
  • Strong consensus across wallet contributors with all review threads addressed
  • Concept support for detecting un-upgraded data after downgrade cycles (ryanofsky)
  • Support after verifying versioning and locking semantics (ajtowns)
  • Verified by running test suites across multiple release versions (Eunovo, Bortlesboat)
  • Support after handling stale records on legacy wallet reloads (w0xlt)
121 + 2 tests
  • 121 lines added or modified outside tests
  • 2 lines added or modified in tests
  • 18 lines removed in total
  • 6 files, 4 commits
(+)#35716 achow101 wallet: Replace mapWallet and wtxOrdered with a boost::multi_index
  • Eliminate synchronization bugs and dangling pointer risks across wallet transaction storage
  • Fix a latent bug where reordering legacy transactions failed to update wallet ordering
P2 · cleanup
  • P2 because it removes a major maintenance burden and pointer risk in wallet transaction tracking
  • Fixes a bug where reordered legacy transactions stayed out of order, and unblocks downstream wallet work
Ready
  • Ready for review, passing CI with feedback incorporated
0 (+1)
Strong
  • Strong consensus on unifying transaction storage without leaking dependencies
  • Verified by reproducing and confirming it fixes an ordering bug on master (pablomartin4btc)
  • Concept approval that unified indexing is the right approach (theuni, rkrux)
331 + 36 tests
  • 331 lines added or modified outside tests
  • 36 lines added or modified in tests
  • 302 lines removed in total
  • 19 files, 4 commits
(+)#33112 Sjors wallet: relax external_signer flag constraints
  • Allow wallets to combine external signing devices with internal keys
  • Remove the restriction that forces external signer wallets to be watch-only and immutable
P2 · new feature
  • P2 because it unlocks hybrid key management combining external signers and hot keys in one wallet
  • Unblocks multisig and future MuSig2 wallet workflows that use hardware devices
  • Ranking pass: Enabling feature for hybrid signer and MuSig2 setups rather than an active defect, so below the correctness P2s.
Ready
  • Ready for review with CI passing and previous feedback addressed
0 (+3)Strong
  • Strong consensus across multiple reviewers with previous concerns resolved
  • Concept approval without stated reasons (achow101)
  • Verified by testing on Ledger Flex hardware with hot descriptor signing (PraneethGunas)
  • Verified by testing Qt and CLI workflows (adyshimony, rkrux)
179 + 91 tests
  • 179 lines added or modified outside tests
  • 91 lines added or modified in tests
  • 136 lines removed in total
  • 11 files, 10 commits
(+)#34457 w0xlt wallet: add private broadcast support for wallet transactions
  • Protect origin privacy for wallet transactions and periodic rebroadcasts
  • Route wallet RPC and GUI transactions over short-lived Tor or I2P connections instead of clearnet
P2 · new feature
  • P2 because it brings substantial origin privacy improvements to wallet users
  • Ensures transactions created directly in Bitcoin Core gain private broadcast protections
  • Ranking pass: Privacy gain depends on the wider private-broadcast stack; placed below ready correctness work.
Needs rebase
  • Needs rebase due to merge conflicts
  • Review #34533 first as author is waiting for its rebroadcast logic before updating
0
Strong
  • Broad concept support with no objections to wallet private broadcast
  • Concept approval for extending private broadcast to wallet transactions (andrewtoth, Bicaru20)
  • Agreed on simplifying rebroadcast model to follow #34533 (andrewtoth, w0xlt)
  • Suggested splitting Qt exception fix and test helpers into standalone PRs (rkrux)
+510/-45
  • 11 files
  • 5 commits
(+)#34533 vasild wallet: resubmit transactions with private broadcast if enabled
  • Prevent periodic wallet rebroadcasts from leaking transactions over standard connections
  • Preserves origin privacy for users who explicitly enable private broadcast
P2 · privacy
  • P2 because periodic rebroadcasts can deanonymize users who opted into private broadcast
  • Prevents unconfirmed transactions from leaking to all peers during resubmission sweeps
  • Ranking pass: Follow-up to #34457 and unresolved on package/CPFP rebroadcast, so reviewed after it.
Needs rebase
  • Needs rebase against master before code review is worthwhile
  • Author has been inactive for over four months
0 (+2)
Disputed
  • Concept supported to complete private broadcast coverage for wallet transactions (pablomartin4btc, w0xlt)
  • Unaddressed objection: dispatches can split dependent transactions and break CPFP rebroadcasts (achow101)
  • Suggested that package private broadcast could address the CPFP rebroadcast failures (andrewtoth)
+54/-11
  • 2 files
  • 2 commits
(+)#36070 PraneethGunas wallet: Add deriveHDKey interface
  • Allow the GUI to derive shareable extended public keys from an HD seed
  • Unblock multisig setup workflows in the GUI without relying on RPC
P2 · unblocks #35645
  • P2 because it unblocks multisig setup workflows in the GUI tracked in #35645
  • Enables users to derive shareable xpubs in the GUI without duplicating sensitive derivation logic
Ready
  • Ready for review
0Mild
  • Concept approval to unblock GUI multisig workflows (polespinasa, jeanpablojp, pseudoramdom)
  • Concern about changing RPC error codes on derivehdkey before release (pseudoramdom, polespinasa)
  • Suggested reordering commits to isolate error code modifications (polespinasa)
118 + 45 tests
  • 118 lines added or modified outside tests
  • 45 lines added or modified in tests
  • 55 lines removed in total
  • 7 files, 3 commits
(+)#35918 pseudoramdom descriptor: Expose key expression parsing
  • Allow callers like the GUI multisig wizard to parse and inspect standalone key expressions
  • Reject invalid trailing characters when parsing multisig expressions
P2 · new feature
  • P2 after comparing with the other PRs here (assessed alone as P3): Raised to P2: like #36070 it is a prerequisite for the multisig wizard (#35645); the two must share a band.
  • P3 because it unblocks GUI multisig setup flows without affecting core wallet safety
  • Enables validating pasted key expressions and inspecting key origins
  • Fixes improper acceptance of trailing characters after multisig expressions
Waiting on author
  • Needs rebase due to merge conflicts
1
Positive
  • Support on the latest commit without objections (w0xlt)
  • Questions whether to parameterize the script context rather than hardcode taproot (uqlidi)
117 + 185 tests
  • 117 lines added or modified outside tests
  • 185 lines added or modified in tests
  • 86 lines removed in total
  • 3 files, 3 commits
(+)#36133 Sjors wallet: store multipath descriptor
  • Preserve multipath descriptor representations when importing or creating wallets
  • Allows exporting unified descriptors needed by hardware wallets for BIP 388 multisig registration
  • Eliminates error-prone manual reconstruction of descriptors for external signer tools
P2 · new feature
  • P2 because it enables exporting multipath descriptors required for BIP 388 multisig registration
  • Improves multisig signing safety and removes user friction when using hardware wallets via HWI
  • Ranking pass: Real hardware-wallet interop value but the storage approach is contested; below the settled P2 items.
Needs rebase
  • Merge conflicts against master require a rebase
  • Marked draft while architectural approach remains under discussion
0
Disputed
  • Disputed approach to multipath representation between storage and dynamic reconstruction
  • Objecting to storing multipath strings in the DB, favoring dynamic pairwise traversal (achow101)
  • Concept approval for matching existing wallet descriptors (jeanpablojp)
698 + 373 tests
  • 698 lines added or modified outside tests
  • 373 lines added or modified in tests
  • 362 lines removed in total
  • 24 files, 18 commits
(+)#36154 jeanpablojp wallet: fill PSBT_GLOBAL_XPUB for descriptors with more than one key
  • Populate BIP 174 global xpubs in wallet PSBTs for descriptors with multiple keys
  • Allow stateless hardware signers to recognize multisig setups and sign directly
P2 · user request
  • P2 because it fixes an issue where hardware signers refuse to sign multisig PSBTs
  • Stateless devices like BitBox02 require global xpubs and otherwise need manual workarounds
Waiting on author
  • Needs rebase
  • Author has not responded to major architectural objections
0
  • No review verdicts recorded
Blocked
  • Unaddressed objection: global xpubs should not be restricted to multisig (achow101)
  • Unaddressed objection: API should return maps directly and support MuSig (achow101)
  • Noted utility for non-HWI flows and clarified device requirements (Sjors)
175 + 161 tests
  • 175 lines added or modified outside tests
  • 161 lines added or modified in tests
  • 36 lines removed in total
  • 13 files, 2 commits
(+)#36284 fjahr wallet: don't double discard output groups with avoidpartialspends
  • Prevent false insufficient funds errors during transaction creation
  • Stops double counting discarded coins when avoidpartialspends or avoid_reuse is active
P3 · bug fix
  • P3 because it fixes false insufficient funds errors during coin selection
  • Affects an edge case combining specific flags with discarded unconfirmed coins
Ready
  • Ready to review
  • Small and isolated fix with a functional test, passing CI
0
  • No review verdicts recorded
Crickets
  • No reviews or comments yet
  • Recently opened without reviewer engagement
2 + 27 tests
  • 2 lines added or modified outside tests
  • 27 lines added or modified in tests
  • 1 lines removed in total
  • 2 files, 1 commits
(+)#35813 pablomartin4btc wallet, rpc: Add listrawtransactions RPC
  • Allow wallet users and automated tools to view transactions that spend change back to the wallet
  • Provide a complete transaction history that includes consolidations and pure self-transfers
P3 · new feature
  • P3 because it allows wallet operators to inspect consolidation and self-transfer transactions
  • Fixes visibility of transactions that spend solely to wallet change addresses
  • Deferrable since it does not impact fund safety, consensus, or wallet storage
Needs rebase
  • Needs rebase due to merge conflicts with the base branch
2 (+3)Strong
  • Strong consensus on introducing a dedicated RPC method
  • Recommended dedicated RPC approach and approved implementation (achow101)
  • Verified and approved after testing pagination and verbosity (Bicaru20)
  • Verified by testing on regtest (BrandonOdiwuor, w0xlt)
  • Approved after reviewing documentation and helper refactoring (polespinasa)
169 + 165 tests
  • 169 lines added or modified outside tests
  • 165 lines added or modified in tests
  • 27 lines removed in total
  • 6 files, 5 commits
(+)#32857 Sjors wallet: allow skipping script paths
  • Allow Taproot spends to skip script paths to avoid exposing fallback scripts on chain
  • Saves transaction fees and protects privacy for users of complex Taproot and MuSig2 setups
P3 · new feature
  • P3 because it benefits Taproot and MuSig2 users by preventing accidental fallback script spends
  • Remains an opt-in safety feature with no impact on default wallet spending behavior
Ready
  • Ready to review: branch is rebased, CI passes, and prior reviewer suggestions are addressed
0 (+1)
Strong
  • Strong support citing multi-party MuSig keypath and script-path race scenarios (rkrux)
  • Verified by testing script-path suppression behavior and test coverage (jeanpablojp)
  • Concept approval without stated reasons (w0xlt)
66 + 177 tests
  • 66 lines added or modified outside tests
  • 177 lines added or modified in tests
  • 27 lines removed in total
  • 13 files, 9 commits
(+)#34371 8144225309 wallet: allow importprunedfunds for spending transactions
  • Allow pruned wallet users to re-import transactions that spend from the wallet
  • Prevents inaccurate wallet balances after using removeprunedfunds
P3 · bug fix
  • P3 because it fixes an accounting bug that leads to incorrect balances in pruned wallets
  • Restores correct balances for users re-importing spending transactions
  • Impact is localized to a specialized RPC command
Ready
  • Ready for review: branch is up to date, passes CI, and only an optional test comment remains
1 (+1)
Strong
  • Strong agreement on the fix and implementation
  • Confirmed it accomplishes the goal and fixes UTXO tracking (Bicaru20)
  • Approval of the updated approach and test coverage (achow101)
  • Advised moving wallet logic into core wallet code rather than RPC (davidgumberg)
+36/-20
  • 3 files
  • 1 commits
(+)#35370 junbyjun1238 rpc: add key-origin modes to PSBT processing RPCs
  • Allow PSBT processing RPCs to strip existing key-origin data and global xpubs
  • Prevent leaking derivation paths to co-signers in collaborative setups
P3 · bug fix
  • P3 because it prevents key derivation and xpub metadata leaks during multiparty signing sessions
  • Provides a clean method to sanitize PSBT key-origin fields in collaborative workflows
Author silent 69 days
  • Code is ready to review with all previous feedback addressed
  • Author has been silent for 69 days
0
Strong
  • Approach approval for stripping metadata in PSBT processing RPCs (willcl-ark)
  • Concept approval for stripping bip32 derivation data on demand (rkrux)
  • Design suggestion to use an enum argument was adopted (achow101, rkrux)
86 + 245 tests
  • 86 lines added or modified outside tests
  • 245 lines added or modified in tests
  • 10 lines removed in total
  • 9 files, 2 commits
(+)#36005 pythcoiner rpc, descriptor: warn when multipath expansion clones single-path key expressions
  • Warn users when single-path key expressions are cloned during multipath descriptor expansion
  • Prevent accidental key reuse between receive and change outputs
  • Expose descriptor warnings in getdescriptorinfo and deduplicate RPC warning outputs
P3 · user request
  • P3 because it mitigates privacy loss when receive and change paths unintentionally share keys
  • Catches subtle misconfigurations without breaking valid descriptors
Needs rebase
  • Needs rebase due to merge conflicts with master
1Strong
  • Strong support with no objections raised
  • Verified by mutating checks and validating test failures (jeanpablojp)
57 + 88 tests
  • 57 lines added or modified outside tests
  • 88 lines added or modified in tests
  • 10 lines removed in total
  • 8 files, 4 commits
(+)#34872 w0xlt wallet: fix mixed-input transaction accounting in history RPCs
  • Stop wallet history RPCs from showing nonsensical negative fees and fabricated sends
  • Accurately report collaborative transactions that mix wallet and foreign inputs
P3 · bug fix
  • P3 because it fixes incorrect fee calculations and phantom sends for collaborative transactions
  • Only affects users participating in CoinJoins or Payjoins, making the fix deferrable
Needs rebase
  • Needs rebase due to merge conflicts with master
  • Author has been silent for over two months following review comments
0 -1
Positive
  • General support for reporting an aggregate send on mixed-input transactions
  • Objected to omitting send entries entirely, addressed by adding aggregate debits (achow101)
  • Concept approval for the reworked approach (rkrux)
  • Strong support calling it a great improvement while requesting minor clarifications (murchandamus)
261 + 732 tests
  • 261 lines added or modified outside tests
  • 732 lines added or modified in tests
  • 59 lines removed in total
  • 11 files, 13 commits
(+)#36039 btcpavao psbt: classify missing Taproot script-path signatures as signer
  • Corrects PSBT analysis to recognize when Taproot script-path transactions are waiting for signatures
  • Prevents misleading wallet warnings that inputs are missing data when ready to sign
P3 · bug fix
  • P3 because it fixes confusing wallet status messages during Taproot script-path signing
  • Improves the usability of Taproot multisig workflows without affecting transaction validity
Ready
  • Ready to review with CI passing and feedback addressed in the latest push
0
  • No review verdicts recorded
Neutral
  • Initial review caught incorrect analysis for transactions involving MuSig (achow101)
  • Author updated the patch to handle MuSig aggregates and awaits re-review
12 + 11 tests
  • 12 lines added or modified outside tests
  • 11 lines added or modified in tests
  • 3 lines removed in total
  • 3 files, 1 commits
(+)#35990 shuv-amp wallet: harden the read-only BDB parser against crafted files
  • Prevent crafted or corrupted legacy wallet files from hanging bitcoind or crashing it via out-of-memory errors
  • Protect users migrating old BDB wallets from malformed inputs
P3 · DoS protection
  • P3 because it fixes three distinct infinite-loop and out-of-memory crash bugs when parsing malformed files
  • Parsing untrusted local wallet files during migration is an infrequent vector rather than a network threat
Ready
  • Ready for review
  • Small diff with passing CI and no conflicts
0
  • No review verdicts recorded
Crickets
  • No reviews or comments on the pull request yet
  • Awaiting initial review from wallet maintainers
47 + 179 tests
  • 47 lines added or modified outside tests
  • 179 lines added or modified in tests
  • 7 lines removed in total
  • 3 files, 3 commits
(+)#36040 Bicaru20 Wallet: Don't backdate locktime rbf
  • Prevent fee-bumped transactions from getting an earlier locktime than the transaction they replace
  • Stops an anomalous on-chain pattern that leaks a recognizable wallet fingerprint
P3 · bug fix
  • P3 because this fixes an observable wallet fingerprinting defect during RBF fee bumping
  • Occurs in under 2% of fee-bumped transactions on chain with clear privacy benefit but no safety risk
  • Ranking pass: Moved up: a confirmed on-chain fingerprinting defect outranks the deferrable convenience items around it.
Ready
  • Worth reviewing now
  • The patch is clean, rebased, and all functional concerns have been addressed in code
2
Strong
  • Strong agreement with no blocking objections
  • Support after architectural discussions and test hardening (molnard)
  • Tested approval with minor commit message typo suggestions (nervana21)
  • Approach approval on earlier revision (polespinasa)
32 + 121 tests
  • 32 lines added or modified outside tests
  • 121 lines added or modified in tests
  • 8 lines removed in total
  • 6 files, 2 commits
(+)#35440 alhudz wallet: check descriptor cache xpub length before decoding
  • Prevent crashes when loading corrupted or truncated wallet descriptor cache records
  • Turn out-of-bounds memory reads into clean database corruption errors when opening a wallet
P3 · bug fix
  • P3 because it prevents an out-of-bounds read crash when loading corrupted descriptor cache records
  • It turns an invalid memory read into a clean database error when reading corrupted wallet files
  • Impact is limited since truncated or corrupted descriptor cache records are rare in practice
  • Ranking pass: Moved up: out-of-bounds read on corrupted descriptor cache is a memory-safety fix, ahead of convenience P3s.
CI failing
  • CI is failing on the latest push and needs a rerun or maintainer workflow approval
1 (+1)
Strong
  • Strong consensus on fixing the bounds check and refactoring key serialization
  • Requested and approved converting extended keys to standard serialization streams (achow101)
  • Thoroughly tested and approved both the initial fix and the stream refactoring (molnard)
  • Validated unit test improvements and stream decoupling (winterrdog)
64 + 161 tests
  • 64 lines added or modified outside tests
  • 161 lines added or modified in tests
  • 85 lines removed in total
  • 11 files, 2 commits
(+)#35760 achow101 wallet: make corrupted transaction records fail wallet loading instead of forcing a rescan
  • Fail wallet loading immediately when transaction records are corrupted instead of forcing a rescan
  • Prevent repeated full rescans from genesis and inconsistent in-memory wallet state when DB records corrupt
P3 · bug fix
  • P3 because it addresses a narrow reliability edge case seen only during database corruption
  • Prevents endless startup rescans from genesis and protects against inconsistent in-memory wallet state
  • Ranking pass: Moved up: today's behaviour is an endless rescan from genesis on every startup, a real reliability defect.
Ready
  • Ready for review: code is small, passes CI, and includes unit test coverage
0Mild
  • General agreement that failing fast is better than triggering broken rescans
  • Support because rescanning cannot repair corrupt records and wastes time (pablomartin4btc, rkrux)
  • Verified by reproducing corruption behavior on regtest (jeanpablojp)
  • Nonblocking objection: users without backups cannot access intact funds or export keys (polespinasa)
39 + 23 tests
  • 39 lines added or modified outside tests
  • 23 lines added or modified in tests
  • 23 lines removed in total
  • 8 files, 5 commits
(+)#35747 nervana21 wallet: Fix FillPSBT failing to sign owned inputs when UTXOs disagree
  • Prevent unexpected PSBT signing failures when external coordinators provide conflicting UTXO data
  • Ensure the wallet can still reliably sign owned inputs from buggy or adversarial coordinators
P3 · bug fix
  • P3 because it fixes an edge-case bug where signing fails on owned inputs with conflicting UTXO data
  • Funds are never at risk since the bug only caused incomplete signing rather than signing the wrong UTXO
  • Ranking pass: Moved up: concrete signing failure on owned inputs, grouped with the other bug fixes.
Needs rebase
  • Needs rebase due to merge conflicts with master
0
  • No review verdicts recorded
Neutral
  • Neutral sentiment with no explicit ACKs yet
  • Asked clarifying questions on the threat model and risks of conflicting UTXOs (vicjuma)
9 + 84 tests
  • 9 lines added or modified outside tests
  • 84 lines added or modified in tests
  • 7 lines removed in total
  • 4 files, 2 commits
(+)#34698 shuv-amp wallet: handle MiniMiner bump fee calculation failures
  • Prevent internal exceptions when spending unconfirmed coins tied to very large mempool clusters
  • Return clear error messages instead to wallet users creating transactions or running sendall
P3 · bug fix
  • P3 because it prevents an unhandled internal exception exposed to users in rare mempool topologies
  • Only occurs when spending coins in clusters over 500 transactions and does not endanger funds
  • Ranking pass: Grouped with the other user-visible failure fixes (unhandled exception surfaced to RPC users).
Ready
  • Ready to review
  • Author incorporated all reviewer feedback and CI is passing
0
  • No review verdicts recorded
Positive
  • Positive sentiment with reviewer feedback addressed
  • Guided error reporting patterns and return types, which the author implemented (achow101)
33 + 114 tests
  • 33 lines added or modified outside tests
  • 114 lines added or modified in tests
  • 37 lines removed in total
  • 11 files, 1 commits
(+)#31668 saikiran57 Added rescan option for import descriptors
  • Allow importing descriptors without triggering any blockchain rescan
  • Avoids the mandatory two-hour rescan delay when importing newly created descriptors
P3 · new feature
  • P3 because it saves time during imports of newly generated descriptors
  • Eliminates the mandatory two-hour rescan window for users importing unused descriptors
Needs rebase
  • Merge conflicts with master require a rebase before review
0
Positive
  • General agreement on avoiding rescans when the user requests it
  • Concept approval to allow skipping rescan on import (rkrux)
  • Reviewers converged on using never as the timestamp parameter value (maflcko, furszy)
  • Feedback on test coverage and avoiding magic numbers was addressed (achow101, furszy)
43 + 157 tests
  • 43 lines added or modified outside tests
  • 157 lines added or modified in tests
  • 29 lines removed in total
  • 2 files, 1 commits
(+)#32861 Sjors Have createwalletdescriptor auto-detect an unused(KEY)
  • Automatically detect unused keys when creating wallet descriptors
  • Avoid requiring users to manually copy-paste xpubs when configuring blank descriptor wallets
P3 · new feature
  • P3 because it is an incremental workflow improvement for descriptor wallets
  • Smooths blank wallet scripting without altering critical wallet safety mechanisms
Ready
  • Ready to review with CI passing and prior reviewer suggestions already incorporated
0
Strong
  • Strong support across reviewers with suggestions incorporated
  • Concept and approach approval proposing a duplicate key check that was added (adyshimony)
  • Concept approval suggesting existing helper reuse that was adopted (jeanpablojp)
  • Concept approval without stated reasons (w0xlt)
49 + 25 tests
  • 49 lines added or modified outside tests
  • 25 lines added or modified in tests
  • 9 lines removed in total
  • 7 files, 5 commits
(+)#34405 8144225309 wallet: skip APS when no partial spend exists
  • Avoid running an extra coin selection pass when partial spends are already absent
  • Saves computation, avoids worse coin selection results, and prevents false tracepoint reporting
P3 · speedup
  • P3 because it avoids redundant computation and worse coin selection results in standard wallet spending
  • Fixes inaccurate reporting in the coin selection tracepoint when partial spends are already absent
Needs rebase
  • Needs rebase due to merge conflicts before review is worthwhile
  • The author has been inactive for over 200 days
0
  • No review verdicts recorded
Neutral
  • Minor test and syntax comments addressed without explicit approval (achow101)
  • No reviewers have posted an explicit concept or code ACK
+62/-16
  • 3 files
  • 1 commits
(+)#34603 achow101 wallet: Fix detection of symlinks on Windows
  • Prevent unintended directory traversal and loops when scanning wallet paths on Windows
  • Properly detect symlinks and reparse points despite MinGW runtime limitations
P3 · platform fix
  • P3 because it prevents potential directory scan loops for Windows wallet users
  • Affects an edge case in Windows multiwallet directory handling, making it deferrable
Author silent
  • Author silent for over 100 days
  • No merge conflicts or CI failures, code is ready for review
0
Positive
  • Generally positive sentiment with technical inquiries addressed
  • Approved with a minor nonblocking request for description updates (sedited)
  • Technical questions on reparse behavior and duplicate calls resolved (hebasto, luke-jr)
30 + 27 tests
  • 30 lines added or modified outside tests
  • 27 lines added or modified in tests
  • 40 lines removed in total
  • 5 files, 4 commits
(+)#35317 stutxo wallet: fix ignored subtract_fee_from_outputs option
  • Prevent wallet funding RPCs from silently ignoring the snake_case subtract_fee_from_outputs option
  • Ensure fees are subtracted from outputs as requested instead of unexpectedly paid from inputs
P3 · bug fix
  • P3 because it fixes a silently ignored funding parameter in wallet RPCs
  • Impact is low because callers using the documented camelCase parameter work as expected
Author silent 85d
  • Author has been inactive for 85 days after asking whether to close the PR
0
  • No review verdicts recorded
Mild
  • Reviewers prefer a codebase-wide transition to snake_case RPC options over piecemeal fixes
  • Suggested standardizing all RPC argument handling and deprecating camelCase (achow101)
  • Suggested closing the PR in favor of an issue (sedited)
  • Pointed to existing tracking work and recommended a linter approach (maflcko)
26 + 31 tests
  • 26 lines added or modified outside tests
  • 31 lines added or modified in tests
  • 13 lines removed in total
  • 3 files, 2 commits
(+)#35041 brunoerg descriptor: speed-up `Parse` (xpub/xpriv) in ~30%
  • Speed up descriptor parsing for extended keys and checksum calculations
  • Accelerates wallet fuzzing harnesses and parsing of complex descriptor setups
P3 · speedup
  • P3 because it yields a verified 30% to 36% speedup when parsing extended keys
  • Benefits wallet fuzz targets and complex multisig descriptor setups
  • Descriptor parsing is rarely a primary bottleneck for standard wallet users
Ready
  • Ready for review: clean state, passing CI, and material feedback addressed
1
Strong
  • Strong support with independent benchmarks confirming a 37% speedup (hodlinator)
  • Concept approval without stated reasons (sedited)
  • Implementation suggestions regarding string_view were adopted (maflcko)
67
  • 67 lines added or modified outside tests
  • 0 lines added or modified in tests
  • 28 lines removed in total
  • 5 files, 2 commits
(+)#34909 rkrux wallet, refactor: modularise wallet by extracting out legacy wallet migration
  • Make the core wallet code easier to navigate and maintain
  • Separate legacy wallet migration logic out of the main wallet codebase
P3 · cleanup
  • P3 because this is an internal cleanup with no user-facing behavioral changes
  • Reduces the oversized core wallet file by separating legacy migration logic
  • Aids long-term maintainability for wallet developers
Ready
  • Ready to review with clean move-only diffs and passing CI
0
Positive
  • Positive sentiment with no objections
  • Concept approval and suggested folding in related BDB file rename (pablomartin4btc)
860 + 4 tests
  • 860 lines added or modified outside tests
  • 4 lines added or modified in tests
  • 832 lines removed in total
  • 16 files, 5 commits
(+)#35444 w0xlt wallet: make descriptor SPKM mutex non-recursive
  • Prevent deadlocks and re-entrancy bugs by making descriptor script manager locking non-recursive
  • Clarify wallet lock hierarchies and safely dispatch callbacks outside of lock scope
P3 · cleanup
  • P3 because it is an internal concurrency cleanup that reduces deadlock risk without fixing an active bug
  • Helps unblock the broader project effort to eliminate recursive mutexes
Ready
  • Ready for review
  • Cleanly rebased with passing CI and no open blockers
0
Strong
  • Strong support for the locking approach and clear commit structure (pablomartin4btc)
  • Concept approval without stated reasons (hebasto)
  • Atomicity concern regarding listdescriptors was resolved by clarifying wallet locking (pablomartin4btc)
388 + 118 tests
  • 388 lines added or modified outside tests
  • 118 lines added or modified in tests
  • 213 lines removed in total
  • 8 files, 4 commits
(+)#36031 achow101 wallet: Remove mapMasterKeys and enforce that only one encryption key can exist
  • Enforce that encrypted wallets only ever have a single master encryption key
  • Remove legacy complexity and inconsistent handling across wallet unlocking and database loading
P3 · cleanup
  • P3 because it resolves historical technical debt in wallet encryption without altering standard workflows
  • Simplifies passphrase handling and loading while preserving backward compatibility for normal wallets
  • Ranking pass: Encryption-key debt removal ranked with the other technical-debt cleanups.
Ready
  • Ready for review
1
Strong
  • Strong consensus to simplify master key handling across the wallet
  • Concept approval citing long-term maintainability (rkrux)
  • Code review approval after verifying failure modes (davidgumberg)
  • Noted legacy double-encrypted blank wallets fail to load but did not object to proceeding (jeanpablojp)
36 + 2 tests
  • 36 lines added or modified outside tests
  • 2 lines added or modified in tests
  • 48 lines removed in total
  • 6 files, 3 commits
(+)#30343 ryanofsky wallet, logging: Replace WalletLogPrintf() with LogInfo()
  • Standardize wallet logging on modern severity macros while preserving wallet name prefixes
  • Ensure source location logging reports the actual call site rather than a wrapper
P3 · cleanup
  • P3 because it eliminates legacy custom logging in favor of project-wide macros
  • Fixes accurate call site reporting for wallet messages under -logsourcelocations
  • Ranking pass: Internal logging standardisation; ranked with the other refactors rather than above behavioural fixes.
Needs rebase
0 (+1)
Strong
  • Strong support across reviewers
  • Tested and approved after temporary object lifetime questions were addressed (adyshimony)
  • Reviewed with suggestions on naming and severity levels incorporated (hodlinator, maflcko)
327 + 207 tests
  • 327 lines added or modified outside tests
  • 207 lines added or modified in tests
  • 193 lines removed in total
  • 17 files, 14 commits
(+)#25722 ryanofsky refactor: Use util::Result class for wallet loading
  • Standardize error and warning handling across wallet loading and initialization
  • Reduce boilerplate and avoid mishandling errors by removing inconsistent out-parameters
P3 · cleanup
  • P3 because it is an internal cleanup of wallet error propagation without changing behavior
  • Reduces maintenance overhead across wallet initialization and migration without direct user impact
  • Ranking pass: Error-handling refactor with no observable behaviour change; ranked with the other refactors.
Needs rebase
  • Needs rebase due to merge conflicts with master
  • Review #25665 and #26022 first
0
  • No review verdicts recorded
Positive
  • Positive feedback on the result helper improvements (martinus)
  • Requested API improvements and compiler fixes were addressed by the author
  • No reviews yet on the wallet loading changes themselves
1,130 + 267 tests
  • 1,130 lines added or modified outside tests
  • 267 lines added or modified in tests
  • 773 lines removed in total
  • 44 files, 25 commits
(+)#32993 brunoerg fuzz: wallet: add target for tx scanning
  • Catch crashes and edge cases in wallet transaction scanning using fuzzed block inputs
  • Protect wallet rescan logic against complex state transitions missed by deterministic tests
P3 · test coverage
  • P3 because it strengthens test coverage for wallet fund tracking and block processing
  • The safety benefits apply to rescan logic without altering any production code
  • Ranking pass: Fuzz harness work grouped together below the behavioural fixes.
Author reworking
  • Author moved the PR to draft to refactor for execution speed and has been inactive
  • Hold review until the author pushes the updated harness and takes it out of draft
0 (+1)
Positive
  • General positive reception with no architectural objections
  • Verified the harness runs without crashing (Chand-ra)
  • Fixed a cleanup memory leak found during review (ekzyis)
  • Fixed an integer overflow in the mock time helper (fanquake)
0 + 111 tests
  • 0 lines added or modified outside tests
  • 111 lines added or modified in tests
  • 0 lines removed in total
  • 2 files, 1 commits
(+)#34264 Chand-ra fuzz: Extend `spend` coverage
  • Catch bugs in wallet transaction creation and funding by fuzzing sequential operations together
P3 · test coverage
  • P3 because it expands fuzz testing across multi-step transaction creation and coin selection
  • It improves test robustness without addressing an immediate fund-safety defect
  • Ranking pass: Fuzz coverage grouped with the other harness PRs.
Needs rebase
  • Needs a rebase onto master to resolve merge conflicts
  • Author has been inactive for over 200 days
0
Strong
  • Strong support for expanding fuzz coverage of transaction funding and coin listing (frankomosh)
  • Suggested adding assertions on fee results and output ranges (frankomosh)
+63/-21
  • 1 files
  • 4 commits
(+)#36013 pablomartin4btc test: Descriptor roundtrip and raw()/ addr() coverage
  • Verify that descriptors inferred from generated scripts preserve their original structure
  • Add test coverage for raw() and addr() descriptor invariants to catch parsing regressions
P3 · test coverage
  • P3 because it adds unit test coverage for descriptor roundtripping and keyless types
  • Protects descriptor parsing and inference against regressions with no user-facing changes
Ready
  • Ready to review with passing CI and clean git state
0
Positive
  • Positive sentiment with concept approval and verified test vectors
  • Concept approval without stated reasons (w0xlt)
  • Concept approval verified with test suite runs and address decoding (jeanpablojp)
  • Nonblocking suggestions for regex cleanup and extra negative assertions await reply (jeanpablojp)
0 + 101 tests
  • 0 lines added or modified outside tests
  • 101 lines added or modified in tests
  • 8 lines removed in total
  • 1 files, 2 commits
(+)#35443 mjdietzx tests: add functional test for tr musig2 miniscript decaying multisig
  • Add end-to-end test coverage and documentation for decaying multisig descriptors
  • Demonstrate wallet PSBT coordination combining MuSig2 key paths with Miniscript script paths
P3 · test coverage
  • P3 because it adds test coverage for an advanced wallet descriptor workflow
  • Validates wallet PSBT signing across multiple wallets without changing wallet code
  • Serves primarily as executable documentation for combining Taproot, MuSig2, and Miniscript
Author silent
  • Author has been silent for over 100 days
0
Positive
  • Overall positive sentiment with no objections raised
  • Concept approval with suggestion to consolidate with existing tests (rkrux)
  • Author preferred keeping it separate for clarity and documentation value
13 + 177 tests
  • 13 lines added or modified outside tests
  • 177 lines added or modified in tests
  • 0 lines removed in total
  • 3 files, 1 commits
(+)#34907 rkrux wallet, test: make wallet_fast_rescan robust
  • Make the wallet fast rescan functional test robust against silent setup failures
  • Ensure descriptor keypool top-ups are genuinely exercised during rescan tests
P3 · test coverage
  • P3 because it hardens regression coverage for descriptor top-ups in fast rescan tests
  • Prevents tests from silently passing when setup fails to trigger intended top-ups
  • Ranking pass: Test hardening grouped with the other test-only changes.
Author silent 104d
  • Author has been silent for 104 days
  • Branch is rebased with an approval on head
1 (+1)
Strong
  • Strong support after author incorporated all reviewer improvements
  • Suggested asserting descriptor range growth and exact txids (w0xlt)
  • Approved the latest commit after simplifying the test flow (Bicaru20)
0 + 56 tests
  • 0 lines added or modified outside tests
  • 56 lines added or modified in tests
  • 33 lines removed in total
  • 1 files, 1 commits
(+)#35433 rkrux wallet: deprecate replaceable argument from transaction (and psbt) creation (and modification) RPCs
  • Deprecate the obsolete replaceable argument across transaction creation and modification RPCs
  • Avoid caller confusion and simplify RPC interfaces now that full-RBF is the network default
P3 · cleanup
  • P3 because it cleans up obsolete BIP 125 opt-in configuration across wallet spending endpoints
  • The change improves long-term wallet maintainability but remains deferrable cleanup
  • Ranking pass: Deprecation cleanup is deferrable; placed below the fix and coverage work.
Needs rebase
  • Needs rebase due to merge conflicts against master
  • Stale while waiting on author response to outstanding review comments
0
Positive
  • General agreement on deprecating BIP 125 replaceability parameters
  • Concept approval for deprecating the replaceable argument (polespinasa)
  • Recommended throwing an RPC error when passed without the deprecation flag (achow101)
  • Unaddressed request for test coverage of replaceable=False across spend RPCs (polespinasa)
58 + 104 tests
  • 58 lines added or modified outside tests
  • 104 lines added or modified in tests
  • 69 lines removed in total
  • 15 files, 8 commits
(+)#36126 polespinasa wallet, rpc: Implements set key label functionality
  • Help users track which master keys correspond to physical devices or cosigners in multisig setups
P3 · new feature
  • P3 because it improves ergonomics for multisig users distinguishing cosigners and hardware keys
  • Convenience feature with no fund safety or consensus impact that can be deferred
Awaiting concept decision
  • Review premature because author marked PR draft while project decides if labels belong in wallet DB
0 -2
Disputed
  • Blocking objection to storing key labels in wallet DB and using collidable fingerprints (achow101)
  • Questions RPC utility and warns of added maintenance burden for guided multisig (rkrux)
  • Concept approval for adding key labels (vicjuma, jeanpablojp, Eunovo)
294 + 156 tests
  • 294 lines added or modified outside tests
  • 156 lines added or modified in tests
  • 1 lines removed in total
  • 11 files, 11 commits
(+)#36143 rxbryan descriptor: add CreateMultisigDescriptor()
  • Help wallet users assemble multisig descriptors safely from cosigner keys
  • Prevent setup mistakes from manual string formatting, duplicate keys, and invalid thresholds
P3 · new feature
  • P3 because safer multisig descriptor creation is a non-critical setup convenience
  • Reduces user errors when assembling descriptors across multiple signers
  • Provides non-essential foundation work for future multisig setup tooling
Needs rebase
  • Needs rebase due to merge conflicts against master
  • Author has been silent following a Concept NACK
0 -1
Blocked
  • Concept NACK against adding an uncalled C++ helper without an end-to-end caller (Sjors)
  • Prefers prototyping the multisig setup flow in Python before committing to C++ (Sjors)
  • Unaddressed follow-up: author suggested exposing via RPC but has not responded further
124 + 79 tests
  • 124 lines added or modified outside tests
  • 79 lines added or modified in tests
  • 0 lines removed in total
  • 3 files, 2 commits
(+)#36236 musaHaruna wallet, rpc: add verify_balance option to importdescriptors
  • Prevent missing funds when importing descriptors by verifying balances against the UTXO set
  • Automatically extends rescan backwards if older unspent payments were missed due to a bad timestamp
P3 · new feature
  • P3 because it offers a helpful usability safety check for users importing descriptors
  • Protects against bad import timestamps but users can already avoid this by scanning from genesis
Needs rebase
  • Review is blocked by merge conflicts against the base branch
  • Needs rebase before review can proceed
0
  • No review verdicts recorded
Crickets
  • No reviews or feedback yet from any reviewers
  • Recently opened without concept comments
334 + 250 tests
  • 334 lines added or modified outside tests
  • 250 lines added or modified in tests
  • 47 lines removed in total
  • 9 files, 4 commits
(+)#35786 furszy wallet: drop spent parents redundant cache invalidation and notification
  • Remove redundant parent transaction cache invalidations and UI notifications when outputs are spent
  • Clean up outdated balance-tracking logic made obsolete by earlier wallet refactoring
P3 · cleanup
  • P3 because it is an internal cleanup removing dead invalidation logic left over from earlier refactoring
  • Improves readability and eliminates minor overhead but fixes no major bug and adds no new capabilities
Ready
  • Ready for review
  • Passing CI and clean mergeability, with an edge-case report awaiting author response
0 (+2)
Mild
  • General support for removing redundant invalidations and cleaning up confusing code
  • Verified code and test changes (pablomartin4btc, Bicaru20)
  • Concept approval noting clearer mark-dirty logic (rkrux)
  • Unaddressed objection: stale cached amounts reported if keypool expands during spend (w0xlt)
7 + 101 tests
  • 7 lines added or modified outside tests
  • 101 lines added or modified in tests
  • 32 lines removed in total
  • 4 files, 3 commits
(+)#36255 murchandamus Refactor CoinGrinder tests
  • Consolidate CoinGrinder tests into the modern test suite to reduce confusion across multiple test files
  • Test coin selection with realistic feerates and effective values rather than obsolete dummy parameters
P3 · cleanup
  • P3 because it cleans up internal unit tests without impacting wallet users or node safety
  • Better test fidelity and consolidation help maintainers navigate coin selection tests
CI failing
  • CI is failing on the head commit
0
Strong
  • Strong concept support for consolidating tests and retiring the legacy test file
  • Concept approval welcoming test cleanup to reduce confusion between test suites (yancyribbens)
  • Concept approval with a branch of test simplification suggestions (l0rinc)
0 + 194 tests
  • 0 lines added or modified outside tests
  • 194 lines added or modified in tests
  • 274 lines removed in total
  • 2 files, 11 commits
(+)#36122 fjahr BIP460: CISA for Taproot key path spends
  • Cut transaction fees and witness size by aggregating signatures across multiple inputs.
  • Encourage collaborative transactions like Payjoin and CoinJoin by removing per-input signature overhead.
P3 · new feature
  • P3 because descriptor and PSBT tooling enables prototyping multi-party aggregated transactions
  • Tooling remains speculative until consensus rules are finalized
  • Ranking pass: Explicitly experimental draft accompanying unfinished BIP proposals; wallet-side value is speculative, so last among substantive P3s.
Needs rebase
  • Wait for a rebase to resolve merge conflicts.
  • Author noted this is a draft reference implementation not yet intended for upstream merge.
0
  • No review verdicts recorded
Mild
  • Commended consensus and policy test coverage after running local tests (BarneyChambers)
  • Reported three bugs in wallet signing and PSBT aggregation that break completion states (BarneyChambers)
  • Author has not yet replied to the reported wallet issues
7,784 + 3,504 tests
  • 7,784 lines added or modified outside tests
  • 3,504 lines added or modified in tests
  • 488 lines removed in total
  • 133 files, 22 commits
(+)#33043 w0xlt [POC] wallet: Enable non-electronic (paper-based) wallet backup with codex32
  • Enable non-electronic paper backup and recovery for wallets using codex32
  • Help users back up and restore wallets without needing to manage raw descriptor strings
P3 · new feature
  • P3 because it fulfills an enduring user request for non-electronic paper backups
  • Backing up raw seeds without descriptor metadata or timestamps offers incomplete wallet recovery
Needs rebase
  • Draft proof of concept needs a rebase and has been inactive for over a year
0
  • No review verdicts recorded
Mild
  • Concept approval for the broad idea of paper backups (rkrux)
  • Unaddressed objection: backing up keys without descriptors or timestamps is insufficient (Sjors)
  • Warning that exported seeds bypass wallet encryption and grant full spending access (apoelstra)
  • Suggested requiring wallet birth timestamps and supporting Shamir shares (BenWestgate)
965 + 472 tests
  • 965 lines added or modified outside tests
  • 472 lines added or modified in tests
  • 46 lines removed in total
  • 31 files, 5 commits
(+)#19460 ryanofsky multiprocess: Add bitcoin-wallet -ipcconnect option
  • Allow the wallet tool to connect directly to a running node over IPC
  • Lays the transport foundation for future online wallet subcommands without HTTP RPC
P3 · new feature
  • P4 because it does not affect core wallet logic, fund safety, or descriptor management
  • It provides no immediate utility to wallet users until subcommands use the socket
Waiting on author
  • Review #29409 first
  • Stacked on top of unmerged multiprocess prerequisite PRs #29409 and #10102
0
  • No review verdicts recorded
Blocked
  • Unaddressed question whether adding IPC to an offline CLI tool creates user footguns (ajtowns)
  • Cautioned against conflating offline wallet tooling with online daemon features (jonasschnelli)
  • Author clarified that standard offline wallet usage remains intact and default
3,873 + 135 tests
  • 3,873 lines added or modified outside tests
  • 135 lines added or modified in tests
  • 549 lines removed in total
  • 139 files, 25 commits
(+)#35342 kevkevinpal test: fuzz wallet_rpc target
  • Fuzz wallet RPC commands with randomized arguments to catch crashes and assertions from malformed input
  • Adds automated fuzz testing where wallet RPC methods currently have zero coverage on OSS-Fuzz
P4 · test coverage
  • P4 after comparing with the other PRs here (assessed alone as P3): Lowered to P4: reviewers argue fuzzing wallet RPCs without chain state or UTXOs has very low efficacy, so the coverage claim is weakly supported.
  • P3 because it exercises parameter parsing rather than core wallet fund-handling or descriptors
  • Helps catch crashes at entrypoints from malformed inputs without touching deeper wallet logic
Author silent
  • Reviewing now is unlikely to be worthwhile while the author is silent following approach objections
0 -1
Blocked
  • Approach NACK because stateless fuzzing without context yields minimal test value (brunoerg)
  • Concerns that chaining RPCs harms performance and creates an intractable search space (maflcko, brunoerg)
  • Author has been silent after asking about alternative approaches
0 + 530 tests
  • 0 lines added or modified outside tests
  • 530 lines added or modified in tests
  • 176 lines removed in total
  • 6 files, 2 commits
(+)#35733 achow101 sign: Remove FillableSigningProvider
  • Remove outdated signing abstractions left over from the legacy wallet
  • Confine legacy signing logic strictly to migration code paths and clean up tool signing interfaces
P4 · cleanup
  • P3 because it consolidates legacy keystore logic directly into migration handlers
  • Removes legacy wallet baggage from general signing interfaces as an architectural cleanup
Ready
  • Ready for review; clean PR with feedback incorporated and passing CI
0
Strong
  • Broad support for isolating legacy wallet codepaths and modernizing signing providers
  • Concept approval because it containerizes legacy wallet code paths (rkrux)
  • Approach approval after verifying tests and script size limits (jeanpablojp)
  • Verified by testing bitcoin-tx commands (vicjuma)
86 + 104 tests
  • 86 lines added or modified outside tests
  • 104 lines added or modified in tests
  • 288 lines removed in total
  • 22 files, 7 commits
(+)#36068 brunoerg fuzz: reuse one fuzzed wallet across inputs
  • Speed up wallet fuzz testing by reusing a single wallet instance across inputs
  • Avoid repeated overhead from descriptor imports, BIP32 derivations, and database setups
P4 · cleanup
  • P4 because it only adds an internal test-only helper to wipe in-memory state
  • Has no impact on production wallet behavior or user-facing functionality
  • Ranking pass: Measured 4-6x fuzz throughput gain makes it the most useful of the marginal items.
Ready
  • Ready to review
  • Clean patch, passing CI, and initial review questions answered
0
Strong
  • Approach approval confirming a roughly 4x fuzz execution speedup (jeanpablojp)
  • Concerns about index drift and container clearing order were addressed (jeanpablojp)
19 + 127 tests
  • 19 lines added or modified outside tests
  • 127 lines added or modified in tests
  • 17 lines removed in total
  • 4 files, 2 commits
(+)#35768 vicjuma wallet: Reject whitespace-only wallet names
  • Prevent creating wallets whose names consist entirely of whitespace
  • Avoids creating blank-looking directories on disk that are difficult to manage
P4 · cleanup
  • P4 because whitespace-only wallet names are an extreme edge case with no safety implications
  • Provides a minor usability cleanup rather than fixing an active failure mode
  • Ranking pass: Minor validation fix, first among the purely cosmetic items.
Ready
  • Ready to review
  • Small, self-contained patch with passing CI
0
  • No review verdicts recorded
Positive
  • General support for rejecting blank-like names at creation while preserving loading for existing ones
  • Supports blocking creation while ensuring existing wallets still load (achow101)
  • Skeptical of practical payoff and cautioned about UX risk, but raised no objection (maflcko)
  • Suggested broadening the restriction to leading and trailing whitespace (polespinasa)
8 + 7 tests
  • 8 lines added or modified outside tests
  • 7 lines added or modified in tests
  • 3 lines removed in total
  • 5 files, 1 commits
(+)#36264 MrHodlX wallet rpc: fix stale argument metadata and help text
  • Fix outdated argument names and misleading help examples across several wallet RPCs
  • Prevents confusion for developers and RPC users debugging wallet calls
P4 · cleanup
  • P4 because this is cosmetic polish to RPC error messages and help documentation
  • Improves clarity for RPC callers but has no impact on fund safety or wallet state
  • Ranking pass: Help-text and error-argument accuracy touches every RPC caller, so ahead of the other cosmetic changes.
Ready
  • Ready for review
  • Small and focused patch directly incorporating previous reviewer feedback
0
  • No review verdicts recorded
Positive
  • Feedback on commit structure addressed by splitting commits (l0rinc)
  • Dropped edits to deprecated arguments after concerns raised (maflcko)
  • No open objections remain
6 + 2 tests
  • 6 lines added or modified outside tests
  • 2 lines added or modified in tests
  • 8 lines removed in total
  • 4 files, 3 commits
(+)#35224 kevkevinpal wallet: importdescriptors should output the request on error
  • Show which descriptor failed when importdescriptors reports a missing timestamp
  • Makes troubleshooting bulk descriptor imports faster for wallet users
P4 · cleanup
  • P4 because it provides a minor diagnostic improvement for RPC errors
  • It does not impact wallet correctness or safety
Author silent
  • Review #34861 first
  • Marked as draft and author has been inactive
0
  • No review verdicts recorded
Crickets
  • No review activity or comments since submission
  • Awaiting initial feedback
1 + 10 tests
  • 1 lines added or modified outside tests
  • 10 lines added or modified in tests
  • 2 lines removed in total
  • 2 files, 2 commits
(+)#35966 vicjuma wallet, rpc: log instead of returning false
  • Prevent reporting wallet encryption failure when encryption succeeded but immediate unlock failed
P4 · cleanup
  • P4 because it addresses an extremely narrow edge case of an unlock failure right after encryption
  • Nuances of the RPC return value are marginal compared to safely handling wallet corruption
Waiting on author
  • Wait for author to update the PR with a different failure-handling mechanism
  • Wallet maintainer rejected continuing execution after a failed unlock as unsafe
0
  • No review verdicts recorded
Disputed
  • Disputed approach to handling unlock failures after encryption
  • Unaddressed objection: failing to unlock indicates catastrophic corruption and should halt (achow101)
  • Author conceded that logging instead of halting may not be the right fix (vicjuma)
  • Questioned whether encryption of new wallets was broken (katesalazar)
1
  • 1 lines added or modified outside tests
  • 0 lines added or modified in tests
  • 1 lines removed in total
  • 1 files, 1 commits
(+)#35429 w0xlt wallet: avoid global access in external signer SPKM
  • Improve wallet modularity by eliminating direct calls to global state in external signer handling
P4 · cleanup
  • P4 because it is an internal cleanup with no user-visible behavior change
  • It incrementally decouples wallet components but does not unblock critical roadmap items
  • Ranking pass: Internal decoupling with no stated payoff; ranked with the other no-op cleanups.
Needs rebase
  • Needs rebase due to merge conflicts
  • Author has been inactive for several months
0
  • No review verdicts recorded
Crickets
  • No concept or code reviews yet
  • Only an administrative ping requesting a rebase (sedited)
44 + 11 tests
  • 44 lines added or modified outside tests
  • 11 lines added or modified in tests
  • 27 lines removed in total
  • 5 files, 2 commits
(+)#34879 rkrux wallet, test: remove unnecessary node restart from wallet_reindex
  • Clarify and speed up a wallet functional test by removing misleading steps and excess block generation
P4 · cleanup
  • P4 because it is internal cleanup for a single functional test and does not touch production code
  • The performance benefit is marginal, shaving only about two seconds off one test run
Waiting on author
  • Reviewing now is worthwhile to help evaluate whether removing the restart drops needed test coverage
1Mild
  • Support for simplifying the test and speeding up its execution (pablomartin4btc)
  • Concern that removing the restart drops coverage of birthtime preservation across restarts (polespinasa)
0 + 90 tests
  • 0 lines added or modified outside tests
  • 90 lines added or modified in tests
  • 93 lines removed in total
  • 3 files, 2 commits
(+)#35834 Herb-ops Test checkunparsable errors
  • Ensure descriptor unit tests properly verify private-form parsing error messages
  • Fixes silent overwriting of expected errors in test helpers without changing production code
P4 · test coverage
  • P4 because it only corrects an assertion oversight in unit test code
  • Does not impact production descriptor parsing or wallet behavior
Needs rebase
  • Needs rebase due to merge conflicts with #35445
0
  • No review verdicts recorded
Crickets
  • No reviews or comments have been posted yet
  • The pull request has received no engagement since being opened
0 + 18 tests
  • 0 lines added or modified outside tests
  • 18 lines added or modified in tests
  • 12 lines removed in total
  • 1 files, 2 commits
(+)#35151 musaHaruna wallet, follow-up: Refactor IsSpent to use HowSpent
  • Remove duplicated transaction spend-checking logic in wallet code
P4 · cleanup
  • P4 because it is a minor internal cleanup deduplicating around twenty lines of code
  • It does not fix any bugs, improve fund safety, or provide user-visible benefit
  • Ranking pass: Pure deduplication of ~20 lines with no user-visible effect.
Author silent 135 days
  • Author silent for 135 days
1 -1
Strong
  • Concept NACK on original version due to extra checks and lost early exit (luke-jr)
  • Author pushed an early-exit fix to address the concern with no follow-up from objector
  • Approval of the refactor as a clean deduplication (polespinasa)
8
  • 8 lines added or modified outside tests
  • 0 lines added or modified in tests
  • 24 lines removed in total
  • 2 files, 1 commits
(+)#36028 sipa miniscript: make typing rule match BIP-379 exactly
  • Align Miniscript typing rules with the BIP-379 specification to match test vectors
  • Resolve edge-case discrepancies without altering descriptor acceptance or spendability
P4 · cleanup
  • P4 because it has zero user-visible impact on descriptor parsing or wallet spendability
  • Benefit is strictly matching BIP-379 test vectors exactly in malleable edge cases
Waiting on BIP agreement
  • Author kept the PR in draft pending approach agreement on BIP PR 2240
0
Positive
  • Concept approval confirming descriptor acceptance remains unchanged (guggero)
  • Noted that discussions on specification wording may suggest amending the BIP instead (guggero)
4
  • 4 lines added or modified outside tests
  • 0 lines added or modified in tests
  • 6 lines removed in total
  • 1 files, 1 commits
(+)#36071 yancyribbens refactor: fuzz: remove cost_of_change from coin-grinder fuzz target
  • Clean up dead setup code in wallet coin-grinder fuzz tests
  • Avoid setting an unused parameter that has no effect on the algorithm
P4 · cleanup
  • P4 because it is a minor cleanup in test code
  • Has no functional impact on wallet logic or fuzzing efficacy
Ready
  • Ready for review now
  • Single-line test cleanup with no conflicts or failing checks
1Strong
  • Approved confirming the assignment is dead code and a no-op in the target (brunoerg)
0
  • 0 lines added or modified outside tests
  • 0 lines added or modified in tests
  • 1 lines removed in total
  • 1 files, 1 commits
(+)#35358 brunoerg external signer: verify PSBT is reliable after signing it
  • Protect wallet users from buggy or malicious external signers altering transaction outputs
  • Ensure signed transactions match expected outputs and do not introduce weak signature hashes
P4 · fund safety
  • P4 after comparing with the other PRs here (assessed alone as P3): Lowered to P4: the code has been absorbed into #36114, making this PR redundant.
  • P3 because it provides defense in depth against compromised or buggy hardware signers mutating outputs
  • Benefit is marginal because external signers are already trusted to present output details to users
Review #36114 instead
  • Review #36114 instead
  • The author is inactive and the changes have been absorbed into #36114
0
  • No review verdicts recorded
Positive
  • Mild concept approval with skepticism over whether signers need verification (achow101)
  • Absorbed the implementation into #36114 to improve signer test mocking (Sjors)
57 + 207 tests
  • 57 lines added or modified outside tests
  • 207 lines added or modified in tests
  • 16 lines removed in total
  • 4 files, 1 commits
(+)#34502 rkrux wallet: remove most asserts of `WALLET_FLAG_DESCRIPTORS` flag
  • Reduce cognitive noise for developers reading wallet code by dropping redundant descriptor assertions
P4 · cleanup
  • P4 because it is minor internal cleanup to simplify reading wallet code
  • Offers no functional, security, or user-facing benefits
Needs rebase
  • Needs rebase and remains in draft while the author investigates test failures
0
  • No review verdicts recorded
Mild
  • Unaddressed objection: removing the assertion leads to crashes or inconsistent state (b-l-u-e)
  • Questioned removing the check because encryption executes descriptor-specific setup (frankomosh)
  • Author has not responded to the technical objections
+8/-17
  • 2 files
  • 2 commits
(+)#35890 GuTS805 doc: use overwrite (>) instead of append (>>) for one-shot PSBT files in offline-signing-tutorial.md
  • Prevent tutorial users from breaking subsequent commands when re-running offline signing steps
  • Avoid accumulating duplicate PSBT lines in output files when commands are repeated without manual cleanup
P4 · cleanup
  • P4 because it touches tutorial documentation rather than wallet implementation or RPC logic
  • Carries no impact on fund safety, descriptor handling, or transaction creation
Ready
  • Ready for review with a minimal diff, no conflicts, and passing CI
0
  • No review verdicts recorded
Neutral
  • No technical review comments or ACKs submitted yet
  • Procedural objection about AI-generated text was resolved by rewriting the description (davidgumberg)
2
  • 2 lines added or modified outside tests
  • 0 lines added or modified in tests
  • 2 lines removed in total
  • 1 files, 1 commits
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.
Ranking pass (2026-09-17): all PRs here were compared with each other; 44 notes on review order, overlaps, and band changes.

Covers

The bitcoind wallet: key and descriptor management, transaction creation and coin selection, PSBT handling, wallet database (SQLite, and the legacy BDB migration path), backup and recovery, rescanning, wallet RPCs, and wallet-facing pieces of the GUI. Fee estimation is wallet when the change is about how the wallet uses it.

Borderline: a descriptor or PSBT change that affects only bitcoin-tx or RPC without wallet code is rpc, not wallet. An interfaces::Wallet change is both wallet and ipc.

What matters here

Fund safety first: anything that could lose keys, double-spend unintentionally, create an unspendable output, or corrupt the wallet database. Then backup, recovery, and migration correctness, including the legacy-to-descriptor migration path that every remaining legacy user must cross. Then privacy of transaction creation (address reuse, change detection, fingerprinting). Then user-visible reliability: hangs, slow rescans, confusing failures, wrong balances. Then features users have asked for, with evidence they have asked.

The rubric does not ask whether Bitcoin Core should have a wallet. It asks, given that it does, what a wallet user needs most.

generated 2026-09-17 21:55 UTC