diff --git a/CHANGELOG.md b/CHANGELOG.md index 260315e044..018c749326 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* (x/staking) [#24391](https://github.com/cosmos/cosmos-sdk/pull/24391) Replace panics with error results; more verbose error messages * (x/staking) [#24354](https://github.com/cosmos/cosmos-sdk/pull/24354) Optimize validator endblock by reducing bech32 conversions, resulting in significant performance improvement * (client/keys) [#18950](https://github.com/cosmos/cosmos-sdk/pull/18950) Improve ` keys add`, ` keys import` and ` keys rename` by checking name validation. * (client/keys) [#18703](https://github.com/cosmos/cosmos-sdk/pull/18703) Improve ` keys add` and ` keys show` by checking whether there are duplicate keys in the multisig case. diff --git a/x/staking/keeper/val_state_change.go b/x/staking/keeper/val_state_change.go index ecdc9d60ff..7761d80a2e 100644 --- a/x/staking/keeper/val_state_change.go +++ b/x/staking/keeper/val_state_change.go @@ -3,6 +3,7 @@ package keeper import ( "bytes" "context" + "errors" "fmt" "sort" @@ -142,13 +143,13 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx context.Context) (updates // (see LastValidatorPowerKey). last, err := k.getLastValidatorsByAddr(ctx) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to get last validator set: %w", err) } // Iterate over validators, highest power to lowest. iterator, err := k.ValidatorsPowerStoreIterator(ctx) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to get validators power store iterator: %w", err) } defer iterator.Close() @@ -156,10 +157,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, errors.New("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 +189,7 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx context.Context) (updates case validator.IsBonded(): // no state change default: - panic("unexpected validator status") + return nil, errors.New("unexpected validator status") } valAddrStr := string(valAddr) @@ -214,14 +218,17 @@ 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 } str, err := k.validatorAddressCodec.StringToBytes(validator.GetOperator()) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to get validator operator address: %w", err) } amtFromBondedToNotBonded = amtFromBondedToNotBonded.Add(validator.GetTokens()) if err = k.DeleteLastValidatorPower(ctx, str); err != nil { @@ -269,7 +276,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) @@ -277,7 +284,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) @@ -285,7 +292,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) @@ -332,63 +339,63 @@ func (k Keeper) unjailValidator(ctx context.Context, validator types.Validator) func (k Keeper) bondValidator(ctx context.Context, validator types.Validator) (types.Validator, error) { // delete the validator by power index, as the key will change if err := k.DeleteValidatorByPowerIndex(ctx, validator); err != nil { - return validator, err + return types.Validator{}, err } validator = validator.UpdateStatus(types.Bonded) // save the now bonded validator record to the two referenced stores if err := k.SetValidator(ctx, validator); err != nil { - return validator, err + return types.Validator{}, err } if err := k.SetValidatorByPowerIndex(ctx, validator); err != nil { - return validator, err + return types.Validator{}, err } // delete from queue if present if err := k.DeleteValidatorQueue(ctx, validator); err != nil { - return validator, err + return types.Validator{}, err } // trigger hook consAddr, err := validator.GetConsAddr() if err != nil { - return validator, err + return types.Validator{}, err } str, err := k.validatorAddressCodec.StringToBytes(validator.GetOperator()) if err != nil { - return validator, err + return types.Validator{}, fmt.Errorf("failed to get validator operator address: %w", err) } if err := k.Hooks().AfterValidatorBonded(ctx, consAddr, str); err != nil { - return validator, err + return types.Validator{}, err } - return validator, err + return validator, nil } // BeginUnbondingValidator performs all the store operations for when a validator begins unbonding func (k Keeper) BeginUnbondingValidator(ctx context.Context, validator types.Validator) (types.Validator, error) { params, err := k.GetParams(ctx) if err != nil { - return validator, err + return types.Validator{}, err } // delete the validator by power index, as the key will change if err = k.DeleteValidatorByPowerIndex(ctx, validator); err != nil { - return validator, err + return types.Validator{}, err } // sanity check if validator.Status != types.Bonded { - panic(fmt.Sprintf("should not already be unbonded or unbonding, validator: %v\n", validator)) + return types.Validator{}, fmt.Errorf("should not already be unbonded or unbonding, validator: %v", validator) } id, err := k.IncrementUnbondingID(ctx) if err != nil { - return validator, err + return types.Validator{}, err } validator = validator.UpdateStatus(types.Unbonding) @@ -402,39 +409,39 @@ func (k Keeper) BeginUnbondingValidator(ctx context.Context, validator types.Val // save the now unbonded validator record and power index if err = k.SetValidator(ctx, validator); err != nil { - return validator, err + return types.Validator{}, err } if err = k.SetValidatorByPowerIndex(ctx, validator); err != nil { - return validator, err + return types.Validator{}, err } // Adds to unbonding validator queue if err = k.InsertUnbondingValidatorQueue(ctx, validator); err != nil { - return validator, err + return types.Validator{}, err } // trigger hook consAddr, err := validator.GetConsAddr() if err != nil { - return validator, err + return types.Validator{}, err } str, err := k.validatorAddressCodec.StringToBytes(validator.GetOperator()) if err != nil { - return validator, err + return types.Validator{}, fmt.Errorf("failed to get validator operator address: %w", err) } if err := k.Hooks().AfterValidatorBeginUnbonding(ctx, consAddr, str); err != nil { - return validator, err + return types.Validator{}, err } if err := k.SetValidatorByUnbondingID(ctx, validator, id); err != nil { - return validator, err + return types.Validator{}, err } if err := k.Hooks().AfterUnbondingInitiated(ctx, id); err != nil { - return validator, err + return types.Validator{}, err } return validator, nil @@ -444,7 +451,7 @@ func (k Keeper) BeginUnbondingValidator(ctx context.Context, validator types.Val func (k Keeper) completeUnbondingValidator(ctx context.Context, validator types.Validator) (types.Validator, error) { validator = validator.UpdateStatus(types.Unbonded) if err := k.SetValidator(ctx, validator); err != nil { - return validator, err + return types.Validator{}, err } return validator, nil