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

feat: add endblocker with valsetupdate type #15829

Merged
merged 10 commits into from
Apr 14, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
3 changes: 3 additions & 0 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,9 @@ func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBloc
}
}

// set the validator set for the next block
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

QQ: When is res, err = app.endBlocker(app.deliverState.ctx, req) going to become err = app.endBlocker(app.deliverState.ctx, req)? i.e. when are we removing the ResponseEndBlock types from core's EndBlock API?

res.ValidatorUpdates = app.updatedValidators
Copy link
Member

Choose a reason for hiding this comment

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

The challenge is that this overwrites updates in the case where the app is not using the update service. So we need to have some bool flag on the update service to indicate that updates have been set I think


return res
Copy link
Contributor

Choose a reason for hiding this comment

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

Change potentially affects state.

Call sequence:

(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).EndBlock (baseapp/abci.go:229)

}

Expand Down
7 changes: 7 additions & 0 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ type BaseApp struct {
idPeerFilter sdk.PeerFilter // filter peers by node ID
fauxMerkleMode bool // if true, IAVL MountStores uses MountStoresDB for simulation speed.

ValidatorSetUpdate // absorb validator setupdate type

// manages snapshots, i.e. dumps of app state at certain intervals
snapshotManager *snapshots.Manager

Expand Down Expand Up @@ -227,6 +229,11 @@ func (app *BaseApp) SetMsgServiceRouter(msgServiceRouter *MsgServiceRouter) {
app.msgServiceRouter = msgServiceRouter
}

// ValidatorService returns the ValidatorUpdateService of a BaseApp.
func (app *BaseApp) ValidatorService() ValidatorUpdateService {
return app
}

// MountStores mounts all IAVL or DB stores to the provided keys in the BaseApp
// multistore.
func (app *BaseApp) MountStores(keys ...storetypes.StoreKey) {
Expand Down
22 changes: 22 additions & 0 deletions baseapp/valset_update.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package baseapp

import (
"context"

abci "github.com/cometbft/cometbft/abci/types"
)

// ValidatorUpdateService is the service that runtime will provide to the module that sets validator updates.
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought BaseApp would just have a reference to the service, which is defined in runtime? Why is this being defined in BaseApp?

type ValidatorUpdateService interface {
SetValidatorUpdates(context.Context, []abci.ValidatorUpdate)
}

var _ = (ValidatorUpdateService)(&ValidatorSetUpdate{})

type ValidatorSetUpdate struct {
updatedValidators []abci.ValidatorUpdate
}

func (v *ValidatorSetUpdate) SetValidatorUpdates(ctx context.Context, updates []abci.ValidatorUpdate) {
v.updatedValidators = updates
}
15 changes: 11 additions & 4 deletions runtime/app.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
package runtime

import (
"context"
"encoding/json"
"fmt"

abci "github.com/cometbft/cometbft/abci/types"
"golang.org/x/exp/slices"

runtimev1alpha1 "cosmossdk.io/api/cosmos/app/runtime/v1alpha1"
appv1alpha1 "cosmossdk.io/api/cosmos/app/v1alpha1"

storetypes "cosmossdk.io/store/types"
abci "github.com/cometbft/cometbft/abci/types"
"golang.org/x/exp/slices"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/client"
Expand Down Expand Up @@ -52,6 +51,8 @@ type App struct {
// initChainer is the init chainer function defined by the app config.
// this is only required if the chain wants to add special InitChainer logic.
initChainer sdk.InitChainer

ValSetUpdate []abci.ValidatorUpdate
Copy link
Member

Choose a reason for hiding this comment

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

why is this needed?

}

// RegisterModules registers the provided modules with the module manager and
Expand Down Expand Up @@ -125,6 +126,12 @@ func (a *App) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) (abci.Respon
return a.ModuleManager.EndBlock(ctx, req)
}

// SetValidatorUpdates sets the validator updates for the next block.
// CONTRACT: this must be called for modules that are updating the validator set
func (a *App) SetValidatorUpdates(ctx context.Context, updates []abci.ValidatorUpdate) {
a.BaseApp.SetValidatorUpdates(ctx, updates)
}

// InitChainer initializes the chain.
func (a *App) InitChainer(ctx sdk.Context, req abci.RequestInitChain) (abci.ResponseInitChain, error) {
var genesisState map[string]json.RawMessage
Expand Down
4 changes: 4 additions & 0 deletions runtime/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,3 +209,7 @@ func ProvideTransientStoreService(key depinject.ModuleKey, app *AppBuilder) stor
func ProvideEventService() event.Service {
return EventService{}
}

func ProvideSetValidatorService(ab *AppBuilder) baseapp.ValidatorUpdateService {
return ab.app
}
9 changes: 1 addition & 8 deletions runtime/services.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package runtime

import (
"context"

appv1alpha1 "cosmossdk.io/api/cosmos/app/v1alpha1"
autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"
reflectionv1 "cosmossdk.io/api/cosmos/reflection/v1"
Expand All @@ -26,14 +24,9 @@ func (a *App) registerRuntimeServices(cfg module.Configurator) error {
}

// ======================================================
// ValidatorUpdateService & BlockInfoService
// BlockInfoService
// ======================================================

// ValidatorUpdateService is the service that runtime will provide to the module that sets validator updates.
type ValidatorUpdateService interface {
SetValidatorUpdates(context.Context, []abci.ValidatorUpdate)
}

// BlockInfoService is the service that runtime will provide to modules which need Comet block information.
type BlockInfoService interface {
GetHeight() int64 // GetHeight returns the height of the block
Expand Down
2 changes: 1 addition & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ func NewSimApp(
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)
app.StakingKeeper = stakingkeeper.NewKeeper(
appCodec, keys[stakingtypes.StoreKey], app.AccountKeeper, app.BankKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(),
appCodec, keys[stakingtypes.StoreKey], app.AccountKeeper, app.BankKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(), app.BaseApp,
)
app.MintKeeper = mintkeeper.NewKeeper(appCodec, keys[minttypes.StoreKey], app.StakingKeeper, app.AccountKeeper, app.BankKeeper, authtypes.FeeCollectorName, authtypes.NewModuleAddress(govtypes.ModuleName).String())

Expand Down
2 changes: 1 addition & 1 deletion x/feegrant/testutil/expected_keepers_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 0 additions & 8 deletions x/staking/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,6 @@ import (
"github.com/cosmos/cosmos-sdk/x/staking/types"
)

// BeginBlocker will persist the current header and validator set as a historical entry
// and prune the oldest entry based on the HistoricalEntries parameter
func BeginBlocker(ctx sdk.Context, k *keeper.Keeper) {
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker)

k.TrackHistoricalInfo(ctx)
}

// Called every block, update validator set
func EndBlocker(ctx sdk.Context, k *keeper.Keeper) []abci.ValidatorUpdate {
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker)
Expand Down
25 changes: 25 additions & 0 deletions x/staking/keeper/abci.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package keeper

import (
"context"
"time"

"github.com/cosmos/cosmos-sdk/telemetry"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/staking/types"
)

// BeginBlocker will persist the current header and validator set as a historical entry
// and prune the oldest entry based on the HistoricalEntries parameter
func (k *Keeper) BeginBlocker(ctx sdk.Context) {
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker)

k.TrackHistoricalInfo(ctx)
Fixed Show fixed Hide fixed
}

// Called every block, update validator set
func (k *Keeper) EndBlocker(ctx context.Context) {
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker)

k.validatorService.SetValidatorUpdates(ctx, k.BlockValidatorUpdates(sdk.UnwrapSDKContext(ctx)))
Fixed Show fixed Hide fixed
}
21 changes: 15 additions & 6 deletions x/staking/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
abci "github.com/cometbft/cometbft/abci/types"

storetypes "cosmossdk.io/store/types"
"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/staking/types"
Expand All @@ -27,6 +28,8 @@ type Keeper struct {
bankKeeper types.BankKeeper
hooks types.StakingHooks
authority string

validatorService baseapp.ValidatorUpdateService
}

// NewKeeper creates a new staking Keeper instance
Expand All @@ -36,6 +39,7 @@ func NewKeeper(
ak types.AccountKeeper,
bk types.BankKeeper,
authority string,
vs baseapp.ValidatorUpdateService,
) *Keeper {
// ensure bonded and not bonded module accounts are set
if addr := ak.GetModuleAddress(types.BondedPoolName); addr == nil {
Expand All @@ -46,18 +50,23 @@ func NewKeeper(
panic(fmt.Sprintf("%s module account has not been set", types.NotBondedPoolName))
}

if vs == nil {
panic("validator service not set")
}

// ensure that authority is a valid AccAddress
if _, err := ak.StringToBytes(authority); err != nil {
panic("authority is not a valid acc address")
}

return &Keeper{
storeKey: key,
cdc: cdc,
authKeeper: ak,
bankKeeper: bk,
hooks: nil,
authority: authority,
storeKey: key,
cdc: cdc,
authKeeper: ak,
bankKeeper: bk,
hooks: nil,
authority: authority,
validatorService: vs,
}
}

Expand Down
1 change: 1 addition & 0 deletions x/staking/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ func (s *KeeperTestSuite) SetupTest() {
accountKeeper,
bankKeeper,
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
&baseapp.ValidatorSetUpdate{},
)
keeper.SetParams(ctx, stakingtypes.DefaultParams())

Expand Down
33 changes: 19 additions & 14 deletions x/staking/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,26 @@ import (
"fmt"
"sort"

authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
"github.com/cosmos/cosmos-sdk/x/staking/exported"

abci "github.com/cometbft/cometbft/abci/types"
gwruntime "github.com/grpc-ecosystem/grpc-gateway/runtime"
"github.com/spf13/cobra"
"golang.org/x/exp/maps"

modulev1 "cosmossdk.io/api/cosmos/staking/module/v1"
"cosmossdk.io/core/appmodule"
"cosmossdk.io/depinject"

store "cosmossdk.io/store/types"
abci "github.com/cometbft/cometbft/abci/types"
gwruntime "github.com/grpc-ecosystem/grpc-gateway/runtime"
"github.com/spf13/cobra"
"golang.org/x/exp/maps"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/codec"
cdctypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
"github.com/cosmos/cosmos-sdk/x/staking/client/cli"
"github.com/cosmos/cosmos-sdk/x/staking/exported"
"github.com/cosmos/cosmos-sdk/x/staking/keeper"
"github.com/cosmos/cosmos-sdk/x/staking/simulation"
"github.com/cosmos/cosmos-sdk/x/staking/types"
Expand All @@ -38,7 +36,9 @@ const (
)

var (
_ module.EndBlockAppModule = AppModule{}
_ appmodule.AppModule = AppModule{}
_ appmodule.HasBeginBlocker = AppModule{}
_ appmodule.HasEndBlocker = AppModule{}
_ module.AppModuleBasic = AppModuleBasic{}
_ module.AppModuleSimulation = AppModule{}
)
Expand Down Expand Up @@ -191,14 +191,16 @@ func (AppModule) ConsensusVersion() uint64 { return consensusVersion }
// BeginBlock returns the begin blocker for the staking module.
func (am AppModule) BeginBlock(ctx context.Context) error {
c := sdk.UnwrapSDKContext(ctx)
BeginBlocker(c, am.keeper)

am.keeper.BeginBlocker(c)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods

path flow from Begin/EndBlock to a panic call path flow from Begin/EndBlock to a panic call path flow from Begin/EndBlock to a panic call path flow from Begin/EndBlock to a panic call path flow from Begin/EndBlock to a panic call
return nil
}

// EndBlock returns the end blocker for the staking module. It returns no validator
// updates.
func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate {
return EndBlocker(ctx, am.keeper)
func (am AppModule) EndBlock(ctx context.Context) error {
am.keeper.EndBlocker(ctx)
Fixed Show fixed Hide fixed
return nil // handle error in the future
}

func init() {
Expand All @@ -218,6 +220,8 @@ type ModuleInputs struct {
Cdc codec.Codec
Key *store.KVStoreKey

Vs baseapp.ValidatorUpdateService

// LegacySubspace is used solely for migration of x/params managed parameters
LegacySubspace exported.Subspace
}
Expand All @@ -243,6 +247,7 @@ func ProvideModule(in ModuleInputs) ModuleOutputs {
in.AccountKeeper,
in.BankKeeper,
authority.String(),
in.Vs,
)
m := NewAppModule(in.Cdc, k, in.AccountKeeper, in.BankKeeper, in.LegacySubspace)
return ModuleOutputs{StakingKeeper: k, Module: m}
Expand Down