From 537708f5642c5fecba7412b9519541c113673449 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 4 Mar 2024 15:41:26 +0100 Subject: [PATCH] refactor(x/gov)!: migrate to use env var --- UPGRADING.md | 15 +- core/gas/service.go | 12 ++ runtime/gas.go | 36 ++++ runtime/router.go | 5 + simapp/app.go | 5 +- tests/integration/gov/abci_test.go | 23 +- tests/integration/gov/genesis_test.go | 2 +- tests/integration/gov/keeper/keeper_test.go | 5 +- x/accounts/internal/implementation/context.go | 4 + x/gov/CHANGELOG.md | 2 + x/gov/abci_internal_test.go | 32 --- x/gov/depinject.go | 8 +- x/gov/{ => keeper}/abci.go | 204 +++++++++--------- x/gov/keeper/abci_internal_test.go | 39 ++++ x/gov/keeper/common_test.go | 4 +- x/gov/keeper/deposit.go | 16 +- x/gov/keeper/hooks_test.go | 5 +- x/gov/keeper/keeper.go | 32 +-- x/gov/keeper/migrations.go | 4 +- x/gov/keeper/msg_server.go | 96 +++++---- x/gov/keeper/msg_server_test.go | 2 +- x/gov/keeper/proposal.go | 57 ++--- x/gov/keeper/vote.go | 18 +- x/gov/module.go | 3 +- x/gov/types/v1beta1/content.go | 4 +- x/gov/types/v1beta1/proposal.go | 3 +- x/params/proposal_handler.go | 9 +- 27 files changed, 353 insertions(+), 292 deletions(-) delete mode 100644 x/gov/abci_internal_test.go rename x/gov/{ => keeper}/abci.go (55%) create mode 100644 x/gov/keeper/abci_internal_test.go diff --git a/UPGRADING.md b/UPGRADING.md index 8ed75edacba2..b044b0265b3c 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -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`. @@ -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` diff --git a/core/gas/service.go b/core/gas/service.go index 9495201aa4c0..40d212e943f1 100644 --- a/core/gas/service.go +++ b/core/gas/service.go @@ -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 @@ -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 +} diff --git a/runtime/gas.go b/runtime/gas.go index 5a18ec511b54..f6e28c9f2f56 100644 --- a/runtime/gas.go +++ b/runtime/gas.go @@ -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 // ______________________________________________________________________________________________ @@ -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 +} diff --git a/runtime/router.go b/runtime/router.go index 441ad935e479..5e2a8a8e9ba9 100644 --- a/runtime/router.go +++ b/runtime/router.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "reflect" + "strings" "github.com/cosmos/gogoproto/proto" protov2 "google.golang.org/protobuf/proto" @@ -59,6 +60,8 @@ func (m *msgRouterService) CanInvoke(ctx context.Context, typeURL string) error return fmt.Errorf("missing type url") } + typeURL = strings.TrimPrefix(typeURL, "/") + handler := m.router.HybridHandlerByMsgName(typeURL) if handler == nil { return fmt.Errorf("unknown message: %s", typeURL) @@ -114,6 +117,8 @@ func (m *queryRouterService) CanInvoke(ctx context.Context, typeURL string) erro return fmt.Errorf("missing type url") } + typeURL = strings.TrimPrefix(typeURL, "/") + handlers := m.router.HybridHandlerByRequestName(typeURL) if len(handlers) == 0 { return fmt.Errorf("unknown request: %s", typeURL) diff --git a/simapp/app.go b/simapp/app.go index 87cc1be99684..7a4582e218e7 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -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) diff --git a/tests/integration/gov/abci_test.go b/tests/integration/gov/abci_test.go index 25816e948ff0..dbe612a20496 100644 --- a/tests/integration/gov/abci_test.go +++ b/tests/integration/gov/abci_test.go @@ -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" @@ -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) @@ -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) @@ -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) } @@ -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) { @@ -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() @@ -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 { @@ -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) @@ -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() @@ -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) @@ -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) diff --git a/tests/integration/gov/genesis_test.go b/tests/integration/gov/genesis_test.go index 77b0701953b0..265ecb66d94c 100644 --- a/tests/integration/gov/genesis_test.go +++ b/tests/integration/gov/genesis_test.go @@ -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) diff --git a/tests/integration/gov/keeper/keeper_test.go b/tests/integration/gov/keeper/keeper_test.go index 68561f4a7336..39e7bcd0fb9e 100644 --- a/tests/integration/gov/keeper/keeper_test.go +++ b/tests/integration/gov/keeper/keeper_test.go @@ -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(), ) diff --git a/x/accounts/internal/implementation/context.go b/x/accounts/internal/implementation/context.go index b47b7843a7c7..01ce89a5fb6f 100644 --- a/x/accounts/internal/implementation/context.go +++ b/x/accounts/internal/implementation/context.go @@ -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) diff --git a/x/gov/CHANGELOG.md b/x/gov/CHANGELOG.md index 0524fd23ff8b..116e87afc38c 100644 --- a/x/gov/CHANGELOG.md +++ b/x/gov/CHANGELOG.md @@ -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. diff --git a/x/gov/abci_internal_test.go b/x/gov/abci_internal_test.go deleted file mode 100644 index 1421a81b5cb6..000000000000 --- a/x/gov/abci_internal_test.go +++ /dev/null @@ -1,32 +0,0 @@ -package gov - -import ( - "testing" - - "github.com/stretchr/testify/require" - - sdk "github.com/cosmos/cosmos-sdk/types" -) - -func failingHandler(_ sdk.Context, _ sdk.Msg) (*sdk.Result, error) { - panic("test-fail") -} - -func okHandler(_ sdk.Context, _ sdk.Msg) (*sdk.Result, error) { - return new(sdk.Result), nil -} - -func TestSafeExecuteHandler(t *testing.T) { - t.Parallel() - - require := require.New(t) - var ctx sdk.Context - - r, err := safeExecuteHandler(ctx, nil, failingHandler) - require.ErrorContains(err, "test-fail") - require.Nil(r) - - r, err = safeExecuteHandler(ctx, nil, okHandler) - require.Nil(err) - require.NotNil(r) -} diff --git a/x/gov/depinject.go b/x/gov/depinject.go index 3776ddf46025..0a24603c421c 100644 --- a/x/gov/depinject.go +++ b/x/gov/depinject.go @@ -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" @@ -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" ) @@ -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 @@ -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(), ) diff --git a/x/gov/abci.go b/x/gov/keeper/abci.go similarity index 55% rename from x/gov/abci.go rename to x/gov/keeper/abci.go index 6f8c44f61d8f..9234377be88e 100644 --- a/x/gov/abci.go +++ b/x/gov/keeper/abci.go @@ -1,42 +1,45 @@ -package gov +package keeper import ( + "context" "errors" "fmt" "time" + "google.golang.org/protobuf/runtime/protoiface" + "cosmossdk.io/collections" + "cosmossdk.io/core/event" + "cosmossdk.io/core/router" "cosmossdk.io/log" - "cosmossdk.io/x/gov/keeper" "cosmossdk.io/x/gov/types" v1 "cosmossdk.io/x/gov/types/v1" - "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" ) // EndBlocker is called every block. -func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { +func (k Keeper) EndBlocker(ctx context.Context) error { defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker) - logger := keeper.Logger(ctx) + logger := k.Logger() // delete dead proposals from store and returns theirs deposits. // A proposal is dead when it's inactive and didn't get enough deposit on time to get into voting phase. - rng := collections.NewPrefixUntilPairRange[time.Time, uint64](ctx.HeaderInfo().Time) - err := keeper.InactiveProposalsQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, uint64], _ uint64) (bool, error) { - proposal, err := keeper.Proposals.Get(ctx, key.K2()) + rng := collections.NewPrefixUntilPairRange[time.Time, uint64](k.environment.HeaderService.GetHeaderInfo(ctx).Time) + err := k.InactiveProposalsQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, uint64], _ uint64) (bool, error) { + proposal, err := k.Proposals.Get(ctx, key.K2()) if err != nil { // if the proposal has an encoding error, this means it cannot be processed by x/gov // this could be due to some types missing their registration // instead of returning an error (i.e, halting the chain), we fail the proposal if errors.Is(err, collections.ErrEncoding) { proposal.Id = key.K2() - if err := failUnsupportedProposal(logger, ctx, keeper, proposal, err.Error(), false); err != nil { + if err := failUnsupportedProposal(logger, ctx, k, proposal, err.Error(), false); err != nil { return false, err } - if err = keeper.DeleteProposal(ctx, proposal.Id); err != nil { + if err = k.DeleteProposal(ctx, proposal.Id); err != nil { return false, err } @@ -46,18 +49,18 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { return false, err } - if err = keeper.DeleteProposal(ctx, proposal.Id); err != nil { + if err = k.DeleteProposal(ctx, proposal.Id); err != nil { return false, err } - params, err := keeper.Params.Get(ctx) + params, err := k.Params.Get(ctx) if err != nil { return false, err } if !params.BurnProposalDepositPrevote { - err = keeper.RefundAndDeleteDeposits(ctx, proposal.Id) // refund deposit if proposal got removed without getting 100% of the proposal + err = k.RefundAndDeleteDeposits(ctx, proposal.Id) // refund deposit if proposal got removed without getting 100% of the proposal } else { - err = keeper.DeleteAndBurnDeposits(ctx, proposal.Id) // burn the deposit if proposal got removed without getting 100% of the proposal + err = k.DeleteAndBurnDeposits(ctx, proposal.Id) // burn the deposit if proposal got removed without getting 100% of the proposal } if err != nil { @@ -65,21 +68,20 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { } // called when proposal become inactive - cacheCtx, writeCache := ctx.CacheContext() - err = keeper.Hooks().AfterProposalFailedMinDeposit(cacheCtx, proposal.Id) - if err == nil { // purposely ignoring the error here not to halt the chain if the hook fails - writeCache() - } else { + // call hook when proposal become inactive + if err := k.environment.BranchService.Execute(ctx, func(ctx context.Context) error { + return k.Hooks().AfterProposalFailedMinDeposit(ctx, proposal.Id) + }); err != nil { + // purposely ignoring the error here not to halt the chain if the hook fails logger.Error("failed to execute AfterProposalFailedMinDeposit hook", "error", err) } - ctx.EventManager().EmitEvent( - sdk.NewEvent( - types.EventTypeInactiveProposal, - sdk.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposal.Id)), - sdk.NewAttribute(types.AttributeKeyProposalResult, types.AttributeValueProposalDropped), - ), - ) + if err := k.environment.EventService.EventManager(ctx).EmitKV(types.EventTypeInactiveProposal, + event.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposal.Id)), + event.NewAttribute(types.AttributeKeyProposalResult, types.AttributeValueProposalDropped), + ); err != nil { + logger.Error("failed to emit event", "error", err) + } logger.Info( "proposal did not meet minimum deposit; deleted", @@ -97,20 +99,20 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { } // fetch active proposals whose voting periods have ended (are passed the block time) - rng = collections.NewPrefixUntilPairRange[time.Time, uint64](ctx.HeaderInfo().Time) - err = keeper.ActiveProposalsQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, uint64], _ uint64) (bool, error) { - proposal, err := keeper.Proposals.Get(ctx, key.K2()) + rng = collections.NewPrefixUntilPairRange[time.Time, uint64](k.environment.HeaderService.GetHeaderInfo(ctx).Time) + err = k.ActiveProposalsQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, uint64], _ uint64) (bool, error) { + proposal, err := k.Proposals.Get(ctx, key.K2()) if err != nil { // if the proposal has an encoding error, this means it cannot be processed by x/gov // this could be due to some types missing their registration // instead of returning an error (i.e, halting the chain), we fail the proposal if errors.Is(err, collections.ErrEncoding) { proposal.Id = key.K2() - if err := failUnsupportedProposal(logger, ctx, keeper, proposal, err.Error(), true); err != nil { + if err := failUnsupportedProposal(logger, ctx, k, proposal, err.Error(), true); err != nil { return false, err } - if err = keeper.ActiveProposalsQueue.Remove(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id)); err != nil { + if err = k.ActiveProposalsQueue.Remove(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id)); err != nil { return false, err } @@ -122,7 +124,7 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { var tagValue, logMsg string - passes, burnDeposits, tallyResults, err := keeper.Tally(ctx, proposal) + passes, burnDeposits, tallyResults, err := k.Tally(ctx, proposal) if err != nil { return false, err } @@ -132,43 +134,36 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { // If a proposal fails, and isn't spammy, deposits are refunded, unless the proposal is expedited or optimistic. // An expedited or optimistic proposal that fails and isn't spammy is converted to a regular proposal. if burnDeposits { - err = keeper.DeleteAndBurnDeposits(ctx, proposal.Id) + err = k.DeleteAndBurnDeposits(ctx, proposal.Id) } else if passes || !(proposal.ProposalType == v1.ProposalType_PROPOSAL_TYPE_EXPEDITED || proposal.ProposalType == v1.ProposalType_PROPOSAL_TYPE_OPTIMISTIC) { - err = keeper.RefundAndDeleteDeposits(ctx, proposal.Id) + err = k.RefundAndDeleteDeposits(ctx, proposal.Id) } if err != nil { // in case of an error, log it and emit an event // we do not want to halt the chain if the refund/burn fails // as it could happen due to a governance mistake (governance has let a proposal pass that sends gov funds that were from proposal deposits) - - keeper.Logger(ctx).Error("failed to refund or burn deposits", "error", err) - ctx.EventManager().EmitEvent( - sdk.NewEvent( - types.EventTypeProposalDeposit, - sdk.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposal.Id)), - sdk.NewAttribute(types.AttributeKeyProposalDepositError, "failed to refund or burn deposits"), - sdk.NewAttribute("error", err.Error()), - ), - ) + k.Logger().Error("failed to refund or burn deposits", "error", err) + + if err := k.environment.EventService.EventManager(ctx).EmitKV(types.EventTypeProposalDeposit, + event.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposal.Id)), + event.NewAttribute(types.AttributeKeyProposalDepositError, "failed to refund or burn deposits"), + event.NewAttribute("error", err.Error()), + ); err != nil { + k.Logger().Error("failed to emit event", "error", err) + } } - if err = keeper.ActiveProposalsQueue.Remove(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id)); err != nil { + if err = k.ActiveProposalsQueue.Remove(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id)); err != nil { return false, err } switch { case passes: var ( - idx int - events sdk.Events - msg sdk.Msg + idx int + msg sdk.Msg ) - // attempt to execute all messages within the passed proposal - // Messages may mutate state thus we use a cached context. If one of - // the handlers fails, no state mutation is written and the error - // message is logged. - cacheCtx, writeCache := ctx.CacheContext() messages, err := proposal.GetMsgs() if err != nil { proposal.Status = v1.StatusFailed @@ -179,35 +174,31 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { break } - // execute all messages - for idx, msg = range messages { - handler := keeper.Router().Handler(msg) - var res *sdk.Result - res, err = safeExecuteHandler(cacheCtx, msg, handler) - if err != nil { - break + // attempt to execute all messages within the passed proposal + // Messages may mutate state thus we use a cached context. If one of + // the handlers fails, no state mutation is written and the error + // message is logged. + if err := k.environment.BranchService.Execute(ctx, func(ctx context.Context) error { + // execute all messages + for idx, msg = range messages { + if _, err := safeExecuteHandler(ctx, msg, k.environment.RouterService.MessageRouterService()); err != nil { + // `idx` and `err` are populated with the msg index and error. + proposal.Status = v1.StatusFailed + proposal.FailedReason = err.Error() + tagValue = types.AttributeValueProposalFailed + logMsg = fmt.Sprintf("passed, but msg %d (%s) failed on execution: %s", idx, sdk.MsgTypeURL(msg), err) + + return err + } } - events = append(events, res.GetEvents()...) - } - - // `err == nil` when all handlers passed. - // Or else, `idx` and `err` are populated with the msg index and error. - if err == nil { proposal.Status = v1.StatusPassed tagValue = types.AttributeValueProposalPassed logMsg = "passed" - // write state to the underlying multi-store - writeCache() - - // propagate the msg events to the current context - ctx.EventManager().EmitEvents(events) - } else { - proposal.Status = v1.StatusFailed - proposal.FailedReason = err.Error() - tagValue = types.AttributeValueProposalFailed - logMsg = fmt.Sprintf("passed, but msg %d (%s) failed on execution: %s", idx, sdk.MsgTypeURL(msg), err) + return nil + }); err != nil { + break // We do not anything with the error. Returning an error halts the chain, and proposal struct is already updated. } case !burnDeposits && (proposal.ProposalType == v1.ProposalType_PROPOSAL_TYPE_EXPEDITED || proposal.ProposalType == v1.ProposalType_PROPOSAL_TYPE_OPTIMISTIC): @@ -217,14 +208,14 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { // according to the regular proposal rules. proposal.ProposalType = v1.ProposalType_PROPOSAL_TYPE_STANDARD proposal.Expedited = false // can be removed as never read but kept for state coherence - params, err := keeper.Params.Get(ctx) + params, err := k.Params.Get(ctx) if err != nil { return false, err } endTime := proposal.VotingStartTime.Add(*params.VotingPeriod) proposal.VotingEndTime = &endTime - err = keeper.ActiveProposalsQueue.Set(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id), proposal.Id) + err = k.ActiveProposalsQueue.Set(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id), proposal.Id) if err != nil { return false, err } @@ -245,16 +236,15 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { proposal.FinalTallyResult = &tallyResults - if err = keeper.Proposals.Set(ctx, proposal.Id, proposal); err != nil { + if err = k.Proposals.Set(ctx, proposal.Id, proposal); err != nil { return false, err } - // when proposal become active - cacheCtx, writeCache := ctx.CacheContext() - err = keeper.Hooks().AfterProposalVotingPeriodEnded(cacheCtx, proposal.Id) - if err == nil { // purposely ignoring the error here not to halt the chain if the hook fails - writeCache() - } else { + // call hook when proposal become active + if err := k.environment.BranchService.Execute(ctx, func(ctx context.Context) error { + return k.Hooks().AfterProposalVotingPeriodEnded(ctx, proposal.Id) + }); err != nil { + // purposely ignoring the error here not to halt the chain if the hook fails logger.Error("failed to execute AfterProposalVotingPeriodEnded hook", "error", err) } @@ -267,37 +257,36 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { "results", logMsg, ) - ctx.EventManager().EmitEvent( - sdk.NewEvent( - types.EventTypeActiveProposal, - sdk.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposal.Id)), - sdk.NewAttribute(types.AttributeKeyProposalResult, tagValue), - sdk.NewAttribute(types.AttributeKeyProposalLog, logMsg), - ), - ) + if err := k.environment.EventService.EventManager(ctx).EmitKV(types.EventTypeActiveProposal, + event.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposal.Id)), + event.NewAttribute(types.AttributeKeyProposalResult, tagValue), + event.NewAttribute(types.AttributeKeyProposalLog, logMsg), + ); err != nil { + logger.Error("failed to emit event", "error", err) + } return false, nil }) return err } -// executes handle(msg) and recovers from panic. -func safeExecuteHandler(ctx sdk.Context, msg sdk.Msg, handler baseapp.MsgServiceHandler, -) (res *sdk.Result, err error) { +// executes route(msg) and recovers from panic. +func safeExecuteHandler(ctx context.Context, msg sdk.Msg, router router.Router) (res protoiface.MessageV1, err error) { defer func() { if r := recover(); r != nil { err = fmt.Errorf("handling x/gov proposal msg [%s] PANICKED: %v", msg, r) } }() - res, err = handler(ctx, msg) + + res, err = router.InvokeUntyped(ctx, msg) return } // failUnsupportedProposal fails a proposal that cannot be processed by gov func failUnsupportedProposal( logger log.Logger, - ctx sdk.Context, - keeper *keeper.Keeper, + ctx context.Context, + k Keeper, proposal v1.Proposal, errMsg string, active bool, @@ -306,11 +295,11 @@ func failUnsupportedProposal( proposal.FailedReason = fmt.Sprintf("proposal failed because it cannot be processed by gov: %s", errMsg) proposal.Messages = nil // clear out the messages - if err := keeper.Proposals.Set(ctx, proposal.Id, proposal); err != nil { + if err := k.Proposals.Set(ctx, proposal.Id, proposal); err != nil { return err } - if err := keeper.RefundAndDeleteDeposits(ctx, proposal.Id); err != nil { + if err := k.RefundAndDeleteDeposits(ctx, proposal.Id); err != nil { return err } @@ -319,13 +308,12 @@ func failUnsupportedProposal( eventType = types.EventTypeActiveProposal } - ctx.EventManager().EmitEvent( - sdk.NewEvent( - eventType, - sdk.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposal.Id)), - sdk.NewAttribute(types.AttributeKeyProposalResult, types.AttributeValueProposalFailed), - ), - ) + if err := k.environment.EventService.EventManager(ctx).EmitKV(eventType, + event.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposal.Id)), + event.NewAttribute(types.AttributeKeyProposalResult, types.AttributeValueProposalFailed), + ); err != nil { + logger.Error("failed to emit event", "error", err) + } logger.Info( "proposal failed to decode; deleted", diff --git a/x/gov/keeper/abci_internal_test.go b/x/gov/keeper/abci_internal_test.go new file mode 100644 index 000000000000..62c2c3403af8 --- /dev/null +++ b/x/gov/keeper/abci_internal_test.go @@ -0,0 +1,39 @@ +package keeper + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/runtime/protoiface" + + "cosmossdk.io/core/router" +) + +type mockRouter struct { + router.Router + + panic bool +} + +func (m *mockRouter) InvokeUntyped(ctx context.Context, req protoiface.MessageV1) (res protoiface.MessageV1, err error) { + if m.panic { + panic("test-fail") + } + + return nil, nil +} + +func TestSafeExecuteHandler(t *testing.T) { + t.Parallel() + + require := require.New(t) + ctx := context.Background() + + r, err := safeExecuteHandler(ctx, nil, &mockRouter{panic: true}) + require.ErrorContains(err, "test-fail") + require.Nil(r) + + _, err = safeExecuteHandler(ctx, nil, &mockRouter{panic: false}) + require.Nil(err) +} diff --git a/x/gov/keeper/common_test.go b/x/gov/keeper/common_test.go index fb78f6fca0ca..5cde59811d31 100644 --- a/x/gov/keeper/common_test.go +++ b/x/gov/keeper/common_test.go @@ -112,6 +112,8 @@ func setupGovKeeper(t *testing.T, expectations ...func(sdk.Context, mocks)) ( baseApp.SetCMS(testCtx.CMS) baseApp.SetInterfaceRegistry(encCfg.InterfaceRegistry) + environment := runtime.NewEnvironment(storeService, log.NewNopLogger(), runtime.EnvWithRouterService(baseApp.GRPCQueryRouter(), baseApp.MsgServiceRouter())) + // gomock initializations ctrl := gomock.NewController(t) m := mocks{ @@ -130,7 +132,7 @@ func setupGovKeeper(t *testing.T, expectations ...func(sdk.Context, mocks)) ( // Gov keeper initializations - govKeeper := keeper.NewKeeper(encCfg.Codec, storeService, m.acctKeeper, m.bankKeeper, m.stakingKeeper, m.poolKeeper, baseApp.MsgServiceRouter(), keeper.DefaultConfig(), govAcct.String()) + govKeeper := keeper.NewKeeper(encCfg.Codec, environment, m.acctKeeper, m.bankKeeper, m.stakingKeeper, m.poolKeeper, keeper.DefaultConfig(), govAcct.String()) require.NoError(t, govKeeper.ProposalID.Set(ctx, 1)) govRouter := v1beta1.NewRouter() // Also register legacy gov handlers to test them too. govRouter.AddRoute(types.RouterKey, v1beta1.ProposalHandler) diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index ece9eac79059..fc05386ce8e2 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -6,6 +6,7 @@ import ( "strings" "cosmossdk.io/collections" + "cosmossdk.io/core/event" "cosmossdk.io/errors" sdkmath "cosmossdk.io/math" "cosmossdk.io/x/gov/types" @@ -176,14 +177,13 @@ func (k Keeper) AddDeposit(ctx context.Context, proposalID uint64, depositorAddr return false, err } - sdkCtx := sdk.UnwrapSDKContext(ctx) - sdkCtx.EventManager().EmitEvent( - sdk.NewEvent( - types.EventTypeProposalDeposit, - sdk.NewAttribute(sdk.AttributeKeyAmount, depositAmount.String()), - sdk.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposalID)), - ), - ) + if err := k.environment.EventService.EventManager(ctx).EmitKV( + types.EventTypeProposalDeposit, + event.NewAttribute(sdk.AttributeKeyAmount, depositAmount.String()), + event.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposalID)), + ); err != nil { + return false, err + } err = k.SetDeposit(ctx, deposit) if err != nil { diff --git a/x/gov/keeper/hooks_test.go b/x/gov/keeper/hooks_test.go index 85d187294c7a..c6861a488f57 100644 --- a/x/gov/keeper/hooks_test.go +++ b/x/gov/keeper/hooks_test.go @@ -7,7 +7,6 @@ import ( "github.com/stretchr/testify/require" - "cosmossdk.io/x/gov" "cosmossdk.io/x/gov/keeper" "cosmossdk.io/x/gov/types" v1 "cosmossdk.io/x/gov/types/v1" @@ -83,7 +82,7 @@ func TestHooks(t *testing.T) { newHeader := ctx.HeaderInfo() newHeader.Time = ctx.HeaderInfo().Time.Add(*params.MaxDepositPeriod).Add(time.Duration(1) * time.Second) ctx = ctx.WithHeaderInfo(newHeader) - err = gov.EndBlocker(ctx, govKeeper) + err = govKeeper.EndBlocker(ctx) require.NoError(t, err) require.True(t, govHooksReceiver.AfterProposalFailedMinDepositValid) @@ -103,7 +102,7 @@ func TestHooks(t *testing.T) { newHeader = ctx.HeaderInfo() newHeader.Time = ctx.HeaderInfo().Time.Add(*params.VotingPeriod).Add(time.Duration(1) * time.Second) ctx = ctx.WithHeaderInfo(newHeader) - err = gov.EndBlocker(ctx, govKeeper) + err = govKeeper.EndBlocker(ctx) require.NoError(t, err) require.True(t, govHooksReceiver.AfterProposalVotingPeriodEndedValid) } diff --git a/x/gov/keeper/keeper.go b/x/gov/keeper/keeper.go index 6ffa959c71c1..0610da71e1d5 100644 --- a/x/gov/keeper/keeper.go +++ b/x/gov/keeper/keeper.go @@ -7,13 +7,12 @@ import ( "time" "cosmossdk.io/collections" - corestoretypes "cosmossdk.io/core/store" + "cosmossdk.io/core/appmodule" "cosmossdk.io/log" "cosmossdk.io/x/gov/types" v1 "cosmossdk.io/x/gov/types/v1" "cosmossdk.io/x/gov/types/v1beta1" - "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -23,25 +22,21 @@ type Keeper struct { authKeeper types.AccountKeeper bankKeeper types.BankKeeper poolKeeper types.PoolKeeper - // The reference to the DelegationSet and ValidatorSet to get information about validators and delegators sk types.StakingKeeper // GovHooks hooks types.GovHooks - // The (unexposed) keys used to access the stores from the Context. - storeService corestoretypes.KVStoreService - // The codec for binary encoding/decoding. cdc codec.Codec + // Module environment + environment appmodule.Environment + // Legacy Proposal router legacyRouter v1beta1.Router - // Msg server router - router baseapp.MessageRouter - // Config represent extra module configuration config Config @@ -87,9 +82,9 @@ func (k Keeper) GetAuthority() string { // // CONTRACT: the parameter Subspace must have the param key table already initialized func NewKeeper( - cdc codec.Codec, storeService corestoretypes.KVStoreService, authKeeper types.AccountKeeper, + cdc codec.Codec, env appmodule.Environment, authKeeper types.AccountKeeper, bankKeeper types.BankKeeper, sk types.StakingKeeper, pk types.PoolKeeper, - router baseapp.MessageRouter, config Config, authority string, + config Config, authority string, ) *Keeper { // ensure governance module account is set if addr := authKeeper.GetModuleAddress(types.ModuleName); addr == nil { @@ -114,15 +109,14 @@ func NewKeeper( config.MaxSummaryLen = defaultConfig.MaxSummaryLen } - sb := collections.NewSchemaBuilder(storeService) + sb := collections.NewSchemaBuilder(env.KVStoreService) k := &Keeper{ - storeService: storeService, + environment: env, authKeeper: authKeeper, bankKeeper: bankKeeper, sk: sk, poolKeeper: pk, cdc: cdc, - router: router, config: config, authority: authority, Constitution: collections.NewItem(sb, types.ConstitutionKey, "constitution", collections.StringValue), @@ -175,14 +169,8 @@ func (k *Keeper) SetLegacyRouter(router v1beta1.Router) { } // Logger returns a module-specific logger. -func (k Keeper) Logger(ctx context.Context) log.Logger { - sdkCtx := sdk.UnwrapSDKContext(ctx) - return sdkCtx.Logger().With("module", "x/"+types.ModuleName) -} - -// Router returns the gov keeper's router -func (k Keeper) Router() baseapp.MessageRouter { - return k.router +func (k Keeper) Logger() log.Logger { + return k.environment.Logger.With("module", "x/"+types.ModuleName) } // LegacyRouter returns the gov keeper's legacy router diff --git a/x/gov/keeper/migrations.go b/x/gov/keeper/migrations.go index d8e02e0d3f8e..d6278326f7a0 100644 --- a/x/gov/keeper/migrations.go +++ b/x/gov/keeper/migrations.go @@ -36,10 +36,10 @@ func (m Migrator) Migrate3to4(ctx context.Context) error { // Migrate4to5 migrates from version 4 to 5. func (m Migrator) Migrate4to5(ctx context.Context) error { - return v5.MigrateStore(ctx, m.keeper.storeService, m.keeper.cdc, m.keeper.Constitution) + return v5.MigrateStore(ctx, m.keeper.environment.KVStoreService, m.keeper.cdc, m.keeper.Constitution) } // Migrate4to5 migrates from version 5 to 6. func (m Migrator) Migrate5to6(ctx context.Context) error { - return v6.MigrateStore(ctx, m.keeper.storeService, m.keeper.Params, m.keeper.Proposals) + return v6.MigrateStore(ctx, m.keeper.environment.KVStoreService, m.keeper.Params, m.keeper.Proposals) } diff --git a/x/gov/keeper/msg_server.go b/x/gov/keeper/msg_server.go index e6cc63a2b8f6..1799cbefcd87 100644 --- a/x/gov/keeper/msg_server.go +++ b/x/gov/keeper/msg_server.go @@ -5,6 +5,9 @@ import ( "encoding/json" "fmt" + "google.golang.org/protobuf/runtime/protoiface" + + "cosmossdk.io/core/event" "cosmossdk.io/errors" "cosmossdk.io/math" govtypes "cosmossdk.io/x/gov/types" @@ -28,7 +31,7 @@ func NewMsgServerImpl(keeper *Keeper) v1.MsgServer { var _ v1.MsgServer = msgServer{} // SubmitProposal implements the MsgServer.SubmitProposal method. -func (k msgServer) SubmitProposal(goCtx context.Context, msg *v1.MsgSubmitProposal) (*v1.MsgSubmitProposalResponse, error) { +func (k msgServer) SubmitProposal(ctx context.Context, msg *v1.MsgSubmitProposal) (*v1.MsgSubmitProposalResponse, error) { proposer, err := k.authKeeper.AddressCodec().StringToBytes(msg.GetProposer()) if err != nil { return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid proposer address: %s", err) @@ -67,7 +70,6 @@ func (k msgServer) SubmitProposal(goCtx context.Context, msg *v1.MsgSubmitPropos return nil, err } - ctx := sdk.UnwrapSDKContext(goCtx) params, err := k.Params.Get(ctx) if err != nil { return nil, fmt.Errorf("failed to get governance parameters: %w", err) @@ -95,8 +97,8 @@ func (k msgServer) SubmitProposal(goCtx context.Context, msg *v1.MsgSubmitPropos } // ref: https://github.com/cosmos/cosmos-sdk/issues/9683 - ctx.GasMeter().ConsumeGas( - 3*ctx.KVGasConfig().WriteCostPerByte*uint64(len(bytes)), + k.environment.GasService.GetGasMeter(ctx).Consume( + 3*k.environment.GasService.GetGasConfig(ctx).WriteCostPerByte*uint64(len(bytes)), "submit proposal", ) @@ -106,11 +108,12 @@ func (k msgServer) SubmitProposal(goCtx context.Context, msg *v1.MsgSubmitPropos } if votingStarted { - ctx.EventManager().EmitEvent( - sdk.NewEvent(govtypes.EventTypeSubmitProposal, - sdk.NewAttribute(govtypes.AttributeKeyVotingPeriodStart, fmt.Sprintf("%d", proposal.Id)), - ), - ) + if err := k.environment.EventService.EventManager(ctx).EmitKV( + govtypes.EventTypeSubmitProposal, + event.NewAttribute(govtypes.AttributeKeyVotingPeriodStart, fmt.Sprintf("%d", proposal.Id)), + ); err != nil { + return nil, errors.Wrapf(err, "failed to emit event: %s", govtypes.EventTypeSubmitProposal) + } } return &v1.MsgSubmitProposalResponse{ @@ -158,36 +161,33 @@ func (k msgServer) SubmitMultipleChoiceProposal(ctx context.Context, msg *v1.Msg } // CancelProposal implements the MsgServer.CancelProposal method. -func (k msgServer) CancelProposal(goCtx context.Context, msg *v1.MsgCancelProposal) (*v1.MsgCancelProposalResponse, error) { +func (k msgServer) CancelProposal(ctx context.Context, msg *v1.MsgCancelProposal) (*v1.MsgCancelProposalResponse, error) { _, err := k.authKeeper.AddressCodec().StringToBytes(msg.Proposer) if err != nil { return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid proposer address: %s", err) } - ctx := sdk.UnwrapSDKContext(goCtx) if err := k.Keeper.CancelProposal(ctx, msg.ProposalId, msg.Proposer); err != nil { return nil, err } - ctx.EventManager().EmitEvent( - sdk.NewEvent( - govtypes.EventTypeCancelProposal, - sdk.NewAttribute(sdk.AttributeKeySender, msg.Proposer), - sdk.NewAttribute(govtypes.AttributeKeyProposalID, fmt.Sprint(msg.ProposalId)), - ), - ) + if err := k.environment.EventService.EventManager(ctx).EmitKV( + govtypes.EventTypeCancelProposal, + event.NewAttribute(sdk.AttributeKeySender, msg.Proposer), + event.NewAttribute(govtypes.AttributeKeyProposalID, fmt.Sprint(msg.ProposalId)), + ); err != nil { + return nil, errors.Wrapf(err, "failed to emit event: %s", govtypes.EventTypeCancelProposal) + } return &v1.MsgCancelProposalResponse{ ProposalId: msg.ProposalId, - CanceledTime: ctx.HeaderInfo().Time, - CanceledHeight: uint64(ctx.BlockHeight()), + CanceledTime: k.environment.HeaderService.GetHeaderInfo(ctx).Time, + CanceledHeight: uint64(k.environment.HeaderService.GetHeaderInfo(ctx).Height), }, nil } // ExecLegacyContent implements the MsgServer.ExecLegacyContent method. -func (k msgServer) ExecLegacyContent(goCtx context.Context, msg *v1.MsgExecLegacyContent) (*v1.MsgExecLegacyContentResponse, error) { - ctx := sdk.UnwrapSDKContext(goCtx) - +func (k msgServer) ExecLegacyContent(ctx context.Context, msg *v1.MsgExecLegacyContent) (*v1.MsgExecLegacyContentResponse, error) { govAcct, err := k.authKeeper.AddressCodec().BytesToString(k.GetGovernanceAccount(ctx).GetAddress()) if err != nil { return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid governance account address: %s", err) @@ -277,7 +277,7 @@ func (k msgServer) VoteWeighted(ctx context.Context, msg *v1.MsgVoteWeighted) (* } // Deposit implements the MsgServer.Deposit method. -func (k msgServer) Deposit(goCtx context.Context, msg *v1.MsgDeposit) (*v1.MsgDepositResponse, error) { +func (k msgServer) Deposit(ctx context.Context, msg *v1.MsgDeposit) (*v1.MsgDepositResponse, error) { accAddr, err := k.authKeeper.AddressCodec().StringToBytes(msg.Depositor) if err != nil { return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid depositor address: %s", err) @@ -287,19 +287,18 @@ func (k msgServer) Deposit(goCtx context.Context, msg *v1.MsgDeposit) (*v1.MsgDe return nil, err } - ctx := sdk.UnwrapSDKContext(goCtx) votingStarted, err := k.Keeper.AddDeposit(ctx, msg.ProposalId, accAddr, msg.Amount) if err != nil { return nil, err } if votingStarted { - ctx.EventManager().EmitEvent( - sdk.NewEvent( - govtypes.EventTypeProposalDeposit, - sdk.NewAttribute(govtypes.AttributeKeyVotingPeriodStart, fmt.Sprintf("%d", msg.ProposalId)), - ), - ) + if err := k.environment.EventService.EventManager(ctx).EmitKV( + govtypes.EventTypeProposalDeposit, + event.NewAttribute(govtypes.AttributeKeyVotingPeriodStart, fmt.Sprintf("%d", msg.ProposalId)), + ); err != nil { + return nil, errors.Wrapf(err, "failed to emit event: %s", govtypes.EventTypeProposalDeposit) + } } return &v1.MsgDepositResponse{}, nil @@ -374,27 +373,32 @@ func (k msgServer) SudoExec(ctx context.Context, msg *v1.MsgSudoExec) (*v1.MsgSu } } - handler := k.router.Handler(sudoedMsg) - if handler == nil { - return nil, errors.Wrapf(govtypes.ErrInvalidProposal, "unrecognized message route: %s", sdk.MsgTypeURL(sudoedMsg)) - } + var msgResp protoiface.MessageV1 + if err := k.environment.BranchService.Execute(ctx, func(ctx context.Context) error { + // TODO add route check here + if err := k.environment.RouterService.MessageRouterService().CanInvoke(ctx, sdk.MsgTypeURL(sudoedMsg)); err != nil { + return errors.Wrapf(govtypes.ErrInvalidProposal, err.Error()) + } - sdkCtx := sdk.UnwrapSDKContext(ctx) - msgResp, err := handler(sdkCtx, sudoedMsg) - if err != nil { - return nil, errors.Wrapf(err, "failed to execute sudo-ed message; message %v", sudoedMsg) + msgResp, err = k.environment.RouterService.MessageRouterService().InvokeUntyped(ctx, sudoedMsg) + if err != nil { + return errors.Wrapf(err, "failed to execute sudo-ed message; message %v", sudoedMsg) + } + + return nil + }); err != nil { + return nil, err } - // emit the events from the executed message - events := msgResp.Events - sdkEvents := make([]sdk.Event, 0, len(events)) - for _, event := range events { - sdkEvents = append(sdkEvents, sdk.Event(event)) + // TODO(@julienrbrt): check if events are properly emitted + + msgRespBytes, err := k.cdc.MarshalJSON(msgResp) + if err != nil { + return nil, errors.Wrapf(err, "failed to marshal sudo-ed message response; message %v", msgResp) } - sdkCtx.EventManager().EmitEvents(sdkEvents) return &v1.MsgSudoExecResponse{ - Result: msgResp.Data, + Result: msgRespBytes, }, nil } diff --git a/x/gov/keeper/msg_server_test.go b/x/gov/keeper/msg_server_test.go index 5163353a069d..3f0632d06fc0 100644 --- a/x/gov/keeper/msg_server_test.go +++ b/x/gov/keeper/msg_server_test.go @@ -2131,7 +2131,7 @@ func (suite *KeeperTestSuite) TestMsgSudoExec() { { name: "invalid msg (not registered)", input: invalidMsg, - expErrMsg: "unrecognized message route", + expErrMsg: "unknown message", }, { name: "valid", diff --git a/x/gov/keeper/proposal.go b/x/gov/keeper/proposal.go index 8e7801f8f520..4529a80d53b4 100644 --- a/x/gov/keeper/proposal.go +++ b/x/gov/keeper/proposal.go @@ -10,6 +10,7 @@ import ( "time" "cosmossdk.io/collections" + "cosmossdk.io/core/event" errorsmod "cosmossdk.io/errors" sdkmath "cosmossdk.io/math" "cosmossdk.io/x/gov/types" @@ -38,7 +39,6 @@ func (k Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, metadata } } - sdkCtx := sdk.UnwrapSDKContext(ctx) msgs := []string{} // will hold a string slice of all Msg type URLs. // Loop through all messages and confirm that each has a handler and the gov module account as the only signer @@ -83,10 +83,8 @@ func (k Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, metadata return v1.Proposal{}, errorsmod.Wrapf(types.ErrInvalidSigner, sdk.AccAddress(signers[0]).String()) } - // use the msg service router to see that there is a valid route for that message. - handler := k.router.Handler(msg) - if handler == nil { - return v1.Proposal{}, errorsmod.Wrap(types.ErrUnroutableProposalMsg, sdk.MsgTypeURL(msg)) + if err := k.environment.RouterService.MessageRouterService().CanInvoke(ctx, sdk.MsgTypeURL(msg)); err != nil { + return v1.Proposal{}, errorsmod.Wrap(types.ErrUnroutableProposalMsg, err.Error()) } // Only if it's a MsgExecLegacyContent we try to execute the @@ -98,14 +96,27 @@ func (k Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, metadata if !ok { continue } - cacheCtx, _ := sdkCtx.CacheContext() - if _, err := handler(cacheCtx, msg); err != nil { - if errors.Is(types.ErrNoProposalHandlerExists, err) { - return v1.Proposal{}, err - } - return v1.Proposal{}, errorsmod.Wrap(types.ErrInvalidProposalContent, err.Error()) + + content, err := v1.LegacyContentFromMessage(msg) + if err != nil { + return v1.Proposal{}, errorsmod.Wrapf(types.ErrInvalidProposalContent, "%+v", err) } + // Ensure that the content has a respective handler + if !k.legacyRouter.HasRoute(content.ProposalRoute()) { + return v1.Proposal{}, errorsmod.Wrap(types.ErrNoProposalHandlerExists, content.ProposalRoute()) + } + + if err = k.environment.BranchService.Execute(ctx, func(ctx context.Context) error { + handler := k.legacyRouter.GetRoute(content.ProposalRoute()) + if err := handler(ctx, content); err != nil { + return types.ErrInvalidProposalContent.Wrapf("failed to run legacy handler %s, %+v", content.ProposalRoute(), err) + } + + return errors.New("we don't want to execude the proposal, we just want to check if it can be executed") + }); errors.Is(err, types.ErrInvalidProposalContent) { + return v1.Proposal{}, err + } } proposalID, err := k.ProposalID.Next(ctx) @@ -113,7 +124,7 @@ func (k Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, metadata return v1.Proposal{}, err } - submitTime := sdkCtx.HeaderInfo().Time + submitTime := k.environment.HeaderService.GetHeaderInfo(ctx).Time proposal, err := v1.NewProposal(messages, proposalID, submitTime, submitTime.Add(*params.MaxDepositPeriod), metadata, title, summary, proposer, proposalType) if err != nil { return v1.Proposal{}, err @@ -133,20 +144,19 @@ func (k Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, metadata return v1.Proposal{}, err } - sdkCtx.EventManager().EmitEvent( - sdk.NewEvent( - types.EventTypeSubmitProposal, - sdk.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposalID)), - sdk.NewAttribute(types.AttributeKeyProposalMessages, strings.Join(msgs, ",")), - ), - ) + if err := k.environment.EventService.EventManager(ctx).EmitKV( + types.EventTypeSubmitProposal, + event.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposalID)), + event.NewAttribute(types.AttributeKeyProposalMessages, strings.Join(msgs, ",")), + ); err != nil { + return v1.Proposal{}, fmt.Errorf("failed to emit event: %w", err) + } return proposal, nil } // CancelProposal will cancel proposal before the voting period ends func (k Keeper) CancelProposal(ctx context.Context, proposalID uint64, proposer string) error { - sdkCtx := sdk.UnwrapSDKContext(ctx) proposal, err := k.Proposals.Get(ctx, proposalID) if err != nil { if errors.Is(err, collections.ErrNotFound) { @@ -178,7 +188,7 @@ func (k Keeper) CancelProposal(ctx context.Context, proposalID uint64, proposer // Check proposal is not too far in voting period to be canceled if proposal.VotingEndTime != nil { - currentTime := sdkCtx.HeaderInfo().Time + currentTime := k.environment.HeaderService.GetHeaderInfo(ctx).Time maxCancelPeriodRate := sdkmath.LegacyMustNewDecFromStr(params.ProposalCancelMaxPeriod) maxCancelPeriod := time.Duration(float64(proposal.VotingEndTime.Sub(*proposal.VotingStartTime)) * maxCancelPeriodRate.MustFloat64()).Round(time.Second) @@ -209,7 +219,7 @@ func (k Keeper) CancelProposal(ctx context.Context, proposalID uint64, proposer return err } - k.Logger(ctx).Info( + k.Logger().Info( "proposal is canceled by proposer", "proposal", proposal.Id, "proposer", proposal.Proposer, @@ -243,8 +253,7 @@ func (k Keeper) DeleteProposal(ctx context.Context, proposalID uint64) error { // ActivateVotingPeriod activates the voting period of a proposal func (k Keeper) ActivateVotingPeriod(ctx context.Context, proposal v1.Proposal) error { - sdkCtx := sdk.UnwrapSDKContext(ctx) - startTime := sdkCtx.HeaderInfo().Time + startTime := k.environment.HeaderService.GetHeaderInfo(ctx).Time proposal.VotingStartTime = &startTime params, err := k.Params.Get(ctx) diff --git a/x/gov/keeper/vote.go b/x/gov/keeper/vote.go index a8dcf8340ab2..41805fe0b891 100644 --- a/x/gov/keeper/vote.go +++ b/x/gov/keeper/vote.go @@ -6,6 +6,7 @@ import ( "fmt" "cosmossdk.io/collections" + "cosmossdk.io/core/event" "cosmossdk.io/errors" "cosmossdk.io/x/gov/types" v1 "cosmossdk.io/x/gov/types/v1" @@ -74,22 +75,15 @@ func (k Keeper) AddVote(ctx context.Context, proposalID uint64, voterAddr sdk.Ac } // called after a vote on a proposal is cast - err = k.Hooks().AfterProposalVote(ctx, proposalID, voterAddr) - if err != nil { + if err = k.Hooks().AfterProposalVote(ctx, proposalID, voterAddr); err != nil { return err } - sdkCtx := sdk.UnwrapSDKContext(ctx) - sdkCtx.EventManager().EmitEvent( - sdk.NewEvent( - types.EventTypeProposalVote, - sdk.NewAttribute(types.AttributeKeyVoter, voterAddr.String()), - sdk.NewAttribute(types.AttributeKeyOption, options.String()), - sdk.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposalID)), - ), + return k.environment.EventService.EventManager(ctx).EmitKV(types.EventTypeProposalVote, + event.NewAttribute(types.AttributeKeyVoter, voterAddr.String()), + event.NewAttribute(types.AttributeKeyOption, options.String()), + event.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposalID)), ) - - return nil } // deleteVotes deletes all the votes from a given proposalID. diff --git a/x/gov/module.go b/x/gov/module.go index 85c95353513a..889ce865af99 100644 --- a/x/gov/module.go +++ b/x/gov/module.go @@ -194,8 +194,7 @@ func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion } // EndBlock returns the end blocker for the gov module. func (am AppModule) EndBlock(ctx context.Context) error { - c := sdk.UnwrapSDKContext(ctx) - return EndBlocker(c, am.keeper) + return am.keeper.EndBlocker(ctx) } // AppModuleSimulation functions diff --git a/x/gov/types/v1beta1/content.go b/x/gov/types/v1beta1/content.go index 64d4ef4f19ae..15120c829cd5 100644 --- a/x/gov/types/v1beta1/content.go +++ b/x/gov/types/v1beta1/content.go @@ -1,6 +1,6 @@ package v1beta1 -import sdk "github.com/cosmos/cosmos-sdk/types" +import context "context" // Content defines an interface that a proposal must implement. It contains // information such as the title and description along with the type and routing @@ -17,7 +17,7 @@ type Content interface { // Handler defines a function that handles a proposal after it has passed the // governance process. -type Handler func(ctx sdk.Context, content Content) error +type Handler func(ctx context.Context, content Content) error type HandlerRoute struct { Handler Handler diff --git a/x/gov/types/v1beta1/proposal.go b/x/gov/types/v1beta1/proposal.go index ec7d4ed8c460..fdc7821ee957 100644 --- a/x/gov/types/v1beta1/proposal.go +++ b/x/gov/types/v1beta1/proposal.go @@ -1,6 +1,7 @@ package v1beta1 import ( + context "context" "fmt" "strings" "time" @@ -249,7 +250,7 @@ func IsValidProposalType(ty string) bool { // proposals (ie. TextProposal ). Since these are // merely signaling mechanisms at the moment and do not affect state, it // performs a no-op. -func ProposalHandler(_ sdk.Context, c Content) error { +func ProposalHandler(_ context.Context, c Content) error { switch c.ProposalType() { case ProposalTypeText: // both proposal types do not change state so this performs a no-op diff --git a/x/params/proposal_handler.go b/x/params/proposal_handler.go index 7483603a7ea8..25ec53162ebd 100644 --- a/x/params/proposal_handler.go +++ b/x/params/proposal_handler.go @@ -1,6 +1,7 @@ package params import ( + "context" "fmt" errorsmod "cosmossdk.io/errors" @@ -14,10 +15,14 @@ import ( // NewParamChangeProposalHandler creates a new governance Handler for a ParamChangeProposal func NewParamChangeProposalHandler(k keeper.Keeper) govtypes.Handler { - return func(ctx sdk.Context, content govtypes.Content) error { + return func(ctx context.Context, content govtypes.Content) error { + // UnwrapSDKContext makes x/params baseapp compatible only and not server/v2 + // We should investigate if we want to make x/params server/v2 compatible + sdkCtx := sdk.UnwrapSDKContext(ctx) + switch c := content.(type) { case *proposal.ParameterChangeProposal: - return handleParameterChangeProposal(ctx, k, c) + return handleParameterChangeProposal(sdkCtx, k, c) default: return errorsmod.Wrapf(sdkerrors.ErrUnknownRequest, "unrecognized param proposal content type: %T", c)