Skip to content

test(integration): validate a real state-sync round-trip + SDK state-sync node support#472

Open
bdchatham wants to merge 2 commits into
mainfrom
feat/statesync-workflow-e2e
Open

test(integration): validate a real state-sync round-trip + SDK state-sync node support#472
bdchatham wants to merge 2 commits into
mainfrom
feat/statesync-workflow-e2e

Conversation

@bdchatham

Copy link
Copy Markdown
Collaborator

Strengthens the nightly TestWorkflowStateSync so it validates the full state-sync workflow procedure end-to-end (the manual procedure just proven live), plus the minimal SDK surface it needs.

Why

The existing nightly ran the state-sync workflow against a genesis-replayed follower, so its "caught up again" check couldn't distinguish a genuine re-sync from a no-op. This points the workflow at a truly state-sync-bootstrapped node and asserts the wipe + re-sync via earliest_block_height.

SDK (additive; nil/absent = unchanged)

  • NodeSpec.StateSync (*NodeStateSync{ RpcServers []string }) → fullNode.snapshot.stateSync + rpcServers in renderNode; validateNodeSpec enforces the ≥2-witness floor (mirrors CRD MinItems=2). Doc-comments the bare-host:port invariant + the url.Parse().Host caller obligation.
  • EarliestHeight readiness reader (+ earliest_block_height on syncInfo), a mirror of LatestHeight.

Test

Create a state-sync follower (witnesses = aggregate validator RPC as bare host:port), assert earliest > 1 (proves state-synced, not replayed); run the StateSync workflow on it; assert Complete + earliest advanced (proves a genuine wipe + fresh re-sync). Existing proven network/snapshot config preserved verbatim; snapshot-interval wait derived from the config.

Review

Idiom lens: RATIFY (nested-pointer shape matches WorkflowSpec.StateSync; RpcServers naming consistent with core; docs conforming). Build/vet/go test ./sdk/... (incl. new TestRenderNode_StateSync + validate-floor cases) green.

Note: consciously duplicates seictl's workflow client pending the future sdk/sei consolidation.

🤖 Generated with Claude Code

…lowStateSync

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 <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 10, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Additive SDK API and validation; integration test is longer and stricter but does not change controller runtime behavior. Misconfigured witnesses fail at create time rather than silently falling back to genesis sync.

Overview
Adds SDK support for CometBFT state-sync nodes and readiness to tell snapshot bootstrap from genesis replay, then rewires the nightly integration test to prove a real wipe + re-sync round trip.

SDK: NodeSpec.StateSync (RpcServers as bare host:port) maps in k8s renderNode to fullNode.snapshot.stateSync + rpcServers; nil leaves genesis block-sync unchanged. validateNodeSpec rejects witness sets with fewer than two distinct entries (aligned with CRD MinItems=2 / controller dedup). EarliestHeight reads earliest_block_height from /status (parallel to LatestHeight).

Integration: TestWorkflowStateSync waits past storage.snapshot_interval, creates a dedicated state-sync follower with two distinct witnesses (validator-0 + provisioned RPC via nodeRPC), asserts earliest > 1, runs the StateSync workflow on that node (nil recipe RpcServers so the planner reuses resolved witnesses), then asserts Complete, caught-up/EVM, and a block-store base jump of at least one snapshot interval after wipe—so a no-op resync cannot pass.

Reviewed by Cursor Bugbot for commit 8819fe4. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 7831156. Configure here.

Comment thread test/integration/workflow_test.go Outdated
Comment thread test/integration/workflow_test.go
Comment thread test/integration/workflow_test.go Outdated
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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Earliest advance asserted without new snapshot

Medium Severity

The post-workflow check requires earliest to rise past the pre-wipe floor, but nothing waits for another snapshot_interval of blocks after the first bootstrap. If the chain has not crossed the next snapshot boundary before re-sync, the follower restores the same snapshot and earliest stays unchanged, failing a genuine wipe+resync.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7831156. Configure here.

…itness set, base-pin

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 <noreply@anthropic.com>
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.

1 participant