diff --git a/api/v1alpha1/seinodetask_types.go b/api/v1alpha1/seinodetask_types.go index 202ce48..1d05ebe 100644 --- a/api/v1alpha1/seinodetask_types.go +++ b/api/v1alpha1/seinodetask_types.go @@ -1,12 +1,13 @@ package v1alpha1 import ( + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // SeiNodeTaskKind discriminates the SeiNodeTask spec union. Exactly one of // the matching payload sub-structs in SeiNodeTaskSpec must be set. -// +kubebuilder:validation:Enum=GovSoftwareUpgrade;GovVote;AwaitCondition;UpdateNodeImage;AwaitNodesAtHeight;RestartSeid;MarkReady +// +kubebuilder:validation:Enum=GovSoftwareUpgrade;GovVote;GovParamChange;AwaitCondition;UpdateNodeImage;AwaitNodesAtHeight;RestartSeid;MarkReady type SeiNodeTaskKind string const ( @@ -21,6 +22,13 @@ const ( // proposalId/voter). SeiNodeTaskKindGovVote SeiNodeTaskKind = "GovVote" + // SeiNodeTaskKindGovParamChange backs the sidecar `gov-param-change` task. + // Submits MsgSubmitProposal wrapping a ParameterChangeProposal from the + // configured operator key. NOT chain-idempotent and — unlike a software + // upgrade — a param-change has no "applies once" safety net; see the + // REHYDRATION WARNING on the sidecar handler before composing with retries. + SeiNodeTaskKindGovParamChange SeiNodeTaskKind = "GovParamChange" + // SeiNodeTaskKindAwaitCondition backs the sidecar `await-condition` task. // Polls a local node until a typed condition (e.g. height) is satisfied, // optionally executing a post-condition action. @@ -105,9 +113,10 @@ const ( // Field names locked at v1alpha1 — see docs/design/seinode-task-lld.md // (PR sei-protocol/sei-k8s-controller#277). // -// +kubebuilder:validation:XValidation:rule="(has(self.govSoftwareUpgrade) ? 1 : 0) + (has(self.govVote) ? 1 : 0) + (has(self.awaitCondition) ? 1 : 0) + (has(self.updateNodeImage) ? 1 : 0) + (has(self.awaitNodesAtHeight) ? 1 : 0) + (has(self.restartSeid) ? 1 : 0) + (has(self.markReady) ? 1 : 0) == 1",message="exactly one of govSoftwareUpgrade, govVote, awaitCondition, updateNodeImage, awaitNodesAtHeight, restartSeid, or markReady must be set" +// +kubebuilder:validation:XValidation:rule="(has(self.govSoftwareUpgrade) ? 1 : 0) + (has(self.govVote) ? 1 : 0) + (has(self.govParamChange) ? 1 : 0) + (has(self.awaitCondition) ? 1 : 0) + (has(self.updateNodeImage) ? 1 : 0) + (has(self.awaitNodesAtHeight) ? 1 : 0) + (has(self.restartSeid) ? 1 : 0) + (has(self.markReady) ? 1 : 0) == 1",message="exactly one of govSoftwareUpgrade, govVote, govParamChange, awaitCondition, updateNodeImage, awaitNodesAtHeight, restartSeid, or markReady must be set" // +kubebuilder:validation:XValidation:rule="self.kind != 'GovSoftwareUpgrade' || has(self.govSoftwareUpgrade)",message="spec.govSoftwareUpgrade is required when kind=GovSoftwareUpgrade" // +kubebuilder:validation:XValidation:rule="self.kind != 'GovVote' || has(self.govVote)",message="spec.govVote is required when kind=GovVote" +// +kubebuilder:validation:XValidation:rule="self.kind != 'GovParamChange' || has(self.govParamChange)",message="spec.govParamChange is required when kind=GovParamChange" // +kubebuilder:validation:XValidation:rule="self.kind != 'AwaitCondition' || has(self.awaitCondition)",message="spec.awaitCondition is required when kind=AwaitCondition" // +kubebuilder:validation:XValidation:rule="self.kind != 'UpdateNodeImage' || has(self.updateNodeImage)",message="spec.updateNodeImage is required when kind=UpdateNodeImage" // +kubebuilder:validation:XValidation:rule="self.kind != 'AwaitNodesAtHeight' || has(self.awaitNodesAtHeight)",message="spec.awaitNodesAtHeight is required when kind=AwaitNodesAtHeight" @@ -144,6 +153,10 @@ type SeiNodeTaskSpec struct { // +optional GovVote *GovVotePayload `json:"govVote,omitempty"` + // GovParamChange is the payload for kind=GovParamChange. + // +optional + GovParamChange *GovParamChangePayload `json:"govParamChange,omitempty"` + // AwaitCondition is the payload for kind=AwaitCondition. // +optional AwaitCondition *AwaitConditionPayload `json:"awaitCondition,omitempty"` @@ -261,6 +274,78 @@ type GovSoftwareUpgradePayload struct { Gas uint64 `json:"gas"` } +// GovParamChangePayload mirrors +// seictl/sidecar/client.GovParamChangeTask. The sidecar handler enforces +// additional invariants (usei-only deposit, keyring presence) at submit +// time — see that file for the canonical validation order. +type GovParamChangePayload struct { + // ChainID is the chain ID the proposal targets. + // +kubebuilder:validation:MinLength=1 + ChainID string `json:"chainId"` + + // KeyName names the keyring entry that signs the proposal. Omit to let + // the controller derive from the target SeiNode: spec.validator. + // operatorKeyring.secret.keyName when .secret is set (defaulting to + // "node_admin"), otherwise "validator". + // +optional + // +kubebuilder:validation:Pattern=`^[a-zA-Z0-9_-]+$` + KeyName string `json:"keyName,omitempty"` + + // Title is the on-chain proposal title. + // +kubebuilder:validation:MinLength=1 + Title string `json:"title"` + + // Description is the on-chain proposal description. + // +kubebuilder:validation:MinLength=1 + Description string `json:"description"` + + // Changes is the non-empty set of parameter changes. + // +kubebuilder:validation:MinItems=1 + Changes []GovParamChangeEntry `json:"changes"` + + // InitialDeposit is the proposal deposit in coin notation (e.g. + // "10000000usei"). usei-only; the sidecar pre-validates. Must be >= + // the chain's min_deposit to enter the voting period rather than the + // deposit period. + // +kubebuilder:validation:MinLength=1 + InitialDeposit string `json:"initialDeposit"` + + // Memo is the optional tx memo. The sidecar appends a `taskID=` tag; + // do not pre-tag. + // +optional + Memo string `json:"memo,omitempty"` + + // Fees is the tx fee in coin notation (e.g. "8000usei"). usei-only, and + // must clear the target node's enforced minimum-gas-prices (e.g. + // 0.02usei/gas on arctic-1) or the tx is silently CheckTx-rejected. + // +kubebuilder:validation:MinLength=1 + Fees string `json:"fees"` + + // Gas is the tx gas limit. + // +kubebuilder:validation:Minimum=1 + Gas uint64 `json:"gas"` +} + +// GovParamChangeEntry is one (subspace, key, value) parameter change. +type GovParamChangeEntry struct { + // Subspace is the params subspace (e.g. "baseapp", "staking"). + // +kubebuilder:validation:MinLength=1 + Subspace string `json:"subspace"` + + // Key is the parameter key within the subspace (e.g. "TimeoutParams"). + // +kubebuilder:validation:MinLength=1 + Key string `json:"key"` + + // Value is the new parameter value as JSON of whatever shape the + // param's registered type expects — a scalar (100), a string + // ("86400000000000"), a bool, or an object. Pass it as structured JSON, + // NOT a pre-escaped string: the controller forwards the raw bytes and + // the sidecar stringifies them exactly once, so an escaped string would + // double-encode and fail at apply. Integer-valued params must be JSON + // strings (Sei convention); see the sidecar handler for why. + Value apiextensionsv1.JSON `json:"value"` +} + // GovVotePayload mirrors seictl/sidecar/tasks/gov_vote.go::GovVoteRequest. type GovVotePayload struct { // ChainID is the chain ID the vote targets. @@ -448,6 +533,10 @@ type SeiNodeTaskOutputs struct { // +optional GovVote *GovVoteOutputs `json:"govVote,omitempty"` + // GovParamChange outputs for kind=GovParamChange. + // +optional + GovParamChange *GovParamChangeOutputs `json:"govParamChange,omitempty"` + // AwaitCondition outputs for kind=AwaitCondition. // +optional AwaitCondition *AwaitConditionOutputs `json:"awaitCondition,omitempty"` @@ -480,6 +569,26 @@ type GovSoftwareUpgradeOutputs struct { ProposalID uint64 `json:"proposalId,omitempty"` } +// GovParamChangeOutputs are the typed results for a completed +// GovParamChange task. Like the other sidecar-backed gov outputs, these +// fields are defined for forward compatibility but are NOT populated in +// this PR (the sidecar TaskResult has no typed-output channel; downstream +// consumers read the proposalId from the chain — see populateOutputs). +type GovParamChangeOutputs struct { + // TxHash is the upper-case hex-encoded transaction hash. + // +optional + TxHash string `json:"txHash,omitempty"` + + // Height is the block height at which the tx was included. + // +optional + Height int64 `json:"height,omitempty"` + + // ProposalID is the on-chain proposal ID parsed from the inclusion + // response events. + // +optional + ProposalID uint64 `json:"proposalId,omitempty"` +} + // GovVoteOutputs are the typed results for a completed GovVote task. type GovVoteOutputs struct { // TxHash is the upper-case hex-encoded transaction hash. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 70970c9..7ccad6a 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -290,6 +290,59 @@ func (in *GenesisCeremonyNodeConfig) DeepCopy() *GenesisCeremonyNodeConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GovParamChangeEntry) DeepCopyInto(out *GovParamChangeEntry) { + *out = *in + in.Value.DeepCopyInto(&out.Value) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GovParamChangeEntry. +func (in *GovParamChangeEntry) DeepCopy() *GovParamChangeEntry { + if in == nil { + return nil + } + out := new(GovParamChangeEntry) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GovParamChangeOutputs) DeepCopyInto(out *GovParamChangeOutputs) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GovParamChangeOutputs. +func (in *GovParamChangeOutputs) DeepCopy() *GovParamChangeOutputs { + if in == nil { + return nil + } + out := new(GovParamChangeOutputs) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GovParamChangePayload) DeepCopyInto(out *GovParamChangePayload) { + *out = *in + if in.Changes != nil { + in, out := &in.Changes, &out.Changes + *out = make([]GovParamChangeEntry, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GovParamChangePayload. +func (in *GovParamChangePayload) DeepCopy() *GovParamChangePayload { + if in == nil { + return nil + } + out := new(GovParamChangePayload) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GovSoftwareUpgradeOutputs) DeepCopyInto(out *GovSoftwareUpgradeOutputs) { *out = *in @@ -1249,6 +1302,11 @@ func (in *SeiNodeTaskOutputs) DeepCopyInto(out *SeiNodeTaskOutputs) { *out = new(GovVoteOutputs) **out = **in } + if in.GovParamChange != nil { + in, out := &in.GovParamChange, &out.GovParamChange + *out = new(GovParamChangeOutputs) + **out = **in + } if in.AwaitCondition != nil { in, out := &in.AwaitCondition, &out.AwaitCondition *out = new(AwaitConditionOutputs) @@ -1290,6 +1348,11 @@ func (in *SeiNodeTaskSpec) DeepCopyInto(out *SeiNodeTaskSpec) { *out = new(GovVotePayload) **out = **in } + if in.GovParamChange != nil { + in, out := &in.GovParamChange, &out.GovParamChange + *out = new(GovParamChangePayload) + (*in).DeepCopyInto(*out) + } if in.AwaitCondition != nil { in, out := &in.AwaitCondition, &out.AwaitCondition *out = new(AwaitConditionPayload) diff --git a/config/crd/sei.io_seinodetasks.yaml b/config/crd/sei.io_seinodetasks.yaml index 515cf48..51fc2ac 100644 --- a/config/crd/sei.io_seinodetasks.yaml +++ b/config/crd/sei.io_seinodetasks.yaml @@ -106,6 +106,96 @@ spec: required: - targetHeight type: object + govParamChange: + description: GovParamChange is the payload for kind=GovParamChange. + properties: + chainId: + description: ChainID is the chain ID the proposal targets. + minLength: 1 + type: string + changes: + description: Changes is the non-empty set of parameter changes. + items: + description: GovParamChangeEntry is one (subspace, key, value) + parameter change. + properties: + key: + description: Key is the parameter key within the subspace + (e.g. "TimeoutParams"). + minLength: 1 + type: string + subspace: + description: Subspace is the params subspace (e.g. "baseapp", + "staking"). + minLength: 1 + type: string + value: + description: |- + Value is the new parameter value as JSON of whatever shape the + param's registered type expects — a scalar (100), a string + ("86400000000000"), a bool, or an object. Pass it as structured JSON, + NOT a pre-escaped string: the controller forwards the raw bytes and + the sidecar stringifies them exactly once, so an escaped string would + double-encode and fail at apply. Integer-valued params must be JSON + strings (Sei convention); see the sidecar handler for why. + x-kubernetes-preserve-unknown-fields: true + required: + - key + - subspace + - value + type: object + minItems: 1 + type: array + description: + description: Description is the on-chain proposal description. + minLength: 1 + type: string + fees: + description: |- + Fees is the tx fee in coin notation (e.g. "8000usei"). usei-only, and + must clear the target node's enforced minimum-gas-prices (e.g. + 0.02usei/gas on arctic-1) or the tx is silently CheckTx-rejected. + minLength: 1 + type: string + gas: + description: Gas is the tx gas limit. + format: int64 + minimum: 1 + type: integer + initialDeposit: + description: |- + InitialDeposit is the proposal deposit in coin notation (e.g. + "10000000usei"). usei-only; the sidecar pre-validates. Must be >= + the chain's min_deposit to enter the voting period rather than the + deposit period. + minLength: 1 + type: string + keyName: + description: |- + KeyName names the keyring entry that signs the proposal. Omit to let + the controller derive from the target SeiNode: spec.validator. + operatorKeyring.secret.keyName when .secret is set (defaulting to + "node_admin"), otherwise "validator". + pattern: ^[a-zA-Z0-9_-]+$ + type: string + memo: + description: |- + Memo is the optional tx memo. The sidecar appends a `taskID=` tag; + do not pre-tag. + type: string + title: + description: Title is the on-chain proposal title. + minLength: 1 + type: string + required: + - chainId + - changes + - description + - fees + - gas + - initialDeposit + - title + type: object govSoftwareUpgrade: description: GovSoftwareUpgrade is the payload for kind=GovSoftwareUpgrade. properties: @@ -242,6 +332,7 @@ spec: enum: - GovSoftwareUpgrade - GovVote + - GovParamChange - AwaitCondition - UpdateNodeImage - AwaitNodesAtHeight @@ -323,17 +414,20 @@ spec: - target type: object x-kubernetes-validations: - - message: exactly one of govSoftwareUpgrade, govVote, awaitCondition, - updateNodeImage, awaitNodesAtHeight, restartSeid, or markReady must - be set + - message: exactly one of govSoftwareUpgrade, govVote, govParamChange, + awaitCondition, updateNodeImage, awaitNodesAtHeight, restartSeid, + or markReady must be set rule: '(has(self.govSoftwareUpgrade) ? 1 : 0) + (has(self.govVote) ? - 1 : 0) + (has(self.awaitCondition) ? 1 : 0) + (has(self.updateNodeImage) - ? 1 : 0) + (has(self.awaitNodesAtHeight) ? 1 : 0) + (has(self.restartSeid) - ? 1 : 0) + (has(self.markReady) ? 1 : 0) == 1' + 1 : 0) + (has(self.govParamChange) ? 1 : 0) + (has(self.awaitCondition) + ? 1 : 0) + (has(self.updateNodeImage) ? 1 : 0) + (has(self.awaitNodesAtHeight) + ? 1 : 0) + (has(self.restartSeid) ? 1 : 0) + (has(self.markReady) + ? 1 : 0) == 1' - message: spec.govSoftwareUpgrade is required when kind=GovSoftwareUpgrade rule: self.kind != 'GovSoftwareUpgrade' || has(self.govSoftwareUpgrade) - message: spec.govVote is required when kind=GovVote rule: self.kind != 'GovVote' || has(self.govVote) + - message: spec.govParamChange is required when kind=GovParamChange + rule: self.kind != 'GovParamChange' || has(self.govParamChange) - message: spec.awaitCondition is required when kind=AwaitCondition rule: self.kind != 'AwaitCondition' || has(self.awaitCondition) - message: spec.updateNodeImage is required when kind=UpdateNodeImage @@ -440,6 +534,25 @@ spec: format: int64 type: integer type: object + govParamChange: + description: GovParamChange outputs for kind=GovParamChange. + properties: + height: + description: Height is the block height at which the tx was + included. + format: int64 + type: integer + proposalId: + description: |- + ProposalID is the on-chain proposal ID parsed from the inclusion + response events. + format: int64 + type: integer + txHash: + description: TxHash is the upper-case hex-encoded transaction + hash. + type: string + type: object govSoftwareUpgrade: description: GovSoftwareUpgrade outputs for kind=GovSoftwareUpgrade. properties: diff --git a/go.mod b/go.mod index 64e7a1b..0ccac84 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/google/uuid v1.6.0 github.com/onsi/gomega v1.39.1 github.com/sei-protocol/sei-config v0.0.19 - github.com/sei-protocol/seictl v0.0.57 + github.com/sei-protocol/seictl v0.0.58-0.20260613152615-8dce7890a3c3 github.com/urfave/cli/v3 v3.6.1 go.opentelemetry.io/otel v1.43.0 go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.43.0 @@ -29,6 +29,7 @@ require ( k8s.io/client-go v0.36.0 k8s.io/utils v0.0.0-20260210185600-b8788abfbbc2 sigs.k8s.io/controller-runtime v0.23.1 + sigs.k8s.io/gateway-api v1.5.1 sigs.k8s.io/yaml v1.6.0 ) @@ -258,7 +259,6 @@ require ( modernc.org/token v1.1.0 // indirect nhooyr.io/websocket v1.8.6 // indirect sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.2 // indirect - sigs.k8s.io/gateway-api v1.5.1 // indirect sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 // indirect sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v6 v6.3.2 // indirect diff --git a/go.sum b/go.sum index 8b80e74..d6c33f9 100644 --- a/go.sum +++ b/go.sum @@ -1790,6 +1790,8 @@ github.com/sei-protocol/seictl v0.0.56 h1:zRCZdGiRrvP+zv/DYhmfP570MOR9nO6o9VWncA github.com/sei-protocol/seictl v0.0.56/go.mod h1:sDWY/llzQPnblG/WS6uQ7vqDtshNQ0WJTJzRUgmfFpg= github.com/sei-protocol/seictl v0.0.57 h1:qyWpv5Iznlp7tQiAU0TYzlFZYHOohfmbVXn/AJp1Co8= github.com/sei-protocol/seictl v0.0.57/go.mod h1:EnEHAT5/egP4aIB4Ir4tg5eK2150RfB9Ohw5xmpZQGE= +github.com/sei-protocol/seictl v0.0.58-0.20260613152615-8dce7890a3c3 h1:2d0cKtxlx/bm2Lr1LfXhxDkXrwACLdfn2ZAmlhL0ZLo= +github.com/sei-protocol/seictl v0.0.58-0.20260613152615-8dce7890a3c3/go.mod h1:EnEHAT5/egP4aIB4Ir4tg5eK2150RfB9Ohw5xmpZQGE= github.com/sei-protocol/seilog v0.0.3 h1:Zi7oWXdX5jv92dY8n482xH032LtNebC89Y+qYZlBn0Y= github.com/sei-protocol/seilog v0.0.3/go.mod h1:CKg58wraWnB3gRxWQ0v1rIVr0gmDHjkfP1bM2giKFFU= github.com/shirou/gopsutil v2.20.5+incompatible/go.mod h1:5b4v6he4MtMOwMlS0TUMTu2PcXUg8+E1lC7eC3UO/RA= diff --git a/internal/controller/nodetask/envtest/gov_param_change_test.go b/internal/controller/nodetask/envtest/gov_param_change_test.go new file mode 100644 index 0000000..8bf93f5 --- /dev/null +++ b/internal/controller/nodetask/envtest/gov_param_change_test.go @@ -0,0 +1,143 @@ +//go:build envtest + +package envtest_test + +import ( + "testing" + + . "github.com/onsi/gomega" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + seiv1alpha1 "github.com/sei-protocol/sei-k8s-controller/api/v1alpha1" +) + +// validGovParamChange returns a well-formed GovParamChange payload with the +// given raw-JSON value for its single change. +func validGovParamChange(value string) *seiv1alpha1.GovParamChangePayload { + return &seiv1alpha1.GovParamChangePayload{ + ChainID: "arctic-1", + Title: "Update Consensus Timeout Params", + Description: "Tighten timeouts.", + Changes: []seiv1alpha1.GovParamChangeEntry{ + {Subspace: "baseapp", Key: "TimeoutParams", Value: apiextensionsv1.JSON{Raw: []byte(value)}}, + }, + InitialDeposit: "10000000usei", + Fees: "8000usei", + Gas: 300000, + } +} + +// kind=GovParamChange with its matching payload is accepted. +func TestCEL_GovParamChange_Accepted(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + snt := baseTask(ns, "govparam-ok", seiv1alpha1.SeiNodeTaskKindGovParamChange) + snt.Spec.GovParamChange = validGovParamChange(`{"propose":"300000000","commit":"200000000"}`) + g.Expect(testCli.Create(testCtx, snt)).To(Succeed()) +} + +// The value field accepts ANY JSON shape (object, string, number, bool) via +// x-kubernetes-preserve-unknown-fields — the apiserver must not reject a +// scalar or coerce the shape. This is the schema-layer half of the +// single-encode contract: the operator passes structured JSON, not a string. +func TestCEL_GovParamChange_ValueAcceptsAnyJSON(t *testing.T) { + for _, value := range []string{ + `{"propose":"300000000"}`, // object (struct param) + `"86400000000000"`, // string (duration ns) + `100`, // bare number + `true`, // bool + } { + t.Run(value, func(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + snt := baseTask(ns, "govparam-val", seiv1alpha1.SeiNodeTaskKindGovParamChange) + snt.Spec.GovParamChange = validGovParamChange(value) + g.Expect(testCli.Create(testCtx, snt)).To(Succeed()) + + // Round-trips byte-identical through the apiserver (no re-encode). + got := &seiv1alpha1.SeiNodeTask{} + g.Expect(testCli.Get(testCtx, client.ObjectKeyFromObject(snt), got)).To(Succeed()) + g.Expect(string(got.Spec.GovParamChange.Changes[0].Value.Raw)).To(Equal(value)) + }) + } +} + +// kind=GovParamChange with NO payload is rejected (exactly-one / kind-required). +func TestCEL_GovParamChange_NoPayload_Rejected(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + snt := baseTask(ns, "govparam-nopayload", seiv1alpha1.SeiNodeTaskKindGovParamChange) + err := testCli.Create(testCtx, snt) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(Or( + ContainSubstring("exactly one"), + ContainSubstring("govParamChange is required"), + )) +} + +// A second payload alongside govParamChange is rejected by the exactly-one rule. +func TestCEL_GovParamChange_MultiplePayloads_Rejected(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + snt := baseTask(ns, "govparam-two", seiv1alpha1.SeiNodeTaskKindGovParamChange) + snt.Spec.GovParamChange = validGovParamChange(`{"propose":"300000000"}`) + snt.Spec.MarkReady = &seiv1alpha1.MarkReadyPayload{} + err := testCli.Create(testCtx, snt) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("exactly one")) +} + +// kind=GovParamChange carrying a mismatched payload (markReady) is rejected by +// the kind/payload agreement rule. +func TestCEL_GovParamChange_KindPayloadMismatch_Rejected(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + snt := baseTask(ns, "govparam-mismatch", seiv1alpha1.SeiNodeTaskKindGovParamChange) + snt.Spec.MarkReady = &seiv1alpha1.MarkReadyPayload{} + err := testCli.Create(testCtx, snt) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("govParamChange is required")) +} + +// An empty changes list is rejected by MinItems=1. +func TestCEL_GovParamChange_EmptyChanges_Rejected(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + snt := baseTask(ns, "govparam-nochanges", seiv1alpha1.SeiNodeTaskKindGovParamChange) + p := validGovParamChange(`{"propose":"300000000"}`) + p.Changes = nil + snt.Spec.GovParamChange = p + err := testCli.Create(testCtx, snt) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("changes")) +} + +// A missing required scalar (empty chainId) is rejected by MinLength=1. +func TestCEL_GovParamChange_MissingChainID_Rejected(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + snt := baseTask(ns, "govparam-nochainid", seiv1alpha1.SeiNodeTaskKindGovParamChange) + p := validGovParamChange(`{"propose":"300000000"}`) + p.ChainID = "" + snt.Spec.GovParamChange = p + err := testCli.Create(testCtx, snt) + g.Expect(err).To(HaveOccurred()) +} + +// spec.kind is immutable — a GovParamChange task cannot be flipped to MarkReady. +func TestCEL_GovParamChange_KindImmutable(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + snt := baseTask(ns, "govparam-immutable", seiv1alpha1.SeiNodeTaskKindGovParamChange) + snt.Spec.GovParamChange = validGovParamChange(`{"propose":"300000000"}`) + g.Expect(testCli.Create(testCtx, snt)).To(Succeed()) + + patch := client.MergeFrom(snt.DeepCopy()) + snt.Spec.Kind = seiv1alpha1.SeiNodeTaskKindMarkReady + snt.Spec.GovParamChange = nil + snt.Spec.MarkReady = &seiv1alpha1.MarkReadyPayload{} + err := testCli.Patch(testCtx, snt, patch) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("kind is immutable")) +} diff --git a/internal/task/seinodetask_params.go b/internal/task/seinodetask_params.go index 2e68ffa..3f2e3d9 100644 --- a/internal/task/seinodetask_params.go +++ b/internal/task/seinodetask_params.go @@ -1,6 +1,7 @@ package task import ( + "encoding/json" "errors" "fmt" @@ -83,6 +84,8 @@ func SeiNodeTaskParamsFor(cr *seiv1alpha1.SeiNodeTask, target *seiv1alpha1.SeiNo return govVoteParams(cr, target) case seiv1alpha1.SeiNodeTaskKindGovSoftwareUpgrade: return govSoftwareUpgradeParams(cr, target) + case seiv1alpha1.SeiNodeTaskKindGovParamChange: + return govParamChangeParams(cr, target) case seiv1alpha1.SeiNodeTaskKindAwaitCondition: return awaitConditionParams(cr) case seiv1alpha1.SeiNodeTaskKindAwaitNodesAtHeight: @@ -143,6 +146,35 @@ func govSoftwareUpgradeParams(cr *seiv1alpha1.SeiNodeTask, target *seiv1alpha1.S }}, nil } +func govParamChangeParams(cr *seiv1alpha1.SeiNodeTask, target *seiv1alpha1.SeiNode) (SeiNodeTaskParams, error) { + p := cr.Spec.GovParamChange + if p == nil { + return SeiNodeTaskParams{}, paramsErr("spec.govParamChange is required for kind=GovParamChange") + } + // Forward each change's value as raw JSON bytes. The sidecar handler does + // the single string() conversion (gov_param_change.go) — converting here + // would double-encode and fail at apply. + changes := make([]sidecar.ParamChangeInput, 0, len(p.Changes)) + for _, c := range p.Changes { + changes = append(changes, sidecar.ParamChangeInput{ + Subspace: c.Subspace, + Key: c.Key, + Value: json.RawMessage(c.Value.Raw), + }) + } + return SeiNodeTaskParams{sidecar.TaskTypeGovParamChange, sidecar.GovParamChangeTask{ + ChainID: p.ChainID, + KeyName: resolveSigningUID(p.KeyName, target), + Title: p.Title, + Description: p.Description, + Changes: changes, + InitialDeposit: p.InitialDeposit, + Memo: p.Memo, + Fees: p.Fees, + Gas: p.Gas, + }}, nil +} + func awaitConditionParams(cr *seiv1alpha1.SeiNodeTask) (SeiNodeTaskParams, error) { p := cr.Spec.AwaitCondition if p == nil { diff --git a/internal/task/seinodetask_params_test.go b/internal/task/seinodetask_params_test.go index d314dfb..73ae7f7 100644 --- a/internal/task/seinodetask_params_test.go +++ b/internal/task/seinodetask_params_test.go @@ -1,14 +1,180 @@ package task import ( + "encoding/json" "errors" "testing" sidecar "github.com/sei-protocol/seictl/sidecar/client" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" seiv1alpha1 "github.com/sei-protocol/sei-k8s-controller/api/v1alpha1" ) +const ( + tpKey = "TimeoutParams" + baseapp = "baseapp" +) + +func govParamChangeCR(value string) *seiv1alpha1.SeiNodeTask { + return &seiv1alpha1.SeiNodeTask{ + Spec: seiv1alpha1.SeiNodeTaskSpec{ + Kind: seiv1alpha1.SeiNodeTaskKindGovParamChange, + GovParamChange: &seiv1alpha1.GovParamChangePayload{ + ChainID: "arctic-1", + KeyName: "node_admin", + Title: "Update Consensus Timeout Params", + Description: "Tighten timeouts.", + Changes: []seiv1alpha1.GovParamChangeEntry{ + {Subspace: baseapp, Key: tpKey, Value: apiextensionsv1.JSON{Raw: []byte(value)}}, + }, + InitialDeposit: "10000000usei", + Fees: "8000usei", + Gas: 300000, + }, + }, + } +} + +// kind=GovParamChange maps to the sidecar gov-param-change task with the +// proposal fields and a per-change value forwarded as RAW bytes (the single +// string() conversion happens in the sidecar, not here). +func TestSeiNodeTaskParamsFor_GovParamChange(t *testing.T) { + cr := govParamChangeCR(`{"propose":"300000000"}`) + + p, err := SeiNodeTaskParamsFor(cr, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if p.Type != sidecar.TaskTypeGovParamChange { + t.Errorf("Type = %q, want %q", p.Type, sidecar.TaskTypeGovParamChange) + } + task, ok := p.Payload.(sidecar.GovParamChangeTask) + if !ok { + t.Fatalf("Payload = %T, want sidecar.GovParamChangeTask", p.Payload) + } + if task.ChainID != "arctic-1" || task.KeyName != "node_admin" || task.Gas != 300000 { + t.Errorf("scalar fields not forwarded: %+v", task) + } + if len(task.Changes) != 1 || task.Changes[0].Subspace != baseapp || task.Changes[0].Key != tpKey { + t.Fatalf("changes not mapped: %+v", task.Changes) + } + // Validate() must pass on the produced task (the sidecar runs it next). + if err := task.Validate(); err != nil { + t.Errorf("produced task fails Validate(): %v", err) + } +} + +// Regression guard for the prop-252 double-encode bug at the controller +// boundary: the CRD value (apiextensionsv1.JSON) must reach the sidecar task as +// raw bytes, byte-identical, for any JSON shape — never re-escaped/re-marshaled. +func TestSeiNodeTaskParamsFor_GovParamChange_ValueSingleEncoded(t *testing.T) { + for _, raw := range []string{ + `{"propose":"300000000","commit":"200000000"}`, // object + `"86400000000000"`, // scalar string + `100`, // scalar number + `true`, // scalar bool + } { + t.Run(raw, func(t *testing.T) { + p, err := SeiNodeTaskParamsFor(govParamChangeCR(raw), nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + task := p.Payload.(sidecar.GovParamChangeTask) + if got := string(task.Changes[0].Value); got != raw { + t.Errorf("forwarded value = %q, want %q (re-encoded?)", got, raw) + } + }) + } +} + +// keyName omitted resolves via the target's operator keyring (node_admin when +// the secret is set without an explicit keyName). +func TestSeiNodeTaskParamsFor_GovParamChange_KeyNameDerivedFromTarget(t *testing.T) { + cr := govParamChangeCR(`{"propose":"300000000"}`) + cr.Spec.GovParamChange.KeyName = "" + target := &seiv1alpha1.SeiNode{ + Spec: seiv1alpha1.SeiNodeSpec{ + Validator: &seiv1alpha1.ValidatorSpec{ + OperatorKeyring: &seiv1alpha1.OperatorKeyringSource{ + Secret: &seiv1alpha1.SecretOperatorKeyringSource{}, + }, + }, + }, + } + p, err := SeiNodeTaskParamsFor(cr, target) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := p.Payload.(sidecar.GovParamChangeTask).KeyName; got != seiv1alpha1.DefaultOperatorKeyName { + t.Errorf("derived KeyName = %q, want %q", got, seiv1alpha1.DefaultOperatorKeyName) + } +} + +// The Type that the build path produces MUST be in the Deserialize registry, +// or the controller fails every GovParamChange task with DeserializeFailed +// before it ever reaches the sidecar. This round-trip guards the registry +// wiring (the build path alone does not exercise it). +func TestSeiNodeTaskParamsFor_GovParamChange_DeserializeRoundTrip(t *testing.T) { + p, err := SeiNodeTaskParamsFor(govParamChangeCR(`{"propose":"300000000"}`), nil) + if err != nil { + t.Fatalf("params: %v", err) + } + raw, err := json.Marshal(p.Payload) + if err != nil { + t.Fatalf("marshal payload: %v", err) + } + exec, err := Deserialize(p.Type, "gpc-1", raw, ExecutionConfig{}) + if err != nil { + t.Fatalf("Deserialize(%q) failed — is the task type registered? %v", p.Type, err) + } + if exec == nil { + t.Fatal("Deserialize returned nil TaskExecution") + } +} + +// Multiple changes are forwarded in order, each value byte-preserved. +func TestSeiNodeTaskParamsFor_GovParamChange_MultipleChanges(t *testing.T) { + cr := govParamChangeCR(`{"propose":"300000000"}`) + cr.Spec.GovParamChange.Changes = []seiv1alpha1.GovParamChangeEntry{ + {Subspace: baseapp, Key: tpKey, Value: apiextensionsv1.JSON{Raw: []byte(`{"propose":"300000000"}`)}}, + {Subspace: "staking", Key: "MaxValidators", Value: apiextensionsv1.JSON{Raw: []byte(`"100"`)}}, + } + p, err := SeiNodeTaskParamsFor(cr, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + task := p.Payload.(sidecar.GovParamChangeTask) + if len(task.Changes) != 2 { + t.Fatalf("got %d changes, want 2", len(task.Changes)) + } + if task.Changes[0].Key != tpKey || task.Changes[1].Key != "MaxValidators" { + t.Errorf("order not preserved: %q, %q", task.Changes[0].Key, task.Changes[1].Key) + } + if string(task.Changes[1].Value) != `"100"` { + t.Errorf("second value = %q, want %q", string(task.Changes[1].Value), `"100"`) + } +} + +// kind=GovParamChange with a nil payload is a param-build failure +// (ParamsBuildFailed), not an unsupported kind. CEL normally blocks this. +func TestSeiNodeTaskParamsFor_GovParamChange_NilPayload_ParamsBuildFailed(t *testing.T) { + cr := &seiv1alpha1.SeiNodeTask{ + Spec: seiv1alpha1.SeiNodeTaskSpec{Kind: seiv1alpha1.SeiNodeTaskKindGovParamChange}, + } + _, err := SeiNodeTaskParamsFor(cr, nil) + if err == nil { + t.Fatal("expected error for missing payload, got nil") + } + var unsupported *ErrUnsupportedKind + if errors.As(err, &unsupported) { + t.Error("missing-payload error must not be *ErrUnsupportedKind") + } + if got := FailureReason(err); got != ReasonParamsBuildFailed { + t.Errorf("FailureReason = %q, want %q", got, ReasonParamsBuildFailed) + } +} + // An unwired kind (one the CRD enum may admit but this build does not dispatch) // returns a typed *ErrUnsupportedKind carrying the offending kind, so the // synthesis site can route it to reason=UnsupportedKind via errors.As. diff --git a/internal/task/task.go b/internal/task/task.go index ab4e171..13f1eaa 100644 --- a/internal/task/task.go +++ b/internal/task/task.go @@ -217,6 +217,7 @@ var registry = map[string]taskDeserializer{ sidecar.TaskTypeSetGenesisPeers: sidecarTask[sidecar.SetGenesisPeersTask](false), sidecar.TaskTypeGovVote: sidecarTask[sidecar.GovVoteTask](false), sidecar.TaskTypeGovSoftwareUpgrade: sidecarTask[sidecar.GovSoftwareUpgradeTask](false), + sidecar.TaskTypeGovParamChange: sidecarTask[sidecar.GovParamChangeTask](false), // Controller-side group tasks TaskTypeAwaitNodesRunning: deserializeAwaitNodesRunning, diff --git a/manifests/sei.io_seinodetasks.yaml b/manifests/sei.io_seinodetasks.yaml index 515cf48..51fc2ac 100644 --- a/manifests/sei.io_seinodetasks.yaml +++ b/manifests/sei.io_seinodetasks.yaml @@ -106,6 +106,96 @@ spec: required: - targetHeight type: object + govParamChange: + description: GovParamChange is the payload for kind=GovParamChange. + properties: + chainId: + description: ChainID is the chain ID the proposal targets. + minLength: 1 + type: string + changes: + description: Changes is the non-empty set of parameter changes. + items: + description: GovParamChangeEntry is one (subspace, key, value) + parameter change. + properties: + key: + description: Key is the parameter key within the subspace + (e.g. "TimeoutParams"). + minLength: 1 + type: string + subspace: + description: Subspace is the params subspace (e.g. "baseapp", + "staking"). + minLength: 1 + type: string + value: + description: |- + Value is the new parameter value as JSON of whatever shape the + param's registered type expects — a scalar (100), a string + ("86400000000000"), a bool, or an object. Pass it as structured JSON, + NOT a pre-escaped string: the controller forwards the raw bytes and + the sidecar stringifies them exactly once, so an escaped string would + double-encode and fail at apply. Integer-valued params must be JSON + strings (Sei convention); see the sidecar handler for why. + x-kubernetes-preserve-unknown-fields: true + required: + - key + - subspace + - value + type: object + minItems: 1 + type: array + description: + description: Description is the on-chain proposal description. + minLength: 1 + type: string + fees: + description: |- + Fees is the tx fee in coin notation (e.g. "8000usei"). usei-only, and + must clear the target node's enforced minimum-gas-prices (e.g. + 0.02usei/gas on arctic-1) or the tx is silently CheckTx-rejected. + minLength: 1 + type: string + gas: + description: Gas is the tx gas limit. + format: int64 + minimum: 1 + type: integer + initialDeposit: + description: |- + InitialDeposit is the proposal deposit in coin notation (e.g. + "10000000usei"). usei-only; the sidecar pre-validates. Must be >= + the chain's min_deposit to enter the voting period rather than the + deposit period. + minLength: 1 + type: string + keyName: + description: |- + KeyName names the keyring entry that signs the proposal. Omit to let + the controller derive from the target SeiNode: spec.validator. + operatorKeyring.secret.keyName when .secret is set (defaulting to + "node_admin"), otherwise "validator". + pattern: ^[a-zA-Z0-9_-]+$ + type: string + memo: + description: |- + Memo is the optional tx memo. The sidecar appends a `taskID=` tag; + do not pre-tag. + type: string + title: + description: Title is the on-chain proposal title. + minLength: 1 + type: string + required: + - chainId + - changes + - description + - fees + - gas + - initialDeposit + - title + type: object govSoftwareUpgrade: description: GovSoftwareUpgrade is the payload for kind=GovSoftwareUpgrade. properties: @@ -242,6 +332,7 @@ spec: enum: - GovSoftwareUpgrade - GovVote + - GovParamChange - AwaitCondition - UpdateNodeImage - AwaitNodesAtHeight @@ -323,17 +414,20 @@ spec: - target type: object x-kubernetes-validations: - - message: exactly one of govSoftwareUpgrade, govVote, awaitCondition, - updateNodeImage, awaitNodesAtHeight, restartSeid, or markReady must - be set + - message: exactly one of govSoftwareUpgrade, govVote, govParamChange, + awaitCondition, updateNodeImage, awaitNodesAtHeight, restartSeid, + or markReady must be set rule: '(has(self.govSoftwareUpgrade) ? 1 : 0) + (has(self.govVote) ? - 1 : 0) + (has(self.awaitCondition) ? 1 : 0) + (has(self.updateNodeImage) - ? 1 : 0) + (has(self.awaitNodesAtHeight) ? 1 : 0) + (has(self.restartSeid) - ? 1 : 0) + (has(self.markReady) ? 1 : 0) == 1' + 1 : 0) + (has(self.govParamChange) ? 1 : 0) + (has(self.awaitCondition) + ? 1 : 0) + (has(self.updateNodeImage) ? 1 : 0) + (has(self.awaitNodesAtHeight) + ? 1 : 0) + (has(self.restartSeid) ? 1 : 0) + (has(self.markReady) + ? 1 : 0) == 1' - message: spec.govSoftwareUpgrade is required when kind=GovSoftwareUpgrade rule: self.kind != 'GovSoftwareUpgrade' || has(self.govSoftwareUpgrade) - message: spec.govVote is required when kind=GovVote rule: self.kind != 'GovVote' || has(self.govVote) + - message: spec.govParamChange is required when kind=GovParamChange + rule: self.kind != 'GovParamChange' || has(self.govParamChange) - message: spec.awaitCondition is required when kind=AwaitCondition rule: self.kind != 'AwaitCondition' || has(self.awaitCondition) - message: spec.updateNodeImage is required when kind=UpdateNodeImage @@ -440,6 +534,25 @@ spec: format: int64 type: integer type: object + govParamChange: + description: GovParamChange outputs for kind=GovParamChange. + properties: + height: + description: Height is the block height at which the tx was + included. + format: int64 + type: integer + proposalId: + description: |- + ProposalID is the on-chain proposal ID parsed from the inclusion + response events. + format: int64 + type: integer + txHash: + description: TxHash is the upper-case hex-encoded transaction + hash. + type: string + type: object govSoftwareUpgrade: description: GovSoftwareUpgrade outputs for kind=GovSoftwareUpgrade. properties: