From 67157175bc4cc7ede7de77889030c3c479f9dff6 Mon Sep 17 00:00:00 2001
From: zer0dot <zer0dot.dev@gmail.com>
Date: Thu, 15 Aug 2024 17:22:16 -0400
Subject: [PATCH] refactor: remove redundant factory check and correct return
 type for SMA

---
 src/account/AccountFactory.sol            | 14 +++++++++-----
 test/mocks/SingleSignerFactoryFixture.sol | 12 +++++++-----
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/src/account/AccountFactory.sol b/src/account/AccountFactory.sol
index 23139142..e8a0c8ef 100644
--- a/src/account/AccountFactory.sol
+++ b/src/account/AccountFactory.sol
@@ -20,6 +20,8 @@ contract AccountFactory is Ownable {
     IEntryPoint public immutable ENTRY_POINT;
     address public immutable SINGLE_SIGNER_VALIDATION_MODULE;
 
+    error SemiModularAccountAddressMismatch(address expected, address returned);
+
     constructor(
         IEntryPoint _entryPoint,
         UpgradeableModularAccount _accountImpl,
@@ -66,7 +68,7 @@ contract AccountFactory is Ownable {
         return UpgradeableModularAccount(payable(addr));
     }
 
-    function createSemiModularAccount(address owner, uint256 salt) external returns (UpgradeableModularAccount) {
+    function createSemiModularAccount(address owner, uint256 salt) external returns (SemiModularAccount) {
         // both module address and entityId for fallback validations are hardcoded at the maximum value.
         bytes32 fullSalt = getSalt(owner, salt, type(uint32).max);
 
@@ -74,13 +76,15 @@ contract AccountFactory is Ownable {
 
         address addr = _getAddressFallbackSigner(immutables, fullSalt);
 
-        // short circuit if exists
-        if (addr.code.length == 0) {
-            // not necessary to check return addr since next call will fail if so
+        // LibClone short-circuits if it's already deployed.
+        (, address instance) =
             LibClone.createDeterministicERC1967(address(SEMI_MODULAR_ACCOUNT_IMPL), immutables, fullSalt);
+
+        if (instance != addr) {
+            revert SemiModularAccountAddressMismatch(addr, instance);
         }
 
-        return UpgradeableModularAccount(payable(addr));
+        return SemiModularAccount(payable(addr));
     }
 
     function addStake(uint32 unstakeDelay) external payable onlyOwner {
diff --git a/test/mocks/SingleSignerFactoryFixture.sol b/test/mocks/SingleSignerFactoryFixture.sol
index 4a48314e..859bc481 100644
--- a/test/mocks/SingleSignerFactoryFixture.sol
+++ b/test/mocks/SingleSignerFactoryFixture.sol
@@ -25,6 +25,8 @@ contract SingleSignerFactoryFixture is OptimizedTest {
 
     address public self;
 
+    error SemiModularAccountAddressMismatch(address expected, address returned);
+
     constructor(IEntryPoint _entryPoint, SingleSignerValidationModule _singleSignerValidationModule) {
         entryPoint = _entryPoint;
 
@@ -81,12 +83,12 @@ contract SingleSignerFactoryFixture is OptimizedTest {
 
         address addr = _getAddressFallbackSigner(immutables, fullSalt);
 
-        // short circuit if exists
-        if (addr.code.length == 0) {
-            // not necessary to check return addr since next call will fail if so
-            // new ERC1967Proxy{salt: getSalt(owner, salt, type(uint32).max)}(address(ACCOUNT_IMPL), "");
-            // UpgradeableModularAccount(payable(addr)).initialize(owner);
+        // LibClone short-circuits if it's already deployed.
+        (, address instance) =
             LibClone.createDeterministicERC1967(address(accountImplementation), immutables, fullSalt);
+
+        if (instance != addr) {
+            revert SemiModularAccountAddressMismatch(addr, instance);
         }
 
         return UpgradeableModularAccount(payable(addr));