silentpayments/send: false error when an intermediate sum hits zero

[shuv-amp]

The silentpayments sending path in SPDK appears to fail when the running partial sum of eligible input keys hits zero, even though the final sum of all eligible keys is non-zero.

I ran into this while testing multiple BIP 352 implementations and reduced it to the self-contained case below.

Minimal case

• three distinct P2WPKH inputs
• private keys are [A, -A, A] modulo secp256k1 order
• so the final sum is A, not zero

Concrete scalars:

A      = a6df6a0bb448992a301df4258e06a89fe7cf7146f59ac3bd5ff26083acb22ceb
-A mod n = 592095f44bb766d5cfe20bda71f9575ed2df6b9fb9addc7e5fdffe0923841456

These are attached to three distinct outpoints:

3a286147b25e16ae80aff406f2673c6e565418c40f45c071245cdebc8a94174e:0
3a286147b25e16ae80aff406f2673c6e565418c40f45c071245cdebc8a94174e:1
3a286147b25e16ae80aff406f2673c6e565418c40f45c071245cdebc8a94174e:2

Expected behavior

This should succeed if the rule is based on the full eligible-input sum:

A + (-A) + A = A

which is non-zero.

The BIP 352 reference-style full-sum logic sums the full set first and only fails if the final sum is zero. On this case it computes:

a_sum      = a6df6a0bb448992a301df4258e06a89fe7cf7146f59ac3bd5ff26083acb22ceb
input_hash = ea876d97952aaee5dd02c892a2ec6631790e7cdbbf84f9923e76a476ec7fa000
output     = bf300962adaaf21b58cf043d91ff46661b2688eeaaf753fe6ee8b642de3b715f

Actual behavior

Using the current silentpayments sending utility directly:

use hex::encode;
use silentpayments::secp256k1::SecretKey;
use silentpayments::utils::sending::calculate_partial_secret;

fn main() {
    let txid = "3a286147b25e16ae80aff406f2673c6e565418c40f45c071245cdebc8a94174e".to_owned();
    let a =
        SecretKey::from_slice(&hex::decode("a6df6a0bb448992a301df4258e06a89fe7cf7146f59ac3bd5ff26083acb22ceb").unwrap()).unwrap();
    let minus_a =
        SecretKey::from_slice(&hex::decode("592095f44bb766d5cfe20bda71f9575ed2df6b9fb9addc7e5fdffe0923841456").unwrap()).unwrap();

    let input_keys = vec![(a, false), (minus_a, false), (a, false)];
    let outpoints = vec![(txid.clone(), 0u32), (txid.clone(), 1u32), (txid, 2u32)];

    match calculate_partial_secret(&input_keys, &outpoints) {
        Ok(secret) => println!("OK {}", encode(secret.secret_bytes())),
        Err(err) => println!("ERR {err}"),
    }
}

This returns:

ERR bad tweak

Likely cause

The implementation in silentpayments/src/utils/sending.rs appears to fold left-to-right with add_tweak:

let result: SecretKey = tail
    .iter()
    .try_fold(*head, |acc, &item| acc.add_tweak(&item.into()))?;

If the intermediate sum reaches zero after A + (-A), add_tweak errors immediately, even though the remaining + A would make the final sum non-zero again.

Question

Is repeated key material across distinct inputs intentionally unsupported for sending, or is this a bug?

I could not find such a restriction in BIP 352, and repeated keys across multiple UTXOs seems valid in general. If this interpretation is wrong, I would appreciate a pointer.

If helpful, I can also share the minimized request JSON I used to derive this case.

[cygnet3]

Yes I think you're right, we should probably do a full sum to avoid this intermediate error. This also means that if a user would order the input keys differently we would calculate (A + A) - A which would suddenly be valid. I don't think that's correct behavior.

[Sosthene00]

The python reference implementation of bip 352 does a full sum before checking for error, so that's certainly a bug on our end.

[nymius]

The problem is rust-secp256k1 doesn't expose the Scalar arithmetic implemented in libsecp256k1, although it is implemented there.
The silentpayments PR there doesn't suffer from this because they have those functions available.
Once we get the bindings for that PR in rust-secp256k1, the issue must be gone.

[Sosthene00]

Sorry I'm getting back to that, I think we can avoid that problem by using PublicKey::combine(), but I haven't tested it with the test case proposed by @shuv-amp yet

[Sosthene00]

Update: turned out I misunderstood @shuv-amp test, since the error happens on add_tweak for Scalar, PublicKey::combine() doesn't help at all.

So probably we have to wait for the module in libsecp to get that one fixed, as @nymius suggested