fix(agd): x/param removal part 1/2#12674
Draft
michaelfig wants to merge 8 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 { |
michaelfig
force-pushed
the
mfig/ago-569-agd-xparam-removal-part-12
branch
from
07a4756 to
8fe2610
Compare
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>
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>
michaelfig
force-pushed
the
mfig/ago-569-agd-xparam-removal-part-12
branch
from
b38996c to
9877384
Compare
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. It also addsagd tx gov simulate-proposal <proposal.json>, which loads a gov v1 proposal or tx JSON file and reports message execution errors as if the proposal had immediately passed and run through the app message router.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. The simulation command does not add new chain authority or mutate state; it executes proposal messages against cached local app state and reports handler, panic, and execution errors before discarding the cache.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. The simulation command is local-only and has no chain runtime impact. Its cost is proportional to the number of proposal messages being simulated and the normal execution cost of those message handlers.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. Developer/operator notes should mentionagd tx gov simulate-proposal <proposal.json>as a preflight check for gov v1 proposal payloads, especially for proposals carryingMsgUpdateParams.Testing Considerations
Added tests for the new update-param paths, legacy param migration, app upgrade keytable wiring, and CLI query command availability. Added coverage for mounting
agd tx gov simulate-proposal, reading both proposal JSON and tx JSON inputs, unpackingMsgSubmitProposal, and aggregating simulated message execution errors. Verified with targeted Go tests for app, swingset, vbank, vbank integration, plus./scripts/codegen.sh,yarn typecheck-quick, andgo test ./daemon/cmd.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.