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

refactor(x/gov)!: migrate to use env var #19481

Merged
merged 2 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,20 @@ Additionally, the `appmodule.Environment` interface is introduced to fetch diffe
This should be used as an alternative to using `sdk.UnwrapContext(ctx)` to fetch the services.
It needs to be passed into a module at instantiation.

`x/circuit` is used as an example.:
`x/circuit` is used as an example:

```go
app.CircuitKeeper = circuitkeeper.NewKeeper(runtime.NewEnvironment((keys[circuittypes.StoreKey])), appCodec, authtypes.NewModuleAddress(govtypes.ModuleName).String(), app.AuthKeeper.AddressCodec())
```

If your module requires a message server or query server, it should be passed in the environment as well.

```diff
-govKeeper := govkeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[govtypes.StoreKey]), app.AuthKeeper, app.BankKeeper,app.StakingKeeper, app.PoolKeeper, app.MsgServiceRouter(), govConfig, authtypes.NewModuleAddress(govtypes.ModuleName).String())
+govKeeper := govkeeper.NewKeeper(appCodec, runtime.NewEnvironment(runtime.NewKVStoreService(keys[govtypes.StoreKey]), logger, runtime.EnvWithRouterService(app.GRPCQueryRouter(), app.MsgServiceRouter())), app.AuthKeeper, app.BankKeeper, app.StakingKeeper, app.PoolKeeper, govConfig, authtypes.NewModuleAddress(govtypes.ModuleName).String())
```


##### Dependency Injection

Previously `cosmossdk.io/core` held functions `Invoke`, `Provide` and `Register` were moved to `cosmossdk.io/depinject/appconfig`.
Expand Down Expand Up @@ -210,6 +218,11 @@ Group was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/group

Gov was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/gov`

Gov v1beta1 proposal handler has been changed to take in a `context.Context` instead of `sdk.Context`.
This change was made to allow legacy proposals to be compatible with server/v2.
If you wish to migrate to server/v2, you should update your proposal handler to take in a `context.Context` and use services.
On the other hand, if you wish to keep using baseapp, simply unwrap the sdk context in your proposal handler.

#### `x/mint`

Mint was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/mint`
Expand Down
12 changes: 12 additions & 0 deletions core/gas/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ type Service interface {

// WithBlockGasMeter returns a new context with the provided block-level gas meter.
WithBlockGasMeter(ctx context.Context, meter Meter) context.Context

GetGasConfig(ctx context.Context) GasConfig
}

// Meter represents a gas meter for modules consumption
Expand All @@ -48,3 +50,13 @@ type Meter interface {
Remaining() Gas
Limit() Gas
}

type GasConfig struct {
HasCost Gas
DeleteCost Gas
ReadCostFlat Gas
ReadCostPerByte Gas
WriteCostFlat Gas
WriteCostPerByte Gas
IterNextCostFlat Gas
}
Comment on lines +54 to +62
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GasConfig struct introduces various fields to manage gas costs dynamically. It's well-structured and covers a comprehensive range of gas-related configurations. However, consider adding documentation comments for each field to explain their purpose and usage clearly, enhancing code readability and maintainability.

+ // GasConfig holds the configuration for gas costs in the system.
type GasConfig struct {
+ // HasCost defines the gas cost for checking the existence of an item.
	HasCost          Gas
+ // DeleteCost defines the gas cost for deleting an item.
	DeleteCost       Gas
+ // ReadCostFlat defines the flat gas cost for reading an item.
	ReadCostFlat     Gas
+ // ReadCostPerByte defines the gas cost per byte for reading an item.
	ReadCostPerByte  Gas
+ // WriteCostFlat defines the flat gas cost for writing an item.
	WriteCostFlat    Gas
+ // WriteCostPerByte defines the gas cost per byte for writing an item.
	WriteCostPerByte Gas
+ // IterNextCostFlat defines the flat gas cost for iterating to the next item.
	IterNextCostFlat Gas
}

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
type GasConfig struct {
HasCost Gas
DeleteCost Gas
ReadCostFlat Gas
ReadCostPerByte Gas
WriteCostFlat Gas
WriteCostPerByte Gas
IterNextCostFlat Gas
}
// GasConfig holds the configuration for gas costs in the system.
type GasConfig struct {
// HasCost defines the gas cost for checking the existence of an item.
HasCost Gas
// DeleteCost defines the gas cost for deleting an item.
DeleteCost Gas
// ReadCostFlat defines the flat gas cost for reading an item.
ReadCostFlat Gas
// ReadCostPerByte defines the gas cost per byte for reading an item.
ReadCostPerByte Gas
// WriteCostFlat defines the flat gas cost for writing an item.
WriteCostFlat Gas
// WriteCostPerByte defines the gas cost per byte for writing an item.
WriteCostPerByte Gas
// IterNextCostFlat defines the flat gas cost for iterating to the next item.
IterNextCostFlat Gas
}

36 changes: 36 additions & 0 deletions runtime/gas.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ func (g GasService) WithBlockGasMeter(ctx context.Context, meter gas.Meter) cont
return sdk.UnwrapSDKContext(ctx).WithGasMeter(SDKGasMeter{gm: meter})
}

func (g GasService) GetGasConfig(ctx context.Context) gas.GasConfig {
return gas.GasConfig(sdk.UnwrapSDKContext(ctx).KVGasConfig())
}

// ______________________________________________________________________________________________
// Gas Meter Wrappers
// ______________________________________________________________________________________________
Expand Down Expand Up @@ -98,3 +102,35 @@ func (cgm CoreGasmeter) Remaining() gas.Gas {
func (cgm CoreGasmeter) Limit() gas.Gas {
return cgm.gm.Limit()
}

type GasConfig struct {
gc gas.GasConfig
}

func (gc GasConfig) HasCost() gas.Gas {
return gc.gc.HasCost
}

func (gc GasConfig) DeleteCost() gas.Gas {
return gc.gc.DeleteCost
}

func (gc GasConfig) ReadCostFlat() gas.Gas {
return gc.gc.ReadCostFlat
}

func (gc GasConfig) ReadCostPerByte() gas.Gas {
return gc.gc.ReadCostPerByte
}

func (gc GasConfig) WriteCostFlat() gas.Gas {
return gc.gc.WriteCostFlat
}

func (gc GasConfig) WriteCostPerByte() gas.Gas {
return gc.gc.WriteCostPerByte
}

func (gc GasConfig) IterNextCostFlat() gas.Gas {
return gc.gc.IterNextCostFlat
}
5 changes: 1 addition & 4 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,10 +380,7 @@ func NewSimApp(
Example of setting gov params:
govConfig.MaxMetadataLen = 10000
*/
govKeeper := govkeeper.NewKeeper(
appCodec, runtime.NewKVStoreService(keys[govtypes.StoreKey]), app.AuthKeeper, app.BankKeeper,
app.StakingKeeper, app.PoolKeeper, app.MsgServiceRouter(), govConfig, authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)
govKeeper := govkeeper.NewKeeper(appCodec, runtime.NewEnvironment(runtime.NewKVStoreService(keys[govtypes.StoreKey]), logger, runtime.EnvWithRouterService(app.GRPCQueryRouter(), app.MsgServiceRouter())), app.AuthKeeper, app.BankKeeper, app.StakingKeeper, app.PoolKeeper, govConfig, authtypes.NewModuleAddress(govtypes.ModuleName).String())

// Set legacy router for backwards compatibility with gov v1beta1
govKeeper.SetLegacyRouter(govRouter)
Expand Down
23 changes: 11 additions & 12 deletions tests/integration/gov/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"cosmossdk.io/math"
authtypes "cosmossdk.io/x/auth/types"
banktypes "cosmossdk.io/x/bank/types"
"cosmossdk.io/x/gov"
"cosmossdk.io/x/gov/keeper"
"cosmossdk.io/x/gov/types"
v1 "cosmossdk.io/x/gov/types/v1"
Expand Down Expand Up @@ -41,7 +40,7 @@ func TestUnregisteredProposal_InactiveProposalFails(t *testing.T) {
err = suite.GovKeeper.InactiveProposalsQueue.Set(ctx, collections.Join(endTime, proposal.Id), proposal.Id)
require.NoError(t, err)

err = gov.EndBlocker(ctx, suite.GovKeeper)
err = suite.GovKeeper.EndBlocker(ctx)
require.NoError(t, err)

_, err = suite.GovKeeper.Proposals.Get(ctx, proposal.Id)
Expand Down Expand Up @@ -69,7 +68,7 @@ func TestUnregisteredProposal_ActiveProposalFails(t *testing.T) {
err = suite.GovKeeper.ActiveProposalsQueue.Set(ctx, collections.Join(endTime, proposal.Id), proposal.Id)
require.NoError(t, err)

err = gov.EndBlocker(ctx, suite.GovKeeper)
err = suite.GovKeeper.EndBlocker(ctx)
require.NoError(t, err)

p, err := suite.GovKeeper.Proposals.Get(ctx, proposal.Id)
Expand Down Expand Up @@ -109,7 +108,7 @@ func TestTickExpiredDepositPeriod(t *testing.T) {
newHeader.Time = ctx.HeaderInfo().Time.Add(*params.MaxDepositPeriod)
ctx = ctx.WithHeaderInfo(newHeader)

err = gov.EndBlocker(ctx, suite.GovKeeper)
err = suite.GovKeeper.EndBlocker(ctx)
require.NoError(t, err)
}

Expand Down Expand Up @@ -159,12 +158,12 @@ func TestTickMultipleExpiredDepositPeriod(t *testing.T) {
newHeader.Time = ctx.HeaderInfo().Time.Add(*params.MaxDepositPeriod).Add(time.Duration(-1) * time.Second)
ctx = ctx.WithHeaderInfo(newHeader)

require.NoError(t, gov.EndBlocker(ctx, suite.GovKeeper))
require.NoError(t, suite.GovKeeper.EndBlocker(ctx))

newHeader = ctx.HeaderInfo()
newHeader.Time = ctx.HeaderInfo().Time.Add(time.Duration(5) * time.Second)
ctx = ctx.WithHeaderInfo(newHeader)
require.NoError(t, gov.EndBlocker(ctx, suite.GovKeeper))
require.NoError(t, suite.GovKeeper.EndBlocker(ctx))
}

func TestTickPassedDepositPeriod(t *testing.T) {
Expand Down Expand Up @@ -246,7 +245,7 @@ func TestProposalDepositRefundFailEndBlocker(t *testing.T) {
newHeader.Time = proposal.VotingEndTime.Add(time.Duration(100) * time.Second)
ctx = ctx.WithHeaderInfo(newHeader)

err = gov.EndBlocker(ctx, suite.GovKeeper)
err = suite.GovKeeper.EndBlocker(ctx)
require.NoError(t, err) // no error, means does not halt the chain

events := ctx.EventManager().Events()
Expand Down Expand Up @@ -314,7 +313,7 @@ func TestTickPassedVotingPeriod(t *testing.T) {
require.NoError(t, err)
require.Equal(t, v1.StatusVotingPeriod, proposal.Status)

err = gov.EndBlocker(ctx, suite.GovKeeper)
err = suite.GovKeeper.EndBlocker(ctx)
require.NoError(t, err)

if tc.proposalType != v1.ProposalType_PROPOSAL_TYPE_EXPEDITED {
Expand Down Expand Up @@ -395,7 +394,7 @@ func TestProposalPassedEndblocker(t *testing.T) {
newHeader.Time = ctx.HeaderInfo().Time.Add(*params.MaxDepositPeriod).Add(*params.VotingPeriod)
ctx = ctx.WithHeaderInfo(newHeader)

err = gov.EndBlocker(ctx, suite.GovKeeper)
err = suite.GovKeeper.EndBlocker(ctx)
require.NoError(t, err)
macc = suite.GovKeeper.GetGovernanceAccount(ctx)
require.NotNil(t, macc)
Expand Down Expand Up @@ -450,7 +449,7 @@ func TestEndBlockerProposalHandlerFailed(t *testing.T) {
ctx = ctx.WithHeaderInfo(newHeader)

// validate that the proposal fails/has been rejected
err = gov.EndBlocker(ctx, suite.GovKeeper)
err = suite.GovKeeper.EndBlocker(ctx)
require.NoError(t, err)
// check proposal events
events := ctx.EventManager().Events()
Expand Down Expand Up @@ -553,7 +552,7 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) {
}

// Here the expedited proposal is converted to regular after expiry.
err = gov.EndBlocker(ctx, suite.GovKeeper)
err = suite.GovKeeper.EndBlocker(ctx)
require.NoError(t, err)
if tc.expeditedPasses {
proposal, err = suite.GovKeeper.Proposals.Get(ctx, res.ProposalId)
Expand Down Expand Up @@ -602,7 +601,7 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) {
}

// Here we validate the converted regular proposal
err = gov.EndBlocker(ctx, suite.GovKeeper)
err = suite.GovKeeper.EndBlocker(ctx)
require.NoError(t, err)
macc = suite.GovKeeper.GetGovernanceAccount(ctx)
require.NotNil(t, macc)
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/gov/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func TestImportExportQueues(t *testing.T) {
assert.DeepEqual(t, sdk.Coins(params.MinDeposit), s2.BankKeeper.GetAllBalances(ctx2, macc.GetAddress()))

// Run the endblocker. Check to make sure that proposal1 is removed from state, and proposal2 is finished VotingPeriod.
err = gov.EndBlocker(ctx2, s2.GovKeeper)
err = s2.GovKeeper.EndBlocker(ctx2)
assert.NilError(t, err)

proposal1, err = s2.GovKeeper.Proposals.Get(ctx2, proposalID1)
Expand Down
5 changes: 3 additions & 2 deletions tests/integration/gov/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,16 @@ func initFixture(tb testing.TB) *fixture {
// keeper.
router := baseapp.NewMsgServiceRouter()
router.SetInterfaceRegistry(cdc.InterfaceRegistry())
queryRouter := baseapp.NewGRPCQueryRouter()
queryRouter.SetInterfaceRegistry(cdc.InterfaceRegistry())

govKeeper := keeper.NewKeeper(
cdc,
runtime.NewKVStoreService(keys[types.StoreKey]),
runtime.NewEnvironment(runtime.NewKVStoreService(keys[types.StoreKey]), log.NewNopLogger(), runtime.EnvWithRouterService(queryRouter, router)),
accountKeeper,
bankKeeper,
stakingKeeper,
poolKeeper,
router,
keeper.DefaultConfig(),
authority.String(),
)
Expand Down
4 changes: 4 additions & 0 deletions x/accounts/internal/implementation/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ func (g gasService) GetBlockGasMeter(ctx context.Context) gas.Meter {
return g.gs.GetBlockGasMeter(getParentContext(ctx))
}

func (g gasService) GetGasConfig(ctx context.Context) gas.GasConfig {
return g.gs.GetGasConfig(getParentContext(ctx))
}

func (g gasService) WithGasMeter(ctx context.Context, meter gas.Meter) context.Context {
v := getCtx(ctx)
v.parentContext = g.gs.WithGasMeter(v.parentContext, meter)
Expand Down
2 changes: 2 additions & 0 deletions x/gov/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* [#19481](https://github.com/cosmos/cosmos-sdk/pull/19481) Migrate module to use `appmodule.Environment`; `NewKeeper` now takes `appmodule.Environment` instead of a store service and no `baseapp.MessageRouter` anymore.
* [#19481](https://github.com/cosmos/cosmos-sdk/pull/19481) v1beta1 proposal handlers now take a `context.Context` instead of an `sdk.Context`.
* [#19592](https://github.com/cosmos/cosmos-sdk/pull/19592) `types.Config` and `types.DefaultConfig` have been moved to the keeper package in order to support the custom tallying function.
* [#19349](https://github.com/cosmos/cosmos-sdk/pull/19349) Simplify state management in `x/gov`. Note `k.VotingPeriodProposals` and `k.SetProposal` are no longer needed and have been removed.
* [#18532](https://github.com/cosmos/cosmos-sdk/pull/18532) All functions that were taking an expedited bool parameter now take a `ProposalType` parameter instead.
Expand Down
32 changes: 0 additions & 32 deletions x/gov/abci_internal_test.go

This file was deleted.

8 changes: 2 additions & 6 deletions x/gov/depinject.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (

modulev1 "cosmossdk.io/api/cosmos/gov/module/v1"
"cosmossdk.io/core/appmodule"
store "cosmossdk.io/core/store"
"cosmossdk.io/depinject"
"cosmossdk.io/depinject/appconfig"
authtypes "cosmossdk.io/x/auth/types"
Expand All @@ -19,7 +18,6 @@ import (
govtypes "cosmossdk.io/x/gov/types"
"cosmossdk.io/x/gov/types/v1beta1"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/codec"
)

Expand All @@ -40,9 +38,8 @@ type ModuleInputs struct {

Config *modulev1.Module
Cdc codec.Codec
StoreService store.KVStoreService
Environment appmodule.Environment
ModuleKey depinject.OwnModuleKey
MsgServiceRouter baseapp.MessageRouter
LegacyProposalHandler []govclient.ProposalHandler `optional:"true"`

AccountKeeper govtypes.AccountKeeper
Expand Down Expand Up @@ -82,12 +79,11 @@ func ProvideModule(in ModuleInputs) ModuleOutputs {

k := keeper.NewKeeper(
in.Cdc,
in.StoreService,
in.Environment,
in.AccountKeeper,
in.BankKeeper,
in.StakingKeeper,
in.PoolKeeper,
in.MsgServiceRouter,
defaultConfig,
authority.String(),
)
Expand Down
Loading
Loading