Merge pull request #12 from euler-xyz/75-negative-interest

Cantina 75: Interest calculation can be negative in logRepay during the pullDebt operation

src/EVault/EVault.sol

@@ -122,7 +122,7 @@ contract EVault is Dispatch {
 
     function repayWithShares(uint256 amount, address receiver) public virtual override callThroughEVC use(MODULE_BORROWING) returns (uint256 shares, uint256 debt) {}
 
-    function pullDebt(uint256 amount, address from) public virtual override callThroughEVC use(MODULE_BORROWING) returns (uint256) {}
+    function pullDebt(uint256 amount, address from) public virtual override callThroughEVC use(MODULE_BORROWING) {}
 
     function flashLoan(uint256 amount, bytes calldata data) public virtual override use(MODULE_BORROWING) {}
 

src/EVault/IEVault.sol

@@ -250,8 +250,9 @@ interface IBorrowing {
     /// @notice Take over debt from another account
     /// @param amount Amount of debt in asset units (use max uint256 for all the account's debt)
     /// @param from Account to pull the debt from
-    /// @return Amount of debt pulled in asset units.
-    function pullDebt(uint256 amount, address from) external returns (uint256);
+    /// @dev Due to internal debt precision accounting, the liability reported on either or both accounts after
+    /// calling `pullDebt` may not match the `amount` requested precisely
+    function pullDebt(uint256 amount, address from) external;
 
     /// @notice Request a flash-loan. A onFlashLoan() callback in msg.sender will be invoked, which must repay the loan
     /// to the main Euler address prior to returning.

src/EVault/modules/Borrowing.sol

@@ -128,19 +128,17 @@ abstract contract BorrowingModule is IBorrowing, AssetTransfers, BalanceUtils, L
     }
 
     /// @inheritdoc IBorrowing
-    function pullDebt(uint256 amount, address from) public virtual nonReentrant returns (uint256) {
+    function pullDebt(uint256 amount, address from) public virtual nonReentrant {
         (VaultCache memory vaultCache, address account) = initOperation(OP_PULL_DEBT, CHECKACCOUNT_CALLER);
 
         if (from == account) revert E_SelfTransfer();
 
         Assets assets = amount == type(uint256).max ? getCurrentOwed(vaultCache, from).toAssetsUp() : amount.toAssets();
 
-        if (assets.isZero()) return 0;
+        if (assets.isZero()) return;
         transferBorrow(vaultCache, from, account, assets);
 
         emit PullDebt(from, account, assets.toUint());
-
-        return assets.toUint();
     }
 
     /// @inheritdoc IBorrowing

src/EVault/shared/BorrowUtils.sol

@@ -95,13 +95,17 @@ abstract contract BorrowUtils is Base {
         toOwed = toOwed + amount;
         setUserBorrow(vaultCache, to, toOwed);
 
-        logRepay(from, assets, fromOwedPrev.toAssetsUp(), fromOwed.toAssetsUp());
+        // with small fractional debt amounts the interest calculation could be negative in `logRepay`
+        Assets fromPrevAssets = fromOwedPrev.toAssetsUp();
+        Assets fromAssets = fromOwed.toAssetsUp();
+        Assets repayAssets = fromPrevAssets > assets + fromAssets ? fromPrevAssets.subUnchecked(fromAssets) : assets;
+        logRepay(from, repayAssets, fromPrevAssets, fromAssets);
 
         // with small fractional debt amounts the interest calculation could be negative in `logBorrow`
         Assets toPrevAssets = toOwedPrev.toAssetsUp();
         Assets toAssets = toOwed.toAssetsUp();
-        if (assets + toPrevAssets > toAssets) assets = toAssets - toPrevAssets;
-        logBorrow(to, assets, toPrevAssets, toAssets);
+        Assets borrowAssets = assets + toPrevAssets > toAssets ? toAssets.subUnchecked(toPrevAssets) : assets;
+        logBorrow(to, borrowAssets, toPrevAssets, toAssets);
     }
 
     function computeInterestRate(VaultCache memory vaultCache) internal virtual returns (uint256) {

test/unit/evault/modules/Vault/borrow.t.sol

@@ -679,6 +679,50 @@ contract VaultTest_Borrow is EVaultTestBase {
         assertEq(eTST.debtOf(borrower), 0);
     }
 
+    function test_repayLogsTransferDebt() external {
+        eTST.setInterestRateModel(address(new IRMTestFixed()));
+
+        startHoax(borrower);
+
+        evc.enableController(borrower, address(eTST));
+        evc.enableCollateral(borrower, address(eTST2));
+        assetTST.approve(address(eTST), type(uint256).max);
+        assetTST.mint(borrower, 1000e18);
+
+        eTST.borrow(1, borrower);
+
+        assetTST2.transfer(borrower2, type(uint256).max / 2);
+
+        startHoax(borrower2);
+
+        assetTST2.approve(address(eTST2), type(uint256).max);
+        eTST2.deposit(10e18, borrower2);
+
+        evc.enableController(borrower2, address(eTST));
+        evc.enableCollateral(borrower2, address(eTST2));
+        assetTST.approve(address(eTST), type(uint256).max);
+        assetTST.mint(borrower2, 1000e18);
+
+        skip(10 days);
+
+        // a little interest accrued (0.3%)
+        assertEq(owedTo1e5(eTST.debtOfExact(borrower)), 1.00274e5);
+
+        // record interest in storage
+        startHoax(borrower);
+        eTST.borrow(1, borrower);
+
+        // now borrower in LogRepay would receive amount = 2, prevOwed = 3, owed = 0.
+        // Amount is adjusted to 3 and interest accrued is 0, so no event is emitted
+        startHoax(borrower2);
+        vm.recordLogs();
+        vm.expectEmit();
+        emit Events.Repay(borrower, 3);
+        eTST.pullDebt(2, borrower);
+
+        assertEq(vm.getRecordedLogs().length, 11); // InterestAccrued would be the 12th event
+    }
+
     function test_borrowLogsTransferDebt() external {
         eTST.setInterestRateModel(address(new IRMTestFixed()));