Skip to content

silentpayments: tweak spend pubkey using ecmult_gen_var (~25% scanning speedup for small N) - #1914

Open
theStack wants to merge 3 commits into
bitcoin-core:masterfrom
theStack:silentpayments-use-ecmult_gen_var-for-P_k-calc
Open

silentpayments: tweak spend pubkey using ecmult_gen_var (~25% scanning speedup for small N)#1914
theStack wants to merge 3 commits into
bitcoin-core:masterfrom
theStack:silentpayments-use-ecmult_gen_var-for-P_k-calc

Conversation

@theStack

Copy link
Copy Markdown
Contributor

Based on the #1883 branch, this PR takes advantage of the variable-time generator point multiplication function ecmult_gen_var for spend public key tweaking in the output public key calculation $P_k = B_{spend} + t_k \cdot G$ (used both for sending and scanning). Note that $t_k$ is not considered a long-term secret (in contrast to e.g. the scan secret key), so using variable-time functions should be fine. This leads to a ~25% scanning speedup if the number of transaction outputs is small (N <= 10).

Results on my arm64 machine (as per $ ./build/bin/bench silentpayments_scan_nomatch, see also the commit body):

N master PR branch speedup
2 39.3 us 30.3 us 28.85%
5 40.4 us 31.7 us 27.44%
10 43.3 us 34.6 us 25.14%
100 89.6 us 81.2 us 10.34%
1000 1259.0 us 1244.0 us 1.99%

This PR is similar to the previous ones #1843, #1844, but it only changes the behavior of the silentpayments module and leaves other API functions unchanged.

Having this available as a global constant allows to introduce an
alternative `ecmult_gen_gej` function that doesn't need access to
a context, see next commit.

Note that the precomputed constant takes the name of the function that
previously generated it at run-time (`secp256k1_ecmult_gen_scalar_diff`),
while the function is now renamed to include the "compute" verb
(`secp256k1_ecmult_gen_compute_scalar_diff`), to match the naming
of the table generation function.

Can be reviewed with `--color-moved=dimmed-zebra` for easier
checking of the move-only parts.
Add faster variable-time variants for generator point multiplication.
This is essentially `ecmult_gen` without side-channel mitigations and
without requiring a context object. Intended for use cases where the
scalar is not representing sensitive data.

On my arm64 machine, this is ~86% faster than the constant-time variant
(with the default build table size, i.e. ECMULT_GEN_KB=86):

```
$ ./build/bin/bench_ecmult
Benchmark                     ,    Min(us)    ,    Avg(us)    ,    Max(us)

ecmult_gen                    ,     9.32      ,     9.35      ,     9.60
ecmult_gen_var                ,     5.02      ,     5.02      ,     5.02
.....
```
…g speedup for small N)

Note that the tweak t_k is not considered a long-term secret, so using
variable-time functions for calculating P_k = B_spend + t_k * G seems fine.
This leads to a ~25% scanning speedup if the number of outputs is small (N <= 10):

master:
```
$ ./build/bin/bench silentpayments_scan_nomatch
Benchmark                               ,    Min(us)       ,    Avg(us)       ,    Max(us)

silentpayments_scan_nomatch_N=2         ,       39.3       ,       39.3       ,       39.3
silentpayments_scan_nomatch_N=5         ,       40.4       ,       40.4       ,       40.4
silentpayments_scan_nomatch_N=10        ,       43.3       ,       43.3       ,       43.3
silentpayments_scan_nomatch_N=100       ,       89.0       ,       89.6       ,       91.0
silentpayments_scan_nomatch_N=1000      ,      560.0       ,      563.0       ,      568.0
silentpayments_scan_nomatch_N=2323      ,     1254.0       ,     1259.0       ,     1271.0
silentpayments_scan_nomatch_N=23250     ,    12196.0       ,    12209.0       ,    12228.0
```

PR branch (using ecmult_gen_var for calculating t_k * G):

```
$ ./build/bin/bench silentpayments_scan_nomatch
Benchmark                               ,    Min(us)       ,    Avg(us)       ,    Max(us)

silentpayments_scan_nomatch_N=2         ,       30.5       ,       30.5       ,       30.5
silentpayments_scan_nomatch_N=5         ,       31.7       ,       31.7       ,       31.7
silentpayments_scan_nomatch_N=10        ,       34.6       ,       34.6       ,       34.6
silentpayments_scan_nomatch_N=100       ,       80.0       ,       81.2       ,       85.0
silentpayments_scan_nomatch_N=1000      ,      550.0       ,      552.0       ,      554.0
silentpayments_scan_nomatch_N=2323      ,     1239.0       ,     1244.0       ,     1253.0
silentpayments_scan_nomatch_N=23250     ,    12164.0       ,    12182.0       ,    12214.0
```

Speedups:

N=2: ~28.85%
N=5: ~27.44%
N=10: ~25.14%
N=100: ~10.34%
N=1000: ~1.99%
@theStack
theStack force-pushed the silentpayments-use-ecmult_gen_var-for-P_k-calc branch from 2f0954e to 4c3013b Compare August 24, 2026 12:14
@real-or-random

Copy link
Copy Markdown
Contributor

Concept ACK

This was discussed in yesterday's IRC meeting:

08:30 < real_or_random> I tend to think that's a bit of a hopeless case. When josie brought this up first, my initial reaction was "uh, let's stay safe here". But now I'm leaning towards the opposite. Any meaningful caller will branch on whether a match was found or not, with much more timing differences than we can ever generate in the raw crypto code. So getting a 25% speedup seems worth the deal
[...]
08:36 < sipa> real_or_random: seems reasonable

@theStack

theStack commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Concept ACK

This was discussed in yesterday's IRC meeting:

08:30 < real_or_random> I tend to think that's a bit of a hopeless case. When josie brought this up first, my initial reaction was "uh, let's stay safe here". But now I'm leaning towards the opposite. Any meaningful caller will branch on whether a match was found or not, with much more timing differences than we can ever generate in the raw crypto code. So getting a 25% speedup seems worth the deal
[...]
08:36 < sipa> real_or_random: seems reasonable

I only notice now that the PR in its current state applies the same optimization also for the sending side, without giving much thought if that's also okay. The argument from the IRC meeting above is based solely on the scanning scenario ("caller will branch on whether a match was found or not"), for the sending side I assume there could theoretically still be (more of) a point treating shared_secret and t_k as actual secret?

Another slightly related follow-up question would be if at places where we treat these values not as secrets, we could (or should) then consequently also remove the memory clearing efforts, like e.g. the following:

/* Leaking these values would break indistinguishability of the transaction, so clear them. */
secp256k1_scalar_clear(&t_k_scalar);
secp256k1_memclear_explicit(&shared_secret, sizeof(shared_secret));

I'd say yes for consistency, though in this case there is no performance upside (I haven't verified, but I'd be surprised if this makes a noticeable difference), it just make the code overall a bit simpler by reducing LOC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants