From 7f339d20ca13fedc3669b52bf003f7d607896570 Mon Sep 17 00:00:00 2001 From: sesheffield Date: Tue, 30 Jul 2024 13:08:48 -0400 Subject: [PATCH] fix(insolvency) kava lend insolvency check bug implementation (#1982) * add additional tests that attempt to borrow funds from the insolvent market(should fail), and attempt to borrow funds from the not insolvent market (it will fail, but shouldn't). The not insolvent market should continue to processs borrows * remove unused code * make tests less specific for string contains * add new get total reserves for denoms functionality * start utilizing GetTotalReservesForDenoms in ValidateBorrow * update tests for Borrow to not fail when borrowing from an insolvent market * use get total reseves in GetTotalReservesForDenoms for reusability * refactor GetTotalReservesForDenoms to GetTotalReservesByCoinDenoms for more clarity * change the structure for new and old tests and add more verbosity for other tests * remove print * remove unneeded code * add paren * adjust structure again after initial PR * remove duplicate test case with invalid test name, and update to use error contains in places where it was validating if true for strings contains * no need for keeper method --- x/hard/keeper/borrow.go | 36 ++++++++++++- x/hard/keeper/borrow_test.go | 97 +++++++++++++++++++++++++++++++++--- 2 files changed, 124 insertions(+), 9 deletions(-) diff --git a/x/hard/keeper/borrow.go b/x/hard/keeper/borrow.go index 5f02361e..df65fc14 100644 --- a/x/hard/keeper/borrow.go +++ b/x/hard/keeper/borrow.go @@ -118,11 +118,14 @@ func (k Keeper) ValidateBorrow(ctx sdk.Context, borrower sdk.AccAddress, amount // The reserve coins aren't available for users to borrow macc := k.accountKeeper.GetModuleAccount(ctx, types.ModuleName) - hardMaccCoins := k.bankKeeper.GetAllBalances(ctx, macc.GetAddress()) + hardMaccCoins := FilterCoinsByDenoms(k.bankKeeper.GetAllBalances(ctx, macc.GetAddress()), amount) reserveCoins, foundReserveCoins := k.GetTotalReserves(ctx) if !foundReserveCoins { reserveCoins = sdk.NewCoins() + } else { + reserveCoins = FilterCoinsByDenoms(reserveCoins, amount) } + fundsAvailableToBorrow, isNegative := hardMaccCoins.SafeSub(reserveCoins...) if isNegative { return errorsmod.Wrapf(types.ErrReservesExceedCash, "reserves %s > cash %s", reserveCoins, hardMaccCoins) @@ -220,6 +223,37 @@ func (k Keeper) ValidateBorrow(ctx sdk.Context, borrower sdk.AccAddress, amount return nil } +// FilterCoinsByDenoms filters the given coins by retaining only those whose denoms +// are present in the filterByCoins list. +// +// Parameters: +// - coins: The list of coins to be filtered. +// - filterByCoins: The list of coins whose denoms will be used as a filter. +// +// Returns: +// - A new list of coins that includes only those coins whose denom is in the filterByCoins list. +func FilterCoinsByDenoms(coins, filterByCoins sdk.Coins) sdk.Coins { + // Create a map to store the denoms that we want to filter by. + denoms := make(map[string]struct{}) + + // Populate the map with denoms from filterByCoins. + for _, denom := range filterByCoins.Denoms() { + denoms[denom] = struct{}{} + } + + // Prepare a slice to hold the filtered coins. + filteredCoins := make(sdk.Coins, 0, len(coins)) + + // Iterate through the list of coins and add those that have a denom in the denoms map. + for _, coin := range coins { + if _, exists := denoms[coin.Denom]; exists { + filteredCoins = append(filteredCoins, coin) + } + } + + return filteredCoins +} + // IncrementBorrowedCoins increments the total amount of borrowed coins by the newCoins parameter func (k Keeper) IncrementBorrowedCoins(ctx sdk.Context, newCoins sdk.Coins) { borrowedCoins, found := k.GetBorrowedCoins(ctx) diff --git a/x/hard/keeper/borrow_test.go b/x/hard/keeper/borrow_test.go index 348cdb47..9f9c8f5d 100644 --- a/x/hard/keeper/borrow_test.go +++ b/x/hard/keeper/borrow_test.go @@ -3,6 +3,8 @@ package keeper_test import ( "time" + "github.com/kava-labs/kava/x/hard/keeper" + sdkmath "cosmossdk.io/math" "github.com/cometbft/cometbft/crypto" tmproto "github.com/cometbft/cometbft/proto/tendermint/types" @@ -419,7 +421,7 @@ func (suite *KeeperTestSuite) TestBorrow() { }, }, { - name: "valid borrow followed by protocol reserves exceed available cash for busd when borrowing from ukava(incorrect)", + name: "valid borrow followed by protocol reserves exceed available cash for busd when borrowing from ukava", setup: setupArgs{ usdxBorrowLimit: sdk.MustNewDecFromStr("100000000000"), priceKAVA: sdk.MustNewDecFromStr("5.00"), @@ -440,10 +442,20 @@ func (suite *KeeperTestSuite) TestBorrow() { expectPass: true, }, { - expectedAccountBalance: sdk.NewCoins(), - expectedModAccountBalance: sdk.NewCoins(), - expectPass: false, - contains: "insolvency - protocol reserves exceed available cash", + expectedAccountBalance: sdk.NewCoins( + sdk.NewCoin("ukava", sdkmath.NewInt(51*KAVA_CF)), // now should be 1 ukava more + sdk.NewCoin("btcb", sdkmath.NewInt(100*BTCB_CF)), + sdk.NewCoin("usdx", sdkmath.NewInt(100*USDX_CF)), + sdk.NewCoin("busd", sdkmath.NewInt(100*BUSD_CF)), + sdk.NewCoin("bnb", sdkmath.NewInt(70*BNB_CF)), + sdk.NewCoin("xyz", sdkmath.NewInt(1)), + ), + expectedModAccountBalance: sdk.NewCoins( + sdk.NewCoin("ukava", sdkmath.NewInt(1049*KAVA_CF)), // now should be 1 ukava less + sdk.NewCoin("bnb", sdkmath.NewInt(30*BUSD_CF)), + sdk.NewCoin("usdx", sdkmath.NewInt(100*USDX_CF)), + ), + expectPass: true, }, }, borrows: []borrowArgs{ @@ -797,12 +809,81 @@ func (suite *KeeperTestSuite) TestValidateBorrow() { suite.ctx = suite.ctx.WithBlockTime(suite.ctx.BlockTime().Add(blockDuration)) hard.BeginBlocker(suite.ctx, suite.keeper) - // TODO: this is wrong, since usdk is not insolvent, ukava is. err = suite.keeper.Borrow( suite.ctx, borrower, sdk.NewCoins(sdk.NewCoin("usdx", sdkmath.NewInt(25*USDX_CF))), ) - suite.Require().Error(err) - suite.Require().ErrorContains(err, "protocol reserves exceed available cash") + suite.Require().NoError(err) +} + +func (suite *KeeperTestSuite) TestFilterCoinsByDenoms() { + type args struct { + coins sdk.Coins + filterByCoins sdk.Coins + } + tests := []struct { + name string + args args + want sdk.Coins + }{ + { + name: "more coins than filtered coins", + args: args{ + coins: sdk.NewCoins( + sdk.NewCoin("ukava", sdkmath.NewInt(1000*KAVA_CF)), + sdk.NewCoin("usdx", sdkmath.NewInt(200*USDX_CF)), + sdk.NewCoin("busd", sdkmath.NewInt(100*BUSD_CF)), + ), + filterByCoins: sdk.NewCoins( + sdk.NewCoin("usdx", sdkmath.NewInt(25*USDX_CF)), + sdk.NewCoin("ukava", sdkmath.NewInt(25*KAVA_CF)), + ), + }, + want: sdk.NewCoins( + sdk.NewCoin("usdx", sdkmath.NewInt(200*USDX_CF)), + sdk.NewCoin("ukava", sdkmath.NewInt(1000*KAVA_CF)), + ), + }, + { + name: "less coins than filtered coins", + args: args{ + coins: sdk.NewCoins( + sdk.NewCoin("ukava", sdkmath.NewInt(1000*KAVA_CF)), + ), + filterByCoins: sdk.NewCoins( + sdk.NewCoin("usdx", sdkmath.NewInt(25*USDX_CF)), + sdk.NewCoin("ukava", sdkmath.NewInt(25*KAVA_CF)), + ), + }, + want: sdk.NewCoins( + sdk.NewCoin("ukava", sdkmath.NewInt(1000*KAVA_CF)), + ), + }, + { + name: "no filter coins ", + args: args{ + coins: sdk.NewCoins( + sdk.NewCoin("ukava", sdkmath.NewInt(1000*KAVA_CF)), + ), + filterByCoins: sdk.NewCoins(), + }, + want: sdk.NewCoins(), + }, + { + name: "no coins ", + args: args{ + coins: sdk.NewCoins(), + filterByCoins: sdk.NewCoins( + sdk.NewCoin("usdx", sdkmath.NewInt(25*USDX_CF))), + }, + want: sdk.NewCoins(), + }, + } + for _, tt := range tests { + suite.Run(tt.name, func() { + got := keeper.FilterCoinsByDenoms(tt.args.coins, tt.args.filterByCoins) + suite.Require().Equal(tt.want, got) + }) + } }