refactor(frost): run a signing session as k FROST instances in lockstep
Phase 2 of docs/frost-batch-signing.md. Pure refactor: proposals still carry one event, the wire is byte-identical, and every test passes unchanged -- 344 jvmTest and 217 testDebugUnitTest, none of them edited in this commit. advance() now loops over FrostSigningItem rows rather than reading the first one. One nonce per item, one aggregate per item, one Session.create per item, one partial signature per item, one signature per item. The signer set, the public shares, the tweak cache and the approval stay shared, because they are the terms that do not enter e = H(R‖P‖m). The coordinator's aggregation is the place where that distinction bites: it builds one AggregatedNonce per item, each from that item's nonce from each chosen signer. Reusing one across two items would be reusing R across two messages. ## The payload codec, early joinPayload/splitPayload land here rather than with the wire change, because at a batch of one a comma join is the identity -- the payload is the bare value it has always been. That leaves Phase 3 to the proposal encoding alone. splitPayload is strict: a payload that is not exactly the batch's length is dropped rather than truncated or padded. It runs in orderedNonces, orderedPartialSignatures and splitForSession -- never in record(), which stores payloads without parsing them so that a nonce can arrive before the proposal that would give it a length to check against. ## Two short-circuits, and one trap in the first advance() runs on every arriving message, so at a batch of k it was k native key generations, k Session.creates and k signs each time, usually to discover there was nothing left to do. - Nonces are generated by `lazy`. The obvious version -- a guard computing `ownNonce == null || (isSigner() && ownPartial == null)` -- is wrong, and wrong in a way that reads fine and fails every signing test: the coordinator settles the signer set further down the same pass, so isSigner() at the top is false on exactly the pass where the coordinator goes on to sign, and the nonces are never generated. Reproduced as IndexOutOfBounds before switching to lazy, which has no prediction to make. - A device that is neither signing nor aggregating leaves before building any FROST session, rather than building k of them to do nothing with. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -179,11 +179,25 @@ k=64 it is 64 nonce generations, 64 `Session.create`s and 64 signs on every
|
||||
message the group sends — several hundred native calls to discover there is
|
||||
nothing to do.
|
||||
|
||||
Fix it in this phase, while it is still cheap to verify: short-circuit the
|
||||
regenerate-and-sign block when this device has already published both its nonce
|
||||
and its partial-signature messages, and skip `Session.create` for a device that
|
||||
is not in the signer set and is not the coordinator. Both are pure
|
||||
optimisations at k=1, which is the point of doing them before k>1 exists.
|
||||
Fix it in this phase, while it is still cheap to verify: generate nonces on
|
||||
demand, and leave before building any `Session` when this device is neither
|
||||
signing nor aggregating. Both are pure optimisations at k=1, which is the point
|
||||
of doing them before k>1 exists.
|
||||
|
||||
**Generate them lazily, not behind a guard.** The obvious version — compute the
|
||||
nonces only when `ownNonce == null || (isSigner() && ownPartial == null)` — is
|
||||
wrong, and wrong in a way that passes a reading and fails every test. The
|
||||
coordinator settles the signer set *further down the same pass*, so `isSigner()`
|
||||
read at the top of `advance()` is false on the pass where the coordinator is
|
||||
about to become a signer, and the nonces it then needs were never generated. A
|
||||
`by lazy` has no such prediction to make: it generates at first use, at most
|
||||
once per pass, and never on a pass with nothing to publish.
|
||||
|
||||
### The payload codec, early
|
||||
|
||||
`joinPayload`/`splitPayload` land here rather than in Phase 3, because at k=1
|
||||
they are the identity — a one-element comma join is the bare value — so they
|
||||
change no byte on the wire and leave Phase 3 to the proposal encoding alone.
|
||||
|
||||
**Test:** existing tests are the test. If
|
||||
[FrostSigningRoundTest](../composeApp/src/commonTest/kotlin/press/mantra/compose/managers/FrostSigningRoundTest.kt)
|
||||
@@ -199,6 +213,9 @@ pass without edits beyond the schema move, the vectorisation is faithful.
|
||||
|
||||
### Payload encoding
|
||||
|
||||
*(Landed in Phase 2 — see above. Restated here because the rest of this phase
|
||||
depends on it.)*
|
||||
|
||||
`FrostSignerMessage` keeps its `(sessionId, signerPublicKey, kind)` primary key
|
||||
and **one row carries all `k` values**, comma-joined — the same encoding
|
||||
`FrostSigningSession.signerIds` already uses.
|
||||
|
||||
Reference in New Issue
Block a user