ctb: Add new OwnerRemovalFailed(string) error type to LM (#10538)

* ctb: Update snapshots and semver-lock

* ctb: Add new OwnerRemovalFailed(string) error type to LM

packages/contracts-bedrock/semver-lock.json

@@ -112,8 +112,8 @@
     "sourceCodeHash": "0xea3872d8f196ae3c863363dfa4b57803cb2a24b0c100244d8f861891e901e03f"
   },
   "src/Safe/LivenessModule.sol": {
-    "initCodeHash": "0x6c3d47fdce3dcd5b8499da08061f3ee4ddd126b726bedec601210667bf73db1d",
-    "sourceCodeHash": "0x6bdfa1a9fe8a7bc6e032c2448b911ca3bd55ece0856866042e29a20e270dd0a3"
+    "initCodeHash": "0xdf37345a47e176266f0ece4c624c7e9c2bd81661675f53cf07706dc44dafad27",
+    "sourceCodeHash": "0xdf17b5a6c068b2cf8fd24383066f0cc8e4ab0002f2470476beae594ca86879f3"
   },
   "src/cannon/MIPS.sol": {
     "initCodeHash": "0xa5d36fc67170ad87322f358f612695f642757bbf5280800d5d878da21402579a",

packages/contracts-bedrock/snapshots/abi/LivenessModule.json

@@ -200,5 +200,16 @@
     ],
     "name": "RemovedOwner",
     "type": "event"
+  },
+  {
+    "inputs": [
+      {
+        "internalType": "string",
+        "name": "",
+        "type": "string"
+      }
+    ],
+    "name": "OwnerRemovalFailed",
+    "type": "error"
   }
 ]
\ No newline at end of file

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

@@ -14,6 +14,9 @@ import { ISemver } from "src/universal/ISemver.sol";
 ///         If the number of owners falls below the minimum number of owners, the ownership of the
 ///         safe will be transferred to the fallback owner.
 contract LivenessModule is ISemver {
+    /// @notice Error message for failed owner removal.
+    error OwnerRemovalFailed(string);
+
     /// @notice Emitted when an owner is removed due to insufficient liveness
     event RemovedOwner(address indexed owner);
 
@@ -184,15 +187,15 @@ contract LivenessModule is ISemver {
     /// @param _prevOwner Owner that pointed to the owner to be replaced in the linked list
     /// @param _oldOwner Owner address to be replaced.
     function _swapToFallbackOwnerSafeCall(address _prevOwner, address _oldOwner) internal {
-        require(
-            SAFE.execTransactionFromModule({
-                to: address(SAFE),
-                value: 0,
-                operation: Enum.Operation.Call,
-                data: abi.encodeCall(OwnerManager.swapOwner, (_prevOwner, _oldOwner, FALLBACK_OWNER))
-            }),
-            "LivenessModule: failed to swap to fallback owner"
-        );
+        (bool success, bytes memory returnData) = SAFE.execTransactionFromModuleReturnData({
+            to: address(SAFE),
+            value: 0,
+            operation: Enum.Operation.Call,
+            data: abi.encodeCall(OwnerManager.swapOwner, (_prevOwner, _oldOwner, FALLBACK_OWNER))
+        });
+        if (!success) {
+            revert OwnerRemovalFailed(string(returnData));
+        }
         emit OwnershipTransferredToFallback();
     }
 
@@ -201,15 +204,15 @@ contract LivenessModule is ISemver {
     /// @param _owner Owner address to be removed.
     /// @param _threshold New threshold.
     function _removeOwnerSafeCall(address _prevOwner, address _owner, uint256 _threshold) internal {
-        require(
-            SAFE.execTransactionFromModule({
-                to: address(SAFE),
-                value: 0,
-                operation: Enum.Operation.Call,
-                data: abi.encodeCall(OwnerManager.removeOwner, (_prevOwner, _owner, _threshold))
-            }),
-            "LivenessModule: failed to remove owner"
-        );
+        (bool success, bytes memory returnData) = SAFE.execTransactionFromModuleReturnData({
+            to: address(SAFE),
+            value: 0,
+            operation: Enum.Operation.Call,
+            data: abi.encodeCall(OwnerManager.removeOwner, (_prevOwner, _owner, _threshold))
+        });
+        if (!success) {
+            revert OwnerRemovalFailed(string(returnData));
+        }
         emit RemovedOwner(_owner);
     }
 

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

@@ -13,6 +13,8 @@ import { LivenessGuard } from "src/Safe/LivenessGuard.sol";
 contract LivenessModule_TestInit is Test, SafeTestTools {
     using SafeTestLib for SafeInstance;
 
+    error OwnerRemovalFailed(string reason);
+
     // LivenessModule events
     event SignersRecorded(bytes32 indexed txHash, address[] signers);
     event RemovedOwner(address indexed owner);
@@ -261,10 +263,13 @@ contract LivenessModule_RemoveOwners_TestFail is LivenessModule_TestInit {
         address[] memory prevOwners = new address[](1);
         address[] memory ownersToRemove = new address[](1);
         ownersToRemove[0] = safeInstance.owners[0];
-        prevOwners[0] = ownersToRemove[0]; // incorrect.
+        // Incorrectly set the previous owner as equal to the owner to remove, which will cause the Safe to revert.
+        prevOwners[0] = ownersToRemove[0];
 
         _warpPastLivenessInterval();
-        vm.expectRevert("LivenessModule: failed to remove owner");
+        vm.expectRevert(
+            abi.encodeWithSelector(OwnerRemovalFailed.selector, (abi.encodeWithSignature("Error(string)", "GS205")))
+        );
         livenessModule.removeOwners(prevOwners, ownersToRemove);
     }
 
@@ -278,11 +283,13 @@ contract LivenessModule_RemoveOwners_TestFail is LivenessModule_TestInit {
         }
         address[] memory prevOwners = safeInstance.getPrevOwners(ownersToRemove);
 
-        // Incorrectly set the final owner to address(0)
+        // Incorrectly set the final owner to address(0), causing the Safe to revert.
         ownersToRemove[ownersToRemove.length - 1] = address(0);
 
         _warpPastLivenessInterval();
-        vm.expectRevert("LivenessModule: failed to swap to fallback owner");
+        vm.expectRevert(
+            abi.encodeWithSelector(OwnerRemovalFailed.selector, (abi.encodeWithSignature("Error(string)", "GS203")))
+        );
         livenessModule.removeOwners(prevOwners, ownersToRemove);
     }