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

chore(x/staking): audit changes #16795

Merged
merged 9 commits into from
Jul 3, 2023
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ Ref: https://keepachangelog.com/en/1.0.0/
* remove `Keeper`: `IterateValidatorOutstandingRewards`, `GetValidatorOutstandingRewards`, `SetValidatorOutstandingRewards`, `DeleteValidatorOutstandingRewards`
* (x/distribution) [#16607](https://github.com/cosmos/cosmos-sdk/pull/16607) use collections for `ValidatorHistoricalRewards` state management:
* remove `Keeper`: `IterateValidatorHistoricalRewards`, `GetValidatorHistoricalRewards`, `SetValidatorHistoricalRewards`, `DeleteValidatorHistoricalRewards`, `DeleteValidatorHistoricalReward`, `DeleteAllValidatorHistoricalRewards`
* (x/auth) [#16621](https://github.com/cosmos/cosmos-sdk/pull/16621), [#16768](https://github.com/cosmos/cosmos-sdk/pull/16768) Pass address codecs to auth new keeper constructor.
Copy link
Member

@julienrbrt julienrbrt Jul 3, 2023

Choose a reason for hiding this comment

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

The changelog is at the wrong place. Please move it under unreleased
nvm, I saw it wrong, but the two extra changelog that were added should be removed (as they are released).

* (x/auth/vesting) [#16741](https://github.com/cosmos/cosmos-sdk/pull/16741) Vesting account constructor now return an error with the result of their validate function.
* (x/staking) [#16795](https://github.com/cosmos/cosmos-sdk/pull/16795) `DelegationToDelegationResponse`, `DelegationsToDelegationResponses`, `RedelegationsToRedelegationResponses` are no longer exported.

## [v0.50.0-alpha.1](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.0-alpha.1) - 2023-06-30

Expand Down
3 changes: 2 additions & 1 deletion api/cosmos/staking/v1beta1/genesis.pulsar.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions proto/cosmos/staking/v1beta1/genesis.proto
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ message GenesisState {
// redelegations defines the redelegations active at genesis.
repeated Redelegation redelegations = 7 [(gogoproto.nullable) = false, (amino.dont_omitempty) = true];

// exported defines a bool to identify whether the chain dealing with exported or initialized genesis.
atheeshp marked this conversation as resolved.
Show resolved Hide resolved
bool exported = 8;
}

Expand Down
8 changes: 4 additions & 4 deletions x/staking/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,13 +134,13 @@ $ %s query staking validators
return cmd
}

// GetCmdQueryValidatorUnbondingDelegations implements the query all unbonding delegatations from a validator command.
// GetCmdQueryValidatorUnbondingDelegations implements the query all unbonding delegations from a validator command.
func GetCmdQueryValidatorUnbondingDelegations() *cobra.Command {
bech32PrefixValAddr := sdk.GetConfig().GetBech32ValidatorAddrPrefix()

cmd := &cobra.Command{
Use: "unbonding-delegations-from [validator-addr]",
Short: "Query all unbonding delegatations from a validator",
Short: "Query all unbonding delegations from a validator",
Long: strings.TrimSpace(
fmt.Sprintf(`Query delegations that are unbonding _from_ a validator.

Expand Down Expand Up @@ -188,14 +188,14 @@ $ %s query staking unbonding-delegations-from %s1gghjut3ccd8ay0zduzj64hwre2fxs9l
return cmd
}

// GetCmdQueryValidatorRedelegations implements the query all redelegatations
// GetCmdQueryValidatorRedelegations implements the query all redelegations
// from a validator command.
func GetCmdQueryValidatorRedelegations() *cobra.Command {
bech32PrefixValAddr := sdk.GetConfig().GetBech32ValidatorAddrPrefix()

cmd := &cobra.Command{
Use: "redelegations-from [validator-addr]",
Short: "Query all outgoing redelegatations from a validator",
Short: "Query all outgoing redelegations from a validator",
Long: strings.TrimSpace(
fmt.Sprintf(`Query delegations that are redelegating _from_ a validator.

Expand Down
2 changes: 1 addition & 1 deletion x/staking/keeper/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func (k *Keeper) BeginBlocker(ctx context.Context) error {
return k.TrackHistoricalInfo(ctx)
}

// Called every block, update validator set
// EndBlocker called at every block, update validator set
func (k *Keeper) EndBlocker(ctx context.Context) ([]abci.ValidatorUpdate, error) {
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker)
return k.BlockValidatorUpdates(ctx)
Expand Down
14 changes: 7 additions & 7 deletions x/staking/keeper/alias_functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

// Validator Set

// iterate through the validator set and perform the provided function
// IterateValidators iterates through the validator set and perform the provided function
func (k Keeper) IterateValidators(ctx context.Context, fn func(index int64, validator types.ValidatorI) (stop bool)) error {
store := k.storeService.OpenKVStore(ctx)
iterator, err := store.Iterator(types.ValidatorsKey, storetypes.PrefixEndBytes(types.ValidatorsKey))
Expand All @@ -38,7 +38,7 @@ func (k Keeper) IterateValidators(ctx context.Context, fn func(index int64, vali
return nil
}

// iterate through the bonded validator set and perform the provided function
// IterateBondedValidatorsByPower iterates through the bonded validator set and perform the provided function
func (k Keeper) IterateBondedValidatorsByPower(ctx context.Context, fn func(index int64, validator types.ValidatorI) (stop bool)) error {
store := k.storeService.OpenKVStore(ctx)
maxValidators, err := k.MaxValidators(ctx)
Expand Down Expand Up @@ -69,7 +69,7 @@ func (k Keeper) IterateBondedValidatorsByPower(ctx context.Context, fn func(inde
return nil
}

// iterate through the active validator set and perform the provided function
// IterateLastValidators iterates through the active validator set and perform the provided function
func (k Keeper) IterateLastValidators(ctx context.Context, fn func(index int64, validator types.ValidatorI) (stop bool)) error {
iterator, err := k.LastValidatorsIterator(ctx)
if err != nil {
Expand Down Expand Up @@ -108,12 +108,12 @@ func (k Keeper) ValidatorByConsAddr(ctx context.Context, addr sdk.ConsAddress) (

// Delegation Set

// Returns self as it is both a validatorset and delegationset
// GetValidatorSet returns self as it is both a validatorset and delegationset
func (k Keeper) GetValidatorSet() types.ValidatorSet {
return k
}

// Delegation get the delegation interface for a particular set of delegator and validator addresses
// Delegation gets the delegation interface for a particular set of delegator and validator addresses
func (k Keeper) Delegation(ctx context.Context, addrDel sdk.AccAddress, addrVal sdk.ValAddress) (types.DelegationI, error) {
bond, err := k.GetDelegation(ctx, addrDel, addrVal)
if err != nil {
Expand All @@ -123,7 +123,7 @@ func (k Keeper) Delegation(ctx context.Context, addrDel sdk.AccAddress, addrVal
return bond, nil
}

// iterate through all of the delegations from a delegator
// IterateDelegations iterates through all of the delegations from a delegator
func (k Keeper) IterateDelegations(ctx context.Context, delAddr sdk.AccAddress,
fn func(index int64, del types.DelegationI) (stop bool),
) error {
Expand Down Expand Up @@ -151,7 +151,7 @@ func (k Keeper) IterateDelegations(ctx context.Context, delAddr sdk.AccAddress,
return nil
}

// return all delegations used during genesis dump
// GetAllSDKDelegations returns all delegations used during genesis dump
// TODO: remove this func, change all usage for iterate functionality
func (k Keeper) GetAllSDKDelegations(ctx context.Context) (delegations []types.Delegation, err error) {
store := k.storeService.OpenKVStore(ctx)
Expand Down
8 changes: 4 additions & 4 deletions x/staking/keeper/delegation.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ func (k Keeper) IterateDelegatorRedelegations(ctx context.Context, delegator sdk
return nil
}

// HasMaxUnbondingDelegationEntries - check if unbonding delegation has maximum number of entries.
// HasMaxUnbondingDelegationEntries checks if unbonding delegation has maximum number of entries.
func (k Keeper) HasMaxUnbondingDelegationEntries(ctx context.Context, delegatorAddr sdk.AccAddress, validatorAddr sdk.ValAddress) (bool, error) {
ubd, err := k.GetUnbondingDelegation(ctx, delegatorAddr, validatorAddr)
if err != nil && !errors.Is(err, types.ErrNoUnbondingDelegation) {
Expand Down Expand Up @@ -616,7 +616,7 @@ func (k Keeper) HasReceivingRedelegation(ctx context.Context, delAddr sdk.AccAdd
return iterator.Valid(), nil
}

// HasMaxRedelegationEntries checks if redelegation has maximum number of entries.
// HasMaxRedelegationEntries checks if the redelegation entries reached maximum limit.
func (k Keeper) HasMaxRedelegationEntries(ctx context.Context, delegatorAddr sdk.AccAddress, validatorSrcAddr, validatorDstAddr sdk.ValAddress) (bool, error) {
red, err := k.GetRedelegation(ctx, delegatorAddr, validatorSrcAddr, validatorDstAddr)
if err != nil {
Expand All @@ -634,7 +634,7 @@ func (k Keeper) HasMaxRedelegationEntries(ctx context.Context, delegatorAddr sdk
return len(red.Entries) >= int(maxEntries), nil
}

// SetRedelegation set a redelegation and associated index.
// SetRedelegation sets a redelegation and associated index.
func (k Keeper) SetRedelegation(ctx context.Context, red types.Redelegation) error {
delegatorAddress, err := k.authKeeper.AddressCodec().StringToBytes(red.DelegatorAddress)
if err != nil {
Expand Down Expand Up @@ -1319,7 +1319,7 @@ func (k Keeper) CompleteRedelegation(
}

// ValidateUnbondAmount validates that a given unbond or redelegation amount is
// valied based on upon the converted shares. If the amount is valid, the total
// valid based on upon the converted shares. If the amount is valid, the total
// amount of respective shares is returned, otherwise an error is returned.
func (k Keeper) ValidateUnbondAmount(
ctx context.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress, amt math.Int,
Expand Down
20 changes: 10 additions & 10 deletions x/staking/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (k Querier) ValidatorDelegations(ctx context.Context, req *types.QueryValid
pageRes = pageResponse
}

delResponses, err := DelegationsToDelegationResponses(ctx, k.Keeper, dels)
delResponses, err := delegationsToDelegationResponses(ctx, k.Keeper, dels)
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
Expand Down Expand Up @@ -234,15 +234,15 @@ func (k Querier) Delegation(ctx context.Context, req *types.QueryDelegationReque
req.DelegatorAddr, req.ValidatorAddr)
}

delResponse, err := DelegationToDelegationResponse(ctx, k.Keeper, delegation)
delResponse, err := delegationToDelegationResponse(ctx, k.Keeper, delegation)
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}

return &types.QueryDelegationResponse{DelegationResponse: &delResponse}, nil
}

// UnbondingDelegation queries unbonding info for give validator delegator pair
// UnbondingDelegation queries unbonding info for given validator delegator pair
func (k Querier) UnbondingDelegation(ctx context.Context, req *types.QueryUnbondingDelegationRequest) (*types.QueryUnbondingDelegationResponse, error) {
if req == nil {
return nil, status.Errorf(codes.InvalidArgument, "empty request")
Expand Down Expand Up @@ -276,7 +276,7 @@ func (k Querier) UnbondingDelegation(ctx context.Context, req *types.QueryUnbond
return &types.QueryUnbondingDelegationResponse{Unbond: unbond}, nil
}

// DelegatorDelegations queries all delegations of a give delegator address
// DelegatorDelegations queries all delegations of a given delegator address
func (k Querier) DelegatorDelegations(ctx context.Context, req *types.QueryDelegatorDelegationsRequest) (*types.QueryDelegatorDelegationsResponse, error) {
if req == nil {
return nil, status.Errorf(codes.InvalidArgument, "empty request")
Expand Down Expand Up @@ -306,7 +306,7 @@ func (k Querier) DelegatorDelegations(ctx context.Context, req *types.QueryDeleg
return nil, status.Error(codes.Internal, err.Error())
}

delegationResps, err := DelegationsToDelegationResponses(ctx, k.Keeper, delegations)
delegationResps, err := delegationsToDelegationResponses(ctx, k.Keeper, delegations)
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
Expand Down Expand Up @@ -420,7 +420,7 @@ func (k Querier) Redelegations(ctx context.Context, req *types.QueryRedelegation
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
redelResponses, err := RedelegationsToRedelegationResponses(ctx, k.Keeper, redels)
redelResponses, err := redelegationsToRedelegationResponses(ctx, k.Keeper, redels)
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
Expand Down Expand Up @@ -564,7 +564,7 @@ func queryAllRedelegations(store storetypes.KVStore, k Querier, req *types.Query

// util

func DelegationToDelegationResponse(ctx context.Context, k *Keeper, del types.Delegation) (types.DelegationResponse, error) {
func delegationToDelegationResponse(ctx context.Context, k *Keeper, del types.Delegation) (types.DelegationResponse, error) {
val, err := k.GetValidator(ctx, del.GetValidatorAddr())
if err != nil {
return types.DelegationResponse{}, err
Expand All @@ -588,11 +588,11 @@ func DelegationToDelegationResponse(ctx context.Context, k *Keeper, del types.De
), nil
}

func DelegationsToDelegationResponses(ctx context.Context, k *Keeper, delegations types.Delegations) (types.DelegationResponses, error) {
func delegationsToDelegationResponses(ctx context.Context, k *Keeper, delegations types.Delegations) (types.DelegationResponses, error) {
resp := make(types.DelegationResponses, len(delegations))

for i, del := range delegations {
delResp, err := DelegationToDelegationResponse(ctx, k, del)
delResp, err := delegationToDelegationResponse(ctx, k, del)
if err != nil {
return nil, err
}
Expand All @@ -603,7 +603,7 @@ func DelegationsToDelegationResponses(ctx context.Context, k *Keeper, delegation
return resp, nil
}

func RedelegationsToRedelegationResponses(ctx context.Context, k *Keeper, redels types.Redelegations) (types.RedelegationResponses, error) {
func redelegationsToRedelegationResponses(ctx context.Context, k *Keeper, redels types.Redelegations) (types.RedelegationResponses, error) {
resp := make(types.RedelegationResponses, len(redels))

for i, redel := range redels {
Expand Down
8 changes: 3 additions & 5 deletions x/staking/keeper/historical_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,9 @@ func (k Keeper) DeleteHistoricalInfo(ctx context.Context, height int64) error {
return store.Delete(key)
}

// IterateHistoricalInfo provides an interator over all stored HistoricalInfo
//
// objects. For each HistoricalInfo object, cb will be called. If the cb returns
//
// true, the iterator will close and stop.
// IterateHistoricalInfo provides an iterator over all stored HistoricalInfo
// objects. For each HistoricalInfo object, cb will be called. If the cb returns
// true, the iterator will break and close.
func (k Keeper) IterateHistoricalInfo(ctx context.Context, cb func(types.HistoricalInfo) bool) error {
store := k.storeService.OpenKVStore(ctx)
iterator, err := store.Iterator(types.HistoricalInfoKey, storetypes.PrefixEndBytes(types.HistoricalInfoKey))
Expand Down
6 changes: 3 additions & 3 deletions x/staking/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (k *Keeper) Hooks() types.StakingHooks {
return k.hooks
}

// SetHooks Set the validator hooks. In contrast to other receivers, this method must take a pointer due to nature
// SetHooks sets the validator hooks. In contrast to other receivers, this method must take a pointer due to nature
// of the hooks interface and SDK start up sequence.
func (k *Keeper) SetHooks(sh types.StakingHooks) {
if k.hooks != nil {
Expand All @@ -89,7 +89,7 @@ func (k *Keeper) SetHooks(sh types.StakingHooks) {
k.hooks = sh
}

// GetLastTotalPower Load the last total validator power.
// GetLastTotalPower loads the last total validator power.
func (k Keeper) GetLastTotalPower(ctx context.Context) (math.Int, error) {
store := k.storeService.OpenKVStore(ctx)
bz, err := store.Get(types.LastTotalPowerKey)
Expand All @@ -110,7 +110,7 @@ func (k Keeper) GetLastTotalPower(ctx context.Context) (math.Int, error) {
return ip.Int, nil
}

// SetLastTotalPower Set the last total validator power.
// SetLastTotalPower sets the last total validator power.
func (k Keeper) SetLastTotalPower(ctx context.Context, power math.Int) error {
store := k.storeService.OpenKVStore(ctx)
bz, err := k.cdc.Marshal(&sdk.IntProto{Int: power})
Expand Down
2 changes: 1 addition & 1 deletion x/staking/keeper/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type Migrator struct {
legacySubspace exported.Subspace
}

// NewMigrator returns a new Migrator.
// NewMigrator returns a new Migrator instance.
func NewMigrator(keeper *Keeper, legacySubspace exported.Subspace) Migrator {
return Migrator{
keeper: keeper,
Expand Down
5 changes: 3 additions & 2 deletions x/staking/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type msgServer struct {
*Keeper
}

// NewMsgServerImpl returns an implementation of the bank MsgServer interface
// NewMsgServerImpl returns an implementation of the staking MsgServer interface
// for the provided Keeper.
func NewMsgServerImpl(keeper *Keeper) types.MsgServer {
return &msgServer{Keeper: keeper}
Expand Down Expand Up @@ -301,7 +301,7 @@ func (k msgServer) Delegate(ctx context.Context, msg *types.MsgDelegate) (*types
return &types.MsgDelegateResponse{}, nil
}

// BeginRedelegate defines a method for performing a redelegation of coins from a delegator and source validator to a destination validator
// BeginRedelegate defines a method for performing a redelegation of coins from a source validator to a destination validator of given delegator
func (k msgServer) BeginRedelegate(ctx context.Context, msg *types.MsgBeginRedelegate) (*types.MsgBeginRedelegateResponse, error) {
valSrcAddr, err := sdk.ValAddressFromBech32(msg.ValidatorSrcAddress)
if err != nil {
Expand Down Expand Up @@ -576,6 +576,7 @@ func (k msgServer) CancelUnbondingDelegation(ctx context.Context, msg *types.Msg
return &types.MsgCancelUnbondingDelegationResponse{}, nil
}

// UpdateParams defines a method to perform updation of params exist in x/staking module.
func (k msgServer) UpdateParams(ctx context.Context, msg *types.MsgUpdateParams) (*types.MsgUpdateParamsResponse, error) {
if k.authority != msg.Authority {
return nil, errorsmod.Wrapf(types.ErrInvalidSigner, "invalid authority; expected %s, got %s", k.authority, msg.Authority)
Expand Down
2 changes: 1 addition & 1 deletion x/staking/keeper/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func (k Keeper) SetParams(ctx context.Context, params types.Params) error {
return store.Set(types.ParamsKey, bz)
}

// GetParams sets the x/staking module parameters.
// GetParams gets the x/staking module parameters.
func (k Keeper) GetParams(ctx context.Context) (params types.Params, err error) {
store := k.storeService.OpenKVStore(ctx)
bz, err := store.Get(types.ParamsKey)
Expand Down
4 changes: 2 additions & 2 deletions x/staking/keeper/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (k Keeper) notBondedTokensToBonded(ctx context.Context, tokens math.Int) er
return k.bankKeeper.SendCoinsFromModuleToModule(ctx, types.NotBondedPoolName, types.BondedPoolName, coins)
}

// burnBondedTokens removes coins from the bonded pool module account
// burnBondedTokens burns coins from the bonded pool module account
func (k Keeper) burnBondedTokens(ctx context.Context, amt math.Int) error {
if !amt.IsPositive() {
// skip as no coins need to be burned
Expand All @@ -58,7 +58,7 @@ func (k Keeper) burnBondedTokens(ctx context.Context, amt math.Int) error {
return k.bankKeeper.BurnCoins(ctx, types.BondedPoolName, coins)
}

// burnNotBondedTokens removes coins from the not bonded pool module account
// burnNotBondedTokens burns coins from the not bonded pool module account
func (k Keeper) burnNotBondedTokens(ctx context.Context, amt math.Int) error {
if !amt.IsPositive() {
// skip as no coins need to be burned
Expand Down
4 changes: 2 additions & 2 deletions x/staking/keeper/power_reduction.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
)

// TokensToConsensusPower - convert input tokens to potential consensus-engine power
// TokensToConsensusPower converts input tokens to potential consensus-engine power
func (k Keeper) TokensToConsensusPower(ctx context.Context, tokens math.Int) int64 {
return sdk.TokensToConsensusPower(tokens, k.PowerReduction(ctx))
}

// TokensFromConsensusPower - convert input power to tokens
// TokensFromConsensusPower converts input power to tokens
func (k Keeper) TokensFromConsensusPower(ctx context.Context, power int64) math.Int {
return sdk.TokensFromConsensusPower(power, k.PowerReduction(ctx))
}
Loading
Loading