[macgyver13]

This PR adds a DLEQ (Discrete Logarithm Equality) proof module as specified in BIP 374.

Based on PR #1651 by @stratospher and the secp256k1-zkp implementation.

Public API

Exposes two functions for proof generation and verification:

• secp256k1_dleq_prove
• secp256k1_dleq_verify

These are designed to support rust FFI bindings and follow the API patterns established in similar modules.

Questions for Reviewers

• Should this module be optional (current behavior) or enabled by default?
• Are there additional changes needed to support existing Silent Payments PRs?
• Feedback on API design, documentation, and test coverage?

Notes

Addressed outstanding comments from PR #1651:

• BIP-374 v0.2.0 test vectors
• Proper memory clearing with memclear

[furszy]

Have you talked with @stratospher before opening this PR?
I don’t want to draw conclusions prematurely, but opening a parallel PR that implements the same changes without first engaging on the existing one by reviewing it, leaving a comment, or waiting for the author's public response is not how we usually collaborate in open source. The lack of (co)authorship on the commits is also unusual.

If you have contacted stratospher, please ignore this comment. It's just to avoid putting the original author in an unfair or difficult position if still working on the PR.

[macgyver13]

Have you talked with @stratospher before opening this PR?

Yes, stratospher and I had discussed who should submit this new PR. I drew the short straw :)

[stratospher]

sorry for the confusion and thank you @macgyver13 for the PR!

did an initial pass and it looks good! mostly left style nits.

1 API design question I had is whether it would be better for the proof generation API to also accept C as an argument - that is GenerateProof(a, B, r, G, m, C) kind of API instead of GenerateProof(a, B, r, G, m).

When computing silent payment output points, we anyways compute shared secret/C for each output - so there's a possibility to avoid recomputation again if we can pass C. But current approach might be safer since callers could accidentally provide incorrect C and errors could happen.

Did you check other use cases of DLEQ for whether they might prefer passing C or computing C internally in the proof generation function?

[qatkk]

I have a question regarding the choice of parameters passed to this API, and I may be missing some design context here — so I’d appreciate clarification.
Is there a specific reason why the second generator B is provided directly as an input? @furszy
In the original proposal of DLEQ proofs by Chaum and Pedersen [1], the correctness of the protocol relies on the prover not knowing the discrete‑log relation between the two generators. Specifically, if the prover knows a scalar b such that B=b⋅G, then the two equations for the verification are no longer distinct and will be dependent on each other.
In the context of silent payments (and the description provided in BIP 374) this makes sense, since B represents another user’s public key — a point for which the prover does not know the corresponding private key. However, as commented by stratospher #1651 (comment) this PR is intended to support more general DLEQ use cases, I am concerned that allowing callers to supply an arbitrary B might introduce risks in scenarios where generator independence is not guaranteed.

Other constructions with two generators on the same curve (e.g., Pedersen commitments, bulletproofs) typically derive the second generator using a hash‑to‑curve procedure, ensuring that no participant knows the discrete‑log ratio between G and B. I was wondering whether a similar approach might help avoid potential misuse here, or whether this falls outside the intended scope of this PR.
I would be very interested to hear your thoughts on this.
[1] D. Chaum and T. P. Pedersen, Wallet Databases with Observers, 1992.
https://www.hsslb.ch/cryptopapers/other/Chaum_WalletDBswObservers.pdf

[real-or-random]

In the original proposal of DLEQ proofs by Chaum and Pedersen [1], the correctness of the protocol relies on the prover not knowing the discrete‑log relation between the two generators.

You probably mean soundness instead of correctness? If I'm not mistaken, this is simply not true. See for instance Nigel Smart's Cryptography: An Introduction (3rd Edition), Section 25.3.1, page 377, PDF page 389. This rather accessible write-up shows that the underlying Sigma protocol has correctness, 2-special soundness, and zero-knowledge, all with probability 1 and without any computational assumption. Technically, one doesn't even need the assumption that the discrete logarithm problem is hard in the group. Note in particular that the proof of special soundness extracts the witness and not the discrete logarithm between g and h (or G and B in our notation); the latter doesn't even show up in the proof.

Specifically, if the prover knows a scalar b such that B=b⋅G, then the two equations for the verification are no longer distinct and will be dependent on each other.

Can you elaborate and also explain why you believe that this requires the assumption that the prover doesn't know the discrete logarithm between G and B?

[macgyver13]

Thank you for the review @stratospher and appreciate the curious questions about the API!

1 API design question I had is whether it would be better for the proof generation API to also accept C as an argument - that is GenerateProof(a, B, r, G, m, C) kind of API instead of GenerateProof(a, B, r, G, m).

When computing silent payment output points, we anyways compute shared secret/C for each output - so there's a possibility to avoid recomputation again if we can pass C. But current approach might be safer since callers could accidentally provide incorrect C and errors could happen.

If we accept C as an input, I would suggest computing C internally to verify correctness before calling secp256k1_dleq_prove_internal. This would counter the optimization benefit. This is the approach I took when testing that variation of the API.

Did you check other use cases of DLEQ for whether they might prefer passing C or computing C internally in the proof generation function?

I am not well versed in other DLEQ use cases outside Silent Payments, so I surveyed related applications of Discrete Log Equivalence starting with Andrew Toth's BIP-374 review on the OpTech Podcast. Below you will find a list of reviewed use cases and my assessment of whether to require output points like C as input parameters to proof generation.

DLEQ implementations reviewed:

• Cashu NUT-12 - computes the blind signature C' internally as C' = a·B'
• JoinMarket PODLE - computes output point P2 internally as P2 = x·J
• Half-Adaptor (DLEQ with adaptors) - computes output points internally as Pₙ = x·Gₙ

Related protocols reviewed (not DLEQ-based):

• Curve Trees - anonymous usage tokens from curve trees
• RIDDLE - ring signatures (LSAG) with key images

I don't see a compelling reason to change the API based on these findings, but I'm open to different perspectives.

[qatkk]

Thanks for your response and for the explanation.

You probably mean soundness instead of correctness?

Yes — thanks for the correction.

If I'm not mistaken, this is simply not true. See for instance Nigel Smart's Cryptography: An Introduction (3rd Edition), Section 25.3.1…

Agreed. I’ve encountered DLEQ mostly in other contexts, and in particular in scenarios involving DLEQ across two different curves. In those settings, Pedersen commitments are typically used, which require the independence of the two generators.
Writing out the equations explicitly for the single-curve setting makes it clear that this independence assumption is not needed here, as you point out.

Can you elaborate and also explain why you believe that this requires the assumption that the prover doesn't know the discrete logarithm between G and B?

My reasoning was that the second verification equation (the one checking R₂ in this notation) is effectively just the first verification equation multiplied by b, due to how the parameters are constructed. This made me think that knowing the discrete logarithm between G and B might collapse the two checks.

[macgyver13]

Rebased on latest master and adapted the module to two API changes it introduced, no behavior changes.

• the pluggable SHA256 hash_ctx
• the ecmult_gen → ecmult_gen_ge split

Folded into the affected commits:

• dleq: add module structure and internal implementation
• dleq: add test framework and BIP-374 test vectors
• dleq: add public API wrappers

[macgyver13]

Thank you for the review @theStack! Your suggestions all made sense and I've applied them.

While I was reviewing your feedback I also made some additional changes based on the findings of several agentic review passes:

• Changed secp256k1_dleq_hash_point to return void. It always returned 1 and went unchecked by callers.
• Constant-time coverage: dleq_prove is now exercised in ctime_tests.c, and A/C are declassified as the proof's public statement. Invalid seckeys return early via secp256k1_scalar_set_b32_seckey + declassify, taking the same "constant-time only for valid inputs" approach as ecdh/ellswift/schnorrsig: return early for invalid inputs (consistent handling w.r.t. constant-time) #1919. Re-ordered pubkey_load(B) ahead of the seckey parse in secp256k1_dleq_prove, so an invalid B returns before the secret scalar is loaded, removing the need to clear it on that path.
• Enabled the module by default in dev mode, so the release job's --enable-dev-mode distcheck builds it. Previously nothing in CI validated the module against the tarball.
• Extended the public API tests with message-binding cases in both directions.
• Rounded out failure-path coverage: zero nonces, infinity points, invalid pubkeys, scalar overflow.

Also rebased on master, and folded the earlier "Apply review feedback" commit into the commits it touched; the series is five commits now. If you spot any other improvements, let me know.

[macgyver13]

My last local rebase missed #1915, which moved these helpers to group.h and renamed them:

• secp256k1_eckey_pubkey_parse -> secp256k1_ge_parse_ext33
• secp256k1_eckey_pubkey_serialize33 -> secp256k1_ge_serialize_ext33

[real-or-random]

This was discussed in yesterday's IRC meeting:

08:39 < theStack> one topic i wanted to raise is dleq proofs and their scope within libsecp
08:40 < theStack> the main use case now would be users of silentpayments (pr #1651), but there is also an open pr with a dedicated module (pr #1802)
08:43 < real_or_random> hm it ticks the first two boxes here for sure https://github.com/bitcoin-core/secp256k1/blob/master/CONTRIBUTING.md#adding-new-functionality-or-modules
08:44 < real_or_random> relevance: well. it's used within SP but that is not an argument because then internal code would suffice. what are other use cases?
08:44 < real_or_random> I tend to think DLEQ is generic enough to be relevant. It came up in multiple contexts so far
08:45 < real_or_random> it's a middle-layer thing like ECDH. Not as useful as ECDH in terms of number of applications but there are a handful?
(...)
08:50 < theStack> other than SP, i'm currently not aware of any concrete applications for DLEQs in the bitcoin ecosystem. i guess my preference is to extend the SP module with a create_outputs_with_proof function and not expose the DLEQ functions unless there are compelling use cases
01:47 < real_or_random> theStack: my thinking was that joinmarket needs DLEQ for example. #1802 has a short list. but sure, they all have their own implementations currently
01:48 < real_or_random> theStack: but agreed. not exposing it for now is the conservative thing. it can always be exposed if there's demand

[theStack]

Thanks for following up! Left some more small findings below, some of them quite pedantic (feel free to ignore if they seem too nit-picky). Seems like a rebase on master is also necessary due to #1908.

A few other notes:

• there is consensus that we prefer directly including the test vector source data from BIPs (the two .csv files in this case) in this repository, see e.g. #1698 (comment) and #1786
• the test vector generation script test_vectors_dleq_generate.py is missing the executable flag
• an entry in the features list of README.md is missing
• adding an example examples/dleq.c would be nice, but could still be done in a follow-up (thinking of how users would multiply with arbitrary points feels a bit "hazmat"-y though, they would need to abuse the pubkey tweaking function...)

As mentioned on IRC earlier today, I don't have a really strong opinion on whether it makes sense to fully expose the public API functions already now or not (curious what others think), but it certainly can't hurt to prepare for it.

[macgyver13]

Thanks @theStack for the detailed review and @real-or-random for relaying the meeting
notes. Both of these are helpful context. I'll work through the review points and push an
update to address your suggestions.

On scope: I've been reflecting on this today, and I've come round to thinking the safer
and more pragmatic approach for BIP375 is a small set of silent payments functions, ahead
of the two DLEQ primitives this PR exposes. They'd give wallet developers a more natural
implementation boundary. Using the current two primitives leaves the developer to handle
several BIP352 and BIP375 details themselves. That's related to your examples/dleq.c
point, since an example can demonstrate the primitives but can't carry these rules for
the caller:

• taproot even-Y negation on input keys
• the input_hash construction
• the k-ordering rule: group by scan key, order by spend key within the group,
  then by output index (the module groups by scan key today; handling the full
  ordering there would be ideal)

The shape follows from BIP375 having two forms of share/proof, a global one over every eligible
input key and a per-input one, plus a verifier holding no key for the inputs it checks and a
combiner holding none at all. Listed below in order of how much key material the caller holds,
starting with @stratospher's existing function for context:

/* Signer holding EVERY eligible input key, on one device:
 * derive the outputs and the global share/proof together, sharing the single ECDH
 * per unique scan key that both steps need. This is #1651's existing function,
 * shown with master's parameter naming. Would fill PSBT_GLOBAL_SP_ECDH_SHARE / _DLEQ,
 * from dleq_data, subject to the question below the block. */
int secp256k1_silentpayments_sender_create_outputs_with_proof(
    const secp256k1_context *ctx,
    secp256k1_xonly_pubkey **generated_outputs,
    secp256k1_silentpayments_dleq_data **dleq_data,
    size_t *n_dleq_size,
    const secp256k1_silentpayments_recipient **recipients,
    size_t n_recipients,
    const unsigned char *outpoint_smallest36,
    const secp256k1_keypair * const *keypairs,
    size_t n_keypairs,
    const unsigned char * const *seckeys,
    size_t n_seckeys
);

/* Signer creates the ECDH share and DLEQ proof for ONE scan key. Derives no outputs,
 * so a caller holding only some of the eligible input keys can still contribute its
 * share to a transaction it cannot complete alone.
 *
 * The share is computed over the sum of whatever keys are passed, and that choice
 * decides which PSBT field the result belongs in:
 *   all eligible input keys -> global share  -> PSBT_GLOBAL_SP_ECDH_SHARE / _DLEQ
 *   one input's key         -> that input's  -> PSBT_IN_SP_ECDH_SHARE / _DLEQ
 *
 * Which mode to use is the caller's decision: BIP375 allows a signer holding every key
 * to still choose per-input ("or does not want to create a global ECDH share").
 *
 * Call once per scan key in global mode; once per (input, scan key) pair in per-input
 * mode. Keys passed via keypairs are treated as taproot inputs and even-Y negated
 * before summing as in _sender_create_outputs. */
int secp256k1_silentpayments_sender_create_share_and_proof(
    const secp256k1_context *ctx,
    unsigned char *share33,
    unsigned char *proof64,
    const secp256k1_pubkey *recipient_scan_pubkey,
    const unsigned char *aux_rand32,
    const secp256k1_keypair * const *keypairs,
    size_t n_keypairs,
    const unsigned char * const *seckeys,
    size_t n_seckeys
);

/* Signer verifying shares it did NOT create, holding no key for those inputs:
 * BIP375 assigns this to the party that verifies proofs "for all inputs it does
 * not have the private keys for".
 * One pubkey for the per-input case, all of them for the global case. */
int secp256k1_silentpayments_verify_share_proof(
    const secp256k1_context *ctx,
    const unsigned char *share33,
    const unsigned char *proof64,
    const secp256k1_pubkey *recipient_scan_pubkey,
    const secp256k1_xonly_pubkey * const *xonly_pubkeys,
    size_t n_xonly_pubkeys,
    const secp256k1_pubkey * const *pubkeys,
    size_t n_pubkeys
);

/* Transaction Extractor, holding NO secret keys at all:
 * compute output scripts from shares supplied by others. This is the case
 * _sender_create_outputs_with_proof cannot cover, since it requires the seckeys
 * in order to derive the outputs itself.
 * Takes one already-summed share per scan key, keyed by share_scan_pubkeys, so a
 * caller holding per-input shares combines them first.
 * Input pubkeys for every eligible input are needed, since input_hash commits
 * to their sum. */
int secp256k1_silentpayments_sender_create_outputs_from_shares(
    const secp256k1_context *ctx,
    secp256k1_xonly_pubkey **generated_outputs,
    const secp256k1_silentpayments_recipient **recipients,
    size_t n_recipients,
    const unsigned char *outpoint_smallest36,
    const secp256k1_pubkey * const *share_scan_pubkeys,
    const unsigned char * const *shares33,
    size_t n_shares,
    const secp256k1_xonly_pubkey * const *xonly_pubkeys,
    size_t n_xonly_pubkeys,
    const secp256k1_pubkey * const *pubkeys,
    size_t n_pubkeys
);

A sender holding every key should keep using the first, unchanged in shape, since
deriving outputs and proofs together shares the one ECDH per unique scan key that both
need. The other three are illustrative. The last one keys each share by its scan
pubkey, matching how BIP375 keys the share fields themselves.

One question came out of reading #1651 to work out how the above would fit alongside it.
In sender_create_outputs_with_proof, seckey_sum_scalar is multiplied by input_hash
before create_shared_secret_with_proof is called, so the value that ends up as the DLEQ
secret is input_hash·a_n rather than a_n. The proof is then a statement about
input_hash·a_n, and the stored shared_secret is input_hash·a_n·B_scan.

If I'm reading BIP375 correctly, the value it wants in PSBT_GLOBAL_SP_ECDH_SHARE is
a_n·B_scan, with the proof over a_n and input_hash applied afterwards during output
computation, so a proof made the other way wouldn't verify against A_n. Am I missing a
reason it's folded in earlier? I can see why it happens in master's
sender_create_outputs, where the comment at main_impl.h:277 notes that multiplying the
scalars first saves an elliptic curve multiplication and nothing observable depends on the
ordering, so I assume it carried over naturally once proofs were added.

For what it's worth, create_shared_secret_with_proof itself looks agnostic here: handed
a_n it would produce C = a_n·B_scan and A = a_n·G directly. So if this does want
changing for BIP375, it looks like the caller rather than the proof code.

@theStack once you've had a chance to look at these signatures we can work out who's best
placed to integrate them into the silent payments module. I can see a future where both
the silent payments functions and the DLEQ primitives are useful.

I am still in the exploratory phases of new work that combines MuSig2 and Silent Payments
and uses the existing DLEQ API. Of the four above, only the combiner
_sender_create_outputs_from_shares carries over to that case; share creation and
verification still want the generic DLEQ pair. Once that work is ready for critique I can
share it for review.

[theStack]

@macgyver13: Thanks for your thoughts and the API proposal! Since this PR is focusing on the generic DLEQ module and could be useful for non-SP use-cases as well, I've opened a SP-specific issue for further discussion (with emphasis on what I think main use-case for DLEQs in SP nowadays, i.e. verification of created outputs), where I will reply in more detail to your post within the next days: #1925

[brunoerg]

Mutation testing report for this PR is available at: https://secp256k1.space/pull/1802

[macgyver13]

Thanks again @theStack. All of your review points were applied to the original commit series. I did not provide a dleq.c example in this update - still on my todo list.

Additionally, included test_vectors_generate_proof.csv and test_vectors_verify_proof.csv, this required modifying .gitignore to explicitly allow *.csv files from /src/modules/dleq/.

Rebased on master and adapted to #1908 (direct ->hash_ctx access).

Verified generator produced same output after G point check instead of relying on row count (no diff):

./tools/test_vectors_dleq_generate.py src/modules/dleq | git diff --no-index -- - src/modules/dleq/dleq_vectors.h

[macgyver13]

Thank you for your thoughts on the GenerateProof shape @theStack! When I first looked at @stratospher's question, I didn't consider returning C once the prover has computed it. Working through your four options got me there, and I think it's the best fit for both consumers and performance.

I surveyed several implementations testing the DLEQ API (Coldcard, Jade/libwally, spdk, silent-pay). Each one computes C itself to populate the PSBT's ECDH share field, and the prover then computes the same C again. If secp256k1 computes C and the proof together, the caller makes one call, gets back the proof and the share, verifies, and stores both in the PSBT.

This is the shape I have in mind:

/** Generate a DLEQ proof.
 *
 *  Proves knowledge of scalar a such that A = a*G and C = a*B without
 *  revealing a. C is computed internally and returned, so callers do not
 *  need to compute it separately.
 ...
 *  Args:        ctx: pointer to a context object (not secp256k1_context_static)
 *  Out:     proof64: pointer to 64-byte proof
 *          pubkey_C: pointer to public key C = a*B
 *  In:     seckey32: pointer to 32-byte secret key (scalar a)
 *          pubkey_B: pointer to public key B (base point)
 *        aux_rand32: pointer to 32-byte auxiliary randomness (can be NULL)
 *                    BIP-374 recommends fresh randomness for each proof
 *             msg32: pointer to optional 32-byte message (can be NULL)
 */
int secp256k1_dleq_prove(
    const secp256k1_context *ctx,
    unsigned char *proof64,
    secp256k1_pubkey *pubkey_C,
    const unsigned char *seckey32,
    const secp256k1_pubkey *pubkey_B,
    const unsigned char *aux_rand32,
    const unsigned char *msg32
);

Interested in your thoughts.

[Yudis-bit]

Concept ACK.

A couple of observations on verifier efficiency and the prover API shape:

1. In secp256k1_dleq_verify_internal (src/modules/dleq/main_impl.h), $R_2 = s \cdot B - e \cdot C$ is currently evaluated with two independent secp256k1_ecmult calls:

secp256k1_ecmult(&tmpj, &Cj, &e_neg, NULL);
secp256k1_gej_set_ge(&Bj, B);
secp256k1_ecmult(&R2j, &Bj, s, NULL);
secp256k1_gej_add_var(&R2j, &R2j, &tmpj, NULL);

Since secp256k1_ecmult_strauss_wnaf accepts an arbitrary num of points, we can compute $R_2$ in a single doubling loop using stack-allocated state for 2 points without heap allocation (~1.6 KB stack):

secp256k1_fe aux[2 * ECMULT_TABLE_SIZE(WINDOW_A)];
secp256k1_ge pre_a[2 * ECMULT_TABLE_SIZE(WINDOW_A)];
struct secp256k1_strauss_point_state ps[2];
struct secp256k1_strauss_state state;
secp256k1_gej pts[2];
secp256k1_scalar scs[2];

pts[0] = Bj;
pts[1] = Cj;
scs[0] = *s;
scs[1] = e_neg;

state.aux = aux;
state.pre_a = pre_a;
state.ps = ps;
secp256k1_ecmult_strauss_wnaf(&state, &R2j, 2, pts, scs, NULL);

In a benchmark over 50,000 runs, this drops $R_2$ evaluation latency from 29.58 µs to 26.92 µs (~9% speedup) by halving the doubling loop steps and dropping the Jacobian addition. Full test suite passes.

2. On returning C in secp256k1_dleq_prove: in examples/dleq.c:115, the signer currently invokes secp256k1_ecdh with a custom identity callback solely to retrieve $C = a \cdot B$ for the coordinator, and secp256k1_dleq_prove then recomputes $a \cdot B$ again. Making secp256k1_pubkey *pubkey_C an optional out-parameter (ignoring when NULL via secp256k1_pubkey_save) avoids that duplicate point multiplication entirely for BIP-375 signers.

[theStack]

Thank you for your thoughts on the GenerateProof shape @theStack! When I first looked at @stratospher's question, I didn't consider returning C once the prover has computed it. Working through your four options got me there, and I think it's the best fit for both consumers and performance.

Returning C seems like a nice middle ground indeed, funny that this (in hindsight obvious) idea didn't even appear to me. Unless I'm missing something, I think this approach doesn't have a drawback, so I'd advocate going for it.

One detail question (inspired by @Yudis-bit's comment 2. immediately above) might be whether this out-parameter should be optional, but I'd lean towards no: even if the user have calculated the C themselves, it still seems useful to compare the re-calculated C against the expected value, so there should always be a use-case for getting it. Only a naming nit (entering bikeshedding territory): maybe the parameter name could be adapted to indicate it's an out-parameter, in order to avoid users passing in an already-calculated C and overwriting it (maybe something like output_pubkey_C or result_pubkey_C?).

Since secp256k1_ecmult_strauss_wnaf accepts an arbitrary num of points, we can compute R 2 in a single doubling loop using stack-allocated state for 2 points without heap allocation (~1.6 KB stack):
[...]

Thanks for investigating this potential speed-up, I think this would make a good follow-up PR candidate.

[Yudis-bit]

Sounds good. Making output_pubkey_C mandatory keeps the prover API clean since PSBT signers need the share anyway.

Happy to defer the 2-point Strauss verifier loop to a follow-up to keep this PR focused on the BIP 374 spec.

[macgyver13]

Thank you for the review @theStack and @Yudis-bit! This push addresses the last round of feedback.

Review nits:

• A, B, C, R1 and R2 are each serialized once and passed to secp256k1_dleq_challenge as 33-byte buffers, matching the nonce function. Removed secp256k1_dleq_hash_point. G is a static 33-byte constant.
• Dropped the bytes(32, e) || bytes(32, s) detail from the proof64 docs for both prove and verify.
• if (msg32 != NULL) in the challenge.
• secp256k1_dleq_prove_internal now takes A, B, C in that order, matching verify_internal and the challenge.

secp256k1_dleq_prove now returns output_pubkey_C:

• The header documents that C is computed internally and returned. Callers don't need to compute it separately.
• examples/dleq.c gets C straight from secp256k1_dleq_prove. The secp256k1_ecdh call with the identity hash callback is gone. The example no longer depends on the ECDH module, so I removed it from Makefile.am and examples/CMakeLists.txt.
• ctime_tests.c marks the returned C as defined, alongside the proof.

BIP-374 vectors moved to the public API tests:

• Reorganized commits 2 and 4 so the vectors run through secp256k1_dleq_prove / secp256k1_dleq_verify.
• The API tests verify against the returned output_pubkey_C instead of recomputing C, and check that it is zeroed on every failure path.
• Skipped the generation vector with B at infinity, since a secp256k1_pubkey can't represent it. The internal tests still cover prove_internal's rejection of infinity points. For each generation vector, the test also checks that the returned C verifies with the proof.
• vectors.h is now part of make testvectors. The CSVs and generator script are in EXTRA_DIST, so release tarballs can regenerate it. This matches the silentpayments module.

Also rebased on master. secp256k1_dleq_pair now uses the new secp256k1_ecmult_const_ge helper, which clears its Jacobian intermediate itself. The manual Cj clear is gone.

@Yudis-bit, thank you for sharing the speed up measurement. I agree with keeping that for a follow-up so this PR stays focused on BIP-374.

[theStack]

Thanks for following up!

ACK modulo two small improvement suggestions

[macgyver13]

Latest push applies @theStack's review comments and adds his benchmark commit:

• Verification now uses secp256k1_ge_set_gej_var for R1 and R2.
• secp256k1_dleq_prove zeroes proof64 and output_pubkey_C before any check that can fail, including the context check, so the outputs have a defined value on failure. The API tests now also check that proof64 is zeroed on each failure path.
• Added the benchmark commit on top. Thank you!

Also rebased on master.

[theStack]

ACK e15009f

organizational nit: Not a blocker at all, but it might be slightly confusing at the first review sight that the API header declarations are added before the corresponding implementation, which comes two commits later. Could still add an (almost empty) header in the "add module structure" commit, and only add the API declarations in the "add public API wrappers" commit.

[fametrano]

Cross-checked e15009f against btclib-ecc's independent pure-Python BIP374 implementation: no disagreement. Over 5000 random inputs, called through ctypes on the shared library built at this head, the proofs were byte-identical and verified both ways, and both sides rejected all 52,552 cases with an altered proof, point or message. The script is available.

[ViniciusCestarii]

ACK e15009f

Nice work! The code and tests look good.

I also wrote some additional BIP-374 test vectors (bitcoin/bips#2335). This implementation passes all of them.

[IsaqueFranklin]

ACK e15009f

I reviewed and understood the code, ran the tests and everything looks good to me! I just raised that question about checking infinity points on inputs, but that's totally non-blocking for me.

[IsaqueFranklin]

Should we also be checking A, B, C for infinity point on this function (not exactly in these lines)? The _prove_internal does it, and if I'm not mistaken those checks seem redundant there, at least when called through the wrapper dleq_prove. The verify wrapper loads (A, B, C) with secp256k1_pubkey_load (a valid secp256k1_pubkey cannot represent infinity) and the prove wrapper computes A and C using a valid nonzero scalar a and a valid point B (that comes from a secp256k1_pubkey). So in both cases the checks to reject infinity on input points seem redundant. Should we try and be consistent in both cases?

[macgyver13]

Thanks @IsaqueFranklin, good catch!

This is an artifact from moving the tests to the public API. The vector tests originally called _prove_internal directly with B at infinity. Your argument for consistency is spot on.

I looked at the rest of the library, which uses VERIFY_CHECK for invariants established at the API boundary. I replaced the runtime check in _prove_internal with VERIFY_CHECK, following the approach suggested in bitcoin-core/secp256k1#1765 (comment).

I kept afb2c9f as a separate commit so reviewers can weigh in before I squash it.

[IsaqueFranklin]

Related question here. Should we also be bypassing the wrapper for the _verify_internal function and be testing it for infinity on the input points? In that case, _verify_internal would need checks on the points to reject infinity inputs.

[macgyver13]

These tests were removed in afb2c9f. They covered the runtime rejection branch, which was needed when the vector tests called _prove_internal directly.

[Yudis-bit]

In secp256k1_dleq_prove, if secp256k1_scalar_set_b32_seckey returns 0 for an invalid secret key, the function returns early without calling secp256k1_scalar_clear(&a), leaving the reduced scalar in the stack frame. Similarly, in secp256k1_dleq_prove_internal, an early return on !ret from secp256k1_dleq_nonce skips clearing k.

--- a/src/modules/dleq/main_impl.h
+++ b/src/modules/dleq/main_impl.h
@@ -160,6 +160,7 @@ static int secp256k1_dleq_prove_internal(const secp256k1_context *ctx, secp256k
     /* The nonce is still secret here, but it being zero is less likely than 1:2^255. */
     secp256k1_declassify(ctx, &ret, sizeof(ret));
     if (!ret) {
+        secp256k1_scalar_clear(&k);
         return 0;
     }
 
@@ -262,6 +263,7 @@ int secp256k1_dleq_prove(
     is_sec_valid = secp256k1_scalar_set_b32_seckey(&a, seckey32);
     secp256k1_declassify(ctx, &is_sec_valid, sizeof(is_sec_valid));
     if (!is_sec_valid) {
+        secp256k1_scalar_clear(&a);
         return 0;
     }

[macgyver13]

Thanks for looking at this @Yudis-bit. I don’t think either clear is needed:

• Invalid seckey path: this clear was in an earlier version of the PR, and theStack pointed out that it isn’t needed (bitcoin-core/secp256k1#1802 (comment)).
• Zero nonce path: secp256k1_dleq_nonce returns !secp256k1_scalar_is_zero(k) so k is already zero for the early return.

[Yudis-bit]

Fair point, missed the prior discussion and that !ret guarantees k is zero here.

[theStack]

re-ACK afb2c9f

I agree with the change in the last commit, doing the non-infinity assurance checks with VERIFY_CHECK in internal functions seems reasonable. The new BIP-374 test vectors in bitcoin/bips#2335 have been merged upstream, so if you squash and force-push, you could also update these to v0.3.1. Doing that in a follow-up seems to be fine as well though.

[macgyver13]

Rebased on master and folded the VERIFY_CHECK commit into the respective commits.

No functional changes since afb2c9f other than updating the test vectors to v0.3.1.

[theStack]

re-ACK 9b5e5e6

[ViniciusCestarii]

ReACK 9b5e5e6