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

Return errors instead of panic #3782

Merged
merged 43 commits into from
Apr 4, 2019
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
8be88f7
Handle error insted of panic
MarinX Mar 1, 2019
6e474a9
Handle error comming for the store
MarinX Mar 1, 2019
d2d398f
Handle error on cli level
MarinX Mar 1, 2019
fd0918a
Modify test - catch error insted of panic
MarinX Mar 1, 2019
89620a9
Change types
MarinX Mar 2, 2019
bfb0be8
Begin/End Blocker error return
MarinX Mar 2, 2019
15e4ca2
Fix cli app
MarinX Mar 2, 2019
cfe1879
Updated PENDING.md
MarinX Mar 2, 2019
3fb8a02
Fix conflict
MarinX Mar 16, 2019
2a1553f
Updated PENDING.md
MarinX Mar 2, 2019
922aa1d
Fix conflict
MarinX Mar 16, 2019
c9a4d27
check for error on error returning funcs
MarinX Mar 27, 2019
4f8510f
Updated PENDING.md
MarinX Mar 2, 2019
b8d5efa
Fix conflict
MarinX Mar 16, 2019
31ed2e1
Updated PENDING.md
MarinX Mar 2, 2019
61cebb5
Fix conflict
MarinX Mar 16, 2019
2b693a1
Updated PENDING.md
MarinX Mar 2, 2019
98fc689
Fix conflict
MarinX Mar 16, 2019
b1b3d6d
Updated PENDING.md
MarinX Mar 2, 2019
8176159
Fix conflict
MarinX Mar 16, 2019
c5c1d7c
Updated PENDING.md
MarinX Mar 2, 2019
f8d123d
Fix conflict
MarinX Mar 16, 2019
9dd9cd8
Updated PENDING.md
MarinX Mar 2, 2019
7762f21
Fix conflict
MarinX Mar 16, 2019
37874b0
Updated PENDING.md
MarinX Mar 2, 2019
25c118c
Fix conflict
MarinX Mar 16, 2019
c37cbb4
Updated PENDING.md
MarinX Mar 2, 2019
a4abe1d
Fix conflict
MarinX Mar 16, 2019
96242e9
Updated PENDING.md
MarinX Mar 2, 2019
d4d2a03
Fix conflict
MarinX Mar 16, 2019
15721b8
Updated PENDING.md
MarinX Mar 2, 2019
21c7c39
Fix conflict
MarinX Mar 16, 2019
37f50ea
Updated PENDING.md
MarinX Mar 2, 2019
fe85e02
Fix conflict
MarinX Mar 16, 2019
61d28c4
Updated PENDING.md
MarinX Mar 2, 2019
9a6c410
Fix conflict
MarinX Mar 16, 2019
4e34bad
Return EmptyTags instead of nil
MarinX Mar 29, 2019
5641573
Use named return params
MarinX Mar 29, 2019
c02021f
Update comment with PR number
MarinX Mar 29, 2019
2a55045
Merge branch 'develop' of https://github.com/cosmos/cosmos-sdk into d…
MarinX Apr 3, 2019
0a2ac3a
Add changelog for breaking sdk change
MarinX Apr 3, 2019
00d4bf8
Fix assignment mismatch with tests
MarinX Apr 3, 2019
33ae73c
Fix lint errors
MarinX Apr 3, 2019
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
12 changes: 10 additions & 2 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
33 changes: 21 additions & 12 deletions cmd/gaia/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,30 +222,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 {
Expand All @@ -255,7 +264,7 @@ func (app *GaiaApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.R
return abci.ResponseEndBlock{
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there a NewResponseEndBlock function on the abci pkg we could use to replace this everywhere?

ValidatorUpdates: validatorUpdates,
Tags: tags,
}
}, nil
}

// initialize store from a genesis state
Expand Down
6 changes: 5 additions & 1 deletion cmd/gaia/app/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
18 changes: 9 additions & 9 deletions cmd/gaia/cmd/gaiadebug/hack.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions types/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 6 additions & 1 deletion x/bank/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
MarinX marked this conversation as resolved.
Show resolved Hide resolved
// and handle error on the caller side
// check PR #3782
}
}

var _ ViewKeeper = (*BaseViewKeeper)(nil)
Expand Down
4 changes: 2 additions & 2 deletions x/distribution/abci_app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
4 changes: 2 additions & 2 deletions x/gov/endblocker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -78,5 +78,5 @@ func EndBlocker(ctx sdk.Context, keeper Keeper) sdk.Tags {
resTags = resTags.AppendTag(tags.ProposalResult, tagValue)
}

return resTags
return resTags, nil
}
42 changes: 36 additions & 6 deletions x/gov/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
MarinX marked this conversation as resolved.
Show resolved Hide resolved
// 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
MarinX marked this conversation as resolved.
Show resolved Hide resolved
// 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
MarinX marked this conversation as resolved.
Show resolved Hide resolved
// 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
MarinX marked this conversation as resolved.
Show resolved Hide resolved
// 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
MarinX marked this conversation as resolved.
Show resolved Hide resolved
// 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
MarinX marked this conversation as resolved.
Show resolved Hide resolved
// and handle error on the caller side
// check PR #3782
}
}

// Votes
Expand Down
6 changes: 3 additions & 3 deletions x/gov/test_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
13 changes: 10 additions & 3 deletions x/mint/abci_app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
MarinX marked this conversation as resolved.
Show resolved Hide resolved

// 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)
Expand All @@ -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

}
24 changes: 17 additions & 7 deletions x/mint/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
MarinX marked this conversation as resolved.
Show resolved Hide resolved
// 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
Expand Down
17 changes: 10 additions & 7 deletions x/mint/keeper.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package mint

import (
"errors"

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/params"
Expand Down Expand Up @@ -56,11 +58,12 @@ const (
//______________________________________________________________________

// get the minter
func (k Keeper) GetMinter(ctx sdk.Context) (minter Minter) {
func (k Keeper) GetMinter(ctx sdk.Context) (minter Minter, err error) {
store := ctx.KVStore(k.storeKey)
b := store.Get(minterKey)
if b == nil {
panic("Stored fee pool should not have been nil")
err = errors.New("Stored fee pool should not have been nil")
return
}
k.cdc.MustUnmarshalBinaryLengthPrefixed(b, &minter)
return
Expand All @@ -76,13 +79,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 {
func (k Keeper) GetParams(ctx sdk.Context) (Params, error) {
var params Params
k.paramSpace.Get(ctx, ParamStoreKeyParams, &params)
return params
err := k.paramSpace.Get(ctx, ParamStoreKeyParams, &params)
return params, err
}

// set inflation params from the global param store
func (k Keeper) SetParams(ctx sdk.Context, params Params) {
k.paramSpace.Set(ctx, ParamStoreKeyParams, &params)
func (k Keeper) SetParams(ctx sdk.Context, params Params) error {
return k.paramSpace.Set(ctx, ParamStoreKeyParams, &params)
}
Loading