From 0343edf0d02b71aa36e19eecbfe4f6b86788109e Mon Sep 17 00:00:00 2001 From: Denali Marsh Date: Fri, 5 Feb 2021 12:31:38 +0100 Subject: [PATCH] Hard Audit: fix duplicate supply/borrow index factors (#794) * types: set/get interest factor * delete 0 balance index factors from deposit * types: set/get borrow interest factor * delete 0 balance index factors from borrow * delete index factor directly --- x/hard/keeper/borrow.go | 27 ++++++++------------------- x/hard/keeper/deposit.go | 27 ++++++++------------------- x/hard/keeper/repay.go | 11 +++++++++++ x/hard/keeper/withdraw.go | 11 +++++++++++ x/hard/types/borrow.go | 32 ++++++++++++++++++++++++++++++++ x/hard/types/deposit.go | 32 ++++++++++++++++++++++++++++++++ x/hard/types/errors.go | 2 ++ 7 files changed, 104 insertions(+), 38 deletions(-) diff --git a/x/hard/keeper/borrow.go b/x/hard/keeper/borrow.go index 20e42e69..8d7018dd 100644 --- a/x/hard/keeper/borrow.go +++ b/x/hard/keeper/borrow.go @@ -60,26 +60,15 @@ func (k Keeper) Borrow(ctx sdk.Context, borrower sdk.AccAddress, coins sdk.Coins } } - // The first time a user borrows a denom we add it the user's borrow interest factor index - var borrowInterestFactors types.BorrowInterestFactors + interestFactors := types.BorrowInterestFactors{} currBorrow, foundBorrow := k.GetBorrow(ctx, borrower) - // On user's first borrow, build borrow index list containing denoms and current global borrow index value if foundBorrow { - // If the coin denom to be borrowed is not in the user's existing borrow, we add it borrow index - for _, coin := range coins { - if !sdk.NewCoins(coin).DenomsSubsetOf(currBorrow.Amount) { - borrowInterestFactorValue, _ := k.GetBorrowInterestFactor(ctx, coin.Denom) - borrowInterestFactor := types.NewBorrowInterestFactor(coin.Denom, borrowInterestFactorValue) - borrowInterestFactors = append(borrowInterestFactors, borrowInterestFactor) - } - } - // Concatenate new borrow interest factors to existing borrow interest factors - borrowInterestFactors = append(borrowInterestFactors, currBorrow.Index...) - } else { - for _, coin := range coins { - borrowInterestFactorValue, _ := k.GetBorrowInterestFactor(ctx, coin.Denom) - borrowInterestFactor := types.NewBorrowInterestFactor(coin.Denom, borrowInterestFactorValue) - borrowInterestFactors = append(borrowInterestFactors, borrowInterestFactor) + interestFactors = currBorrow.Index + } + for _, coin := range coins { + interestFactorValue, foundValue := k.GetBorrowInterestFactor(ctx, coin.Denom) + if foundValue { + interestFactors = interestFactors.SetInterestFactor(coin.Denom, interestFactorValue) } } @@ -91,7 +80,7 @@ func (k Keeper) Borrow(ctx sdk.Context, borrower sdk.AccAddress, coins sdk.Coins amount = coins } // Construct the user's new/updated borrow with amount and interest factors - borrow := types.NewBorrow(borrower, amount, borrowInterestFactors) + borrow := types.NewBorrow(borrower, amount, interestFactors) // Calculate the new Loan-to-Value ratio of Deposit-to-Borrow deposit, foundDeposit := k.GetDeposit(ctx, borrower) diff --git a/x/hard/keeper/deposit.go b/x/hard/keeper/deposit.go index 1fbdfa7e..021bf8c8 100644 --- a/x/hard/keeper/deposit.go +++ b/x/hard/keeper/deposit.go @@ -63,26 +63,15 @@ func (k Keeper) Deposit(ctx sdk.Context, depositor sdk.AccAddress, coins sdk.Coi return err } - // The first time a user deposits a denom we add it the user's supply interest factor index - var supplyInterestFactors types.SupplyInterestFactors + interestFactors := types.SupplyInterestFactors{} currDeposit, foundDeposit := k.GetDeposit(ctx, depositor) - // On user's first deposit, build deposit index list containing denoms and current global deposit index value if foundDeposit { - // If the coin denom to be deposited is not in the user's existing deposit, we add it deposit index - for _, coin := range coins { - if !sdk.NewCoins(coin).DenomsSubsetOf(currDeposit.Amount) { - supplyInterestFactorValue, _ := k.GetSupplyInterestFactor(ctx, coin.Denom) - supplyInterestFactor := types.NewSupplyInterestFactor(coin.Denom, supplyInterestFactorValue) - supplyInterestFactors = append(supplyInterestFactors, supplyInterestFactor) - } - } - // Concatenate new deposit interest factors to existing deposit interest factors - supplyInterestFactors = append(supplyInterestFactors, currDeposit.Index...) - } else { - for _, coin := range coins { - supplyInterestFactorValue, _ := k.GetSupplyInterestFactor(ctx, coin.Denom) - supplyInterestFactor := types.NewSupplyInterestFactor(coin.Denom, supplyInterestFactorValue) - supplyInterestFactors = append(supplyInterestFactors, supplyInterestFactor) + interestFactors = currDeposit.Index + } + for _, coin := range coins { + interestFactorValue, foundValue := k.GetSupplyInterestFactor(ctx, coin.Denom) + if foundValue { + interestFactors = interestFactors.SetInterestFactor(coin.Denom, interestFactorValue) } } @@ -94,7 +83,7 @@ func (k Keeper) Deposit(ctx sdk.Context, depositor sdk.AccAddress, coins sdk.Coi amount = coins } // Update the depositer's amount and supply interest factors in the store - deposit := types.NewDeposit(depositor, amount, supplyInterestFactors) + deposit := types.NewDeposit(depositor, amount, interestFactors) // Calculate the new Loan-to-Value ratio of Deposit-to-Borrow borrow, _ := k.GetBorrow(ctx, depositor) diff --git a/x/hard/keeper/repay.go b/x/hard/keeper/repay.go index 8a36d4ad..f35b4d6c 100644 --- a/x/hard/keeper/repay.go +++ b/x/hard/keeper/repay.go @@ -43,6 +43,17 @@ func (k Keeper) Repay(ctx sdk.Context, sender sdk.AccAddress, coins sdk.Coins) e return err } + // If any coin denoms have been completely repaid reset the denom's borrow index factor + for _, coin := range payment { + if coin.Amount.Equal(borrow.Amount.AmountOf(coin.Denom)) { + borrowIndex, removed := borrow.Index.RemoveInterestFactor(coin.Denom) + if !removed { + return sdkerrors.Wrapf(types.ErrInvalidIndexFactorDenom, "%s", coin.Denom) + } + borrow.Index = borrowIndex + } + } + // Update user's borrow in store borrow.Amount = borrow.Amount.Sub(payment) diff --git a/x/hard/keeper/withdraw.go b/x/hard/keeper/withdraw.go index b5fc956a..4d056b50 100644 --- a/x/hard/keeper/withdraw.go +++ b/x/hard/keeper/withdraw.go @@ -49,6 +49,17 @@ func (k Keeper) Withdraw(ctx sdk.Context, depositor sdk.AccAddress, coins sdk.Co return err } + // If any coin denoms have been completely withdrawn reset the denom's supply index factor + for _, coin := range deposit.Amount { + if !sdk.NewCoins(coin).DenomsSubsetOf(proposedDeposit.Amount) { + depositIndex, removed := deposit.Index.RemoveInterestFactor(coin.Denom) + if !removed { + return sdkerrors.Wrapf(types.ErrInvalidIndexFactorDenom, "%s", coin.Denom) + } + deposit.Index = depositIndex + } + } + deposit.Amount = deposit.Amount.Sub(amount) newLtv, err := k.CalculateLtv(ctx, deposit, borrow) if err != nil { diff --git a/x/hard/types/borrow.go b/x/hard/types/borrow.go index 81251252..e10cc31c 100644 --- a/x/hard/types/borrow.go +++ b/x/hard/types/borrow.go @@ -100,6 +100,38 @@ func (bif BorrowInterestFactor) String() string { // BorrowInterestFactors is a slice of BorrowInterestFactor, because Amino won't marshal maps type BorrowInterestFactors []BorrowInterestFactor +// GetInterestFactor returns a denom's interest factor value +func (bifs BorrowInterestFactors) GetInterestFactor(denom string) (sdk.Dec, bool) { + for _, bif := range bifs { + if bif.Denom == denom { + return bif.Value, true + } + } + return sdk.ZeroDec(), false +} + +// SetInterestFactor sets a denom's interest factor value +func (bifs BorrowInterestFactors) SetInterestFactor(denom string, factor sdk.Dec) BorrowInterestFactors { + for i, bif := range bifs { + if bif.Denom == denom { + bif.Value = factor + bifs[i] = bif + return bifs + } + } + return append(bifs, NewBorrowInterestFactor(denom, factor)) +} + +// RemoveInterestFactor removes a denom's interest factor value +func (bifs BorrowInterestFactors) RemoveInterestFactor(denom string) (BorrowInterestFactors, bool) { + for i, bif := range bifs { + if bif.Denom == denom { + return append(bifs[:i], bifs[i+1:]...), true + } + } + return bifs, false +} + // Validate validates BorrowInterestFactors func (bifs BorrowInterestFactors) Validate() error { for _, bif := range bifs { diff --git a/x/hard/types/deposit.go b/x/hard/types/deposit.go index 7fa3e38a..ad517e14 100644 --- a/x/hard/types/deposit.go +++ b/x/hard/types/deposit.go @@ -100,6 +100,38 @@ func (sif SupplyInterestFactor) String() string { // SupplyInterestFactors is a slice of SupplyInterestFactor, because Amino won't marshal maps type SupplyInterestFactors []SupplyInterestFactor +// GetInterestFactor returns a denom's interest factor value +func (sifs SupplyInterestFactors) GetInterestFactor(denom string) (sdk.Dec, bool) { + for _, sif := range sifs { + if sif.Denom == denom { + return sif.Value, true + } + } + return sdk.ZeroDec(), false +} + +// SetInterestFactor sets a denom's interest factor value +func (sifs SupplyInterestFactors) SetInterestFactor(denom string, factor sdk.Dec) SupplyInterestFactors { + for i, sif := range sifs { + if sif.Denom == denom { + sif.Value = factor + sifs[i] = sif + return sifs + } + } + return append(sifs, NewSupplyInterestFactor(denom, factor)) +} + +// RemoveInterestFactor removes a denom's interest factor value +func (sifs SupplyInterestFactors) RemoveInterestFactor(denom string) (SupplyInterestFactors, bool) { + for i, sif := range sifs { + if sif.Denom == denom { + return append(sifs[:i], sifs[i+1:]...), true + } + } + return sifs, false +} + // Validate validates SupplyInterestFactors func (sifs SupplyInterestFactors) Validate() error { for _, sif := range sifs { diff --git a/x/hard/types/errors.go b/x/hard/types/errors.go index 2d6a0f91..17c35490 100644 --- a/x/hard/types/errors.go +++ b/x/hard/types/errors.go @@ -61,4 +61,6 @@ var ( ErrInvalidWithdrawDenom = sdkerrors.Register(ModuleName, 26, "no coins of this type deposited") // ErrInvalidRepaymentDenom error for when user attempts to repay a non-borrowed coin type ErrInvalidRepaymentDenom = sdkerrors.Register(ModuleName, 27, "no coins of this type borrowed") + // ErrInvalidIndexFactorDenom error for when index factor denom cannot be found + ErrInvalidIndexFactorDenom = sdkerrors.Register(ModuleName, 28, "no index factor found for denom") )