Tracking issues surfaced during the Coral cross-review of the gov-software-upgrade PR (#177).
Status update 2026-05-12: PR #177 was refactored from a meta-handler (gov-submit-proposal with a Type discriminator) into per-type handlers (gov-software-upgrade as a single-purpose task). Items 1 and 2 below — both products of the meta-shape — are now moot and marked CLOSED. Items 3-6 still bind future handlers and remain open.
1. Nested request shape for typed params ✅ CLOSED — refactor obsoleted
GovSubmitProposalRequest currently puts UpgradeName, UpgradeHeight, UpgradeInfo at the top level alongside type-agnostic fields. When the next proposal type lands the struct becomes either an ever-growing flat struct or a retroactive nested shape that breaks the wire format.
Resolved by the meta-handler → per-type-handler refactor on #177. Each proposal type now has its own typed request struct with no shared shape.
2. Shared ValidateProposalParams between client and server ✅ CLOSED — refactor obsoleted
Client GovSubmitProposalTask.Validate() and server buildProposalContent both inspect Type / UpgradeName / UpgradeHeight / Title / Description. Drift risk when a new proposal type lands.
Resolved by the refactor. No type-discriminator switch remains; each handler validates its own fields locally.
3. UpgradeHeight lower-bound check against current chain height
The handler accepts any positive UpgradeHeight. A typo (current+5 instead of current+50000) would land a same-day halt at the named height. Cosmos x/upgrade rejects past heights in ValidateBasic but does not enforce a minimum-lead-blocks floor.
Action when #165 (kube-rbac-proxy authn) lands: add a sidecar-side guard that queries local seid /status for the current height and requires UpgradeHeight >= current + MIN_LEAD_BLOCKS (e.g., 1000 blocks or operator-configurable). Pre-authn this is loopback only and a confused-but-trusted caller; post-authn the surface expands and the guard becomes load-bearing.
4. Title / Description length caps
Cosmos gov.Content.ValidateBasic caps at 140 / 10000 chars and signAndBroadcast will catch oversize content before broadcast. But a 10MB description gets fully marshaled and Any-packed before rejection. Currently bounded by the loopback trust model + engine UUID dedupe.
Action when #165 lands: add a length check at the top of buildSoftwareUpgradeMsg (e.g., 200 / 16384) to short-circuit before SDK paths allocate. Future proposal handlers (per-type) get the same check at their own buildXxxMsg.
5. Engine run value plumbed via context
Engine runTask already takes run int but does NOT put it on ctx alongside TaskID. Sign-tx handlers logging the run field would give on-call engineers a single-grep signal of "did this broadcast happen on a rehydrated run?" — relevant when chasing a duplicate-proposal incident.
Action: add WithRun(ctx, run) + RunFromContext(ctx) int to sidecar/engine. Update all sign-tx handler log lines to include the field. Probably ships with #174 (pre-broadcast txHash marker) since they're related forensic improvements.
6. Process-start banner enumerating handlers shipping with #174 risk
A single log line at sidecar startup: "sign-tx handlers without pre-broadcast dedup: gov-software-upgrade (see #174)". Gives ops visibility without per-broadcast noise. Cheap; consider adding when a third non-idempotent sign-tx handler ships.
References
Tracking issues surfaced during the Coral cross-review of the gov-software-upgrade PR (#177).
1.
Nested request shape for typed params✅ CLOSED — refactor obsoletedGovSubmitProposalRequest currently puts UpgradeName, UpgradeHeight, UpgradeInfo at the top level alongside type-agnostic fields. When the next proposal type lands the struct becomes either an ever-growing flat struct or a retroactive nested shape that breaks the wire format.Resolved by the meta-handler → per-type-handler refactor on #177. Each proposal type now has its own typed request struct with no shared shape.
2.
Shared ValidateProposalParams between client and server✅ CLOSED — refactor obsoletedClient GovSubmitProposalTask.Validate() and server buildProposalContent both inspect Type / UpgradeName / UpgradeHeight / Title / Description. Drift risk when a new proposal type lands.Resolved by the refactor. No type-discriminator switch remains; each handler validates its own fields locally.
3. UpgradeHeight lower-bound check against current chain height
The handler accepts any positive UpgradeHeight. A typo (current+5 instead of current+50000) would land a same-day halt at the named height. Cosmos x/upgrade rejects past heights in ValidateBasic but does not enforce a minimum-lead-blocks floor.
Action when #165 (kube-rbac-proxy authn) lands: add a sidecar-side guard that queries local seid /status for the current height and requires UpgradeHeight >= current + MIN_LEAD_BLOCKS (e.g., 1000 blocks or operator-configurable). Pre-authn this is loopback only and a confused-but-trusted caller; post-authn the surface expands and the guard becomes load-bearing.
4. Title / Description length caps
Cosmos gov.Content.ValidateBasic caps at 140 / 10000 chars and signAndBroadcast will catch oversize content before broadcast. But a 10MB description gets fully marshaled and Any-packed before rejection. Currently bounded by the loopback trust model + engine UUID dedupe.
Action when #165 lands: add a length check at the top of buildSoftwareUpgradeMsg (e.g., 200 / 16384) to short-circuit before SDK paths allocate. Future proposal handlers (per-type) get the same check at their own buildXxxMsg.
5. Engine run value plumbed via context
Engine runTask already takes run int but does NOT put it on ctx alongside TaskID. Sign-tx handlers logging the run field would give on-call engineers a single-grep signal of "did this broadcast happen on a rehydrated run?" — relevant when chasing a duplicate-proposal incident.
Action: add WithRun(ctx, run) + RunFromContext(ctx) int to sidecar/engine. Update all sign-tx handler log lines to include the field. Probably ships with #174 (pre-broadcast txHash marker) since they're related forensic improvements.
6. Process-start banner enumerating handlers shipping with #174 risk
A single log line at sidecar startup: "sign-tx handlers without pre-broadcast dedup: gov-software-upgrade (see #174)". Gives ops visibility without per-broadcast noise. Cheap; consider adding when a third non-idempotent sign-tx handler ships.
References