Skip to content

feat(seinode): add a freeze sub-spec for fullNode and archive nodes - #508

Merged
bdchatham merged 7 commits into
mainfrom
feat/freeze-node
Aug 27, 2026
Merged

feat(seinode): add a freeze sub-spec for fullNode and archive nodes#508
bdchatham merged 7 commits into
mainfrom
feat/freeze-node

Conversation

@bdchatham

Copy link
Copy Markdown
Collaborator

Makes a node held at a block height a declared configuration. Spec: specs/freeze-node/spec.md.

Stacked on #507 (the sei-config v0.0.27 bump). Review that one first; this PR retargets to main when it merges.

What

A freeze sub-spec on the fullNode and archive modes:

spec:
  archive:
    freeze:
      height: 5000000

The node executes to that height, stops, and serves query RPC there indefinitely.

Why not spec.overrides

The key is reachable through spec.overrides since v0.0.27, and that route is wrong for two reasons.

The controller cannot derive its own behaviour from an opaque string map. A frozen node needs a different readiness probe. Making readinessProbeForNode parse spec.overrides would be fragile and semantically wrong.

mergeOverrides copies user overrides last, so a user key silently outranks a controller-derived one. Only one source can stay writable.

Peer review

Three specialists reviewed this independently: kubernetes-specialist, systems-engineer, platform-engineer. Two reproduced the top finding against a live 1.34 API server. The second commit is the response.

The defect they found

The field-level self == oldSelf on height was insufficient. CEL skips a rule reading oldSelf when the path is absent from the stored object, so adding freeze to an existing node and removing it from a frozen one both passed admission.

Adding it was the dangerous half:

  1. Admission accepts.
  2. reconcileStatefulSet runs an unconditional SSA on every reconcile, so the pod template gets the frozen probe immediately.
  3. chain.freeze_height never arrives — only TaskConfigApply carries a ConfigIntent, and it appears only in bootstrap programs. A Running node's plan writes config.toml [p2p] keys alone.
  4. UpdateStrategy: OnDelete leaves the live pod untouched, so nothing looks wrong.
  5. Weeks later an image bump or a Karpenter consolidation replaces the pod. It returns as an ordinary chain-following RPC node with lag-based readiness removed, and reports Ready however far behind it falls.

Removing freeze is the mirror failure: app.toml keeps the height while the probe reverts to /lag_status, so the node goes NotReady permanently at the next pod replacement. The operator's obvious remedy for a mistake is the action that breaks it.

FN-3a — has(self.freeze) == has(oldSelf.freeze) on the mode sub-spec, where has() is observable on both sides — makes both states unreachable.

A model I had wrong

I originally wrote that a frozen node's lag "grows without bound". It does not. Lag is a constant: block sync hands off at the freeze height and cancels the peer-update routines, but the pool is never cleared, so it keeps returning the tip it last observed. For a historical freeze height that constant sits far above the default threshold of 300, so /lag_status returns HTTP 417 permanently and the pod never becomes Ready.

FN-8 is therefore a fix for a real first-bootstrap outage, not the hardening I first called it. Freeze and Autobahn also cannot coexist (sei-tendermint/node/node.go:164), so my earlier giga reasoning was unreachable. The wrong model has been corrected in the code, the tests, and the spec.

Validation, all at admission

ID Rule
FN-3 freeze.height is immutable
FN-3a freeze is create-only: no add, no remove
FN-5 chain.freeze_height is rejected in spec.overrides
FN-6 freeze excludes snapshotGeneration
FN-10 freeze excludes chain.halt_height / chain.halt_time — sei-config refuses to resolve the combination
FN-11 snapshot.s3.targetHeight must be below freeze.height — seid refuses to start once a store has reached it
FN-12 freeze excludes snapshot.stateSync — seid silently falls back to block sync from genesis

FN-10 through FN-12 came out of review. Each one turns a failure that happened on the node into a rejection at kubectl apply.

Controller changes

  • controllerOverrides in both planners emits chain.freeze_height (FN-7).
  • readinessProbeForNode uses HTTP /status for a frozen node (FN-8). A bare TCP connect succeeds while the listener is saturated, because seid wraps it in a LimitListener that stops accepting rather than refusing.
  • ResourceLabels stamps sei.io/frozen: "true" (FN-13). NodeFellBehind selects on sei.io/role and would fire forever without resolving once the chain advanced past its threshold beyond the freeze height.

Merge preconditions

Run this first. FN-5 and FN-10 narrow validation on an existing served field, which the spec's own NFR-1 forbids. A reviewer reproduced the consequence: a stored SeiNode already carrying the key becomes un-updatable for any spec edit, and SeiNetworkReconciler performs full updates on children, so such an object would error every loop.

kubectl get seinodes -A -o json | jq -r '.items[]
  | select(.spec.overrides["chain.freeze_height"] != null
        or .spec.overrides["chain.halt_height"]  != null
        or .spec.overrides["chain.halt_time"]    != null)
  | "\(.metadata.namespace)/\(.metadata.name)"'

kubectl get seinetworks -A -o json | jq -r '.items[]
  | select(.spec.configOverrides["chain.freeze_height"] != null)
  | "\(.metadata.namespace)/\(.metadata.name)"'

Empty output on every cell retires the concern. Non-empty output is a blocker.

A frozen node's manifest MUST omit kustomize.toolkit.fluxcd.io/force: enabled. Every prod SeiNode carries it today. Flux's IsImmutableError returns true for any 422 Invalid — what every CEL rejection returns — and the force annotation then makes it delete and recreate. Nothing enforces sei.io/deletion-protected: no policy, no webhook, no controller reads it. So a git edit to freeze.height triggers a delete, and the controller removes the data PVC unless the operator imported the volume. The frozen state is the product.

The spec's Platform preconditions section carries this and three more, all found by reading the platform repo.

Testing

Check Result
go test ./... all three modules pass
envtest — 13 freeze cases pass
envtest — full suite (node, seinetwork, nodetask) pass
go vet ./... clean
gofmt -l . clean
staticcheck ./... 13 findings, same 13 as main — none introduced
make manifests generate idempotent
vale specs/freeze-node/spec.md 0 errors
Rendered StatefulSet for 7 unfrozen shapes byte-identical to main (FN-9 / SC-5)

Every requirement in the spec's traceability table has a test that runs. The one criterion left open is SC-3 — a frozen node holding Ready on a real cluster past its freeze height.

Deliberately deferred

  • A ValidateFrozenProbes render guard and a FrozenReady condition. All three reviewers ranked these cuttable, and FN-3a removes the silent-failure state that made the condition urgent.
  • An exec probe comparing /status's height against freeze.height - 1. /status does not prove the node reached its height, so it reads Ready during the initial block sync. /status is still strictly better than the TCP probe it replaced, at the same cost. Un-defer condition is in the spec (Q-2).
  • A mirror guard on SeiNetwork.spec.configOverrides (D-5), and a Freeze field on sdk/sei. Both recorded in the spec.

🤖 Generated with Claude Code

bdchatham and others added 4 commits August 27, 2026 13:36
v0.0.27 carries chain.freeze_height (sei-config#49, released in #50). Until the
pin moves, the sidecar's override resolution rejects the key as unknown, so a
SeiNode cannot set a freeze height declaratively.

All three modules move together so the schema stays consistent across the
controller, the sidecar that resolves overrides, and the shared API types.

Pins only; no source change. All three modules build and test clean.
Specifies a freeze sub-spec on the fullNode and archive modes, replacing the
spec.overrides route. Two reasons overrides cannot serve: the controller cannot
derive its own readiness probe from an opaque string map, and mergeOverrides
copies user overrides last, so a user key silently outranks a controller one.

Records the four decisions that are one-way doors once an operator depends on
them, and leaves them open for approval.

Records Q-1 honestly: whether /lag_status actually fails on a frozen node is
unsettled. On the giga path the reactor carries no syncer, so lag reads 0 and
the probe passes. The non-giga path is unresolved. FN-8 makes readiness
deterministic either way and should not be sold as fixing a proven outage.

vale: 0 errors, 0 warnings.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Implements specs/freeze-node/spec.md.

A frozen node runs seid with a freeze height: it executes to that height, stops,
and serves query RPC there indefinitely. spec.overrides cannot express this. The
controller has to derive the node's readiness probe from the intent, and it
cannot read an opaque string map to do so. mergeOverrides also copies user
overrides last, so a user key would outrank a controller-derived one.

FreezeSpec carries an immutable int64 height, matching every other height in
this CRD. It sits on fullNode and archive only: seid refuses to freeze a
validator, and a seed serves no query RPC. Which history a frozen node serves
follows from its mode, because pruning stops when the node stops.

CEL enforces three invariants at admission: the height is immutable, overrides
cannot carry chain.freeze_height, and freeze excludes snapshotGeneration on both
modes.

readinessProbeForNode now targets the RPC listener for a frozen node instead of
/lag_status. A frozen node's lag grows without bound, so the lag probe would
eventually pull the pod from its Service endpoints while seid served correctly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ations

Refines the freeze sub-spec against three independent peer reviews. Two
reviewers reproduced the top finding against a live 1.34 API server.

The field-level 'self == oldSelf' on height was insufficient. CEL skips a rule
that reads oldSelf when the path is absent from the stored object, so adding
freeze to an existing node and removing it from a frozen one both passed
admission. Adding it was the worse half: the unconditional StatefulSet apply
stamps the frozen probe immediately, while only the bootstrap path carries a
ConfigIntent, so the height never reaches app.toml. The node then presented as
frozen and behaved as an ordinary RPC node with lag-based readiness removed.
A presence rule on the mode sub-spec closes both, because has() is observable
there on both sides of the transition.

Three combinations that seid refuses or silently degrades now fail at admission
instead of on the node: a halt key alongside freeze, a snapshot target at or
above the freeze height, and state sync under freeze.

The readiness probe moves from a TCP connect to HTTP /status. A bare connect
succeeds while the listener is saturated, because seid wraps it in a
LimitListener that stops accepting rather than refusing.

Corrects the lag model recorded in the code, the tests, and the spec. Lag on a
frozen node is a constant, not a growing quantity: block sync hands off at the
freeze height and the peer pool keeps the tip it last observed. Freeze and
Autobahn also cannot coexist, so the giga reasoning in the original Q-1 was
unreachable.

Adds sei.io/frozen to the pod template. NodeFellBehind selects on sei.io/role
and would fire forever without resolving once the chain advanced past its
threshold beyond the freeze height.

Also fixes two doc comments the first pass misattached, and adds the test that
pins chain.freeze_height to sei-config's registry rather than only to the CEL
literal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cursor

cursor Bot commented Aug 27, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes CRD validation and pod readiness/labels for a new operational mode; existing SeiNodes with forbidden override keys may become un-updatable until cleaned up.

Overview
Adds a declared freeze sub-spec on fullNode and archive so operators pin a node at a block height (execute through height-1, then serve query RPC there) instead of using spec.overrides for chain.freeze_height.

API / admission: Introduces FreezeSpec with immutable height, SeiNodeSpec.Freeze(), and extensive CEL on the CRD—freeze is create-only (including across mode switches), mutually exclusive with snapshot generation, state sync, and halt overrides, and blocks chain.freeze_height in overrides.

Controller: Full/archive planners emit chain.freeze_height via freezeOverrides on bootstrap; frozen pods use HTTP /status readiness (not /lag_status, whose lag never shrinks) and get sei.io/frozen: "true so height-based alerts can skip them. Bumps sei-config to v0.0.27.

Tests: envtest admission suite for freeze rules, planner override/key pinning tests, and readiness/label unit tests. Unfrozen node manifests stay unchanged.

Reviewed by Cursor Bugbot for commit d74d5ff. 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 1 potential issue.

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 bca4c31. Configure here.

Comment thread api/v1alpha1/full_node_types.go
bdchatham and others added 2 commits August 27, 2026 15:08
This repository has no specs/ directory on main, so the file introduced a
convention rather than following one. The requirement IDs it defined resolved
only to that document, which made the test comments cite something a reader of
this repo could not open.

The test comments now state the invariant and the reason directly. Nothing that
guided the implementation is lost: the reasoning lives in the PR description,
and the tests are the executable form of it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses the Bugbot finding on #508, and a second hole the same mechanism
opens that the report did not name.

The per-mode presence rule reads oldSelf on fullNode/archive, which are
themselves optional. CEL skips a transition rule when its path is absent from
the stored object, so the rule never fires for a mode switch: swapping an
unfrozen fullNode for a frozen archive was admitted. The height rule has the
same gap on the destination path, so swapping between two frozen modes could
also change the height.

Both leave a Running node carrying the frozen readiness probe with no
chain.freeze_height in app.toml, which is the split the create-only rule exists
to make unreachable.

The new rule lives on SeiNodeSpec, where self and oldSelf always exist, and
reduces both freeze-capable modes to a single effective height (0 when
unfrozen). One expression now covers add, remove, change, and every mode
switch. The per-mode rules stay for their clearer messages in the common case.

A mode switch that leaves the node unfrozen is still permitted; a test pins
that so the rule cannot grow over-broad unnoticed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bdchatham
bdchatham changed the base branch from chore/sei-config-v0.0.27 to main August 27, 2026 22:20
@bdchatham bdchatham closed this Aug 27, 2026
@bdchatham bdchatham reopened this Aug 27, 2026
goconst flagged "archive" and "validator" as repeated literals that duplicate
roleArchive and roleValidator. Those constants are pod label values for
sei.io/role, so borrowing them for a subtest name would couple the test's
display text to a wire value.

The subtests now read "archive node" and "validator node", matching the
"full node" case already beside them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bdchatham
bdchatham merged commit bd9a558 into main Aug 27, 2026
9 checks passed
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