Fix: delete incentive reward factors on full withdraw/repay (#885)

* delete incentive reward index on repay/withdraw

* call hook on borrow in all cases

* additional types functionality

* extend tests to cover fix

* update naming convention in tests

* update test comment

* feat: add set difference unit tests

* clarify test names

Co-authored-by: karzak <kjydavis3@gmail.com>
This commit is contained in:
Denali Marsh 2021-03-24 00:28:03 +01:00 committed by GitHub
parent d601481b95
commit eb856b5a1f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 223 additions and 45 deletions

View File

@ -67,9 +67,7 @@ func (k Keeper) Repay(ctx sdk.Context, sender, owner sdk.AccAddress, coins sdk.C
}
// Call incentive hook
if !borrow.Amount.Empty() {
k.AfterBorrowModified(ctx, borrow)
}
k.AfterBorrowModified(ctx, borrow)
ctx.EventManager().EmitEvent(
sdk.NewEvent(

View File

@ -0,0 +1,29 @@
package keeper
import (
"testing"
"github.com/stretchr/testify/require"
)
func TestSetDiff(t *testing.T) {
tests := []struct {
name string
setA []string
setB []string
expected []string
}{
{"empty", []string{}, []string{}, []string(nil)},
{"diff equal sets", []string{"busd", "usdx"}, []string{"busd", "usdx"}, []string(nil)},
{"diff set empty", []string{"bnb", "ukava", "usdx"}, []string{}, []string{"bnb", "ukava", "usdx"}},
{"input set empty", []string{}, []string{"bnb", "ukava", "usdx"}, []string(nil)},
{"diff set with common elements", []string{"bnb", "btcb", "usdx", "xrpb"}, []string{"bnb", "usdx"}, []string{"btcb", "xrpb"}},
{"diff set with all common elements", []string{"bnb", "usdx"}, []string{"bnb", "btcb", "usdx", "xrpb"}, []string(nil)},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
require.Equal(t, tt.expected, setDifference(tt.setA, tt.setB))
})
}
}

View File

@ -427,51 +427,71 @@ func (k Keeper) UpdateHardSupplyIndexDenoms(ctx sdk.Context, deposit hardtypes.D
claim = types.NewHardLiquidityProviderClaim(deposit.Depositor, sdk.Coins{}, nil, nil, nil)
}
depositDenoms := getDenoms(deposit.Amount)
supplyRewardIndexDenoms := claim.SupplyRewardIndexes.GetCollateralTypes()
uniqueDepositDenoms := setDifference(depositDenoms, supplyRewardIndexDenoms)
uniqueSupplyRewardDenoms := setDifference(supplyRewardIndexDenoms, depositDenoms)
supplyRewardIndexes := claim.SupplyRewardIndexes
for _, coin := range deposit.Amount {
_, foundUserRewardIndexes := claim.SupplyRewardIndexes.GetRewardIndex(coin.Denom)
// Create a new multi-reward index in the claim for every new deposit denom
for _, denom := range uniqueDepositDenoms {
_, foundUserRewardIndexes := claim.SupplyRewardIndexes.GetRewardIndex(denom)
if !foundUserRewardIndexes {
globalSupplyRewardIndexes, foundGlobalSupplyRewardIndexes := k.GetHardSupplyRewardIndexes(ctx, coin.Denom)
globalSupplyRewardIndexes, foundGlobalSupplyRewardIndexes := k.GetHardSupplyRewardIndexes(ctx, denom)
var multiRewardIndex types.MultiRewardIndex
if foundGlobalSupplyRewardIndexes {
multiRewardIndex = types.NewMultiRewardIndex(coin.Denom, globalSupplyRewardIndexes)
multiRewardIndex = types.NewMultiRewardIndex(denom, globalSupplyRewardIndexes)
} else {
multiRewardIndex = types.NewMultiRewardIndex(coin.Denom, types.RewardIndexes{})
multiRewardIndex = types.NewMultiRewardIndex(denom, types.RewardIndexes{})
}
supplyRewardIndexes = append(supplyRewardIndexes, multiRewardIndex)
}
}
if len(supplyRewardIndexes) == 0 {
return
// Delete multi-reward index from claim if the collateral type is no longer deposited
for _, denom := range uniqueSupplyRewardDenoms {
supplyRewardIndexes = supplyRewardIndexes.RemoveRewardIndex(denom)
}
claim.SupplyRewardIndexes = supplyRewardIndexes
k.SetHardLiquidityProviderClaim(ctx, claim)
}
// UpdateHardBorrowIndexDenoms adds any new borrow denoms to the claim's supply reward index
// UpdateHardBorrowIndexDenoms adds any new borrow denoms to the claim's borrow reward index
func (k Keeper) UpdateHardBorrowIndexDenoms(ctx sdk.Context, borrow hardtypes.Borrow) {
claim, found := k.GetHardLiquidityProviderClaim(ctx, borrow.Borrower)
if !found {
claim = types.NewHardLiquidityProviderClaim(borrow.Borrower, sdk.Coins{}, nil, nil, nil)
}
borrowDenoms := getDenoms(borrow.Amount)
borrowRewardIndexDenoms := claim.BorrowRewardIndexes.GetCollateralTypes()
uniqueBorrowDenoms := setDifference(borrowDenoms, borrowRewardIndexDenoms)
uniqueBorrowRewardDenoms := setDifference(borrowRewardIndexDenoms, borrowDenoms)
borrowRewardIndexes := claim.BorrowRewardIndexes
for _, coin := range borrow.Amount {
_, foundUserRewardIndexes := claim.BorrowRewardIndexes.GetRewardIndex(coin.Denom)
// Create a new multi-reward index in the claim for every new borrow denom
for _, denom := range uniqueBorrowDenoms {
_, foundUserRewardIndexes := claim.BorrowRewardIndexes.GetRewardIndex(denom)
if !foundUserRewardIndexes {
globalBorrowRewardIndexes, foundGlobalBorrowRewardIndexes := k.GetHardBorrowRewardIndexes(ctx, coin.Denom)
globalBorrowRewardIndexes, foundGlobalBorrowRewardIndexes := k.GetHardBorrowRewardIndexes(ctx, denom)
var multiRewardIndex types.MultiRewardIndex
if foundGlobalBorrowRewardIndexes {
multiRewardIndex = types.NewMultiRewardIndex(coin.Denom, globalBorrowRewardIndexes)
multiRewardIndex = types.NewMultiRewardIndex(denom, globalBorrowRewardIndexes)
} else {
multiRewardIndex = types.NewMultiRewardIndex(coin.Denom, types.RewardIndexes{})
multiRewardIndex = types.NewMultiRewardIndex(denom, types.RewardIndexes{})
}
borrowRewardIndexes = append(borrowRewardIndexes, multiRewardIndex)
}
}
if len(borrowRewardIndexes) == 0 {
return
// Delete multi-reward index from claim if the collateral type is no longer borrowed
for _, denom := range uniqueBorrowRewardDenoms {
borrowRewardIndexes = borrowRewardIndexes.RemoveRewardIndex(denom)
}
claim.BorrowRewardIndexes = borrowRewardIndexes
k.SetHardLiquidityProviderClaim(ctx, claim)
}
@ -870,3 +890,27 @@ func (k Keeper) SimulateUSDXMintingSynchronization(ctx sdk.Context, claim types.
return claim
}
// Set setDifference: A - B
func setDifference(a, b []string) (diff []string) {
m := make(map[string]bool)
for _, item := range b {
m[item] = true
}
for _, item := range a {
if _, ok := m[item]; !ok {
diff = append(diff, item)
}
}
return
}
func getDenoms(coins sdk.Coins) []string {
denoms := []string{}
for _, coin := range coins {
denoms = append(denoms, coin.Denom)
}
return denoms
}

View File

@ -1702,9 +1702,14 @@ func (suite *KeeperTestSuite) TestSynchronizeHardSupplyReward() {
}
func (suite *KeeperTestSuite) TestUpdateHardSupplyIndexDenoms() {
type depositModification struct {
coins sdk.Coins
withdraw bool
}
type args struct {
firstDeposit sdk.Coins
secondDeposit sdk.Coins
modification depositModification
rewardsPerSecond sdk.Coins
initialTime time.Time
expectedSupplyIndexDenoms []string
@ -1719,7 +1724,7 @@ func (suite *KeeperTestSuite) TestUpdateHardSupplyIndexDenoms() {
"single reward denom: update adds one supply reward index",
args{
firstDeposit: cs(c("bnb", 10000000000)),
secondDeposit: cs(c("ukava", 10000000000)),
modification: depositModification{coins: cs(c("ukava", 10000000000))},
rewardsPerSecond: cs(c("hard", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedSupplyIndexDenoms: []string{"bnb", "ukava"},
@ -1729,7 +1734,7 @@ func (suite *KeeperTestSuite) TestUpdateHardSupplyIndexDenoms() {
"single reward denom: update adds multiple supply reward indexes",
args{
firstDeposit: cs(c("bnb", 10000000000)),
secondDeposit: cs(c("ukava", 10000000000), c("btcb", 10000000000), c("xrp", 10000000000)),
modification: depositModification{coins: cs(c("ukava", 10000000000), c("btcb", 10000000000), c("xrp", 10000000000))},
rewardsPerSecond: cs(c("hard", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedSupplyIndexDenoms: []string{"bnb", "ukava", "btcb", "xrp"},
@ -1739,7 +1744,7 @@ func (suite *KeeperTestSuite) TestUpdateHardSupplyIndexDenoms() {
"single reward denom: update doesn't add duplicate supply reward index for same denom",
args{
firstDeposit: cs(c("bnb", 10000000000)),
secondDeposit: cs(c("bnb", 5000000000)),
modification: depositModification{coins: cs(c("bnb", 5000000000))},
rewardsPerSecond: cs(c("hard", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedSupplyIndexDenoms: []string{"bnb"},
@ -1749,7 +1754,7 @@ func (suite *KeeperTestSuite) TestUpdateHardSupplyIndexDenoms() {
"multiple reward denoms: update adds one supply reward index",
args{
firstDeposit: cs(c("bnb", 10000000000)),
secondDeposit: cs(c("ukava", 10000000000)),
modification: depositModification{coins: cs(c("ukava", 10000000000))},
rewardsPerSecond: cs(c("hard", 122354), c("ukava", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedSupplyIndexDenoms: []string{"bnb", "ukava"},
@ -1759,7 +1764,7 @@ func (suite *KeeperTestSuite) TestUpdateHardSupplyIndexDenoms() {
"multiple reward denoms: update adds multiple supply reward indexes",
args{
firstDeposit: cs(c("bnb", 10000000000)),
secondDeposit: cs(c("ukava", 10000000000), c("btcb", 10000000000), c("xrp", 10000000000)),
modification: depositModification{coins: cs(c("ukava", 10000000000), c("btcb", 10000000000), c("xrp", 10000000000))},
rewardsPerSecond: cs(c("hard", 122354), c("ukava", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedSupplyIndexDenoms: []string{"bnb", "ukava", "btcb", "xrp"},
@ -1769,12 +1774,42 @@ func (suite *KeeperTestSuite) TestUpdateHardSupplyIndexDenoms() {
"multiple reward denoms: update doesn't add duplicate supply reward index for same denom",
args{
firstDeposit: cs(c("bnb", 10000000000)),
secondDeposit: cs(c("bnb", 5000000000)),
modification: depositModification{coins: cs(c("bnb", 5000000000))},
rewardsPerSecond: cs(c("hard", 122354), c("ukava", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedSupplyIndexDenoms: []string{"bnb"},
},
},
{
"single reward denom: fully withdrawing a denom deletes the denom's supply reward index",
args{
firstDeposit: cs(c("bnb", 1000000000)),
modification: depositModification{coins: cs(c("bnb", 1100000000)), withdraw: true},
rewardsPerSecond: cs(c("hard", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedSupplyIndexDenoms: []string{},
},
},
{
"single reward denom: fully withdrawing a denom deletes only the denom's supply reward index",
args{
firstDeposit: cs(c("bnb", 1000000000), c("ukava", 100000000)),
modification: depositModification{coins: cs(c("bnb", 1100000000)), withdraw: true},
rewardsPerSecond: cs(c("hard", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedSupplyIndexDenoms: []string{"ukava"},
},
},
{
"multiple reward denoms: fully repaying a denom deletes the denom's supply reward index",
args{
firstDeposit: cs(c("bnb", 1000000000)),
modification: depositModification{coins: cs(c("bnb", 1100000000)), withdraw: true},
rewardsPerSecond: cs(c("hard", 122354), c("ukava", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedSupplyIndexDenoms: []string{},
},
},
}
for _, tc := range testCases {
suite.Run(tc.name, func() {
@ -1833,18 +1868,22 @@ func (suite *KeeperTestSuite) TestUpdateHardSupplyIndexDenoms() {
}
suite.Require().True(len(claimAfterFirstDeposit.SupplyRewardIndexes) == len(tc.args.firstDeposit))
// User deposits (second time)
err = hardKeeper.Deposit(suite.ctx, userAddr, tc.args.secondDeposit)
// User modifies their Deposit by withdrawing or depositing more
if tc.args.modification.withdraw {
err = hardKeeper.Withdraw(suite.ctx, userAddr, tc.args.modification.coins)
} else {
err = hardKeeper.Deposit(suite.ctx, userAddr, tc.args.modification.coins)
}
suite.Require().NoError(err)
// Confirm that the claim contains all expected supply indexes
claimAfterSecondDeposit, found := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, suite.addrs[3])
claimAfterModification, found := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, suite.addrs[3])
suite.Require().True(found)
for _, denom := range tc.args.expectedSupplyIndexDenoms {
_, hasIndex := claimAfterSecondDeposit.HasSupplyRewardIndex(denom)
_, hasIndex := claimAfterModification.HasSupplyRewardIndex(denom)
suite.Require().True(hasIndex)
}
suite.Require().True(len(claimAfterSecondDeposit.SupplyRewardIndexes) == len(tc.args.expectedSupplyIndexDenoms))
suite.Require().True(len(claimAfterModification.SupplyRewardIndexes) == len(tc.args.expectedSupplyIndexDenoms))
})
}
}
@ -2036,10 +2075,15 @@ func (suite *KeeperTestSuite) TestInitializeHardBorrowRewards() {
}
func (suite *KeeperTestSuite) TestUpdateHardBorrowIndexDenoms() {
type withdrawModification struct {
coins sdk.Coins
repay bool
}
type args struct {
initialDeposit sdk.Coins
firstBorrow sdk.Coins
secondBorrow sdk.Coins
modification withdrawModification
rewardsPerSecond sdk.Coins
initialTime time.Time
expectedBorrowIndexDenoms []string
@ -2055,7 +2099,7 @@ func (suite *KeeperTestSuite) TestUpdateHardBorrowIndexDenoms() {
args{
initialDeposit: cs(c("bnb", 10000000000)),
firstBorrow: cs(c("bnb", 50000000)),
secondBorrow: cs(c("ukava", 500000000)),
modification: withdrawModification{coins: cs(c("ukava", 500000000))},
rewardsPerSecond: cs(c("hard", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedBorrowIndexDenoms: []string{"bnb", "ukava"},
@ -2066,7 +2110,7 @@ func (suite *KeeperTestSuite) TestUpdateHardBorrowIndexDenoms() {
args{
initialDeposit: cs(c("btcb", 10000000000)),
firstBorrow: cs(c("btcb", 50000000)),
secondBorrow: cs(c("ukava", 500000000), c("bnb", 50000000000), c("xrp", 50000000000)),
modification: withdrawModification{coins: cs(c("ukava", 500000000), c("bnb", 50000000000), c("xrp", 50000000000))},
rewardsPerSecond: cs(c("hard", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedBorrowIndexDenoms: []string{"btcb", "ukava", "bnb", "xrp"},
@ -2077,7 +2121,7 @@ func (suite *KeeperTestSuite) TestUpdateHardBorrowIndexDenoms() {
args{
initialDeposit: cs(c("bnb", 100000000000)),
firstBorrow: cs(c("bnb", 50000000)),
secondBorrow: cs(c("bnb", 50000000000)),
modification: withdrawModification{coins: cs(c("bnb", 50000000000))},
rewardsPerSecond: cs(c("hard", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedBorrowIndexDenoms: []string{"bnb"},
@ -2088,7 +2132,7 @@ func (suite *KeeperTestSuite) TestUpdateHardBorrowIndexDenoms() {
args{
initialDeposit: cs(c("bnb", 10000000000)),
firstBorrow: cs(c("bnb", 50000000)),
secondBorrow: cs(c("ukava", 500000000)),
modification: withdrawModification{coins: cs(c("ukava", 500000000))},
rewardsPerSecond: cs(c("hard", 122354), c("ukava", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedBorrowIndexDenoms: []string{"bnb", "ukava"},
@ -2099,7 +2143,7 @@ func (suite *KeeperTestSuite) TestUpdateHardBorrowIndexDenoms() {
args{
initialDeposit: cs(c("btcb", 10000000000)),
firstBorrow: cs(c("btcb", 50000000)),
secondBorrow: cs(c("ukava", 500000000), c("bnb", 50000000000), c("xrp", 50000000000)),
modification: withdrawModification{coins: cs(c("ukava", 500000000), c("bnb", 50000000000), c("xrp", 50000000000))},
rewardsPerSecond: cs(c("hard", 122354), c("ukava", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedBorrowIndexDenoms: []string{"btcb", "ukava", "bnb", "xrp"},
@ -2110,12 +2154,45 @@ func (suite *KeeperTestSuite) TestUpdateHardBorrowIndexDenoms() {
args{
initialDeposit: cs(c("bnb", 100000000000)),
firstBorrow: cs(c("bnb", 50000000)),
secondBorrow: cs(c("bnb", 50000000000)),
modification: withdrawModification{coins: cs(c("bnb", 50000000000))},
rewardsPerSecond: cs(c("hard", 122354), c("ukava", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedBorrowIndexDenoms: []string{"bnb"},
},
},
{
"single reward denom: fully repaying a denom deletes the denom's supply reward index",
args{
initialDeposit: cs(c("bnb", 1000000000)),
firstBorrow: cs(c("bnb", 100000000)),
modification: withdrawModification{coins: cs(c("bnb", 1100000000)), repay: true},
rewardsPerSecond: cs(c("hard", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedBorrowIndexDenoms: []string{},
},
},
{
"single reward denom: fully repaying a denom deletes only the denom's supply reward index",
args{
initialDeposit: cs(c("bnb", 1000000000)),
firstBorrow: cs(c("bnb", 100000000), c("ukava", 10000000)),
modification: withdrawModification{coins: cs(c("bnb", 1100000000)), repay: true},
rewardsPerSecond: cs(c("hard", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedBorrowIndexDenoms: []string{"ukava"},
},
},
{
"multiple reward denoms: fully repaying a denom deletes the denom's supply reward index",
args{
initialDeposit: cs(c("bnb", 1000000000)),
firstBorrow: cs(c("bnb", 100000000), c("ukava", 10000000)),
modification: withdrawModification{coins: cs(c("bnb", 1100000000)), repay: true},
rewardsPerSecond: cs(c("hard", 122354), c("ukava", 122354)),
initialTime: time.Date(2020, 12, 15, 14, 0, 0, 0, time.UTC),
expectedBorrowIndexDenoms: []string{"ukava"},
},
},
}
for _, tc := range testCases {
suite.Run(tc.name, func() {
@ -2124,7 +2201,7 @@ func (suite *KeeperTestSuite) TestUpdateHardBorrowIndexDenoms() {
// Mint coins to hard module account so it can service borrow requests
supplyKeeper := suite.app.GetSupplyKeeper()
hardMaccCoins := tc.args.firstBorrow.Add(tc.args.secondBorrow...)
hardMaccCoins := tc.args.firstBorrow.Add(tc.args.modification.coins...)
supplyKeeper.MintCoins(suite.ctx, hardtypes.ModuleAccountName, hardMaccCoins)
// Set up generic reward periods
@ -2183,18 +2260,29 @@ func (suite *KeeperTestSuite) TestUpdateHardBorrowIndexDenoms() {
}
suite.Require().True(len(claimAfterFirstBorrow.BorrowRewardIndexes) == len(tc.args.firstBorrow))
// User borrows (second time)
err = hardKeeper.Borrow(suite.ctx, userAddr, tc.args.secondBorrow)
// User modifies their Borrow by either repaying or borrowing more
if tc.args.modification.repay {
err = hardKeeper.Repay(suite.ctx, userAddr, userAddr, tc.args.modification.coins)
} else {
err = hardKeeper.Borrow(suite.ctx, userAddr, tc.args.modification.coins)
}
suite.Require().NoError(err)
// Confirm that claim's borrow reward indexes contain expected values
claimAfterSecondBorrow, found := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, suite.addrs[3])
claimAfterModification, found := suite.keeper.GetHardLiquidityProviderClaim(suite.ctx, suite.addrs[3])
suite.Require().True(found)
for _, coin := range tc.args.secondBorrow {
_, hasIndex := claimAfterSecondBorrow.HasBorrowRewardIndex(coin.Denom)
suite.Require().True(hasIndex)
for _, coin := range tc.args.modification.coins {
_, hasIndex := claimAfterModification.HasBorrowRewardIndex(coin.Denom)
if tc.args.modification.repay {
// Only false if denom is repaid in full
if tc.args.modification.coins.AmountOf(coin.Denom).GTE(tc.args.firstBorrow.AmountOf(coin.Denom)) {
suite.Require().False(hasIndex)
}
} else {
suite.Require().True(hasIndex)
}
}
suite.Require().True(len(claimAfterSecondBorrow.BorrowRewardIndexes) == len(tc.args.expectedBorrowIndexDenoms))
suite.Require().True(len(claimAfterModification.BorrowRewardIndexes) == len(tc.args.expectedBorrowIndexDenoms))
})
}
}

View File

@ -486,6 +486,25 @@ func (mris MultiRewardIndexes) GetRewardIndexIndex(denom string) (int, bool) {
return -1, false
}
// GetCollateralTypes returns a slice of containing all collateral types
func (mris MultiRewardIndexes) GetCollateralTypes() []string {
var collateralTypes []string
for _, ri := range mris {
collateralTypes = append(collateralTypes, ri.CollateralType)
}
return collateralTypes
}
// RemoveRewardIndex removes a denom's reward interest factor value
func (mris MultiRewardIndexes) RemoveRewardIndex(denom string) MultiRewardIndexes {
for i, ri := range mris {
if ri.CollateralType == denom {
return append(mris[:i], mris[i+1:]...)
}
}
return mris
}
// Validate validation for reward indexes
func (mris MultiRewardIndexes) Validate() error {
for _, mri := range mris {