fix: decrement coins for supply/borrow properly (#862)

* fix: decrement coins for supply/borrow properly

* fix: decrement keeper liquidation reward coins from hard total supply (#865)

* fix: use proper safe subtraction for withdraw/repay methods

* fix: decrement keeper liquidation rewards from total supply

* address review comments

x/hard/keeper/borrow.go

@@ -238,15 +238,19 @@ func (k Keeper) DecrementBorrowedCoins(ctx sdk.Context, coins sdk.Coins) error {
 		return sdkerrors.Wrapf(types.ErrBorrowedCoinsNotFound, "cannot repay coins if no coins are currently borrowed")
 	}
 
-	updatedBorrowedCoins := sdk.NewCoins()
+	updatedBorrowedCoins, isNegative := borrowedCoins.SafeSub(coins)
+	if isNegative {
+		coinsToSubtract := sdk.NewCoins()
 		for _, coin := range coins {
-		// If amount is greater than total borrowed amount due to rounding, set total borrowed amount to 0
+			if borrowedCoins.AmountOf(coin.Denom).LT(coin.Amount) {
-		// by skipping the coin such that it's not included in the updatedBorrowedCoins object
+				if borrowedCoins.AmountOf(coin.Denom).GT(sdk.ZeroInt()) {
-		if coin.Amount.GTE(borrowedCoins.AmountOf(coin.Denom)) {
+					coinsToSubtract = coinsToSubtract.Add(sdk.NewCoin(coin.Denom, borrowedCoins.AmountOf(coin.Denom)))
-			continue
 				}
-		updatedBorrowCoin := sdk.NewCoin(coin.Denom, borrowedCoins.AmountOf(coin.Denom).Sub(coin.Amount))
+			} else {
-		updatedBorrowedCoins = updatedBorrowedCoins.Add(updatedBorrowCoin)
+				coinsToSubtract = coinsToSubtract.Add(coin)
+			}
+		}
+		updatedBorrowedCoins = borrowedCoins.Sub(coinsToSubtract)
 	}
 
 	k.SetBorrowedCoins(ctx, updatedBorrowedCoins)

x/hard/keeper/deposit.go

@@ -145,15 +145,19 @@ func (k Keeper) DecrementSuppliedCoins(ctx sdk.Context, coins sdk.Coins) error {
 		return sdkerrors.Wrapf(types.ErrSuppliedCoinsNotFound, "cannot withdraw if no coins are deposited")
 	}
 
-	updatedSuppliedCoins := sdk.NewCoins()
+	updatedSuppliedCoins, isNegative := suppliedCoins.SafeSub(coins)
+	if isNegative {
+		coinsToSubtract := sdk.NewCoins()
 		for _, coin := range coins {
-		// If amount is greater than total supplied amount due to rounding, set total supplied amount to 0
+			if suppliedCoins.AmountOf(coin.Denom).LT(coin.Amount) {
-		// by skipping the coin such that it's not included in the updatedSuppliedCoins object
+				if suppliedCoins.AmountOf(coin.Denom).GT(sdk.ZeroInt()) {
-		if coin.Amount.GTE(suppliedCoins.AmountOf(coin.Denom)) {
+					coinsToSubtract = coinsToSubtract.Add(sdk.NewCoin(coin.Denom, suppliedCoins.AmountOf(coin.Denom)))
-			continue
 				}
-		updatedSupplyCoin := sdk.NewCoin(coin.Denom, suppliedCoins.AmountOf(coin.Denom).Sub(coin.Amount))
+			} else {
-		updatedSuppliedCoins = updatedSuppliedCoins.Add(updatedSupplyCoin)
+				coinsToSubtract = coinsToSubtract.Add(coin)
+			}
+		}
+		updatedSuppliedCoins = suppliedCoins.Sub(coinsToSubtract)
 	}
 
 	k.SetSuppliedCoins(ctx, updatedSuppliedCoins)

x/hard/keeper/deposit_test.go

@@ -189,3 +189,139 @@ func (suite *KeeperTestSuite) TestDeposit() {
 		})
 	}
 }
+
+func (suite *KeeperTestSuite) TestDecrementSuppliedCoins() {
+	type args struct {
+		suppliedInitial       sdk.Coins
+		decrementCoins        sdk.Coins
+		expectedSuppliedFinal sdk.Coins
+	}
+	type errArgs struct {
+		expectPass bool
+		contains   string
+	}
+	type decrementTest struct {
+		name    string
+		args    args
+		errArgs errArgs
+	}
+	testCases := []decrementTest{
+		{
+			"valid",
+			args{
+				suppliedInitial:       cs(c("bnb", 10000000000000), c("busd", 3000000000000), c("xrpb", 2500000000000)),
+				decrementCoins:        cs(c("bnb", 5000000000000)),
+				expectedSuppliedFinal: cs(c("bnb", 5000000000000), c("busd", 3000000000000), c("xrpb", 2500000000000)),
+			},
+			errArgs{
+				expectPass: true,
+				contains:   "",
+			},
+		},
+		{
+			"valid-negative",
+			args{
+				suppliedInitial:       cs(c("bnb", 10000000000000), c("busd", 3000000000000), c("xrpb", 2500000000000)),
+				decrementCoins:        cs(c("bnb", 10000000000001)),
+				expectedSuppliedFinal: cs(c("busd", 3000000000000), c("xrpb", 2500000000000)),
+			},
+			errArgs{
+				expectPass: true,
+				contains:   "",
+			},
+		},
+		{
+			"valid-multiple negative",
+			args{
+				suppliedInitial:       cs(c("bnb", 10000000000000), c("busd", 3000000000000), c("xrpb", 2500000000000)),
+				decrementCoins:        cs(c("bnb", 10000000000001), c("busd", 5000000000000)),
+				expectedSuppliedFinal: cs(c("xrpb", 2500000000000)),
+			},
+			errArgs{
+				expectPass: true,
+				contains:   "",
+			},
+		},
+		{
+			"valid-absent coin denom",
+			args{
+				suppliedInitial:       cs(c("bnb", 10000000000000), c("xrpb", 2500000000000)),
+				decrementCoins:        cs(c("busd", 5)),
+				expectedSuppliedFinal: cs(c("bnb", 10000000000000), c("xrpb", 2500000000000)),
+			},
+			errArgs{
+				expectPass: true,
+				contains:   "",
+			},
+		},
+	}
+	for _, tc := range testCases {
+		suite.Run(tc.name, func() {
+			// Initialize test app and set context
+			tApp := app.NewTestApp()
+			ctx := tApp.NewContext(true, abci.Header{Height: 1, Time: tmtime.Now()})
+			loanToValue, _ := sdk.NewDecFromStr("0.6")
+			depositor := sdk.AccAddress(crypto.AddressHash([]byte("test")))
+			authGS := app.NewAuthGenState([]sdk.AccAddress{depositor}, []sdk.Coins{tc.args.suppliedInitial})
+			hardGS := types.NewGenesisState(types.NewParams(
+				types.MoneyMarkets{
+					types.NewMoneyMarket("bnb", types.NewBorrowLimit(false, sdk.NewDec(1000000000000000), loanToValue), "bnb:usd", sdk.NewInt(100000000), types.NewInterestRateModel(sdk.MustNewDecFromStr("0.05"), sdk.MustNewDecFromStr("2"), sdk.MustNewDecFromStr("0.8"), sdk.MustNewDecFromStr("10")), sdk.MustNewDecFromStr("0.05"), sdk.ZeroDec()),
+					types.NewMoneyMarket("busd", types.NewBorrowLimit(false, sdk.NewDec(1000000000000000), loanToValue), "busd:usd", sdk.NewInt(100000000), types.NewInterestRateModel(sdk.MustNewDecFromStr("0.05"), sdk.MustNewDecFromStr("2"), sdk.MustNewDecFromStr("0.8"), sdk.MustNewDecFromStr("10")), sdk.MustNewDecFromStr("0.05"), sdk.ZeroDec()),
+					types.NewMoneyMarket("xrpb", types.NewBorrowLimit(false, sdk.NewDec(1000000000000000), loanToValue), "xrpb:usd", sdk.NewInt(100000000), types.NewInterestRateModel(sdk.MustNewDecFromStr("0.05"), sdk.MustNewDecFromStr("2"), sdk.MustNewDecFromStr("0.8"), sdk.MustNewDecFromStr("10")), sdk.MustNewDecFromStr("0.05"), sdk.ZeroDec()),
+				},
+				sdk.MustNewDecFromStr("10"),
+			), types.DefaultAccumulationTimes, types.DefaultDeposits, types.DefaultBorrows,
+				types.DefaultTotalSupplied, types.DefaultTotalBorrowed, types.DefaultTotalReserves,
+			)
+			// Pricefeed module genesis state
+			pricefeedGS := pricefeed.GenesisState{
+				Params: pricefeed.Params{
+					Markets: []pricefeed.Market{
+						{MarketID: "xrpb:usd", BaseAsset: "kava", QuoteAsset: "usd", Oracles: []sdk.AccAddress{}, Active: true},
+						{MarketID: "busd:usd", BaseAsset: "btcb", QuoteAsset: "usd", Oracles: []sdk.AccAddress{}, Active: true},
+						{MarketID: "bnb:usd", BaseAsset: "bnb", QuoteAsset: "usd", Oracles: []sdk.AccAddress{}, Active: true},
+					},
+				},
+				PostedPrices: []pricefeed.PostedPrice{
+					{
+						MarketID:      "busd:usd",
+						OracleAddress: sdk.AccAddress{},
+						Price:         sdk.MustNewDecFromStr("1.00"),
+						Expiry:        time.Now().Add(1 * time.Hour),
+					},
+					{
+						MarketID:      "xrpb:usd",
+						OracleAddress: sdk.AccAddress{},
+						Price:         sdk.MustNewDecFromStr("2.00"),
+						Expiry:        time.Now().Add(1 * time.Hour),
+					},
+					{
+						MarketID:      "bnb:usd",
+						OracleAddress: sdk.AccAddress{},
+						Price:         sdk.MustNewDecFromStr("200.00"),
+						Expiry:        time.Now().Add(1 * time.Hour),
+					},
+				},
+			}
+			tApp.InitializeFromGenesisStates(authGS,
+				app.GenesisState{pricefeed.ModuleName: pricefeed.ModuleCdc.MustMarshalJSON(pricefeedGS)},
+				app.GenesisState{types.ModuleName: types.ModuleCdc.MustMarshalJSON(hardGS)},
+			)
+			keeper := tApp.GetHardKeeper()
+			suite.app = tApp
+			suite.ctx = ctx
+			suite.keeper = keeper
+
+			// Run BeginBlocker once to transition MoneyMarkets
+			hard.BeginBlocker(suite.ctx, suite.keeper)
+
+			err := suite.keeper.Deposit(suite.ctx, depositor, tc.args.suppliedInitial)
+			suite.Require().NoError(err)
+			err = suite.keeper.DecrementSuppliedCoins(ctx, tc.args.decrementCoins)
+			suite.Require().NoError(err)
+			totalSuppliedActual, found := suite.keeper.GetSuppliedCoins(suite.ctx)
+			suite.Require().True(found)
+			suite.Require().Equal(totalSuppliedActual, tc.args.expectedSuppliedFinal)
+		})
+	}
+}

x/hard/keeper/liquidation.go

@@ -86,6 +86,7 @@ func (k Keeper) SeizeDeposits(ctx sdk.Context, keeper sdk.AccAddress, deposit ty
 		}
 	}
 	if !keeperRewardCoins.Empty() {
+		k.DecrementSuppliedCoins(ctx, keeperRewardCoins)
 		err := k.supplyKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleAccountName, keeper, keeperRewardCoins)
 		if err != nil {
 			return err

x/hard/keeper/liquidation_test.go

@@ -72,7 +72,7 @@ func (suite *KeeperTestSuite) TestKeeperLiquidation() {
 				depositCoins:               sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(10*KAVA_CF))),
 				borrowCoins:                sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(8*KAVA_CF))),
 				liquidateAfter:             oneMonthInSeconds,
-				expectedTotalSuppliedCoins: sdk.NewCoins(sdk.NewInt64Coin("ukava", 504138)),
+				expectedTotalSuppliedCoins: sdk.NewCoins(sdk.NewInt64Coin("ukava", 100004118)),
 				expectedTotalBorrowedCoins: nil,
 				expectedKeeperCoins:        sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100500020))),
 				expectedBorrowerCoins:      sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(98000001))), // initial - deposit + borrow + liquidation leftovers
@@ -111,7 +111,12 @@ func (suite *KeeperTestSuite) TestKeeperLiquidation() {
 				depositCoins:         sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(50*KAVA_CF))),                                                                                                                                     // $100 * 0.8 = $80 borrowable
 				borrowCoins:          sdk.NewCoins(sdk.NewCoin("usdc", sdk.NewInt(20*KAVA_CF)), sdk.NewCoin("ukava", sdk.NewInt(10*KAVA_CF)), sdk.NewCoin("bnb", sdk.NewInt(2*BNB_CF)), sdk.NewCoin("btc", sdk.NewInt(0.2*BTCB_CF))), // $20+$20+$20 = $80 borrowed
 				liquidateAfter:       oneMonthInSeconds,
-				expectedTotalSuppliedCoins: sdk.NewCoins(sdk.NewInt64Coin("ukava", 2500711)),
+				expectedTotalSuppliedCoins: sdk.NewCoins(
+					sdk.NewInt64Coin("ukava", 1000000710),
+					sdk.NewInt64Coin("usdc", 1000003120),
+					sdk.NewInt64Coin("bnb", 100000003123),
+					sdk.NewInt64Coin("btc", 100000000031),
+				),
 				expectedTotalBorrowedCoins: nil,
 				expectedKeeperCoins:        sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(102500001))),
 				expectedBorrowerCoins:      sdk.NewCoins(sdk.NewCoin("usdc", sdk.NewInt(20*KAVA_CF)), sdk.NewCoin("ukava", sdk.NewInt(60000002)), sdk.NewCoin("bnb", sdk.NewInt(2*BNB_CF)), sdk.NewCoin("btc", sdk.NewInt(0.2*BTCB_CF))), // initial - deposit + borrow + liquidation leftovers
@@ -195,7 +200,9 @@ func (suite *KeeperTestSuite) TestKeeperLiquidation() {
 				depositCoins:         sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(50*KAVA_CF)), sdk.NewCoin("bnb", sdk.NewInt(10*BNB_CF)), sdk.NewCoin("btc", sdk.NewInt(1*BTCB_CF))), // $100 + $100 + $100 = $300 * 0.8 = $240 borrowable                                                                                                                                       // $100 * 0.8 = $80 borrowable
 				borrowCoins:          sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(120*KAVA_CF))),                                                                                      // $240 borrowed
 				liquidateAfter:       oneMonthInSeconds,
-				expectedTotalSuppliedCoins: nil,
+				expectedTotalSuppliedCoins: sdk.NewCoins(
+					sdk.NewInt64Coin("ukava", 1000101456),
+				),
 				expectedTotalBorrowedCoins: nil,
 				expectedKeeperCoins:        sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(102500253)), sdk.NewCoin("bnb", sdk.NewInt(0.5*BNB_CF)), sdk.NewCoin("btc", sdk.NewInt(0.05*BTCB_CF))), // 5% of each seized coin + initial balances
 				expectedBorrowerCoins:      sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(170.000001*KAVA_CF)), sdk.NewCoin("bnb", sdk.NewInt(90*BNB_CF)), sdk.NewCoin("btc", sdk.NewInt(99*BTCB_CF))),
@@ -253,7 +260,7 @@ func (suite *KeeperTestSuite) TestKeeperLiquidation() {
 			},
 		},
 		{
-			"valid: mutliple stablecoin deposits, multiple variable coin borrows",
+			"valid: multiple stablecoin deposits, multiple variable coin borrows",
 			// Auctions: total lot value = $285 ($300 of deposits - $15 keeper reward), total max bid value = $270
 			args{
 				borrower:             borrower,
@@ -265,7 +272,12 @@ func (suite *KeeperTestSuite) TestKeeperLiquidation() {
 				depositCoins:         sdk.NewCoins(sdk.NewCoin("usdc", sdk.NewInt(100*KAVA_CF)), sdk.NewCoin("usdt", sdk.NewInt(100*KAVA_CF)), sdk.NewCoin("usdx", sdk.NewInt(100*KAVA_CF))), // $100 + $100 + $100 = $300 * 0.9 = $270 borrowable
 				borrowCoins:          sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(35*KAVA_CF)), sdk.NewCoin("bnb", sdk.NewInt(10*BNB_CF)), sdk.NewCoin("btc", sdk.NewInt(1*BTCB_CF))),       // $270 borrowed
 				liquidateAfter:       oneMonthInSeconds,
-				expectedTotalSuppliedCoins: nil,
+				expectedTotalSuppliedCoins: sdk.NewCoins(
+					sdk.NewInt64Coin("bnb", 100000078047),
+					sdk.NewInt64Coin("btc", 100000000780),
+					sdk.NewInt64Coin("ukava", 1000009550),
+					sdk.NewInt64Coin("usdx", 1),
+				),
 				expectedTotalBorrowedCoins: nil,
 				expectedKeeperCoins:        sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF)), sdk.NewCoin("usdc", sdk.NewInt(5*KAVA_CF)), sdk.NewCoin("usdt", sdk.NewInt(5*KAVA_CF)), sdk.NewCoin("usdx", sdk.NewInt(5*KAVA_CF))), // 5% of each seized coin + initial balances
 				expectedBorrowerCoins:      sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(135*KAVA_CF)), sdk.NewCoin("bnb", sdk.NewInt(10*BNB_CF)), sdk.NewCoin("btc", sdk.NewInt(1*BTCB_CF)), sdk.NewCoin("usdx", sdk.NewInt(0.000001*KAVA_CF))),
@@ -364,7 +376,12 @@ func (suite *KeeperTestSuite) TestKeeperLiquidation() {
 				depositCoins:         sdk.NewCoins(sdk.NewCoin("dai", sdk.NewInt(350*KAVA_CF)), sdk.NewCoin("usdc", sdk.NewInt(200*KAVA_CF))),
 				borrowCoins:          sdk.NewCoins(sdk.NewCoin("usdt", sdk.NewInt(250*KAVA_CF)), sdk.NewCoin("usdx", sdk.NewInt(245*KAVA_CF))),
 				liquidateAfter:       oneMonthInSeconds,
-				expectedTotalSuppliedCoins: nil,
+				expectedTotalSuppliedCoins: sdk.NewCoins(
+					sdk.NewInt64Coin("dai", 1000000000),
+					sdk.NewInt64Coin("usdc", 1000000001),
+					sdk.NewInt64Coin("usdt", 1000482503),
+					sdk.NewInt64Coin("usdx", 1000463500),
+				),
 				expectedTotalBorrowedCoins: nil,
 				expectedKeeperCoins:        sdk.NewCoins(sdk.NewCoin("dai", sdk.NewInt(1017.50*KAVA_CF)), sdk.NewCoin("usdt", sdk.NewInt(1000*KAVA_CF)), sdk.NewCoin("usdc", sdk.NewInt(1010*KAVA_CF)), sdk.NewCoin("usdx", sdk.NewInt(1000*KAVA_CF))),
 				expectedBorrowerCoins:      sdk.NewCoins(sdk.NewCoin("dai", sdk.NewInt(650*KAVA_CF)), sdk.NewCoin("usdc", sdk.NewInt(800000001)), sdk.NewCoin("usdt", sdk.NewInt(1250*KAVA_CF)), sdk.NewCoin("usdx", sdk.NewInt(1245*KAVA_CF))),
@@ -433,7 +450,7 @@ func (suite *KeeperTestSuite) TestKeeperLiquidation() {
 				depositCoins:               sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(20*KAVA_CF))), // Deposit 20 KAVA
 				borrowCoins:                sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(5*KAVA_CF))),  // Borrow 5 KAVA
 				liquidateAfter:             oneMonthInSeconds,
-				expectedTotalSuppliedCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(20001624))),
+				expectedTotalSuppliedCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(120001624))),
 				expectedTotalBorrowedCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(5001709))),
 				expectedKeeperCoins:        sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100.5*KAVA_CF))),
 				expectedBorrowerCoins:      sdk.NewCoins(),
@@ -452,10 +469,13 @@ func (suite *KeeperTestSuite) TestKeeperLiquidation() {
 			tApp := app.NewTestApp()
 			ctx := tApp.NewContext(true, abci.Header{Height: 1, Time: tmtime.Now()})
 
+			// account which will deposit "initial module account coins"
+			depositor := sdk.AccAddress(crypto.AddressHash([]byte("testdepositor")))
+
 			// Auth module genesis state
 			authGS := app.NewAuthGenState(
-				[]sdk.AccAddress{tc.args.borrower, tc.args.keeper},
+				[]sdk.AccAddress{tc.args.borrower, tc.args.keeper, depositor},
-				[]sdk.Coins{tc.args.initialBorrowerCoins, tc.args.initialKeeperCoins},
+				[]sdk.Coins{tc.args.initialBorrowerCoins, tc.args.initialKeeperCoins, tc.args.initialModuleCoins},
 			)
 
 			// Hard module genesis state
@@ -580,10 +600,6 @@ func (suite *KeeperTestSuite) TestKeeperLiquidation() {
 				app.GenesisState{pricefeed.ModuleName: pricefeed.ModuleCdc.MustMarshalJSON(pricefeedGS)},
 				app.GenesisState{types.ModuleName: types.ModuleCdc.MustMarshalJSON(hardGS)})
 
-			// Mint coins to Hard module account
-			supplyKeeper := tApp.GetSupplyKeeper()
-			supplyKeeper.MintCoins(ctx, types.ModuleAccountName, tc.args.initialModuleCoins)
-
 			auctionKeeper := tApp.GetAuctionKeeper()
 
 			keeper := tApp.GetHardKeeper()
@@ -597,6 +613,10 @@ func (suite *KeeperTestSuite) TestKeeperLiquidation() {
 			// Run begin blocker to set up state
 			hard.BeginBlocker(suite.ctx, suite.keeper)
 
+			// Deposit initial module account coins
+			err = suite.keeper.Deposit(suite.ctx, depositor, tc.args.initialModuleCoins)
+			suite.Require().NoError(err)
+
 			// Deposit coins
 			err = suite.keeper.Deposit(suite.ctx, tc.args.borrower, tc.args.depositCoins)
 			suite.Require().NoError(err)