Return errors instead of panic (#3782)

This is related to issue #3741 where fetching data from
invalid store, package panic.

Modify subspace.go to return errors instead of panic.

Also update other packages that import subspace and
handle errors.

.pending/breaking/sdk/3782-Return-errors-i

@@ -0,0 +1 @@
+#3782 Return errors instead of panic

baseapp/baseapp.go

@@ -538,6 +538,7 @@ func (app *BaseApp) validateHeight(req abci.RequestBeginBlock) error {
 
 // BeginBlock implements the ABCI application interface.
 func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeginBlock) {
+	var err error
 	if app.cms.TracingEnabled() {
 		app.cms.SetTracingContext(sdk.TraceContext(
 			map[string]interface{}{"blockHeight": req.Header.Height},
@@ -572,7 +573,10 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg
 	app.deliverState.ctx = app.deliverState.ctx.WithBlockGasMeter(gasMeter)
 
 	if app.beginBlocker != nil {
-		res = app.beginBlocker(app.deliverState.ctx, req)
+		res, err = app.beginBlocker(app.deliverState.ctx, req)
+		if err != nil {
+			panic(err)
+		}
 	}
 
 	// set the signed validators for addition to context in deliverTx
@@ -874,12 +878,16 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk
 
 // EndBlock implements the ABCI interface.
 func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBlock) {
+	var err error
 	if app.deliverState.ms.TracingEnabled() {
 		app.deliverState.ms = app.deliverState.ms.SetTracingContext(nil).(sdk.CacheMultiStore)
 	}
 
 	if app.endBlocker != nil {
-		res = app.endBlocker(app.deliverState.ctx, req)
+		res, err = app.endBlocker(app.deliverState.ctx, req)
+		if err != nil {
+			panic(err)
+		}
 	}
 
 	return

cmd/gaia/app/app.go

@@ -218,30 +218,39 @@ func MakeCodec() *codec.Codec {
 }
 
 // application updates every end block
-func (app *GaiaApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock {
+func (app *GaiaApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) (resp abci.ResponseBeginBlock, err error) {
 	// mint new tokens for the previous block
-	mint.BeginBlocker(ctx, app.mintKeeper)
+	err = mint.BeginBlocker(ctx, app.mintKeeper)
+	if err != nil {
+		return resp, err
+	}
 
 	// distribute rewards for the previous block
-	distr.BeginBlocker(ctx, req, app.distrKeeper)
+	err = distr.BeginBlocker(ctx, req, app.distrKeeper)
+	if err != nil {
+		return resp, err
+	}
 
 	// slash anyone who double signed.
 	// NOTE: This should happen after distr.BeginBlocker so that
 	// there is nothing left over in the validator fee pool,
 	// so as to keep the CanWithdrawInvariant invariant.
 	// TODO: This should really happen at EndBlocker.
-	tags := slashing.BeginBlocker(ctx, req, app.slashingKeeper)
-
-	return abci.ResponseBeginBlock{
-		Tags: tags.ToKVPairs(),
-	}
+	resp.Tags, err = slashing.BeginBlocker(ctx, req, app.slashingKeeper)
+	return resp, err
 }
 
 // application updates every end block
 // nolint: unparam
-func (app *GaiaApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock {
-	tags := gov.EndBlocker(ctx, app.govKeeper)
-	validatorUpdates, endBlockerTags := staking.EndBlocker(ctx, app.stakingKeeper)
+func (app *GaiaApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) (abci.ResponseEndBlock, error) {
+	tags, err := gov.EndBlocker(ctx, app.govKeeper)
+	if err != nil {
+		return abci.ResponseEndBlock{}, err
+	}
+	validatorUpdates, endBlockerTags, err := staking.EndBlocker(ctx, app.stakingKeeper)
+	if err != nil {
+		return abci.ResponseEndBlock{}, err
+	}
 	tags = append(tags, endBlockerTags...)
 
 	if app.assertInvariantsBlockly {
@@ -251,7 +260,7 @@ func (app *GaiaApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.R
 	return abci.ResponseEndBlock{
 		ValidatorUpdates: validatorUpdates,
 		Tags:             tags,
-	}
+	}, nil
 }
 
 // initialize store from a genesis state

cmd/gaia/app/export.go

@@ -39,12 +39,16 @@ func (app *GaiaApp) ExportAppStateAndValidators(forZeroHeight bool, jailWhiteLis
 	}
 	app.accountKeeper.IterateAccounts(ctx, appendAccount)
 
+	mintGenesisState, err := mint.ExportGenesis(ctx, app.mintKeeper)
+	if err != nil {
+		return nil, nil, err
+	}
 	genState := NewGenesisState(
 		accounts,
 		auth.ExportGenesis(ctx, app.accountKeeper, app.feeCollectionKeeper),
 		bank.ExportGenesis(ctx, app.bankKeeper),
 		staking.ExportGenesis(ctx, app.stakingKeeper),
-		mint.ExportGenesis(ctx, app.mintKeeper),
+		mintGenesisState,
 		distr.ExportGenesis(ctx, app.distrKeeper),
 		gov.ExportGenesis(ctx, app.govKeeper),
 		crisis.ExportGenesis(ctx, app.crisisKeeper),

cmd/gaia/cmd/gaiadebug/hack.go

@@ -218,23 +218,23 @@ func MakeCodec() *codec.Codec {
 }
 
 // application updates every end block
-func (app *GaiaApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock {
-	tags := slashing.BeginBlocker(ctx, req, app.slashingKeeper)
-
-	return abci.ResponseBeginBlock{
-		Tags: tags.ToKVPairs(),
-	}
+func (app *GaiaApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) (resp abci.ResponseBeginBlock, err error) {
+	resp.Tags, err = slashing.BeginBlocker(ctx, req, app.slashingKeeper)
+	return resp, err
 }
 
 // application updates every end block
 // nolint: unparam
-func (app *GaiaApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock {
-	validatorUpdates, tags := staking.EndBlocker(ctx, app.stakingKeeper)
+func (app *GaiaApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) (abci.ResponseEndBlock, error) {
+	validatorUpdates, tags, err := staking.EndBlocker(ctx, app.stakingKeeper)
+	if err != nil {
+		return abci.ResponseEndBlock{}, err
+	}
 
 	return abci.ResponseEndBlock{
 		ValidatorUpdates: validatorUpdates,
 		Tags:             tags,
-	}
+	}, nil
 }
 
 // custom logic for gaia initialization

types/abci.go

@@ -6,10 +6,10 @@ import abci "github.com/tendermint/tendermint/abci/types"
 type InitChainer func(ctx Context, req abci.RequestInitChain) abci.ResponseInitChain
 
 // run code before the transactions in a block
-type BeginBlocker func(ctx Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock
+type BeginBlocker func(ctx Context, req abci.RequestBeginBlock) (abci.ResponseBeginBlock, error)
 
 // run code after the transactions in a block and return updates to the validator set
-type EndBlocker func(ctx Context, req abci.RequestEndBlock) abci.ResponseEndBlock
+type EndBlocker func(ctx Context, req abci.RequestEndBlock) (abci.ResponseEndBlock, error)
 
 // respond to p2p filtering queries from Tendermint
 type PeerFilter func(info string) abci.ResponseQuery

x/bank/keeper.go

@@ -168,7 +168,12 @@ func (keeper BaseSendKeeper) GetSendEnabled(ctx sdk.Context) bool {
 
 // SetSendEnabled sets the send enabled
 func (keeper BaseSendKeeper) SetSendEnabled(ctx sdk.Context, enabled bool) {
-	keeper.paramSpace.Set(ctx, ParamStoreKeySendEnabled, &enabled)
+	err := keeper.paramSpace.Set(ctx, ParamStoreKeySendEnabled, &enabled)
+	if err != nil {
+		// TODO: return error - needs rewrite interfaces
+		// and handle error on the caller side
+		// check PR #3782
+	}
 }
 
 var _ ViewKeeper = (*BaseViewKeeper)(nil)

x/crisis/params.go

@@ -24,11 +24,19 @@ func ParamKeyTable() params.KeyTable {
 
 // GetConstantFee get's the constant fee from the paramSpace
 func (k Keeper) GetConstantFee(ctx sdk.Context) (constantFee sdk.Coin) {
-	k.paramSpace.Get(ctx, ParamStoreKeyConstantFee, &constantFee)
+	if err := k.paramSpace.Get(ctx, ParamStoreKeyConstantFee, &constantFee); err != nil {
+		// TODO: return error - needs rewrite interfaces
+		// and handle error on the caller side
+		// check PR #3782
+	}
 	return
 }
 
 // GetConstantFee set's the constant fee in the paramSpace
 func (k Keeper) SetConstantFee(ctx sdk.Context, constantFee sdk.Coin) {
-	k.paramSpace.Set(ctx, ParamStoreKeyConstantFee, constantFee)
+	if err := k.paramSpace.Set(ctx, ParamStoreKeyConstantFee, constantFee); err != nil {
+		// TODO: return error - needs rewrite interfaces
+		// and handle error on the caller side
+		// check PR #3782
+	}
 }

x/distribution/abci_app.go

@@ -8,7 +8,7 @@ import (
 )
 
 // set the proposer for determining distribution during endblock
-func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper) {
+func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper) error {
 
 	// determine the total power signing the block
 	var previousTotalPower, sumPreviousPrecommitPower int64
@@ -29,5 +29,5 @@ func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper)
 	// record the proposer for when we payout on the next block
 	consAddr := sdk.ConsAddress(req.Header.ProposerAddress)
 	k.SetPreviousProposerConsAddr(ctx, consAddr)
-
+	return nil
 }

x/gov/endblocker.go

@@ -8,7 +8,7 @@ import (
 )
 
 // Called every block, process inflation, update validator set
-func EndBlocker(ctx sdk.Context, keeper Keeper) sdk.Tags {
+func EndBlocker(ctx sdk.Context, keeper Keeper) (sdk.Tags, error) {
 	logger := ctx.Logger().With("module", "x/gov")
 	resTags := sdk.NewTags()
 
@@ -78,5 +78,5 @@ func EndBlocker(ctx sdk.Context, keeper Keeper) sdk.Tags {
 		resTags = resTags.AppendTag(tags.ProposalResult, tagValue)
 	}
 
-	return resTags
+	return resTags, nil
 }

x/gov/keeper.go

@@ -261,34 +261,64 @@ func (keeper Keeper) activateVotingPeriod(ctx sdk.Context, proposal Proposal) {
 // Returns the current DepositParams from the global param store
 func (keeper Keeper) GetDepositParams(ctx sdk.Context) DepositParams {
 	var depositParams DepositParams
-	keeper.paramSpace.Get(ctx, ParamStoreKeyDepositParams, &depositParams)
+	err := keeper.paramSpace.Get(ctx, ParamStoreKeyDepositParams, &depositParams)
+	if err != nil {
+		// TODO: return error - needs rewrite interfaces
+		// and handle error on the caller side
+		// check PR #3782
+	}
 	return depositParams
 }
 
 // Returns the current VotingParams from the global param store
 func (keeper Keeper) GetVotingParams(ctx sdk.Context) VotingParams {
 	var votingParams VotingParams
-	keeper.paramSpace.Get(ctx, ParamStoreKeyVotingParams, &votingParams)
+	err := keeper.paramSpace.Get(ctx, ParamStoreKeyVotingParams, &votingParams)
+	if err != nil {
+		// TODO: return error - needs rewrite interfaces
+		// and handle error on the caller side
+		// check PR #3782
+	}
 	return votingParams
 }
 
 // Returns the current TallyParam from the global param store
 func (keeper Keeper) GetTallyParams(ctx sdk.Context) TallyParams {
 	var tallyParams TallyParams
-	keeper.paramSpace.Get(ctx, ParamStoreKeyTallyParams, &tallyParams)
+	err := keeper.paramSpace.Get(ctx, ParamStoreKeyTallyParams, &tallyParams)
+	if err != nil {
+		// TODO: return error - needs rewrite interfaces
+		// and handle error on the caller side
+		// check PR #3782
+	}
 	return tallyParams
 }
 
 func (keeper Keeper) setDepositParams(ctx sdk.Context, depositParams DepositParams) {
-	keeper.paramSpace.Set(ctx, ParamStoreKeyDepositParams, &depositParams)
+	err := keeper.paramSpace.Set(ctx, ParamStoreKeyDepositParams, &depositParams)
+	if err != nil {
+		// TODO: return error - needs rewrite interfaces
+		// and handle error on the caller side
+		// check PR #3782
+	}
 }
 
 func (keeper Keeper) setVotingParams(ctx sdk.Context, votingParams VotingParams) {
-	keeper.paramSpace.Set(ctx, ParamStoreKeyVotingParams, &votingParams)
+	err := keeper.paramSpace.Set(ctx, ParamStoreKeyVotingParams, &votingParams)
+	if err != nil {
+		// TODO: return error - needs rewrite interfaces
+		// and handle error on the caller side
+		// check PR #3782
+	}
 }
 
 func (keeper Keeper) setTallyParams(ctx sdk.Context, tallyParams TallyParams) {
-	keeper.paramSpace.Set(ctx, ParamStoreKeyTallyParams, &tallyParams)
+	err := keeper.paramSpace.Set(ctx, ParamStoreKeyTallyParams, &tallyParams)
+	if err != nil {
+		// TODO: return error - needs rewrite interfaces
+		// and handle error on the caller side
+		// check PR #3782
+	}
 }
 
 // Votes

x/gov/test_common.go

@@ -58,11 +58,11 @@ func getMockApp(t *testing.T, numGenAccs int, genState GenesisState, genAccs []a
 
 // gov and staking endblocker
 func getEndBlocker(keeper Keeper) sdk.EndBlocker {
-	return func(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock {
-		tags := EndBlocker(ctx, keeper)
+	return func(ctx sdk.Context, req abci.RequestEndBlock) (abci.ResponseEndBlock, error) {
+		tags, err := EndBlocker(ctx, keeper)
 		return abci.ResponseEndBlock{
 			Tags: tags,
-		}
+		}, err
 	}
 }
 

x/mint/abci_app.go

@@ -5,11 +5,17 @@ import (
 )
 
 // Inflate every block, update inflation parameters once per hour
-func BeginBlocker(ctx sdk.Context, k Keeper) {
+func BeginBlocker(ctx sdk.Context, k Keeper) error {
 
 	// fetch stored minter & params
-	minter := k.GetMinter(ctx)
-	params := k.GetParams(ctx)
+	minter, err := k.GetMinter(ctx)
+	if err != nil {
+		return err
+	}
+	params, err := k.GetParams(ctx)
+	if err != nil {
+		return err
+	}
 
 	// recalculate inflation rate
 	totalSupply := k.sk.TotalTokens(ctx)
@@ -22,5 +28,6 @@ func BeginBlocker(ctx sdk.Context, k Keeper) {
 	mintedCoin := minter.BlockProvision(params)
 	k.fck.AddCollectedFees(ctx, sdk.Coins{mintedCoin})
 	k.sk.InflateSupply(ctx, mintedCoin.Amount)
+	return nil
 
 }

x/mint/genesis.go

@@ -29,15 +29,25 @@ func DefaultGenesisState() GenesisState {
 // new mint genesis
 func InitGenesis(ctx sdk.Context, keeper Keeper, data GenesisState) {
 	keeper.SetMinter(ctx, data.Minter)
-	keeper.SetParams(ctx, data.Params)
+	if err := keeper.SetParams(ctx, data.Params); err != nil {
+		// TODO: return error - needs rewrite interfaces
+		// and handle error on the caller side
+		// check PR #3782
+	}
 }
 
-// ExportGenesis returns a GenesisState for a given context and keeper.
-func ExportGenesis(ctx sdk.Context, keeper Keeper) GenesisState {
-
-	minter := keeper.GetMinter(ctx)
-	params := keeper.GetParams(ctx)
-	return NewGenesisState(minter, params)
+// ExportGenesis returns a GenesisState for a given context and keeper. The
+// GenesisState will contain the pool, and validator/delegator distribution info's
+func ExportGenesis(ctx sdk.Context, keeper Keeper) (GenesisState, error) {
+	minter, err := keeper.GetMinter(ctx)
+	if err != nil {
+		return GenesisState{}, err
+	}
+	params, err := keeper.GetParams(ctx)
+	if err != nil {
+		return GenesisState{}, err
+	}
+	return NewGenesisState(minter, params), nil
 }
 
 // ValidateGenesis validates the provided genesis state to ensure the