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

Pre-work / Refactors

The first three commits are pre-work / refactors needed for this PR. They are broken out into the following PRS:

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.

Before reviewing, it is strongly recommended you read bitcoin/bips#1458 and take a look at the reference python implementation on the BIP.

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

c0db6509bd docs: update README 8339232b7e ci: enable silentpayments module 635745fc3a tests: add constant time tests b1de2ee2f7 tests: add BIP-352 test vectors aea372837f silentpayments: add benchmarks for scanning 1ec7857aed silentpayments: add examples/silentpayments.c c9bec084eb silentpayments: receiving 28fd17d7c4 silentpayments: recipient label support 065e8b7793 silentpayments: sending a6d8b11754 build: add skeleton for new silentpayments (BIP352) module 6274359346 bench: add ellswift to bench help output 0258186573 configure: Show exhaustive tests in summary 53b578d10b include: remove WARN_UNUSED_RESULT for functions always returning 1 f75c985604 ci: Fix exiting from ci.sh on error 947761b842 tests: remove unused uncounting_illegal_callback_fn 5d01f375c6 build: Drop no longer needed `-fvisibility=hidden` compiler option dbf1e95d2a ci: Run `tools/symbol-check.py` 8174c88f47 test: Add `tools/symbol-check.py` 8a287f9a32 Introduce `SECP256K1_LOCAL_VAR` macro 7106544a16 Remove deprecated _ec_privkey_{negate,tweak_add,tweak_mul} aliases 1e2da62eff gha: Print all *.log files, in a separate action REVERT: 4187a46649 Merge bitcoin-core/secp256k1#1492: tests: Add Wycheproof ECDH vectors REVERT: e266ba11ae tests: Add Wycheproof ECDH vectors REVERT: 13906b7154 Merge bitcoin-core/secp256k1#1669: gitignore: Add Python cache files REVERT: c1bcb03276 gitignore: Add Python cache files REVERT: 70f149b9a1 Merge bitcoin-core/secp256k1#1662: bench: add ellswift to bench help output REVERT: 6b3fe51fb6 bench: add ellswift to bench help output REVERT: d84bb83e26 Merge bitcoin-core/secp256k1#1661: configure: Show exhaustive tests in summary REVERT: 3f54ed8c1b Merge bitcoin-core/secp256k1#1659: include: remove WARN_UNUSED_RESULT for functions always returning 1 REVERT: 20b05c9d3f configure: Show exhaustive tests in summary REVERT: e56716a3bc Merge bitcoin-core/secp256k1#1660: ci: Fix exiting from ci.sh on error REVERT: d87c3bc58f ci: Fix exiting from ci.sh on error REVERT: 1b6e081538 include: remove WARN_UNUSED_RESULT for functions always returning 1 REVERT: 2abb35b034 Merge bitcoin-core/secp256k1#1657: tests: remove unused uncounting_illegal_callback_fn REVERT: 51907fa918 tests: remove unused uncounting_illegal_callback_fn REVERT: a7a5117144 Merge bitcoin-core/secp256k1#1359: Fix symbol visibility issues, add test for it REVERT: 13ed6f65dc Merge bitcoin-core/secp256k1#1593: Remove deprecated `_ec_privkey_{negate,tweak_add,tweak_mul}` aliases from API REVERT: d1478763a5 build: Drop no longer needed `-fvisibility=hidden` compiler option REVERT: 8ed1d83d92 ci: Run `tools/symbol-check.py` REVERT: 41d32ab2de test: Add `tools/symbol-check.py` REVERT: 88548058b3 Introduce `SECP256K1_LOCAL_VAR` macro REVERT: 03bbe8c615 Merge bitcoin-core/secp256k1#1655: gha: Print all *.log files, in a separate action REVERT: 59860bcc24 gha: Print all *.log files, in a separate action REVERT: 37d2c60bec Remove deprecated _ec_privkey_{negate,tweak_add,tweak_mul} aliases git-subtree-dir: src/secp256k1 git-subtree-split: c0db6509bd2cb0777ce0d335e2582f74364fb8ec

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

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 10000 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