diff --git a/.pending/breaking/sdk/3782-Return-errors-i b/.pending/breaking/sdk/3782-Return-errors-i deleted file mode 100644 index 6c717963ce..0000000000 --- a/.pending/breaking/sdk/3782-Return-errors-i +++ /dev/null @@ -1 +0,0 @@ -#3782 Return errors instead of panic diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 2b8dfd3e94..b244a98b77 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -538,7 +538,6 @@ 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}, @@ -573,10 +572,7 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg app.deliverState.ctx = app.deliverState.ctx.WithBlockGasMeter(gasMeter) if app.beginBlocker != nil { - res, err = app.beginBlocker(app.deliverState.ctx, req) - if err != nil { - panic(err) - } + res = app.beginBlocker(app.deliverState.ctx, req) } // set the signed validators for addition to context in deliverTx @@ -878,16 +874,12 @@ 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, err = app.endBlocker(app.deliverState.ctx, req) - if err != nil { - panic(err) - } + res = app.endBlocker(app.deliverState.ctx, req) } return diff --git a/cmd/gaia/app/app.go b/cmd/gaia/app/app.go index 24d24ee87b..a856094728 100644 --- a/cmd/gaia/app/app.go +++ b/cmd/gaia/app/app.go @@ -216,39 +216,30 @@ func MakeCodec() *codec.Codec { } // application updates every end block -func (app *GaiaApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) (resp abci.ResponseBeginBlock, err error) { +func (app *GaiaApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock { // mint new tokens for the previous block - err = mint.BeginBlocker(ctx, app.mintKeeper) - if err != nil { - return resp, err - } + mint.BeginBlocker(ctx, app.mintKeeper) // distribute rewards for the previous block - err = distr.BeginBlocker(ctx, req, app.distrKeeper) - if err != nil { - return resp, err - } + distr.BeginBlocker(ctx, req, app.distrKeeper) // 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. - resp.Tags, err = slashing.BeginBlocker(ctx, req, app.slashingKeeper) - return resp, err + tags := slashing.BeginBlocker(ctx, req, app.slashingKeeper) + + return abci.ResponseBeginBlock{ + Tags: tags.ToKVPairs(), + } } // application updates every end block // nolint: unparam -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 - } +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) tags = append(tags, endBlockerTags...) if app.assertInvariantsBlockly { @@ -258,7 +249,7 @@ func (app *GaiaApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) (abci. return abci.ResponseEndBlock{ ValidatorUpdates: validatorUpdates, Tags: tags, - }, nil + } } // initialize store from a genesis state diff --git a/cmd/gaia/app/export.go b/cmd/gaia/app/export.go index 217d9a1b3d..5174c1498a 100644 --- a/cmd/gaia/app/export.go +++ b/cmd/gaia/app/export.go @@ -39,16 +39,12 @@ 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), - mintGenesisState, + mint.ExportGenesis(ctx, app.mintKeeper), distr.ExportGenesis(ctx, app.distrKeeper), gov.ExportGenesis(ctx, app.govKeeper), crisis.ExportGenesis(ctx, app.crisisKeeper), diff --git a/cmd/gaia/cmd/gaiadebug/hack.go b/cmd/gaia/cmd/gaiadebug/hack.go index 74346a2d92..51927592a4 100644 --- a/cmd/gaia/cmd/gaiadebug/hack.go +++ b/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) (resp abci.ResponseBeginBlock, err error) { - resp.Tags, err = slashing.BeginBlocker(ctx, req, app.slashingKeeper) - return resp, err +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(), + } } // application updates every end block // nolint: unparam -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 - } +func (app *GaiaApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock { + validatorUpdates, tags := staking.EndBlocker(ctx, app.stakingKeeper) return abci.ResponseEndBlock{ ValidatorUpdates: validatorUpdates, Tags: tags, - }, nil + } } // custom logic for gaia initialization diff --git a/types/abci.go b/types/abci.go index bbf2e7225d..0646d21e32 100644 --- a/types/abci.go +++ b/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, error) +type BeginBlocker func(ctx Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock // 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, error) +type EndBlocker func(ctx Context, req abci.RequestEndBlock) abci.ResponseEndBlock // respond to p2p filtering queries from Tendermint type PeerFilter func(info string) abci.ResponseQuery diff --git a/x/bank/keeper.go b/x/bank/keeper.go index cd8bc2c358..c5322ec5c7 100644 --- a/x/bank/keeper.go +++ b/x/bank/keeper.go @@ -168,12 +168,7 @@ func (keeper BaseSendKeeper) GetSendEnabled(ctx sdk.Context) bool { // SetSendEnabled sets the send enabled func (keeper BaseSendKeeper) SetSendEnabled(ctx sdk.Context, enabled bool) { - 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 - } + keeper.paramSpace.Set(ctx, ParamStoreKeySendEnabled, &enabled) } var _ ViewKeeper = (*BaseViewKeeper)(nil) diff --git a/x/crisis/params.go b/x/crisis/params.go index 91d68d1bf3..14c8a25673 100644 --- a/x/crisis/params.go +++ b/x/crisis/params.go @@ -24,19 +24,11 @@ func ParamKeyTable() params.KeyTable { // GetConstantFee get's the constant fee from the paramSpace func (k Keeper) GetConstantFee(ctx sdk.Context) (constantFee sdk.Coin) { - 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 - } + k.paramSpace.Get(ctx, ParamStoreKeyConstantFee, &constantFee) return } // GetConstantFee set's the constant fee in the paramSpace func (k Keeper) SetConstantFee(ctx sdk.Context, constantFee sdk.Coin) { - 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 - } + k.paramSpace.Set(ctx, ParamStoreKeyConstantFee, constantFee) } diff --git a/x/distribution/abci_app.go b/x/distribution/abci_app.go index 46e2652749..0a2ef5627a 100644 --- a/x/distribution/abci_app.go +++ b/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) error { +func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper) { // 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 + } diff --git a/x/gov/endblocker.go b/x/gov/endblocker.go index ed08ae6c32..3da7f3a1da 100644 --- a/x/gov/endblocker.go +++ b/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, error) { +func EndBlocker(ctx sdk.Context, keeper Keeper) sdk.Tags { logger := ctx.Logger().With("module", "x/gov") resTags := sdk.NewTags() @@ -78,5 +78,5 @@ func EndBlocker(ctx sdk.Context, keeper Keeper) (sdk.Tags, error) { resTags = resTags.AppendTag(tags.ProposalResult, tagValue) } - return resTags, nil + return resTags } diff --git a/x/gov/keeper.go b/x/gov/keeper.go index 297915303a..46085cf2d9 100644 --- a/x/gov/keeper.go +++ b/x/gov/keeper.go @@ -261,64 +261,34 @@ 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 - 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 - } + keeper.paramSpace.Get(ctx, ParamStoreKeyDepositParams, &depositParams) return depositParams } // Returns the current VotingParams from the global param store func (keeper Keeper) GetVotingParams(ctx sdk.Context) VotingParams { var votingParams 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 - } + keeper.paramSpace.Get(ctx, ParamStoreKeyVotingParams, &votingParams) return votingParams } // Returns the current TallyParam from the global param store func (keeper Keeper) GetTallyParams(ctx sdk.Context) TallyParams { var tallyParams 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 - } + keeper.paramSpace.Get(ctx, ParamStoreKeyTallyParams, &tallyParams) return tallyParams } func (keeper Keeper) setDepositParams(ctx sdk.Context, depositParams 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 - } + keeper.paramSpace.Set(ctx, ParamStoreKeyDepositParams, &depositParams) } func (keeper Keeper) setVotingParams(ctx sdk.Context, votingParams 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 - } + keeper.paramSpace.Set(ctx, ParamStoreKeyVotingParams, &votingParams) } func (keeper Keeper) setTallyParams(ctx sdk.Context, tallyParams 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 - } + keeper.paramSpace.Set(ctx, ParamStoreKeyTallyParams, &tallyParams) } // Votes diff --git a/x/gov/test_common.go b/x/gov/test_common.go index 1703dfcf52..a14251841d 100644 --- a/x/gov/test_common.go +++ b/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, error) { - tags, err := EndBlocker(ctx, keeper) + return func(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock { + tags := EndBlocker(ctx, keeper) return abci.ResponseEndBlock{ Tags: tags, - }, err + } } } diff --git a/x/mint/abci_app.go b/x/mint/abci_app.go index ea998185a4..5016a464d1 100644 --- a/x/mint/abci_app.go +++ b/x/mint/abci_app.go @@ -5,17 +5,11 @@ import ( ) // Inflate every block, update inflation parameters once per hour -func BeginBlocker(ctx sdk.Context, k Keeper) error { +func BeginBlocker(ctx sdk.Context, k Keeper) { // fetch stored minter & params - minter, err := k.GetMinter(ctx) - if err != nil { - return err - } - params, err := k.GetParams(ctx) - if err != nil { - return err - } + minter := k.GetMinter(ctx) + params := k.GetParams(ctx) // recalculate inflation rate totalSupply := k.sk.TotalTokens(ctx) @@ -28,6 +22,5 @@ func BeginBlocker(ctx sdk.Context, k Keeper) error { mintedCoin := minter.BlockProvision(params) k.fck.AddCollectedFees(ctx, sdk.Coins{mintedCoin}) k.sk.InflateSupply(ctx, mintedCoin.Amount) - return nil } diff --git a/x/mint/genesis.go b/x/mint/genesis.go index 0408eb2dfb..11f96c6f90 100644 --- a/x/mint/genesis.go +++ b/x/mint/genesis.go @@ -29,25 +29,15 @@ func DefaultGenesisState() GenesisState { // new mint genesis func InitGenesis(ctx sdk.Context, keeper Keeper, data GenesisState) { keeper.SetMinter(ctx, data.Minter) - 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 - } + keeper.SetParams(ctx, data.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 +// 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) } // ValidateGenesis validates the provided genesis state to ensure the diff --git a/x/mint/keeper.go b/x/mint/keeper.go index 4b48b02edf..42a9f19fba 100644 --- a/x/mint/keeper.go +++ b/x/mint/keeper.go @@ -1,8 +1,6 @@ package mint import ( - "errors" - "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/params" @@ -64,12 +62,11 @@ func ParamKeyTable() params.KeyTable { //______________________________________________________________________ // get the minter -func (k Keeper) GetMinter(ctx sdk.Context) (minter Minter, err error) { +func (k Keeper) GetMinter(ctx sdk.Context) (minter Minter) { store := ctx.KVStore(k.storeKey) b := store.Get(minterKey) if b == nil { - err = errors.New("Stored fee pool should not have been nil") - return + panic("Stored fee pool should not have been nil") } k.cdc.MustUnmarshalBinaryLengthPrefixed(b, &minter) return @@ -85,13 +82,13 @@ func (k Keeper) SetMinter(ctx sdk.Context, minter Minter) { //______________________________________________________________________ // get inflation params from the global param store -func (k Keeper) GetParams(ctx sdk.Context) (Params, error) { +func (k Keeper) GetParams(ctx sdk.Context) Params { var params Params - err := k.paramSpace.Get(ctx, ParamStoreKeyParams, ¶ms) - return params, err + k.paramSpace.Get(ctx, ParamStoreKeyParams, ¶ms) + return params } // set inflation params from the global param store -func (k Keeper) SetParams(ctx sdk.Context, params Params) error { - return k.paramSpace.Set(ctx, ParamStoreKeyParams, ¶ms) +func (k Keeper) SetParams(ctx sdk.Context, params Params) { + k.paramSpace.Set(ctx, ParamStoreKeyParams, ¶ms) } diff --git a/x/mint/querier.go b/x/mint/querier.go index e70142cac8..4de3fdeeb5 100644 --- a/x/mint/querier.go +++ b/x/mint/querier.go @@ -36,10 +36,7 @@ func NewQuerier(k Keeper) sdk.Querier { } func queryParams(ctx sdk.Context, k Keeper) ([]byte, sdk.Error) { - params, err := k.GetParams(ctx) - if err != nil { - return nil, sdk.ErrInternal(sdk.AppendMsgToErr("failed to get params", err.Error())) - } + params := k.GetParams(ctx) res, err := codec.MarshalJSONIndent(k.cdc, params) if err != nil { @@ -50,10 +47,7 @@ func queryParams(ctx sdk.Context, k Keeper) ([]byte, sdk.Error) { } func queryInflation(ctx sdk.Context, k Keeper) ([]byte, sdk.Error) { - minter, err := k.GetMinter(ctx) - if err != nil { - return nil, sdk.ErrInternal(sdk.AppendMsgToErr("failed to get params", err.Error())) - } + minter := k.GetMinter(ctx) res, err := codec.MarshalJSONIndent(k.cdc, minter.Inflation) if err != nil { @@ -64,10 +58,7 @@ func queryInflation(ctx sdk.Context, k Keeper) ([]byte, sdk.Error) { } func queryAnnualProvisions(ctx sdk.Context, k Keeper) ([]byte, sdk.Error) { - minter, err := k.GetMinter(ctx) - if err != nil { - return nil, sdk.ErrInternal(sdk.AppendMsgToErr("failed to get params", err.Error())) - } + minter := k.GetMinter(ctx) res, err := codec.MarshalJSONIndent(k.cdc, minter.AnnualProvisions) if err != nil { diff --git a/x/mint/querier_test.go b/x/mint/querier_test.go index f0478c61bf..7a73853d0a 100644 --- a/x/mint/querier_test.go +++ b/x/mint/querier_test.go @@ -43,9 +43,7 @@ func TestQueryParams(t *testing.T) { err := input.cdc.UnmarshalJSON(res, ¶ms) require.NoError(t, err) - parm, err := input.mintKeeper.GetParams(input.ctx) - require.NoError(t, err) - require.Equal(t, parm, params) + require.Equal(t, input.mintKeeper.GetParams(input.ctx), params) } func TestQueryInflation(t *testing.T) { @@ -59,9 +57,7 @@ func TestQueryInflation(t *testing.T) { err := input.cdc.UnmarshalJSON(res, &inflation) require.NoError(t, err) - parm, err := input.mintKeeper.GetMinter(input.ctx) - require.NoError(t, err) - require.Equal(t, parm.Inflation, inflation) + require.Equal(t, input.mintKeeper.GetMinter(input.ctx).Inflation, inflation) } func TestQueryAnnualProvisions(t *testing.T) { @@ -75,7 +71,5 @@ func TestQueryAnnualProvisions(t *testing.T) { err := input.cdc.UnmarshalJSON(res, &annualProvisions) require.NoError(t, err) - parm, err := input.mintKeeper.GetMinter(input.ctx) - require.NoError(t, err) - require.Equal(t, parm.AnnualProvisions, annualProvisions) + require.Equal(t, input.mintKeeper.GetMinter(input.ctx).AnnualProvisions, annualProvisions) } diff --git a/x/mint/test_common.go b/x/mint/test_common.go index 768391755d..f2bec95157 100644 --- a/x/mint/test_common.go +++ b/x/mint/test_common.go @@ -69,8 +69,7 @@ func newTestInput(t *testing.T) testInput { ctx := sdk.NewContext(ms, abci.Header{Time: time.Unix(0, 0)}, false, log.NewTMLogger(os.Stdout)) - err = mintKeeper.SetParams(ctx, DefaultParams()) - require.Nil(t, err) + mintKeeper.SetParams(ctx, DefaultParams()) mintKeeper.SetMinter(ctx, DefaultInitialMinter()) return testInput{ctx, cdc, mintKeeper} diff --git a/x/params/keeper_test.go b/x/params/keeper_test.go index f96081e5fc..4d20e56ad9 100644 --- a/x/params/keeper_test.go +++ b/x/params/keeper_test.go @@ -108,12 +108,12 @@ func TestKeeper(t *testing.T) { // Test invalid space.Get for i, kv := range kvs { var param bool - require.Error(t, space.Get(ctx, []byte(kv.key), ¶m), "invalid space.Get not error, tc #%d", i) + require.Panics(t, func() { space.Get(ctx, []byte(kv.key), ¶m) }, "invalid space.Get not panics, tc #%d", i) } // Test invalid space.Set for i, kv := range kvs { - require.Error(t, space.Set(ctx, []byte(kv.key), true), "invalid space.Set not error, tc #%d", i) + require.Panics(t, func() { space.Set(ctx, []byte(kv.key), true) }, "invalid space.Set not panics, tc #%d", i) } // Test GetSubspace @@ -122,12 +122,12 @@ func TestKeeper(t *testing.T) { gspace, ok := keeper.GetSubspace("test") require.True(t, ok, "cannot retrieve subspace, tc #%d", i) - require.NoError(t, gspace.Get(ctx, []byte(kv.key), &gparam)) - require.NoError(t, space.Get(ctx, []byte(kv.key), ¶m)) + require.NotPanics(t, func() { gspace.Get(ctx, []byte(kv.key), &gparam) }) + require.NotPanics(t, func() { space.Get(ctx, []byte(kv.key), ¶m) }) require.Equal(t, gparam, param, "GetSubspace().Get not equal with space.Get, tc #%d", i) - require.NoError(t, gspace.Set(ctx, []byte(kv.key), int64(i))) - require.NoError(t, space.Get(ctx, []byte(kv.key), ¶m)) + require.NotPanics(t, func() { gspace.Set(ctx, []byte(kv.key), int64(i)) }) + require.NotPanics(t, func() { space.Get(ctx, []byte(kv.key), ¶m) }) require.Equal(t, int64(i), param, "GetSubspace().Set not equal with space.Get, tc #%d", i) } } @@ -184,27 +184,27 @@ func TestSubspace(t *testing.T) { // Test space.Set, space.Modified for i, kv := range kvs { require.False(t, space.Modified(ctx, []byte(kv.key)), "space.Modified returns true before setting, tc #%d", i) - require.NoError(t, space.Set(ctx, []byte(kv.key), kv.param), "space.Set error, tc #%d", i) + require.NotPanics(t, func() { space.Set(ctx, []byte(kv.key), kv.param) }, "space.Set panics, tc #%d", i) require.True(t, space.Modified(ctx, []byte(kv.key)), "space.Modified returns false after setting, tc #%d", i) } // Test space.Get, space.GetIfExists for i, kv := range kvs { - require.Error(t, space.GetIfExists(ctx, []byte("invalid"), kv.ptr), "space.GetIfExists error when no value exists, tc #%d", i) + require.NotPanics(t, func() { space.GetIfExists(ctx, []byte("invalid"), kv.ptr) }, "space.GetIfExists panics when no value exists, tc #%d", i) require.Equal(t, kv.zero, indirect(kv.ptr), "space.GetIfExists unmarshalls when no value exists, tc #%d", i) - require.Error(t, space.Get(ctx, []byte("invalid"), kv.ptr), "invalid space.Get not error when no value exists, tc #%d", i) + require.Panics(t, func() { space.Get(ctx, []byte("invalid"), kv.ptr) }, "invalid space.Get not panics when no value exists, tc #%d", i) require.Equal(t, kv.zero, indirect(kv.ptr), "invalid space.Get unmarshalls when no value exists, tc #%d", i) - require.NoError(t, space.GetIfExists(ctx, []byte(kv.key), kv.ptr), "space.GetIfExists error, tc #%d", i) + require.NotPanics(t, func() { space.GetIfExists(ctx, []byte(kv.key), kv.ptr) }, "space.GetIfExists panics, tc #%d", i) require.Equal(t, kv.param, indirect(kv.ptr), "stored param not equal, tc #%d", i) - require.NoError(t, space.Get(ctx, []byte(kv.key), kv.ptr), "space.Get error, tc #%d", i) + require.NotPanics(t, func() { space.Get(ctx, []byte(kv.key), kv.ptr) }, "space.Get panics, tc #%d", i) require.Equal(t, kv.param, indirect(kv.ptr), "stored param not equal, tc #%d", i) - require.Error(t, space.Get(ctx, []byte("invalid"), kv.ptr), "invalid space.Get not error when no value exists, tc #%d", i) + require.Panics(t, func() { space.Get(ctx, []byte("invalid"), kv.ptr) }, "invalid space.Get not panics when no value exists, tc #%d", i) require.Equal(t, kv.param, indirect(kv.ptr), "invalid space.Get unmarshalls when no value existt, tc #%d", i) - require.Error(t, space.Get(ctx, []byte(kv.key), nil), "invalid space.Get not error when the pointer is nil, tc #%d", i) - require.Error(t, space.Get(ctx, []byte(kv.key), new(invalid)), "invalid space.Get not error when the pointer is different type, tc #%d", i) + require.Panics(t, func() { space.Get(ctx, []byte(kv.key), nil) }, "invalid space.Get not panics when the pointer is nil, tc #%d", i) + require.Panics(t, func() { space.Get(ctx, []byte(kv.key), new(invalid)) }, "invalid space.Get not panics when the pointer is different type, tc #%d", i) } // Test store.Get equals space.Get diff --git a/x/params/subspace/subspace.go b/x/params/subspace/subspace.go index 5ffb415709..72cb5f209b 100644 --- a/x/params/subspace/subspace.go +++ b/x/params/subspace/subspace.go @@ -1,7 +1,6 @@ package subspace import ( - "errors" "reflect" "github.com/cosmos/cosmos-sdk/codec" @@ -91,31 +90,37 @@ func concatKeys(key, subkey []byte) (res []byte) { } // Get parameter from store -func (s Subspace) Get(ctx sdk.Context, key []byte, ptr interface{}) error { +func (s Subspace) Get(ctx sdk.Context, key []byte, ptr interface{}) { store := s.kvStore(ctx) bz := store.Get(key) - return s.cdc.UnmarshalJSON(bz, ptr) + err := s.cdc.UnmarshalJSON(bz, ptr) + if err != nil { + panic(err) + } } // GetIfExists do not modify ptr if the stored parameter is nil -func (s Subspace) GetIfExists(ctx sdk.Context, key []byte, ptr interface{}) error { +func (s Subspace) GetIfExists(ctx sdk.Context, key []byte, ptr interface{}) { store := s.kvStore(ctx) bz := store.Get(key) if bz == nil { - return errors.New("store key does not exist") + return + } + err := s.cdc.UnmarshalJSON(bz, ptr) + if err != nil { + panic(err) } - return s.cdc.UnmarshalJSON(bz, ptr) } // GetWithSubkey returns a parameter with a given key and a subkey. -func (s Subspace) GetWithSubkey(ctx sdk.Context, key, subkey []byte, ptr interface{}) error { - return s.Get(ctx, concatKeys(key, subkey), ptr) +func (s Subspace) GetWithSubkey(ctx sdk.Context, key, subkey []byte, ptr interface{}) { + s.Get(ctx, concatKeys(key, subkey), ptr) } // GetWithSubkeyIfExists returns a parameter with a given key and a subkey but does not // modify ptr if the stored parameter is nil. -func (s Subspace) GetWithSubkeyIfExists(ctx sdk.Context, key, subkey []byte, ptr interface{}) error { - return s.GetIfExists(ctx, concatKeys(key, subkey), ptr) +func (s Subspace) GetWithSubkeyIfExists(ctx sdk.Context, key, subkey []byte, ptr interface{}) { + s.GetIfExists(ctx, concatKeys(key, subkey), ptr) } // Get raw bytes of parameter from store @@ -136,10 +141,10 @@ func (s Subspace) Modified(ctx sdk.Context, key []byte) bool { return tstore.Has(key) } -func (s Subspace) checkType(store sdk.KVStore, key []byte, param interface{}) error { +func (s Subspace) checkType(store sdk.KVStore, key []byte, param interface{}) { attr, ok := s.table.m[string(key)] if !ok { - return errors.New("Parameter not registered") + panic("Parameter not registered") } ty := attr.ty @@ -149,61 +154,51 @@ func (s Subspace) checkType(store sdk.KVStore, key []byte, param interface{}) er } if pty != ty { - return errors.New("Type mismatch with registered table") + panic("Type mismatch with registered table") } - return nil } // Set stores the parameter. It returns error if stored parameter has different type from input. // It also set to the transient store to record change. -func (s Subspace) Set(ctx sdk.Context, key []byte, param interface{}) error { +func (s Subspace) Set(ctx sdk.Context, key []byte, param interface{}) { store := s.kvStore(ctx) - if err := s.checkType(store, key, param); err != nil { - return err - } + s.checkType(store, key, param) bz, err := s.cdc.MarshalJSON(param) if err != nil { - return err + panic(err) } store.Set(key, bz) tstore := s.transientStore(ctx) tstore.Set(key, []byte{}) - return nil + } // SetWithSubkey set a parameter with a key and subkey // Checks parameter type only over the key -func (s Subspace) SetWithSubkey(ctx sdk.Context, key []byte, subkey []byte, param interface{}) error { +func (s Subspace) SetWithSubkey(ctx sdk.Context, key []byte, subkey []byte, param interface{}) { store := s.kvStore(ctx) - if err := s.checkType(store, key, param); err != nil { - return err - } + s.checkType(store, key, param) newkey := concatKeys(key, subkey) bz, err := s.cdc.MarshalJSON(param) if err != nil { - return err + panic(err) } store.Set(newkey, bz) tstore := s.transientStore(ctx) tstore.Set(newkey, []byte{}) - return nil } // Get to ParamSet func (s Subspace) GetParamSet(ctx sdk.Context, ps ParamSet) { for _, pair := range ps.ParamSetPairs() { - err := s.Get(ctx, pair.Key, pair.Value) - if err != nil { - // TODO: return error - needs rewrite interfaces - // and handle error on the caller side - } + s.Get(ctx, pair.Key, pair.Value) } } @@ -215,11 +210,7 @@ func (s Subspace) SetParamSet(ctx sdk.Context, ps ParamSet) { // since SetStruct is meant to be used in InitGenesis // so this method will not be called frequently v := reflect.Indirect(reflect.ValueOf(pair.Value)).Interface() - err := s.Set(ctx, pair.Key, v) - if err != nil { - // TODO: return error - needs rewrite interfaces - // and handle error on the caller side - } + s.Set(ctx, pair.Key, v) } } @@ -234,8 +225,8 @@ type ReadOnlySubspace struct { } // Exposes Get -func (ros ReadOnlySubspace) Get(ctx sdk.Context, key []byte, ptr interface{}) error { - return ros.s.Get(ctx, key, ptr) +func (ros ReadOnlySubspace) Get(ctx sdk.Context, key []byte, ptr interface{}) { + ros.s.Get(ctx, key, ptr) } // Exposes GetRaw diff --git a/x/slashing/app_test.go b/x/slashing/app_test.go index 31093fd98a..94d53f5889 100644 --- a/x/slashing/app_test.go +++ b/x/slashing/app_test.go @@ -47,12 +47,12 @@ func getMockApp(t *testing.T) (*mock.App, staking.Keeper, Keeper) { // staking endblocker func getEndBlocker(keeper staking.Keeper) sdk.EndBlocker { - return func(ctx sdk.Context, req abci.RequestEndBlock) (abci.ResponseEndBlock, error) { - validatorUpdates, tags, err := staking.EndBlocker(ctx, keeper) + return func(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock { + validatorUpdates, tags := staking.EndBlocker(ctx, keeper) return abci.ResponseEndBlock{ ValidatorUpdates: validatorUpdates, Tags: tags, - }, err + } } } diff --git a/x/slashing/keeper_test.go b/x/slashing/keeper_test.go index 240c7cb73b..7b0bd11745 100644 --- a/x/slashing/keeper_test.go +++ b/x/slashing/keeper_test.go @@ -394,8 +394,7 @@ func TestValidatorDippingInAndOut(t *testing.T) { newAmt := sdk.TokensFromTendermintPower(101) got = sh(ctx, NewTestMsgCreateValidator(addrs[1], pks[1], newAmt)) require.True(t, got.IsOK()) - validatorUpdates, _, err := staking.EndBlocker(ctx, sk) - require.NoError(t, err) + validatorUpdates, _ := staking.EndBlocker(ctx, sk) require.Equal(t, 2, len(validatorUpdates)) validator, _ := sk.GetValidator(ctx, addr) require.Equal(t, sdk.Unbonding, validator.Status) @@ -408,8 +407,7 @@ func TestValidatorDippingInAndOut(t *testing.T) { delTokens := sdk.TokensFromTendermintPower(3) got = sh(ctx, newTestMsgDelegate(sdk.AccAddress(addrs[2]), addrs[0], delTokens)) require.True(t, got.IsOK()) - validatorUpdates, _, err = staking.EndBlocker(ctx, sk) - require.NoError(t, err) + validatorUpdates, _ = staking.EndBlocker(ctx, sk) require.Equal(t, 2, len(validatorUpdates)) validator, _ = sk.GetValidator(ctx, addr) require.Equal(t, sdk.Bonded, validator.Status) diff --git a/x/slashing/params.go b/x/slashing/params.go index 5dd4ce8346..5a4f18f167 100644 --- a/x/slashing/params.go +++ b/x/slashing/params.go @@ -88,35 +88,20 @@ func DefaultParams() Params { // MaxEvidenceAge - max age for evidence func (k Keeper) MaxEvidenceAge(ctx sdk.Context) (res time.Duration) { - err := k.paramspace.Get(ctx, KeyMaxEvidenceAge, &res) - if err != nil { - // TODO: return error - needs rewrite interfaces - // and handle error on the caller side - // check PR #3782 - } + k.paramspace.Get(ctx, KeyMaxEvidenceAge, &res) return } // SignedBlocksWindow - sliding window for downtime slashing func (k Keeper) SignedBlocksWindow(ctx sdk.Context) (res int64) { - err := k.paramspace.Get(ctx, KeySignedBlocksWindow, &res) - if err != nil { - // TODO: return error - needs rewrite interfaces - // and handle error on the caller side - // check PR #3782 - } + k.paramspace.Get(ctx, KeySignedBlocksWindow, &res) return } // Downtime slashing threshold func (k Keeper) MinSignedPerWindow(ctx sdk.Context) int64 { var minSignedPerWindow sdk.Dec - err := k.paramspace.Get(ctx, KeyMinSignedPerWindow, &minSignedPerWindow) - if err != nil { - // TODO: return error - needs rewrite interfaces - // and handle error on the caller side - // check PR #3782 - } + k.paramspace.Get(ctx, KeyMinSignedPerWindow, &minSignedPerWindow) signedBlocksWindow := k.SignedBlocksWindow(ctx) // NOTE: RoundInt64 will never panic as minSignedPerWindow is @@ -126,34 +111,19 @@ func (k Keeper) MinSignedPerWindow(ctx sdk.Context) int64 { // Downtime unbond duration func (k Keeper) DowntimeJailDuration(ctx sdk.Context) (res time.Duration) { - err := k.paramspace.Get(ctx, KeyDowntimeJailDuration, &res) - if err != nil { - // TODO: return error - needs rewrite interfaces - // and handle error on the caller side - // check PR #3782 - } + k.paramspace.Get(ctx, KeyDowntimeJailDuration, &res) return } // SlashFractionDoubleSign func (k Keeper) SlashFractionDoubleSign(ctx sdk.Context) (res sdk.Dec) { - err := k.paramspace.Get(ctx, KeySlashFractionDoubleSign, &res) - if err != nil { - // TODO: return error - needs rewrite interfaces - // and handle error on the caller side - // check PR #3782 - } + k.paramspace.Get(ctx, KeySlashFractionDoubleSign, &res) return } // SlashFractionDowntime func (k Keeper) SlashFractionDowntime(ctx sdk.Context) (res sdk.Dec) { - err := k.paramspace.Get(ctx, KeySlashFractionDowntime, &res) - if err != nil { - // TODO: return error - needs rewrite interfaces - // and handle error on the caller side - // check PR #3782 - } + k.paramspace.Get(ctx, KeySlashFractionDowntime, &res) return } diff --git a/x/slashing/tick.go b/x/slashing/tick.go index ab1606e1a5..bd779973f1 100644 --- a/x/slashing/tick.go +++ b/x/slashing/tick.go @@ -10,7 +10,7 @@ import ( ) // slashing begin block functionality -func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, sk Keeper) (sdk.Tags, error) { +func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, sk Keeper) sdk.Tags { // Iterate over all the validators which *should* have signed this block // store whether or not they have actually signed it and slash/unbond any @@ -27,9 +27,9 @@ func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, sk Keeper) (sdk.T case tmtypes.ABCIEvidenceTypeDuplicateVote: sk.handleDoubleSign(ctx, evidence.Validator.Address, evidence.Height, evidence.Time, evidence.Validator.Power) default: - return sdk.EmptyTags(), fmt.Errorf("ignored unknown evidence type: %s", evidence.Type) + ctx.Logger().With("module", "x/slashing").Error(fmt.Sprintf("ignored unknown evidence type: %s", evidence.Type)) } } - return sdk.EmptyTags(), nil + return sdk.EmptyTags() } diff --git a/x/slashing/tick_test.go b/x/slashing/tick_test.go index 33bd0d1091..cc0e5f13b4 100644 --- a/x/slashing/tick_test.go +++ b/x/slashing/tick_test.go @@ -42,8 +42,7 @@ func TestBeginBlocker(t *testing.T) { }}, }, } - _, err := BeginBlocker(ctx, req, keeper) - require.NoError(t, err) + BeginBlocker(ctx, req, keeper) info, found := keeper.getValidatorSigningInfo(ctx, sdk.ConsAddress(pk.Address())) require.True(t, found) @@ -65,8 +64,7 @@ func TestBeginBlocker(t *testing.T) { }}, }, } - _, err := BeginBlocker(ctx, req, keeper) - require.NoError(t, err) + BeginBlocker(ctx, req, keeper) } // for 500 blocks, mark the validator as having not signed @@ -80,8 +78,7 @@ func TestBeginBlocker(t *testing.T) { }}, }, } - _, err := BeginBlocker(ctx, req, keeper) - require.NoError(t, err) + BeginBlocker(ctx, req, keeper) } // end block diff --git a/x/staking/app_test.go b/x/staking/app_test.go index 0d5de7c5e7..da8c52ea5b 100644 --- a/x/staking/app_test.go +++ b/x/staking/app_test.go @@ -34,13 +34,13 @@ func getMockApp(t *testing.T) (*mock.App, Keeper) { // getEndBlocker returns a staking endblocker. func getEndBlocker(keeper Keeper) sdk.EndBlocker { - return func(ctx sdk.Context, req abci.RequestEndBlock) (abci.ResponseEndBlock, error) { - validatorUpdates, tags, err := EndBlocker(ctx, keeper) + return func(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock { + validatorUpdates, tags := EndBlocker(ctx, keeper) return abci.ResponseEndBlock{ ValidatorUpdates: validatorUpdates, Tags: tags, - }, err + } } } diff --git a/x/staking/handler.go b/x/staking/handler.go index dc60b1bb84..7da83cec2e 100644 --- a/x/staking/handler.go +++ b/x/staking/handler.go @@ -34,7 +34,7 @@ func NewHandler(k keeper.Keeper) sdk.Handler { } // Called every block, update validator set -func EndBlocker(ctx sdk.Context, k keeper.Keeper) ([]abci.ValidatorUpdate, sdk.Tags, error) { +func EndBlocker(ctx sdk.Context, k keeper.Keeper) ([]abci.ValidatorUpdate, sdk.Tags) { resTags := sdk.NewTags() // Calculate validator set changes. @@ -83,7 +83,7 @@ func EndBlocker(ctx sdk.Context, k keeper.Keeper) ([]abci.ValidatorUpdate, sdk.T )) } - return validatorUpdates, resTags, nil + return validatorUpdates, resTags } // These functions assume everything has been authenticated, diff --git a/x/staking/keeper/params.go b/x/staking/keeper/params.go index 27f7774193..dede56b2c1 100644 --- a/x/staking/keeper/params.go +++ b/x/staking/keeper/params.go @@ -20,42 +20,26 @@ func ParamKeyTable() params.KeyTable { // UnbondingTime func (k Keeper) UnbondingTime(ctx sdk.Context) (res time.Duration) { - if err := k.paramstore.Get(ctx, types.KeyUnbondingTime, &res); err != nil { - // TODO: return error - needs rewrite interfaces - // and handle error on the caller side - // check PR #3782 - } + k.paramstore.Get(ctx, types.KeyUnbondingTime, &res) return } // MaxValidators - Maximum number of validators func (k Keeper) MaxValidators(ctx sdk.Context) (res uint16) { - if err := k.paramstore.Get(ctx, types.KeyMaxValidators, &res); err != nil { - // TODO: return error - needs rewrite interfaces - // and handle error on the caller side - // check PR #3782 - } + k.paramstore.Get(ctx, types.KeyMaxValidators, &res) return } // MaxEntries - Maximum number of simultaneous unbonding // delegations or redelegations (per pair/trio) func (k Keeper) MaxEntries(ctx sdk.Context) (res uint16) { - if err := k.paramstore.Get(ctx, types.KeyMaxEntries, &res); err != nil { - // TODO: return error - needs rewrite interfaces - // and handle error on the caller side - // check PR #3782 - } + k.paramstore.Get(ctx, types.KeyMaxEntries, &res) return } // BondDenom - Bondable coin denomination func (k Keeper) BondDenom(ctx sdk.Context) (res string) { - if err := k.paramstore.Get(ctx, types.KeyBondDenom, &res); err != nil { - // TODO: return error - needs rewrite interfaces - // and handle error on the caller side - // check PR #3782 - } + k.paramstore.Get(ctx, types.KeyBondDenom, &res) return }