Remove from batch issues

contracts/src/BorrowerOperations.sol

@@ -1034,17 +1034,21 @@ contract BorrowerOperations is LiquityBase, AddRemoveManagers, IBorrowerOperatio
         uint256 _maxUpfrontFee
     ) public override {
         _requireIsNotShutDown();
-        _requireCallerIsBorrower(_troveId);
 
         LocalVariables_removeFromBatch memory vars;
         vars.troveManager = troveManager;
         vars.sortedTroves = sortedTroves;
 
-        vars.batchManager = interestBatchManagerOf[_troveId];
+        _requireTroveIsActive(vars.troveManager, _troveId);
+        _requireCallerIsBorrower(_troveId);
+        _requireValidAnnualInterestRate(_newAnnualInterestRate);
+
+        vars.batchManager = _requireIsInBatch(_troveId);
         delete interestBatchManagerOf[_troveId];
 
         // Remove trove from Batch in SortedTroves
         vars.sortedTroves.removeFromBatch(_troveId);
+        // Reinsert as single trove
         vars.sortedTroves.insert(_troveId, _newAnnualInterestRate, _upperHint, _lowerHint);
 
         vars.trove = vars.troveManager.getLatestTroveData(_troveId);

contracts/src/test/TestContracts/InvariantsTestHandler.t.sol

@@ -238,7 +238,6 @@ contract InvariantsTestHandler is BaseHandler, BaseMultiCollateralTest {
         LatestTroveData t;
         address batchManager;
         uint256 batchManagementFee;
-        bool wasOpen;
         bool wasActive;
         bool premature;
         uint256 upfrontFee;
@@ -1930,7 +1929,6 @@ contract InvariantsTestHandler is BaseHandler, BaseMultiCollateralTest {
         v.t = v.c.troveManager.getLatestTroveData(v.troveId);
         v.batchManager = _batchManagerOf[i][v.troveId];
         v.batchManagementFee = v.c.troveManager.getLatestBatchData(v.batchManager).accruedManagementFee;
-        v.wasOpen = _isOpen(i, v.troveId);
         v.wasActive = _isActive(i, v.troveId);
         v.premature = _timeSinceLastBatchInterestRateAdjustment[i][v.batchManager] < INTEREST_RATE_ADJ_COOLDOWN;
         v.upfrontFee = v.batchManager != address(0)
@@ -1984,15 +1982,6 @@ contract InvariantsTestHandler is BaseHandler, BaseMultiCollateralTest {
 
             // Effects (system)
             _mintYield(i, v.pendingInterest, v.upfrontFee);
-        } catch Error(string memory reason) {
-            v.errorString = reason;
-
-            // Justify failures
-            if (reason.equals("ERC721: invalid token ID")) {
-                assertFalse(v.wasOpen, "Open Trove should have an NFT");
-            } else {
-                revert(reason);
-            }
         } catch (bytes memory revertData) {
             bytes4 selector;
             (selector, v.errorString) = _decodeCustomError(revertData);
@@ -2781,6 +2770,10 @@ contract InvariantsTestHandler is BaseHandler, BaseMultiCollateralTest {
                 return (selector, "BorrowerOperations.TroveInBatch()");
             }
 
+            if (selector == BorrowerOperations.TroveNotInBatch.selector) {
+                return (selector, "BorrowerOperations.TroveNotInBatch()");
+            }
+
             if (selector == BorrowerOperations.InterestNotInRange.selector) {
                 return (selector, "BorrowerOperations.InterestNotInRange()");
             }

contracts/src/test/interestBatchManagement.t.sol

@@ -170,6 +170,51 @@ contract InterestBatchManagementTest is DevTestSetup {
         assertEq(annualInterestRate, newAnnualInterestRate, "Wrong interest rate");
     }
 
+    function testRemoveBatchManagerFailsIfNewRateIsTooLow() public {
+        uint256 troveId = openTroveAndJoinBatchManager();
+        (,,,,,,,, address tmBatchManagerAddress,) = troveManager.Troves(troveId);
+        LatestTroveData memory trove = troveManager.getLatestTroveData(troveId);
+        uint256 annualInterestRate = trove.annualInterestRate;
+
+        uint256 newAnnualInterestRate = 4e15;
+        assertEq(tmBatchManagerAddress, B, "Wrong batch manager in TM");
+        assertNotEq(newAnnualInterestRate, annualInterestRate, "New interest rate should be different");
+
+        vm.startPrank(A);
+        vm.expectRevert(BorrowerOperations.InterestRateTooLow.selector);
+        borrowerOperations.removeFromBatch(troveId, newAnnualInterestRate, 0, 0, 1e24);
+        vm.stopPrank();
+    }
+
+    function testRemoveBatchManagerFailsIfNewRateIsTooHigh() public {
+        uint256 troveId = openTroveAndJoinBatchManager();
+        (,,,,,,,, address tmBatchManagerAddress,) = troveManager.Troves(troveId);
+        LatestTroveData memory trove = troveManager.getLatestTroveData(troveId);
+        uint256 annualInterestRate = trove.annualInterestRate;
+
+        uint256 newAnnualInterestRate = 101e16;
+        assertEq(tmBatchManagerAddress, B, "Wrong batch manager in TM");
+        assertNotEq(newAnnualInterestRate, annualInterestRate, "New interest rate should be different");
+
+        vm.startPrank(A);
+        vm.expectRevert(BorrowerOperations.InterestRateTooHigh.selector);
+        borrowerOperations.removeFromBatch(troveId, newAnnualInterestRate, 0, 0, 1e24);
+        vm.stopPrank();
+    }
+
+    function testRemoveBatchManagerFailsIfTroveNotInBatch() public {
+        uint256 troveId = openTroveNoHints100pct(A, 100e18, 5000e18, 5e16);
+        (,,,,,,,, address tmBatchManagerAddress,) = troveManager.Troves(troveId);
+
+        uint256 newAnnualInterestRate = 4e16;
+        assertEq(tmBatchManagerAddress, address(0), "Wrong batch manager in TM");
+
+        vm.startPrank(A);
+        vm.expectRevert(BorrowerOperations.TroveNotInBatch.selector);
+        borrowerOperations.removeFromBatch(troveId, newAnnualInterestRate, 0, 0, 1e24);
+        vm.stopPrank();
+    }
+
     function testOnlyBorrowerCanSetBatchManager() public {
         registerBatchManager(A);
         registerBatchManager(B);