3 Commits

Author	SHA1	Message	Date
Kgothatso Ngako	303a7caeae	frost_enrollment: add the test suite and the regression vectors ... Fourth of six commits. Twelve tests replacing the Phase 2 smoke test, plus a vector generator and the frozen vectors it produces. The regression vectors are the one part of this worth being precise about, because they are easy to over-claim. FROST enrollment has no BIP and no published vectors, and the reference proof of concept draws its randomness from secrets.randbits, which is not seedable -- so there is nothing to cross-validate against. tools/test_vectors_frost_enrollment_generate.py therefore re-implements the math independently in stdlib-only Python, including the group arithmetic written from the secp256k1 parameters rather than borrowed, and freezes the output. What that buys: the two tag strings, the params hash serialization, the share-splitting derivation and the identifier conventions are now pinned, and changing any of them is a loud vector-breaking change. What it does not buy is evidence of protocol correctness. The generator header comment and the generated file both say so, as does frost_enrollment.md. The vectors passed on the first run against the C code, which is worth recording: two independent implementations agree byte for byte on the params hash, every delta, every sigma, the derived public share and the final share, across four cases (2-of-3 minimal, 2-of-3 oversized at u = 3 > t = 2, a 3-of-5 repair with a deliberately UNSORTED helper set, and a 4-of-6 enrollment), covering both threshold-key Y parities. The algebraic invariants are what actually carry correctness: - Reconstruction (PoC test_generate_frost_share): after a 2-of-3 group enrolls id 3, every pair {i, 3} reconstructs the original threshold secret, and so does the untouched pair {0, 1}. - Signing (PoC test_sign): a real BIP340 signature from {2, 3} verifying against the unchanged threshold public key, with every partial signature individually verified, plus the n -> n+1 bookkeeping -- secp256k1_frost_threshold_info_validate must accept the public share table extended with pubshare_derive's output at n+1. - Repair: byte-for-byte equality with the lost share, and the repaired participant keeps its old public share. - Oversized helper set: u = 3 and u = 2 over the same key material produce the same share and the same derived public share. - Randomized: COUNT iterations over 2 <= t <= u <= n <= 7, half enrollment and half repair, with EVERY HELPER GIVEN THE IDENTIFIER SET IN ITS OWN SHUFFLED ORDER. The params hash must come out identical while the delta buffers stay aligned per helper -- which is the whole point of canonicalizing ids inside the hash and nowhere else. Each iteration then checks every t-subset containing the new participant. The negative tests are organized around what each gate is actually for: - Fault injection flips a bit in one sigma. secshare_gen fails and wipes its output; the same call with expected_pubshare = NULL SUCCEEDS and returns a wrong share. That second assertion is the point -- it is the evidence that the parameter is load-bearing rather than decorative. Tampered public shares are caught earlier, by secp256k1_frost_threshold_info_validate, so the test exercises the recommended flow and not just the module. - Parameter mismatch, four angles: (a) one helper runs round 1.1 for a different target and every other helper's share_agg aborts naming it by identifier; (b) a caller that IGNORES that abort and finishes round 1.2 anyway still cannot produce a usable share, because the public-share check catches the inconsistent sum -- defence in depth, not a test of the test's own control flow; (c) the helpers agree with each other on new_id = 3 while the target expects 4, which round 1.2 cannot see and round 2's own recomputation does; (d) two groups with identical (t, n, ids, new_id) get different hashes, and a hash from one fails share_agg in the other. - Own-slot semantics: filling the caller's own slot of received_params_hashes32 with garbage changes nothing, because it is never read -- but the same garbage in a slot that IS read still aborts. That pair is what makes "recomputation, not string comparison" testable rather than merely asserted. - Invalid parameters, including both deliberate divergences: t = 1 refused, enrollment refused at n = 128 while repair at n = 128 is accepted, n_ids > 128 returning 0 with the output zeroed in a production build. Three bugs found while writing these, all in the tests, all worth naming: - pubshare_derive takes public shares ALIGNED WITH ids, and the test helper was handing it the participant-indexed table. Those coincide exactly when the helper set is 0..u-1, which every test until the repair case used, so the first non-contiguous helper set {0, 2} was what exposed it. There is now one helper that does the gather, with a comment saying which confusion it exists to prevent. - The fault-injection test compared against r.new_secshare without ever running round 2, and the mismatch test compared against r.params_hashes[0] one line before round 1.1 filled it. Both were reads of uninitialized memory that happened to pass; valgrind found both. The randomized test loops COUNT times so -i scales it, following the iceberg module (tests_impl.h:1322) rather than prefractal's run-once convention -- a fuzzing loop that ignores the iteration count is not much of one. Verification: all twelve tests pass at the default iteration count, at -i=200 and at -i=2000; ./tests, ./noverify_tests and ./exhaustive_tests exit 0 with all five FROST-stack modules enabled; the module runs clean under valgrind (0 errors from 0 contexts); ctime_tests is clean under valgrind; regenerating vectors.h reproduces it byte for byte. One note for anyone running these locally: ctime_tests must not be run against a CPPFLAGS='-DVERIFY' build. secp256k1_scalar_verify branches on scalar values, which ctime_tests deliberately marks secret, so every scalar operation in the library reports a finding -- 75997 of them, none in this module. The CI matrix already pairs -DVERIFY with CTIMETESTS: 'no' (.github/workflows/ci.yml:119, :596) for this reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-04 04:24:21 +02:00
Kgothatso Ngako	89b253b50e	frost_enrollment: implement the three rounds ... Third of six commits. Replaces the Phase 1 stubs with the real arithmetic, adds a smoke test that a 2-of-3 group really does grow into a working 2-of-4 one, and wires the entry points into ctime_tests. The Lagrange machinery is frost's, called in place. pubshare_derive is a skin over secp256k1_frost_derive_pubshare_at (src/modules/frost/keygen_impl.h:150) evaluated at identifier new_id, and the id canonicalization is secp256k1_frost_sort_ids, reached through the declaration the previous commit added. The one piece frost could not supply is the scalar Lagrange coefficient at an arbitrary point. frost's secp256k1_frost_derive_interpolating_value evaluates at x-coordinate 0, which is what reconstructing the group secret needs; enrollment needs the basis polynomial at the TARGET x-coordinate. secp256k1_frost_enrollment_lagrange_at is that, and it is deliberately the same product derive_pubshare_at applies to each pubshare, in the same identifier space -- so the scalar path and the point path agree by construction rather than by coincidence. Working in identifier space is what makes the id-to-x-coordinate +1 cancel: an x-coordinate difference x_j - x_i is the identifier difference id_j - id_i. Round 1.1 computes v = lambda * secshare and splits it. Every share but the one kept locally is masking randomness derived as Scalar.from_bytes_wrapping( TH("FROST enrollment/share_split", rand32 || params_hash32 || ser32(my_id) || ser32(recipient_id))) with rand32 = TH(same tag, session_secrand32) XOR secshare32; the kept share absorbs the remainder so the set sums to v. Three details: - The reduction wraps rather than rejects, chilldkg's from_bytes_wrapping (src/modules/chilldkg/util_impl.h:394). A 256-bit hash mod the group order is about 2^-128 from uniform; rejection sampling would buy that back in exchange for a variable-time loop. - Masking with the secret share is the secp256k1_frost_nonce_gen pattern (session_impl.h:340), so a broken RNG alone does not reveal the split. - The derivation is indexed by the recipient's IDENTIFIER, not by its position in the caller's ids array. The plan called for a counter; identifiers are unique, so they are one, and using them makes the split independent of the order a caller lists the helper set in. What the binding buys is DOMAIN SEPARATION only: params_hash32 carries the group key and the whole parameter tuple, so two runs sharing a seed but differing in either cannot produce the same deltas. It cannot detect a disagreement between helpers, because nothing cross-checks per-helper private randomness. That is the params hash's job. session_secrand32 is wiped whether the call succeeds or fails, so a caller cannot retry a failed run on the same randomness. Round 1.2 recomputes its own params hash from the group key and the tuple, compares every received hash against it, then sums. The slot at the caller's own position in received_params_hashes32 is skipped, while the same position in all_shares32 is read -- the asymmetry the header documents, and the thing that makes this a recomputation rather than a string comparison. The mode and bounds are re-validated here rather than trusted from the round 1.1 call site, since the full tuple is present. An out-of-range share is reported through mismatch_id the way secp256k1_frost_partial_sig_agg reports an unparseable partial signature. Round 2 compares the params hash against its own recomputation over the authenticated group key, sums, rejects a zero share, and checks secshare*G against the expected public share. Three deviations from the plan, all to match what the tree already does: - Value ranges return 0; only pointers get ARG_CHECK. The plan called for an ARG_CHECK on the n_ids bound, but the frost module's split is the one used here (secp256k1_frost_trusted_dealer_keygen, keygen_impl.h:227), and the header already documents these as return-0 conditions. The bound is still enforced in production builds -- params_are_valid requires 2 <= threshold <= n_ids <= n_participants <= 128 -- so it does not ride on the VERIFY_CHECK inside secp256k1_frost_sort_ids, which is what the plan was guarding against. - The public-share check declassifies the derived point and compares with secp256k1_ge_eq_var, rather than comparing 33 serialized bytes in constant time. There is no constant-time memcmp in this tree, and secshare*G is a public key: secp256k1_frost_sign declassifies exactly this quantity before exactly this comparison (src/modules/frost/session_impl.h:770, :789). Inventing a primitive to avoid following that precedent would be the worse trade. - params_hash's public entry point delegates to the same internal routine every gate uses, so the encoding has exactly one implementation to keep in step with the vectors. One real bug found by the tooling rather than by reading. Accumulators were initialized with secp256k1_scalar_clear, and secp256k1_memclear_explicit marks its target UNDEFINED in VERIFY builds (src/util.h:295) precisely so that reading cleared memory is caught. It was: valgrind reported 143752 errors in share_agg's summation loop. Accumulators now start at secp256k1_scalar_set_int(x, 0); scalar_clear is used only where it means "done with this secret". Worth stating plainly because the failure mode is invisible in a production build, where memclear_explicit only zeroes. ctime_tests gains a 2-of-3-enrolls-a-fourth block covering all three rounds, following prefractal's b66c757b. The threshold key, the secret shares, the session randomness and every delta and sigma on the wire are marked secret; the identifiers, public shares, group key, parameters hashes and derived public share are not. Under valgrind: 0 errors from 0 contexts, so no branch or memory access in the new code depends on secret data. Verification: ./tests, ./noverify_tests and ./exhaustive_tests exit 0; the frost_enrollment module runs clean under valgrind (0 errors); a separate CPPFLAGS='-DVERIFY' build compiles without warnings and passes; the module builds warning-free alongside frost, chilldkg, iceberg and prefractal. The smoke test is the substantive check: after a 2-of-3 group enrolls participant 3, every pair {i, 3} for i in 0..2 reconstructs the original threshold secret and matches the threshold public key, and the untouched pair {0, 1} still does too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-04 04:06:59 +02:00
Kgothatso Ngako	a7d4337778	build: wire the frost_enrollment module into both build systems ... Second of six commits adding the frost_enrollment module. This one is scaffolding only: the five entry points are stubs that validate their pointer arguments, zero their outputs and return 0. What is being verified here is that the module configures, compiles, links, exports its symbols and registers its test module in both build systems -- so that the next commit changes nothing but arithmetic. Ordering is the one thing in this commit that can go silently wrong, and it goes wrong in opposite directions in the two build systems: - configure.ac executes its `if` blocks in file order, and enable_module_frost defaults to no (configure.ac:243). A block placed after the frost block at :601 that sets enable_module_frost=yes flips the variable too late: AM_CONDITIONAL goes true, so the header is installed and the Makefile fragment is pulled in, but -DENABLE_MODULE_FROST=1 is never appended, so src/secp256k1.c never includes frost's implementation and every secp256k1_frost_* symbol fails to link. The new block therefore goes ahead of both the frost block and prefractal's, which documents the same trap. - src/CMakeLists.txt processes dependents FIRST, so the same block goes above the FROST block there, beside prefractal's. Verified rather than assumed: configuring with ONLY --enable-module-frost-enrollment emits -DENABLE_MODULE_FROST=1 alongside -DENABLE_MODULE_FROST_ENROLLMENT=1, and the CMake summary prints "frost ON" for the same configuration -- the latter is what the PARENT_SCOPE lift buys, since the summary runs after add_subdirectory(src) and would otherwise report a module it is compiling in as OFF. The dependency guard is prefractal's implies-frost idiom, copied verbatim along with its reasoning. frost is default-OFF, so the `test x"$enable_module_frost" = x"no"` / `DEFINED X AND NOT X` guard every other module uses -- which reads as "the user disabled it explicitly" for a default-ON dependency -- is true by default here and cannot tell an explicit --disable-module-frost from the default once both are in the cache. Enabling frost-enrollment simply implies frost, with no error. The one frost-module change in the whole series is in this commit: src/modules/frost/session.h gains a declaration for secp256k1_frost_sort_ids, which is defined at session_impl.h:517 and declared nowhere. The params hash needs it to canonicalize identifier order. Prefractal reaches frost's statics through translation-unit ordering alone; rather than inherit reuse-by-link-order, this declares the function where keygen.h:48 already declares derive_pubshare_at, so the reuse goes through an interface. No behavior change: it is a declaration for an existing static definition in the same TU. CI wiring is two files, and skipping either half fails quietly: - ci/ci.sh gets FROST_ENROLLMENT in the reproduction header's variable list and --enable-module-frost-enrollment="$FROST_ENROLLMENT" after the prefractal line. - .github/workflows/ci.yml gets FROST_ENROLLMENT at every PREFRACTAL site: the global default, 11 inline matrix entries and 10 job-level env blocks. Without the default, ci.sh runs under set -eux with an empty $FROST_ENROLLMENT, passes --enable-module-frost-enrollment="", `test x"" = x"yes"` is false, and the module is off in all of CI while ci.sh visibly has the plumbing. Verified programmatically over the parsed workflow: across the 106 effective job contexts, PREFRACTAL and FROST_ENROLLMENT now agree in every single one (45 set to yes, no mismatches), no context sets FROST_ENROLLMENT without FROST or without EXPERIMENTAL, and no context leaves it undefined. ci.sh passes sh -n. The stub test is not a placeholder that has to be deleted later: every entry point must reject an empty helper set and leave its output zeroed, which is true of the stubs and stays true of the finished implementation, so it doubles as the check that all five symbols are reachable from the test binary. Verification. Autotools: ./autogen.sh, then a frost-enrollment-only configure and a full configure with frost, chilldkg, iceberg, prefractal and frost-enrollment all on -- both build with zero warnings under the project's -Werror-grade flag set, ./tests and ./exhaustive_tests exit 0, and `./tests -l` lists the frost_enrollment module. CMake: configure with -DSECP256K1_EXPERIMENTAL=ON -DSECP256K1_ENABLE_MODULE_FROST_ENROLLMENT=ON builds clean and ctest passes 391 tests. nm shows the five new symbols exported from libsecp256k1.so; tools/symbol-check.py could not be run here because python3-lief is not installed in this environment, but all five carry the required secp256k1_ prefix. make dist succeeds and the tarball carries src/modules/frost_enrollment/frost_enrollment.md alongside the other module documents. One unrelated observation from this build: a stale src/ctime_tests-ctime_tests.o left over from an earlier configure with a different module set will fail to link, because automake does not track CPPFLAGS changes across reconfigures. make clean between configurations with different module sets, not a fault in this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-04 03:53:03 +02:00