fix(mpc): realign KMS↔MPC ZAP signing wire to mpcd contract; kill false-green

The KMS ZapClient and the mpcd KMS-facing ZAP server (luxfi/mpc
pkg/api/zap_kms_server.go) had drifted out of lockstep despite the
"MUST stay in lockstep" comment — each was only unit-tested against its
own stub, never against the other's real bytes. Three drifts:

1. Sign request: KMS sent {key_type, wallet_id, message}; mpcd requires
   {vault_id, wallet_id, payload} and hard-fails "vault_id and wallet_id
   required". SignRequest now carries VaultID (Manager supplies it from
   MPC_VAULT_ID) and Payload (json "payload").
2. Keygen response: mpcd returns snake_case (wallet_id/ecdsa_pub_key/
   evm_address); KMS KeygenResult was camelCase → decoded to an empty
   struct. Tags realigned to snake_case (the ZAP path is the only live
   decode; the REST Client is unused).
3. False-green: ZapClient.call() json-decoded the daemon's {"error":...}
   frame into a zero-value SignResult and returned it with nil error —
   the KMS looked MPC-backed but returned an empty signature. call() now
   surfaces a non-empty top-level "error" as a real error (fail closed).

Adds pkg/mpc/wire_contract_test.go — a cross-repo contract test that
transcribes the mpcd request/response shapes and fails CI if the KMS
side drifts again (the guard that was missing). Existing tests updated
to the corrected snake_case wire.

This is a library fix in luxfi/kms — it fixes the wire for all three
orgs (Lux/Zoo/Hanzo) once. ed25519/Corona keygen over this wire remains
a follow-up (mpcd's KMS-ZAP keygen carries no key_type/protocol).

Co-authored-by: Hanzo Dev <dev@hanzo.ai>
This commit is contained in:
zeekay
2026-07-14 14:15:05 -07:00
co-authored by Hanzo Dev
parent 415097f535
commit 7377ad63d2
7 changed files with 193 additions and 35 deletions
+4 -2
View File
@@ -156,9 +156,10 @@ func (m *Manager) SignWithBLS(ctx context.Context, validatorID string, message [
}
result, err := m.signer.Sign(ctx, mpc.SignRequest{
VaultID: m.vaultID,
WalletID: ks.BLSWalletID,
KeyType: "secp256k1",
Message: message,
Payload: message,
})
if err != nil {
return nil, fmt.Errorf("keys: bls sign: %w", err)
@@ -179,9 +180,10 @@ func (m *Manager) SignWithCorona(ctx context.Context, validatorID string, messag
}
result, err := m.signer.Sign(ctx, mpc.SignRequest{
VaultID: m.vaultID,
WalletID: ks.CoronaWalletID,
KeyType: "ed25519",
Message: message,
Payload: message,
})
if err != nil {
return nil, fmt.Errorf("keys: corona sign: %w", err)
+11 -8
View File
@@ -101,15 +101,18 @@ func mockMPCServer(t *testing.T) *httptest.Server {
pub := "04pubkey" + string(rune('0'+keygenCount))
edpub := "edpub" + string(rune('0'+keygenCount))
w.WriteHeader(http.StatusCreated)
// snake_case keys match the mpcd wire (KeygenResult in
// luxfi/mpc pkg/api/server.go). camelCase here was the drift that
// silently decoded to an empty result.
json.NewEncoder(w).Encode(map[string]interface{}{
"id": "id-" + string(rune('0'+keygenCount)),
"walletId": "wallet-" + string(rune('0'+keygenCount)),
"vaultId": "vault-1",
"ecdsaPubkey": pub,
"eddsaPubkey": edpub,
"threshold": 3,
"participants": []string{"node0", "node1", "node2", "node3", "node4"},
"status": "active",
"id": "id-" + string(rune('0'+keygenCount)),
"wallet_id": "wallet-" + string(rune('0'+keygenCount)),
"vault_id": "vault-1",
"ecdsa_pub_key": pub,
"eddsa_pub_key": edpub,
"threshold": 3,
"participants": []string{"node0", "node1", "node2", "node3", "node4"},
"status": "active",
})
default:
w.WriteHeader(http.StatusNotFound)
+34 -18
View File
@@ -47,30 +47,45 @@ type KeygenRequest struct {
// downstream EVM L1s, Hanzo EVM, etc.). The derivation hashes the secp256k1
// pubkey with Keccak256 — that's HOW. The value IS "EVM-runtime
// account address" — that's WHAT.
// Wire tags are snake_case to match the mpcd ZAP keygen response
// (luxfi/mpc pkg/api/server.go KeygenResult: wallet_id / ecdsa_pub_key /
// eddsa_pub_key / evm_address / btc_address / sol_address). The ZAP path in
// zap_client.go is the ONLY live decode path for this struct; the REST Client
// is unused. Do NOT reintroduce camelCase — that silently decoded to an empty
// result (the KMS↔MPC wire drift). See mpc.wire_contract_test.go, which fails
// CI if this diverges from the mpcd contract again.
type KeygenResult struct {
ID string `json:"id"`
WalletID string `json:"walletId"`
VaultID string `json:"vaultId"`
Name *string `json:"name"`
KeyType string `json:"keyType"`
Protocol string `json:"protocol"`
ECDSAPubkey *string `json:"ecdsaPubkey"`
EDDSAPubkey *string `json:"eddsaPubkey"`
EVMAddress *string `json:"evmAddress,omitempty"`
BtcAddress *string `json:"btcAddress"`
SolAddress *string `json:"solAddress"`
Threshold int `json:"threshold"`
Participants []string `json:"participants"`
Version int `json:"version"`
Status string `json:"status"`
ID string `json:"id,omitempty"`
WalletID string `json:"wallet_id"`
VaultID string `json:"vault_id,omitempty"`
Name *string `json:"name,omitempty"`
KeyType string `json:"key_type,omitempty"`
Protocol string `json:"protocol,omitempty"`
ECDSAPubkey *string `json:"ecdsa_pub_key"`
EDDSAPubkey *string `json:"eddsa_pub_key"`
EVMAddress *string `json:"evm_address,omitempty"`
BtcAddress *string `json:"btc_address,omitempty"`
SolAddress *string `json:"sol_address,omitempty"`
Threshold int `json:"threshold,omitempty"`
Participants []string `json:"participants,omitempty"`
Version int `json:"version,omitempty"`
Status string `json:"status,omitempty"`
}
// SignRequest is the body sent to POST /v1/generate_mpc_sig or through
// the transaction flow. For validator signing we use the bridge sign endpoint.
// SignRequest is the KMS→MPC threshold-sign payload. Field names/tags match
// the mpcd ZAP sign handler (luxfi/mpc pkg/api/zap_kms_server.go
// kmsZapSignRequest{vault_id, wallet_id, payload}); mpcd requires vault_id AND
// wallet_id and reads the message from `payload`. VaultID is the MPC org/vault
// that owns the wallet — the KMS Manager supplies it from MPC_VAULT_ID.
// KeyType is advisory (mpcd resolves the curve from its key-info store); it is
// carried for the REST path and ignored by the ZAP server.
type SignRequest struct {
KeyType string `json:"key_type"`
VaultID string `json:"vault_id"`
WalletID string `json:"wallet_id"`
Message []byte `json:"message"`
KeyType string `json:"key_type,omitempty"`
Payload []byte `json:"payload"`
}
// SignResult is the response from a signing operation.
@@ -188,9 +203,10 @@ func (c *Client) Keygen(ctx context.Context, vaultID string, req KeygenRequest)
func (c *Client) Sign(ctx context.Context, req SignRequest) (*SignResult, error) {
url := fmt.Sprintf("%s/v1/transactions", c.BaseURL)
body, err := json.Marshal(map[string]interface{}{
"vault_id": req.VaultID,
"wallet_id": req.WalletID,
"key_type": req.KeyType,
"payload": req.Message,
"payload": req.Payload,
"type": "sign",
})
if err != nil {
+7 -5
View File
@@ -56,15 +56,17 @@ func TestKeygenResult_EVMAddress(t *testing.T) {
t.Fatal(err)
}
s := string(out)
if !strings.Contains(s, `"evmAddress":"`+addr+`"`) {
t.Fatalf("missing evmAddress: %s", s)
// snake_case matches the mpcd ZAP keygen response (evm_address). camelCase
// was the wire drift that silently decoded to empty — see wire_contract_test.go.
if !strings.Contains(s, `"evm_address":"`+addr+`"`) {
t.Fatalf("missing evm_address: %s", s)
}
if strings.Contains(s, "keccakAddress") || strings.Contains(s, "ethAddress") {
if strings.Contains(s, "keccakAddress") || strings.Contains(s, "ethAddress") || strings.Contains(s, "evmAddress") {
t.Fatalf("legacy alias leaked: %s", s)
}
// Unmarshal: evmAddress populates EVMAddress.
src := `{"id":"k2","evmAddress":"` + addr + `"}`
// Unmarshal: evm_address populates EVMAddress.
src := `{"id":"k2","evm_address":"` + addr + `"}`
var k2 KeygenResult
if err := json.Unmarshal([]byte(src), &k2); err != nil {
t.Fatal(err)
+1 -1
View File
@@ -87,7 +87,7 @@ func TestSign(t *testing.T) {
defer srv.Close()
c, _ := NewClient(srv.URL, "test-token")
result, err := c.Sign(context.Background(), SignRequest{WalletID: "wallet-1", KeyType: "secp256k1", Message: []byte("hello")})
result, err := c.Sign(context.Background(), SignRequest{VaultID: "vault-1", WalletID: "wallet-1", KeyType: "secp256k1", Payload: []byte("hello")})
if err != nil {
t.Fatalf("sign: %v", err)
}
+111
View File
@@ -0,0 +1,111 @@
package mpc
// wire_contract_test.go — pins the KMS↔MPC ZAP wire to the mpcd server
// contract so the two hand-maintained copies of the wire can't silently drift
// again. The shapes below are transcribed from luxfi/mpc
// pkg/api/zap_kms_server.go (kmsZapSignRequest, kmsZapKeygenRequest) and
// pkg/api/server.go (KeygenResult) as of mpc v1.17.9. If the KMS-side structs
// drift from these, these tests fail in CI — which is the guard that was
// missing when Sign silently sent {key_type,wallet_id,message} while the
// daemon required {vault_id,wallet_id,payload}.
import (
"encoding/json"
"testing"
)
// mpcdSignRequest mirrors luxfi/mpc pkg/api/zap_kms_server.go kmsZapSignRequest.
// The daemon rejects the request unless vault_id AND wallet_id are non-empty
// and reads the message bytes from `payload`.
type mpcdSignRequest struct {
VaultID string `json:"vault_id"`
WalletID string `json:"wallet_id"`
Payload []byte `json:"payload"`
}
// mpcdKeygenResult mirrors luxfi/mpc pkg/api/server.go KeygenResult (the body
// the daemon marshals back for OpKMSKeygen).
type mpcdKeygenResult struct {
WalletID string `json:"wallet_id"`
ECDSAPubKey string `json:"ecdsa_pub_key"`
EDDSAPubKey string `json:"eddsa_pub_key"`
EVMAddress string `json:"evm_address,omitempty"`
}
func TestSignRequest_SatisfiesMPCDContract(t *testing.T) {
// A KMS SignRequest, marshaled exactly as ZapClient.Sign sends it, must
// deserialize into the daemon's request shape with all required fields set.
req := SignRequest{
VaultID: "zoo",
WalletID: "zoo-treasury-v1",
KeyType: "secp256k1",
Payload: []byte("message-to-sign"),
}
wire, err := json.Marshal(req)
if err != nil {
t.Fatalf("marshal SignRequest: %v", err)
}
var got mpcdSignRequest
if err := json.Unmarshal(wire, &got); err != nil {
t.Fatalf("daemon cannot decode SignRequest: %v", err)
}
if got.VaultID == "" {
t.Fatalf("vault_id empty on the wire — daemon rejects with %q", "vault_id and wallet_id required")
}
if got.WalletID == "" {
t.Fatalf("wallet_id empty on the wire — daemon rejects")
}
if len(got.Payload) == 0 {
t.Fatalf("payload empty on the wire — daemon rejects with %q (regression: message vs payload)", "payload required")
}
if string(got.Payload) != "message-to-sign" {
t.Fatalf("payload round-trip mismatch: %q", string(got.Payload))
}
}
func TestKeygenResult_DecodesMPCDResponse(t *testing.T) {
// The daemon's snake_case keygen response must populate the KMS struct.
// camelCase tags (the drift) silently produced an all-empty result.
daemon := mpcdKeygenResult{
WalletID: "zoo-treasury-v1",
ECDSAPubKey: "502a9a8e4b5a4869f5a290bd087fd3b87ae3866948dc53ba7fe224d940c96776",
EDDSAPubKey: "",
EVMAddress: "0x8756621734fe274fdc426381c9a4f9dec8656243",
}
wire, err := json.Marshal(daemon)
if err != nil {
t.Fatalf("marshal daemon result: %v", err)
}
var got KeygenResult
if err := json.Unmarshal(wire, &got); err != nil {
t.Fatalf("KMS cannot decode daemon keygen response: %v", err)
}
if got.WalletID != daemon.WalletID {
t.Fatalf("wallet_id not decoded (camelCase drift?): got %q", got.WalletID)
}
if got.ECDSAPubkey == nil || *got.ECDSAPubkey != daemon.ECDSAPubKey {
t.Fatalf("ecdsa_pub_key not decoded (camelCase drift?): got %v", got.ECDSAPubkey)
}
if got.EVMAddress == nil || *got.EVMAddress != daemon.EVMAddress {
t.Fatalf("evm_address not decoded: got %v", got.EVMAddress)
}
}
func TestZapErrorString_SurfacesDaemonError(t *testing.T) {
// The daemon's error frame must become a real error, never a silent empty
// result. This is the structural guard against the false-green.
if msg := zapErrorString([]byte(`{"error":"vault_id and wallet_id required"}`)); msg != "vault_id and wallet_id required" {
t.Fatalf("daemon error not surfaced: got %q", msg)
}
// A valid SignResult carries no "error" field → must NOT be flagged.
okBody, _ := json.Marshal(SignResult{R: "ab", S: "cd", Signature: "ef"})
if msg := zapErrorString(okBody); msg != "" {
t.Fatalf("valid SignResult misflagged as error: %q", msg)
}
// Empty object is not an error.
if msg := zapErrorString([]byte(`{}`)); msg != "" {
t.Fatalf("empty object misflagged as error: %q", msg)
}
}
+25 -1
View File
@@ -138,7 +138,31 @@ func (c *ZapClient) call(ctx context.Context, op uint16, payload any) ([]byte, e
if len(body) <= zap.HeaderSize+2 {
return []byte("{}"), nil
}
return body[zap.HeaderSize+2:], nil // skip header + opcode
respBody := body[zap.HeaderSize+2:] // skip header + opcode
// Surface server-side errors. mpcd frames failures as {"error":"..."}
// under the SAME opcode as the request (pkg/api/zap_kms_server.go errBody).
// Without this, the caller would json-decode the error object into a
// zero-value SignResult/KeygenResult and return success with empty fields —
// the "false-green" empty-signature footgun. Fail closed instead.
if msg := zapErrorString(respBody); msg != "" {
return nil, fmt.Errorf("mpc: op=0x%04x rejected by daemon: %s", op, msg)
}
return respBody, nil
}
// zapErrorString returns the top-level "error" string if body is a JSON object
// carrying a non-empty one, else "". None of the success payloads
// (SignResult/KeygenResult/ClusterStatus/Wallet) define an "error" field, so a
// present, non-empty "error" is unambiguously a daemon-side failure.
func zapErrorString(body []byte) string {
var probe struct {
Error string `json:"error"`
}
if err := json.Unmarshal(body, &probe); err != nil {
return "" // not a JSON object (or not decodable) → treat as payload
}
return probe.Error
}
// Keygen creates a new MPC wallet.