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.

Sign up for GitHub

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

Jump to bottom

fix: claim accured rewards first before adding delegation #334

Merged
merged 1 commit into from
Mar 25, 2024
Merged

fix: claim accured rewards first before adding delegation #334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions x/alliance/keeper/delegation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ func (k Keeper) Delegate(ctx context.Context, delAddr sdk.AccAddress, validator
if err != nil {
return nil, err
}
} else {
// Claim validator rewards first to ensure that we distribute accrued rewards to delegators before the new delegation
_, err = k.ClaimValidatorRewards(ctx, validator)
if err != nil {
return nil, err
}
}

// Create or update a delegation
Expand Down
38 changes: 37 additions & 1 deletion x/alliance/keeper/tests/delegation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"testing"
"time"

authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"

"cosmossdk.io/math"

test_helpers "github.com/terra-money/alliance/app"
Expand Down Expand Up @@ -616,6 +618,7 @@ func TestSuccessfulUndelegation(t *testing.T) {
types.NewAllianceAsset(AllianceDenomTwo, math.LegacyNewDec(10), math.LegacyNewDec(2), math.LegacyNewDec(12), math.LegacyNewDec(0), ctx.BlockTime()),
},
})
queryServer := keeper.NewQueryServerImpl(app.AllianceKeeper)
delegations, err := app.StakingKeeper.GetAllDelegations(ctx)
require.NoError(t, err)
require.Len(t, delegations, 1)
Expand All @@ -636,6 +639,12 @@ func TestSuccessfulUndelegation(t *testing.T) {
err = app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, delAddr, sdk.NewCoins(sdk.NewCoin(AllianceDenom, math.NewInt(2000_000))))
require.NoError(t, err)

// Mint reward tokens
err = app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, sdk.NewCoins(sdk.NewCoin("reward", math.NewInt(2000_000))))
require.NoError(t, err)
err = app.BankKeeper.SendCoinsFromModuleToModule(ctx, minttypes.ModuleName, authtypes.FeeCollectorName, sdk.NewCoins(sdk.NewCoin("reward", math.NewInt(2000_000))))
require.NoError(t, err)

// Delegate to a validator
_, err = app.AllianceKeeper.Delegate(ctx, delAddr, val, sdk.NewCoin(AllianceDenom, math.NewInt(1000_000)))
require.NoError(t, err)
Expand All @@ -661,15 +670,31 @@ func TestSuccessfulUndelegation(t *testing.T) {
Shares: math.LegacyNewDec(2),
}, d)

err = app.AllianceKeeper.AddAssetsToRewardPool(ctx, app.AccountKeeper.GetModuleAddress(authtypes.FeeCollectorName), val, sdk.NewCoins(sdk.NewCoin("reward", math.NewInt(1000_000))))
require.NoError(t, err)

rewards, err := queryServer.AllianceDelegationRewards(ctx, &types.QueryAllianceDelegationRewardsRequest{
DelegatorAddr: delAddr.String(),
ValidatorAddr: valAddr.String(),
Denom: AllianceDenom,
Pagination: nil,
})
require.NoError(t, err)
require.Equal(t, 1, len(rewards.Rewards))
rewardAmount := rewards.Rewards[0].Amount

// Immediately undelegate from the validator
_, err = app.AllianceKeeper.Undelegate(ctx, delAddr, val, sdk.NewCoin(AllianceDenom, math.NewInt(250_000)))
require.NoError(t, err)

_, err = app.AllianceKeeper.Undelegate(ctx, delAddr, val, sdk.NewCoin(AllianceDenom, math.NewInt(250_000)))
require.NoError(t, err)

// Check that rewards sent out
rewardBalance := app.BankKeeper.GetBalance(ctx, delAddr, "reward")
require.Equal(t, rewardAmount, rewardBalance.Amount)

// Query unbondings directly from the entry point
queryServer := keeper.NewQueryServerImpl(app.AllianceKeeper)
res, err := queryServer.AllianceUnbondingsByDelegator(ctx, &types.QueryAllianceUnbondingsByDelegatorRequest{
DelegatorAddr: delAddr.String(),
})
Expand Down Expand Up @@ -748,6 +773,17 @@ func TestSuccessfulUndelegation(t *testing.T) {
// Completing again should not process anymore undelegations
err = app.AllianceKeeper.CompleteUnbondings(ctx)
require.NoError(t, err)

q := keeper.NewQueryServerImpl(app.AllianceKeeper)
rewardRes, err := q.AllianceDelegationRewards(ctx, &types.QueryAllianceDelegationRewardsRequest{
DelegatorAddr: delAddr.String(),
ValidatorAddr: valAddr.String(),
Denom: AllianceDenom,
Pagination: nil,
})

require.NoError(t, err)
require.Equal(t, 0, len(rewardRes.Rewards))
}

func TestUndelegationWithoutDelegation(t *testing.T) {
Expand Down
90 changes: 90 additions & 0 deletions x/alliance/keeper/tests/reward_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1341,6 +1341,96 @@ func TestMigratedRewards(t *testing.T) {
require.Equal(t, math.NewInt(0), rewards4.AmountOf(bondDenom))
}

func TestRewardsDelegateBeforeValidatorClaim(t *testing.T) {
var err error
app, ctx := createTestContext(t)
ctx = ctx.WithBlockHeight(1)
app.AllianceKeeper.InitGenesis(ctx, &types.GenesisState{
Params: types.DefaultParams(),
Assets: []types.AllianceAsset{
types.NewAllianceAsset(AllianceDenom, math.LegacyNewDec(2), math.LegacyNewDec(0), math.LegacyNewDec(5), math.LegacyNewDec(0), ctx.BlockTime()),
types.NewAllianceAsset(AllianceDenomTwo, math.LegacyNewDec(8), math.LegacyNewDec(2), math.LegacyNewDec(12), math.LegacyNewDec(0), ctx.BlockTime()),
},
})

// Set tax and rewards to be zero for easier calculation
distParams, err := app.DistrKeeper.Params.Get(ctx)
require.NoError(t, err)
distParams.CommunityTax = math.LegacyZeroDec()
err = app.DistrKeeper.Params.Set(ctx, distParams)
require.NoError(t, err)

// Accounts
// mintPoolAddr := app.AccountKeeper.GetModuleAddress(minttypes.ModuleName)
bondDenom, err := app.StakingKeeper.BondDenom(ctx)
require.NoError(t, err)
addrs := test_helpers.AddTestAddrsIncremental(app, ctx, 4, sdk.NewCoins(
sdk.NewCoin(AllianceDenom, math.NewInt(10_000_000)),
sdk.NewCoin(AllianceDenomTwo, math.NewInt(10_000_000)),
))

pks := test_helpers.CreateTestPubKeys(2)

// Creating two validators with 0% commission
valAddr1 := sdk.ValAddress(addrs[0])
_val1 := teststaking.NewValidator(t, valAddr1, pks[0])
_val1.Commission = stakingtypes.Commission{
CommissionRates: stakingtypes.CommissionRates{
Rate: math.LegacyNewDec(0),
MaxRate: math.LegacyNewDec(0),
MaxChangeRate: math.LegacyNewDec(0),
},
UpdateTime: time.Now(),
}
test_helpers.RegisterNewValidator(t, app, ctx, _val1)
val1, _ := app.AllianceKeeper.GetAllianceValidator(ctx, valAddr1)

user1 := addrs[2]
user2 := addrs[3]

// Mint bond denom
err = app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, sdk.NewCoins(sdk.NewCoin(bondDenom, math.NewInt(40_000_000))))
require.NoError(t, err)

// New delegations
_, err = app.AllianceKeeper.Delegate(ctx, user2, val1, sdk.NewCoin(AllianceDenom, math.NewInt(1000_000)))
require.NoError(t, err)
assets := app.AllianceKeeper.GetAllAssets(ctx)
err = app.AllianceKeeper.RebalanceBondTokenWeights(ctx, assets)
require.NoError(t, err)

// Transfer to rewards to fee pool to be distributed
err = app.BankKeeper.SendCoinsFromModuleToModule(ctx, minttypes.ModuleName, authtypes.FeeCollectorName, sdk.NewCoins(sdk.NewCoin(bondDenom, math.NewInt(10_000_000))))
require.NoError(t, err)

ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1)
// Distribute in the next begin block
// At the next begin block, tokens will be distributed from the fee pool
cons1, _ := val1.GetConsAddr()
var votingPower int64 = 100
err = app.DistrKeeper.AllocateTokens(ctx, votingPower, []abcitypes.VoteInfo{
{
Validator: abcitypes.Validator{
Address: cons1,
Power: 100,
},
},
})
require.NoError(t, err)

// New delegations
_, err = app.AllianceKeeper.Delegate(ctx, user1, val1, sdk.NewCoin(AllianceDenom, math.NewInt(1000_000)))
require.NoError(t, err)

assets = app.AllianceKeeper.GetAllAssets(ctx)
err = app.AllianceKeeper.RebalanceBondTokenWeights(ctx, assets)
require.NoError(t, err)

rewards1, err := app.AllianceKeeper.ClaimDelegationRewards(ctx, user1, val1, AllianceDenom)
require.NoError(t, err)
require.Equal(t, math.NewInt(0), rewards1.AmountOf(bondDenom))
}

func TestClaimRewardsWithDifferentValidators(t *testing.T) {
var err error
app, ctx := createTestContext(t)
Expand Down
Loading