test(ctb): Extract owner prevOwners generation into helper function

packages/contracts-bedrock/src/Safe/LivenessModule.sol

@@ -59,6 +59,11 @@ contract LivenessModule is ISemver {
         );
     }
 
+    /// @notice This function can be called by anyone to remove a set of owners that have not signed a transaction
+    ///         during the liveness interval. If the number of owners drops below the minimum, then all owners
+    ///         must be removed.
+    /// @param _previousOwners The previous owners in the linked list of owners
+    /// @param _ownersToRemove The owners to remove
     function removeOwners(address[] memory _previousOwners, address[] memory _ownersToRemove) external {
         require(_previousOwners.length == _ownersToRemove.length, "LivenessModule: arrays must be the same length");
 
@@ -80,9 +85,11 @@ contract LivenessModule is ISemver {
         _verifyFinalState();
     }
 
-    /// @notice This function can be called by anyone to remove an owner that has not signed a transaction
-    ///         during the liveness interval. If the number of owners drops below the minimum, then the
-    ///         ownership of the Safe is transferred to the fallback owner.
+    /// @notice Removes an owner from the Safe and updates the threshold.
+    /// @param _prevOwner Owner that pointed to the owner to be removed in the linked list
+    /// @param _ownerToRemove Owner address to be removed.
+    /// @param _newOwnersCount New number of owners after removal.
+    /// @param _newThreshold New threshold.
     function _removeOwner(
         address _prevOwner,
         address _ownerToRemove,
@@ -95,7 +102,8 @@ contract LivenessModule is ISemver {
             // Remove the owner and update the threshold
             _removeOwnerSafeCall({ _prevOwner: _prevOwner, _owner: _ownerToRemove, _threshold: _newThreshold });
         } else {
-            // Add the fallback owner as the sole owner of the Safe
+            // There is only one owner left. The Safe will not allow a safe with no owners, so we will
+            // need to swap owners instead.
             _swapToFallbackOwnerSafeCall({ _prevOwner: _prevOwner, _oldOwner: _ownerToRemove });
         }
     }

packages/contracts-bedrock/test/LivenessModule.t.sol

@@ -50,6 +50,23 @@ contract LivenessModule_TestInit is Test, SafeTestTools {
         }
     }
 
+    /// @notice Given an arrary of owners to remove, this function will return an array of the previous owners
+    ///         in the order that they must be provided to the LivenessMoules's removeOwners() function.
+    ///         Because owners are removed one at a time, and not necessarily in order, we need to simulate
+    ///         the owners list after each removal, in order to identify the correct previous owner.
+    /// @param _ownersToRemove The owners to remove
+    /// @return prevOwners_ The previous owners in the linked list
+    function _getPrevOwners(address[] memory _ownersToRemove) internal returns (address[] memory prevOwners_) {
+        prevOwners_ = new address[](_ownersToRemove.length);
+        address[] memory currentOwners;
+        for (uint256 i = 0; i < _ownersToRemove.length; i++) {
+            currentOwners = ownerSimulator.getOwners();
+            prevOwners_[i] = _getPrevOwner(safeInstance.owners[i], currentOwners);
+            if (currentOwners.length == 1) break;
+            ownerSimulator.removeOwnerWrapped(prevOwners_[i], _ownersToRemove[i], 1);
+        }
+    }
+
     function setUp() public {
         // Create a Safe with 10 owners
         (, uint256[] memory keys) = makeAddrsAndKeys(10);
@@ -122,20 +139,15 @@ contract LivenessModule_RemoveOwner_Test is LivenessModule_TestInit {
     }
 
     function test_removeOwner_allOwners_succeeds() external {
-        vm.warp(block.timestamp + 30 days);
         uint256 numOwners = safeInstance.owners.length;
 
-        address[] memory prevOwners = new address[](numOwners);
         address[] memory ownersToRemove = new address[](numOwners);
-        address[] memory currentOwners;
         for (uint256 i = 0; i < numOwners; i++) {
-            currentOwners = ownerSimulator.getOwners();
             ownersToRemove[i] = safeInstance.owners[i];
-            prevOwners[i] = _getPrevOwner(safeInstance.owners[i], currentOwners);
-            if (currentOwners.length == 1) break;
-            ownerSimulator.removeOwnerWrapped(prevOwners[i], ownersToRemove[i], 1);
         }
+        address[] memory prevOwners = _getPrevOwners(ownersToRemove);
 
+        vm.warp(block.timestamp + 30 days);
         livenessModule.removeOwners(prevOwners, ownersToRemove);
         assertEq(safeInstance.safe.getOwners().length, 1);
         assertEq(safeInstance.safe.getOwners()[0], fallbackOwner);