Skip to content

BIP-352: add test vector for edge case - input key intermediate sum zero#2142

Merged
jonatack merged 2 commits into
bitcoin:masterfrom
macgyver13:bip352-intermediate-sum
Apr 17, 2026
Merged

BIP-352: add test vector for edge case - input key intermediate sum zero#2142
jonatack merged 2 commits into
bitcoin:masterfrom
macgyver13:bip352-intermediate-sum

Conversation

@macgyver13
Copy link
Copy Markdown
Contributor

@shuv-amp discovered this edge case and reported to bdk-sp and BlueWallet.

Exercises [A, -A, A] input key pattern where the intermediate sum hits zero after the first two keys, but the final sum is non-zero. Implementations that validate after each pairwise addition (rather than summing all keys first) will incorrectly reject this case.

@theStack - the reference implementation summation handles this case correctly as-is

@murchandamus murchandamus added Proposed BIP modification PR by non-owner to update BIP content Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified labels Apr 15, 2026
@theStack
Copy link
Copy Markdown
Contributor

Concept ACK

Thanks for adding! Can you add a brief entry to the changelog with bumped PATCH version?

@murchandamus murchandamus removed the Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified label Apr 16, 2026
Exercises [A, -A, A] input key pattern where the intermediate sum
hits zero after the first two keys, but the final sum is non-zero.
Implementations that validate after each pairwise addition (rather
than summing all keys first) will incorrectly reject this case.
Add patch version 1.1.1 to Changelog
Remove extra leading double-quote in CoinJoin ref name
@macgyver13 macgyver13 force-pushed the bip352-intermediate-sum branch from eaa80d3 to 00957af Compare April 16, 2026 16:07
Copy link
Copy Markdown
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for latest push diff only (removal of extra double-quote and changelog entry)

Copy link
Copy Markdown
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 00957af

Verified that the first two input keys cancel each other out and the corresponding pubkeys match as well:

$ uv run --with git+https://github.com/secp256k1lab/secp256k1lab python
Python 3.11.13 (main, Aug 18 2025, 19:22:12) [Clang 20.1.4 ] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from secp256k1lab.secp256k1 import Scalar, GE, G
>>> seckey1 = Scalar.from_bytes_checked(bytes.fromhex("a6df6a0bb448992a301df4258e06a89fe7cf7146f59ac3bd5ff26083acb22ceb"))
>>> seckey2 = Scalar.from_bytes_checked(bytes.fromhex("592095f44bb766d5cfe20bda71f9575ed2df6b9fb9addc7e5fdffe0923841456"))
>>> seckey1 + seckey2
Scalar(0x0)
>>> (seckey1 * G).to_bytes_compressed().hex()
'02557ef3e55b0a52489b4454c1169e06bdea43687a69c1f190eb50781644ab6975'
>>> (seckey2 * G).to_bytes_compressed().hex()
'03557ef3e55b0a52489b4454c1169e06bdea43687a69c1f190eb50781644ab6975'

@jonatack
Copy link
Copy Markdown
Member

Sanity-checked the new test (second to last) passes.

jon|(00957af8...):~/bitcoin/bips/bip-0352$ ./reference.py send_and_receive_test_vectors.json
Simple send: two inputs
Simple send: two inputs, order reversed
Simple send: two inputs from the same transaction
Simple send: two inputs from the same transaction, order reversed
Outpoint ordering byte-lexicographically vs. vout-integer
Single recipient: multiple UTXOs from the same public key
Single recipient: taproot only inputs with even y-values
Single recipient: taproot only with mixed even/odd y-values
Single recipient: taproot input with even y-value and non-taproot input
Single recipient: taproot input with odd y-value and non-taproot input
Multiple outputs: multiple outputs, same recipient
Multiple outputs: multiple outputs, multiple recipients
Receiving with labels: label with even parity
Receiving with labels: label with odd parity
Receiving with labels: large label integer
Multiple outputs with labels: un-labeled and labeled address; same recipient
Multiple outputs with labels: multiple outputs for labeled address; same recipient
Multiple outputs with labels: un-labeled, labeled, and multiple outputs for labeled address; same recipients
Single recipient: use silent payments for sender change
Single recipient: taproot input with NUMS point
Pubkey extraction from malleated p2pkh
P2PKH and P2WPKH Uncompressed Keys are skipped
Skip invalid P2SH inputs
Recipient ignores unrelated outputs
No valid inputs, sender generates no outputs
Input keys sum up to zero / point at infinity: sending fails, receiver skips tx
Input keys intermediate sum is zero but final sum is non-zero
Maximum per-group recipient limit K_max is exceeded (2324 matches): sending fails, receiver doesn't scan beyond limit
All tests passed

@jonatack jonatack merged commit 83c1fc8 into bitcoin:master Apr 17, 2026
4 checks passed
macgyver13 added a commit to macgyver13/bdk-sp that referenced this pull request May 19, 2026
…rialization

Refresh BIP-352 test vectors to match version [1.1.1](bitcoin/bips#2142)
- Add tests 26 & 27
- Modify serialization to parse recipients map
nymius added a commit to bitcoindevkit/bdk-sp that referenced this pull request May 21, 2026
bcac039 test: refresh BIP-352 test vectors to v1.1.1 (macgyver13)

Pull request description:

  ### Description

  Refresh BIP-352 test vectors to match version [1.1.1](bitcoin/bips#2142)
  - Add tests 26 & 27 to send.rs and receive.rs.
  - Modify serialization to parse the updated sending.given.recipients map and maintain receiving.expected.addresses string format.

  ### Notes to the reviewers

  cargo t --test functional_tests bip352_vectors

  All tests pass - 3 are ignored for this PR
  ```
  test bip352_vectors::send::input_keys_intermediate_sum_is_zero_but_final_sum_is_non_zero ... ignored, currently fails due to intermediate sum being zero
  test bip352_vectors::send::maximum_per_group_recipient_limit_k_max_is_exceeded ... ignored, limit k-max not implemented
  test bip352_vectors::receive::maximum_per_group_recipient_limit_k_max_is_exceeded ... ignored, limit k-max not implemented
  ```

  Resolving ignored tests will be handled in future PRs.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [conventional commit guidelines](https://www.conventionalcommits.org/en/v1.0.0/)
  * [x] I ran `just p` (fmt, clippy and test) before committing

ACKs for top commit:
  nymius:
    ACK bcac039

Tree-SHA512: ae04dfaec03e833605d2deb7262b168c0892484432f7ebc8263e3db46cdfd54e5d24b34da65de9fa69bc2c5f94d8de901d6dbba3271f8ef89d422ca25baff40c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Proposed BIP modification PR by non-owner to update BIP content

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants