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

refactor(x/distribution)!: Use KVStoreService, context.Context and return errors instead of panic #15948

Merged
merged 29 commits into from
Apr 27, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
29d9853
init
facundomedica Apr 19, 2023
8805808
progress
facundomedica Apr 19, 2023
46dcd03
progress
facundomedica Apr 19, 2023
2efea8d
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Apr 25, 2023
ee67d10
progress
facundomedica Apr 25, 2023
25cac02
progress
facundomedica Apr 25, 2023
ae008bc
progress
facundomedica Apr 25, 2023
ead54cc
progress
facundomedica Apr 25, 2023
6142f3e
fix
facundomedica Apr 25, 2023
fc4adf9
Merge branch 'main' into facu/distr-kvservice
facundomedica Apr 25, 2023
1f707cb
fix
facundomedica Apr 25, 2023
47012a8
sdk.Context -> context.Context
facundomedica Apr 25, 2023
4496fd7
some test fixes
facundomedica Apr 25, 2023
032b624
update expected keepers
facundomedica Apr 25, 2023
e56c725
fix test
facundomedica Apr 25, 2023
16ad475
fix test and linter
facundomedica Apr 25, 2023
0b21215
progress
facundomedica Apr 25, 2023
9b0e372
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Apr 26, 2023
3158740
update changes from bank
facundomedica Apr 26, 2023
fcd42a8
last changes
facundomedica Apr 26, 2023
d89758c
forgot about the migrationhandler
facundomedica Apr 26, 2023
be2d87c
fix
facundomedica Apr 26, 2023
180a672
fix
facundomedica Apr 26, 2023
0d8ff2e
fix
facundomedica Apr 26, 2023
b171c2a
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Apr 26, 2023
77ce060
cl and upgrading ++
facundomedica Apr 26, 2023
ba7b121
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Apr 26, 2023
7a648a1
add nolint
facundomedica Apr 26, 2023
bf3467d
added address codec to autocli opts in app v1
facundomedica Apr 26, 2023
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
Prev Previous commit
Next Next commit
update changes from bank
  • Loading branch information
facundomedica committed Apr 26, 2023
commit 3158740f63e20eb28612b504e36a1d2811b8e356
6 changes: 3 additions & 3 deletions x/distribution/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ https://github.com/cosmos/cosmos-sdk/blob/v0.47.0-rc1/proto/cosmos/distribution/
```

```go
func (k Keeper) SetWithdrawAddr(ctx sdk.Context, delegatorAddr sdk.AccAddress, withdrawAddr sdk.AccAddress) error
func (k Keeper) SetWithdrawAddr(ctx context.Context, delegatorAddr sdk.AccAddress, withdrawAddr sdk.AccAddress) error
if k.blockedAddrs[withdrawAddr.String()] {
fail with "`{withdrawAddr}` is not allowed to receive external funds"
}
Expand Down Expand Up @@ -351,7 +351,7 @@ This message sends coins directly from the sender to the community pool.
The transaction fails if the amount cannot be transferred from the sender to the distribution module account.

```go
func (k Keeper) FundCommunityPool(ctx sdk.Context, amount sdk.Coins, sender sdk.AccAddress) error {
func (k Keeper) FundCommunityPool(ctx context.Context, amount sdk.Coins, sender sdk.AccAddress) error {
if err := k.bankKeeper.SendCoinsFromAccountToModule(ctx, sender, types.ModuleName, amount); err != nil {
return err
}
Expand All @@ -375,7 +375,7 @@ Initializing a delegation increments the validator period and keeps track of the

```go
// initialize starting info for a new delegation
func (k Keeper) initializeDelegation(ctx sdk.Context, val sdk.ValAddress, del sdk.AccAddress) {
func (k Keeper) initializeDelegation(ctx context.Context, val sdk.ValAddress, del sdk.AccAddress) {
// period has already been incremented - we want to store the period ended by this delegation action
previousPeriod := k.GetValidatorCurrentRewards(ctx, val).Period - 1

Expand Down
38 changes: 28 additions & 10 deletions x/distribution/keeper/delegation.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
)

// initialize starting info for a new delegation
func (k Keeper) initializeDelegation(ctx sdk.Context, val sdk.ValAddress, del sdk.AccAddress) error {
func (k Keeper) initializeDelegation(ctx context.Context, val sdk.ValAddress, del sdk.AccAddress) error {
// period has already been incremented - we want to store the period ended by this delegation action
valCurrentRewards, err := k.GetValidatorCurrentRewards(ctx, val)
if err != nil {
Expand All @@ -23,14 +23,15 @@ func (k Keeper) initializeDelegation(ctx sdk.Context, val sdk.ValAddress, del sd
// increment reference count for the period we're going to track
k.incrementReferenceCount(ctx, val, previousPeriod)

validator := k.stakingKeeper.Validator(ctx, val)
delegation := k.stakingKeeper.Delegation(ctx, del, val)
sdkCtx := sdk.UnwrapSDKContext(ctx)
validator := k.stakingKeeper.Validator(sdkCtx, val)
delegation := k.stakingKeeper.Delegation(sdkCtx, del, val)

// calculate delegation stake in tokens
// we don't store directly, so multiply delegation shares * (tokens per share)
// note: necessary to truncate so we don't allow withdrawing more rewards than owed
stake := validator.TokensFromSharesTruncated(delegation.GetShares())
return k.SetDelegatorStartingInfo(ctx, val, del, types.NewDelegatorStartingInfo(previousPeriod, stake, uint64(ctx.BlockHeight())))
return k.SetDelegatorStartingInfo(ctx, val, del, types.NewDelegatorStartingInfo(previousPeriod, stake, uint64(sdkCtx.BlockHeight())))
}

// calculate the rewards accrued by a delegation between two periods
Expand Down Expand Up @@ -164,7 +165,7 @@ func (k Keeper) CalculateDelegationRewards(ctx context.Context, val stakingtypes
return rewards, nil
}

func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val stakingtypes.ValidatorI, del stakingtypes.DelegationI) (sdk.Coins, error) {
func (k Keeper) withdrawDelegationRewards(ctx context.Context, val stakingtypes.ValidatorI, del stakingtypes.DelegationI) (sdk.Coins, error) {
// check existence of delegator starting info
hasInfo, err := k.HasDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr())
if err != nil {
Expand All @@ -176,7 +177,11 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val stakingtypes.Vali
}

// end current period and calculate rewards
endingPeriod := k.IncrementValidatorPeriod(ctx, val)
endingPeriod, err := k.IncrementValidatorPeriod(ctx, val)
if err != nil {
return nil, err
}

rewardsRaw, err := k.CalculateDelegationRewards(ctx, val, del, endingPeriod)
if err != nil {
return nil, err
Expand Down Expand Up @@ -219,14 +224,21 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val stakingtypes.Vali

// update the outstanding rewards and the community pool only if the
// transaction was successful
k.SetValidatorOutstandingRewards(ctx, del.GetValidatorAddr(), types.ValidatorOutstandingRewards{Rewards: outstanding.Sub(rewards)})
err = k.SetValidatorOutstandingRewards(ctx, del.GetValidatorAddr(), types.ValidatorOutstandingRewards{Rewards: outstanding.Sub(rewards)})
if err != nil {
return nil, err
}

feePool, err := k.GetFeePool(ctx)
if err != nil {
return nil, err
}

feePool.CommunityPool = feePool.CommunityPool.Add(remainder...)
k.SetFeePool(ctx, feePool)
err = k.SetFeePool(ctx, feePool)
if err != nil {
return nil, err
}

// decrement reference count of starting period
startingInfo, err := k.GetDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr())
Expand All @@ -235,10 +247,16 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val stakingtypes.Vali
}

startingPeriod := startingInfo.PreviousPeriod
k.decrementReferenceCount(ctx, del.GetValidatorAddr(), startingPeriod)
err = k.decrementReferenceCount(ctx, del.GetValidatorAddr(), startingPeriod)
if err != nil {
return nil, err
}

// remove delegator starting info
k.DeleteDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr())
err = k.DeleteDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr())
if err != nil {
return nil, err
}

if finalRewards.IsZero() {
baseDenom, _ := sdk.GetBaseDenom()
Expand Down
3 changes: 1 addition & 2 deletions x/distribution/keeper/fee_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ func (k Keeper) DistributeFromFeePool(ctx context.Context, amount sdk.Coins, rec

feePool.CommunityPool = newPool

sdkCtx := sdk.UnwrapSDKContext(ctx)
err = k.bankKeeper.SendCoinsFromModuleToAccount(sdkCtx, types.ModuleName, receiveAddr, amount)
err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, receiveAddr, amount)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions x/distribution/keeper/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ func (h Hooks) AfterValidatorRemoved(ctx sdk.Context, _ sdk.ConsAddress, valAddr
// increment period
func (h Hooks) BeforeDelegationCreated(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) error {
val := h.k.stakingKeeper.Validator(ctx, valAddr)
_ = h.k.IncrementValidatorPeriod(ctx, val)
return nil
_, err := h.k.IncrementValidatorPeriod(ctx, val)
return err
}

// withdraw delegation rewards (which also increments period)
Expand Down
8 changes: 3 additions & 5 deletions x/distribution/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,6 @@ func (k Keeper) WithdrawDelegationRewards(ctx context.Context, delAddr sdk.AccAd

// withdraw validator commission
func (k Keeper) WithdrawValidatorCommission(ctx context.Context, valAddr sdk.ValAddress) (sdk.Coins, error) {
sdkCtx := sdk.UnwrapSDKContext(ctx)

// fetch validator accumulated commission
accumCommission, err := k.GetValidatorAccumulatedCommission(ctx, valAddr)
if err != nil {
Expand Down Expand Up @@ -150,12 +148,13 @@ func (k Keeper) WithdrawValidatorCommission(ctx context.Context, valAddr sdk.Val
return nil, err
}

err = k.bankKeeper.SendCoinsFromModuleToAccount(sdkCtx, types.ModuleName, withdrawAddr, commission)
err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, withdrawAddr, commission)
if err != nil {
return nil, err
}
}

sdkCtx := sdk.UnwrapSDKContext(ctx)
sdkCtx.EventManager().EmitEvent(
sdk.NewEvent(
types.EventTypeWithdrawCommission,
Expand Down Expand Up @@ -183,8 +182,7 @@ func (k Keeper) GetTotalRewards(ctx context.Context) (totalRewards sdk.DecCoins)
// added to the pool. An error is returned if the amount cannot be sent to the
// module account.
func (k Keeper) FundCommunityPool(ctx context.Context, amount sdk.Coins, sender sdk.AccAddress) error {
sdkCtx := sdk.UnwrapSDKContext(ctx)
if err := k.bankKeeper.SendCoinsFromAccountToModule(sdkCtx, sender, types.ModuleName, amount); err != nil {
if err := k.bankKeeper.SendCoinsFromAccountToModule(ctx, sender, types.ModuleName, amount); err != nil {
return err
}

Expand Down
24 changes: 9 additions & 15 deletions x/distribution/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func NewMsgServerImpl(keeper Keeper) types.MsgServer {
return &msgServer{Keeper: keeper}
}

func (k msgServer) SetWithdrawAddress(goCtx context.Context, msg *types.MsgSetWithdrawAddress) (*types.MsgSetWithdrawAddressResponse, error) {
func (k msgServer) SetWithdrawAddress(ctx context.Context, msg *types.MsgSetWithdrawAddress) (*types.MsgSetWithdrawAddressResponse, error) {
delegatorAddress, err := k.authKeeper.StringToBytes(msg.DelegatorAddress)
if err != nil {
return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid delegator address: %s", err)
Expand All @@ -36,7 +36,6 @@ func (k msgServer) SetWithdrawAddress(goCtx context.Context, msg *types.MsgSetWi
return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid withdraw address: %s", err)
}

ctx := sdk.UnwrapSDKContext(goCtx)
err = k.SetWithdrawAddr(ctx, delegatorAddress, withdrawAddress)
if err != nil {
return nil, err
Expand All @@ -45,7 +44,7 @@ func (k msgServer) SetWithdrawAddress(goCtx context.Context, msg *types.MsgSetWi
return &types.MsgSetWithdrawAddressResponse{}, nil
}

func (k msgServer) WithdrawDelegatorReward(goCtx context.Context, msg *types.MsgWithdrawDelegatorReward) (*types.MsgWithdrawDelegatorRewardResponse, error) {
func (k msgServer) WithdrawDelegatorReward(ctx context.Context, msg *types.MsgWithdrawDelegatorReward) (*types.MsgWithdrawDelegatorRewardResponse, error) {
valAddr, err := sdk.ValAddressFromBech32(msg.ValidatorAddress)
if err != nil {
return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err)
Expand All @@ -56,7 +55,6 @@ func (k msgServer) WithdrawDelegatorReward(goCtx context.Context, msg *types.Msg
return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid delegator address: %s", err)
}

ctx := sdk.UnwrapSDKContext(goCtx)
amount, err := k.WithdrawDelegationRewards(ctx, delegatorAddress, valAddr)
if err != nil {
return nil, err
Expand All @@ -77,13 +75,12 @@ func (k msgServer) WithdrawDelegatorReward(goCtx context.Context, msg *types.Msg
return &types.MsgWithdrawDelegatorRewardResponse{Amount: amount}, nil
}

func (k msgServer) WithdrawValidatorCommission(goCtx context.Context, msg *types.MsgWithdrawValidatorCommission) (*types.MsgWithdrawValidatorCommissionResponse, error) {
func (k msgServer) WithdrawValidatorCommission(ctx context.Context, msg *types.MsgWithdrawValidatorCommission) (*types.MsgWithdrawValidatorCommissionResponse, error) {
valAddr, err := sdk.ValAddressFromBech32(msg.ValidatorAddress)
if err != nil {
return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid validator address: %s", err)
}

ctx := sdk.UnwrapSDKContext(goCtx)
amount, err := k.Keeper.WithdrawValidatorCommission(ctx, valAddr)
if err != nil {
return nil, err
Expand All @@ -104,7 +101,7 @@ func (k msgServer) WithdrawValidatorCommission(goCtx context.Context, msg *types
return &types.MsgWithdrawValidatorCommissionResponse{Amount: amount}, nil
}

func (k msgServer) FundCommunityPool(goCtx context.Context, msg *types.MsgFundCommunityPool) (*types.MsgFundCommunityPoolResponse, error) {
func (k msgServer) FundCommunityPool(ctx context.Context, msg *types.MsgFundCommunityPool) (*types.MsgFundCommunityPoolResponse, error) {
depositor, err := k.authKeeper.StringToBytes(msg.Depositor)
if err != nil {
return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid depositor address: %s", err)
Expand All @@ -114,15 +111,14 @@ func (k msgServer) FundCommunityPool(goCtx context.Context, msg *types.MsgFundCo
return nil, err
}

ctx := sdk.UnwrapSDKContext(goCtx)
if err := k.Keeper.FundCommunityPool(ctx, msg.Amount, depositor); err != nil {
return nil, err
}

return &types.MsgFundCommunityPoolResponse{}, nil
}

func (k msgServer) UpdateParams(goCtx context.Context, msg *types.MsgUpdateParams) (*types.MsgUpdateParamsResponse, error) {
func (k msgServer) UpdateParams(ctx context.Context, msg *types.MsgUpdateParams) (*types.MsgUpdateParamsResponse, error) {
if err := k.validateAuthority(msg.Authority); err != nil {
return nil, err
}
Expand All @@ -136,15 +132,14 @@ func (k msgServer) UpdateParams(goCtx context.Context, msg *types.MsgUpdateParam
return nil, err
}

ctx := sdk.UnwrapSDKContext(goCtx)
if err := k.SetParams(ctx, msg.Params); err != nil {
return nil, err
}

return &types.MsgUpdateParamsResponse{}, nil
}

func (k msgServer) CommunityPoolSpend(goCtx context.Context, msg *types.MsgCommunityPoolSpend) (*types.MsgCommunityPoolSpendResponse, error) {
func (k msgServer) CommunityPoolSpend(ctx context.Context, msg *types.MsgCommunityPoolSpend) (*types.MsgCommunityPoolSpendResponse, error) {
if err := k.validateAuthority(msg.Authority); err != nil {
return nil, err
}
Expand All @@ -162,7 +157,6 @@ func (k msgServer) CommunityPoolSpend(goCtx context.Context, msg *types.MsgCommu
return nil, errors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to receive external funds", msg.Recipient)
}

ctx := sdk.UnwrapSDKContext(goCtx)
if err := k.DistributeFromFeePool(ctx, msg.Amount, recipient); err != nil {
return nil, err
}
Expand All @@ -173,13 +167,12 @@ func (k msgServer) CommunityPoolSpend(goCtx context.Context, msg *types.MsgCommu
return &types.MsgCommunityPoolSpendResponse{}, nil
}

func (k msgServer) DepositValidatorRewardsPool(goCtx context.Context, msg *types.MsgDepositValidatorRewardsPool) (*types.MsgDepositValidatorRewardsPoolResponse, error) {
func (k msgServer) DepositValidatorRewardsPool(ctx context.Context, msg *types.MsgDepositValidatorRewardsPool) (*types.MsgDepositValidatorRewardsPoolResponse, error) {
depositor, err := k.authKeeper.StringToBytes(msg.Depositor)
if err != nil {
return nil, err
}

ctx := sdk.UnwrapSDKContext(goCtx)
// deposit coins from depositor's account to the distribution module
if err := k.bankKeeper.SendCoinsFromAccountToModule(ctx, depositor, types.ModuleName, msg.Amount); err != nil {
return nil, err
Expand All @@ -190,7 +183,8 @@ func (k msgServer) DepositValidatorRewardsPool(goCtx context.Context, msg *types
return nil, err
}

validator := k.stakingKeeper.Validator(ctx, valAddr)
sdkCtx := sdk.UnwrapSDKContext(ctx)
validator := k.stakingKeeper.Validator(sdkCtx, valAddr)
if validator == nil {
return nil, errors.Wrapf(types.ErrNoValidatorExists, valAddr.String())
}
Expand Down
Loading