From d601481b95d3e65f7b9249f64db22e15367a77df Mon Sep 17 00:00:00 2001 From: Ruaridh Date: Tue, 23 Mar 2021 04:31:21 +0000 Subject: [PATCH] Fix index sync when deposits/borrows are small (#886) * sync indexes when deposit is small * add test for borrow index sync when zero rewards * update test for borrow index updates * fix synchronize hard supply reward Co-authored-by: denalimarsh --- x/incentive/keeper/integration_test.go | 2 +- x/incentive/keeper/payout_test.go | 5 +-- x/incentive/keeper/rewards.go | 27 +++++++-------- x/incentive/keeper/rewards_test.go | 47 ++++++++++++++++++++++++-- 4 files changed, 61 insertions(+), 20 deletions(-) diff --git a/x/incentive/keeper/integration_test.go b/x/incentive/keeper/integration_test.go index 1381a7e7..b2294682 100644 --- a/x/incentive/keeper/integration_test.go +++ b/x/incentive/keeper/integration_test.go @@ -222,7 +222,7 @@ func (suite *KeeperTestSuite) SetupWithGenState() { ctx := tApp.NewContext(true, abci.Header{Height: 1, Time: tmtime.Now()}) tApp.InitializeFromGenesisStates( - NewAuthGenState(allAddrs, cs(c("ukava", 5_000_000))), + NewAuthGenState(allAddrs, cs(c("ukava", 1_000_000_000))), NewStakingGenesisState(), NewPricefeedGenStateMulti(), NewCDPGenStateMulti(), diff --git a/x/incentive/keeper/payout_test.go b/x/incentive/keeper/payout_test.go index cb58489d..188fa965 100644 --- a/x/incentive/keeper/payout_test.go +++ b/x/incentive/keeper/payout_test.go @@ -11,6 +11,7 @@ import ( abci "github.com/tendermint/tendermint/abci/types" "github.com/cosmos/cosmos-sdk/x/auth" + "github.com/kava-labs/kava/app" cdptypes "github.com/kava-labs/kava/x/cdp/types" "github.com/kava-labs/kava/x/hard" @@ -54,7 +55,7 @@ func (suite *KeeperTestSuite) TestPayoutUSDXMintingClaim() { multipliers: types.Multipliers{types.NewMultiplier(types.MultiplierName("small"), 1, d("0.25")), types.NewMultiplier(types.MultiplierName("large"), 12, d("1.0"))}, multiplier: types.MultiplierName("large"), timeElapsed: 86400, - expectedBalance: cs(c("usdx", 10000000000), c("ukava", 10576385600)), + expectedBalance: cs(c("usdx", 10000000000), c("ukava", 11571385600)), expectedPeriods: vesting.Periods{vesting.Period{Length: 32918400, Amount: cs(c("ukava", 10571385600))}}, isPeriodicVestingAccount: true, }, @@ -135,7 +136,7 @@ func (suite *KeeperTestSuite) TestPayoutUSDXMintingClaim() { suite.Require().NoError(err) ak := suite.app.GetAccountKeeper() acc := ak.GetAccount(suite.ctx, suite.addrs[0]) - suite.Require().Equal(tc.args.expectedBalance, acc.GetCoins()) + suite.Require().Equal(tc.args.expectedBalance, acc.GetCoins()) // TODO check balance change to decouple from initialized account balance. if tc.args.isPeriodicVestingAccount { vacc, ok := acc.(*vesting.PeriodicVestingAccount) diff --git a/x/incentive/keeper/rewards.go b/x/incentive/keeper/rewards.go index 8aeba890..f6c3267b 100644 --- a/x/incentive/keeper/rewards.go +++ b/x/incentive/keeper/rewards.go @@ -1,6 +1,7 @@ package keeper import ( + "fmt" "math" "time" @@ -252,7 +253,6 @@ func (k Keeper) SynchronizeUSDXMintingReward(ctx sdk.Context, cdp cdptypes.CDP) newRewardsCoin := sdk.NewCoin(types.USDXMintingRewardDenom, newRewardsAmount) claim.Reward = claim.Reward.Add(newRewardsCoin) k.SetUSDXMintingClaim(ctx, claim) - return } // InitializeHardSupplyReward initializes the supply-side of a hard liquidity provider claim @@ -321,19 +321,18 @@ func (k Keeper) SynchronizeHardSupplyReward(ctx sdk.Context, deposit hardtypes.D globalRewardFactor := globalRewardIndex.RewardFactor userRewardFactor := userRewardIndex.RewardFactor rewardsAccumulatedFactor := globalRewardFactor.Sub(userRewardFactor) - if rewardsAccumulatedFactor.IsZero() { - continue - } - newRewardsAmount := rewardsAccumulatedFactor.Mul(deposit.Amount.AmountOf(coin.Denom).ToDec()).RoundInt() - if newRewardsAmount.IsZero() || newRewardsAmount.IsNegative() { - continue + if rewardsAccumulatedFactor.IsNegative() { + panic(fmt.Sprintf("reward accumulation factor cannot be negative: %s", rewardsAccumulatedFactor)) } + newRewardsAmount := rewardsAccumulatedFactor.Mul(deposit.Amount.AmountOf(coin.Denom).ToDec()).RoundInt() + factorIndex, foundFactorIndex := userMultiRewardIndex.RewardIndexes.GetFactorIndex(globalRewardIndex.CollateralType) - if !foundFactorIndex { + if !foundFactorIndex { // should never trigger, as we basically do this check at the start of this loop continue } claim.SupplyRewardIndexes[userRewardIndexIndex].RewardIndexes[factorIndex].RewardFactor = globalRewardIndex.RewardFactor + newRewardsCoin := sdk.NewCoin(userRewardIndex.CollateralType, newRewardsAmount) claim.Reward = claim.Reward.Add(newRewardsCoin) } @@ -403,16 +402,14 @@ func (k Keeper) SynchronizeHardBorrowReward(ctx sdk.Context, borrow hardtypes.Bo globalRewardFactor := globalRewardIndex.RewardFactor userRewardFactor := userRewardIndex.RewardFactor rewardsAccumulatedFactor := globalRewardFactor.Sub(userRewardFactor) - if rewardsAccumulatedFactor.IsZero() { - continue - } - newRewardsAmount := rewardsAccumulatedFactor.Mul(borrow.Amount.AmountOf(coin.Denom).ToDec()).RoundInt() - if newRewardsAmount.IsZero() || newRewardsAmount.IsNegative() { - continue + if rewardsAccumulatedFactor.IsNegative() { + panic(fmt.Sprintf("reward accumulation factor cannot be negative: %s", rewardsAccumulatedFactor)) } + newRewardsAmount := rewardsAccumulatedFactor.Mul(borrow.Amount.AmountOf(coin.Denom).ToDec()).RoundInt() + factorIndex, foundFactorIndex := userMultiRewardIndex.RewardIndexes.GetFactorIndex(globalRewardIndex.CollateralType) - if !foundFactorIndex { + if !foundFactorIndex { // should never trigger continue } claim.BorrowRewardIndexes[userRewardIndexIndex].RewardIndexes[factorIndex].RewardFactor = globalRewardIndex.RewardFactor diff --git a/x/incentive/keeper/rewards_test.go b/x/incentive/keeper/rewards_test.go index 508a0479..daa16163 100644 --- a/x/incentive/keeper/rewards_test.go +++ b/x/incentive/keeper/rewards_test.go @@ -391,7 +391,7 @@ func (suite *KeeperTestSuite) TestSynchronizeHardBorrowReward() { testCases := []test{ { - "10 blocks", + "single reward denom: 10 blocks", args{ incentiveBorrowRewardDenom: "bnb", borrow: c("bnb", 10000000000), @@ -404,7 +404,7 @@ func (suite *KeeperTestSuite) TestSynchronizeHardBorrowReward() { }, }, { - "10 blocks - long block time", + "single reward denom: 10 blocks - long block time", args{ incentiveBorrowRewardDenom: "bnb", borrow: c("bnb", 10000000000), @@ -415,6 +415,19 @@ func (suite *KeeperTestSuite) TestSynchronizeHardBorrowReward() { expectedRewards: cs(c("hard", 105713856031)), }, }, + { + "single reward denom: user reward index updated when reward is zero", + args{ + incentiveBorrowRewardDenom: "ukava", + borrow: c("ukava", 1), // borrow a tiny amount so that rewards round to zero + rewardsPerSecond: cs(c("hard", 122354)), + initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC), + blockTimes: []int{10, 10, 10, 10, 10, 10, 10, 10, 10, 10}, + expectedRewardIndexes: types.RewardIndexes{types.NewRewardIndex("hard", d("0.122354003908172328"))}, + expectedRewards: cs(), + updateRewardsViaCommmittee: false, + }, + }, { "multiple reward denoms: 10 blocks", args{ @@ -602,6 +615,18 @@ func (suite *KeeperTestSuite) TestSynchronizeHardBorrowReward() { suite.hardKeeper.SetSupplyInterestFactor(suite.ctx, tc.args.borrow.Denom, sdk.MustNewDecFromStr("1.0")) suite.hardKeeper.SetBorrowInterestFactor(suite.ctx, tc.args.borrow.Denom, sdk.MustNewDecFromStr("1.0")) suite.hardKeeper.SetPreviousAccrualTime(suite.ctx, tc.args.borrow.Denom, tc.args.initialTime) + // Set the minimum borrow to 0 to allow testing small borrows + hardParams := suite.hardKeeper.GetParams(suite.ctx) + hardParams.MinimumBorrowUSDValue = sdk.ZeroDec() + suite.hardKeeper.SetParams(suite.ctx, hardParams) + + // Borrow a fixed amount from another user to dilute primary user's rewards per second. + suite.Require().NoError( + suite.hardKeeper.Deposit(suite.ctx, suite.addrs[2], cs(c("ukava", 200_000_000))), + ) + suite.Require().NoError( + suite.hardKeeper.Borrow(suite.ctx, suite.addrs[2], cs(c("ukava", 100_000_000))), + ) // User deposits and borrows to increase total borrowed amount hardKeeper := suite.app.GetHardKeeper() @@ -1284,6 +1309,19 @@ func (suite *KeeperTestSuite) TestSynchronizeHardSupplyReward() { updateRewardsViaCommmittee: false, }, }, + { + "single reward denom: user reward index updated when reward is zero", + args{ + incentiveSupplyRewardDenom: "ukava", + deposit: c("ukava", 1), + rewardsPerSecond: cs(c("hard", 122354)), + initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC), + blockTimes: []int{10, 10, 10, 10, 10, 10, 10, 10, 10, 10}, + expectedRewardIndexes: types.RewardIndexes{types.NewRewardIndex("hard", d("0.122353998776460010"))}, + expectedRewards: cs(), + updateRewardsViaCommmittee: false, + }, + }, { "multiple reward denoms: 10 blocks", args{ @@ -1475,6 +1513,11 @@ func (suite *KeeperTestSuite) TestSynchronizeHardSupplyReward() { suite.hardKeeper.SetBorrowInterestFactor(suite.ctx, tc.args.incentiveSupplyRewardDenom, sdk.MustNewDecFromStr("1.0")) suite.hardKeeper.SetPreviousAccrualTime(suite.ctx, tc.args.incentiveSupplyRewardDenom, tc.args.initialTime) + // Deposit a fixed amount from another user to dilute primary user's rewards per second. + suite.Require().NoError( + suite.hardKeeper.Deposit(suite.ctx, suite.addrs[2], cs(c("ukava", 100_000_000))), + ) + // User deposits and borrows to increase total borrowed amount hardKeeper := suite.app.GetHardKeeper() userAddr := suite.addrs[3]