From 783115658942a79a6458e9b92a23e5a9235fd08f Mon Sep 17 00:00:00 2001 From: bdchatham Date: Fri, 10 Jul 2026 16:48:44 -0700 Subject: [PATCH 1/2] test(integration): validate a real state-sync round-trip in TestWorkflowStateSync The nightly TestWorkflowStateSync ran the state-sync workflow against a genesis-replayed follower, so "caught up again" could not distinguish a real re-sync from a no-op. Point it at a genuinely state-sync-bootstrapped node and assert the wipe + re-sync via earliest_block_height. SDK support this needs (both additive; map to existing CRD/RPC surface; nil or absent leaves behavior unchanged): - NodeSpec.StateSync (*NodeStateSync{RpcServers}) -> fullNode.snapshot.stateSync + rpcServers, wired in renderNode; validateNodeSpec enforces the >=2-witness floor mirroring the CRD MinItems=2. - EarliestHeight readiness reader (+ earliest_block_height on syncInfo), a mirror of LatestHeight. earliest>1 proves a node state-synced (not genesis-replayed); earliest advancing across the workflow proves a genuine wipe + fresh re-sync. The snapshot-interval wait is derived from the config so retuning the interval can't silently under-wait and make the assertion flaky. Co-Authored-By: Claude Opus 4.8 --- sdk/sei/provider/k8s/render.go | 8 ++ sdk/sei/provider/k8s/render_test.go | 26 +++++++ sdk/sei/readiness.go | 27 ++++++- sdk/sei/sei.go | 2 + sdk/sei/spec.go | 16 ++++ sdk/sei/spec_test.go | 2 + test/integration/workflow_test.go | 109 ++++++++++++++++++++++++---- 7 files changed, 172 insertions(+), 18 deletions(-) diff --git a/sdk/sei/provider/k8s/render.go b/sdk/sei/provider/k8s/render.go index 596e6182..a4abb567 100644 --- a/sdk/sei/provider/k8s/render.go +++ b/sdk/sei/provider/k8s/render.go @@ -111,6 +111,14 @@ func renderNode(spec sei.NodeSpec, namespace, networkNS string) *seiv1alpha1.Sei if len(spec.Config) > 0 { node.Spec.Overrides = maps.Clone(spec.Config) } + if ss := spec.StateSync; ss != nil { + // Bootstrap via CometBFT state sync: the witnesses are the caller's bare + // host:port RpcServers; genesis block-sync leaves Snapshot nil (unchanged). + node.Spec.FullNode.Snapshot = &seiv1alpha1.SnapshotSource{ + StateSync: &seiv1alpha1.StateSyncSource{}, + RpcServers: ss.RpcServers, + } + } return node } diff --git a/sdk/sei/provider/k8s/render_test.go b/sdk/sei/provider/k8s/render_test.go index 07a8df0f..ec782847 100644 --- a/sdk/sei/provider/k8s/render_test.go +++ b/sdk/sei/provider/k8s/render_test.go @@ -1,6 +1,7 @@ package k8s import ( + "slices" "testing" "github.com/sei-protocol/sei-k8s-controller/sdk/sei" @@ -126,6 +127,31 @@ func TestRenderNode_CallerLabelsMergeUnderCanonical(t *testing.T) { } } +func TestRenderNode_StateSync(t *testing.T) { + witnesses := []string{"validator:26657", "validator:26657"} + spec := sei.NodeSpec{ + Name: rpc0Name, Network: testNet, Image: testImage, + StateSync: &sei.NodeStateSync{RpcServers: witnesses}, + } + node := renderNode(spec, testNS, testNS) + + // StateSync maps to spec.fullNode.snapshot with the stateSync variant and the + // caller's bare host:port witnesses. + snap := node.Spec.FullNode.Snapshot + if snap == nil || snap.StateSync == nil { + t.Fatalf("state-sync node must render fullNode.snapshot.stateSync: %+v", node.Spec.FullNode) + } + if !slices.Equal(snap.RpcServers, witnesses) { + t.Errorf("snapshot.rpcServers = %v, want %v", snap.RpcServers, witnesses) + } + + // Genesis block-sync (nil StateSync) leaves Snapshot unset — the unchanged path. + genesis := renderNode(sei.NodeSpec{Name: rpc0Name, Network: testNet, Image: testImage}, testNS, testNS) + if genesis.Spec.FullNode.Snapshot != nil { + t.Errorf("genesis node must leave snapshot nil, got %+v", genesis.Spec.FullNode.Snapshot) + } +} + func TestRenderNode_ConfigIntoOverrides(t *testing.T) { spec := sei.NodeSpec{ Name: rpc0Name, Network: testNet, Image: testImage, diff --git a/sdk/sei/readiness.go b/sdk/sei/readiness.go index 7065d058..979e4eff 100644 --- a/sdk/sei/readiness.go +++ b/sdk/sei/readiness.go @@ -34,8 +34,9 @@ type tendermintStatus struct { } type syncInfo struct { - LatestBlockHeight string `json:"latest_block_height"` - CatchingUp bool `json:"catching_up"` + LatestBlockHeight string `json:"latest_block_height"` + EarliestBlockHeight string `json:"earliest_block_height"` + CatchingUp bool `json:"catching_up"` } func (s *tendermintStatus) sync() syncInfo { @@ -92,6 +93,28 @@ func latestHeight(ctx context.Context, hc *http.Client, tmRPC string) (int64, bo return h, true } +// EarliestHeight reads tmRPC's earliest retained block height from /status. +// ok=false on an unreachable endpoint or unparseable body. A state-synced node +// reports an earliest > 1 (it started from a snapshot); a genesis-replayed node +// reports 1 — so this is the point read that distinguishes the two. It accepts +// both the enveloped and unwrapped /status shapes the Sei fork emits. Point read +// only; there is no Wait wrapper. +func EarliestHeight(ctx context.Context, hc *http.Client, tmRPC string) (int64, bool) { + body, ok := getJSON(ctx, hc, http.MethodGet, tmRPC+"/status", "") + if !ok { + return 0, false + } + var s tendermintStatus + if json.Unmarshal(body, &s) != nil { + return 0, false + } + h, err := strconv.ParseInt(s.sync().EarliestBlockHeight, 10, 64) + if err != nil { + return 0, false + } + return h, true +} + // WaitHeightAdvances blocks until tmRPC's committed height has risen by at least // delta from the height observed on the first successful read — proof the chain // is producing blocks, not merely reachable. Unlike WaitCaughtUp this catches a diff --git a/sdk/sei/sei.go b/sdk/sei/sei.go index 0d842123..b4575577 100644 --- a/sdk/sei/sei.go +++ b/sdk/sei/sei.go @@ -218,6 +218,8 @@ func validateNodeSpec(s NodeSpec) error { return usageErr("NodeSpec.Network is required (the peer-wire target)") case strings.TrimSpace(s.Image) == "": return usageErr("NodeSpec.Image is required") + case s.StateSync != nil && len(s.StateSync.RpcServers) < 2: + return usageErr("NodeSpec.StateSync.RpcServers must have >= 2 entries (the state-sync witness floor), got %d", len(s.StateSync.RpcServers)) } return nil } diff --git a/sdk/sei/spec.go b/sdk/sei/spec.go index 16b0ae8b..9d7700e9 100644 --- a/sdk/sei/spec.go +++ b/sdk/sei/spec.go @@ -51,4 +51,20 @@ type NodeSpec struct { Image string // -> spec.image Config map[string]string // -> spec.overrides (TOML-path keys) Labels map[string]string // extra labels on the SeiNode object, merged UNDER the canonical sei.io/role + sei.io/seinetwork (which win on key conflict) + + // StateSync bootstraps the node through CometBFT state sync instead of + // genesis block-sync; nil => genesis block-sync (unchanged). Set it to bring + // a follower up from a peer-served snapshot rather than replaying from height + // 1. -> spec.fullNode.snapshot{stateSync{}, rpcServers}. + StateSync *NodeStateSync +} + +// NodeStateSync configures a SeiNode to bootstrap via CometBFT state sync. +type NodeStateSync struct { + // RpcServers are the light-client witnesses used for trust-point acquisition + // and verification. Entries are BARE host:port (no scheme) and there must be + // >= 2 (the CRD's fail-closed floor; CometBFT dedups identical entries). + // Network.TendermintRPC() returns a URL, so a caller deriving a witness from + // it must url.Parse(...).Host to strip the scheme. -> spec.fullNode.snapshot.rpcServers. + RpcServers []string } diff --git a/sdk/sei/spec_test.go b/sdk/sei/spec_test.go index 30cddf92..eca94ec3 100644 --- a/sdk/sei/spec_test.go +++ b/sdk/sei/spec_test.go @@ -36,6 +36,8 @@ func TestValidateNodeSpec(t *testing.T) { {"missing name", func(s *NodeSpec) { s.Name = "" }, true}, {"missing network", func(s *NodeSpec) { s.Network = "" }, true}, {"missing image", func(s *NodeSpec) { s.Image = "" }, true}, + {"statesync one witness", func(s *NodeSpec) { s.StateSync = &NodeStateSync{RpcServers: []string{"a:26657"}} }, true}, + {"statesync two witnesses", func(s *NodeSpec) { s.StateSync = &NodeStateSync{RpcServers: []string{"a:26657", "b:26657"}} }, false}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/test/integration/workflow_test.go b/test/integration/workflow_test.go index 4452938e..dfba199e 100644 --- a/test/integration/workflow_test.go +++ b/test/integration/workflow_test.go @@ -5,7 +5,9 @@ package integration import ( "context" "net/http" + "net/url" "os/signal" + "strconv" "syscall" "testing" "time" @@ -32,13 +34,16 @@ var snapshotProductionConfig = map[string]string{ "storage.snapshot_keep_recent": "3", } -// TestWorkflowStateSync drives the SeiNodeTaskWorkflow StateSync recipe through -// the SDK end to end: provision a chain + one caught-up RPC follower, then -// CreateWorkflow(StateSync) against the follower and WaitWorkflowTerminal. The -// recipe holds seid, wipes the follower's data directory, re-configures -// state-sync against the witnesses, and releases — so a Complete workflow with -// the follower caught up AGAIN is the re-bootstrap observable (wiped-then-synced, -// no manual data-directory surgery). Acceptance criterion 5. +// TestWorkflowStateSync drives the full state-sync procedure through the SDK end +// to end: provision a snapshot-producing chain, wait past the snapshot interval, +// then bring up a state-sync-bootstrapped follower and assert it started FROM a +// snapshot (earliest retained height > 1, not a genesis replay's 1). It then runs +// the SeiNodeTaskWorkflow StateSync recipe against THAT follower — the recipe +// holds seid, wipes the data directory, re-configures state-sync against the +// witnesses, and releases — and asserts a Complete workflow with the follower +// caught up AGAIN and its earliest height advanced past the pre-wipe floor: proof +// of a genuine wipe + fresh re-sync to a newer snapshot, not a no-op. Acceptance +// criterion 5. // // Witnesses: the recipe's RpcServers are the network's aggregate TM RPC (the // validator service), passed as the >=2 the controller's fail-closed floor @@ -80,14 +85,75 @@ func TestWorkflowStateSync(t *testing.T) { if err != nil { t.Fatalf("provision: %v", err) } - node := ch.rpcNodes[0] hc := &http.Client{Timeout: 10 * time.Second} - // Re-bootstrap the follower through the StateSync recipe. + // A follower can only state-sync from a snapshot that already exists. The + // witness validators emit one every storage.snapshot_interval blocks + // (snapshotProductionConfig); advance the aggregate RPC one interval + a + // margin to guarantee at least one snapshot boundary is crossed before we + // bootstrap from it. Derive the delta from the config so retuning the + // interval can't silently under-wait and make the state-sync assertion flaky. + interval, err := strconv.Atoi(snapshotProductionConfig["storage.snapshot_interval"]) + if err != nil { + t.Fatalf("snapshot_interval not an int: %v", err) + } + if err := sei.WaitHeightAdvances(ctx, hc, ch.network.TendermintRPC(), int64(interval)+10); err != nil { + t.Fatalf("network did not advance past the snapshot interval: %v", err) + } + + // Witnesses are the aggregate validator TM RPC as BARE host:port (the CRD + // SnapshotSource.RpcServers shape); TendermintRPC() is a URL, so strip the + // scheme. One aggregate witness passed twice satisfies the >=2 floor — + // CometBFT dedups identical entries; a distinct N-witness topology is a later + // refinement. + u, err := url.Parse(ch.network.TendermintRPC()) + if err != nil { + t.Fatalf("parse network TM RPC %q: %v", ch.network.TendermintRPC(), err) + } + witness := u.Host + + // Bring up a state-sync-bootstrapped follower: it must fetch a snapshot from a + // peer rather than replay from genesis. Appended to ch.rpcNodes so cleanupChain + // (registered above, evaluated at teardown) reaps it too. + ssNode, err := c.CreateNode(ctx, sei.NodeSpec{ + Name: "statesync-" + chainID, + Network: chainID, + Namespace: ns, + Image: seid, + Labels: map[string]string{runLabelKey: chainID}, + StateSync: &sei.NodeStateSync{RpcServers: []string{witness, witness}}, + }) + if ssNode != nil { + ch.rpcNodes = append(ch.rpcNodes, ssNode) + } + if err != nil { + t.Fatalf("create state-sync node: %v", err) + } + if err := ssNode.WaitReady(ctx); err != nil { + t.Fatalf("state-sync node %q running: %v", ssNode.Name(), err) + } + if err := sei.WaitCaughtUp(ctx, hc, ssNode.TendermintRPC()); err != nil { + t.Fatalf("state-sync node %q caught up: %v", ssNode.Name(), err) + } + + // A state-synced node starts from a snapshot, so its earliest retained height + // is > 1; a genesis replay would report 1. This is the proof the bootstrap + // took the state-sync path, and the pre-wipe floor the post-workflow earliest + // must exceed. + preEarliest, ok := sei.EarliestHeight(ctx, hc, ssNode.TendermintRPC()) + if !ok { + t.Fatalf("read earliest height of %q", ssNode.Name()) + } + if preEarliest <= 1 { + t.Fatalf("state-sync node %q earliest height = %d, want > 1 (a genesis replay reports 1)", ssNode.Name(), preEarliest) + } + t.Logf("state-sync node %s: bootstrapped from snapshot (earliest height %d)", ssNode.Name(), preEarliest) + + // Re-bootstrap that state-sync follower through the StateSync recipe. wf, err := c.CreateWorkflow(ctx, sei.WorkflowSpec{ Name: "resync-" + chainID, Namespace: ns, - Node: node.Name(), + Node: ssNode.Name(), Kind: sei.WorkflowStateSync, Labels: map[string]string{runLabelKey: chainID}, StateSync: &sei.StateSyncWorkflow{ @@ -98,7 +164,7 @@ func TestWorkflowStateSync(t *testing.T) { t.Fatalf("create workflow: %v", err) } cleanupWorkflow(t, wf) - t.Logf("workflow %s: created against %s", wf.Name(), node.Name()) + t.Logf("workflow %s: created against %s", wf.Name(), ssNode.Name()) if err := wf.WaitTerminal(ctx); err != nil { t.Fatalf("workflow %s did not complete: %v", wf.Name(), err) @@ -111,13 +177,24 @@ func TestWorkflowStateSync(t *testing.T) { // Re-bootstrap observable: after the wipe + resync the follower rejoins and // catches up again (it cannot catch up to a chain it is not synced with, so // this covers the wiped-then-synced round trip) and serves EVM again. - if err := sei.WaitCaughtUp(ctx, hc, node.TendermintRPC()); err != nil { - t.Fatalf("follower %s not caught up after resync: %v", node.Name(), err) + if err := sei.WaitCaughtUp(ctx, hc, ssNode.TendermintRPC()); err != nil { + t.Fatalf("follower %s not caught up after resync: %v", ssNode.Name(), err) + } + if err := sei.WaitEVMServing(ctx, hc, ssNode.EVMRPC()); err != nil { + t.Errorf("follower %s EVM not serving after resync: %v", ssNode.Name(), err) + } + + // A genuine wipe + fresh re-sync lands on a NEW (higher) snapshot, so the + // earliest retained height must rise past the pre-workflow floor; a no-op + // recipe would leave earliest unchanged. + postEarliest, ok := sei.EarliestHeight(ctx, hc, ssNode.TendermintRPC()) + if !ok { + t.Fatalf("read post-workflow earliest height of %q", ssNode.Name()) } - if err := sei.WaitEVMServing(ctx, hc, node.EVMRPC()); err != nil { - t.Errorf("follower %s EVM not serving after resync: %v", node.Name(), err) + if postEarliest <= preEarliest { + t.Fatalf("follower %s earliest height = %d after resync, want > %d (a genuine wipe + re-sync lands on a newer snapshot)", ssNode.Name(), postEarliest, preEarliest) } - t.Logf("follower %s: caught up + EVM serving after state-sync re-bootstrap — TestWorkflowStateSync OK", node.Name()) + t.Logf("follower %s: caught up + EVM serving, earliest advanced %d -> %d after state-sync re-bootstrap — TestWorkflowStateSync OK", ssNode.Name(), preEarliest, postEarliest) // Interrupt-resume variant (deferred): kill the follower pod mid-recipe and // assert the workflow resumes to Complete. Deferred here — it needs a From 8819fe4a9e9153f2ea6d3b6099f3bc6890985b35 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Fri, 10 Jul 2026 17:41:25 -0700 Subject: [PATCH 2/2] =?UTF-8?q?test(integration):=20xreview=20fixes=20?= =?UTF-8?q?=E2=80=94=20distinct=20witnesses,=20sound=20recipe=20witness=20?= =?UTF-8?q?set,=20base-pin?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the blinded /xreview slate (systems + kubernetes + idiom): - Witnesses are two DISTINCT light-client sources (genesis validator-0 + the provisioned rpc follower) instead of the aggregate RPC counted twice, which the node CRD (listType=set/MinItems=2) rejects/collapses below the >=2 floor. Keeps a single validator (avoids the 2-validator genesis blocksync stall + 2/3-liveness fragility) and revives the otherwise-dead provisioned follower. - The workflow recipe now inherits the node's resolved DISTINCT syncers (RpcServers: nil) instead of re-syncing from the aggregate-twice unsound set. - validateNodeSpec requires >=2 DISTINCT witnesses (dedup-then-count, mirroring the controller's slices.Sorted+Compact) and reports the distinct count. - Pin the follower's block-store base explicitly (chain.min_retain_blocks=0) so the earliest-height discontinuity assertion rests on a stated invariant, not an image default; corrected the comment (earliest is the CometBFT block-store base governed by min-retain-blocks, not storage.pruning's state-store subsystem). - witnessA/witnessB test consts to clear the goconst golangci-lint gate. golangci-lint run ./sdk/... = 0 issues; go build/vet, go test ./sdk/..., gofmt green. Co-Authored-By: Claude Opus 4.8 --- sdk/sei/provider/k8s/render_test.go | 4 +- sdk/sei/sei.go | 16 +++- sdk/sei/spec.go | 6 +- sdk/sei/spec_test.go | 15 ++- sdk/sei/workflow_test.go | 2 +- test/integration/workflow_test.go | 140 ++++++++++++++++++++++------ 6 files changed, 149 insertions(+), 34 deletions(-) diff --git a/sdk/sei/provider/k8s/render_test.go b/sdk/sei/provider/k8s/render_test.go index ec782847..4e5780d2 100644 --- a/sdk/sei/provider/k8s/render_test.go +++ b/sdk/sei/provider/k8s/render_test.go @@ -128,7 +128,9 @@ func TestRenderNode_CallerLabelsMergeUnderCanonical(t *testing.T) { } func TestRenderNode_StateSync(t *testing.T) { - witnesses := []string{"validator:26657", "validator:26657"} + // Two DISTINCT witnesses: the served CRD field is a MinItems=2 listType=set, + // so a duplicated set is not what render carries end to end. + witnesses := []string{"validator-0:26657", "validator-1:26657"} spec := sei.NodeSpec{ Name: rpc0Name, Network: testNet, Image: testImage, StateSync: &sei.NodeStateSync{RpcServers: witnesses}, diff --git a/sdk/sei/sei.go b/sdk/sei/sei.go index b4575577..7f2465a7 100644 --- a/sdk/sei/sei.go +++ b/sdk/sei/sei.go @@ -21,6 +21,7 @@ package sei import ( "context" "os" + "slices" "strings" ) @@ -218,8 +219,19 @@ func validateNodeSpec(s NodeSpec) error { return usageErr("NodeSpec.Network is required (the peer-wire target)") case strings.TrimSpace(s.Image) == "": return usageErr("NodeSpec.Image is required") - case s.StateSync != nil && len(s.StateSync.RpcServers) < 2: - return usageErr("NodeSpec.StateSync.RpcServers must have >= 2 entries (the state-sync witness floor), got %d", len(s.StateSync.RpcServers)) + case s.StateSync != nil: + // Count DISTINCT entries, not the raw length: the served CRD field is a + // MinItems=2 listType=set, and the controller normalizes it with the same + // sort+Compact before its canonical-syncer floor — so a duplicated witness + // set collapses below the floor and never produces a state-sync plan. + // Mirror that here rather than green-lighting a set the apiserver/controller + // will reject. Witnesses must be distinct light-client sources. + if distinct := len(slices.Compact(slices.Sorted(slices.Values(s.StateSync.RpcServers)))); distinct < 2 { + // Report the DISTINCT count, not the raw len: the floor is on distinct + // entries, so a two-element set of identical witnesses fails with + // distinct=1 — reporting len=2 would misdescribe the failure. + return usageErr("NodeSpec.StateSync.RpcServers must have >= 2 DISTINCT entries (the state-sync witness floor: served CRD MinItems=2 listType=set + the controller's canonical-syncer floor), got %d distinct", distinct) + } } return nil } diff --git a/sdk/sei/spec.go b/sdk/sei/spec.go index 9d7700e9..22fcb095 100644 --- a/sdk/sei/spec.go +++ b/sdk/sei/spec.go @@ -63,7 +63,11 @@ type NodeSpec struct { type NodeStateSync struct { // RpcServers are the light-client witnesses used for trust-point acquisition // and verification. Entries are BARE host:port (no scheme) and there must be - // >= 2 (the CRD's fail-closed floor; CometBFT dedups identical entries). + // >= 2 DISTINCT endpoints: the served CRD field is a MinItems=2 listType=set, + // and the controller sort+dedups the set before its canonical-syncer floor — + // so duplicates collapse below the floor and no state-sync plan is built (the + // gate fails CLOSED). Witnesses must also be distinct light-client sources; an + // aggregate round-robin RPC counted twice is not a sound witness set. // Network.TendermintRPC() returns a URL, so a caller deriving a witness from // it must url.Parse(...).Host to strip the scheme. -> spec.fullNode.snapshot.rpcServers. RpcServers []string diff --git a/sdk/sei/spec_test.go b/sdk/sei/spec_test.go index eca94ec3..00e46ff5 100644 --- a/sdk/sei/spec_test.go +++ b/sdk/sei/spec_test.go @@ -2,6 +2,16 @@ package sei import "testing" +// witnessA and witnessB are the two distinct bare host:port light-client +// witnesses reused across the state-sync validation tests in this package. +// Named consts (mirroring sdk/sei/provider/k8s) keep the literal below the +// goconst threshold. The values are placeholders — the shape (host:port), not +// the address, is what these tests exercise. +const ( + witnessA = "a:26657" + witnessB = "b:26657" +) + func TestValidateNetworkSpec(t *testing.T) { good := NetworkSpec{Name: "net", Image: "img", Validators: 1} cases := []struct { @@ -36,8 +46,9 @@ func TestValidateNodeSpec(t *testing.T) { {"missing name", func(s *NodeSpec) { s.Name = "" }, true}, {"missing network", func(s *NodeSpec) { s.Network = "" }, true}, {"missing image", func(s *NodeSpec) { s.Image = "" }, true}, - {"statesync one witness", func(s *NodeSpec) { s.StateSync = &NodeStateSync{RpcServers: []string{"a:26657"}} }, true}, - {"statesync two witnesses", func(s *NodeSpec) { s.StateSync = &NodeStateSync{RpcServers: []string{"a:26657", "b:26657"}} }, false}, + {"statesync one witness", func(s *NodeSpec) { s.StateSync = &NodeStateSync{RpcServers: []string{witnessA}} }, true}, + {"statesync two identical witnesses", func(s *NodeSpec) { s.StateSync = &NodeStateSync{RpcServers: []string{witnessA, witnessA}} }, true}, + {"statesync two distinct witnesses", func(s *NodeSpec) { s.StateSync = &NodeStateSync{RpcServers: []string{witnessA, witnessB}} }, false}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/sdk/sei/workflow_test.go b/sdk/sei/workflow_test.go index 99e16417..c5b1d264 100644 --- a/sdk/sei/workflow_test.go +++ b/sdk/sei/workflow_test.go @@ -31,7 +31,7 @@ func TestValidateWorkflowSpec(t *testing.T) { func(s *WorkflowSpec) { s.StateSync = &StateSyncWorkflow{ ConfigPatch: map[string]map[string]any{"app.toml": {"state-store.evm-ss-split": true}}, - RpcServers: []string{"a:26657", "b:26657"}, + RpcServers: []string{witnessA, witnessB}, } }, false, diff --git a/test/integration/workflow_test.go b/test/integration/workflow_test.go index dfba199e..a9f22c26 100644 --- a/test/integration/workflow_test.go +++ b/test/integration/workflow_test.go @@ -4,10 +4,12 @@ package integration import ( "context" + "fmt" "net/http" "net/url" "os/signal" "strconv" + "strings" "syscall" "testing" "time" @@ -15,10 +17,13 @@ import ( "github.com/sei-protocol/sei-k8s-controller/sdk/sei" ) -// snapshotProductionConfig makes the witness validators emit CometBFT state-sync +// snapshotProductionConfig makes the witness nodes emit CometBFT state-sync // snapshots so a wiped follower can re-bootstrap from them. Overlaid on the -// memiavl baseline for the network in this suite. Without it, the resync has -// nothing to sync FROM and the workflow's await-caught-up step never clears. +// memiavl baseline for the network in this suite, it reaches BOTH witnesses: it +// is applied to the network (so the genesis validator inherits it) and to the +// rpc follower via provision's storageConfig, so both witnesses serve chunks. +// Without it, the resync has nothing to sync FROM and the workflow's +// await-caught-up step never clears. // // Keys are the sei-config storage.* overrides that sei-config.SnapshotGenerationOverrides // emits (storage.snapshot_interval / storage.snapshot_keep_recent, plus @@ -45,15 +50,26 @@ var snapshotProductionConfig = map[string]string{ // of a genuine wipe + fresh re-sync to a newer snapshot, not a no-op. Acceptance // criterion 5. // -// Witnesses: the recipe's RpcServers are the network's aggregate TM RPC (the -// validator service), passed as the >=2 the controller's fail-closed floor -// requires; CometBFT dedups identical entries. A dedicated witness topology -// (N distinct RPC endpoints) is a later refinement. +// Witnesses: the follower's RpcServers are TWO DISTINCT full-node RPC endpoints — +// genesis validator-0 and the provisioned rpc follower (rpcNodes[0]) — each a +// caught-up node that serves snapshot chunks (nodeRPC below). Two DISTINCT +// witnesses drawn from different nodes (rather than two equal validators) keeps +// liveness robust: the chain needs only its single validator online, and there is +// no 2-validator genesis blocksync-bootstrap deadlock to stall on. The +// >=2-DISTINCT requirement is NOT a CometBFT property — it is the served CRD field +// (SnapshotSource.RpcServers is a MinItems=2 listType=set) plus the controller's +// canonical-syncer floor, which sort+dedups before counting. A duplicated set is +// rejected at admission or collapses below the floor, so no state-sync plan is +// built (fail CLOSED) and the follower falls back to a genesis block-sync — which +// the earliest>1 assertion would then catch. Distinct light-client sources are +// also the point of a witness set; an aggregate round-robin service counted twice +// is unsound. // // Cluster dependencies the CronJob wiring owns (documented, not asserted here): // - the deployed controller + seid image carry the hold-aware start gate and // the mark-not-ready/stop-seid/reset-data sidecar tasks; -// - the witness validators produce state-sync snapshots (snapshotProductionConfig). +// - both witnesses (validator-0 and the rpc follower) produce state-sync +// snapshots (snapshotProductionConfig, applied to both — see above). // // Inputs (env): SEI_CHAIN_ID, SEID_IMAGE [required]; SEI_NAMESPACE, // SEI_VALIDATORS [optional]. Run as the nightly CronJob: @@ -72,12 +88,20 @@ func TestWorkflowStateSync(t *testing.T) { c := openClient(ctx, t) + // State-sync needs >=2 DISTINCT light-client witnesses (nodeRPC below). This + // suite draws them from TWO DIFFERENT nodes — the single genesis validator and + // the one provisioned rpc follower — so a single validator suffices. A second + // equal validator would be strictly more fragile (both must stay online for + // 2/3, and some seid images stall a 2-validator genesis in a blocksync + // bootstrap deadlock), so keep the default at one. + validators := envInt(t, "SEI_VALIDATORS", 1) + ch, err := provision(ctx, t, c, spec{ chainID: chainID, runID: chainID, namespace: ns, seidImage: seid, - validators: envInt(t, "SEI_VALIDATORS", 1), + validators: validators, rpcNodes: 1, storageConfig: mergeConfig(memiavlStorageConfig, snapshotProductionConfig), }) @@ -101,27 +125,57 @@ func TestWorkflowStateSync(t *testing.T) { t.Fatalf("network did not advance past the snapshot interval: %v", err) } - // Witnesses are the aggregate validator TM RPC as BARE host:port (the CRD - // SnapshotSource.RpcServers shape); TendermintRPC() is a URL, so strip the - // scheme. One aggregate witness passed twice satisfies the >=2 floor — - // CometBFT dedups identical entries; a distinct N-witness topology is a later - // refinement. + // Two DISTINCT full-node RPC endpoints as BARE host:port (the CRD + // SnapshotSource.RpcServers shape; nodeRPC encodes the controller's + // per-node headless-service naming): genesis validator-0 (service -0) + // and the provisioned rpc follower (service rpcNodeName(chainID,0) = + // -rpc-0). Both are caught-up full nodes and valid light-client + // witnesses; the follower is otherwise dead weight since the workflow target + // moved to ssNode. Derive the namespace from the aggregate RPC host rather than + // the SEI_NAMESPACE input, which may be "" — the SDK resolves an empty + // namespace to a kubeconfig/SA default that only the served endpoint reflects. + // The aggregate host is -internal..svc[.cluster.local]; chainID + // carries no dots, so the second dotted label is the namespace. u, err := url.Parse(ch.network.TendermintRPC()) if err != nil { t.Fatalf("parse network TM RPC %q: %v", ch.network.TendermintRPC(), err) } - witness := u.Host + hostLabels := strings.Split(u.Hostname(), ".") + if len(hostLabels) < 2 || hostLabels[1] == "" { + t.Fatalf("cannot derive namespace from aggregate RPC host %q", u.Host) + } + witnessNS := hostLabels[1] + witnesses := []string{ + nodeRPC(fmt.Sprintf("%s-0", chainID), witnessNS), // genesis validator-0 + nodeRPC(rpcNodeName(chainID, 0), witnessNS), // the provisioned rpc follower + } // Bring up a state-sync-bootstrapped follower: it must fetch a snapshot from a // peer rather than replay from genesis. Appended to ch.rpcNodes so cleanupChain // (registered above, evaluated at teardown) reaps it too. + // + // Pin the block-store base with chain.min_retain_blocks=0. earliest_block_height + // is the CometBFT BLOCK-store base, governed by min-retain-blocks (0 = keep all, + // no Tendermint block pruning); storage.pruning is sei-config STATE-store pruning, + // a different subsystem that does NOT move the block-store base. We set + // min_retain_blocks explicitly (it defaults to 0, but stating it makes the + // base-pin a fact of the recipe, not an inherited default) so earliest moves + // ONLY on a genuine wipe + fresh restore — which is what the discontinuity + // assertion below relies on. storage.pruning=nothing is kept for the state store + // (unpruned state is also what snapshot production needs). memiavl matches the + // network's write-mode (the nightly image rejects the controller default; see + // memiavlStorageConfig). ssNode, err := c.CreateNode(ctx, sei.NodeSpec{ Name: "statesync-" + chainID, Network: chainID, Namespace: ns, Image: seid, Labels: map[string]string{runLabelKey: chainID}, - StateSync: &sei.NodeStateSync{RpcServers: []string{witness, witness}}, + Config: mergeConfig(memiavlStorageConfig, map[string]string{ + "storage.pruning": "nothing", // state-store pruning off + "chain.min_retain_blocks": "0", // block-store base pin (keep all blocks) + }), + StateSync: &sei.NodeStateSync{RpcServers: witnesses}, }) if ssNode != nil { ch.rpcNodes = append(ch.rpcNodes, ssNode) @@ -147,18 +201,30 @@ func TestWorkflowStateSync(t *testing.T) { if preEarliest <= 1 { t.Fatalf("state-sync node %q earliest height = %d, want > 1 (a genesis replay reports 1)", ssNode.Name(), preEarliest) } - t.Logf("state-sync node %s: bootstrapped from snapshot (earliest height %d)", ssNode.Name(), preEarliest) + // Capture the tip too: the post-workflow discontinuity check is about the base + // (earliest) jumping, not the tip advancing — logging both makes the jump + // legible against ordinary block production. + preLatest, ok := sei.LatestHeight(ctx, hc, ssNode.TendermintRPC()) + if !ok { + t.Fatalf("read latest height of %q", ssNode.Name()) + } + t.Logf("state-sync node %s: bootstrapped from snapshot (earliest %d, latest %d)", ssNode.Name(), preEarliest, preLatest) - // Re-bootstrap that state-sync follower through the StateSync recipe. + // Re-bootstrap that state-sync follower through the StateSync recipe. Leave the + // recipe's RpcServers nil: the workflow planner inherits the node's already- + // resolved DISTINCT witnesses (node.Status.ResolvedStateSyncers, set from this + // ssNode's spec rpcServers = the two witnesses above) when the recipe carries + // none (internal/planner/workflow.go BuildPlan). Passing the aggregate + // round-robin RPC twice would be an unsound duplicate witness set that the + // planner's raw-len count does not dedup — nil sidesteps it and reuses the + // vetted distinct set. wf, err := c.CreateWorkflow(ctx, sei.WorkflowSpec{ Name: "resync-" + chainID, Namespace: ns, Node: ssNode.Name(), Kind: sei.WorkflowStateSync, Labels: map[string]string{runLabelKey: chainID}, - StateSync: &sei.StateSyncWorkflow{ - RpcServers: []string{ch.network.TendermintRPC(), ch.network.TendermintRPC()}, - }, + StateSync: &sei.StateSyncWorkflow{RpcServers: nil}, }) if err != nil { t.Fatalf("create workflow: %v", err) @@ -184,17 +250,24 @@ func TestWorkflowStateSync(t *testing.T) { t.Errorf("follower %s EVM not serving after resync: %v", ssNode.Name(), err) } - // A genuine wipe + fresh re-sync lands on a NEW (higher) snapshot, so the - // earliest retained height must rise past the pre-workflow floor; a no-op - // recipe would leave earliest unchanged. + // Prove a genuine wipe + fresh re-sync via a DISCONTINUITY in the block-store + // base, not merely a rising earliest: on a running full node routine pruning + // also advances earliest_block_height, so postEarliest > preEarliest alone + // would false-green a no-op workflow. Two things make the jump conclusive: + // this follower pins chain.min_retain_blocks=0 (so block-store pruning cannot + // move the base at all), and a fresh state-sync restore lands on a NEW snapshot + // at least a full snapshot_interval higher than the previous one. Incremental pruning + // cannot move the base a whole interval inside the test window; only a wipe + + // restore can. So require the base to have jumped by >= one interval. postEarliest, ok := sei.EarliestHeight(ctx, hc, ssNode.TendermintRPC()) if !ok { t.Fatalf("read post-workflow earliest height of %q", ssNode.Name()) } - if postEarliest <= preEarliest { - t.Fatalf("follower %s earliest height = %d after resync, want > %d (a genuine wipe + re-sync lands on a newer snapshot)", ssNode.Name(), postEarliest, preEarliest) + if postEarliest-preEarliest < int64(interval) { + t.Fatalf("follower %s earliest height %d -> %d after resync (jump %d), want a jump >= snapshot_interval %d (a genuine wipe + re-sync lands on a snapshot at least one interval higher; a smaller move is routine pruning, not a fresh restore)", + ssNode.Name(), preEarliest, postEarliest, postEarliest-preEarliest, interval) } - t.Logf("follower %s: caught up + EVM serving, earliest advanced %d -> %d after state-sync re-bootstrap — TestWorkflowStateSync OK", ssNode.Name(), preEarliest, postEarliest) + t.Logf("follower %s: caught up + EVM serving, block-store base jumped %d -> %d (>= interval %d) after state-sync re-bootstrap — TestWorkflowStateSync OK", ssNode.Name(), preEarliest, postEarliest, interval) // Interrupt-resume variant (deferred): kill the follower pod mid-recipe and // assert the workflow resumes to Complete. Deferred here — it needs a @@ -208,6 +281,19 @@ func TestWorkflowStateSync(t *testing.T) { }) } +// nodeRPC is a SeiNode's CometBFT RPC as a bare host:port state-sync witness (no +// scheme — the CRD SnapshotSource.RpcServers shape, ^[^\s:/,]+:[0-9]{1,5}$). +// service is the controller's per-node headless Service name, which the +// controller creates same-named as the SeiNode exposing CometBFT RPC on 26657 +// (noderesource.GenerateHeadlessService), resolvable at +// ..svc.cluster.local. A SeiNetwork names each child validator +// - (seinetwork.seiNodeName); a standalone follower is named by +// the harness (rpcNodeName => -rpc-). Distinct services yield +// the distinct witnesses the state-sync floor requires. +func nodeRPC(service, ns string) string { + return fmt.Sprintf("%s.%s.svc.cluster.local:26657", service, ns) +} + // cleanupWorkflow deletes the workflow on a fresh context (the scenario ctx may // be expired). A completed workflow's finalizer is already reaped, so the delete // resolves promptly; best-effort like the chain teardown.