Silent Payments: Implement BIP352 by josibake · Pull Request #28122 · bitcoin/bitcoin

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:

#33625 (Update secp256k1 subtree to latest master by fanquake)
#32579 (p2p: Correct unrealistic headerssync 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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

exctracting -> extracting [spelling error in comment; misspelling impedes comprehension]
desintaions -> destinations [spelling error in documentation sentence]
std::<CPubKey, uint156> -> std::pair<CPubKey, uint256> [invalid/garbled type in documentation; wrong template/name and wrong integer size]
intendended -> intended [typo in function description]
faily -> fail [typo in comment causing confusion]
A explanation -> An explanation [article error in comment; should be "An" before vowel sound]

No other added code comments or documentation contained typographic/grammatical errors that made the English invalid or incomprehensible.

^{drahtbot_id_5_m}

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-04-04T10:51:23Z

🚧 At least one of the CI tasks failed.
_{Debug: https://github.com/bitcoin/bitcoin/runs/39980345983}

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.

DrahtBot · 2025-05-14T11:28:28Z

🚧 At least one of the CI tasks failed.
_{Task lint: https://github.com/bitcoin/bitcoin/runs/42203729137}
_{LLM reason (✨ experimental): The CI failure is due to a lint check failure related to subtree merging.}

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.

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.

src/common/bip352.cpp

aa85bfb530 docs: update README 9f42a30b82 ci: enable silentpayments module d504e48145 tests: add sha256 tag test 124750d580 tests: add constant time tests b35ffa2e30 tests: add BIP-352 test vectors 038c5b9c9d silentpayments: add benchmarks for scanning 88eb3d4545 silentpayments: add examples/silentpayments.c 22b20fd617 silentpayments: receiving df1de93765 silentpayments: recipient label support 76a0451c76 silentpayments: sending 3cd3a93bff build: add skeleton for new silentpayments (BIP352) module f36afb8b3d Merge bitcoin-core/secp256k1#1725: tests: refactor tagged hash verification 5153cf1c91 tests: refactor tagged hash tests d2dcf52091 Merge bitcoin-core/secp256k1#1726: docs: fix broken link to Tromer's cache.pdf paper 489a43d1bf docs: fix broken link to eprint cache.pdf paper d599714147 Merge bitcoin-core/secp256k1#1722: docs: Exclude modules' `bench_impl.h` headers from coverage report 0458def51e doc: Add `--gcov-ignore-parse-errors=all` option to `gcovr` invocations 1aecce5936 doc: Add `--merge-mode-functions=separate` option to `gcovr` invocations 106a7cbf41 doc: Exclude modules' `bench_impl.h` headers from coverage report a9e955d3ea autotools, docs: Adjust help string for `--enable-coverage` option e523e4f90e Merge bitcoin-core/secp256k1#1720: chore(ci): Fix typo in Dockerfile comment 24ba8ff168 chore(ci): Fix typo in Dockerfile comment 74b8068c5d Merge bitcoin-core/secp256k1#1717: test: update wycheproof test vectors c25c3c8a88 test: update wycheproof test vectors 20e3b44746 Merge bitcoin-core/secp256k1#1688: cmake: Avoid contaminating parent project's cache with `BUILD_SHARED_LIBS` 2c076d907a Merge bitcoin-core/secp256k1#1711: tests: update Wycheproof 7b07b22957 cmake: Avoid contaminating parent project's cache with BUILD_SHARED_LIBS 5433648ca0 Fix typos and spellings 9ea54c69b7 tests: update Wycheproof files git-subtree-dir: src/secp256k1 git-subtree-split: aa85bfb530b9ffc3dde6eaa7a976e129b8bd2f58

…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.

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

DrahtBot · 2025-10-19T15:22:25Z

🐙 This pull request conflicts with the target branch and needs rebase.

DrahtBot · 2026-01-16T02:00:10Z

⌛ There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it with one of the labels 'Up for grabs' or 'Insufficient Review', so that it can be picked up in the future.

PedroRosalba · 2026-02-07T20:08:04Z

hey guys what is the status of the BIP 352? is it still being worked on? has the feature been completed? if missing something can you guys elaborate on what?

macgyver13 · 2026-02-08T05:20:35Z

hey guys what is the status of the BIP 352? is it still being worked on? has the feature been completed? if missing something can you guys elaborate on what?

Yes, development is active and progressing on multiple fronts:

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