From d00a751697a220ece464854ef96cf218e1287046 Mon Sep 17 00:00:00 2001 From: stackman27 Date: Fri, 9 Dec 2022 18:46:00 -0800 Subject: [PATCH 1/7] started work --- app/keepers/keepers.go | 1 + go.mod | 4 ++++ x/valset-pref/keeper.go | 15 +++++++++------ x/valset-pref/msg_server.go | 7 +++++++ x/valset-pref/types/expected_interfaces.go | 6 ++++++ 5 files changed, 27 insertions(+), 6 deletions(-) diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index 5953b717aa5..02af47b25fc 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -363,6 +363,7 @@ func (appKeepers *AppKeepers) InitNormalKeepers( appKeepers.keys[valsetpreftypes.StoreKey], appKeepers.GetSubspace(valsetpreftypes.ModuleName), appKeepers.StakingKeeper, + appKeepers.DistrKeeper, ) appKeepers.ValidatorSetPreferenceKeeper = &validatorSetPreferenceKeeper diff --git a/go.mod b/go.mod index bc84e493f66..1912b8391cd 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,11 @@ go 1.18 require ( github.com/CosmWasm/wasmd v0.29.2-osmo-v13 github.com/cosmos/cosmos-proto v1.0.0-alpha8 +<<<<<<< HEAD github.com/cosmos/cosmos-sdk v0.46.7 +======= + github.com/cosmos/cosmos-sdk v0.46.6 +>>>>>>> 8c4b14063 (started work) github.com/cosmos/go-bip39 v1.0.0 github.com/cosmos/iavl v0.19.4 github.com/cosmos/ibc-go/v3 v3.4.0 diff --git a/x/valset-pref/keeper.go b/x/valset-pref/keeper.go index 10896eb0223..73312cb006c 100644 --- a/x/valset-pref/keeper.go +++ b/x/valset-pref/keeper.go @@ -13,19 +13,22 @@ import ( ) type Keeper struct { - storeKey sdk.StoreKey - paramSpace paramtypes.Subspace - stakingKeeper types.StakingInterface + storeKey sdk.StoreKey + paramSpace paramtypes.Subspace + stakingKeeper types.StakingInterface + communityPoolKeeper types.CommunityPoolKeeper } func NewKeeper(storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, stakingKeeper types.StakingInterface, + communityPoolKeeper types.CommunityPoolKeeper, ) Keeper { return Keeper{ - storeKey: storeKey, - paramSpace: paramSpace, - stakingKeeper: stakingKeeper, + storeKey: storeKey, + paramSpace: paramSpace, + stakingKeeper: stakingKeeper, + communityPoolKeeper: communityPoolKeeper, } } diff --git a/x/valset-pref/msg_server.go b/x/valset-pref/msg_server.go index 104c8aca030..9ce5d0cd965 100644 --- a/x/valset-pref/msg_server.go +++ b/x/valset-pref/msg_server.go @@ -92,5 +92,12 @@ func (server msgServer) RedelegateValidatorSet(goCtx context.Context, msg *types } func (server msgServer) WithdrawDelegationRewards(goCtx context.Context, msg *types.MsgWithdrawDelegationRewards) (*types.MsgWithdrawDelegationRewardsResponse, error) { + ctx := sdk.UnwrapSDKContext(goCtx) + + err := server.keeper.WithdrawDelegationRewards(ctx, msg.Delegator) + if err != nil { + return nil, err + } + return &types.MsgWithdrawDelegationRewardsResponse{}, nil } diff --git a/x/valset-pref/types/expected_interfaces.go b/x/valset-pref/types/expected_interfaces.go index 5c125e34d1b..7caa4f43edc 100644 --- a/x/valset-pref/types/expected_interfaces.go +++ b/x/valset-pref/types/expected_interfaces.go @@ -20,3 +20,9 @@ type StakingInterface interface { type BankKeeper interface { GetBalance(ctx sdk.Context, addr sdk.AccAddress, denom string) sdk.Coin } + +type CommunityPoolKeeper interface { + WithdrawDelegationRewards(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) (sdk.Coins, error) + IncrementValidatorPeriod(ctx sdk.Context, val stakingtypes.ValidatorI) uint64 + CalculateDelegationRewards(ctx sdk.Context, val stakingtypes.ValidatorI, del stakingtypes.DelegationI, endingPeriod uint64) (rewards sdk.DecCoins) +} From aa2da21dfa783336079846cdea47280208c6273b Mon Sep 17 00:00:00 2001 From: stackman27 Date: Sat, 10 Dec 2022 15:24:50 -0800 Subject: [PATCH 2/7] withdraw reward message added --- go.mod | 1 + go.sum | 2 ++ x/valset-pref/keeper_test.go | 18 ++++++++++++++++++ x/valset-pref/types/expected_interfaces.go | 1 + 4 files changed, 22 insertions(+) diff --git a/go.mod b/go.mod index 1912b8391cd..5d548c1b73f 100644 --- a/go.mod +++ b/go.mod @@ -41,6 +41,7 @@ require ( ) require ( + cosmossdk.io/math v1.0.0-beta.4 // indirect github.com/Abirdcfly/dupword v0.0.7 // indirect github.com/Djarvur/go-err113 v0.0.0-20210108212216-aea10b59be24 // indirect github.com/alingse/asasalint v0.0.11 // indirect diff --git a/go.sum b/go.sum index c431c4a4045..c9f83a58cd1 100644 --- a/go.sum +++ b/go.sum @@ -36,6 +36,8 @@ cloud.google.com/go/storage v1.6.0/go.mod h1:N7U0C8pVQ/+NIKOBQyamJIeKQKkZ+mxpohl cloud.google.com/go/storage v1.8.0/go.mod h1:Wv1Oy7z6Yz3DshWRJFhqM/UCfaWIRTdp0RXyy7KQOVs= cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9ullr3+Kg0= cloud.google.com/go/storage v1.14.0/go.mod h1:GrKmX003DSIwi9o29oFT7YDnHYwZoctc3fOKtUw0Xmo= +cosmossdk.io/math v1.0.0-beta.4 h1:JtKedVLGzA0vv84xjYmZ75RKG35Kf2WwcFu8IjRkIIw= +cosmossdk.io/math v1.0.0-beta.4/go.mod h1:An0MllWJY6PxibUpnwGk8jOm+a/qIxlKmL5Zyp9NnaM= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= filippo.io/edwards25519 v1.0.0-beta.2 h1:/BZRNzm8N4K4eWfK28dL4yescorxtO7YG1yun8fy+pI= filippo.io/edwards25519 v1.0.0-beta.2/go.mod h1:X+pm78QAUPtFLi1z9PYIlS/bdDnvbCOGKtZ+ACWEf7o= diff --git a/x/valset-pref/keeper_test.go b/x/valset-pref/keeper_test.go index d7baaa0a316..cfb25c68307 100644 --- a/x/valset-pref/keeper_test.go +++ b/x/valset-pref/keeper_test.go @@ -4,6 +4,7 @@ import ( "testing" sdk "github.com/cosmos/cosmos-sdk/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" "github.com/osmosis-labs/osmosis/v13/app/apptesting" "github.com/osmosis-labs/osmosis/v13/x/valset-pref/types" "github.com/stretchr/testify/suite" @@ -41,6 +42,23 @@ func (suite *KeeperTestSuite) PrepareDelegateToValidatorSet() []types.ValidatorP return valPreferences } +func (suite *KeeperTestSuite) GetDelegationRewards(ctx sdk.Context, val types.ValidatorPreference, delegator sdk.AccAddress) (sdk.DecCoins, stakingtypes.Validator) { + valAddr, err := sdk.ValAddressFromBech32(val.ValOperAddress) + suite.Require().NoError(err) + + validator, found := suite.App.StakingKeeper.GetValidator(ctx, valAddr) + suite.Require().True(found) + + endingPeriod := suite.App.DistrKeeper.IncrementValidatorPeriod(ctx, validator) + + delegation, found := suite.App.StakingKeeper.GetDelegation(ctx, delegator, valAddr) + suite.Require().True(found) + + rewards := suite.App.DistrKeeper.CalculateDelegationRewards(ctx, validator, delegation, endingPeriod) + + return rewards, validator +} + func TestKeeperTestSuite(t *testing.T) { suite.Run(t, new(KeeperTestSuite)) } diff --git a/x/valset-pref/types/expected_interfaces.go b/x/valset-pref/types/expected_interfaces.go index 7caa4f43edc..8ea15f4708e 100644 --- a/x/valset-pref/types/expected_interfaces.go +++ b/x/valset-pref/types/expected_interfaces.go @@ -25,4 +25,5 @@ type CommunityPoolKeeper interface { WithdrawDelegationRewards(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) (sdk.Coins, error) IncrementValidatorPeriod(ctx sdk.Context, val stakingtypes.ValidatorI) uint64 CalculateDelegationRewards(ctx sdk.Context, val stakingtypes.ValidatorI, del stakingtypes.DelegationI, endingPeriod uint64) (rewards sdk.DecCoins) + AllocateTokensToValidator(ctx sdk.Context, val stakingtypes.ValidatorI, tokens sdk.DecCoins) } From 45adaadb4619ceb44592782f0af540cfe6563cba Mon Sep 17 00:00:00 2001 From: stackman27 Date: Sat, 10 Dec 2022 15:27:01 -0800 Subject: [PATCH 3/7] updated gomod --- go.mod | 1 - go.sum | 2 -- 2 files changed, 3 deletions(-) diff --git a/go.mod b/go.mod index 5d548c1b73f..1912b8391cd 100644 --- a/go.mod +++ b/go.mod @@ -41,7 +41,6 @@ require ( ) require ( - cosmossdk.io/math v1.0.0-beta.4 // indirect github.com/Abirdcfly/dupword v0.0.7 // indirect github.com/Djarvur/go-err113 v0.0.0-20210108212216-aea10b59be24 // indirect github.com/alingse/asasalint v0.0.11 // indirect diff --git a/go.sum b/go.sum index c9f83a58cd1..c431c4a4045 100644 --- a/go.sum +++ b/go.sum @@ -36,8 +36,6 @@ cloud.google.com/go/storage v1.6.0/go.mod h1:N7U0C8pVQ/+NIKOBQyamJIeKQKkZ+mxpohl cloud.google.com/go/storage v1.8.0/go.mod h1:Wv1Oy7z6Yz3DshWRJFhqM/UCfaWIRTdp0RXyy7KQOVs= cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9ullr3+Kg0= cloud.google.com/go/storage v1.14.0/go.mod h1:GrKmX003DSIwi9o29oFT7YDnHYwZoctc3fOKtUw0Xmo= -cosmossdk.io/math v1.0.0-beta.4 h1:JtKedVLGzA0vv84xjYmZ75RKG35Kf2WwcFu8IjRkIIw= -cosmossdk.io/math v1.0.0-beta.4/go.mod h1:An0MllWJY6PxibUpnwGk8jOm+a/qIxlKmL5Zyp9NnaM= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= filippo.io/edwards25519 v1.0.0-beta.2 h1:/BZRNzm8N4K4eWfK28dL4yescorxtO7YG1yun8fy+pI= filippo.io/edwards25519 v1.0.0-beta.2/go.mod h1:X+pm78QAUPtFLi1z9PYIlS/bdDnvbCOGKtZ+ACWEf7o= From 1cbc820141b6f7b6e86964c9403df5ad65e78226 Mon Sep 17 00:00:00 2001 From: stackman27 Date: Tue, 20 Dec 2022 13:44:41 -0800 Subject: [PATCH 4/7] adam and dev feedback --- go.mod | 4 - x/valset-pref/keeper.go | 18 ++--- x/valset-pref/msg_server_test.go | 91 ++++++++++++++++++++++ x/valset-pref/types/expected_interfaces.go | 3 +- x/valset-pref/validator_set.go | 30 +++++++ 5 files changed, 132 insertions(+), 14 deletions(-) diff --git a/go.mod b/go.mod index 1912b8391cd..bc84e493f66 100644 --- a/go.mod +++ b/go.mod @@ -5,11 +5,7 @@ go 1.18 require ( github.com/CosmWasm/wasmd v0.29.2-osmo-v13 github.com/cosmos/cosmos-proto v1.0.0-alpha8 -<<<<<<< HEAD github.com/cosmos/cosmos-sdk v0.46.7 -======= - github.com/cosmos/cosmos-sdk v0.46.6 ->>>>>>> 8c4b14063 (started work) github.com/cosmos/go-bip39 v1.0.0 github.com/cosmos/iavl v0.19.4 github.com/cosmos/ibc-go/v3 v3.4.0 diff --git a/x/valset-pref/keeper.go b/x/valset-pref/keeper.go index 73312cb006c..ed91d12db98 100644 --- a/x/valset-pref/keeper.go +++ b/x/valset-pref/keeper.go @@ -13,22 +13,22 @@ import ( ) type Keeper struct { - storeKey sdk.StoreKey - paramSpace paramtypes.Subspace - stakingKeeper types.StakingInterface - communityPoolKeeper types.CommunityPoolKeeper + storeKey sdk.StoreKey + paramSpace paramtypes.Subspace + stakingKeeper types.StakingInterface + distirbutionKeeper types.DistributionKeeper } func NewKeeper(storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, stakingKeeper types.StakingInterface, - communityPoolKeeper types.CommunityPoolKeeper, + distirbutionKeeper types.DistributionKeeper, ) Keeper { return Keeper{ - storeKey: storeKey, - paramSpace: paramSpace, - stakingKeeper: stakingKeeper, - communityPoolKeeper: communityPoolKeeper, + storeKey: storeKey, + paramSpace: paramSpace, + stakingKeeper: stakingKeeper, + distirbutionKeeper: distirbutionKeeper, } } diff --git a/x/valset-pref/msg_server_test.go b/x/valset-pref/msg_server_test.go index 15e87b46445..f357163b5fb 100644 --- a/x/valset-pref/msg_server_test.go +++ b/x/valset-pref/msg_server_test.go @@ -470,3 +470,94 @@ func (suite *KeeperTestSuite) TestRedelegateValidatorSet() { }) } } + +func (suite *KeeperTestSuite) TestWithdrawDelegationRewards() { + tests := []struct { + name string + delegator sdk.AccAddress + coinToStake sdk.Coin + setValSet bool + setDelegation bool + expectPass bool + }{ + { + name: "Withdraw all rewards with existing delegation", + delegator: sdk.AccAddress([]byte("addr1---------------")), + coinToStake: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(20_000_000)), // delegate 20osmo + setValSet: true, + setDelegation: true, + expectPass: true, + }, + { + name: "Withdraw all rewards, val-set doesnot exist", + delegator: sdk.AccAddress([]byte("addr1---------------")), + coinToStake: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(20_000_000)), + expectPass: false, + }, + { + name: "Withdraw all rewards, delegation doesnot exist", + delegator: sdk.AccAddress([]byte("addr1---------------")), + coinToStake: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(20_000_000)), + setDelegation: true, + expectPass: false, + }, + } + + for _, test := range tests { + suite.Run(test.name, func() { + suite.SetupTest() + + suite.FundAcc(test.delegator, sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 100_000_000)}) // 100 osmo + + // setup message server + msgServer := valPref.NewMsgServerImpl(suite.App.ValidatorSetPreferenceKeeper) + c := sdk.WrapSDKContext(suite.Ctx) + + // call the create validator set preference + preferences := suite.PrepareDelegateToValidatorSet() + ctx := suite.Ctx + + // delegators have to set val-set before delegating tokens + if test.setValSet { + _, err := msgServer.SetValidatorSetPreference(c, types.NewMsgSetValidatorSetPreference(test.delegator, preferences)) + suite.Require().NoError(err) + + if test.setDelegation { + // call the create validator set preference + _, err := msgServer.DelegateToValidatorSet(c, types.NewMsgDelegateToValidatorSet(test.delegator, test.coinToStake)) + suite.Require().NoError(err) + + // incrementing the blockheight by 1 for reward + ctx = suite.Ctx.WithBlockHeight(suite.Ctx.BlockHeight() + 1) + + // only necessary if there are tokens delegated + for _, val := range preferences { + // check that there is enough reward to withdraw + _, validator := suite.GetDelegationRewards(ctx, val, test.delegator) + + // allocate some rewards + tokens := sdk.NewDecCoins(sdk.NewInt64DecCoin(sdk.DefaultBondDenom, 10)) + suite.App.DistrKeeper.AllocateTokensToValidator(ctx, validator, tokens) + + rewardsAfterAllocation, _ := suite.GetDelegationRewards(ctx, val, test.delegator) + suite.Require().NotNil(rewardsAfterAllocation) + suite.Require().NotZero(rewardsAfterAllocation[0].Amount) + } + } + } + + _, err := msgServer.WithdrawDelegationRewards(c, types.NewMsgWithdrawDelegationRewards(test.delegator)) + if test.expectPass { + suite.Require().NoError(err) + + for _, val := range preferences { + rewardAfterWithdraw, _ := suite.GetDelegationRewards(ctx, val, test.delegator) + suite.Require().Nil(rewardAfterWithdraw) + } + + } else { + suite.Require().Error(err) + } + }) + } +} diff --git a/x/valset-pref/types/expected_interfaces.go b/x/valset-pref/types/expected_interfaces.go index 8ea15f4708e..263b8a3351d 100644 --- a/x/valset-pref/types/expected_interfaces.go +++ b/x/valset-pref/types/expected_interfaces.go @@ -21,7 +21,8 @@ type BankKeeper interface { GetBalance(ctx sdk.Context, addr sdk.AccAddress, denom string) sdk.Coin } -type CommunityPoolKeeper interface { +// For testing only +type DistributionKeeper interface { WithdrawDelegationRewards(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) (sdk.Coins, error) IncrementValidatorPeriod(ctx sdk.Context, val stakingtypes.ValidatorI) uint64 CalculateDelegationRewards(ctx sdk.Context, val stakingtypes.ValidatorI, del stakingtypes.DelegationI, endingPeriod uint64) (rewards sdk.DecCoins) diff --git a/x/valset-pref/validator_set.go b/x/valset-pref/validator_set.go index 33db909bbae..5c95ea013d7 100644 --- a/x/valset-pref/validator_set.go +++ b/x/valset-pref/validator_set.go @@ -207,6 +207,36 @@ func (k Keeper) PreformRedelegation(ctx sdk.Context, delegator sdk.AccAddress, e return nil } +// WithdrawDelegationRewards withdraws all the delegation rewards from the validator in the val-set. +// Delegation reward is collected by the validator and in doing so, they can charge commission to the delegators. +// Rewards are calculated per period, and is updated each time validator delegation changes. For ex: when a delegator +// receives new delgation the rewards can be calculated by taking (total rewards before new delegation - the total current rewards). +func (k Keeper) WithdrawDelegationRewards(ctx sdk.Context, delegatorAddr string) error { + // get the existing validator set preference + existingSet, found := k.GetValidatorSetPreference(ctx, delegatorAddr) + if !found { + return fmt.Errorf("user %s doesn't have validator set", delegatorAddr) + } + + delegator, err := sdk.AccAddressFromBech32(delegatorAddr) + if err != nil { + return err + } + + for _, val := range existingSet.Preferences { + valAddr, err := sdk.ValAddressFromBech32(val.ValOperAddress) + if err != nil { + return fmt.Errorf("validator address not formatted") + } + + _, err = k.distirbutionKeeper.WithdrawDelegationRewards(ctx, delegator, valAddr) + if err != nil { + return nil + } + } + return nil +} + // GetValAddrAndVal checks if the validator address is valid and the validator provided exists on chain. func (k Keeper) getValAddrAndVal(ctx sdk.Context, valOperAddress string) (sdk.ValAddress, stakingtypes.Validator, error) { valAddr, err := sdk.ValAddressFromBech32(valOperAddress) From 0ee104ceb6ec67342e1c7de31ba9b5843a6608f6 Mon Sep 17 00:00:00 2001 From: stackman27 Date: Wed, 21 Dec 2022 12:27:22 -0800 Subject: [PATCH 5/7] nit --- x/valset-pref/msg_server_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/valset-pref/msg_server_test.go b/x/valset-pref/msg_server_test.go index f357163b5fb..bc8078d2c44 100644 --- a/x/valset-pref/msg_server_test.go +++ b/x/valset-pref/msg_server_test.go @@ -481,7 +481,7 @@ func (suite *KeeperTestSuite) TestWithdrawDelegationRewards() { expectPass bool }{ { - name: "Withdraw all rewards with existing delegation", + name: "Withdraw all rewards with existing valset delegations", delegator: sdk.AccAddress([]byte("addr1---------------")), coinToStake: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(20_000_000)), // delegate 20osmo setValSet: true, From f125b824431e341ce6fe003d73ad0dde4692ac4c Mon Sep 17 00:00:00 2001 From: stackman27 Date: Wed, 21 Dec 2022 19:10:51 -0800 Subject: [PATCH 6/7] added existing staking position --- x/valset-pref/keeper_test.go | 45 +++++++- x/valset-pref/msg_server_test.go | 116 +++++++++++++-------- x/valset-pref/types/expected_interfaces.go | 1 + x/valset-pref/validator_set.go | 61 +++++++++-- 4 files changed, 171 insertions(+), 52 deletions(-) diff --git a/x/valset-pref/keeper_test.go b/x/valset-pref/keeper_test.go index cfb25c68307..7a030da5504 100644 --- a/x/valset-pref/keeper_test.go +++ b/x/valset-pref/keeper_test.go @@ -42,8 +42,8 @@ func (suite *KeeperTestSuite) PrepareDelegateToValidatorSet() []types.ValidatorP return valPreferences } -func (suite *KeeperTestSuite) GetDelegationRewards(ctx sdk.Context, val types.ValidatorPreference, delegator sdk.AccAddress) (sdk.DecCoins, stakingtypes.Validator) { - valAddr, err := sdk.ValAddressFromBech32(val.ValOperAddress) +func (suite *KeeperTestSuite) GetDelegationRewards(ctx sdk.Context, valAddrStr string, delegator sdk.AccAddress) (sdk.DecCoins, stakingtypes.Validator) { + valAddr, err := sdk.ValAddressFromBech32(valAddrStr) suite.Require().NoError(err) validator, found := suite.App.StakingKeeper.GetValidator(ctx, valAddr) @@ -59,6 +59,47 @@ func (suite *KeeperTestSuite) GetDelegationRewards(ctx sdk.Context, val types.Va return rewards, validator } +func (suite *KeeperTestSuite) SetupExistingValidatorDelegations(ctx sdk.Context, valAddrStr string, delegator sdk.AccAddress, delegateAmt sdk.Int) { + valAddr, err := sdk.ValAddressFromBech32(valAddrStr) + suite.Require().NoError(err) + + validator, found := suite.App.StakingKeeper.GetValidator(ctx, valAddr) + suite.Require().True(found) + + _, err = suite.App.StakingKeeper.Delegate(ctx, delegator, delegateAmt, stakingtypes.Unbonded, validator, true) + suite.Require().NoError(err) + +} + +func (suite *KeeperTestSuite) SetupDelegationReward(ctx sdk.Context, delegator sdk.AccAddress, preferences []types.ValidatorPreference, existingValAddrStr string, setValSetDel, setExistingdel bool) { + // incrementing the blockheight by 1 for reward + ctx = suite.Ctx.WithBlockHeight(suite.Ctx.BlockHeight() + 1) + + if setValSetDel { + // only necessary if there are tokens delegated + for _, val := range preferences { + suite.AllocateRewards(ctx, delegator, val.ValOperAddress) + } + } + + if setExistingdel { + suite.AllocateRewards(ctx, delegator, existingValAddrStr) + } +} + +func (suite *KeeperTestSuite) AllocateRewards(ctx sdk.Context, delegator sdk.AccAddress, valAddrStr string) { + // check that there is enough reward to withdraw + _, validator := suite.GetDelegationRewards(ctx, valAddrStr, delegator) + + // allocate some rewards + tokens := sdk.NewDecCoins(sdk.NewInt64DecCoin(sdk.DefaultBondDenom, 10)) + suite.App.DistrKeeper.AllocateTokensToValidator(ctx, validator, tokens) + + rewardsAfterAllocation, _ := suite.GetDelegationRewards(ctx, valAddrStr, delegator) + suite.Require().NotNil(rewardsAfterAllocation) + suite.Require().NotZero(rewardsAfterAllocation[0].Amount) +} + func TestKeeperTestSuite(t *testing.T) { suite.Run(t, new(KeeperTestSuite)) } diff --git a/x/valset-pref/msg_server_test.go b/x/valset-pref/msg_server_test.go index bc8078d2c44..95197dcf931 100644 --- a/x/valset-pref/msg_server_test.go +++ b/x/valset-pref/msg_server_test.go @@ -473,33 +473,44 @@ func (suite *KeeperTestSuite) TestRedelegateValidatorSet() { func (suite *KeeperTestSuite) TestWithdrawDelegationRewards() { tests := []struct { - name string - delegator sdk.AccAddress - coinToStake sdk.Coin - setValSet bool - setDelegation bool - expectPass bool + name string + delegator sdk.AccAddress + coinsToDelegate sdk.Coin + setValSetDelegation bool + setStakingDelegation bool + expectPass bool }{ { - name: "Withdraw all rewards with existing valset delegations", - delegator: sdk.AccAddress([]byte("addr1---------------")), - coinToStake: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(20_000_000)), // delegate 20osmo - setValSet: true, - setDelegation: true, - expectPass: true, + name: "Withdraw all rewards with existing valset delegations, and existing staking position", + delegator: sdk.AccAddress([]byte("addr1---------------")), + coinsToDelegate: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(20_000_000)), // delegate 20osmo + setValSetDelegation: true, + setStakingDelegation: true, + expectPass: true, }, { - name: "Withdraw all rewards, val-set doesnot exist", - delegator: sdk.AccAddress([]byte("addr1---------------")), - coinToStake: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(20_000_000)), - expectPass: false, + name: "Withdraw all rewards with no existing valset delegation, but existing staking position", + delegator: sdk.AccAddress([]byte("addr1---------------")), + coinsToDelegate: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(20_000_000)), // delegate 20osmo + setValSetDelegation: false, + setStakingDelegation: true, + expectPass: true, }, { - name: "Withdraw all rewards, delegation doesnot exist", - delegator: sdk.AccAddress([]byte("addr1---------------")), - coinToStake: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(20_000_000)), - setDelegation: true, - expectPass: false, + name: "Withdraw all rewards with existing valset delegation, but no existing staking position", + delegator: sdk.AccAddress([]byte("addr1---------------")), + coinsToDelegate: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(20_000_000)), // delegate 20osmo + setValSetDelegation: true, + setStakingDelegation: false, + expectPass: true, + }, + { + name: "Withdraw all rewards with no existing valset delegation, no existing staking position", + delegator: sdk.AccAddress([]byte("addr1---------------")), + coinsToDelegate: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(20_000_000)), // delegate 20osmo + setValSetDelegation: false, + setStakingDelegation: false, + expectPass: false, }, } @@ -515,48 +526,65 @@ func (suite *KeeperTestSuite) TestWithdrawDelegationRewards() { // call the create validator set preference preferences := suite.PrepareDelegateToValidatorSet() + // setup extra validator for non val-set staking position + valAddrs := suite.SetupMultipleValidators(1) ctx := suite.Ctx - // delegators have to set val-set before delegating tokens - if test.setValSet { + // setup test for only valset delegation + if test.setValSetDelegation && !test.setStakingDelegation { + // delegators have to set val-set before delegating tokens _, err := msgServer.SetValidatorSetPreference(c, types.NewMsgSetValidatorSetPreference(test.delegator, preferences)) suite.Require().NoError(err) - if test.setDelegation { - // call the create validator set preference - _, err := msgServer.DelegateToValidatorSet(c, types.NewMsgDelegateToValidatorSet(test.delegator, test.coinToStake)) - suite.Require().NoError(err) + // call the delegate to validator set preference message + _, err = msgServer.DelegateToValidatorSet(c, types.NewMsgDelegateToValidatorSet(test.delegator, test.coinsToDelegate)) + suite.Require().NoError(err) - // incrementing the blockheight by 1 for reward - ctx = suite.Ctx.WithBlockHeight(suite.Ctx.BlockHeight() + 1) + suite.SetupDelegationReward(ctx, test.delegator, preferences, "", test.setValSetDelegation, test.setStakingDelegation) + } - // only necessary if there are tokens delegated - for _, val := range preferences { - // check that there is enough reward to withdraw - _, validator := suite.GetDelegationRewards(ctx, val, test.delegator) + // setup test for only existing staking position + if test.setStakingDelegation && !test.setValSetDelegation { + suite.SetupExistingValidatorDelegations(suite.Ctx, valAddrs[0], test.delegator, test.coinsToDelegate.Amount) - // allocate some rewards - tokens := sdk.NewDecCoins(sdk.NewInt64DecCoin(sdk.DefaultBondDenom, 10)) - suite.App.DistrKeeper.AllocateTokensToValidator(ctx, validator, tokens) + suite.SetupDelegationReward(ctx, test.delegator, nil, valAddrs[0], test.setValSetDelegation, test.setStakingDelegation) + } - rewardsAfterAllocation, _ := suite.GetDelegationRewards(ctx, val, test.delegator) - suite.Require().NotNil(rewardsAfterAllocation) - suite.Require().NotZero(rewardsAfterAllocation[0].Amount) - } - } + // setup test for existing staking position and valset delegation position + if test.setStakingDelegation && test.setValSetDelegation { + suite.SetupExistingValidatorDelegations(suite.Ctx, valAddrs[0], test.delegator, test.coinsToDelegate.Amount) + + // delegators have to set val-set before delegating tokens + _, err := msgServer.SetValidatorSetPreference(c, types.NewMsgSetValidatorSetPreference(test.delegator, preferences)) + suite.Require().NoError(err) + + // call the delegate to validator set preference message + _, err = msgServer.DelegateToValidatorSet(c, types.NewMsgDelegateToValidatorSet(test.delegator, test.coinsToDelegate)) + suite.Require().NoError(err) + + suite.SetupDelegationReward(ctx, test.delegator, preferences, valAddrs[0], test.setValSetDelegation, test.setStakingDelegation) } _, err := msgServer.WithdrawDelegationRewards(c, types.NewMsgWithdrawDelegationRewards(test.delegator)) if test.expectPass { suite.Require().NoError(err) - for _, val := range preferences { - rewardAfterWithdraw, _ := suite.GetDelegationRewards(ctx, val, test.delegator) - suite.Require().Nil(rewardAfterWithdraw) + // the rewards for valset and exising delegations should be nil + if test.setValSetDelegation { + for _, val := range preferences { + rewardAfterWithdrawValSet, _ := suite.GetDelegationRewards(ctx, val.ValOperAddress, test.delegator) + suite.Require().Nil(rewardAfterWithdrawValSet) + } + } + + if test.setStakingDelegation { + rewardAfterWithdrawExistingSet, _ := suite.GetDelegationRewards(ctx, valAddrs[0], test.delegator) + suite.Require().Nil(rewardAfterWithdrawExistingSet) } } else { suite.Require().Error(err) + } }) } diff --git a/x/valset-pref/types/expected_interfaces.go b/x/valset-pref/types/expected_interfaces.go index 263b8a3351d..3d9b65c90f9 100644 --- a/x/valset-pref/types/expected_interfaces.go +++ b/x/valset-pref/types/expected_interfaces.go @@ -15,6 +15,7 @@ type StakingInterface interface { GetDelegation(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) (delegation stakingtypes.Delegation, found bool) Undelegate(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress, sharesAmount sdk.Dec) (time.Time, error) BeginRedelegation(ctx sdk.Context, delAddr sdk.AccAddress, valSrcAddr, valDstAddr sdk.ValAddress, sharesAmount sdk.Dec) (completionTime time.Time, err error) + GetDelegatorDelegations(ctx sdk.Context, delegator sdk.AccAddress, maxRetrieve uint16) (delegations []stakingtypes.Delegation) } type BankKeeper interface { diff --git a/x/valset-pref/validator_set.go b/x/valset-pref/validator_set.go index 5c95ea013d7..14f9689d3ff 100644 --- a/x/valset-pref/validator_set.go +++ b/x/valset-pref/validator_set.go @@ -2,6 +2,7 @@ package keeper import ( "fmt" + "math" "sort" sdk "github.com/cosmos/cosmos-sdk/types" @@ -212,26 +213,74 @@ func (k Keeper) PreformRedelegation(ctx sdk.Context, delegator sdk.AccAddress, e // Rewards are calculated per period, and is updated each time validator delegation changes. For ex: when a delegator // receives new delgation the rewards can be calculated by taking (total rewards before new delegation - the total current rewards). func (k Keeper) WithdrawDelegationRewards(ctx sdk.Context, delegatorAddr string) error { + delegator, err := sdk.AccAddressFromBech32(delegatorAddr) + if err != nil { + return err + } + + // check if there is existing staking position that's not val-set + delegations := k.stakingKeeper.GetDelegatorDelegations(ctx, delegator, math.MaxUint16) + // get the existing validator set preference existingSet, found := k.GetValidatorSetPreference(ctx, delegatorAddr) - if !found { - return fmt.Errorf("user %s doesn't have validator set", delegatorAddr) + if !found && len(delegations) == 0 { + return fmt.Errorf("user %s doesn't have validator set or existing delegations", delegatorAddr) } - delegator, err := sdk.AccAddressFromBech32(delegatorAddr) + // there is existing staking position, but it's not valset + if !found && len(delegations) != 0 { + err := k.withdrawExistingStakingPosition(ctx, delegator, delegations) + if err != nil { + return err + } + return nil + } + + // there is no existing staking position, but there is val-set delegation + if found && len(delegations) == 0 { + err := k.withdrawExistingValSetStakingPosition(ctx, delegator, existingSet.Preferences) + if err != nil { + return err + } + return nil + } + + // there is staking position delegation, as well as val-set delegation + err = k.withdrawExistingStakingPosition(ctx, delegator, delegations) if err != nil { return err } - for _, val := range existingSet.Preferences { - valAddr, err := sdk.ValAddressFromBech32(val.ValOperAddress) + err = k.withdrawExistingValSetStakingPosition(ctx, delegator, existingSet.Preferences) + if err != nil { + return err + } + + return nil +} + +// withdrawExistingStakingPosition takes the existing staking delegator delegations and withdraws the rewards. +func (k Keeper) withdrawExistingStakingPosition(ctx sdk.Context, delegator sdk.AccAddress, delegations []stakingtypes.Delegation) error { + for _, dels := range delegations { + _, err := k.distirbutionKeeper.WithdrawDelegationRewards(ctx, delegator, dels.GetValidatorAddr()) + if err != nil { + return err + } + } + return nil +} + +// withdrawExistingValSetStakingPosition takes the existing valset delegator delegations and withdraws the rewards. +func (k Keeper) withdrawExistingValSetStakingPosition(ctx sdk.Context, delegator sdk.AccAddress, delegations []types.ValidatorPreference) error { + for _, dels := range delegations { + valAddr, err := sdk.ValAddressFromBech32(dels.ValOperAddress) if err != nil { return fmt.Errorf("validator address not formatted") } _, err = k.distirbutionKeeper.WithdrawDelegationRewards(ctx, delegator, valAddr) if err != nil { - return nil + return err } } return nil From 787644c4b7a3395e56bbbc576a20672ac72d6c1c Mon Sep 17 00:00:00 2001 From: stackman27 Date: Wed, 21 Dec 2022 19:17:28 -0800 Subject: [PATCH 7/7] nit --- x/valset-pref/msg_server_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x/valset-pref/msg_server_test.go b/x/valset-pref/msg_server_test.go index 95197dcf931..907dadb6f2e 100644 --- a/x/valset-pref/msg_server_test.go +++ b/x/valset-pref/msg_server_test.go @@ -490,7 +490,7 @@ func (suite *KeeperTestSuite) TestWithdrawDelegationRewards() { }, { name: "Withdraw all rewards with no existing valset delegation, but existing staking position", - delegator: sdk.AccAddress([]byte("addr1---------------")), + delegator: sdk.AccAddress([]byte("addr2---------------")), coinsToDelegate: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(20_000_000)), // delegate 20osmo setValSetDelegation: false, setStakingDelegation: true, @@ -498,7 +498,7 @@ func (suite *KeeperTestSuite) TestWithdrawDelegationRewards() { }, { name: "Withdraw all rewards with existing valset delegation, but no existing staking position", - delegator: sdk.AccAddress([]byte("addr1---------------")), + delegator: sdk.AccAddress([]byte("addr3---------------")), coinsToDelegate: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(20_000_000)), // delegate 20osmo setValSetDelegation: true, setStakingDelegation: false, @@ -506,7 +506,7 @@ func (suite *KeeperTestSuite) TestWithdrawDelegationRewards() { }, { name: "Withdraw all rewards with no existing valset delegation, no existing staking position", - delegator: sdk.AccAddress([]byte("addr1---------------")), + delegator: sdk.AccAddress([]byte("addr4---------------")), coinsToDelegate: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(20_000_000)), // delegate 20osmo setValSetDelegation: false, setStakingDelegation: false,