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

@@ -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)

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)

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)
 

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 {

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 {

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 {

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")
 )