silentpayments: perform tweak addition with actual scalars #1929

pull theStack wants to merge 1 commits into bitcoin-core:master from theStack:sp_tweak_addition_with_scalars changing 1 files +7 −16

theStack commented at 3:07 AM on September 4, 2026: contributor

When having a labeled match in the silentpayments scanning function (_silentpayments_recipient_scan_outputs), the addition of the spending tweak (being t_k + label_tweak) is currently done using the secret key tweaking function secp256k1_ec_seckey_tweak_add. This, and the long comment explaining why this call could fail, stem from what I'd argue is ultimately a type mismatch: a tweak is not a secret key, and the value 0, both for individual tweaks and for the result, is just fine for additive tweaking. Simplify by just using scalar addition instead, so the long comment and the branch can be removed.

There is one subtle behavior change in the case a label tweak is returned from the label cache that is overflowing (i.e. larger than or equal than the group order N). As this is an user error, I'd argue that the concrete tweak output doesn't matter in this case. If anything, we should consider if we return 0 in this case to indicate that the user did something wrong (though I don't think it's needed).
theStack added the label tweak/refactor on Sep 4, 2026
in src/modules/silentpayments/main_impl.h:772 in 12d0d3b070 outdated
```
 783 | -                 */
 784 | -                if (!secp256k1_ec_seckey_tweak_add(ctx, found_outputs[k]->tweak, label_tweak)) {
 785 | -                    memset(found_outputs[k]->tweak, 0, 32);
 786 | -                }
 787 | +                secp256k1_scalar_set_b32(&label_tweak_scalar, label_tweak, NULL);
 788 | +                secp256k1_scalar_add(&final_tweak_scalar, &final_tweak_scalar, &label_tweak_scalar);
```
nymius commented at 2:13 PM on September 7, 2026:

I would be conservative and fail scanning if overflowed, the caller may not be aware of the corrupt cache.

theStack commented at 4:32 PM on September 8, 2026:

Not fully opposed to the idea, but I'd argue introducing a new error condition is outside of scope for this PR.

nymius commented at 5:30 PM on September 8, 2026:

It's fine for me.
in src/modules/silentpayments/main_impl.h:779 in 12d0d3b070 outdated
```
 790 |              } else {
 791 |                  found_outputs[k]->found_with_label = 0;
 792 |                  /* Set the label to an invalid value. */
 793 |                  memset(&found_outputs[k]->label, 0, sizeof(found_outputs[k]->label));
 794 |              }
 795 | +            secp256k1_scalar_get_b32(found_outputs[k]->tweak, &final_tweak_scalar);
```
nymius commented at 2:23 PM on September 7, 2026:

Why we don't need to clear final_tweak_scalar here like t_k_scalar in 767? How that tx identification leak works?

theStack commented at 4:32 PM on September 8, 2026:

Good catch, I have missed that. I'm generally somewhat skeptical now if there is even a point in clearing these anymore, since we have reached agreement that we can drop the constant-time requirements for them (see discussion #1914 (comment) ff.).

Added clearing of the label tweak and the final tweak for consistency (we can always remove those later if we have consensus that it's not necessary).
nymius commented at 2:24 PM on September 7, 2026: contributor

cACK 12d0d3b0708b38c6a5523bbaccdbb88dcce162eb

silentpayments: perform tweak addition with actual scalars

When having a labeled match in the silentpayments scanning function
(`secp256k1_silentpayments_recipient_scan_outputs`), the addition of the
final tweak (being t_k + label_tweak) is currently done using the secret
key tweaking function `secp256k1_ec_seckey_tweak_add`. This, and the
long comment explaining why this call could fail, stem from what I'd
argue is ultimately a type mismatch: a tweak is not a secret key, and
the value 0, both for individual tweaks and for the result, is just fine
for additive tweaking. Simplify by just using scalar addition instead,
so the long comment and the branch can be removed.

11e9c6ede7

theStack force-pushed on Sep 8, 2026

Contributors

theStack

nymius

Labels

tweak/refactor