Skip to content

Commit

Permalink
chore(x/staking): audit changes (#16795)
Browse files Browse the repository at this point in the history
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
(cherry picked from commit 9b237c7)

# Conflicts:
#	CHANGELOG.md
  • Loading branch information
atheeshp authored and mergify[bot] committed Jul 3, 2023
1 parent d9c53bf commit a1208af
Show file tree
Hide file tree
Showing 22 changed files with 102 additions and 123 deletions.
19 changes: 19 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,25 @@ Ref: https://keepachangelog.com/en/1.0.0/
### API Breaking Changes

* (types/math) [#16040](https://github.com/cosmos/cosmos-sdk/pull/16798) Remove aliases in `types/math.go` (part 2).
<<<<<<< HEAD
=======
* (x/distribution) [#16440](https://github.com/cosmos/cosmos-sdk/pull/16440) use collections for `DelegatorWithdrawAddresState`:
* remove `Keeper`: `SetDelegatorWithdrawAddr`, `DeleteDelegatorWithdrawAddr`, `IterateDelegatorWithdrawAddrs`.
* (x/distribution) [#16459](https://github.com/cosmos/cosmos-sdk/pull/16459) use collections for `ValidatorCurrentRewards` state management:
* remove `Keeper`: `IterateValidatorCurrentRewards`, `GetValidatorCurrentRewards`, `SetValidatorCurrentRewards`, `DeleteValidatorCurrentRewards`
* (x/authz) [#16509](https://github.com/cosmos/cosmos-sdk/pull/16509) `AcceptResponse` has been moved to sdk/types/authz and the `Updated` field is now of the type `sdk.Msg` instead of `authz.Authorization`.
* (x/distribution) [#16483](https://github.com/cosmos/cosmos-sdk/pull/16483) use collections for `DelegatorStartingInfo` state management:
* remove `Keeper`: `IterateDelegatorStartingInfo`, `GetDelegatorStartingInfo`, `SetDelegatorStartingInfo`, `DeleteDelegatorStartingInfo`, `HasDelegatorStartingInfo`
* (x/distribution) [#16571](https://github.com/cosmos/cosmos-sdk/pull/16571) use collections for `ValidatorAccumulatedCommission` state management:
* remove `Keeper`: `IterateValidatorAccumulatedCommission`, `GetValidatorAccumulatedCommission`, `SetValidatorAccumulatedCommission`, `DeleteValidatorAccumulatedCommission`
* (x/distribution) [#16590](https://github.com/cosmos/cosmos-sdk/pull/16590) use collections for `ValidatorOutstandingRewards` state management:
* 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.
* (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.
>>>>>>> 9b237c718 (chore(x/staking): audit changes (#16795))
## [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.
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 @@ -24,7 +24,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 @@ -302,7 +302,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 @@ -577,6 +577,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(govtypes.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
Loading

0 comments on commit a1208af

Please sign in to comment.