cosmos-sdk icon indicating copy to clipboard operation
cosmos-sdk copied to clipboard

Published 20 hours ago • verified publisher icon cosmos

EPIC: Refactor modules away from using x/params

Open aaronc opened this issue 3 years ago • 11 comments

As discussed in https://github.com/cosmos/cosmos-sdk/discussions/9913

In adr-046, the sdk decided to move away from a param module in favor of having each sdk module handle its own parameters. This is an optional change for applications and the Param module will live on in a deprecated form. The sdk will be migrating the core modules to use this new paradigm.

See https://github.com/cosmos/cosmos-sdk/blob/master/docs/architecture/adr-046-module-params.md for instructions on how to do this.

TODO:

  • [x] #12283
  • [x] #12284
  • [x] #12285
  • [x] #12286
  • [x] #12287
  • [x] #12288
  • [x] #12445
  • [x] https://github.com/cosmos/cosmos-sdk/pull/12631
  • [x] #12496
  • [x] #12497
  • [ ] https://github.com/cosmos/cosmos-sdk/issues/12652

aaronc avatar Nov 09 '21 15:11 aaronc

added @gsk967 to gov per https://github.com/cosmos/cosmos-sdk/pull/9979#issuecomment-964292230.

technicallyty avatar Nov 10 '21 00:11 technicallyty

is this blocked on https://github.com/cosmos/cosmos-sdk/issues/9438

EDIT: its blocking

tac0turtle avatar Nov 15 '21 10:11 tac0turtle

Talked with @cmwaters and the ability to list a module as the only one to execute this message is merged into master, unblocking this work. @technicallyty do you still want to kick off this work?

tac0turtle avatar Jan 19 '22 11:01 tac0turtle

@technicallyty lets work on this together 👍

alexanderbez avatar Jan 19 '22 15:01 alexanderbez

@technicallyty lets work on this together 👍

sounds good!

technicallyty avatar Jan 19 '22 16:01 technicallyty

@technicallyty Can I start on gov params

gsk967 avatar Jan 21 '22 06:01 gsk967

Two Open questions that came to me:

  • how to handle consensus parameters from tendermint, make base app stateful or role a very simple tendermint param module for these things.
  • Do we want deprecate the transient store? The only module using it was the param module.

tac0turtle avatar Jul 18 '22 13:07 tac0turtle

how to handle consensus parameters from tendermint, make base app stateful or role a very simple tendermint param module for these things.

A dedicated module (e.g. x/consensus or x/consensus-params) with a simple message MsgUpdatePararms and a message server -- nothing else.

Do we want deprecate the transient store? The only module using it was the param module.

Yes, I believe so.

alexanderbez avatar Jul 19 '22 02:07 alexanderbez

A dedicated module (e.g. x/consensus or x/consensus-params) with a simple message MsgUpdatePararms and a message serve -- nothing else.

Agreed. The tricky part is how to make baseapp aware of this module. Baseapp should obviously not depend on x/consensus-params, so I guess the correct dep is the other way round, e.g. a simple setter on BaseApp that would be called from inside x/consensus-params (similar to how x/upgrade updates the AppVersion in baseapp)

amaury1093 avatar Jul 19 '22 12:07 amaury1093

@amaurym why can't we just provide an interface/generics type to BaseApp (that the keeper implements)?

alexanderbez avatar Jul 19 '22 13:07 alexanderbez

Yeah, that works too 👍

amaury1093 avatar Jul 19 '22 13:07 amaury1093