fix: S2 auditor suggestions i

Author: livingrockrises

src/core/ModuleManager.sol

@@ -233,9 +233,14 @@ abstract contract ModuleManager is AllStorage, EIP712, IModuleManager {
     // Perform the removal first
     validators.pop(prev, validator);
 
-    validator.excessivelySafeCall(
+    (bool success, bytes memory returnData) = validator.excessivelySafeCall(
       gasleft(), 0, 0, abi.encodeWithSelector(IModule.onUninstall.selector, disableModuleData)
     );
+    if (!success) {
+      emit ExternalCallFailed(
+        validator, abi.encodeWithSelector(IModule.onUninstall.selector, disableModuleData), returnData
+      );
+    }
   }
 
   /// Review: _tryUninstallValidators Might as well not call onUninstall() at all
@@ -270,9 +275,14 @@ abstract contract ModuleManager is AllStorage, EIP712, IModuleManager {
   function _uninstallExecutor(address executor, bytes calldata data) internal virtual {
     (address prev, bytes memory disableModuleData) = abi.decode(data, (address, bytes));
     _getAccountStorage().executors.pop(prev, executor);
-    executor.excessivelySafeCall(
+    (bool success, bytes memory returnData) = executor.excessivelySafeCall(
       gasleft(), 0, 0, abi.encodeWithSelector(IModule.onUninstall.selector, disableModuleData)
     );
+    if (!success) {
+      emit ExternalCallFailed(
+        executor, abi.encodeWithSelector(IModule.onUninstall.selector, disableModuleData), returnData
+      );
+    }
   }
 
   /// Review: _tryUninstallExecutors Might as well not call onUninstall() at all
@@ -314,7 +324,11 @@ abstract contract ModuleManager is AllStorage, EIP712, IModuleManager {
     {
       _uninstallPreValidationHook(hook, hookType, data);
     }
-    hook.excessivelySafeCall(gasleft(), 0, 0, abi.encodeWithSelector(IModule.onUninstall.selector, data));
+    (bool success, bytes memory returnData) =
+      hook.excessivelySafeCall(gasleft(), 0, 0, abi.encodeWithSelector(IModule.onUninstall.selector, data));
+    if (!success) {
+      emit ExternalCallFailed(hook, abi.encodeWithSelector(IModule.onUninstall.selector, data), returnData);
+    }
   }
 
   /// Review: _tryUninstallHook
@@ -341,9 +355,14 @@ abstract contract ModuleManager is AllStorage, EIP712, IModuleManager {
 
       if (address(handler.handler) == address(0)) continue;
 
-      handler.handler.excessivelySafeCall(
+      (bool success, bytes memory returnData) = handler.handler.excessivelySafeCall(
         gasleft(), 0, 0, abi.encodeWithSelector(IModule.onUninstall.selector, abi.encodePacked(selector))
       );
+      if (!success) {
+        emit ExternalCallFailed(
+          handler.handler, abi.encodeWithSelector(IModule.onUninstall.selector, abi.encodePacked(selector)), returnData
+        );
+      }
 
       ds.fallbacks[selector] = FallbackHandler(address(0), CallType.wrap(0x00));
     }
@@ -479,7 +498,14 @@ abstract contract ModuleManager is AllStorage, EIP712, IModuleManager {
       }
     }
 
-    fallbackHandler.excessivelySafeCall(gasleft(), 0, 0, abi.encodeWithSelector(IModule.onUninstall.selector, data[4:]));
+    (bool success, bytes memory returnData) = fallbackHandler.excessivelySafeCall(
+      gasleft(), 0, 0, abi.encodeWithSelector(IModule.onUninstall.selector, data[4:])
+    );
+    if (!success) {
+      emit ExternalCallFailed(
+        fallbackHandler, abi.encodeWithSelector(IModule.onUninstall.selector, data[4:]), returnData
+      );
+    }
   }
 
   /// @dev Installs a pre-validation hook module, ensuring no other pre-validation hooks are installed before proceeding.
@@ -523,12 +549,14 @@ abstract contract ModuleManager is AllStorage, EIP712, IModuleManager {
         emit PreValidationHookUninstallFailed(hook, '');
       }
       _setPreValidationHook(hookType, address(0));
+      emit ModuleUninstalled(hookType, hook);
     } else if (hookType == MODULE_TYPE_PREVALIDATION_HOOK_ERC4337) {
       try _getAccountStorage().preValidationHookERC4337.onUninstall('') {}
       catch {
         emit PreValidationHookUninstallFailed(hook, '');
       }
       _setPreValidationHook(hookType, address(0));
+      emit ModuleUninstalled(hookType, hook);
     } else {
       revert InvalidHookType(hookType);
     }

src/interfaces/core/IModuleManagerEventsAndErrors.sol

@@ -37,6 +37,12 @@ interface IModuleManagerEventsAndErrors {
   /// @param data The data of the hook.
   event HookUninstallFailed(address hook, bytes data);
 
+  /// @notice Emitted when an external call fails.
+  /// @param target The address of the target.
+  /// @param callData The data of the call.
+  /// @param returnData The return data of the call.
+  event ExternalCallFailed(address target, bytes callData, bytes returnData);
+
   /// @notice Thrown when attempting to remove the last validator.
   error CanNotRemoveLastValidator();
 

src/modules/validators/ECDSAValidator.sol

@@ -63,6 +63,12 @@ contract ECDSAValidator is IValidator, ERC7739Validator {
    */
   event OwnerRegistered(address indexed account, address indexed owner);
 
+  /**
+   * @notice Emitted when an owner is removed for an account
+   * @param account The smart account address
+   */
+  event OwnerRemoved(address indexed account);
+
   // Storage
   /**
    * @notice Mapping of smart account addresses to their respective owner addresses
@@ -108,6 +114,7 @@ contract ECDSAValidator is IValidator, ERC7739Validator {
     }
 
     delete smartAccountOwners[msg.sender];
+    emit OwnerRemoved(msg.sender);
     _safeSenders.removeAll(msg.sender);
   }
 
@@ -138,6 +145,7 @@ contract ECDSAValidator is IValidator, ERC7739Validator {
     }
 
     smartAccountOwners[msg.sender] = _newOwner;
+    emit OwnerRegistered(msg.sender, _newOwner);
   }
 
   /**