fix(sei-tendermint): make empty validator-set proto round-trip lossless so state-sync can boot on gentx chains

[bdchatham]

Problem

State-sync on a chain whose genesis carries no validators (the standard collect-gentxs launch shape — validators arrive at InitChain) crash-loops deterministically on first boot and permanently wedges the data dir. Field case + full trace: sei-k8s-controller#457.

Error: error starting node: cannot load state: fromProto: validatorSet proposer error: nil validator

then every restart, earlier:

Error: error creating node: LoadStateFromDBOrGenesisDocProvider(): fromProto: validatorSet proposer error: nil validator

Root cause is a broken proto round-trip in types: ValidatorSet.ToProto short-circuits IsNilOrEmpty to an empty proto, but ValidatorSetFromProto only handles nil and falls through to a nil-proposer error — FromProto(ToProto(empty)) fails by construction. The node persists the genesis-derived state at construction (LoadStateFromDBOrGenesisDocProvider saves it "so its fetchable by other callers"); with state sync enabled InitChain is skipped, so that state carries the empty set, and OnStart's stateStore.Load() dies re-reading what the process just wrote. Chains whose genesis embeds validators (all canonical networks) never hit this — only gentx-launch chains do. Two existing tests pinned the broken behavior as the contract, which is how it survived.

Fix (3 files, ~20 production lines)

1. types/validator_set.go — ValidatorSetFromProto mirrors ToProto's short-circuit: a non-nil empty proto canonicalizes to NewValidatorSet(nil) — the exact value MakeGenesisState produces for a validator-less genesis (and the established representation in sm.FromProto's height<1 LastValidators branch). Nil still errors.
2. Emptiness stays rejected everywhere it is illegitimate:
   • types/light.go LightBlockFromProto — rejects empty sets at decode with ErrValidatorSetEmpty, preserving its exact prior contract (covered by the existing TestLightBlockProtobuf empty row). The other untrusted consumers are sei-ibc-go's 07-tendermint client (six decode sites in header/update/misbehaviour — caught in review by seidroid); those now go through a package-local validatorSetFromProto that rejects empty sets the same way. state.go and store.go LoadValidators are trusted local-store loads. Untrusted-surface delta: zero.
   • internal/state/state.go FromProto — refuses empty Validators/NextValidators past LastBlockHeight 0: for a post-genesis state that can only be corruption, and a clean load error beats the consensus-time IncrementProposerPriority panic it would otherwise defer to.
   • internal/state/store.go LoadValidators — refuses a decoded empty set (both the checkpoint path, which would panic in IncrementProposerPriority, and the exact-height return).

Review notes (for reviewers)

• ValidatorSetFromProto is exported and its contract changes for one input: non-nil empty proto now canonicalizes instead of erroring. All in-fork callers audited; the one untrusted consumer re-tightens at decode.
• Two load-bearing facts a future refactor should not disturb: (a) consensus updateStateFromStore runs unconditionally at reactor OnStart (not gated on waitSync) and executes updateToState on the empty genesis state during a state-sync boot — safe because NewVoteSet tolerates Size()==0 and proposer derefs live in the stepping code behind readySignal; (b) State.IsEmpty() keys on Validators == nil, and the canonicalized empty set is non-nil, which is why the IsEmpty short-circuit does not fire there.

Testing

• New regressions at all three layers: valset proto round-trip, sm.State round-trip (+ past-genesis rejection), state-store Save/Load of a validator-less genesis state — the store test fails on main with the exact production error and passes here; plus LoadValidators refusal of the genesis empty entry.
• ./types, ./internal/state, ./light/..., ./internal/statesync, ./internal/consensus, ./internal/evidence all pass; ./node has 3 pre-existing environmental failures identical on clean main.
• Reviewed by systems-engineer (verdict: sound; the height-bounded rejection is its refinement) and idiomatic-reviewer (verdict: reads native).

Fixes the seid half of sei-k8s-controller#457 (the controller-side gate guard is tracked there).

🤖 Generated with Claude Code

[cursor]

PR Summary

Medium Risk
Touches consensus state load, validator-set proto semantics, and IBC light-client decode paths; behavior change is intentional and bounded by height/context, with regressions at each layer.

Overview
Fixes state-sync crash-loops on chains whose genesis has no embedded validators (gentx / InitChain path): persisted empty validator sets could not be reloaded because ValidatorSetFromProto failed after ToProto's empty short-circuit.

ValidatorSetFromProto now mirrors ToProto by canonicalizing a non-nil empty proto to NewValidatorSet(nil), so genesis-derived state round-trips through the DB.

Untrusted paths still reject empty sets: LightBlockFromProto returns ErrValidatorSetEmpty; IBC 07-tendermint routes header/update/misbehaviour decodes through a package-local validatorSetFromProto that re-applies that rule.

Trusted load backstops: state.FromProto and store.LoadValidators return ErrEmptyValidatorSet when sets are empty past genesis (or before real validators exist), avoiding IncrementProposerPriority panics on corruption or pre-InitChain store entries.

Tests cover empty proto round-trip, past-genesis rejection, and save/load of validator-less genesis state.

^{Reviewed by Cursor Bugbot for commit 8f24d7c. Bugbot is set up for automated code reviews on this repo. Configure here.}

[github-actions]

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	✅ passed	✅ passed	✅ passed	Jul 8, 2026, 6:51 PM

[codecov]

Codecov Report

❌ Patch coverage is 77.77778% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.62%. Comparing base (c6ab0be) to head (8f24d7c).

Files with missing lines	Patch %	Lines
...odules/light-clients/07-tendermint/types/header.go	75.00%	1 Missing and 1 partial ⚠️
sei-tendermint/internal/state/errors.go	0.00%	2 Missing ⚠️
sei-tendermint/internal/state/store.go	50.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3724      +/-   ##
==========================================
- Coverage   59.85%   58.62%   -1.23%     
==========================================
  Files        2278     2188      -90     
  Lines      189140   178436   -10704     
==========================================
- Hits       113202   104612    -8590     
+ Misses      65861    64547    -1314     
+ Partials    10077     9277     -800     

Flag	Coverage Δ
sei-chain-pr	80.57% <77.77%> (?)
sei-db	70.41% <ø> (ø)
sei-db-state-db	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
.../light-clients/07-tendermint/types/misbehaviour.go	80.30% <100.00%> (ø)
...clients/07-tendermint/types/misbehaviour_handle.go	85.45% <100.00%> (ø)
...odules/light-clients/07-tendermint/types/update.go	85.59% <100.00%> (ø)
sei-tendermint/internal/state/state.go	83.23% <100.00%> (+0.41%)	⬆️
sei-tendermint/types/light.go	78.84% <100.00%> (+0.41%)	⬆️
sei-tendermint/types/validator_set.go	93.22% <100.00%> (+0.03%)	⬆️
...odules/light-clients/07-tendermint/types/header.go	95.00% <75.00%> (-5.00%)	⬇️
sei-tendermint/internal/state/errors.go	5.40% <0.00%> (-0.31%)	⬇️
sei-tendermint/internal/state/store.go	61.76% <50.00%> (-0.16%)	⬇️

... and 148 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[seidroid]

A focused, well-tested fix that makes the empty-validator-set proto round-trip lossless so state-sync can boot on gentx-launch chains, with empty-set rejection re-tightened at the trusted load paths and the light-block decode. The change is sound; the one substantive note is that the PR's audit under-counts untrusted callers of the relaxed ValidatorSetFromProto (sei-ibc-go's 07-tendermint client), though downstream signature verification keeps that path safe.

Findings: 0 blocking | 4 non-blocking | 1 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• Audit-claim gap (defense-in-depth): The PR states light.go's LightBlockFromProto is "the only untrusted-input caller" of ValidatorSetFromProto, but sei-ibc-go/modules/light-clients/07-tendermint/types/{header,update,misbehaviour,misbehaviour_handle}.go also decode untrusted validator sets via this function. As of this PR, Header.ValidateBasic (header.go:74) will accept an empty proto set whose hash matches Header.ValidatorsHash, whereas it previously errored at decode. This is NOT exploitable — checkValidity→light.Verify still rejects an empty untrusted set (VerifyCommitLight fails at validation.go:270, and adjacent verification requires a non-empty hash match) — so I disagree with Codex's P2/exploitable severity. But to preserve the prior contract and match the PR's own stated principle of re-tightening untrusted surfaces at decode, consider rejecting empty sets in the IBC header decode path (or in Header.ValidateBasic) as well.
• Second-opinion inputs: cursor-review.md and REVIEW_GUIDELINES.md are both empty (no Cursor output / no repo-specific guidelines to apply). Codex produced one finding, addressed above.
• The three new corruption backstops (state.go past-genesis rejection, store.go LoadValidators empty-set rejection) are strict improvements over the prior downstream IncrementProposerPriority panic; no concerns.
• 1 suggestion(s)/nit(s) flagged inline on specific lines.

[seidroid]

A well-scoped, defensive fix making the empty validator-set proto round-trip lossless so gentx-launch chains can boot via state sync. The core ValidatorSetFromProto change is correct and every downstream caller that must still reject empty sets re-tightens at decode; regressions cover all three layers. No blocking issues found.

Findings: 0 blocking | 3 non-blocking | 0 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• Cursor's second-opinion review (cursor-review.md) is empty — that pass produced no output. Codex (codex-review.md) reported no material findings but noted it could not run the Go tests (runner had Go 1.24.13 vs. required 1.25.6) or goimports; test verification was not independently performed by the automated reviewers.
• Verified the state-transition invariant by hand: after applying a block, LastValidators = state.Validators.Copy(), so no legitimate state at LastBlockHeight >= 1 has an empty LastValidators and the new state.go backstop cannot produce a false-positive rejection. Worth a maintainer sanity-check given the pre-existing (and somewhat misleading) // At Block 1 LastValidators is nil comment.
• The behavioral contract of the exported ValidatorSetFromProto changes for one input (non-nil empty proto now canonicalizes rather than erroring). This is well-documented in code and PR, and all in-fork callers were audited, but any external/downstream consumers of this fork's types package that relied on the old error should be aware.

[seidroid]

The automated review did not complete; see the failing AI Review check for details.

[seidroid]

A focused, correct fix that makes the empty validator-set proto round-trip lossless so state-sync can boot on gentx-launched chains, while re-tightening every untrusted decode path to keep rejecting empty sets. The change is internally consistent, preserves prior error contracts, and is well covered by new regression tests; no blocking issues found.

Findings: 0 blocking | 4 non-blocking | 0 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• Verified: all 9 in-tree callers of ValidatorSetFromProto are audited. Untrusted surfaces (types/light.go and the six sei-ibc-go 07-tendermint decode sites via the new package-local validatorSetFromProto) re-reject empty sets with ErrValidatorSetEmpty, so FromProto(empty) still yields an error there — outcome-equivalent to the old nil-proposer error, just cleaner. Trusted local-store loads (state.go/store.go) gain explicit empty-set backstops. Net untrusted-surface delta is zero, as the PR claims.
• Round-trip symmetry confirmed: ToProto short-circuits IsNilOrEmpty() to &tmproto.ValidatorSet{}; the new FromProto guard len(vp.Validators)==0 && vp.Proposer==nil matches exactly, and NewValidatorSet(nil) is the same value MakeGenesisState produces, so no data is silently dropped (a maliciously-set TotalVotingPower on an otherwise-empty proto is correctly discarded to 0).
• The Cursor second-opinion review file (cursor-review.md) was empty — that pass produced no output. The Codex pass (codex-review.md) reported no material findings but noted it could not run the focused Go tests (sandbox had Go 1.24.13 vs required 1.25.6, no network for toolchain download), so its assessment is static-only.
• Minor: the comment in validator_set.go says untrusted callers reject empty sets "via ValidateBasic (ErrValidatorSetEmpty)", but the actual untrusted callers (light.go, ibc wrapper) reject via an explicit IsNilOrEmpty() check returning ErrValidatorSetEmpty, not via ValidateBasic. The behavior is correct; the comment's mechanism reference is slightly imprecise.