mirror of
https://github.com/luxfi/mpc.git
synced 2026-09-24 00:26:24 +00:00
fix(mpc): /healthz reports quorum — not all-peers-ready (P0-1)
Survivor nodes in a t-of-n ensemble now report healthy so long as they still have enough ready peers (including self) to form a signing quorum for their configured threshold. Previously, ArePeersReady() was the gate and any single peer loss collapsed availability on the remaining nodes. Repro (Scientist, 2026-04-20): scale liquid-mpc-mpc-node StatefulSet from 3 → 2 replicas in the liquidity devnet; surviving pods returned HTTP 503 with log "Peers not ready expected=3 ready=2" even though 2 nodes are sufficient for the 2-of-3 (threshold=1) signing quorum. Fix: - pkg/transport/registry.go: add HasSigningQuorum(threshold) which returns true when readyCount >= threshold+1 (the CGGMP21/FROST signers-required count used everywhere in pkg/mpc/node.go). - pkg/mpc/registry.go: matching method on the legacy Consul-backed registry plus interface addition to PeerRegistry. - cmd/mpcd/main.go: /healthz handler switched to HasSigningQuorum(threshold) for the 200/503 gate. ArePeersReady() is still reported in the JSON body (as `ready`) for observability. `status` becomes "healthy" / "healthy-reduced" / "degraded" so operators can see the in-between state. - cmd/mpcd/main.go: ConsensusPeerRegistry wrapper delegates the new method to the underlying transport registry. Keygen sessions deliberately continue to require ArePeersReady() — a fresh wallet still needs every participant present. Only the /healthz gate is relaxed, which matches the availability story for signing. Tests: pkg/transport/registry_test.go covers - Test_HasSigningQuorum_3Node_2of3 (the Scientist repro — survivors still quorum-ok after one peer loss) - Test_HasSigningQuorum_5Node_3of5 (larger ensemble, f=2 tolerance) - Test_HasSigningQuorum_ArePeersReady_Decoupled (the two APIs do not gate each other) - Test_HasSigningQuorum_BoundaryCheck (pins arithmetic: required = threshold+1, not threshold) All 11 test cases pass; transport + mpc package suites clean.
This commit is contained in:
+39
-3
@@ -471,15 +471,42 @@ func runNodeConsensus(ctx context.Context, c *cli.Command) error {
|
||||
mux := http.NewServeMux()
|
||||
// Health probe handler — unauthenticated (K8s liveness/readiness probes).
|
||||
// Served on both /health (legacy) and /healthz (platform standard).
|
||||
//
|
||||
// Red finding P0-1 (2026-04-20): the previous implementation returned
|
||||
// 503 whenever ArePeersReady() was false — i.e. whenever ANY peer was
|
||||
// unreachable. For a 3-node 2-of-3 ensemble that collapsed availability
|
||||
// the moment one peer restarted, because the surviving two nodes
|
||||
// reported themselves "unhealthy" even though they could still form a
|
||||
// 2-of-3 signing quorum between them.
|
||||
//
|
||||
// Fixed behaviour: the node is healthy when it has enough ready peers
|
||||
// (including self) to participate in a signing round for the
|
||||
// configured CGGMP21/FROST threshold — i.e. `readyCount >= threshold+1`.
|
||||
//
|
||||
// Keygen of fresh wallets still requires all peers (checked separately
|
||||
// on the /keygen endpoint via peerRegistry.ArePeersReady()). Signing
|
||||
// with an existing key only needs threshold+1 parties, which is what
|
||||
// /healthz now reports.
|
||||
healthHandler := func(w http.ResponseWriter, r *http.Request) {
|
||||
ready := peerRegistry.ArePeersReady()
|
||||
// allReady: every expected peer connected. Used for observability.
|
||||
allReady := peerRegistry.ArePeersReady()
|
||||
// quorum: enough ready peers for signing. Gates 200 vs 503.
|
||||
quorum := peerRegistry.HasSigningQuorum(threshold)
|
||||
readyCount := peerRegistry.GetReadyPeersCount()
|
||||
connected := factory.Transport().GetPeers()
|
||||
|
||||
status := "healthy"
|
||||
httpCode := http.StatusOK
|
||||
if !ready {
|
||||
switch {
|
||||
case !quorum:
|
||||
status = "degraded"
|
||||
httpCode = http.StatusServiceUnavailable
|
||||
case !allReady:
|
||||
// Signing quorum holds, but we're missing non-critical peers.
|
||||
// Stay 200 — the node is usable — but report the nuance.
|
||||
status = "healthy-reduced"
|
||||
}
|
||||
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(httpCode)
|
||||
resp := map[string]interface{}{
|
||||
@@ -488,8 +515,11 @@ func runNodeConsensus(ctx context.Context, c *cli.Command) error {
|
||||
"mode": "consensus",
|
||||
"expected_peers": len(peerIDs),
|
||||
"connected_peers": len(connected),
|
||||
"ready": ready,
|
||||
"ready": allReady,
|
||||
"signing_quorum": quorum,
|
||||
"ready_count": readyCount,
|
||||
"threshold": threshold,
|
||||
"required_signers": threshold + 1,
|
||||
"version": Version,
|
||||
}
|
||||
json.NewEncoder(w).Encode(resp)
|
||||
@@ -1146,6 +1176,12 @@ func (r *ConsensusPeerRegistry) ArePeersReady() bool {
|
||||
return r.registry.ArePeersReady()
|
||||
}
|
||||
|
||||
// HasSigningQuorum delegates to the underlying transport registry. See
|
||||
// pkg/transport/registry.go for the rationale (ready peer count >= threshold+1).
|
||||
func (r *ConsensusPeerRegistry) HasSigningQuorum(threshold int) bool {
|
||||
return r.registry.HasSigningQuorum(threshold)
|
||||
}
|
||||
|
||||
func (r *ConsensusPeerRegistry) GetReadyPeersCount() int64 {
|
||||
return r.registry.GetReadyPeersCount()
|
||||
}
|
||||
|
||||
@@ -19,7 +19,16 @@ const (
|
||||
|
||||
type PeerRegistry interface {
|
||||
Ready() error
|
||||
// ArePeersReady returns true only when ALL expected peers are ready.
|
||||
// Required for keygen (fresh wallets need every peer's contribution).
|
||||
ArePeersReady() bool
|
||||
// HasSigningQuorum returns true when ready peers (including self) are
|
||||
// sufficient to participate in a signing round for a key with the given
|
||||
// polynomial-degree threshold. Stricter than "any peer reachable",
|
||||
// weaker than ArePeersReady — survives partial node loss up to f = n-t-1
|
||||
// failures without marking the node unhealthy. See pkg/transport/registry.go
|
||||
// for the full rationale.
|
||||
HasSigningQuorum(threshold int) bool
|
||||
WatchPeersReady()
|
||||
// Resign is called by the node when it is going to shutdown
|
||||
Resign() error
|
||||
@@ -198,6 +207,14 @@ func (r *registry) ArePeersReady() bool {
|
||||
return r.ready
|
||||
}
|
||||
|
||||
// HasSigningQuorum: see PeerRegistry interface comment. For the Consul-backed
|
||||
// legacy registry the readyCount is maintained the same way as the consensus
|
||||
// registry, so the check is identical.
|
||||
func (r *registry) HasSigningQuorum(threshold int) bool {
|
||||
required := int64(threshold + 1)
|
||||
return atomic.LoadInt64(&r.readyCount) >= required
|
||||
}
|
||||
|
||||
func (r *registry) GetTotalPeersCount() int64 {
|
||||
var self int64 = 1
|
||||
return int64(len(r.peerNodeIDs)) + self
|
||||
|
||||
@@ -223,11 +223,34 @@ func (r *Registry) logReadyStatus() {
|
||||
}
|
||||
}
|
||||
|
||||
// ArePeersReady returns true if all peers are ready
|
||||
// ArePeersReady returns true if ALL peers are ready. Used by keygen sessions,
|
||||
// which genuinely need every participant present to generate a fresh key.
|
||||
func (r *Registry) ArePeersReady() bool {
|
||||
return r.ready.Load()
|
||||
}
|
||||
|
||||
// HasSigningQuorum returns true when this node has enough ready peers
|
||||
// (including itself) to participate in a threshold signing round.
|
||||
//
|
||||
// The CGGMP21 / FROST / LSS / SR25519 / BLS signing APIs require
|
||||
// `len(signerPeerIDs) >= keyInfo.Threshold + 1` — i.e. threshold+1 parties
|
||||
// (where threshold is the degree of the polynomial). A healthy node therefore
|
||||
// needs at least `threshold + 1` peers ready (including self) to contribute
|
||||
// to any signing round.
|
||||
//
|
||||
// This is stricter than "can reach >= 1 peer" but more permissive than
|
||||
// ArePeersReady. For a 3-node ensemble with CGGMP21 `threshold=1` (2-of-3
|
||||
// signing), losing 1 peer leaves 2 ready → signing quorum holds. Losing 2
|
||||
// peers leaves 1 ready → no quorum, health reports degraded.
|
||||
//
|
||||
// Callers should pass the CGGMP21/FROST polynomial-degree threshold, which is
|
||||
// what the KeyInfo.Threshold field stores. The "signers required" count is
|
||||
// threshold+1.
|
||||
func (r *Registry) HasSigningQuorum(threshold int) bool {
|
||||
required := int64(threshold + 1)
|
||||
return atomic.LoadInt64(&r.readyCount) >= required
|
||||
}
|
||||
|
||||
// GetReadyPeersCount returns number of ready peers
|
||||
func (r *Registry) GetReadyPeersCount() int64 {
|
||||
return atomic.LoadInt64(&r.readyCount)
|
||||
|
||||
@@ -0,0 +1,197 @@
|
||||
// Unit tests for Registry quorum semantics — P0-1 from the 2026-04-20
|
||||
// Red/Scientist evidence report.
|
||||
//
|
||||
// Repro: Scientist scaled liquid-mpc from 3 → 2 replicas. Surviving nodes
|
||||
// returned HTTP 503 because ArePeersReady() reported false (not all peers
|
||||
// connected), even though 2 nodes were sufficient to form a 2-of-3 signing
|
||||
// quorum. The fix added HasSigningQuorum(threshold) which is true when
|
||||
// readyCount >= threshold+1 (CGGMP21/FROST signers-required count).
|
||||
package transport
|
||||
|
||||
import (
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// newTestRegistry constructs a Registry with the given node and peer IDs
|
||||
// without wiring any transport/consensus plumbing. readyCount starts at 1
|
||||
// (self).
|
||||
func newTestRegistry(nodeID string, peerIDs []string) *Registry {
|
||||
r := &Registry{
|
||||
nodeID: nodeID,
|
||||
peerNodeIDs: filterSelf(nodeID, peerIDs),
|
||||
readyMap: make(map[string]bool),
|
||||
readyCount: 1, // self
|
||||
}
|
||||
return r
|
||||
}
|
||||
|
||||
// markPeerReady simulates a peer-ready signal without invoking the
|
||||
// transport layer. Mirrors what checkPeerConnections() does atomically.
|
||||
func markPeerReady(r *Registry, peerID string) {
|
||||
r.readyMu.Lock()
|
||||
defer r.readyMu.Unlock()
|
||||
if !r.readyMap[peerID] {
|
||||
r.readyMap[peerID] = true
|
||||
atomic.AddInt64(&r.readyCount, 1)
|
||||
}
|
||||
}
|
||||
|
||||
func markPeerDisconnected(r *Registry, peerID string) {
|
||||
r.readyMu.Lock()
|
||||
defer r.readyMu.Unlock()
|
||||
if r.readyMap[peerID] {
|
||||
r.readyMap[peerID] = false
|
||||
atomic.AddInt64(&r.readyCount, -1)
|
||||
}
|
||||
}
|
||||
|
||||
// Test_HasSigningQuorum_3Node_2of3 — the Scientist repro.
|
||||
// 3 nodes total, threshold=1 (i.e. 2-of-3: need 2 signers = threshold+1).
|
||||
// Kill 1 peer → 2 ready (self + 1). Must still report quorum.
|
||||
func Test_HasSigningQuorum_3Node_2of3(t *testing.T) {
|
||||
r := newTestRegistry("node0", []string{"node0", "node1", "node2"})
|
||||
threshold := 1 // "2-of-3" in CGGMP21 semantics (signers = threshold+1 = 2)
|
||||
|
||||
// Bootstrap: only self ready.
|
||||
if r.HasSigningQuorum(threshold) {
|
||||
t.Fatal("alone: 1 peer ready, threshold=1, required=2 — expected no quorum")
|
||||
}
|
||||
|
||||
// Peer 1 joins → 2 ready. Should form quorum.
|
||||
markPeerReady(r, "node1")
|
||||
if !r.HasSigningQuorum(threshold) {
|
||||
t.Fatalf("2 ready, threshold=1, required=2 — expected quorum, got none. readyCount=%d",
|
||||
r.GetReadyPeersCount())
|
||||
}
|
||||
|
||||
// Peer 2 joins → 3 ready. Still quorum (no regression).
|
||||
markPeerReady(r, "node2")
|
||||
if !r.HasSigningQuorum(threshold) {
|
||||
t.Fatalf("3 ready, threshold=1 — expected quorum")
|
||||
}
|
||||
|
||||
// Peer 2 disconnects → back to 2 ready. This is the Scientist repro:
|
||||
// survivors must still report quorum.
|
||||
markPeerDisconnected(r, "node2")
|
||||
if got := r.GetReadyPeersCount(); got != 2 {
|
||||
t.Fatalf("after peer disconnect expected readyCount=2, got %d", got)
|
||||
}
|
||||
if !r.HasSigningQuorum(threshold) {
|
||||
t.Fatal("REGRESSION (Scientist P0-1): 2 ready, threshold=1 — quorum lost on single peer loss")
|
||||
}
|
||||
|
||||
// Peer 1 also disconnects → 1 ready. Now we genuinely have no quorum.
|
||||
markPeerDisconnected(r, "node1")
|
||||
if r.HasSigningQuorum(threshold) {
|
||||
t.Fatal("1 ready alone, threshold=1, required=2 — expected no quorum")
|
||||
}
|
||||
}
|
||||
|
||||
// Test_HasSigningQuorum_5Node_3of5 — larger ensemble.
|
||||
// 5 nodes, threshold=2 (3-of-5: signers = threshold+1 = 3).
|
||||
func Test_HasSigningQuorum_5Node_3of5(t *testing.T) {
|
||||
r := newTestRegistry("node0", []string{"node0", "node1", "node2", "node3", "node4"})
|
||||
threshold := 2
|
||||
|
||||
// 1 ready (self only) — no quorum.
|
||||
if r.HasSigningQuorum(threshold) {
|
||||
t.Fatal("1 ready alone — expected no quorum")
|
||||
}
|
||||
|
||||
// 2 ready — still below required=3.
|
||||
markPeerReady(r, "node1")
|
||||
if r.HasSigningQuorum(threshold) {
|
||||
t.Fatal("2 ready — expected no quorum (required=3)")
|
||||
}
|
||||
|
||||
// 3 ready — exactly at required. Quorum.
|
||||
markPeerReady(r, "node2")
|
||||
if !r.HasSigningQuorum(threshold) {
|
||||
t.Fatalf("3 ready, threshold=2, required=3 — expected quorum, got none. readyCount=%d",
|
||||
r.GetReadyPeersCount())
|
||||
}
|
||||
|
||||
// 4 ready — above required. Quorum.
|
||||
markPeerReady(r, "node3")
|
||||
if !r.HasSigningQuorum(threshold) {
|
||||
t.Fatal("4 ready, threshold=2 — expected quorum")
|
||||
}
|
||||
|
||||
// 5 ready — all ready. Quorum AND ArePeersReady would be true if transport set it.
|
||||
markPeerReady(r, "node4")
|
||||
if !r.HasSigningQuorum(threshold) {
|
||||
t.Fatal("5 ready, threshold=2 — expected quorum")
|
||||
}
|
||||
|
||||
// Two simultaneous failures → 3 ready. Still quorum (no regression).
|
||||
markPeerDisconnected(r, "node3")
|
||||
markPeerDisconnected(r, "node4")
|
||||
if got := r.GetReadyPeersCount(); got != 3 {
|
||||
t.Fatalf("after 2 disconnects expected readyCount=3, got %d", got)
|
||||
}
|
||||
if !r.HasSigningQuorum(threshold) {
|
||||
t.Fatal("3 ready after 2 failures, threshold=2 — expected quorum")
|
||||
}
|
||||
|
||||
// Third failure → 2 ready. Quorum lost (correctly).
|
||||
markPeerDisconnected(r, "node2")
|
||||
if r.HasSigningQuorum(threshold) {
|
||||
t.Fatal("2 ready, threshold=2, required=3 — expected NO quorum (f-1 tolerance exceeded)")
|
||||
}
|
||||
}
|
||||
|
||||
// Test_HasSigningQuorum_ArePeersReady_Decoupled proves the two APIs do
|
||||
// NOT gate each other. ArePeersReady remains strict (all peers required);
|
||||
// HasSigningQuorum is the relaxed check used by /healthz.
|
||||
func Test_HasSigningQuorum_ArePeersReady_Decoupled(t *testing.T) {
|
||||
r := newTestRegistry("node0", []string{"node0", "node1", "node2"})
|
||||
threshold := 1
|
||||
|
||||
// Two peers ready, one missing. HasSigningQuorum=true, ArePeersReady=false.
|
||||
markPeerReady(r, "node1")
|
||||
|
||||
if !r.HasSigningQuorum(threshold) {
|
||||
t.Fatal("expected quorum with 2 of 3 ready")
|
||||
}
|
||||
|
||||
// ArePeersReady derives from r.ready which is set via checkPeerConnections;
|
||||
// we didn't call that, so it stays false. That's the point — these two
|
||||
// booleans are deliberately decoupled.
|
||||
if r.ArePeersReady() {
|
||||
t.Fatal("ArePeersReady must stay false when not all peers connected")
|
||||
}
|
||||
}
|
||||
|
||||
// Test_HasSigningQuorum_BoundaryCheck covers the exact threshold arithmetic
|
||||
// to pin the semantics: required = threshold + 1, NOT threshold.
|
||||
func Test_HasSigningQuorum_BoundaryCheck(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
readyCount int64
|
||||
threshold int
|
||||
wantQuorum bool
|
||||
}{
|
||||
{"threshold=0, ready=0 → no quorum (need >=1)", 0, 0, false},
|
||||
{"threshold=0, ready=1 → quorum (single-party sign)", 1, 0, true},
|
||||
{"threshold=1, ready=1 → no quorum (need >=2)", 1, 1, false},
|
||||
{"threshold=1, ready=2 → quorum", 2, 1, true},
|
||||
{"threshold=2, ready=2 → no quorum (need >=3)", 2, 2, false},
|
||||
{"threshold=2, ready=3 → quorum", 3, 2, true},
|
||||
{"threshold=2, ready=4 → quorum", 4, 2, true},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
r := &Registry{
|
||||
readyCount: tc.readyCount,
|
||||
readyMap: make(map[string]bool),
|
||||
}
|
||||
got := r.HasSigningQuorum(tc.threshold)
|
||||
if got != tc.wantQuorum {
|
||||
t.Fatalf("readyCount=%d threshold=%d → HasSigningQuorum=%v, want %v",
|
||||
tc.readyCount, tc.threshold, got, tc.wantQuorum)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user