Silent Payments: Implement bip352 (take 2) #35301

pull Eunovo wants to merge 8 commits into bitcoin:master from Eunovo:implement-bip352 changing 44 files +17149 −52
  1. Eunovo commented at 3:46 AM on May 16, 2026: contributor

    This PR is part of integrating silent payments into Bitcoin Core. It is the second iteration of #28122, now based on https://github.com/bitcoin-core/secp256k1/pull/1765.

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

  2. Squashed 'src/secp256k1/' changes from 7262adb4b4..14be7f2abe
    14be7f2abe silentpayments: skip slow benchmarks for low iters count (<= 2)
    ea46d6502e docs: update README
    8587aeb029 ci: enable silentpayments module
    1375d38a32 tests: add sha256 tag test
    eb3748dab1 tests: add constant time tests
    e1674f389a tests: add BIP-352 test vectors
    9163035ab7 silentpayments: optimize scanning by using batch inversion
    dc36ff6ec3 silentpayments: add benchmarks for scanning
    b7f09f3eb5 silentpayments: add examples/silentpayments.c
    460565c662 silentpayments: respect per-group recipients protocol limit (K_max=2323)
    72658fc85e silentpayments: receiving
    c7a64d0cd8 silentpayments: recipient label support
    be56a66992 silentpayments: sending
    7bfae739e3 build: add skeleton for new silentpayments (BIP352) module
    b11340b3ce Merge bitcoin-core/secp256k1#1849: musig: always clear out secret key in `secp256k1_musig_nonce_gen_counter`
    8479eafa57 musig: always clear out secret key in `secp256k1_musig_nonce_gen_counter`
    c1a9e4fe64 Merge bitcoin-core/secp256k1#1848: ci: Bump GCC snapshot major version to 17
    3cca6451a2 ci: Bump GCC snapshot major version to 17
    ea174fe045 Merge bitcoin-core/secp256k1#1846: ci: Replace `ilammy/msvc-dev-cmd` with manual MSVC setup
    285cb788e9 ci: Replace `ilammy/msvc-dev-cmd` with manual MSVC setup
    
    git-subtree-dir: src/secp256k1
    git-subtree-split: 14be7f2abe4797c1a58249cb901cc6f02972cb4d
    bb1bce36cf
  3. Merge commit 'bb1bce36cf8b9292a65f0f03ab855e7133357b9f' into new-sp-base ece2374760
  4. crypto: add read-only method to KeyPair
    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.
    bccfd81105
  5. Add "sp" HRP 602835e080
  6. DrahtBot commented at 3:46 AM on May 16, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

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

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  7. in src/common/bip352.cpp:25 in 6355a90494
      20 | +#include <script/solver.h>
      21 | +#include <script/script_error.h>
      22 | +#include <util/check.h>
      23 | +#include <util/strencodings.h>
      24 | +
      25 | +extern secp256k1_context* secp256k1_context_sign; // TODO: this is hacky, is there a better solution?
    


    theStack commented at 1:43 AM on May 18, 2026:

    in 6355a904945d0d5704032e464f20a69a5d82a91b: Since #34225 (commit f36d89f4363a25f7948a0f7096201ef8e15045d8), the sign context can be accessed via GetSecp256k1SignContext, i.e. by using this in the API calls below, this line and the symbol visibility change in key.cpp are not needed anymore


    Eunovo commented at 4:03 PM on May 18, 2026:

    Done.

  8. in src/common/bip352.h:103 in 6355a90494
      98 | + * Label public keys can be stored in a cache, mapping the public key to the label tweak. This cache
      99 | + * is used during scanning to determine if a label was used and if so to retrieve the label tweak.
     100 | + *
     101 | + * @param scan_key                 The recipient's scan_key, used to salt the hash
     102 | + * @param m                        An integer m (only use m = 0 for the change label)
     103 | + * @return std::<CPubKey, uint156> The label public key and label tweak.
    


    theStack commented at 1:59 AM on May 18, 2026:

    in 6355a904945d0d5704032e464f20a69a5d82a91b: typo: s/uint156/uint256/


    Eunovo commented at 4:03 PM on May 18, 2026:

    Done.

  9. in src/common/bip352.h:156 in 6355a90494 outdated
     151 | + * @param spend_pubkey                                      The recipient's spend public key.
     152 | + * @param output_pub_keys                                   The taproot output public keys.
     153 | + * @param labels                                            The recipient's labels.
     154 | + * @return std::<optional<std::vector<SilentPaymentOutput>> The found outputs, nullopt if none found.
     155 | + */
     156 | +std::optional<std::vector<SilentPaymentOutput>> ScanForSilentPaymentOutputs(const CKey& scan_key, const PrevoutsSummary& prevouts_summary, const CPubKey& spend_pubkey, const std::vector<XOnlyPubKey>& output_pub_keys, const std::map<CPubKey, uint256>& labels);
    


    theStack commented at 2:04 AM on May 18, 2026:

    in 6355a904945d0d5704032e464f20a69a5d82a91b: nit: missing doxygen @param entry for prevouts_summary above


    Eunovo commented at 4:04 PM on May 18, 2026:

    Done.

  10. in src/common/bip352.cpp:211 in 6355a90494 outdated
     206 | +    generated_output_ptrs.reserve(recipients.size());
     207 | +
     208 | +    for (size_t i = 0; i < recipients.size(); i++) {
     209 | +        secp256k1_silentpayments_recipient recipient_obj;
     210 | +        ret = secp256k1_ec_pubkey_parse(secp256k1_context_static, &recipient_obj.scan_pubkey, recipients[i].m_scan_pubkey.data(), recipients[i].m_scan_pubkey.size());
     211 | +        ret = secp256k1_ec_pubkey_parse(secp256k1_context_static, &recipient_obj.spend_pubkey, recipients[i].m_spend_pubkey.data(), recipients[i].m_spend_pubkey.size());
    


    theStack commented at 2:10 AM on May 18, 2026:

    in 6355a904945d0d5704032e464f20a69a5d82a91b:

            ret &= secp256k1_ec_pubkey_parse(secp256k1_context_static, &recipient_obj.spend_pubkey, recipients[i].m_spend_pubkey.data(), recipients[i].m_spend_pubkey.size());
    

    to ensure both pubkey_parse are successful (or alternatively, could place an extra assert(ret) line after the first call)


    Eunovo commented at 4:06 PM on May 18, 2026:

    I added an assert(ret); after the first call, so that it is easier to determine which of the pubkeys is invalid, in the event of a crash.

  11. in src/common/bip352.cpp:336 in 6355a90494
     331 | +        secp256k1_silentpayments_found_output found_output{};
     332 | +        secp256k1_xonly_pubkey tx_output_obj;
     333 | +        found_output_objs.push_back(found_output);
     334 | +        found_output_ptrs.push_back(&found_output_objs[i]);
     335 | +        ret = secp256k1_xonly_pubkey_parse(secp256k1_context_static, &tx_output_obj, tx_outputs[i].data());
     336 | +        assert(ret);
    


    theStack commented at 2:21 AM on May 18, 2026:

    in 6355a904945d0d5704032e464f20a69a5d82a91b: IIUC, this call could fail if a transaction is scanned where one of the P2TR outputs encodes an invalid x-only pubkey (i.e. not on the curve), so I suppose this should be changed to e.g. if (!ret) continue; to avoid a crash (unless we demand from the caller that tx_outputs only contain valid x-only pubkeys already)


    Eunovo commented at 4:04 PM on May 18, 2026:

    Changed to if (!ret) continue;

  12. in src/common/bip352.cpp:32 in 6355a90494
      27 | +namespace bip352 {
      28 | +
      29 | +class PrevoutsSummaryImpl
      30 | +{
      31 | +private:
      32 | +    //! The actual secnonce itself
    


    theStack commented at 2:24 AM on May 18, 2026:

    in 6355a904945d0d5704032e464f20a69a5d82a91b: comment doesn't apply


    Eunovo commented at 4:04 PM on May 18, 2026:

    Done.

  13. in src/addresstype.h:162 in ba4b734ec8
     158 | @@ -140,7 +159,7 @@ struct PayToAnchor : public WitnessUnknown
     159 |   *  * WitnessUnknown: TxoutType::WITNESS_UNKNOWN destination (P2W??? address)
     160 |   *  A CTxDestination is the internal data type encoded in a bitcoin address
     161 |   */
     162 | -using CTxDestination = std::variant<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>;
     163 | +using CTxDestination = std::variant<CNoDestination, V0SilentPaymentDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>;
    


    theStack commented at 2:30 AM on May 18, 2026:

    in ba4b734ec89d590951574d2714867afab27d1347: nit: could add a corresponding new entry to the comment list a few lines above


    Eunovo commented at 4:11 PM on May 18, 2026:

    Done.

  14. in src/bech32.h:40 in ba4b734ec8
      36 | @@ -37,6 +37,7 @@ enum class Encoding {
      37 |   *  and we would never encode an address with such a massive value */
      38 |  enum CharLimit : size_t {
      39 |      BECH32 = 90,            //!< BIP173/350 imposed character limit for Bech32(m) encoded addresses. This guarantees finding up to 4 errors.
      40 | +    SILENT_PAYMENTS = 1024, //!< BIP352 imposed 1024 character limit on Bech32m encoded silent payment addresses. This guarantees finding up to 3 errors
    


    theStack commented at 2:37 AM on May 18, 2026:

    in ba4b734ec89d590951574d2714867afab27d1347: pedantic nit: according to BIP-352 the limit is 1023 (not sure which of the two values make more sense, as I'm not very familiar with BIP-173; for SPV0 it doesn't matter anyways)


    Eunovo commented at 4:10 PM on May 18, 2026:

    Changed to 1023 to match the BIP specification. AFAICT, there is no reason to use 1024; the most likely reason for it being 1024 is that the BIP might have stated 1024 and was updated to 1023 at some point in the past.

  15. Add V0SilentPaymentDestination address type 6b71f145cc
  16. common: add bip352.{h,cpp} secp256k1 module
    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
    0287192f09
  17. wallet: disable sending to silent payment address
    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.
    09f361be4a
  18. tests: add BIP352 test vectors as unit tests
    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
    aca8a8f3da
  19. Eunovo force-pushed on May 18, 2026

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:51 UTC