feat: migrate minfee module to use self managed params #59
Conversation
| type Params struct { | ||
| NetworkMinGasPrice math.LegacyDec | ||
| } |
There was a problem hiding this comment.
this was the non-proto type that implemented the ParamSet interface, and has been replaced with proto Params
| @@ -9,14 +9,11 @@ import ( | |||
| "github.com/celestiaorg/celestia-app/v4/pkg/appconsts" | |||
| ) | |||
|
|
|||
| const ModuleName = "minfee" | |||
| // TODO: this file can be removed once the upgrade to self managed modules has been completed. | |||
There was a problem hiding this comment.
I think this is TBD based on if we want to support the fallback mechanism.
| // DefaultGenesis returns the default genesis state. | ||
| func DefaultGenesis() *GenesisState { | ||
| return &GenesisState{ | ||
| NetworkMinGasPrice: DefaultNetworkMinGasPrice, // TODO: remove this field |
There was a problem hiding this comment.
I think it will eventually be safe to remove this in a GenesisStaet v2, but the current logic should basically ignore this value, and only read from params.
| if !exists { | ||
| panic("minfee subspace not set") | ||
| } | ||
| RegisterMinFeeParamTable(subspace) | ||
| types.RegisterMinFeeParamTable(subspace) |
There was a problem hiding this comment.
if we don't do a fallback, all this can be removed after the migration.
| sdkCtx := sdk.UnwrapSDKContext(ctx) | ||
| genesis := types.DefaultGenesis() | ||
| // TODO: genesis should hold params not this field. | ||
| genesis.NetworkMinGasPrice = k.GetParams(sdkCtx).NetworkMinGasPrice |
There was a problem hiding this comment.
the previous GenesisState basically was just params, need to decide on cleanest approach to handle the field existing at the genesis. root level and also the new param level.
| import "gogoproto/gogo.proto"; | ||
| import "google/api/annotations.proto"; | ||
| import "cosmos_proto/cosmos.proto"; | ||
|
|
||
| option go_package = "github.com/celestiaorg/celestia-app/x/minfee"; | ||
| option go_package = "github.com/celestiaorg/celestia-app/x/minfee/types"; |
There was a problem hiding this comment.
is this breakage a problem? Would we be better off doing a proto v2 here
chatton
changed the title
This PR updates the minfee module to use self managed params. The diff is quite large as some files needed to be moved around to make the package structure work correctly and there are a lot of proto changes.
ParamSet) with a proto Params type.ValidateTxFeeante handler to look at params instead of subspace.follow up for cel-5