fix(agd): x/param removal part 1/2#12674
Draft
michaelfig wants to merge 6 commits into
Draft
Conversation
michaelfig
force-pushed
the
mfig/ago-569-agd-xparam-removal-part-12
branch
from
d754384 to
83734ba
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR advances the x/params removal for agd by introducing governance MsgUpdateParams support for x/swingset and x/vbank, and migrating both modules’ active parameter storage from legacy x/params subspaces into module-owned KV stores while preserving read/migration compatibility during upgrades.
Changes:
- Add
MsgUpdateParams(proto, generated TS/Go types, RPC clients) for swingset and vbank so governance can update params without legacyx/params. - Move swingset/vbank param persistence into their module KV stores, keeping legacy subspaces only for one-time migration reads.
- Wire upgrade handling to register the legacy param key tables needed to read existing
x/paramsstate; add/adjust tests for CLI queries, migrations, and vbank msg handling.
Reviewed changes
Copilot reviewed 36 out of 38 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/cosmic-proto/src/codegen/typeFromUrl.ts | Adds URL→type mappings for new swingset/vbank MsgUpdateParams message types. |
| packages/cosmic-proto/src/codegen/agoric/vbank/msgs.ts | New generated TS message types/codecs for vbank MsgUpdateParams. |
| packages/cosmic-proto/src/codegen/agoric/swingset/msgs.ts | Adds swingset MsgUpdateParams and updates MsgCoreEval amino type. |
| packages/cosmic-proto/proto/agoric/vbank/msgs.proto | Defines vbank Msg.UpdateParams service + request/response messages. |
| packages/cosmic-proto/proto/agoric/swingset/msgs.proto | Defines swingset Msg.UpdateParams and updates MsgCoreEval amino name. |
| packages/client-utils/src/codegen/agoric/vbank/msgs.rpc.msg.ts | Adds vbank Tx RPC client stub for UpdateParams. |
| packages/client-utils/src/codegen/agoric/swingset/msgs.rpc.msg.ts | Adds swingset Tx RPC client method for UpdateParams. |
| packages/client-utils/src/codegen/agoric/rpc.tx.ts | Exposes vbank Msg client under agoric.vbank. |
| golang/cosmos/x/vbank/vbank_test.go | Updates keeper construction to include governance authority string. |
| golang/cosmos/x/vbank/types/params.go | Removes direct x/params ParamSet wiring from the “active” params definition. |
| golang/cosmos/x/vbank/types/params_legacy.go | Restores legacy x/params key table + ParamSetPairs for migrations. |
| golang/cosmos/x/vbank/types/msgs.pb.go | Generated protobuf updates for vbank Msg service + MsgUpdateParams. |
| golang/cosmos/x/vbank/types/msgs.go | Implements sdk.Msg methods for MsgUpdateParams. |
| golang/cosmos/x/vbank/types/key.go | Introduces ParamsKey for module-store params persistence. |
| golang/cosmos/x/vbank/types/codec.go | Registers amino + interface implementations for MsgUpdateParams. |
| golang/cosmos/x/vbank/msg_update_params_test.go | Adds unit tests for vbank UpdateParams and legacy params migration. |
| golang/cosmos/x/vbank/module.go | Registers real vbank MsgServer implementation. |
| golang/cosmos/x/vbank/keeper/msg_server.go | Implements vbank Msg.UpdateParams handler with authority/validation checks. |
| golang/cosmos/x/vbank/keeper/migrations.go | Adds legacy params migration and default/backfill behavior for module params. |
| golang/cosmos/x/vbank/keeper/keeper.go | Switches GetParams/SetParams to module KV store; keeps legacy subspace for migration reads. |
| golang/cosmos/x/vbank/keeper/grpc_query.go | Updates params query to read from module KV store. |
| golang/cosmos/x/vbank/client/cli/query_test.go | Ensures agd query vbank params command remains present. |
| golang/cosmos/x/swingset/types/params.go | Removes direct x/params ParamSet wiring from active swingset params. |
| golang/cosmos/x/swingset/types/params_legacy.go | Adds legacy x/params key table + ParamSetPairs for migrations. |
| golang/cosmos/x/swingset/types/msgs.pb.go | Generated protobuf updates for swingset Msg service + MsgUpdateParams. |
| golang/cosmos/x/swingset/types/msgs.go | Implements sdk.Msg methods for swingset MsgUpdateParams. |
| golang/cosmos/x/swingset/types/key.go | Introduces ParamsKey for module-store params persistence. |
| golang/cosmos/x/swingset/types/codec.go | Registers amino + interface implementations for MsgCoreEval and MsgUpdateParams. |
| golang/cosmos/x/swingset/keeper/msg_server.go | Implements swingset Msg.UpdateParams handler (authority/validation + SetParams). |
| golang/cosmos/x/swingset/keeper/migrations.go | Switches migration path to read legacy x/params into module store. |
| golang/cosmos/x/swingset/keeper/migrations_test.go | Adds test covering swingset legacy params migration into module store. |
| golang/cosmos/x/swingset/keeper/keeper.go | Switches GetParams/SetParams to module KV store; keeps legacy subspace for migration reads. |
| golang/cosmos/x/swingset/client/cli/query_test.go | Ensures agd query swingset params command remains present. |
| golang/cosmos/tests/integrations/vbank/vbank_test.go | Updates integration wiring to register real vbank Msg/Query servers and pass authority. |
| golang/cosmos/proto/agoric/vbank/msgs.proto | Go-side proto definition for vbank Msg.UpdateParams. |
| golang/cosmos/proto/agoric/swingset/msgs.proto | Go-side proto definition for swingset Msg.UpdateParams and updated MsgCoreEval amino name. |
| golang/cosmos/app/upgrade.go | Registers legacy param key tables for swingset/vbank during upgrades. |
| golang/cosmos/app/app.go | Passes governance authority into vbank keeper constructor. |
Files not reviewed (2)
- golang/cosmos/x/swingset/types/msgs.pb.go: Language not supported
- golang/cosmos/x/vbank/types/msgs.pb.go: Language not supported
| legacy.RegisterAminoMsg(cdc, &MsgUpdateParams{}, ModuleName+"/UpdateParams") | ||
| } | ||
|
|
||
| // RegisterInterfaces registers the x/swingset interfaces types with the interface registry |
| vbankStore := runtime.NewKVStoreService(vbankStoreKey) | ||
| transientStore := runtime.NewTransientStoreService(paramsTStoreKey) | ||
| keeper := NewKeeper(cdc, vbankStore, transientStore, subspace, account, bank, "feeCollectorName", pushAction) | ||
| keeper := NewKeeper(cdc, vbankStore, transientStore, subspace, account, bank, "feeCollectorName", authtypes.NewModuleAddress("gov").String(), pushAction) |
Comment on lines
+346
to
+352
| func (k msgServer) UpdateParams(goCtx context.Context, msg *types.MsgUpdateParams) (*types.MsgUpdateParamsResponse, error) { | ||
| ctx := sdk.UnwrapSDKContext(goCtx) | ||
|
|
||
| if msg.Authority != k.authority { | ||
| return nil, sdkioerrors.Wrapf(govtypes.ErrInvalidSigner, "invalid authority; expected %s, got %s", k.authority, msg.Authority) | ||
| } | ||
| if err := msg.Params.ValidateBasic(); err != nil { |
Add swingset and vbank MsgUpdateParams proto surfaces, regenerate codegen output, and register the new message types with Agoric amino names. Co-authored-by: Codex <codex@openai.com>
Move active swingset params into the module store while retaining legacy x/params adapters for upgrade migration. Co-authored-by: Codex <codex@openai.com>
Move active vbank params into the module store, add authority-gated MsgUpdateParams handling, and migrate legacy x/params values independently from swingset. Co-authored-by: Codex <codex@openai.com>
Install swingset and vbank legacy param keytables during upgrade registration and cover the module params query commands. Co-authored-by: Codex <codex@openai.com>
michaelfig
force-pushed
the
mfig/ago-569-agd-xparam-removal-part-12
branch
from
07a4756 to
8fe2610
Compare
Refresh the cosmic-proto export snapshot for the new swingset MsgUpdateParams API so the package test matches regenerated proto output. Co-authored-by: Codex <codex@openai.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
refs: https://linear.app/agoric/issue/AGO-569/agd-legacy-xparam-migration-part-12
Description
This adds
MsgUpdateParamssupport across the Agoric swingset and vbank proto/generated client files, plus the matching Go type, codec, and keeper message-server handling so governance can update params without legacyx/params. It also movesx/swingsetandx/vbankactive params into their own module stores with legacy migration adapters, while app upgrade wiring registers the old key tables needed to read existingx/paramsstate. The CLI query tests cover the existingagd query swingset paramsandagd query vbank paramspaths after the storage move.Security Considerations
This introduces new authority-gated
MsgUpdateParamsmessages for swingset and vbank. The configured governance authority must sign those messages, and params are validated before being written, so this preserves the intended governance boundary while replacing legacyx/paramsmutation.Scaling Considerations
No significant scaling impact is expected. Each module stores one params record in its own KV store, and the upgrade migration performs a one-time read from legacy
x/paramsplus a single write per module.Documentation Considerations
Release notes should mention that swingset and vbank param changes move from legacy param-change proposals to gov v1 proposals carrying
MsgUpdateParams. Existing chain data is migrated during upgrade, and the read-side CLI remainsagd query swingset params/agd query vbank params.Testing Considerations
Added tests for the new update-param paths, legacy param migration, app upgrade keytable wiring, and CLI query command availability. Verified with targeted Go tests for app, swingset, vbank, vbank integration, plus
./scripts/codegen.shandyarn typecheck-quick.Upgrade Considerations
The upgrade handler must install swingset and vbank legacy param key tables before module migrations run, because migration reads the old
x/paramssubspaces before writing module-owned params. Release verification should compare pre-upgrade and post-upgrade swingset/vbank params, then confirmagd query swingset paramsandagd query vbank paramsreturn the migrated values.