Files
amethyst/quartz
Claude 566a0997a2 fix(secp256k1): harden C/Kotlin field and scalar arithmetic; add dedicated fe_sqr
Correctness (C):
- scalar_mul: rewrite with a loop-driven fold so the 512-bit product is
  fully reduced regardless of the pre-existing third-fold carry-drop bug;
  also fixes the portable (!HAVE_INT128) fallback which was returning
  (a*b) mod 2^256 instead of (a*b) mod n.
- fe_mul / reduce_wide: loop the final carry fold in a while(carry) rather
  than a single if(carry), so a secondary carry-out is never silently
  dropped for adversarial or deeply lazy-reduced inputs.
- scalar_add: reuse the precomputed SCALAR_NC constant instead of
  recomputing n's two's-complement arithmetically each call.
- fe_negate: remove the data-dependent early-return for a == 0; compute
  P - a unconditionally and fold P back to 0 with a final fe_normalize,
  matching the Kotlin FieldP.neg path and dropping a branch.

Performance (C):
- Add a dedicated 10-mul fe_sqr_inline using __int128 (4 diagonal + 6
  doubled cross products, three-pass structure mirroring Kotlin
  U256.sqrWide). The previous fe_sqr delegated to fe_mul(a, a) using all
  16 schoolbook products; the new path saves ~37% of the multiplications
  at every squaring, and doublePoint/addPoints do ~9 sqrs each in the hot
  Jacobian loop. Col-3 mixes two products so the accumulator is split
  explicitly to avoid a uint128 overflow; the bug was caught by the C
  benchmark's self-test.
- Enable LTO (CMAKE_INTERPROCEDURAL_OPTIMIZATION) when the toolchain
  supports it, recovering cross-TU inlining of fe_mul/fe_sqr into point.c
  and schnorr.c.
- jni_bridge: replace the pinning GetByteArrayElements path (which blocks
  GC compaction) with a stack-or-heap copy_msg_bytes helper that uses
  GetByteArrayRegion. Short messages (32 B event digests, the common case
  for Nostr) hit a 512 B on-stack buffer.

Correctness (Kotlin):
- FieldP.neg: remove the early-return on zero for the same reasons as the
  C side, with a trailing reduceSelf(out) to collapse the P result back
  to 0 when the input was zero.
- Secp256k1.signSchnorrInternal / privKeyTweakAdd: switch the crypto-
  edge-case failure modes from generic require() to check() with clear
  messages, documenting them as invariants rather than argument errors
  and matching the C side's return-code semantics.

Tests & benchmarks:
- Add Secp256k1CrossValidationTest (jvmTest) that byte-for-byte compares
  Kotlin, ACINQ, and the custom C implementation across pubkey creation,
  Schnorr signing (incl. variable message lengths on the Kotlin side),
  privKeyTweakAdd, and x-only ECDH. This is the strongest parity check we
  can run without a third reference, and it's deterministic for
  reproducibility (fixed LCG seed).
- Add adversarial FieldPTest cases that chain lazy adds into mul/sqr/inv
  to exercise the new fe_mul fold loop and the dedicated fe_sqr path.
- Fix pre-existing FieldPTest/GlvTest failures (addNearP, addNegIsZero,
  halfOfOdd, invMulIsOne, invOfTwo, reduceWideWithMaxValues, betaCubedIsOne)
  that were asserting on raw limbs of lazy-reduced values; they now
  reduceSelf before comparing, consistent with the rest of the suite.
- Secp256k1Benchmark: document the intentional apples-to-apples
  asymmetries (signSchnorrWithPubKey vs signSchnorr, ecdhXOnly vs
  pubKeyTweakMul, privKeyTweakAdd's copyOf() penalty) and add a
  taggedHash benchmark since NIP-44 leans heavily on it.

All 188 secp256k1 Kotlin tests pass; the C library builds cleanly with
LTO enabled and the secp256k1_bench self-verification succeeds.

https://claude.ai/code/session_01KExJURZATpL59ZKXP6AVP6
2026-04-12 19:58:34 +00:00
..