Silent Payments: Implement BIP352 #28122

josibake · 2023-07-21T18:36:37Z

This PR is part of integrating silent payments into Bitcoin Core. The project is tracked in #28536

BIP352

This PR focuses strictly on the BIP logic and attempts to separate it from the wallet and transaction implementation details. This is accomplished by working directly with public and private keys, instead of needing a wallet backend and transactions for testing. Labels for the receiver are optional and thus deferred for a later PR.

Test vectors from the BIP are included as unit tests.

DrahtBot · 2023-07-21T18:36:41Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/28122.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32579 (headerssync: Preempt unrealistic unit test behavior by hodlinator)
#31974 (Drop testnet3 by Sjors)
#27260 (Enhanced error messages for invalid network prefix during address parsing. by portlandhodl)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

src/kernel/chainparams.cpp

src/wallet/silentpayments.h

src/key.cpp

aureleoules

Left some comments. I will review further later.

src/pubkey.h

src/wallet/silentpayments.cpp

josibake · 2023-08-01T15:48:43Z

Needs rebase, if still relevant

thanks, fixed!

src/kernel/chainparams.cpp

src/bech32.cpp

src/key_io.cpp

src/key.cpp

src/key_io.cpp

src/wallet/silentpayments.cpp

DrahtBot · 2025-07-16T09:28:44Z

🚧 At least one of the CI tasks failed.
_{Task lint: https://github.com/bitcoin/bitcoin/runs/46079814035}
_{LLM reason (✨ experimental): The CI failure is caused by errors in the lint step.}

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

josibake · 2025-07-16T09:50:27Z

CI failure is due to the "test each commit" job not being able to handle a subtree merge commit. Ignoring for now; will look into fixing the CI job.

Sjors · 2025-07-17T07:51:33Z

will look into fixing the CI job

Just add a dummy commit so the subtree merge is more than 6 deep :-)

Anyway, if you have to rebase again in the future, can you go past #32948? That makes it easier for me to rebase my index PR.

DrahtBot · 2025-07-24T18:18:03Z

🚧 At least one of the CI tasks failed.
_{Task MSan, depends: https://github.com/bitcoin/bitcoin/runs/46645416048}
_{LLM reason (✨ experimental): Memory sanitizer detected use of uninitialized memory during secp256k1 tests, causing CI failure.}

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

1580e3b fuzz: add ConstructPubKeyBytes function (josibake) Pull request description: In bitcoin#28246 and bitcoin#28122 , we add a `PubKeyDestination` and a `V0SilentPaymentsDestination`. Both of these PRs update `fuzz/util.cpp` and need a way to create well-formed pubkeys. Currently in `fuzz/util.cpp`, we have some logic for creating pubkeys in the multisig data provider. This logic is duplicated in bitcoin#28246 and duplicated again in bitcoin#28122. Seems much better to have a `ConstructPubKeyBytes` function that both PRs (and any future work) can reuse. This PR introduces a function to do this and has the existing code use it. While the purpose is to introduce a utility function, the previous multisig code used `ConsumeIntegralInRange(4, 7)` which would have created some uncompressed pubkeys with the prefix 0x05, which is incorrect (see https://bitcoin.stackexchange.com/questions/57855/c-secp256k1-what-do-prefixes-0x06-and-0x07-in-an-uncompressed-public-key-signif) tldr; using `PickValueFromArray` is more correct as it limits to the set of defined prefixes for compressed and uncompressed pubkeys. ACKs for top commit: Sjors: ACK 1580e3b Tree-SHA512: c87c8bcd1f6b3a97ef772be93102efb912811c59f32211cfd531a116f1da8a57c8c6ff106b34f2a2b88d8b34fb5bc30d9f9ed6d2720113ffcaaa2f8d5dc9eb27

2b57d2ad89 docs: update README 5d2d4adabd ci: enable silentpayments module d6788a888c tests: add constant time tests 4c32ba9613 tests: add BIP-352 test vectors 183e5414f7 silentpayments: add benchmarks for scanning 288390446a silentpayments: add examples/silentpayments.c 250beff5d2 silentpayments: receiving f90d7a76b0 silentpayments: recipient label support 1a5f53f2cf silentpayments: sending ca2538a878 build: add skeleton for new silentpayments (BIP352) module git-subtree-dir: src/secp256k1 git-subtree-split: 2b57d2ad8964e536508fae0b6ab1331396fe0308

…secp256k1

Add a method for passing a KeyPair object to secp256k1 functions expecting a secp256k1_keypair. This allows for passing a KeyPair directly to a secp256k1 function without needing to create a temporary secp256k1_keypair object.

src/common/bip352.cpp

Wrap the silentpayments module from libsecp256k1. This is placed in common as it is intended to be used by: * RPCs: for parsing addresses * Wallet: for sending, receiving, spending silent payment outputs * Node: for creating silent payment indexes for light clients

Have `IsValidDestination` return false for silent payment destinations and set an error string when decoding a silent payment address. This prevents anyone from sending to a silent payment address before sending is implemented in the wallet, but also allows the functions to be used in the unit testing famework.

Use the test vectors to test sending and receiving. A few cases are not covered here, namely anything that requires testing specific to the wallet. For example: * Taproot script path spending is not tested, as that is better tested in a wallets coin selection / signing logic * Re-computing outputs during RBF is not tested, as that is better tested in a wallets RBF logic The unit tests are written in such a way that adding new test cases is as easy as updating the JSON file

josibake mentioned this pull request Jul 21, 2023

Silent Payments: send and receive #27827

Closed

3 tasks

josibake force-pushed the implement-bip352 branch from db0ab56 to be049d2 Compare July 21, 2023 18:54

DrahtBot added the CI failed label Jul 21, 2023

josibake force-pushed the implement-bip352 branch from be049d2 to 13529c9 Compare July 21, 2023 19:23

This was referenced Jul 22, 2023

build: pass sanitize flags to instrument libsecp256k1 code #27991

Closed

wallet: add seeds argument to importdescriptors #27351

Closed

theStack reviewed Jul 22, 2023

View reviewed changes

src/kernel/chainparams.cpp Show resolved Hide resolved

josibake force-pushed the implement-bip352 branch from 13529c9 to 1d75df2 Compare July 23, 2023 12:18

jonatack reviewed Jul 23, 2023

View reviewed changes

src/wallet/silentpayments.h Outdated Show resolved Hide resolved

josibake force-pushed the implement-bip352 branch from 1d75df2 to dc18efa Compare July 26, 2023 09:10

DrahtBot removed the CI failed label Jul 26, 2023

rajarshimaitra mentioned this pull request Jul 26, 2023

Silent Payments: Implement BIP352 #28122 Bitshala/BitcoinCore-PR-Review-Club#39

Closed

achow101 reviewed Jul 26, 2023

View reviewed changes

src/key.cpp Outdated Show resolved Hide resolved

src/key.cpp Outdated Show resolved Hide resolved

aureleoules reviewed Jul 28, 2023

View reviewed changes

DrahtBot added the CI failed label Jul 29, 2023

josibake force-pushed the implement-bip352 branch from dc18efa to fa00edf Compare August 1, 2023 15:48

DrahtBot removed the CI failed label Aug 1, 2023

josibake force-pushed the implement-bip352 branch from fa00edf to 8ee791e Compare August 2, 2023 12:42

This was referenced Aug 2, 2023

Silent Payments: sending #28201

Draft

Silent Payments: receiving #28202

Closed

murchandamus reviewed Aug 2, 2023

View reviewed changes

src/kernel/chainparams.cpp Show resolved Hide resolved

josibake commented Aug 3, 2023

View reviewed changes

src/bech32.cpp Outdated Show resolved Hide resolved

BrandonOdiwuor reviewed Aug 4, 2023

View reviewed changes

src/key_io.cpp Outdated Show resolved Hide resolved