From b72f9edb805caca7f64041a012ff66d063293ac5 Mon Sep 17 00:00:00 2001 From: zeekay Date: Thu, 25 Jun 2026 22:10:09 -0700 Subject: [PATCH] message: prove the op table is a bijection onto the router op space MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kills the bug CLASS behind the α-of-K finality wedge (v1.30.65), not just the one Gossip instance. The chain router (node/chain_router.go) forwards an inbound message only if message.ToConsensusOp maps it, then dispatches on the router op; the node op table and the consensus router op enum are two halves of ONE routing contract. When they diverged — router.Gossip existed but no node op mapped to it — every broadcast vote was dropped as "unhandled message op" and finality wedged, with nothing to catch it. - ToConsensusOp now returns the router constants (byte(router.Get) ...) instead of 0..11 magic numbers, so values cannot drift and the table reads as the correspondence it is. Bumps consensus v1.25.29 -> v1.25.30 (adds router.NumOps, the single source of truth for the op count). - message/ops_test.go TestToConsensusOp_TableAlignedWithRouter is now EXHAUSTIVE and BIDIRECTIONAL: it reconstructs the inverse mapping over the whole node op space and requires a bijection onto [0, router.NumOps). A router op with no node preimage (the original bug), a collision, a wrong target, or a stray mapping all go RED. Pinned to router.NumOps, so a future op added to one table but not the other fails the test instead of at runtime. Proven RED against the original bug (drop the GossipOp case -> router.Gossip loses its preimage -> fail); folds in the old Gossip-specific and hand-listed table checks. Build rc=0 (GOWORK=off go build ./...); message tests green; finality fix (GossipOp -> router.Gossip -> blockHandler.Gossip) unchanged and still covered. --- go.mod | 3 +- go.sum | 4 ++ message/ops.go | 42 ++++++++--------- message/ops_test.go | 112 ++++++++++++++++++++++++++------------------ 4 files changed, 93 insertions(+), 68 deletions(-) diff --git a/go.mod b/go.mod index 3fcd4c858..8b2a0fd71 100644 --- a/go.mod +++ b/go.mod @@ -26,7 +26,7 @@ require ( github.com/huin/goupnp v1.3.0 github.com/jackpal/gateway v1.1.1 github.com/jackpal/go-nat-pmp v1.0.2 - github.com/luxfi/consensus v1.25.29 + github.com/luxfi/consensus v1.25.30 github.com/luxfi/crypto v1.19.26 github.com/luxfi/database v1.20.4 github.com/luxfi/ids v1.2.15 @@ -188,6 +188,7 @@ require ( github.com/hanzos3/go-sdk v1.0.2 // indirect github.com/klauspost/crc32 v1.3.0 // indirect github.com/luxfi/age v1.5.0 // indirect + github.com/luxfi/bft v0.1.5 // indirect github.com/luxfi/corona v0.7.9 // indirect github.com/luxfi/crypto/ipa v1.2.4 // indirect github.com/luxfi/kms v1.11.7 // indirect diff --git a/go.sum b/go.sum index 8c31b771c..5723c3c07 100644 --- a/go.sum +++ b/go.sum @@ -300,6 +300,8 @@ github.com/luxfi/api v1.0.15 h1:Q5ox3Ompw/AZNMfB9wpHZosrj9C+ZldAEHKtHEotFdY= github.com/luxfi/api v1.0.15/go.mod h1:Eer59msIXMnOlFncG0XjEGH3TZML0Dd1bUu4GtB7f4Q= github.com/luxfi/atomic v1.0.0 h1:xUV60MuzRvXngaQ1sM0yVC2v4TRoLlUGkkH7M9PS4yw= github.com/luxfi/atomic v1.0.0/go.mod h1:0G2mTlQ6TXWHICUHrUUPu1/qAiIyR4gSZ2tva9ci/bI= +github.com/luxfi/bft v0.1.5 h1:5xVLPkog4e5LTgaVlb9pgxA0EWE6tkrKwHPZVRz+RZw= +github.com/luxfi/bft v0.1.5/go.mod h1:5I8Ft8yA69xZlDe3RB0i4MgbqFKLZe65o/sha8JuKvU= github.com/luxfi/cache v1.2.1 h1:kAzOS55/hmYeNKR+0HAKv4ma48Y6JjkI8UQeqdZ8bfI= github.com/luxfi/cache v1.2.1/go.mod h1:co7JTxZZHpKT31Yh01LFp5aZOxmoUg157FhBLQdQHVU= github.com/luxfi/chains v1.3.21 h1:Oe3/T0MnMKYBVzd+wK13R5qzRGQEMytysbs3PkCLWNc= @@ -312,6 +314,8 @@ github.com/luxfi/concurrent v0.0.3 h1:eJyv1fhaC0jMLMw6+QS774cUmp7GK+ouMgvLCqnC7c github.com/luxfi/concurrent v0.0.3/go.mod h1:Aj/FR5NpM0cB2P4Nt3+tz9+dV6V+LUW4HuMgSjwq5hw= github.com/luxfi/consensus v1.25.29 h1:z9UWFnuBY7XtTJ6klA7u8UV8ZV5JjHqh+jfKqcRcUPU= github.com/luxfi/consensus v1.25.29/go.mod h1:cerfisfzmUJv8gbMcjcQUSdxlLpfn/+LdLY+4D95Nw0= +github.com/luxfi/consensus v1.25.30 h1:rxNFKKTJ7hMRGbePBi+XX51Nr1VUaeSTLIbSP3qdMac= +github.com/luxfi/consensus v1.25.30/go.mod h1:cerfisfzmUJv8gbMcjcQUSdxlLpfn/+LdLY+4D95Nw0= github.com/luxfi/constants v1.5.8 h1:iNP9AWNUcM4Tps7jYnx49CwtCWAC9mYRxJfGou2za0g= github.com/luxfi/constants v1.5.8/go.mod h1:Pu5jWHdnUtQRbWC43yTUjU/pbIIKMDOd2a2yroSfo48= github.com/luxfi/container v0.0.4 h1:BXhF82WyfqVP5mjlNcr7tP0Fcnvl0Ap1rkiu+rq5XuM= diff --git a/message/ops.go b/message/ops.go index aa2a586c9..9331a8160 100644 --- a/message/ops.go +++ b/message/ops.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" + "github.com/luxfi/consensus/core/router" "github.com/luxfi/math/set" "github.com/luxfi/node/proto/p2p" ) @@ -246,41 +247,40 @@ func Unwrap(m *p2p.Message) (fmt.Stringer, error) { } } -// ToConsensusOp maps message.Op to consensus router Op values -// Returns the consensus Op value and whether the mapping exists +// ToConsensusOp maps a wire message Op to the consensus router op the chain +// router (node/chain_router.go) dispatches on, returning false for ops that are +// not consensus-routed (the router drops those as "unhandled message op"). The +// returned values ARE the router constants, so they cannot drift in value; the +// mapping must also stay a bijection onto the router op space [0, router.NumOps) +// — proven by message/ops_test.go. A missing case silently drops that op: that +// is the α-of-K finality wedge, where GossipOp had no case and every vote +// vanished before reaching blockHandler.Gossip -> engine.HandleIncomingVote. func ToConsensusOp(op Op) (byte, bool) { switch op { case GetAcceptedFrontierOp: - return 0, true // GetAcceptedFrontier + return byte(router.GetAcceptedFrontier), true case AcceptedFrontierOp: - return 1, true // AcceptedFrontier + return byte(router.AcceptedFrontier), true case GetAcceptedOp: - return 2, true // GetAccepted + return byte(router.GetAccepted), true case AcceptedOp: - return 3, true // Accepted + return byte(router.Accepted), true case GetOp: - return 4, true // Get + return byte(router.Get), true case PutOp: - return 5, true // Put + return byte(router.Put), true case PushQueryOp: - return 6, true // PushQuery + return byte(router.PushQuery), true case PullQueryOp: - return 7, true // PullQuery + return byte(router.PullQuery), true case QbitOp: - return 8, true // Qbit + return byte(router.Vote), true // votes ride the Qbit wire op case GetAncestorsOp: - return 9, true // GetContext (wire protocol still uses GetAncestors) + return byte(router.GetContext), true // wire op is still GetAncestors case AncestorsOp: - return 10, true // Context (wire protocol still uses Ancestors) + return byte(router.Context), true // wire op is still Ancestors case GossipOp: - // Gossip (= router.Gossip). The α-of-K quorum vote/cert transport rides on - // app-gossip. Without this mapping the chain router drops every inbound vote - // as "unhandled message op" before it reaches blockHandler.Gossip -> - // engine.HandleIncomingVote, so the finality cert never assembles and the - // chain wedges (blocks verify but never accept). Routed to blockHandler's - // Gossip method, which demuxes the quorum envelope. MUST match the value the - // consensus side decodes it as (TestToConsensusOp_TableAlignedWithRouter). - return 11, true // Gossip + return byte(router.Gossip), true // α-of-K vote/cert transport default: return 0, false } diff --git a/message/ops_test.go b/message/ops_test.go index f926a2204..884113abe 100644 --- a/message/ops_test.go +++ b/message/ops_test.go @@ -12,59 +12,79 @@ import ( "github.com/luxfi/ids" ) -// TestToConsensusOp_GossipIsRouted is the regression guard for the finality -// wedge: the α-of-K quorum vote/cert transport rides on app-gossip (GossipOp). -// The chain router (node/chain_router.go) gates EVERY inbound message through -// ToConsensusOp; an op that does not map is dropped as "unhandled message op" -// BEFORE it can reach the engine. GossipOp was missing from the table, so every -// broadcast vote was silently dropped at the router — blocks (PutOp) propagated -// and verified, but no vote ever reached HandleIncomingVote, the cert never -// assembled, and the chain wedged at "verified but never accepted". +// TestToConsensusOp_TableAlignedWithRouter is the systemic guard for the +// finality-wedge bug CLASS, not just the one Gossip instance. The chain router +// (node/chain_router.go) forwards an inbound message only if ToConsensusOp maps +// it, then dispatches on handler.Op(value). The node op table and the consensus +// router op enum are therefore two halves of ONE routing contract: if they +// diverge, every message for the divergent op is silently dropped. That is what +// wedged α-of-K finality — router.Gossip existed but no node op mapped to it, so +// every broadcast vote vanished before reaching blockHandler.Gossip -> +// engine.HandleIncomingVote (blocks rode PutOp and verified; votes rode the +// unmapped GossipOp and disappeared; the cert never reached alpha). // -// This asserts GossipOp maps to the consensus router's Gossip op so votes are -// delivered to blockHandler.Gossip -> engine.HandleIncomingVote. -func TestToConsensusOp_GossipIsRouted(t *testing.T) { - require := require.New(t) - - got, ok := ToConsensusOp(GossipOp) - require.True(ok, "GossipOp MUST map to a consensus router op — votes ride app-gossip; "+ - "an unmapped GossipOp is dropped by the chain router and finality wedges") - require.Equal(byte(router.Gossip), got, - "GossipOp must map to router.Gossip so blockHandler routes it to engine.HandleIncomingVote") -} - -// TestToConsensusOp_TableAlignedWithRouter pins the full message-op -> router-op -// table. Every op the chain router forwards to a consensus handler must map to -// the SAME byte the consensus side decodes it as (handler.Op == router.Op). A -// drift here re-breaks routing for that op exactly as the missing GossipOp broke -// vote delivery. +// The test proves ToConsensusOp is a BIJECTION onto the router op space +// [0, router.NumOps), so divergence is un-shippable: +// - exhaustive/surjective: every router op has exactly one node-op preimage, +// so a router op added without a node mapping (the original bug) -> RED. +// - injective/well-typed: no two node ops collide onto one router op and +// nothing maps outside the op space -> RED. +// +// router.NumOps is the single source of truth for the op count, so a future op +// added to one table but not the other fails HERE, not at runtime. func TestToConsensusOp_TableAlignedWithRouter(t *testing.T) { require := require.New(t) - cases := []struct { - name string - in Op - want router.Op - }{ - {"GetAcceptedFrontier", GetAcceptedFrontierOp, router.GetAcceptedFrontier}, - {"AcceptedFrontier", AcceptedFrontierOp, router.AcceptedFrontier}, - {"GetAccepted", GetAcceptedOp, router.GetAccepted}, - {"Accepted", AcceptedOp, router.Accepted}, - {"Get", GetOp, router.Get}, - {"Put", PutOp, router.Put}, - {"PushQuery", PushQueryOp, router.PushQuery}, - {"PullQuery", PullQueryOp, router.PullQuery}, - {"Qbit/Vote", QbitOp, router.Vote}, - {"GetAncestors/GetContext", GetAncestorsOp, router.GetContext}, - {"Ancestors/Context", AncestorsOp, router.Context}, - {"Gossip", GossipOp, router.Gossip}, + // want: the authoritative router-op <-> node-op correspondence. + want := map[router.Op]Op{ + router.GetAcceptedFrontier: GetAcceptedFrontierOp, + router.AcceptedFrontier: AcceptedFrontierOp, + router.GetAccepted: GetAcceptedOp, + router.Accepted: AcceptedOp, + router.Get: GetOp, + router.Put: PutOp, + router.PushQuery: PushQueryOp, + router.PullQuery: PullQueryOp, + router.Vote: QbitOp, // votes ride the Qbit wire op + router.GetContext: GetAncestorsOp, // wire op is still GetAncestors + router.Context: AncestorsOp, // wire op is still Ancestors + router.Gossip: GossipOp, // α-of-K vote/cert transport } - for _, tc := range cases { - got, ok := ToConsensusOp(tc.in) - require.True(ok, "%s must map to a consensus router op", tc.name) - require.Equal(byte(tc.want), got, "%s maps to the wrong router op", tc.name) + // EXHAUSTIVE: the table must name every router op exactly once. Pinned to + // router.NumOps, so a new router op with no entry here fails immediately. + require.Len(want, int(router.NumOps), + "node op table names %d ops but the router defines router.NumOps=%d — a "+ + "router op with no node mapping is dropped by the chain router and "+ + "finality wedges; add it to want and to ToConsensusOp", + len(want), int(router.NumOps)) + + // BIJECTION: rebuild the actual inverse mapping by sweeping the whole node op + // space (Op is a byte, so [0,256)) and require it to equal want. This one + // comparison catches a missing mapping (router op absent from got), a wrong + // target (got[r] != want[r]) and a stray mapping into the op space; the dup + // guard catches a collision masked by the later write winning the slot. + got := make(map[router.Op]Op, int(router.NumOps)) + for i := 0; i < 256; i++ { + nodeOp := Op(i) + v, ok := ToConsensusOp(nodeOp) + if !ok { + continue + } + require.Less(int(v), int(router.NumOps), + "%s maps to router op %d outside [0, NumOps=%d)", nodeOp, v, int(router.NumOps)) + routerOp := router.Op(v) + _, dup := got[routerOp] + require.False(dup, + "router op %d is mapped from two node ops (%s and %s) — ambiguous routing", + v, got[routerOp], nodeOp) + got[routerOp] = nodeOp } + require.Equal(want, got, + "node op table diverged from the router op space: every router op "+ + "[0, NumOps) must have exactly one node-op preimage and each node op must "+ + "map to its assigned router op. A divergence here is the finality-wedge "+ + "bug class — an op routed in one table and dropped in the other.") } // TestInboundGossip_DeliveredNotDropped reproduces the chain router's inbound