fix bug that skipped minimum borrow check (#870)

add tests for non-borrower repayer address
This commit is contained in:
Ruaridh 2021-03-11 04:13:21 +00:00 committed by GitHub
parent e21a04ca57
commit 509d2edbca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 116 additions and 46 deletions

View File

@ -23,13 +23,13 @@ func (k Keeper) Repay(ctx sdk.Context, sender, owner sdk.AccAddress, coins sdk.C
// Refresh borrow after syncing interest
borrow, _ = k.GetBorrow(ctx, owner)
// Validate that sender holds coins for repayment
err := k.ValidateRepay(ctx, sender, owner, coins)
// cap the repayment by what's available to repay (the borrow amount)
payment, err := k.CalculatePaymentAmount(borrow.Amount, coins)
if err != nil {
return err
}
payment, err := k.CalculatePaymentAmount(borrow.Amount, coins)
// Validate that sender holds coins for repayment
err = k.ValidateRepay(ctx, sender, owner, payment)
if err != nil {
return err
}
@ -144,8 +144,10 @@ func (k Keeper) ValidateRepay(ctx sdk.Context, sender, owner sdk.AccAddress, coi
// If the proposed repayment would results in a borrowed USD value below the minimum borrow USD value, reject it.
// User can overpay their loan to close it out, but underpaying by such a margin that the USD value is in an
// invalid range is not allowed
// Unless the user is fully repaying their loan
proposedBorrowNewUSDValue := existingBorrowUSDValue.Sub(repayTotalUSDValue)
if proposedBorrowNewUSDValue.IsPositive() && proposedBorrowNewUSDValue.LT(k.GetMinimumBorrowUSDValue(ctx)) {
isFullRepayment := coins.IsEqual(existingBorrow.Amount)
if proposedBorrowNewUSDValue.LT(k.GetMinimumBorrowUSDValue(ctx)) && !isFullRepayment {
return sdkerrors.Wrapf(types.ErrBelowMinimumBorrowValue, "the proposed borrow's USD value $%s is below the minimum borrow limit $%s", proposedBorrowNewUSDValue, k.GetMinimumBorrowUSDValue(ctx))
}

View File

@ -1,7 +1,6 @@
package keeper_test
import (
"strings"
"time"
sdk "github.com/cosmos/cosmos-sdk/types"
@ -18,13 +17,13 @@ import (
func (suite *KeeperTestSuite) TestRepay() {
type args struct {
borrower sdk.AccAddress
repayer sdk.AccAddress
initialBorrowerCoins sdk.Coins
initialRepayerCoins sdk.Coins
initialModuleCoins sdk.Coins
depositCoins []sdk.Coin
borrowCoins sdk.Coins
repayCoins sdk.Coins
expectedAccountBalance sdk.Coins
expectedModAccountBalance sdk.Coins
}
type errArgs struct {
@ -45,10 +44,30 @@ func (suite *KeeperTestSuite) TestRepay() {
{
"valid: partial repay",
args{
borrower: sdk.AccAddress(crypto.AddressHash([]byte("test"))),
borrower: sdk.AccAddress(crypto.AddressHash([]byte("borrower"))),
repayer: sdk.AccAddress(crypto.AddressHash([]byte("borrower"))),
initialBorrowerCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
initialRepayerCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
initialModuleCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(1000*KAVA_CF)), sdk.NewCoin("usdx", sdk.NewInt(1000*USDX_CF))),
depositCoins: []sdk.Coin{sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))},
depositCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
borrowCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(50*KAVA_CF))),
repayCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(10*KAVA_CF))),
},
errArgs{
expectPass: true,
expectDelete: false,
contains: "",
},
},
{
"valid: partial repay by non borrower",
args{
borrower: sdk.AccAddress(crypto.AddressHash([]byte("borrower"))),
repayer: sdk.AccAddress(crypto.AddressHash([]byte("repayer"))),
initialBorrowerCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
initialRepayerCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
initialModuleCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(1000*KAVA_CF)), sdk.NewCoin("usdx", sdk.NewInt(1000*USDX_CF))),
depositCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
borrowCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(50*KAVA_CF))),
repayCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(10*KAVA_CF))),
},
@ -61,10 +80,12 @@ func (suite *KeeperTestSuite) TestRepay() {
{
"valid: repay in full",
args{
borrower: sdk.AccAddress(crypto.AddressHash([]byte("test"))),
borrower: sdk.AccAddress(crypto.AddressHash([]byte("borrower"))),
repayer: sdk.AccAddress(crypto.AddressHash([]byte("borrower"))),
initialBorrowerCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
initialRepayerCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
initialModuleCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(1000*KAVA_CF)), sdk.NewCoin("usdx", sdk.NewInt(1000*USDX_CF))),
depositCoins: []sdk.Coin{sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))},
depositCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
borrowCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(50*KAVA_CF))),
repayCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(50*KAVA_CF))),
},
@ -77,10 +98,12 @@ func (suite *KeeperTestSuite) TestRepay() {
{
"valid: overpayment is adjusted",
args{
borrower: sdk.AccAddress(crypto.AddressHash([]byte("test"))),
borrower: sdk.AccAddress(crypto.AddressHash([]byte("borrower"))),
repayer: sdk.AccAddress(crypto.AddressHash([]byte("borrower"))),
initialBorrowerCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
initialRepayerCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
initialModuleCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(1000*KAVA_CF)), sdk.NewCoin("usdx", sdk.NewInt(1000*USDX_CF))),
depositCoins: []sdk.Coin{sdk.NewCoin("ukava", sdk.NewInt(80*KAVA_CF))}, // Deposit less so user still has some KAVA
depositCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(80*KAVA_CF))), // Deposit less so user still has some KAVA
borrowCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(50*KAVA_CF))),
repayCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(60*KAVA_CF))), // Exceeds borrowed coins but not user's balance
},
@ -93,42 +116,48 @@ func (suite *KeeperTestSuite) TestRepay() {
{
"invalid: attempt to repay non-supplied coin",
args{
borrower: sdk.AccAddress(crypto.AddressHash([]byte("test"))),
borrower: sdk.AccAddress(crypto.AddressHash([]byte("borrower"))),
repayer: sdk.AccAddress(crypto.AddressHash([]byte("borrower"))),
initialBorrowerCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
initialRepayerCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
initialModuleCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(1000*KAVA_CF)), sdk.NewCoin("usdx", sdk.NewInt(1000*USDX_CF))),
depositCoins: []sdk.Coin{sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))},
depositCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
borrowCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(50*KAVA_CF))),
repayCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(10*KAVA_CF)), sdk.NewCoin("bnb", sdk.NewInt(10*KAVA_CF))),
},
errArgs{
expectPass: false,
expectDelete: false,
contains: "account can only repay up to 0bnb",
contains: "no coins of this type borrowed",
},
},
{
"invalid: insufficent balance for repay",
"invalid: insufficient balance for repay",
args{
borrower: sdk.AccAddress(crypto.AddressHash([]byte("test"))),
borrower: sdk.AccAddress(crypto.AddressHash([]byte("borrower"))),
repayer: sdk.AccAddress(crypto.AddressHash([]byte("repayer"))),
initialBorrowerCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
initialRepayerCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(49*KAVA_CF))),
initialModuleCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(1000*KAVA_CF)), sdk.NewCoin("usdx", sdk.NewInt(1000*USDX_CF))),
depositCoins: []sdk.Coin{sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))},
depositCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
borrowCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(50*KAVA_CF))),
repayCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(51*KAVA_CF))), // Exceeds user's KAVA balance
repayCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(50*KAVA_CF))), // Exceeds repayer's balance, but not borrow amount
},
errArgs{
expectPass: false,
expectDelete: false,
contains: "account can only repay up to 50000000ukava",
contains: "account can only repay up to 49000000ukava",
},
},
{
"invalid: repaying a single coin type results in borrow position below the minimum USD value",
args{
borrower: sdk.AccAddress(crypto.AddressHash([]byte("test"))),
borrower: sdk.AccAddress(crypto.AddressHash([]byte("borrower"))),
repayer: sdk.AccAddress(crypto.AddressHash([]byte("borrower"))),
initialBorrowerCoins: sdk.NewCoins(sdk.NewCoin("usdx", sdk.NewInt(100*USDX_CF))),
initialRepayerCoins: sdk.NewCoins(sdk.NewCoin("usdx", sdk.NewInt(100*USDX_CF))),
initialModuleCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(1000*KAVA_CF)), sdk.NewCoin("usdx", sdk.NewInt(1000*USDX_CF))),
depositCoins: []sdk.Coin{sdk.NewCoin("usdx", sdk.NewInt(100*USDX_CF))},
depositCoins: sdk.NewCoins(sdk.NewCoin("usdx", sdk.NewInt(100*USDX_CF))),
borrowCoins: sdk.NewCoins(sdk.NewCoin("usdx", sdk.NewInt(50*USDX_CF))),
repayCoins: sdk.NewCoins(sdk.NewCoin("usdx", sdk.NewInt(45*USDX_CF))),
},
@ -141,12 +170,14 @@ func (suite *KeeperTestSuite) TestRepay() {
{
"invalid: repaying multiple coin types results in borrow position below the minimum USD value",
args{
borrower: sdk.AccAddress(crypto.AddressHash([]byte("test"))),
borrower: sdk.AccAddress(crypto.AddressHash([]byte("borrower"))),
repayer: sdk.AccAddress(crypto.AddressHash([]byte("borrower"))),
initialBorrowerCoins: sdk.NewCoins(sdk.NewCoin("usdx", sdk.NewInt(100*USDX_CF))),
initialRepayerCoins: sdk.NewCoins(sdk.NewCoin("usdx", sdk.NewInt(100*USDX_CF)), sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
initialModuleCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(1000*KAVA_CF)), sdk.NewCoin("usdx", sdk.NewInt(1000*USDX_CF))),
depositCoins: []sdk.Coin{sdk.NewCoin("usdx", sdk.NewInt(100*USDX_CF))},
depositCoins: sdk.NewCoins(sdk.NewCoin("usdx", sdk.NewInt(100*USDX_CF))),
borrowCoins: sdk.NewCoins(sdk.NewCoin("usdx", sdk.NewInt(50*USDX_CF)), sdk.NewCoin("ukava", sdk.NewInt(10*KAVA_CF))), // (50*$1)+(10*$2) = $70
repayCoins: sdk.NewCoins(sdk.NewCoin("usdx", sdk.NewInt(45*USDX_CF)), sdk.NewCoin("ukava", sdk.NewInt(8*KAVA_CF))), // (45*$1)+(8*2) = $61
repayCoins: sdk.NewCoins(sdk.NewCoin("usdx", sdk.NewInt(45*USDX_CF)), sdk.NewCoin("ukava", sdk.NewInt(8*KAVA_CF))), // (45*$1)+(8*$2) = $61
},
errArgs{
expectPass: false,
@ -154,6 +185,24 @@ func (suite *KeeperTestSuite) TestRepay() {
contains: "proposed borrow's USD value $9.000000000000000000 is below the minimum borrow limit",
},
},
{
"invalid: overpaying multiple coin types results in borrow position below the minimum USD value",
args{
borrower: sdk.AccAddress(crypto.AddressHash([]byte("borrower"))),
repayer: sdk.AccAddress(crypto.AddressHash([]byte("borrower"))),
initialBorrowerCoins: sdk.NewCoins(sdk.NewCoin("usdx", sdk.NewInt(100*USDX_CF))),
initialRepayerCoins: sdk.NewCoins(sdk.NewCoin("usdx", sdk.NewInt(100*USDX_CF)), sdk.NewCoin("ukava", sdk.NewInt(100*KAVA_CF))),
initialModuleCoins: sdk.NewCoins(sdk.NewCoin("ukava", sdk.NewInt(1000*KAVA_CF)), sdk.NewCoin("usdx", sdk.NewInt(1000*USDX_CF))),
depositCoins: sdk.NewCoins(sdk.NewCoin("usdx", sdk.NewInt(100*USDX_CF))),
borrowCoins: sdk.NewCoins(sdk.NewCoin("usdx", sdk.NewInt(50*USDX_CF)), sdk.NewCoin("ukava", sdk.NewInt(10*KAVA_CF))), // (50*$1)+(10*$2) = $70
repayCoins: sdk.NewCoins(sdk.NewCoin("usdx", sdk.NewInt(500*USDX_CF)), sdk.NewCoin("ukava", sdk.NewInt(8*KAVA_CF))), // (500*$1)+(8*$2) = $516, or capping to borrowed amount, (50*$1)+(8*$2) = $66
},
errArgs{
expectPass: false,
expectDelete: false,
contains: "proposed borrow's USD value $4.000000000000000000 is below the minimum borrow limit",
},
},
}
for _, tc := range testCases {
@ -163,9 +212,11 @@ func (suite *KeeperTestSuite) TestRepay() {
ctx := tApp.NewContext(true, abci.Header{Height: 1, Time: tmtime.Now()})
// Auth module genesis state
authGS := app.NewAuthGenState(
[]sdk.AccAddress{tc.args.borrower},
[]sdk.Coins{tc.args.initialBorrowerCoins})
addrs, coinses := uniqueAddressCoins(
[]sdk.AccAddress{tc.args.borrower, tc.args.repayer},
[]sdk.Coins{tc.args.initialBorrowerCoins, tc.args.initialRepayerCoins},
)
authGS := app.NewAuthGenState(addrs, coinses)
// Hard module genesis state
hardGS := types.NewGenesisState(types.NewParams(
@ -242,17 +293,19 @@ func (suite *KeeperTestSuite) TestRepay() {
err = suite.keeper.Borrow(suite.ctx, tc.args.borrower, tc.args.borrowCoins)
suite.Require().NoError(err)
err = suite.keeper.Repay(suite.ctx, tc.args.borrower, tc.args.borrower, tc.args.repayCoins)
previousRepayerCoins := suite.getAccount(tc.args.repayer).GetCoins()
err = suite.keeper.Repay(suite.ctx, tc.args.repayer, tc.args.borrower, tc.args.repayCoins)
if tc.errArgs.expectPass {
suite.Require().NoError(err)
// If we overpaid expect an adjustment
repaymentCoins, err := suite.keeper.CalculatePaymentAmount(tc.args.borrowCoins, tc.args.repayCoins)
suite.Require().NoError(err)
// Check borrower balance
expectedBorrowerCoins := tc.args.initialBorrowerCoins.Sub(tc.args.depositCoins).Add(tc.args.borrowCoins...).Sub(repaymentCoins)
acc := suite.getAccount(tc.args.borrower)
suite.Require().Equal(expectedBorrowerCoins, acc.GetCoins())
// Check repayer balance
expectedRepayerCoins := previousRepayerCoins.Sub(repaymentCoins)
acc := suite.getAccount(tc.args.repayer)
suite.Require().Equal(expectedRepayerCoins, acc.GetCoins())
// Check module account balance
expectedModuleCoins := tc.args.initialModuleCoins.Add(tc.args.depositCoins...).Sub(tc.args.borrowCoins).Add(repaymentCoins...)
@ -271,12 +324,11 @@ func (suite *KeeperTestSuite) TestRepay() {
}
} else {
suite.Require().Error(err)
suite.Require().True(strings.Contains(err.Error(), tc.errArgs.contains))
suite.Require().Contains(err.Error(), tc.errArgs.contains)
// Check borrower balance (no repay coins)
expectedBorrowerCoins := tc.args.initialBorrowerCoins.Sub(tc.args.depositCoins).Add(tc.args.borrowCoins...)
acc := suite.getAccount(tc.args.borrower)
suite.Require().Equal(expectedBorrowerCoins, acc.GetCoins())
// Check repayer balance (no repay coins)
acc := suite.getAccount(tc.args.repayer)
suite.Require().Equal(previousRepayerCoins, acc.GetCoins())
// Check module account balance (no repay coins)
expectedModuleCoins := tc.args.initialModuleCoins.Add(tc.args.depositCoins...).Sub(tc.args.borrowCoins)
@ -291,3 +343,19 @@ func (suite *KeeperTestSuite) TestRepay() {
})
}
}
// uniqueAddressCoins removes duplicate addresses, and the corresponding elements in a list of coins.
func uniqueAddressCoins(addresses []sdk.AccAddress, coinses []sdk.Coins) ([]sdk.AccAddress, []sdk.Coins) {
uniqueAddresses := []sdk.AccAddress{}
filteredCoins := []sdk.Coins{}
addrMap := map[string]bool{}
for i, a := range addresses {
if !addrMap[a.String()] {
uniqueAddresses = append(uniqueAddresses, a)
filteredCoins = append(filteredCoins, coinses[i])
}
addrMap[a.String()] = true
}
return uniqueAddresses, filteredCoins
}