diff --git a/CHANGELOG.md b/CHANGELOG.md index c9577206d408..9b7495a0f55e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,11 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* (x/bank) [#18636](https://github.com/cosmos/cosmos-sdk/pull/18636) `SendCoinsFromModuleToAccount`, `SendCoinsFromModuleToModule`, `SendCoinsFromAccountToModule`, `DelegateCoinsFromAccountToModule`, `UndelegateCoinsFromModuleToAccount`, `MintCoins` and `BurnCoins` methods now returns an error instead of panicking if any module accounts does not exist or unauthorized. +* (x/distribution) [#18636](https://github.com/cosmos/cosmos-sdk/pull/18636) `CalculateDelegationRewards` and `DelegationTotalRewards` methods no longer panics on any sanity checks and instead returns appropriate errors. +* (x/slashing) [#18636](https://github.com/cosmos/cosmos-sdk/pull/18636) `JailUntil` and `Tombstone` methods no longer panics if the signing info does not exist for the validator but instead returns error. +* (x/staking) [#18636](https://github.com/cosmos/cosmos-sdk/pull/18636) `IterateBondedValidatorsByPower`, `GetDelegatorBonded`, `Delegate`, `Unbond`, `Slash`, `Jail`, `SlashRedelegation`, `ApplyAndReturnValidatorSetUpdates` methods no longer panics on any kind of errors but instead returns appropriate errors. + * Usage of `Must...` kind of functions are avoided in keeper methods. * (client/keys) [#18687](https://github.com/cosmos/cosmos-sdk/pull/18687) Improve ` keys mnemonic` by displaying mnemonic discreetly on an alternate screen and adding `--indiscreet` option to disable it. * (client/keys) [#18663](https://github.com/cosmos/cosmos-sdk/pull/18663) Improve ` keys add` by displaying mnemonic discreetly on an alternate screen and adding `--indiscreet` option to disable it. * (types) [#18440](https://github.com/cosmos/cosmos-sdk/pull/18440) Add `AmountOfNoValidation` to `sdk.DecCoins`. diff --git a/x/bank/keeper/keeper.go b/x/bank/keeper/keeper.go index 5ce4c51fb0e4..129c8ce4a839 100644 --- a/x/bank/keeper/keeper.go +++ b/x/bank/keeper/keeper.go @@ -258,14 +258,14 @@ func (k BaseKeeper) SetDenomMetaData(ctx context.Context, denomMetaData types.Me } // SendCoinsFromModuleToAccount transfers coins from a ModuleAccount to an AccAddress. -// It will panic if the module account does not exist. An error is returned if +// An error is returned if the module account does not exist or if // the recipient address is black-listed or if sending the tokens fails. func (k BaseKeeper) SendCoinsFromModuleToAccount( ctx context.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins, ) error { senderAddr := k.ak.GetModuleAddress(senderModule) if senderAddr == nil { - panic(errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", senderModule)) + return errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", senderModule) } if k.BlockedAddr(recipientAddr) { @@ -276,74 +276,74 @@ func (k BaseKeeper) SendCoinsFromModuleToAccount( } // SendCoinsFromModuleToModule transfers coins from a ModuleAccount to another. -// It will panic if either module account does not exist. +// An error is returned if either module accounts does not exist. func (k BaseKeeper) SendCoinsFromModuleToModule( ctx context.Context, senderModule, recipientModule string, amt sdk.Coins, ) error { senderAddr := k.ak.GetModuleAddress(senderModule) if senderAddr == nil { - panic(errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", senderModule)) + return errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", senderModule) } recipientAcc := k.ak.GetModuleAccount(ctx, recipientModule) if recipientAcc == nil { - panic(errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", recipientModule)) + return errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", recipientModule) } return k.SendCoins(ctx, senderAddr, recipientAcc.GetAddress(), amt) } // SendCoinsFromAccountToModule transfers coins from an AccAddress to a ModuleAccount. -// It will panic if the module account does not exist. +// An error is returned if the module account does not exist. func (k BaseKeeper) SendCoinsFromAccountToModule( ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins, ) error { recipientAcc := k.ak.GetModuleAccount(ctx, recipientModule) if recipientAcc == nil { - panic(errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", recipientModule)) + return errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", recipientModule) } return k.SendCoins(ctx, senderAddr, recipientAcc.GetAddress(), amt) } // DelegateCoinsFromAccountToModule delegates coins and transfers them from a -// delegator account to a module account. It will panic if the module account +// delegator account to a module account. An error is returned if the module account // does not exist or is unauthorized. func (k BaseKeeper) DelegateCoinsFromAccountToModule( ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins, ) error { recipientAcc := k.ak.GetModuleAccount(ctx, recipientModule) if recipientAcc == nil { - panic(errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", recipientModule)) + return errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", recipientModule) } if !recipientAcc.HasPermission(authtypes.Staking) { - panic(errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "module account %s does not have permissions to receive delegated coins", recipientModule)) + return errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "module account %s does not have permissions to receive delegated coins", recipientModule) } return k.DelegateCoins(ctx, senderAddr, recipientAcc.GetAddress(), amt) } // UndelegateCoinsFromModuleToAccount undelegates the unbonding coins and transfers -// them from a module account to the delegator account. It will panic if the +// them from a module account to the delegator account. An error is returned if the // module account does not exist or is unauthorized. func (k BaseKeeper) UndelegateCoinsFromModuleToAccount( ctx context.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins, ) error { acc := k.ak.GetModuleAccount(ctx, senderModule) if acc == nil { - panic(errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", senderModule)) + return errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", senderModule) } if !acc.HasPermission(authtypes.Staking) { - panic(errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "module account %s does not have permissions to undelegate coins", senderModule)) + return errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "module account %s does not have permissions to undelegate coins", senderModule) } return k.UndelegateCoins(ctx, acc.GetAddress(), recipientAddr, amt) } // MintCoins creates new coins from thin air and adds it to the module account. -// It will panic if the module account does not exist or is unauthorized. +// An error is returned if the module account does not exist or is unauthorized. func (k BaseKeeper) MintCoins(ctx context.Context, moduleName string, amounts sdk.Coins) error { sdkCtx := sdk.UnwrapSDKContext(ctx) @@ -354,11 +354,11 @@ func (k BaseKeeper) MintCoins(ctx context.Context, moduleName string, amounts sd } acc := k.ak.GetModuleAccount(ctx, moduleName) if acc == nil { - panic(errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", moduleName)) + return errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", moduleName) } if !acc.HasPermission(authtypes.Minter) { - panic(errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "module account %s does not have permissions to mint tokens", moduleName)) + return errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "module account %s does not have permissions to mint tokens", moduleName) } err = k.addCoins(ctx, acc.GetAddress(), amounts) @@ -387,7 +387,7 @@ func (k BaseKeeper) MintCoins(ctx context.Context, moduleName string, amounts sd } // BurnCoins burns coins deletes coins from the balance of the module account. -// It will panic if the module account does not exist or is unauthorized. +// An error is returned if the module account does not exist or is unauthorized. func (k BaseKeeper) BurnCoins(ctx context.Context, address []byte, amounts sdk.Coins) error { acc := k.ak.GetAccount(ctx, address) if acc == nil { diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index 5a25feb7d26c..4aa3b503f08e 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -396,20 +396,17 @@ func (suite *KeeperTestSuite) TestSupply_DelegateUndelegateCoins() { require.NoError(keeper.SendCoinsFromModuleToAccount(ctx, banktypes.MintModuleName, holderAcc.GetAddress(), initCoins)) authKeeper.EXPECT().GetModuleAddress("").Return(nil) - require.Panics(func() { - _ = keeper.SendCoinsFromModuleToAccount(ctx, "", holderAcc.GetAddress(), initCoins) - }) + err := keeper.SendCoinsFromModuleToAccount(ctx, "", holderAcc.GetAddress(), initCoins) + require.Error(err) authKeeper.EXPECT().GetModuleAddress(burnerAcc.Name).Return(burnerAcc.GetAddress()) authKeeper.EXPECT().GetModuleAccount(ctx, "").Return(nil) - require.Panics(func() { - _ = keeper.SendCoinsFromModuleToModule(ctx, authtypes.Burner, "", initCoins) - }) + err = keeper.SendCoinsFromModuleToModule(ctx, authtypes.Burner, "", initCoins) + require.Error(err) authKeeper.EXPECT().GetModuleAddress("").Return(nil) - require.Panics(func() { - _ = keeper.SendCoinsFromModuleToAccount(ctx, "", baseAcc.GetAddress(), initCoins) - }) + err = keeper.SendCoinsFromModuleToAccount(ctx, "", baseAcc.GetAddress(), initCoins) + require.Error(err) authKeeper.EXPECT().GetModuleAddress(holderAcc.Name).Return(holderAcc.GetAddress()) authKeeper.EXPECT().GetAccount(suite.ctx, holderAcc.GetAddress()).Return(holderAcc) @@ -456,20 +453,17 @@ func (suite *KeeperTestSuite) TestSupply_SendCoins() { require.NoError(keeper.SendCoinsFromModuleToAccount(ctx, banktypes.MintModuleName, holderAcc.GetAddress(), initCoins)) authKeeper.EXPECT().GetModuleAddress("").Return(nil) - require.Panics(func() { - _ = keeper.SendCoinsFromModuleToModule(ctx, "", holderAcc.GetName(), initCoins) - }) + err := keeper.SendCoinsFromModuleToModule(ctx, "", holderAcc.GetName(), initCoins) + require.Error(err) authKeeper.EXPECT().GetModuleAddress(burnerAcc.Name).Return(burnerAcc.GetAddress()) authKeeper.EXPECT().GetModuleAccount(ctx, "").Return(nil) - require.Panics(func() { - _ = keeper.SendCoinsFromModuleToModule(ctx, authtypes.Burner, "", initCoins) - }) + err = keeper.SendCoinsFromModuleToModule(ctx, authtypes.Burner, "", initCoins) + require.Error(err) authKeeper.EXPECT().GetModuleAddress("").Return(nil) - require.Panics(func() { - _ = keeper.SendCoinsFromModuleToAccount(ctx, "", baseAcc.GetAddress(), initCoins) - }) + err = keeper.SendCoinsFromModuleToAccount(ctx, "", baseAcc.GetAddress(), initCoins) + require.Error(err) authKeeper.EXPECT().GetModuleAddress(holderAcc.Name).Return(holderAcc.GetAddress()) authKeeper.EXPECT().GetAccount(suite.ctx, holderAcc.GetAddress()).Return(holderAcc) @@ -508,16 +502,21 @@ func (suite *KeeperTestSuite) TestSupply_MintCoins() { require.NoError(err) authKeeper.EXPECT().GetModuleAccount(ctx, "").Return(nil) - require.Panics(func() { _ = keeper.MintCoins(ctx, "", initCoins) }, "no module account") + err = keeper.MintCoins(ctx, "", initCoins) + require.Error(err) + require.ErrorContains(err, "module account does not exist") suite.mockMintCoins(burnerAcc) - require.Panics(func() { _ = keeper.MintCoins(ctx, authtypes.Burner, initCoins) }, "invalid permission") + err = keeper.MintCoins(ctx, authtypes.Burner, initCoins) + require.Error(err) + require.ErrorContains(err, fmt.Sprintf("module account %s does not have permissions to mint tokens: unauthorized", authtypes.Burner)) suite.mockMintCoins(minterAcc) require.Error(keeper.MintCoins(ctx, authtypes.Minter, sdk.Coins{sdk.Coin{Denom: "denom", Amount: math.NewInt(-10)}}), "insufficient coins") authKeeper.EXPECT().GetModuleAccount(ctx, randomPerm).Return(nil) - require.Panics(func() { _ = keeper.MintCoins(ctx, randomPerm, initCoins) }) + err = keeper.MintCoins(ctx, randomPerm, initCoins) + require.Error(err) suite.mockMintCoins(minterAcc) require.NoError(keeper.MintCoins(ctx, authtypes.Minter, initCoins)) diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index 8b9008a1e19f..36482b407b68 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -51,17 +51,17 @@ func (k Keeper) calculateDelegationRewardsBetween(ctx context.Context, val sdk.V ) (sdk.DecCoins, error) { // sanity check if startingPeriod > endingPeriod { - panic("startingPeriod cannot be greater than endingPeriod") + return sdk.DecCoins{}, fmt.Errorf("startingPeriod cannot be greater than endingPeriod") } // sanity check if stake.IsNegative() { - panic("stake should not be negative") + return sdk.DecCoins{}, fmt.Errorf("stake should not be negative") } valBz, err := k.stakingKeeper.ValidatorAddressCodec().StringToBytes(val.GetOperator()) if err != nil { - panic(err) + return sdk.DecCoins{}, err } // return staking * (ending - starting) @@ -77,7 +77,7 @@ func (k Keeper) calculateDelegationRewardsBetween(ctx context.Context, val sdk.V difference := ending.CumulativeRewardRatio.Sub(starting.CumulativeRewardRatio) if difference.IsAnyNegative() { - panic("negative rewards should not be possible") + return sdk.DecCoins{}, fmt.Errorf("negative rewards should not be possible") } // note: necessary to truncate so we don't allow withdrawing more rewards than owed rewards := difference.MulDecTruncate(stake) @@ -123,6 +123,7 @@ func (k Keeper) CalculateDelegationRewards(ctx context.Context, val sdk.Validato // Slashes this block happened after reward allocation, but we have to account // for them for the stake sanity check below. endingHeight := uint64(sdkCtx.BlockHeight()) + var iterErr error if endingHeight > startingHeight { err = k.IterateValidatorSlashEventsBetween(ctx, valAddr, startingHeight, endingHeight, func(height uint64, event types.ValidatorSlashEvent) (stop bool) { @@ -130,7 +131,8 @@ func (k Keeper) CalculateDelegationRewards(ctx context.Context, val sdk.Validato if endingPeriod > startingPeriod { delRewards, err := k.calculateDelegationRewardsBetween(ctx, val, startingPeriod, endingPeriod, stake) if err != nil { - panic(err) + iterErr = err + return true } rewards = rewards.Add(delRewards...) @@ -142,6 +144,9 @@ func (k Keeper) CalculateDelegationRewards(ctx context.Context, val sdk.Validato return false }, ) + if iterErr != nil { + return sdk.DecCoins{}, iterErr + } if err != nil { return sdk.DecCoins{}, err } @@ -178,10 +183,10 @@ func (k Keeper) CalculateDelegationRewards(ctx context.Context, val sdk.Validato if stake.LTE(currentStake.Add(marginOfErr)) { stake = currentStake } else { - panic(fmt.Sprintf("calculated final stake for delegator %s greater than current stake"+ + return sdk.DecCoins{}, fmt.Errorf("calculated final stake for delegator %s greater than current stake"+ "\n\tfinal stake:\t%s"+ "\n\tcurrent stake:\t%s", - del.GetDelegatorAddr(), stake, currentStake)) + del.GetDelegatorAddr(), stake, currentStake) } } diff --git a/x/distribution/keeper/grpc_query.go b/x/distribution/keeper/grpc_query.go index 48a66ba21fdc..16039f340f0f 100644 --- a/x/distribution/keeper/grpc_query.go +++ b/x/distribution/keeper/grpc_query.go @@ -264,27 +264,32 @@ func (k Querier) DelegationTotalRewards(ctx context.Context, req *types.QueryDel return nil, err } + var iterErr error err = k.stakingKeeper.IterateDelegations( ctx, delAdr, func(_ int64, del sdk.DelegationI) (stop bool) { valAddr, err := k.stakingKeeper.ValidatorAddressCodec().StringToBytes(del.GetValidatorAddr()) if err != nil { - panic(err) + iterErr = err + return true } val, err := k.stakingKeeper.Validator(ctx, valAddr) if err != nil { - panic(err) + iterErr = err + return true } endingPeriod, err := k.IncrementValidatorPeriod(ctx, val) if err != nil { - panic(err) + iterErr = err + return true } delReward, err := k.CalculateDelegationRewards(ctx, val, del, endingPeriod) if err != nil { - panic(err) + iterErr = err + return true } delRewards = append(delRewards, types.NewDelegationDelegatorReward(del.GetValidatorAddr(), delReward)) @@ -292,6 +297,9 @@ func (k Querier) DelegationTotalRewards(ctx context.Context, req *types.QueryDel return false }, ) + if iterErr != nil { + return nil, iterErr + } if err != nil { return nil, err } diff --git a/x/gov/keeper/tally_test.go b/x/gov/keeper/tally_test.go index 3061d9f5f354..5db9a2db0fe7 100644 --- a/x/gov/keeper/tally_test.go +++ b/x/gov/keeper/tally_test.go @@ -430,6 +430,7 @@ func TestTally(t *testing.T) { } return nil }) + // Submit and activate a proposal proposal, err := govKeeper.SubmitProposal(ctx, TestProposal, "", "title", "summary", delAddrs[0], tt.proposalType) require.NoError(t, err) diff --git a/x/slashing/keeper/signing_info.go b/x/slashing/keeper/signing_info.go index 80ebc6550305..51e258fea960 100644 --- a/x/slashing/keeper/signing_info.go +++ b/x/slashing/keeper/signing_info.go @@ -3,6 +3,7 @@ package keeper import ( "context" "errors" + "fmt" "time" "github.com/bits-and-blooms/bitset" @@ -22,23 +23,22 @@ func (k Keeper) HasValidatorSigningInfo(ctx context.Context, consAddr sdk.ConsAd } // JailUntil attempts to set a validator's JailedUntil attribute in its signing -// info. It will panic if the signing info does not exist for the validator. +// info. func (k Keeper) JailUntil(ctx context.Context, consAddr sdk.ConsAddress, jailTime time.Time) error { signInfo, err := k.ValidatorSigningInfo.Get(ctx, consAddr) if err != nil { - return errorsmod.Wrap(err, "cannot jail validator that does not have any signing information") + return errorsmod.Wrap(err, fmt.Sprintf("cannot jail validator with consensus address %s that does not have any signing information", consAddr.String())) } signInfo.JailedUntil = jailTime return k.ValidatorSigningInfo.Set(ctx, consAddr, signInfo) } -// Tombstone attempts to tombstone a validator. It will panic if signing info for -// the given validator does not exist. +// Tombstone attempts to tombstone a validator. func (k Keeper) Tombstone(ctx context.Context, consAddr sdk.ConsAddress) error { signInfo, err := k.ValidatorSigningInfo.Get(ctx, consAddr) if err != nil { - return types.ErrNoSigningInfoFound.Wrap("cannot tombstone validator that does not have any signing information") + return types.ErrNoSigningInfoFound.Wrap(fmt.Sprintf("cannot tombstone validator with consensus address %s that does not have any signing information", consAddr.String())) } if signInfo.Tombstoned { diff --git a/x/staking/keeper/alias_functions.go b/x/staking/keeper/alias_functions.go index 671bc8a7323c..fde1c5254e1e 100644 --- a/x/staking/keeper/alias_functions.go +++ b/x/staking/keeper/alias_functions.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "fmt" "cosmossdk.io/collections" storetypes "cosmossdk.io/store/types" @@ -56,8 +57,10 @@ func (k Keeper) IterateBondedValidatorsByPower(ctx context.Context, fn func(inde i := int64(0) for ; iterator.Valid() && i < int64(maxValidators); iterator.Next() { address := iterator.Value() - validator := k.mustGetValidator(ctx, address) - + validator, err := k.GetValidator(ctx, address) + if err != nil { + return fmt.Errorf("validator record not found for address: %s", sdk.ValAddress(address).String()) + } if validator.IsBonded() { stop := fn(i, validator) // XXX is this safe will the validator unexposed fields be able to get written to? if stop { diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index cab69544e827..651cc6310b62 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -216,10 +216,12 @@ func (k Keeper) GetDelegatorUnbonding(ctx context.Context, delegator sdk.AccAddr func (k Keeper) GetDelegatorBonded(ctx context.Context, delegator sdk.AccAddress) (math.Int, error) { bonded := math.LegacyZeroDec() + var iterErr error err := k.IterateDelegatorDelegations(ctx, delegator, func(delegation types.Delegation) bool { validatorAddr, err := k.validatorAddressCodec.StringToBytes(delegation.ValidatorAddress) if err != nil { - panic(err) // shouldn't happen + iterErr = err + return true } validator, err := k.GetValidator(ctx, validatorAddr) if err == nil { @@ -229,6 +231,9 @@ func (k Keeper) GetDelegatorBonded(ctx context.Context, delegator sdk.AccAddress } return false }) + if iterErr != nil { + return bonded.RoundInt(), iterErr + } return bonded.RoundInt(), err } @@ -717,7 +722,7 @@ func (k Keeper) Delegate( // all non bonded if subtractAccount { if tokenSrc == types.Bonded { - panic("delegation token source cannot be bonded") + return math.LegacyZeroDec(), fmt.Errorf("delegation token source cannot be bonded; expected Unbonded or Unbonding, got Bonded") } var sendName string @@ -728,7 +733,7 @@ func (k Keeper) Delegate( case validator.IsUnbonding(), validator.IsUnbonded(): sendName = types.NotBondedPoolName default: - panic("invalid validator status") + return math.LegacyZeroDec(), fmt.Errorf("invalid validator status: %v", validator.Status) } bondDenom, err := k.BondDenom(ctx) @@ -760,7 +765,7 @@ func (k Keeper) Delegate( return math.LegacyDec{}, err } default: - panic("unknown token source bond status") + return math.LegacyZeroDec(), fmt.Errorf("unknown token source bond status: %v", tokenSrc) } } @@ -832,9 +837,12 @@ func (k Keeper) Unbond( validator.TokensFromShares(delegation.Shares).TruncateInt().LT(validator.MinSelfDelegation) { err = k.jailValidator(ctx, validator) if err != nil { - return amount, err + return amount, fmt.Errorf("failed to jail validator: %v", err) + } + validator, err = k.GetValidator(ctx, valbz) + if err != nil { + return amount, fmt.Errorf("validator record not found for address: %X", valbz) } - validator = k.mustGetValidator(ctx, valbz) } if delegation.Shares.IsZero() { @@ -906,7 +914,7 @@ func (k Keeper) getBeginInfo( return validator.UnbondingTime, validator.UnbondingHeight, false, nil default: - panic(fmt.Sprintf("unknown validator status: %s", validator.Status)) + return completionTime, height, false, fmt.Errorf("unknown validator status: %v", validator.Status) } } diff --git a/x/staking/keeper/keeper_test.go b/x/staking/keeper/keeper_test.go index d1bbe681b131..b13404cabc69 100644 --- a/x/staking/keeper/keeper_test.go +++ b/x/staking/keeper/keeper_test.go @@ -356,7 +356,7 @@ func (s *KeeperTestSuite) TestUnbondingDelegationsMigrationToColls() { }, }, } - bz := stakingtypes.MustMarshalUBD(s.cdc, ubd) + bz := s.cdc.MustMarshal(&ubd) s.ctx.KVStore(s.key).Set(getUBDKey(delAddrs[i], valAddrs[i]), bz) s.ctx.KVStore(s.key).Set(getUBDByValIndexKey(delAddrs[i], valAddrs[i]), []byte{}) }, @@ -444,7 +444,7 @@ func (s *KeeperTestSuite) TestValidatorsMigrationToColls() { Commission: stakingtypes.NewCommission(math.LegacyZeroDec(), math.LegacyZeroDec(), math.LegacyZeroDec()), MinSelfDelegation: math.ZeroInt(), } - valBz := stakingtypes.MustMarshalValidator(s.cdc, &val) + valBz := s.cdc.MustMarshal(&val) // legacy Set method s.ctx.KVStore(s.key).Set(getValidatorKey(valAddrs[i]), valBz) }, diff --git a/x/staking/keeper/slash.go b/x/staking/keeper/slash.go index 0d21e7404595..17d07a8da57a 100644 --- a/x/staking/keeper/slash.go +++ b/x/staking/keeper/slash.go @@ -57,7 +57,7 @@ func (k Keeper) Slash(ctx context.Context, consAddr sdk.ConsAddress, infractionH // Log the slash attempt for future reference (maybe we should tag it too) conStr, err := k.consensusAddressCodec.BytesToString(consAddr) if err != nil { - panic(err) + return math.NewInt(0), err } logger.Error( @@ -191,7 +191,7 @@ func (k Keeper) Slash(ctx context.Context, consAddr sdk.ConsAddress, infractionH return math.NewInt(0), err } default: - panic("invalid validator status") + return math.NewInt(0), fmt.Errorf("invalid validator status") } logger.Info( @@ -210,7 +210,10 @@ func (k Keeper) SlashWithInfractionReason(ctx context.Context, consAddr sdk.Cons // jail a validator func (k Keeper) Jail(ctx context.Context, consAddr sdk.ConsAddress) error { - validator := k.mustGetValidatorByConsAddr(ctx, consAddr) + validator, err := k.GetValidatorByConsAddr(ctx, consAddr) + if err != nil { + return fmt.Errorf("validator with consensus-Address %s not found", consAddr) + } if err := k.jailValidator(ctx, validator); err != nil { return err } @@ -222,7 +225,10 @@ func (k Keeper) Jail(ctx context.Context, consAddr sdk.ConsAddress) error { // unjail a validator func (k Keeper) Unjail(ctx context.Context, consAddr sdk.ConsAddress) error { - validator := k.mustGetValidatorByConsAddr(ctx, consAddr) + validator, err := k.GetValidatorByConsAddr(ctx, consAddr) + if err != nil { + return fmt.Errorf("validator with consensus-Address %s not found", consAddr) + } if err := k.unjailValidator(ctx, validator); err != nil { return err } @@ -363,7 +369,7 @@ func (k Keeper) SlashRedelegation(ctx context.Context, srcValidator types.Valida case dstValidator.IsUnbonded() || dstValidator.IsUnbonding(): notBondedBurnedAmount = notBondedBurnedAmount.Add(tokensToBurn) default: - panic("unknown validator status") + return math.ZeroInt(), fmt.Errorf("unknown validator status") } } diff --git a/x/staking/keeper/val_state_change.go b/x/staking/keeper/val_state_change.go index 73675b3d4c5c..3d09937c1ef4 100644 --- a/x/staking/keeper/val_state_change.go +++ b/x/staking/keeper/val_state_change.go @@ -156,10 +156,13 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx context.Context) (updates // everything that is iterated in this loop is becoming or already a // part of the bonded validator set valAddr := sdk.ValAddress(iterator.Value()) - validator := k.mustGetValidator(ctx, valAddr) + validator, err := k.GetValidator(ctx, valAddr) + if err != nil { + return nil, fmt.Errorf("validator record not found for address: %X", valAddr) + } if validator.Jailed { - panic("should never retrieve a jailed validator from the power store") + return nil, fmt.Errorf("should never retrieve a jailed validator from the power store") } // if we get to a zero-power validator (which we don't bond), @@ -185,7 +188,7 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx context.Context) (updates case validator.IsBonded(): // no state change default: - panic("unexpected validator status") + return nil, fmt.Errorf("unexpected validator status") } // fetch the old power bytes @@ -218,7 +221,10 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx context.Context) (updates } for _, valAddrBytes := range noLongerBonded { - validator := k.mustGetValidator(ctx, sdk.ValAddress(valAddrBytes)) + validator, err := k.GetValidator(ctx, sdk.ValAddress(valAddrBytes)) + if err != nil { + return nil, fmt.Errorf("validator record not found for address: %X", sdk.ValAddress(valAddrBytes)) + } validator, err = k.bondedToUnbonding(ctx, validator) if err != nil { return nil, err @@ -274,7 +280,7 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx context.Context) (updates func (k Keeper) bondedToUnbonding(ctx context.Context, validator types.Validator) (types.Validator, error) { if !validator.IsBonded() { - panic(fmt.Sprintf("bad state transition bondedToUnbonding, validator: %v\n", validator)) + return types.Validator{}, fmt.Errorf("bad state transition bondedToUnbonding, validator: %v", validator) } return k.BeginUnbondingValidator(ctx, validator) @@ -282,7 +288,7 @@ func (k Keeper) bondedToUnbonding(ctx context.Context, validator types.Validator func (k Keeper) unbondingToBonded(ctx context.Context, validator types.Validator) (types.Validator, error) { if !validator.IsUnbonding() { - panic(fmt.Sprintf("bad state transition unbondingToBonded, validator: %v\n", validator)) + return types.Validator{}, fmt.Errorf("bad state transition unbondingToBonded, validator: %v", validator) } return k.bondValidator(ctx, validator) @@ -290,7 +296,7 @@ func (k Keeper) unbondingToBonded(ctx context.Context, validator types.Validator func (k Keeper) unbondedToBonded(ctx context.Context, validator types.Validator) (types.Validator, error) { if !validator.IsUnbonded() { - panic(fmt.Sprintf("bad state transition unbondedToBonded, validator: %v\n", validator)) + return types.Validator{}, fmt.Errorf("bad state transition unbondedToBonded, validator: %v", validator) } return k.bondValidator(ctx, validator) @@ -388,7 +394,7 @@ func (k Keeper) BeginUnbondingValidator(ctx context.Context, validator types.Val // sanity check if validator.Status != types.Bonded { - panic(fmt.Sprintf("should not already be unbonded or unbonding, validator: %v\n", validator)) + return validator, fmt.Errorf("should not already be unbonded or unbonding, validator: %v", validator) } id, err := k.IncrementUnbondingID(ctx) diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go index 6b6eef790975..b95dd87a7742 100644 --- a/x/staking/keeper/validator.go +++ b/x/staking/keeper/validator.go @@ -34,15 +34,6 @@ func (k Keeper) GetValidator(ctx context.Context, addr sdk.ValAddress) (validato return validator, nil } -func (k Keeper) mustGetValidator(ctx context.Context, addr sdk.ValAddress) types.Validator { - validator, err := k.GetValidator(ctx, addr) - if err != nil { - panic(fmt.Sprintf("validator record not found for address: %X\n", addr)) - } - - return validator -} - // GetValidatorByConsAddr gets a single validator by consensus address func (k Keeper) GetValidatorByConsAddr(ctx context.Context, consAddr sdk.ConsAddress) (validator types.Validator, err error) { opAddr, err := k.ValidatorByConsensusAddress.Get(ctx, consAddr) @@ -57,15 +48,6 @@ func (k Keeper) GetValidatorByConsAddr(ctx context.Context, consAddr sdk.ConsAdd return k.GetValidator(ctx, opAddr) } -func (k Keeper) mustGetValidatorByConsAddr(ctx context.Context, consAddr sdk.ConsAddress) types.Validator { - validator, err := k.GetValidatorByConsAddr(ctx, consAddr) - if err != nil { - panic(fmt.Errorf("validator with consensus-Address %s not found", consAddr)) - } - - return validator -} - // SetValidator sets the main record holding validator details func (k Keeper) SetValidator(ctx context.Context, validator types.Validator) error { valBz, err := k.ValidatorAddressCodec().StringToBytes(validator.GetOperator()) @@ -317,7 +299,10 @@ func (k Keeper) GetBondedValidatorsByPower(ctx context.Context) ([]types.Validat i := 0 for ; iterator.Valid() && i < int(maxValidators); iterator.Next() { address := iterator.Value() - validator := k.mustGetValidator(ctx, address) + validator, err := k.GetValidator(ctx, address) + if err != nil { + return nil, fmt.Errorf("validator record not found for address: %X", address) + } if validator.IsBonded() { validators[i] = validator @@ -383,7 +368,7 @@ func (k Keeper) GetLastValidators(ctx context.Context) (validators []types.Valid err = k.LastValidatorPower.Walk(ctx, nil, func(key []byte, _ gogotypes.Int64Value) (bool, error) { // sanity check if i >= int(maxValidators) { - panic("more validators than maxValidators found") + return true, fmt.Errorf("more validators than maxValidators found") } validator, err := k.GetValidator(ctx, key)