fix: address PR #37 review findings

[piotr-roslaniec]

Follow-up to #37. It fixes the confirmed findings from a multi-lens review of the dev integration (9ff573c against master). The review found no P0 or P1 issues. It confirmed 68 findings: 16 P2 and 52 P3. This PR fixes 65 of them. Three are not applied because they conflict with another fix or with a recorded decision; the reasons are listed below. A second multi-lens review of this PR found more gaps; they are fixed in the later commits and summarized under "Second review pass".

Changes callers can see

• Narrower constant-time exponents in MtA. New paillier.PublicKey.HomoMultBounded(m, c1, bound) takes a public exclusive bound, rejects m outside [0, bound) (ErrMessageTooLong) and a bad bound (ErrInvalidBound), and pads the constant-time exponent to bound.BitLen(). HomoMult(m, c1) is HomoMultBounded(m, c1, N). BobMid/BobMidWC pass q. The Bob and Alice provers pad x and m to q.BitLen() instead of the 2048-bit Paillier width; y keeps its Paillier-width domain and padding. Proof bytes do not change: the new oracle leg below regenerates the historical vectors byte for byte. Measured with the new BenchmarkBobMid (constant-time on, security-v2 session, -benchtime=15x -count=3 -cpu=1): BobMid 73.6 ms -> 58.7 ms (-20%), BobMidWC 72.9 -> 58.7 ms, one full MtA exchange about -10%. This helps merge gate 3 but does not close it by itself.
• Tighter witness domains. BobMid/BobMidWC reject b outside [0, q). ProveBob/ProveBobWC reject x outside [0, q). ProveRangeAlice rejects m outside [0, q). The provers return errors instead of panicking for degenerate pk, N, NTilde or X inputs. common.GetRandomPositiveInt and the other samplers return nil instead of panicking when the limit is wider than the 5000-bit sampler cap, so the provers return an error for any NTilde that is too wide. Signing round 2 checks the local gamma and w are in [0, q) before the per-peer MtA work and blames no peer if they are not.
• paillier.HomoAdd validates the key modulus the same way Encrypt, HomoMult and Decrypt already do.
• common.IsUsableUnknownOrderModulus enforces the 65,536-bit ceiling itself, before the primality test. The six separate ceiling pre-checks in the verifiers are removed and the helper is unexported. Verifier errors print the modulus bit length, not its value.
• ckd.NewExtendedKeyFromString picks its parser by curve parameters (crypto.SameCurve). Other curves are decoded with elliptic.UnmarshalCompressed (a P-256 key now round-trips); undecodable key data returns an error instead of a key with nil coordinates.
• Keygen computes the ring-Pedersen beta = alpha^-1 mod pq with the constant-time inverse when constant-time operations are on. The reduction of alpha mod pq before the inverse is still variable-time; this is recorded as a known gap.
• Signing constructors copy keyDerivationDelta, the same way they already copy the message.
• ModProof wipes its pre-encoded secret exponents through a defer, so the wipe also runs when proof generation panics. A test hook proves the wipe runs on panic.

CI and the legacy transcript checks

• verify.sh gains a third leg. It regenerates the vectors with the HEAD legacy provers, verifies them with the pinned historical 2e712689 verifier, and checks they equal prior_v2.5.2.json outside provenance. Before this, CI only checked historical proofs against HEAD verifiers.
• New Mixed-binary interop job: it runs verify_mixed_interop.sh (the live exchange with the historical binary), plus vet and tests for the harness. go test ./... skips testdata/, so this code never ran in CI before.
• New Go minimum build job: it builds and vets with Go 1.25.7 and GOTOOLCHAIN=local.
• Mixed-binary interop and Go minimum build are required checks on dev and master.

Tests

New or rewritten tests cover:

• the modulus policy at every exported unknown-order verifier, and the 2,048- and 65,536-bit edges;
• the compat bound (q+1)*N, the security-v2 q^7 bound, the historicalBobCompat=false path, and one Bob response bound shifted past its limit with the verification equation still holding;
• the DLN reject paths, which used to stop at the 2,048-bit floor first;
• the constructor mode and nonce panics, and the legacy and security-v2 proof contexts;
• mixed-mode keygen and signing failing closed with exact culprits;
• the delivered *common.SignatureData: fields, low-S, recovery id and ownership;
• the round 5 and round 7 store-before-emit ordering (fix(signing): publish round state before messages #36);
• the round 2 local witness check (no culprits) and BobMid/BobMidWC at b = q and b = -1;
• the provers with a 4501-bit NTilde (error, no panic);
• HomoMultBounded bound edges, and x, m, b at q-1 in constant-time mode;
• the keygen ringPedersenBeta helper in both timing modes;
• the copy of keyDerivationDelta;
• a signing ceremony with constant-time operations disabled;
• the m = q-1 boundary in both modes;
• the Paillier error sentinels.

Three log-only timing tests and the signing "CT wiring" test are removed: none of them could fail.

Docs

CHANGELOG and README corrections:

• the constant-time coverage now also lists the MtA proof blinds and the variable-time secp256k1 scalar multiplication;
• BC4 now describes the ModProof and FactorProof challenge derivation the code actually uses;
• BC4 and BC5 are scoped to security-v2;
• the break ledger uses the 2e712689 baseline;
• "three of four caller obligations are enforced at runtime";
• the README example uses one Parameters and one nonce per ceremony, and its keygen call compiles;
• the release tag must stay on v0/v1 because the module path has no /v2;
• keygen mixed-binary interop is backed by oracle vectors only;
• rx is a public value;
• undefined review labels are removed.

Not applied

• M025 (drop ModProof's pre-encoded exponents) conflicts with M020 and M002, which keep the reviewed 873b8ad design and add the deferred wipe and its test.
• M028 (per-ceremony Decryptor to cache mu) is contradicted by the fix(paillier): stop caching private-key decryption state; review follow-ups #38 measurement: caching LambdaN and mu gave no Decrypt gain (70.7 vs 70.6 ms). Author decision 1 removed that cache.
• M055 (widen legacy tau to q^3*NTilde): not applied, but now recorded as a known gap in the CHANGELOG. The legacy range matches the deployed historical prover 2e712689 and keeps legacy proofs byte-identical; verifiers do not depend on it. GG18 specifies q^3*NTilde to hide e*sigma statistically; security-v2 uses that range.

Not in this repo: #37's description

Some findings point at text in #37's description. The "Known risks" bullet should add the MtA blinds and secp256k1 scalar multiplication, and should drop the round-5 rx reduction, which handles a public value. Next step 4.1's example tag v2.0.0-threshold.1 cannot be consumed under the current module path; a v1.x tag works.

Second review pass

A multi-lens review of this PR (correctness, security, simplicity, contrarian, performance, docs, test coverage, spec compliance, intent) found these gaps. All are fixed in the later commits unless marked otherwise.

• Prover panic not fully fixed: the first fix compared NTilde against the sampler cap, but the provers sample below q^3*NTilde, so a 4233-5000-bit NTilde still panicked (reproduced). Fixed at the root in the samplers.
• Tests that could not fail: mutation runs showed the DLN reject tests, the compat and q^7 bound-edge tests, the verifier modulus-gate tests, the ModProof deferred-wipe test, the x padding at q-1, and the keygen beta test all still passed with the guarded check removed. Each is rewritten with equation-valid inputs or exact edges, and each mutation now fails a test. The DLN "non-unit" row now uses a real factor of N.
• API footgun: HomoMultWithBitLen let a caller pass a secret-derived width; replaced by HomoMultBounded.
• Wrong culprits: a local witness error in BobMid was blamed on every peer; now checked once in round 2 with no culprits.
• M008 left half done: the six verifier pre-checks are now removed.
• Docs: the PR perf(paillier,common): narrow CT exponent widths, apply Paillier binomial identity, fix sync.Pool allocation (SA6002) #31 CHANGELOG bullet is restored to its original text; the tighter witness domains are listed under tightened validation; the constant-time coverage header names the real AliceEnd variable-time steps; new known gaps for keygen alpha mod pq and legacy tau; constructor panic lists are complete; review labels removed.
• Test files renamed after their behavior: bob_bounds_test.go, protocol_mode_test.go, signature_delivery_test.go.
• Partly addressed rows in the table below: M001 and M141 (doc wording, now completed), M042 and M050 (now exact-edge and shift tests), M056 and M127 (remaining labels removed), M059 (ModVerify rows added). M125 is answered by the live mixed-binary harness, not by a new oracle vector.
• Reviewed and left as is: FactorProof, ModProof and NewDLNProof use sampler output without a nil check. They have no error return; the panic is not new; the wire path pins moduli to 2048 bits.
• Rollback: all changes are local. No wire format or proof bytes change, so a revert has no interop impact.

Verification

At the PR head after the second review pass (local, Go 1.27.1):

• gofmt -l: clean. go vet and go build ./...: pass.
• go test -count=1 -timeout 60m $(go list ./...): all packages pass in 229 s (keygen 228 s, signing 86 s, paillier 46 s, mta 37 s).
• ./testdata/legacy_transcript/verify.sh: all three legs pass; canonical_legacy_vectors_equal=true.
• ./testdata/legacy_transcript/verify_mixed_interop.sh: exit 0. Reject run: default rejected at round 3 with 2 culprit entries. Accept run: accepted, both peers reach round 8. The homogeneous control reaches round 8.
• Mutation checks: each mutation listed under "Second review pass" now fails at least one test.

The first-pass run (Go 1.26.8, before the second review pass) also passed go vet/go test for ./testdata/legacy_transcript/mixed_interop and targeted -race runs of the concurrency tests in signing, keygen and paillier; these were not re-run.

Findings and where each is addressed

ID	Sev	Review location	Finding	Commit
M001	P2	common/constant_time.go:14-40	CHANGELOG and PR risk text overclaim constant-time coverage: MtA proof blinds and secret EC scalar multiplications stay variable-time and ...	docs
M002	P3	common/constant_time.go:207-215	The secret-wipe behavior advertised by 873b8ad has no assertion	fix(paillier)
M003	P3	common/constant_time.go:292	GetCTModInt keeps an exported, unbounded, never-evicted global cache keyed by modulus	fix(common)
M004	P3	common/constant_time.go:310	NewCTModIntWithPhi duplicates NewCTModInt's body and has one caller	fix(common)
M007	P3	common/validation.go:21-30,39-41	The 65,536-bit modulus ceiling does not bound verifier CPU as its comment claims (ProbablyPrime is superlinear)	fix(common)
M008	P3	common/validation.go:33-48	Modulus-policy check is a two-call protocol (ceiling before primality) hand-ordered at each of 6 verifiers	fix(common)
M010	P3	common/constant_time.go:13-14,34-38	common/constant_time.go header: garbled COVERAGE sentence, change-narration with no issue link, stale Encrypt coverage entry	fix(common)
M014	P3	common/constant_time_test.go:431-555	TestMulCTTimingConsistency, TestModInverseCTTimingConsistency and TestExpCTTimingConsistency can never fail	fix(common)
M015	P3	common/int.go:104	CHANGELOG "Added" says IsInInterval and tss.SameCurve back the hardened range checks; IsInInterval has no production caller	docs
M017	P3	common/random.go:80,98,123-130	New nil-return contracts are undocumented	fix(common)
M019	P3	common/validation.go:13-18,24-26,33-38	common/validation.go modulus-policy comments: illogical Min rationale and false O(bitLen) claims	fix(common)
M020	P3	crypto/paillier/mod_proof.go:96	ModProof exponent wipe is not deferred, so a panic skips it; its doc comment says it also runs on abort	fix(paillier)
M021	P2	crypto/paillier/paillier.go:194	Constant-time exponents are padded to N.BitLen() (2048) instead of public protocol bounds: HomoMult and the MtA provers do 2-8x the needed ...	fix(paillier), fix(mta)
M023	P3	crypto/paillier/key_reuse_regression_test.go:81-119	TestKeyReuseAfterKeyValuesReplaced and TestByValueKeyCopyIsIndependent use the public-key N² cache only for the ciphertext producers ...	fix(paillier)
M024	P2	crypto/paillier/mod_proof.go:184-199	CHANGELOG Breaking Change 4 says ModProof/FactorProof use HashToNTagged; no production code calls it	docs
M025	P3	crypto/paillier/mod_proof.go:244-298,96,87,307,328	modProofCTContext holds four pre-encoded secret exponent buffers, and they only partly pay off	not applied
M026	P3	crypto/paillier/mod_proof.go:368-405	Four standalone QR helpers are test-only copies of the context methods	fix(paillier)
M027	P3	crypto/paillier/paillier.go:121-123,170-180,233-239	No test references ErrMessageTooLong or ErrMessageMalFormed	fix(paillier)
M028	P3	crypto/paillier/paillier.go:252-259	Decrypt rebuilds a key-constant value on every call	not applied
M035	P3	crypto/paillier/mod_proof.go:65-70,232-236	mod_proof.go comments narrate the change history instead of describing the current design	fix(paillier)
M037	P3	crypto/paillier/paillier.go:157,193,196	runtime.KeepAlive(publicKey) appears three times with no comment	fix(paillier)
M038	P3	crypto/paillier/paillier.go:200-212	HomoAdd lacks the checkPaillierModulus guard the other Paillier operations gained	fix(paillier)
M039	P3	crypto/paillier/paillier.go:259-271	The ErrMalformedKey doc says a non-invertible decryption coefficient is reported "consistently by both" modes	fix(paillier)
M040	P3	crypto/paillier/paillier.go:284-285	The Proof doc runs two sentences together with no break: "...square-free from 3.1" then "It panics if..."	fix(paillier)
M042	P2	crypto/mta/proof_bob_verifier.go:74-79	Nothing tests the upper edge of the opt-in historical-compat bound T1 < (q+1)*N	fix(mta)
M043	P2	crypto/mta/legacy_bob_compatibility_test.go:119-138	The security-v2 Bob T1 bound (q^7 + 1) is never exercised by a test	fix(mta)
M044	P3	crypto/mta/proof_bob_verifier.go:44-60	Bob and RangeAlice verifiers duplicate the same modulus/generator/ciphertext preamble	fix(mta)
M045	P3	crypto/mta/proof_bob_verifier.go:84-95	The default tight Bob bound is justified with the wrong prover, and "legacy" names two different provers	fix(mta)
M046	P3	crypto/mta/proofs.go:40-52,61-62,76-78,116	Constant-time path panics on even/nil/degenerate moduli in exported provers; 3b609ae no-panic claim overstated	fix(mta)
M048	P3	crypto/mta/share_protocol.go:105-185	Four near-identical "verify, then decrypt, then reduce mod q" functions	fix(mta)
M049	P3	crypto/mta/share_protocol.go:144-148	AliceEndLegacy doc comment has a truncated sentence about the historicalBobCompat=false case	fix(mta)
M050	P2	crypto/mta/share_protocol_test.go:183-203	"Overwide ... must fail before exponentiation" MtA tests cannot tell the cheap bound check from the later equation failure	fix(paillier), fix(mta)
M055	P3	crypto/mta/proofs.go:70-79	Legacy mode now samples tau below qNTilde, while master used q^3NTilde and the security-v2 branch still does	not applied
M056	P3	crypto/mta/proofs.go:92	Review-process labels appear with no definition in production comments, CHANGELOG, READMEs and CI: "PRIOR", "R1", "R01", "D9", "D10", "R2", ...	fix(mta), ci, docs
M058	P3	crypto/mta/share_protocol.go:149-185	AliceEndLegacy and AliceEndWCLegacy with historicalBobCompat=false rejecting a historical-witness proof, and the plain AliceEndLegacy ...	fix(mta)
M059	P2	crypto/dlnproof/proof.go:92-97	The #34 modulus policy (2048..65536 bits, odd, composite) is tested at one verifier and only for the ceiling	fix(common), fix(paillier), fix(mta), fix(crypto)
M060	P2	crypto/dlnproof/proof_test.go:37-95	DLN Verify reject tests use N=23, so the 2048-bit floor rejects first and the named T/Alpha/nil checks are never reached	fix(crypto)
M062	P3	crypto/ckd/child_key_derivation.go:108-123	ckd curve type switch returns a nil-coordinate key with nil error for non-v2 secp256k1 curve objects	fix(crypto), docs
M067	P3	crypto/commitments/commitment.go:55-71	CHANGELOG says canonical-generator checks were added in crypto/commitments; that package has none	docs
M075	P2	ecdsa/keygen/local_party.go:71	No test shows the keygen/signing constructors enforce the explicit protocol-mode rule (the headline runtime break)	fix(keygen), fix(signing)
M077	P2	ecdsa/keygen/rounds.go:104-121	Legacy keygen transcript selection (proofSession/proofContext return nil) is not pinned by any test	fix(keygen)
M079	P3	ecdsa/keygen/prepare.go:129	Keygen ring-Pedersen trapdoor inverse (beta = alpha^-1 mod pq) stays variable-time, contrary to Breaking Change 8	fix(keygen)
M083	P3	ecdsa/keygen/rounds.go:125-131	The keygen getSSID doc still says "Callers must invoke this exactly once, in round 1", but legacy mode never calls it; the call sits only ...	fix(keygen)
M084	P2	ecdsa/signing/finalize.go:82	No test inspects the delivered *common.SignatureData (breaking change 9): R/S/recovery id/M width and the proto.Clone copy are unpinned	fix(signing)
M085	P2	ecdsa/signing/round_5.go:102-111	The #36 store-before-emit ordering in signing rounds 5 and 7 has no regression test	fix(signing)
M086	P3	ecdsa/signing/local_party.go:156	The constructor copies msg (lines 157-159) to stop caller mutation changing the signing context, but keyDerivationDelta is stored by ...	fix(signing)
M088	P3	ecdsa/signing/local_party.go:~110-122	The constructor docs omit the new caller obligation, which is the main compatibility break	fix(keygen), fix(signing), docs
M093	P3	ecdsa/signing/round_5.go:77	CHANGELOG, constant_time.go and round_5.go disagree on whether rx is secret; it is derived from public values by round 5	fix(common), fix(signing), docs
M094	P2	ecdsa/signing/round_ct_wiring_test.go:27	The signing "CT wiring" tests cannot fail if the constant-time branch is removed	fix(signing)
M096	P3	ecdsa/signing/signing_boundaries_test.go:297-376	The m = q / q-1 boundaries are covered only in legacy mode and not for the SSID or full-ceremony effect	fix(signing)
M097	P3	ecdsa/signing/signing_boundaries_test.go:32	Mixed-mode ceremonies (a legacy party among security-v2 peers, or the reverse) are not tested	fix(keygen), fix(signing)
M104	P3	ecdsa/signing/signing_boundaries_test.go:361-367	The m = q-1 accept subtest receives from out with select { case m := <-out: } and no timeout	fix(signing)
M113	P3	tss/params.go:160-166	SetLegacyHistoricalBobCompatibility(false) panics outside legacy mode, contrary to the documented contract	docs
M116	P3	tss/params.go:226-227	SetSessionNonce godoc says keygen/signing fail closed without a nonce; true only in security-v2	docs
M122	P2	testdata/legacy_transcript/verify.sh:22-37	The CI "bidirectional" oracle never checks that HEAD legacy provers produce proofs the historical verifier accepts	ci
M123	P3	testdata/legacy_transcript/README.md	Mixed-binary evidence is narrower than claimed: no live mixed keygen, stops at round 8	ci, docs
M124	P3	testdata/legacy_transcript/README.md:30-46	The README describes the proof-source digest as SHA-256 over the listed repository files, which implies a reader can reproduce it from the ...	ci
M125	P3	testdata/legacy_transcript/oracle/main.go:225-227	The oracle vector holds no wide-witness historical Bob/BobWC proof, the shape the compat toggle exists for	ci
M127	P3	testdata/legacy_transcript/README.md:93,145,160	The fixture shape is called "2-of-20" in some places and "2-of-2" in others	ci
M128	P3	testdata/legacy_transcript/verify_mixed_interop.sh:16-17	The script header says the compat-on party accepts "the identical exchange"	ci
M129	P2	.github/workflows/test.yml:99-103	The live mixed-binary harness (verify_mixed_interop.sh) and its own tests never run in CI	ci
M130	P3	.github/workflows/test.yml:42,74,96	Documented Go 1.25.7 minimum is never built in CI	ci
M132	P3	go.mod:1	Proposed release tag v2.0.0-threshold.1 is not consumable as a Go module version: module path has no /v2 suffix	docs
M136	P2	README.md:63-98,145-150	README example reuses one Parameters (and session nonce) for keygen and signing, and never says Parameters is frozen per ceremony	docs
M138	P3	CHANGELOG.md:322-328	Breaking Changes 4 and 5 were not updated for dual-mode, and they contradict #3 (lines 335-350) and the legacy contract	docs
M140	P3	CHANGELOG.md:478-480	CHANGELOG break-ledger blockquote: wrong verification baseline and incomplete list	docs
M141	P3	CHANGELOG.md:91-99	CHANGELOG claims four caller obligations are enforced at runtime; obligation 4 (and parts of 2/3) are not	docs
M149	P3	README.md:83	README keygen example passes *LocalPreParams where NewLocalParty takes LocalPreParams by value	docs

[piotr-roslaniec]

Follow-ups from the second review pass (the repo has issues turned off, so they are recorded here):

1. Legacy MtA prover samples tau below q*NTilde: confirm zero-knowledge impact

In ProtocolModeLegacy, the MtA provers ProveBob/ProveBobWC (crypto/mta/proofs.go) sample tau below q*NTilde. GG18 Fig. 10 specifies q^3*NTilde, so that t3 = e*sigma + tau statistically hides e*sigma (about q^2*NTilde). With the narrower range, t3 may reveal information about sigma, the blind on h1^y.

Context:

• The deployed historical prover (threshold-network/tss-lib@2e712689) uses the same q*NTilde range, so this is existing production behavior, not a new weakness.
• origin/master (bnb upstream) and security-v2 mode use q^3*NTilde.
• Verifiers do not depend on the tau distribution, so widening it would not break interop with historical peers. Only the byte-equality leg of testdata/legacy_transcript/verify.sh relies on the narrow range.
• Recorded as a known gap in CHANGELOG.md (PR fix: address PR #37 review findings #41).

Ask: confirm with a cryptographer how much the narrow range weakens zero knowledge in practice, and decide whether to widen it in legacy mode (changing the verify.sh leg to check acceptance by the historical verifier instead of byte equality) or to rely on the move to security-v2.

2. perf(mta): use Gamma^gamma = 1 + gamma*N in the Bob prover

ProveBob/ProveBobWC (crypto/mta/proofs.go, step 9 of GG18 Fig. 10) compute Gamma^gamma mod N^2 with a full exponentiation. With Gamma = N+1, the binomial identity gives Gamma^gamma mod N^2 = 1 + gamma*N mod N^2. paillier.Encrypt already uses this identity.

Using it in the prover (and in the matching verifier term, if one exists) removes one ~2k-bit modular exponentiation. Proof bytes do not change.

Estimated saving: about 8-10 ms per BobMid (not measured). Current figures from BenchmarkBobMid (constant-time on, security-v2 session): 58.7 ms per call after PR #41.

Ask: implement after #37 merges, measure with go test ./crypto/mta/ -run '^$' -bench BobMid -benchtime=15x -count=3 -cpu=1, and check byte equality with testdata/legacy_transcript/verify.sh. Note: gamma is secret, so the multiplication gamma*N should keep the constant-time handling the current path has.