#32966 Silent Payments: Receiving
| Reviewability | Reviews | Agreement | Size | |||||||
|---|---|---|---|---|---|---|---|---|---|---|
| #32966 Silent Payments: Receiving | Needs rebase | 0 | Neutral | 15,289 + 3,665 tests | ||||||
|
|
|
|
| ||||||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | ||||||||||
Goal
This pull request introduces BIP 352 Silent Payments receiving capabilities to the Bitcoin Core wallet. It adds an `sp()` descriptor implementation, a dedicated `SilentPaymentDescriptorScriptPubKeyMan`, full block scanning logic for silent payment outputs, and automatic silent payments change type selection. It also includes the required libsecp256k1 BIP 352 module and tests.
Problem: Traditional Bitcoin addresses cannot be safely reused without severely compromising user privacy and transaction graph unlinkability. Silent payments solve this by allowing recipients to publish a single static address while payers derive unique one-time output keys on-chain.
Reviewability
Ready · Needs rebase
Conflicts with master; review the logic now, the rebase is mechanical.
Waiting on (from the thread read):
- author: author to rebase and apply fixes for rkrux and bitsagarob 2026-09-22
Agreement
Neutral
objections settled, nobody has spoken for the PR
Thread notes: Eunovo acknowledged review comments from rkrux and bitsagarob, indicating they will be resolved when rebasing.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| Sjors (member) | objection, interface | requiring a bech32 descriptor for change in an SP-only wallet prevents transaction creation and risks backup safety | fixed · fix pushed · author updated change destination logic to SP · v1, not yet re-checked | answered | yes | 2025-07-14: 'it insists on having a bech32 descriptor for change... It seems safer, for backups, to send change to the same silent payment address.' Status from: 2025-09-17: Sjors: 'checked that the change went back to the descriptor (there are no other descriptors in the wallet).' |
| Sjors (member) | objection, correctness | silent payment wallets require full block scanning, so failing to disable block filters causes incorrect or failed rescanning | fixed · fix pushed · rescan logic updated to bypass blockfilter index · v1, not yet re-checked | answered | yes | 2025-07-14: 'while testing I found that it doesn't disable block filters.' Status from: 2025-07-25: Sjors: 'rescan uses the slow version (when `-blockfilterindex` is enabled), as expected' |
| macgyver13 (none) | objection, correctness | importing sp descriptors with scan and spend private keys produced missing key warnings and failed to provide addresses | fixed · fix pushed · import warning resolved · v1, not yet re-checked | answered | yes | 2025-07-25: 'I think this change is breaking the ability to import sp descriptor with private keys defined for scan and spend' Status from: 2025-08-13: macgyver13: 'I can confirm the import warning is resolved.' |
| josibake (member) | objection, safety | generic tweak addition functions on CKey and CPubKey introduce footguns that could be misused outside silent payments | fixed · fix pushed · renamed and scoped tweak functions specifically to BIP352 · v1, not yet re-checked | answered | yes | 2025-09-05: 'instead of a generic `CKey::TweakAdd` function that might be abused/footgunny.' Status from: 2025-09-18: Eunovo: 'I changed `CKey::TweakAdd()` to `CKey::ComputeBIP352Key()`' |
| Sjors (member) | objection, interface | silent payment descriptors cannot be imported into existing wallets because the silent-payments flag cannot be toggled | answered · author replied explaining why flag is immutable; no follow-up from Sjors · v1, not yet re-checked | answered | yes | 2025-09-17: 'It would be nicer if it just toggled the flag; I don't think anyone would accidentally perform such an import - and the flag is immutable via `setwalletflag`.' |
| rkrux (member) | objection, maintenance | hardcoding descriptor checksums in tests creates fragile test fixtures when descriptors change | answered · author agreed to fix on next rebase · v1, not yet re-checked | answered | yes | 2026-06-27: 'Let's avoid hardcoding the descriptor checksum and instead use the `descsum_create` utility function.' |
| bitsagarob (none) | objection, correctness | omitting spent_coins in SyncTransaction prevents block-connected transactions from being scanned, leaving mined payments undetected until manual rescan | answered · author agreed to fix on next rebase · v1, not yet re-checked | answered | yes | 2026-09-18: 'spent_coins is built here but not passed to SyncTransaction, and IsMine gates silent payment scanning on !spent_coins.empty(), so block-connected transactions are never scanned.' |
Participants:
- Sjors (member, objection, 10 statements 2025-07-14 to 2025-09-18): tested wallet workflows, identified block filter and change issues, and raised concern about immutable silent-payments wallet flag
- josibake (member, objection, 55 statements 2025-07-15 to 2025-09-09): provided architectural review on descriptor handling and warned against generic key-tweaking footguns
- macgyver13 (none, objection, 7 statements 2025-07-25 to 2025-08-18): reported descriptor import failure with private keys and derivation path issues
- PedroRosalba (none, question, 1 statements 2026-02-08 to 2026-02-08): asked about progress and blocking issues for the PR
- rkrux (member, objection, 4 statements 2026-06-27 to 2026-06-27): reviewed functional tests, noting hardcoded checksums and duplicate helpers
- bitsagarob (none, objection, 1 statements 2026-09-18 to 2026-09-18): reported that block-connected transactions omit spent_coins in SyncTransaction, causing mined payments to be missed
Dependencies
Enables:
- BIP 352 Silent Payments recipient labels and complete silent payment wallet support
Files
3676 lines under test/bench/ci.
- src/secp256k1/src/modules/silentpayments/bip352_send_and_receive_test_vectors.json +5729/-0
- src/secp256k1/src/modules/silentpayments/vectors.h +4550/-0
- src/test/data/bip352_send_and_receive_vectors.json +2760/-0
- src/secp256k1/src/modules/silentpayments/tests_impl.h +844/-0
- src/secp256k1/src/modules/silentpayments/main_impl.h +782/-0
- src/secp256k1/examples/silentpayments.c +463/-0
- src/secp256k1/include/secp256k1_silentpayments.h +400/-0
- test/functional/wallet_silentpayments_receiving.py +391/-0
- src/common/bip352.cpp +372/-0
- src/secp256k1/tools/tests_silentpayments_generate.py +301/-0
- src/wallet/scriptpubkeyman.cpp +249/-2
- src/wallet/spend.cpp +213/-34
- src/secp256k1/src/modules/silentpayments/bench_impl.h +227/-0
- src/test/bip352_tests.cpp +202/-0
- test/functional/wallet_silentpayments_sending.py +201/-0
- src/wallet/wallet.cpp +170/-19
- src/script/descriptor.cpp +175/-2
- src/common/bip352.h +161/-0
- src/secp256k1/src/ctime_tests.c +78/-0
- src/secp256k1/src/bench.c +47/-17
- src/wallet/walletutil.cpp +53/-11
- src/wallet/scriptpubkeyman.h +55/-5
- src/test/descriptor_tests.cpp +52/-0
- src/key_io.cpp +45/-3
- src/secp256k1/.github/workflows/ci.yml +28/-18
- src/wallet/rpc/spend.cpp +29/-1
- src/wallet/walletdb.cpp +27/-0
- src/secp256k1/Makefile.am +25/-0
- src/addresstype.h +20/-1
- src/wallet/test/wallet_tests.cpp +21/-0
- src/wallet/wallet.h +14/-7
- src/key.h +19/-0
- src/node/interfaces.cpp +13/-3
- src/secp256k1/configure.ac +14/-0
- src/validation.cpp +9/-3
- src/key.cpp +10/-1
- src/qt/forms/createwalletdialog.ui +10/-0
- src/test/fuzz/script.cpp +9/-1
- src/interfaces/chain.h +8/-1
- src/qt/createwalletdialog.cpp +9/-0
- src/script/descriptor.h +9/-0
- src/secp256k1/src/CMakeLists.txt +9/-0
- src/wallet/rpc/wallet.cpp +8/-1
- src/wallet/types.h +6/-2
- src/wallet/walletutil.h +7/-1
- src/rpc/util.cpp +7/-0
- src/secp256k1/src/tests.c +7/-0
- src/secp256k1/src/bench.h +3/-3
- src/secp256k1/src/modules/musig/session_impl.h +2/-4
- src/test/policyestimator_tests.cpp +3/-3
- src/validationinterface.cpp +3/-3
- src/wallet/rpc/backup.cpp +5/-1
- test/functional/wallet_fast_rescan.py +6/-0
- src/addresstype.cpp +5/-0
- src/kernel/chainparams.cpp +5/-0
- src/outputtype.cpp +5/-0
- src/secp256k1/src/modules/silentpayments/Makefile.am.include +5/-0
- src/test/fuzz/util.cpp +5/-0
- src/validationinterface.h +3/-2
- src/secp256k1/examples/CMakeLists.txt +4/-0
- src/secp256k1/src/secp256k1.c +4/-0
- src/test/fuzz/package_eval.cpp +2/-2
- src/test/transaction_tests.cpp +2/-2
- src/wallet/coincontrol.h +3/-1
- src/wallet/interfaces.cpp +4/-0
- src/interfaces/wallet.h +3/-0
- src/qt/receivecoinsdialog.cpp +3/-0
- src/qt/walletcontroller.cpp +3/-0
- src/secp256k1/ci/ci.sh +2/-1
- src/test/fuzz/util/wallet.h +2/-1
- src/wallet/rpc/addresses.cpp +2/-1
- src/wallet/walletdb.h +3/-0
- src/kernel/chainparams.h +2/-0
- src/outputtype.h +2/-0
- src/policy/fees/block_policy_estimator.cpp +1/-1
- src/policy/fees/block_policy_estimator.h +1/-1
- src/secp256k1/CMakeLists.txt +2/-0
- src/secp256k1/README.md +2/-0
- src/secp256k1/ci/linux-debian.Dockerfile +1/-1
- src/test/CMakeLists.txt +2/-0
- src/test/fuzz/tx_pool.cpp +1/-1
- src/wallet/spend.h +2/-0
- src/zmq/zmqnotificationinterface.cpp +1/-1
- src/zmq/zmqnotificationinterface.h +1/-1
- test/functional/rpc_getdescriptorinfo.py +2/-0
- test/functional/test_runner.py +2/-0
- test/functional/wallet_anchor.py +1/-1
- src/CMakeLists.txt +1/-0
- src/bech32.h +1/-0
- src/pubkey.cpp +0/-1
- src/qt/createwalletdialog.h +1/-0
- src/rpc/client.cpp +1/-0
- src/secp256k1/.gitignore +1/-0
- src/wallet/test/util.cpp +1/-0
Uncertainties
- Whether the secp256k1 subtree commits will be separated cleanly into an upstream dependency bump prior to rebasing this PR.
Card
The short text the ranking pass compares across PRs.
This pull request implements BIP 352 Silent Payments receiving for the Bitcoin Core wallet. It introduces the sp() descriptor, a dedicated ScriptPubKeyMan to manage keys and tweaks, full block scanning routines, and automated silent payments change handling. This represents a significant user privacy enhancement by eliminating address reuse for incoming transactions. Review is currently blocked by merge conflicts and upstream libsecp256k1 dependency work in issue #1799.
Data
thread read through 2026-09-22 (122 statements), head c773e16628
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: Needs rebase · draft
Categories
Utilities (logging, arguments, libraries)
P2 · new feature (ranking pass: Large feature, but its utils-relevant content is secp256k1 primitives still pending upstream, so it sits mid-band.)
Implements substantial new cryptographic primitives for BIP 352 inside the vendored libsecp256k1 subtree and common crypto utilities. Adding elliptic curve modules and test vectors directly affects the shared cryptographic primitives used across the codebase.
Membership: Adds extensive libsecp256k1 BIP 352 module code (+18k lines) and common BIP 352 cryptographic primitives.
Factors: security/stability 1, bug 0, performance 0, user value 3, leverage 2
Wallet
P2 · new feature (ranking pass: Moved down: depends on the sending work (#35302) and on upstream libsecp256k1, so it is not the first thing a reviewer can land.)
Introduces major user-facing privacy capabilities to the wallet through BIP 352 silent payment reception. While solving address reuse and transaction graph linkage without compromising privacy is a high user-value addition, as a brand-new feature it fits strategic milestone priority rather than critical release-blocking bug/fund-safety fixes.
Membership: Implements BIP 352 silent payment receiving, adding sp() descriptors, SilentPaymentDescriptorScriptPubKeyMan, and block scanning.
Factors: security/stability 1, bug 0, performance 0, user value 3, leverage 2