silentpayments: address #1765 follow-ups, add changelog entry #1896

pull theStack wants to merge 2 commits into bitcoin-core:master from theStack:pr1765_sp_followups changing 4 files +30 −22
  1. theStack commented at 12:44 PM on July 23, 2026: contributor

    This PR addresses follow-up reviewer comments and Claude nits from #1765, see

    and adds an entry to the changelog.

  2. in include/secp256k1_silentpayments.h:106 in 1701e0e7e7
     111 | + *                            within the array, i.e., after the call (including on
     112 | + *                            failure), the index fields of the recipient objects
     113 | + *                            may no longer correspond to the positions in the
     114 | + *                            array. Multiple recipient objects with the same scan
     115 | + *                            public key and/or same spend public key can be passed
     116 | + *                            if they carry a different index.
    


    real-or-random commented at 12:54 PM on July 23, 2026:
     *                            if they carry different indices.
    
  3. in src/modules/silentpayments/main_impl.h:654 in 1701e0e7e7
     650 | @@ -648,6 +651,8 @@ int secp256k1_silentpayments_recipient_scan_outputs(
     651 |      }
     652 |      secp256k1_ge_from_bytes(&prevouts_pubkey_sum_ge, &prevouts_summary->data[5]);
     653 |      combined = (int)prevouts_summary->data[4];
     654 | +    /* Note that the "combined" flag can currently only be 0, as we only have support for full-nodes, i.e.
    


    real-or-random commented at 12:54 PM on July 23, 2026:
        /* Note that the "combined" flag can currently only be 0, as we only have support for full nodes, i.e.,
    
  4. in include/secp256k1_silentpayments.h:85 in 1701e0e7e7
      86 | - *             - A hash output is not a valid scalar (negligible probability
      87 | - *               per hash evaluation)
      88 | - *             - Any group (i.e. recipients sharing the same scan public key) exceeds
      89 | - *               the protocol limit SECP256K1_SILENTPAYMENTS_RECIPIENT_GROUP_LIMIT
      90 | + *             - An invalid output public key is created (can only happen for an
      91 | + *               adversarially chosen recipient spend public key)
    


    real-or-random commented at 1:01 PM on July 23, 2026:

    just nits:

     *           0 on failure, i.e., when one of the following occurs:
     *             - The size of any group (i.e. recipients sharing the same scan public key) exceeds
     *               the protocol limit SECP256K1_SILENTPAYMENTS_RECIPIENT_GROUP_LIMIT.
    *             - The sum of the all input secret keys is 0.
    *               (This occurs only with negligible probability if at least one of the input secret
    *               keys is uniformly random and independent of all other keys.)
     *             - An invalid output public key is created. (This can only happen for an
     *               adversarially chosen recipient spend public key.)
    
  5. in CHANGELOG.md:14 in 1701e0e7e7
       6 | @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
       7 |  
       8 |  ## [Unreleased]
       9 |  
      10 | +#### Added
      11 | + - New module `silentpayments` implements sending and receiving of Silent Payments according to the [BIP 352 specification](https://github.com/bitcoin/bips/blob/master/bip-0352.mediawiki). See:
      12 | +   - Header file `include/secp256k1_silentpayments.h` which defines the new API.
      13 | +   - Usage example `examples/silentpayments.c`.
      14 | + - The `silentpayments` API currently requires full access to the transaction data (light client support is not implemented).
    


    real-or-random commented at 1:04 PM on July 23, 2026:
     - The `silentpayments` API currently requires full access to the transaction data (light client scanning is not implemented).
    

    a bit more precise, I think

  6. in CHANGELOG.md:11 in 1701e0e7e7
       6 | @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
       7 |  
       8 |  ## [Unreleased]
       9 |  
      10 | +#### Added
      11 | + - New module `silentpayments` implements sending and receiving of Silent Payments according to the [BIP 352 specification](https://github.com/bitcoin/bips/blob/master/bip-0352.mediawiki). See:
    


    real-or-random commented at 1:04 PM on July 23, 2026:

    micro nit but I believe it's just simpler:

     - New module `silentpayments` implements sending and receiving of Silent Payments according to [BIP 352](https://github.com/bitcoin/bips/blob/master/bip-0352.mediawiki). See:
    
  7. real-or-random commented at 1:05 PM on July 23, 2026: contributor

    ACK mod some language nits

  8. real-or-random commented at 2:46 PM on July 23, 2026: contributor
  9. silentpayments: API docs and internal comment followups
    Addresses follow-up comments and Claude nits from #1765, see
    - https://github.com/bitcoin-core/secp256k1/pull/1765#discussion_r3629070333
    - https://github.com/bitcoin-core/secp256k1/pull/1765#discussion_r3628888249
    - https://github.com/bitcoin-core/secp256k1/pull/1765#discussion_r3629066762
    - https://github.com/bitcoin-core/secp256k1/pull/1765#pullrequestreview-4754517850
    
    Note that the "hash output is not a valid scalar" error mentioning in
    the API docs for sending is dropped intentionally to reflect the
    scanning API docs, where we don't mention this negligible case either.
    
    Co-authored-by: Tim Ruffing <me@real-or-random.org>
    0fa38f3d29
  10. Add changelog entry for silentpayments module d9ac2ee5e6
  11. theStack force-pushed on Jul 23, 2026
  12. in include/secp256k1_silentpayments.h:96 in d9ac2ee5e6
      95 | @@ -97,16 +96,14 @@ typedef struct secp256k1_silentpayments_recipient {
      96 |   *  In:           recipients: pointer to an array of pointers to Silent Payments
    


    real-or-random commented at 2:53 PM on July 23, 2026:

    Did you keep this intentionally an "In" argument? (I guess that's a fine decision, just asking if it was an oversight)


    theStack commented at 3:07 PM on July 23, 2026:

    Oh sorry, wanted to reply to this below the suggesting comment but forgot. Whether this should be "In" or "In/Out" was discussed in take 2 already, and it seemed "In" was preferred at least back then: #1519 (review) (b10c mirror).

  13. real-or-random approved
  14. real-or-random commented at 2:57 PM on July 23, 2026: contributor

    ACK d9ac2ee5e69b2fd1df1c6651e650856f7c54a95f

  15. real-or-random added the label user-documentation on Jul 23, 2026
  16. real-or-random added the label tweak/refactor on Jul 23, 2026

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-07-26 01:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me