test(tbtc/signer): replay the 600-case cross-language coordinator-shuffle corpus#4035
Merged
mswilkison merged 2 commits intoJun 11, 2026
Conversation
…ffle corpus Consumes the byte-identical copy of the differential corpus generated from keep-core's Go implementation (pkg/frost/roast/testdata/coordinator_shuffle_corpus.json on the RFC-21 branch): 216 integer-boundary cases (0/+-1/i64 MIN/MAX seeds, wrapping seed+attempt composition up to u32::MAX, unsorted and reversed member inputs) plus 384 generated sweeps over set sizes 1..255 with full-range seeds. select_coordinator_matches_cross_language_differential_corpus replays every case through the go_math_rand port, so any drift in source seeding, Fisher-Yates order, int31n bounds, sign handling, wrapping, or internal sorting fails this suite directly instead of fracturing coordinator agreement in a mixed deployment. Pairs with the Go-side corpus PR on feat/frost-schnorr-migration-scaffold. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review follow-up (F2). Sync the byte-identical 648-case corpus (adds the +/-MaxInt32 source-seed normalization collision from the Go side) and document the two go_math_rand port branches the differential corpus cannot reach -- int63n (dead for any u16 member set) and the int31n_fast rejection loop (fires with probability ~set_size/2^31 per draw) -- as accepted faithful 1:1 ports of Go's math/rand covered by Go's own stdlib tests. Full signer suite passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mswilkison
merged commit f3f0ac4
into
extraction/frost-signer-mirror-2026-05-26
19 checks passed
mswilkison
added a commit
that referenced
this pull request
Jun 12, 2026
…submodules (#4036) Post-merge follow-up #2 from the June 2026 review stack (#4028–#4035). `engine.rs` was deferred-split to avoid conflicting with the open stack; with the stack merged it had grown to 18,248 lines (absorbing four merges plus the round-nonce-v3 fix), and every new PR contends for the same file. This lands the split before anything new piles onto the monolith. ## What this is A **pure code move** — no behavior change, no API change, no test-path change. | Module | Lines | Contents | |---|---|---| | `state` | 434 | in-memory engine/session state, state-file lock, registry capacity guards | | `persistence` | 1,421 | encrypted state envelope, key providers/commands, corruption recovery, persisted↔live conversions | | `config` | 392 | the `TBTC_SIGNER_*` env surface: const names, defaults, parsers, profile detection | | `policy` | 633 | admission, signing-policy firewall, rate limiting, auto-quarantine config | | `provenance` | 353 | runtime provenance attestation gate | | `telemetry` | 313 | hardening latency trackers + metrics | | `lifecycle` | 468 | canary rollout, refresh cadence/shares, emergency rekey, quarantine status | | `audit` | 376 | transcript audit, blame-proof verification, differential fuzzing | | `codec` | 430 | hex/struct codecs, Go↔frost identifier conversions | | `frost_ops` | 303 | stateless `dkg_part1..3`, nonces, signing package, share, aggregate | | `nonce` | 99 | **`RoundNonceBinding` + deterministic round-nonce derivation (round-nonce-v3), isolated for audit** | | `roast` | 1,003 | RFC-21 attempt machinery: request fingerprints, round/attempt ids, attempt-context + transition-evidence validation | | `dkg` | 257 | `run_dkg` flow + transitional-dealer production gates | | `signing` | 970 | `start_sign_round` / `finalize_sign_round` flows, bootstrap synthetic contributions | | `transaction` | 227 | taproot tx building | | `testsupport` | 88 | cfg(test) cross-module helpers (`lock_test_state`, `reset_for_tests`, …) | | `tests` | 10,558 | the former inline `mod tests`, moved **verbatim** | ## Design decisions - **`engine::tests::*` paths are preserved.** `mod tests` moved as a single child module (`engine/tests.rs`), so `scripts/run_phase5_chaos_suite.sh`'s five `cargo test … -- --exact` filters and every `engine::tests::<name>` reference in the phase docs remain valid. Splitting tests further would force rewriting those contracts — left as an explicit team decision. - **Visibility:** formerly-private items are now `pub(crate)`; each submodule opens with `use super::*;` against glob re-exports in `mod.rs`. Since `lib.rs` keeps `mod engine;` **private**, the crate-external surface is byte-identical. Per-module visibility tightening can happen incrementally later. - **`config.rs` deliberately concentrates the env surface** — it pre-stages follow-up #3 (move `TBTC_SIGNER_*` env vars into an init-time FFI config struct) as a mostly-one-file change. - **Only semantic edit in the whole diff:** the `include_str!("../testdata/coordinator_seed_vectors.json")` in the tests gains one `../` because the file now sits one directory deeper. ## Verification - `cargo fmt --check` ✅, `cargo clippy --all-targets -- -D warnings` ✅ - Full suite: **223 passed + 1 ignored / 24 / 1 — counts identical to the pre-split HEAD** (verified by stashing the split and re-running on d47f009) - `cargo test formal_verification_` ✅ (5/5); all five chaos-suite `--exact` paths ✅ - `testdata/` untouched — seed vectors and shuffle corpus remain byte-identical - Review aid: `git diff d47f009 --color-moved=zebra --color-moved-ws=ignore-all-space` renders nearly the entire diff as moved lines; `git blame -C -C` follows history across the split. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
mswilkison
added a commit
that referenced
this pull request
Jun 12, 2026
…ig (#4037) Post-merge follow-up **#3** from the June 2026 review stack (#4028–#4035; #4036 landed the engine split that stages this change): move the `TBTC_SIGNER_*` env-var surface into an init-time FFI config struct, shrinking the ops/audit surface from ~40 scattered `std::env::var` reads to one explicit, validated installation at startup. ## What this adds New FFI entry `frost_tbtc_init_signer_config(request_ptr, request_len)` taking a typed JSON `InitSignerConfigRequest` (40 optional fields; field name = lowercased `TBTC_SIGNER_*` suffix). The host installs it once at startup. ## Semantics - **Wholesale source of truth.** Once installed, the environment is *not consulted* for any covered knob; an unset field means the built-in default. No per-knob mixing of config and env — split-brain configs can't exist. - **Fail-closed init.** `deny_unknown_fields` rejects typo'd knobs; enforcement-gated policy combinations (admission, signing-policy firewall, auto-quarantine) are validated at install by running the same loaders the runtime gates use, with rollback on rejection — a misconfigured signer fails at startup, not at first signing. - **Idempotent re-init** for an identical request (fingerprint match); conflicting re-init rejected. - **Secrets never ride the config FFI.** `TBTC_SIGNER_STATE_ENCRYPTION_KEY_HEX` is read exclusively from the dedicated env/command key-provider channel even when a config is installed (the one deliberate `std::env::var` left outside the chokepoint, commented at the read). - **Transitional compatibility.** With no config installed, `engine::signer_env_var` falls through to the process environment — existing hosts and the entire pre-existing test suite run unchanged; non-development profiles log a one-time warning suggesting the init FFI. ## Why parity is safe by construction The typed request converts to the same canonical strings the existing env parsers consume (`"true"`/`"false"`, decimal ints, comma-joined identifier lists), and every existing clamp/warn/reject path runs unchanged on identical inputs. The diff swaps `std::env::var(X)` → `signer_env_var(X)` at 31 sites and changes nothing else about how values are interpreted. Also deletes `lib.rs`'s duplicated profile/truthy parsing in favor of the engine's single implementation. Thanks to #4036, this lands as one new ~400-line module (`engine/init_config.rs`) plus one-line touches across `config/lifecycle/persistence/policy/provenance/state` — not an 18k-line-file churn. `engine/config.rs` remains the single home of the env-name constants. ## Verification - `cargo fmt --check` ✅; `cargo clippy --all-targets -- -D warnings` ✅ - Full suite **235 passed + 1 ignored / 24 / 1** — all 224 pre-existing tests pass unchanged (env-fallback parity), plus 11 new tests: config-over-env precedence, wholesale env-ignoring for unset fields, idempotent/conflicting re-init, invalid-profile rejection, install rollback on incomplete firewall policy, complete-admission-policy validation, secret-stays-on-env-channel, production-profile-forces-strict via config, `reset_for_tests` clearing, `deny_unknown_fields`, list/bool canonicalization, and an FFI round-trip - `--features bench-restart-hook` builds; chaos suite (5/5 `--exact` paths) and `formal_verification_` filter pass - `include/frost_tbtc.h` gains the symbol; README documents the contract ## Notes for reviewers - Knobs the runtime warn-and-defaults on (e.g. out-of-range timeouts) keep that behavior under config values — init validation only rejects what the runtime gates would reject. Tightening init further is possible later without breaking the contract. - Go-host adoption is a follow-up: this is additive ABI; nothing changes for hosts until they call the new entry. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
mswilkison
added a commit
that referenced
this pull request
Jun 12, 2026
…ence (#4040) Post-merge follow-up **#4** from the June 2026 review stack (#4028–#4035): replace `json.Marshal` as the canonical signed-bytes encoding for evidence snapshots/bundles — explicitly scheduled to land **before Phase 7 wiring ossifies the format**. (Items 2 and 3 landed as #4036/#4037 on the mirror branch; this is the Go-side sibling on the scaffold branch.) ## Why now The RFC-21 Layer B evidence signatures were computed over canonical JSON. That byte stability is a Go-implementation accident — field-order-stable `encoding/json` output — not a portable contract. The moment Phase 7 wires evidence verification into the Rust signer (or any second implementation appears), every verifier would need to replicate Go's exact JSON emission. No persisted or cross-component evidence exists yet, so the format can still change for free. ## Design: sign what you transmit, verify what you received New `pkg/frost/roast/gen/pb/evidence.proto`: - A snapshot travels as `SignedLocalEvidenceSnapshot{body, operator_signature}` where `body` is the serialized `LocalEvidenceSnapshotBody` — the operator signs those exact bytes. - A transition message travels as `SignedTransitionMessage{body, coordinator_signature}` whose `TransitionMessageBody` embeds every member's signed snapshot envelope **verbatim** (`repeated bytes signed_snapshots`) — the coordinator attests to the exact signed snapshots it assembled, in order. - Producers marshal a body **exactly once**, at signing time, and cache it; parsed messages retain received body/envelope bytes verbatim; verification always runs over exact received bytes. **Nothing in the evidence chain is ever re-encoded**, so signature validity never depends on any serializer's canonical form — across protobuf library versions or across languages. This deliberately sidesteps protobuf's own caveat that deterministic serialization is not canonical across implementations. - `Marshal` of a received message returns the received envelope verbatim — evidence bytes survive re-broadcast, including wire-legal but non-canonical encodings (pinned by a handcrafted reversed-field-order test). - `CanonicalSnapshotBytes`/`CanonicalBundleBytes` → `SignableBytes()` accessors; the coordinator's first-write-wins conflict check now compares exact signed bytes. ## Tests - Existing suite migrated off JSON fixtures: test-only encode helpers bypass production signing so every structural-rejection path (zero sender, bad hash length, unsorted/duplicate entries, oversize caps, bundle ordering/hash-binding) is still exercised at the wire level. - New `wire_test.go` pins the format's core properties: byte-preservation through unmarshal→re-marshal, verbatim snapshot-envelope embedding inside bundle bodies, producer-signed bytes == receiver-verified bytes, non-canonical-encoding survival, tampered-body verification failure. - `go build ./...`, `go vet`, `gofmt` clean; frost + tbtc package tests green. Generated with protoc 33.4 / protoc-gen-go v1.36.3 (matches the go.mod protobuf runtime v1.36.3). ## Docs RFC-21 "Evidence message format" decision rewritten: signed-body protobuf envelopes, with the retirement rationale for canonical JSON recorded. ## Notes for reviewers - The in-memory model types (`LocalEvidenceSnapshot`, `TransitionMessage`) are unchanged apart from two unexported byte caches; all call sites kept their shapes. - Immutability contract: evidence fields must not be mutated after `SignableBytes()` is first computed (documented on the cache fields); the aggregation flow already treats snapshots as immutable post-receipt. - Phase 7 cross-language note: the Rust signer will verify operator/coordinator signatures over `body` bytes and parse them with any protobuf implementation — no canonicalization requirements transfer. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #4005 (base:
extraction/frost-signer-mirror-2026-05-26). Rust half of the corpus-based differential parity item from the review; pairs with the Go-side PR #4034.Adds
testdata/coordinator_shuffle_corpus.json— a byte-identical copy of the canonical 600-case corpus generated from keep-core's GoSelectCoordinator— andselect_coordinator_matches_cross_language_differential_corpus, which replays every case through thego_math_randport: 216 integer-boundary cases (seeds 0/±1/i64::MIN/i64::MAX/the #4026 pin seed; wrappingseed + attemptcomposition up tou32::MAX; unsorted and reversed member inputs pinning the internal sort) plus 384 generated sweeps over set sizes 1..255 with full-range seeds.All 600 cases replay identically today — direct evidence the
math/randport is bit-exact across the boundary regions where ports diverge first. Any future drift in source seeding, Fisher-Yates order,int31nbounds, sign handling, wrapping, or sorting fails this suite on the drifting side.Full signer suite passes (245 tests); clippy/rustfmt clean. Mirror note: port back to the tBTC monorepo signer with the next extraction sync.
🤖 Generated with Claude Code