Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EPIC: Refactor modules away from using x/params #10514

Closed
11 tasks done
aaronc opened this issue Nov 9, 2021 · 11 comments
Closed
11 tasks done

EPIC: Refactor modules away from using x/params #10514

aaronc opened this issue Nov 9, 2021 · 11 comments

Comments

@aaronc
Copy link
Member

aaronc commented Nov 9, 2021

As discussed in #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:

@technicallyty
Copy link
Contributor

added @gsk967 to gov per #9979 (comment).

@technicallyty technicallyty self-assigned this Nov 10, 2021
@tac0turtle
Copy link
Member

tac0turtle commented Nov 15, 2021

is this blocked on #9438

EDIT: its blocking

@tac0turtle tac0turtle added the S:blocked Status: Blocked label Nov 15, 2021
@tac0turtle tac0turtle removed the S:blocked Status: Blocked label Jan 19, 2022
@tac0turtle
Copy link
Member

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?

@alexanderbez
Copy link
Contributor

@technicallyty lets work on this together 👍

@technicallyty
Copy link
Contributor

@technicallyty lets work on this together 👍

sounds good!

@gsk967
Copy link
Contributor

gsk967 commented Jan 21, 2022

@technicallyty Can I start on gov params

@tac0turtle tac0turtle changed the title Refactor modules away from using x/params EPIC: Refactor modules away from using x/params Jun 22, 2022
@tac0turtle tac0turtle added T:Epic Epics G:Q3 and removed T:Sprint labels Jun 22, 2022
@tac0turtle tac0turtle pinned this issue Jul 11, 2022
@tac0turtle
Copy link
Member

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.

@alexanderbez
Copy link
Contributor

alexanderbez commented Jul 19, 2022

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.

@amaury1093
Copy link
Contributor

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)

@alexanderbez
Copy link
Contributor

alexanderbez commented Jul 19, 2022

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

@amaury1093
Copy link
Contributor

Yeah, that works too 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants