Skip to content

feat(genesis): add Vesting field to SeiNetwork's GenesisAccount - #496

Merged
bdchatham merged 4 commits into
mainfrom
feat/vesting-genesis-accounts
Jul 23, 2026
Merged

feat(genesis): add Vesting field to SeiNetwork's GenesisAccount#496
bdchatham merged 4 commits into
mainfrom
feat/vesting-genesis-accounts

Conversation

@bdchatham

Copy link
Copy Markdown
Collaborator

Summary

  • Adds an optional Vesting field (nested GenesisAccountVesting: amount, endTime, delayed) to SeiNetwork.spec.genesis.accounts[]. nil/absent is unchanged behavior — every existing caller (e.g. admin/alice/bob funding via the qa-testing nightly harness) is unaffected.
  • Threads the field through every mirror point: the CRD type (api/v1alpha1/seinetwork_types.go, regenerated deepcopy + CRD YAML via make generate manifests), the plain Go SDK type (sdk/sei/spec.go), the k8s-provider render step (sdk/sei/provider/k8s/render.go), and the planner's mapping into the sidecar task request (internal/planner/group.go).
  • Restores a coverage path lost when sei-chain#3714 deprecated live MsgCreateVestingAccount: constructing the vesting account directly at genesis-assembly time sidesteps that deprecation entirely (see companion PR below).

⚠️ Blocked — draft until sei-protocol/seictl#238 merges and tags

This PR is code-complete and fully tested locally (built + tested against seictl's branch via a temporary, uncommitted go.mod replace pointing at a local checkout — never pushed), but go.mod still pins github.com/sei-protocol/seictl v0.0.64, which doesn't have the new GenesisAccountVesting wire type / Vesting field that seictl#238 adds. CI on this PR is expected to fail to build until that dependency is bumped.

Attempted to pre-pin to seictl#238's exact commit via a pseudo-version (go get github.com/sei-protocol/seictl@<sha>) so this PR could build standalone, but that surfaced an unrelated, pre-existing gogo/protobuf@v1.3.3 module-resolution error (unknown revision v1.3.3) once MVS re-resolved transitive versions — not something introduced by this change, and not worth fixing as a side quest here.

Next step once seictl#238 merges and cuts a tagged release: go get github.com/sei-protocol/seictl@v0.0.6X && go mod tidy, confirm clean, then this PR is ready for real review/merge.

Test plan

  • go build ./... clean (with local seictl replace)
  • go vet ./..., gofmt -l . clean
  • go test ./... (excluding e2e) passes, including two new tests: TestBuildPlan_PropagatesVesting (planner → sidecar task JSON round-trip) and TestRenderNetwork_PropagatesVesting (SDK spec → CRD render), both also asserting a nil-Vesting sibling account stays nil, not zero-valued
  • make test-integration (full envtest suite against a live API server, validating the actual generated CRD schema) passes
  • Full build against the real (non-local) seictl dependency — blocked, see above

🤖 Generated with Claude Code

Adds an optional Vesting field (amount, end-time, delayed) to
spec.genesis.accounts[] so a genesis-time account can be seeded under
a vesting schedule instead of as a plain account; nil (the default)
is unchanged. Threads through the SDK's plain GenesisAccount mirror,
the k8s-provider render step, and the planner's mapping into the
sidecar's assemble-and-upload-genesis task params.

Restores a coverage path lost when sei-chain#3714 deprecated live
MsgCreateVestingAccount: constructing the vesting account directly at
genesis-assembly time sidesteps that deprecation entirely, since it
never goes through the msg-server tx path.

Depends on sei-protocol/seictl#<vesting-genesis-accounts PR>, which
adds the matching GenesisAccountVesting wire type and the actual
account-construction logic. This PR is code-complete and fully tested
locally against that branch (via a temporary go.mod replace, not
committed), but go.mod still pins seictl v0.0.64 — bumping it to a
version containing the new field is a required follow-up once that PR
merges and tags, not before. CI on this PR is expected red until then.
…names

Addresses xreview idiom finding (S1): the GenesisAccount.Vesting field
doc becomes operator-facing via kubectl explain and the rendered CRD
description, but named internal sei-chain implementation symbols
(ContinuousVestingAccount, DelayedVestingAccount, MsgCreateVestingAccount)
that no other field doc in GenesisCeremonyConfig references. Rewritten to
describe what the operator gets (locked coins that count toward balance,
can be staked, unlock linearly or all-at-once at EndTime); the
type-mapping provenance stays at the seictl construction site where it
belongs. Regenerated CRD manifests. No schema/deepcopy change.
@bdchatham

Copy link
Copy Markdown
Collaborator Author

xreview findings (5 independent reviewers: kubernetes-specialist [dissenter], solidity-developer, systems-engineer, security-specialist, idiomatic-reviewer)

Ledger verdict: RESOLVED — every correctness-grade finding fixed or explicitly accepted. Fixes pushed to both PRs. Summary of what changed and what's deliberately deferred:

Fixed in code (seictl#238 @ 51396b5, this PR @ 42bf5d2)

  • Degenerate-schedule silent-unlock (3 reviewers converged — highest-signal finding). EndTime > 0 alone didn't guarantee a locked fixture: a continuous account with EndTime <= genesisTime, or a delayed account with a past EndTime, is fully unlocked from block 1 with no error (auth.InitGenesis never calls acc.Validate()). Now rejected at assembly time: non-zero amount, EndTime > genesisTime, and a belt-and-suspenders acc.Validate(). Tests added for all three paths.
  • Wire-mapping test gap (dissenter + idiom). genesisAccountsToWire's hand-built map had no vesting coverage; added a JSON round-trip test proving the CLI map and the server struct agree on every key.
  • CRD doc leaked internal type names (idiom S1). kubectl explain-facing text now describes behavior, not ContinuousVestingAccount/MsgCreateVestingAccount.

Deliberately accepted / deferred (with rationale)

  • ⚠️ Sidecar-image skew silent-drop (dissenter, strongest finding — ACCEPTED via the lockstep constraint, not a code guard). A controller running this PR's code against a pre-chore(config): bump default manager image to 0d3a156 #238 sidecar image would have its vesting field silently dropped by encoding/json (unknown fields ignored) → a plain unlocked account, no error. This is inherent to the additive-field-over-HTTP model and is why this PR is blocked on seictl#238 merging + tagging + a lockstep SEITASK_IMAGE/sidecar-image bump (already the stated gating condition). Adding DisallowUnknownFields server-side would convert the silent drop into a loud error but is a separate hardening decision with its own blast radius (it would reject any forward-compatible field on the whole task API, not just this one) — flagged for a maintainer call, not bundled here.
  • Non-atomic genesis.json write (systems-engineer). SaveAs truncates-then-writes with no temp+rename; pre-existing, shared by every genesis writer, not a regression from this change. Deferred.
  • Vesting-amount coin-validation is server-side, not planner-time (dissenter MISMATCH). Rejected as a finding: the client package deliberately avoids importing the full sei-cosmos SDK (documented at the validateSeiAccountAddress comment), and Balance itself is already validated server-side only — so this follows established precedent, not a regression. The client does now validate the vesting sub-fields it can cheaply check (non-empty amount, positive EndTime).

Full ledger committed at bdchatham-designs/designs/qa-testing-stack/xreview/.

…type

v0.0.68 ships seictl#238's GenesisAccountVesting field that this branch's
planner mapping and SDK depend on. Unblocks the previously-red CI (the
draft was pinned to v0.0.64, which lacked the field). Verified locally:
the seictl/sidecar/client-consuming packages build and the vesting
propagation tests (TestBuildPlan_PropagatesVesting,
TestRenderNetwork_PropagatesVesting) pass against the real v0.0.68.
CI's golangci-lint (--new-from-patch) flagged two goconst issues my new
tests introduced: reuse the existing testAccountBalance const for the
vesting amount in group_accounts_test.go, and add a shared testGenesisAddr
const for the repeated "sei1abc" fixture in render_test.go. Verified with
golangci-lint locally (the tool I'd skipped — ran go vet/gofmt only).
@bdchatham
bdchatham marked this pull request as ready for review July 23, 2026 15:03
@bdchatham
bdchatham merged commit 59e5c3a into main Jul 23, 2026
4 checks passed
@cursor

cursor Bot commented Jul 23, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes immutable genesis ceremony inputs and initial chain state; mistakes in vesting amounts or schedules are baked in at bootstrap, though absent vesting is unchanged.

Overview
Adds optional vesting on spec.genesis.accounts[] so non-validator genesis accounts can be funded with a lock schedule instead of only fully spendable balances. Each account may set vesting (amount, endTime, optional delayed for cliff vs linear unlock); omitting it keeps today’s behavior.

The field is wired through the CRD/API (GenesisAccount / GenesisAccountVesting, regenerated deepcopy and CRD YAML), the Go SDK (sdk/sei/spec.go), k8s render (renderNetwork), and the genesis planner into the sidecar AssembleAndUploadGenesisTask JSON. seictl is bumped to v0.0.68 for the matching sidecar wire types.

New tests assert vesting propagates in planner and render paths and that accounts without vesting stay nil, not zero-valued.

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

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