Skip to content

Simplify result verification: t+1 only for keygen - #175

Open
anhthii wants to merge 5 commits into
masterfrom
feat/result-signing-simplify
Open

anhthii wants to merge 5 commits into
masterfrom
feat/result-signing-simplify

Conversation

@anhthii

@anhthii anhthii commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Builds on #174. It keeps node-signed results and client-side verification, and narrows t+1 aggregation to keygen, the only result a client can't check on its own.

Result A lying node could send How the client catches it
Signing A bad signature Verify it against the wallet public key (client.VerifySigningResult)
Reshare A different public key Check the wallet public key is unchanged
Keygen A fake public key Can't be checked locally, so t+1 nodes must agree

Changes

Signing and reshare results are checked one message at a time (off / log / enforce) and delivered as soon as one valid copy arrives. No aggregator, timers or remembered outcomes.

New client.VerifySigningResult(keyType, pubKey, tx, result) verifies secp256k1 and ed25519 signing results against the wallet (or derived child) public key. It turns tx into the signed message the same way the nodes do.

Keygen keeps t+1. RequiredNodeSignatures is renamed to KeygenRequiredNodeSignatures, and a value above 1 requires enforce. RequiredNodeSignaturesForWallet and the reshare-committee threshold tracking are removed.

Per-node copies are requested by the client, not configured on nodes. result_signing.per_node_results is removed. When KeygenRequiredNodeSignatures > 1, CreateWallet sends Mpc-Keygen-Result-Copies: per-node. The keygen consumer forwards it, and the event consumer adds the node ID to the keygen result's idempotency key for that request only.

Why

  • No node config to coordinate. Under per_node_results, the setting had to match on every node and be turned on in a fixed order. Turning it on too early gave every client N callbacks per result, and turning it on too late made every operation time out. Now old clients don't send the header and keep getting one copy.

  • Fewer edge cases in the library. The shared aggregator for signing and reshare caused two problems:

    • a retry answered by a different node was dropped as a late copy;
    • a second OnSignResult replaced the first callback.

    Its remembered outcomes were also in-memory only, so they were lost on restart or failover.

  • Duplicate handling belongs in the application. JetStream delivers at least once, so apps need durable handling by TxID, wallet or session anyway.

  • Easier to port to mpcium-client-ts: the signing and reshare check is a stateless per-message function.

Compatibility and rollout

  • No node config change, and the result body is unchanged.
  • Deploy nodes before any client sets KeygenRequiredNodeSignatures > 1. Older nodes ignore the header and send one copy, so keygen would time out.
  • Keygen errors reported by a single node (for example cluster not ready) reach a t+1 client only after the 15s aggregation timeout, the same as before.

Testing

  • go test -race ./... passes.
  • New and updated unit tests:
    • per-message verification: valid copy, unsigned, tampered body, wrong subject, unknown node, impostor key;
    • keygen quorum: waits for t+1, acks only after delivery, drops late copies, a forged pubkey loses to two honest nodes, copies split across subscriptions are counted together;
    • CreateWallet sends the header only when t+1 is required;
    • VerifySigningResult for secp256k1 and ed25519, including a tx with a leading zero byte.
  • E2E suite not run yet.

vietddude and others added 3 commits November 5, 2026 14:42
Nodes sign the exact bytes they publish (domain || subject || body) with
their identity key and send Mpc-Node-Id and Mpc-Result-Sig headers. The
message body is unchanged, so existing clients keep working.

- pkg/resultauth: sign/verify, key lookup, t+1 aggregator, signing queue
- result_signing.per_node_results (default false) keeps one copy per node
- client: ResultVerifyMode off|log|enforce, NodePublicKeys,
  RequiredNodeSignatures(+ per-wallet override); copies are acked only
  after their result is delivered
- mpcium-cli export-node-keys builds node_public_keys.json from identity files
- examples, e2e (enforce, t+1), setup script and deployment guides updated
- Build the signing payload with append instead of a summed capacity (CodeQL)
- Write node_public_keys.json with 0600 (gosec)
- Split Aggregator.Add into roundFor and record to cut its complexity (Sonar)
Signing and reshare results are now checked per message (off/log/enforce)
and delivered immediately; callers verify them against keys they already
hold. Only keygen needs t+1, because a new wallet public key cannot be
checked locally.

- Remove result_signing.per_node_results; the client requests per-node
  keygen copies with the Mpc-Keygen-Result-Copies request header
- Rename RequiredNodeSignatures to KeygenRequiredNodeSignatures and drop
  RequiredNodeSignaturesForWallet and the reshare threshold tracking
- Add client.VerifySigningResult for secp256k1 and ed25519 results
- Update README, deployment guides, examples and e2e
@anhthii
anhthii requested a review from vietddude November 9, 2026 14:40
@vietddude vietddude changed the title Simplify result verification: t+1 only for keygen Sign MPC results in NATS headers; t+1 quorum for keygen only Nov 10, 2026
@vietddude
vietddude changed the base branch from feat/result-header-signing to master November 10, 2026 04:15
@vietddude vietddude changed the title Sign MPC results in NATS headers; t+1 quorum for keygen only Simplify result verification: t+1 only for keygen Nov 10, 2026
@vietddude
vietddude changed the base branch from master to feat/result-header-signing November 10, 2026 04:50
@vietddude

vietddude commented Nov 10, 2026 •

Copy link
Copy Markdown
Collaborator

E2E with fystack/apex#828, 3 local nodes from this branch, keygen t+1 = 3, enforce:

  • each node sent its own keygen copy and each wallet was created once;
  • Stellar withdrawals succeeded from one signed copy;
  • replayed, unsigned and forged results were dropped.

@sonarqubecloud

Copy link
Copy Markdown

@vietddude
vietddude changed the base branch from feat/result-header-signing to master November 10, 2026 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants