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.

Sign up for GitHub

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

Jump to bottom

feat!: return errors in module manager ABCI methods #14847

Merged
merged 11 commits into from
Jan 31, 2023
Merged

feat!: return errors in module manager ABCI methods #14847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7cf6bcf
refactor: return errors in module manager ABCI methods
facundomedica Jan 30, 2023
ef68e9e
fix
facundomedica Jan 31, 2023
b848ecc
fix
facundomedica Jan 31, 2023
7bcaa74
lint
facundomedica Jan 31, 2023
a1231e1
Merge branch 'main' into facu/refactor-modulemanager-methods
facundomedica Jan 31, 2023
946f5a7
Merge branch 'main' into facu/refactor-modulemanager-methods
facundomedica Jan 31, 2023
173a3f7
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Jan 31, 2023
ec0edac
add cl
facundomedica Jan 31, 2023
8344e92
add cl
facundomedica Jan 31, 2023
fe4ea57
update doc
facundomedica Jan 31, 2023
83b47f1
lint
facundomedica Jan 31, 2023
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: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,9 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* [#14847](https://github.com/cosmos/cosmos-sdk/pull/14847) App and ModuleManager methods `InitGenesis`, `ExportGenesis`, `BeginBlock` and `EndBlock` now also return an error.
* (simulation) [#14728](https://github.com/cosmos/cosmos-sdk/pull/14728) Rename the `ParamChanges` field to `LegacyParamChange` and `Contents` to `LegacyProposalContents` in `simulation.SimulationState`. Additionally it adds a `ProposalMsgs` field to `simulation.SimulationState`.
* (x/upgrade) [14764](https://github.com/cosmos/cosmos-sdk/pull/14764) The `x/upgrade` module is extracted to have a separate go.mod file which allows it to be a standalone module.
* (x/upgrade) [#14764](https://github.com/cosmos/cosmos-sdk/pull/14764) The `x/upgrade` module is extracted to have a separate go.mod file which allows it to be a standalone module.
* (x/gov) [#14782](https://github.com/cosmos/cosmos-sdk/pull/14782) Move the `metadata` argument in `govv1.NewProposal` alongside `title` and `summary`.
* (store) [#14746](https://github.com/cosmos/cosmos-sdk/pull/14746) Extract Store in its own go.mod and rename the package to `cosmossdk.io/store`.
* (simulation) [#14751](https://github.com/cosmos/cosmos-sdk/pull/14751) Remove the `MsgType` field from `simulation.OperationInput` struct.
Expand Down
17 changes: 14 additions & 3 deletions baseapp/abci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,10 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC
// add block gas meter for any genesis transactions (allow infinite gas)
app.deliverState.ctx = app.deliverState.ctx.WithBlockGasMeter(storetypes.NewInfiniteGasMeter())

res = app.initChainer(app.deliverState.ctx, req)
res, err := app.initChainer(app.deliverState.ctx, req)
if err != nil {
panic(err)
}

// sanity check
if len(req.Validators) > 0 {
Expand Down Expand Up @@ -195,7 +198,11 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg
}

if app.beginBlocker != nil {
res = app.beginBlocker(app.deliverState.ctx, req)
var err error
res, err = app.beginBlocker(app.deliverState.ctx, req)
if err != nil {
panic(err)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
}
res.Events = sdk.MarkEventsToIndex(res.Events, app.indexEvents)
}
// set the signed validators for addition to context in deliverTx
Expand All @@ -218,7 +225,11 @@ func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBloc
}

if app.endBlocker != nil {
res = app.endBlocker(app.deliverState.ctx, req)
var err error
res, err = app.endBlocker(app.deliverState.ctx, req)
if err != nil {
panic(err)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
}
res.Events = sdk.MarkEventsToIndex(res.Events, app.indexEvents)
}

Expand Down
12 changes: 6 additions & 6 deletions baseapp/abci_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ func TestABCI_InitChain(t *testing.T) {

// set a value in the store on init chain
key, value := []byte("hello"), []byte("goodbye")
var initChainer sdk.InitChainer = func(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain {
var initChainer sdk.InitChainer = func(ctx sdk.Context, req abci.RequestInitChain) (abci.ResponseInitChain, error) {
store := ctx.KVStore(capKey)
store.Set(key, value)
return abci.ResponseInitChain{}
return abci.ResponseInitChain{}, nil
}

query := abci.RequestQuery{
Expand Down Expand Up @@ -579,12 +579,12 @@ func TestABCI_EndBlock(t *testing.T) {
ConsensusParams: cp,
})

app.SetEndBlocker(func(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock {
app.SetEndBlocker(func(ctx sdk.Context, req abci.RequestEndBlock) (abci.ResponseEndBlock, error) {
return abci.ResponseEndBlock{
ValidatorUpdates: []abci.ValidatorUpdate{
{Power: 100},
},
}
}, nil
})
app.Seal()

Expand Down Expand Up @@ -1384,9 +1384,9 @@ func TestABCI_Proposal_Read_State_PrepareProposal(t *testing.T) {
someKey := []byte("some-key")

setInitChainerOpt := func(bapp *baseapp.BaseApp) {
bapp.SetInitChainer(func(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain {
bapp.SetInitChainer(func(ctx sdk.Context, req abci.RequestInitChain) (abci.ResponseInitChain, error) {
ctx.KVStore(capKey1).Set(someKey, []byte("foo"))
return abci.ResponseInitChain{}
return abci.ResponseInitChain{}, nil
})
}

Expand Down
6 changes: 3 additions & 3 deletions runtime/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,17 +116,17 @@ func (a *App) Load(loadLatest bool) error {
}

// BeginBlocker application updates every begin block
func (a *App) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock {
func (a *App) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) (abci.ResponseBeginBlock, error) {
return a.ModuleManager.BeginBlock(ctx, req)
}

// EndBlocker application updates every end block
func (a *App) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock {
func (a *App) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) (abci.ResponseEndBlock, error) {
return a.ModuleManager.EndBlock(ctx, req)
}

// InitChainer initializes the chain.
func (a *App) InitChainer(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain {
func (a *App) InitChainer(ctx sdk.Context, req abci.RequestInitChain) (abci.ResponseInitChain, error) {
var genesisState map[string]json.RawMessage
if err := json.Unmarshal(req.AppStateBytes, &genesisState); err != nil {
panic(err)
Expand Down
6 changes: 3 additions & 3 deletions runtime/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ type AppI interface {
LegacyAmino() *codec.LegacyAmino

// Application updates every begin block.
BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock
BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) (abci.ResponseBeginBlock, error)

// Application updates every end block.
EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock
EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) (abci.ResponseEndBlock, error)

// Application update at chain (i.e app) initialization.
InitChainer(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain
InitChainer(ctx sdk.Context, req abci.RequestInitChain) (abci.ResponseInitChain, error)

// Loads the app at a given height.
LoadHeight(height int64) error
Expand Down
9 changes: 4 additions & 5 deletions server/mock/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,22 +96,21 @@ type GenesisJSON struct {

// InitChainer returns a function that can initialize the chain
// with key/value pairs
func InitChainer(key storetypes.StoreKey) func(sdk.Context, abci.RequestInitChain) abci.ResponseInitChain {
return func(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain {
func InitChainer(key storetypes.StoreKey) func(sdk.Context, abci.RequestInitChain) (abci.ResponseInitChain, error) {
return func(ctx sdk.Context, req abci.RequestInitChain) (abci.ResponseInitChain, error) {
stateJSON := req.AppStateBytes

genesisState := new(GenesisJSON)
err := json.Unmarshal(stateJSON, genesisState)
if err != nil {
panic(err) // TODO https://github.com/cosmos/cosmos-sdk/issues/468
// return sdk.ErrGenesisParse("").TraceCause(err, "")
return abci.ResponseInitChain{}, err
}

for _, val := range genesisState.Values {
store := ctx.KVStore(key)
store.Set([]byte(val.Key), []byte(val.Value))
}
return abci.ResponseInitChain{}
return abci.ResponseInitChain{}, nil
}
}

Expand Down
9 changes: 5 additions & 4 deletions simapp/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,12 @@ import (
upgradeclient "cosmossdk.io/x/upgrade/client"
upgradekeeper "cosmossdk.io/x/upgrade/keeper"
upgradetypes "cosmossdk.io/x/upgrade/types"

storetypes "cosmossdk.io/store/types"
"cosmossdk.io/x/feegrant"
feegrantkeeper "cosmossdk.io/x/feegrant/keeper"
feegrantmodule "cosmossdk.io/x/feegrant/module"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
Expand Down Expand Up @@ -559,12 +560,12 @@ func (app *SimApp) setPostHandler() {
func (app *SimApp) Name() string { return app.BaseApp.Name() }

// BeginBlocker application updates every begin block
func (app *SimApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock {
func (app *SimApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) (abci.ResponseBeginBlock, error) {
Copy link
Member

Choose a reason for hiding this comment

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

need a changelog and upgrade entry for this

return app.ModuleManager.BeginBlock(ctx, req)
}

// EndBlocker application updates every end block
func (app *SimApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock {
func (app *SimApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) (abci.ResponseEndBlock, error) {
return app.ModuleManager.EndBlock(ctx, req)
}

Expand All @@ -573,7 +574,7 @@ func (a *SimApp) Configurator() module.Configurator {
}

// InitChainer application update at chain initialization
func (app *SimApp) InitChainer(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain {
func (app *SimApp) InitChainer(ctx sdk.Context, req abci.RequestInitChain) (abci.ResponseInitChain, error) {
var genesisState GenesisState
if err := json.Unmarshal(req.AppStateBytes, &genesisState); err != nil {
panic(err)
Expand Down
6 changes: 5 additions & 1 deletion simapp/export.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ func (app *SimApp) ExportAppStateAndValidators(forZeroHeight bool, jailAllowedAd
app.prepForZeroHeightGenesis(ctx, jailAllowedAddrs)
}

genState := app.ModuleManager.ExportGenesisForModules(ctx, app.appCodec, modulesToExport)
genState, err := app.ModuleManager.ExportGenesisForModules(ctx, app.appCodec, modulesToExport)
if err != nil {
return servertypes.ExportedApp{}, err
}

appState, err := json.MarshalIndent(genState, "", " ")
if err != nil {
return servertypes.ExportedApp{}, err
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/params/suite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (s *E2ETestSuite) SetupSuite() {

// Make sure not to forget to persist `myParams` into the actual store,
// this is done in InitChain.
app.SetInitChainer(func(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain {
app.SetInitChainer(func(ctx sdk.Context, req abci.RequestInitChain) (abci.ResponseInitChain, error) {
subspace.SetParamSet(ctx, &paramSet)

return app.InitChainer(ctx, req)
Expand Down
1 change: 0 additions & 1 deletion tests/e2e/staking/suite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1497,7 +1497,6 @@ func (s *E2ETestSuite) TestBlockResults() {
)

return nil

}, 10)
}

Expand Down
6 changes: 3 additions & 3 deletions types/abci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@ import (
)

// InitChainer initializes application state at genesis
type InitChainer func(ctx Context, req abci.RequestInitChain) abci.ResponseInitChain
type InitChainer func(ctx Context, req abci.RequestInitChain) (abci.ResponseInitChain, error)

// BeginBlocker runs code before the transactions in a block
//
// Note: applications which set create_empty_blocks=false will not have regular block timing and should use
// e.g. BFT timestamps rather than block height for any periodic BeginBlock logic
type BeginBlocker func(ctx Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock
type BeginBlocker func(ctx Context, req abci.RequestBeginBlock) (abci.ResponseBeginBlock, error)

// EndBlocker runs code after the transactions in a block and return updates to the validator set
//
// Note: applications which set create_empty_blocks=false will not have regular block timing and should use
// e.g. BFT timestamps rather than block height for any periodic EndBlock logic
type EndBlocker func(ctx Context, req abci.RequestEndBlock) abci.ResponseEndBlock
type EndBlocker func(ctx Context, req abci.RequestEndBlock) (abci.ResponseEndBlock, error)

// PeerFilter responds to p2p filtering queries from Tendermint
type PeerFilter func(info string) abci.ResponseQuery
Expand Down
Loading