feat: allow p256 keys to be super admins (#378)

howydev · actions-user · web-flow · commit 7997f6ab22dc · 2025-09-18T11:11:00.000-04:00
* feat: allow p256 keys to be super admins

* chore: bump contract versions due to bytecode changes - Contracts updated: IthacaAccount

---------

Co-authored-by: GitHub Action &lt;action@github.com&gt;
diff --git a/.gitmodules b/.gitmodules
@@ -17,6 +17,3 @@
 	path = lib/solady
 	url = https://github.com/vectorized/solady
 	branch = main
-[submodule "lib/account_v0_5_5"]
-	path = lib/account_v0_5_5
-	url = https://github.com/ithacaxyz/account
diff --git a/lib/account_v0_5_5 b/lib/account_v0_5_5
diff --git a/snapshots/BenchmarkTest.json b/snapshots/BenchmarkTest.json
@@ -16,11 +16,11 @@
   "testERC20Transfer_ERC4337MinimalAccount": "171906",
   "testERC20Transfer_ERC4337MinimalAccount_AppSponsor": "168917",
   "testERC20Transfer_ERC4337MinimalAccount_ERC20SelfPay": "217488",
-  "testERC20Transfer_IthacaAccount": "129986",
-  "testERC20Transfer_IthacaAccountWithSpendLimits": "193499",
-  "testERC20Transfer_IthacaAccount_AppSponsor": "141385",
-  "testERC20Transfer_IthacaAccount_AppSponsor_ERC20": "163850",
-  "testERC20Transfer_IthacaAccount_ERC20SelfPay": "147639",
+  "testERC20Transfer_IthacaAccount": "129918",
+  "testERC20Transfer_IthacaAccountWithSpendLimits": "190799",
+  "testERC20Transfer_IthacaAccount_AppSponsor": "141362",
+  "testERC20Transfer_IthacaAccount_AppSponsor_ERC20": "163827",
+  "testERC20Transfer_IthacaAccount_ERC20SelfPay": "147571",
   "testERC20Transfer_Safe4337": "197515",
   "testERC20Transfer_Safe4337_AppSponsor": "191679",
   "testERC20Transfer_Safe4337_ERC20SelfPay": "238658",
@@ -29,25 +29,25 @@
   "testERC20Transfer_ZerodevKernel_ERC20SelfPay": "252683",
   "testERC20Transfer_batch100_AlchemyModularAccount": "10104466",
   "testERC20Transfer_batch100_AlchemyModularAccount_ERC20SelfPay": "11635798",
-  "testERC20Transfer_batch100_IthacaAccount": "7702476",
-  "testERC20Transfer_batch100_IthacaAccount_AppSponsor": "8397224",
-  "testERC20Transfer_batch100_IthacaAccount_AppSponsor_ERC20": "8246556",
-  "testERC20Transfer_batch100_IthacaAccount_ERC20SelfPay": "7547236",
+  "testERC20Transfer_batch100_IthacaAccount": "7695676",
+  "testERC20Transfer_batch100_IthacaAccount_AppSponsor": "8394924",
+  "testERC20Transfer_batch100_IthacaAccount_AppSponsor_ERC20": "8244256",
+  "testERC20Transfer_batch100_IthacaAccount_ERC20SelfPay": "7540436",
   "testERC20Transfer_batch100_ZerodevKernel": "12626718",
   "testERC20Transfer_batch100_ZerodevKernel_ERC20SelfPay": "14176437",
   "testNativeTransfer_AlchemyModularAccount": "180829",
   "testNativeTransfer_CoinbaseSmartWallet": "178916",
-  "testNativeTransfer_IthacaAccount": "131388",
-  "testNativeTransfer_IthacaAccount_AppSponsor": "142818",
-  "testNativeTransfer_IthacaAccount_ERC20SelfPay": "156341",
+  "testNativeTransfer_IthacaAccount": "131320",
+  "testNativeTransfer_IthacaAccount_AppSponsor": "142795",
+  "testNativeTransfer_IthacaAccount_ERC20SelfPay": "156273",
   "testNativeTransfer_Safe4337": "198595",
   "testNativeTransfer_ZerodevKernel": "208635",
   "testUniswapV2Swap_AlchemyModularAccount": "238767",
   "testUniswapV2Swap_CoinbaseSmartWallet": "237571",
   "testUniswapV2Swap_ERC4337MinimalAccount": "231242",
-  "testUniswapV2Swap_IthacaAccount": "189296",
-  "testUniswapV2Swap_IthacaAccount_AppSponsor": "200671",
-  "testUniswapV2Swap_IthacaAccount_ERC20SelfPay": "211749",
+  "testUniswapV2Swap_IthacaAccount": "189228",
+  "testUniswapV2Swap_IthacaAccount_AppSponsor": "200648",
+  "testUniswapV2Swap_IthacaAccount_ERC20SelfPay": "211681",
   "testUniswapV2Swap_Safe4337": "257453",
   "testUniswapV2Swap_ZerodevKernel": "266487"
 }
diff --git a/src/GuardedExecutor.sol b/src/GuardedExecutor.sol
@@ -120,9 +120,6 @@ abstract contract GuardedExecutor is ERC7821 {
     /// @dev Emitted when a spend limit is removed.
     event SpendLimitRemoved(bytes32 keyHash, address token, SpendPeriod period);
 
-    /// @dev Emitted when spend limits are enabled or disabled for a key.
-    event SpendLimitsEnabledSet(bytes32 keyHash, bool enabled);
-
     ////////////////////////////////////////////////////////////////////////
     // Constants
     ////////////////////////////////////////////////////////////////////////
@@ -184,8 +181,6 @@ abstract contract GuardedExecutor is ERC7821 {
         SpendStorage spends;
         /// @dev Mapping of 3rd-party checkers for determining if an address can execute a function.
         EnumerableMapLib.AddressToAddressMap callCheckers;
-        /// @dev Whether spend limits are disabled for this key. Defaults to false (limits enabled).
-        bool spendLimitsDisabled;
     }
 
     /// @dev Returns the storage pointer.
@@ -234,11 +229,6 @@ abstract contract GuardedExecutor is ERC7821 {
             return ERC7821._execute(calls, keyHash);
         }
 
-        // If spend limits are disabled for this key, execute without spend checks
-        if (!spendLimitsEnabled(keyHash)) {
-            return ERC7821._execute(calls, keyHash);
-        }
-
         SpendStorage storage spends = _getGuardedExecutorKeyStorage(keyHash).spends;
         _ExecuteTemps memory t;
 
@@ -469,21 +459,6 @@ abstract contract GuardedExecutor is ERC7821 {
         emit SpendLimitRemoved(keyHash, token, period);
     }
 
-    /// @dev Sets whether spend limits are enabled or disabled for a specific key.
-    /// By default, spend limits are enabled for all keys.
-    function setSpendLimitsEnabled(bytes32 keyHash, bool enabled)
-        public
-        virtual
-        onlyThis
-        checkKeyHashIsNonZero(keyHash)
-    {
-        if (_isSuperAdmin(keyHash)) revert SuperAdminCanSpendAnything();
-
-        _getGuardedExecutorKeyStorage(keyHash).spendLimitsDisabled = !enabled;
-
-        emit SpendLimitsEnabledSet(keyHash, enabled);
-    }
-
     ////////////////////////////////////////////////////////////////////////
     // Public View Functions
     ////////////////////////////////////////////////////////////////////////
@@ -614,13 +589,6 @@ abstract contract GuardedExecutor is ERC7821 {
         }
     }
 
-    /// @dev Returns whether spend limits are enabled for a specific key.
-    /// Defaults to true if never set.
-    function spendLimitsEnabled(bytes32 keyHash) public view virtual returns (bool) {
-        // disabled defaults to false, so !false = true (enabled by default)
-        return !_getGuardedExecutorKeyStorage(keyHash).spendLimitsDisabled;
-    }
-
     /// @dev Rounds the unix timestamp down to the period.
     function startOfSpendPeriod(uint256 unixTimestamp, SpendPeriod period)
         public
diff --git a/src/IthacaAccount.sol b/src/IthacaAccount.sol
@@ -575,9 +575,6 @@ contract IthacaAccount is IIthacaAccount, EIP712, GuardedExecutor {
 
     /// @dev Adds the key. If the key already exist, its expiry will be updated.
     function _addKey(Key memory key) internal virtual returns (bytes32 keyHash) {
-        if (key.isSuperAdmin) {
-            if (!_keyTypeCanBeSuperAdmin(key.keyType)) revert KeyTypeCannotBeSuperAdmin();
-        }
         // `keccak256(abi.encode(key.keyType, keccak256(key.publicKey)))`.
         keyHash = hash(key);
         AccountStorage storage $ = _getAccountStorage();
@@ -587,11 +584,6 @@ contract IthacaAccount is IIthacaAccount, EIP712, GuardedExecutor {
         $.keyHashes.add(keyHash);
     }
 
-    /// @dev Returns if the `keyType` can be a super admin.
-    function _keyTypeCanBeSuperAdmin(KeyType keyType) internal view virtual returns (bool) {
-        return keyType != KeyType.P256;
-    }
-
     /// @dev Removes the key corresponding to the `keyHash`. Reverts if the key does not exist.
     function _removeKey(bytes32 keyHash) internal virtual {
         AccountStorage storage $ = _getAccountStorage();
@@ -763,6 +755,6 @@ contract IthacaAccount is IIthacaAccount, EIP712, GuardedExecutor {
         returns (string memory name, string memory version)
     {
         name = "IthacaAccount";
-        version = "0.5.9";
+        version = "0.5.10";
     }
 }
diff --git a/test/Account.t.sol b/test/Account.t.sol
@@ -260,29 +260,6 @@ contract AccountTest is BaseTest {
         assert(keys[1].expiry == 5);
     }
 
-    function testAddDisallowedSuperAdminKeyTypeReverts() public {
-        address orchestrator = address(new Orchestrator());
-        address accountImplementation = address(new IthacaAccount(address(orchestrator)));
-        address accountProxy = address(LibEIP7702.deployProxy(accountImplementation, address(0)));
-        account = MockAccount(payable(accountProxy));
-
-        DelegatedEOA memory d = _randomEIP7702DelegatedEOA();
-
-        PassKey memory k = _randomSecp256k1PassKey();
-        k.k.isSuperAdmin = true;
-
-        vm.startPrank(d.eoa);
-
-        d.d.authorize(k.k);
-
-        k = _randomSecp256r1PassKey();
-        k.k.isSuperAdmin = true;
-        vm.expectRevert(bytes4(keccak256("KeyTypeCannotBeSuperAdmin()")));
-        d.d.authorize(k.k);
-
-        vm.stopPrank();
-    }
-
     function testCrossChainKeyPreCallsAuthorization() public {
         // Setup Keys
         PassKey memory adminKey = _randomSecp256k1PassKey();
diff --git a/test/GuardedExecutor.t.sol b/test/GuardedExecutor.t.sol
@@ -654,127 +654,6 @@ contract GuardedExecutorTest is BaseTest {
         }
     }
 
-    function testSetSpendLimitsEnabled() public {
-        Orchestrator.Intent memory u;
-        DelegatedEOA memory d = _randomEIP7702DelegatedEOA();
-
-        u.eoa = d.eoa;
-        u.combinedGas = 1000000;
-        u.nonce = d.d.getNonce(0);
-
-        PassKey memory k = _randomSecp256k1PassKey();
-
-        address token = LibClone.clone(address(paymentToken));
-        _mint(token, u.eoa, type(uint192).max);
-
-        // Test that spend limits are enabled by default for the key
-        assertTrue(d.d.spendLimitsEnabled(k.keyHash), "Spend limits should be enabled by default");
-
-        // Authorize the key and set up spend limit
-        {
-            ERC7821.Call[] memory calls = new ERC7821.Call[](3);
-            calls[0].data = abi.encodeWithSelector(IthacaAccount.authorize.selector, k.k);
-            // Only allow the key to execute on the token, not on the account itself
-            calls[1].data = abi.encodeWithSelector(
-                GuardedExecutor.setCanExecute.selector, k.keyHash, token, _ANY_FN_SEL, true
-            );
-            calls[2] = _setSpendLimitCall(k, token, GuardedExecutor.SpendPeriod.Day, 1 ether);
-
-            u.executionData = abi.encode(calls);
-            u.nonce = 0xc1d0 << 240;
-            u.signature = _eoaSig(d.privateKey, u);
-
-            assertEq(oc.execute(abi.encode(u)), 0);
-        }
-
-        // Test that non-EOA cannot call setSpendLimitsEnabled
-        {
-            ERC7821.Call[] memory calls = new ERC7821.Call[](1);
-            calls[0].to = address(0);
-            calls[0].data = abi.encodeWithSelector(
-                GuardedExecutor.setSpendLimitsEnabled.selector, k.keyHash, false
-            );
-
-            u.nonce = d.d.getNonce(0);
-            u.executionData = abi.encode(calls);
-            u.signature = _sig(k, u);
-
-            // This should fail because setSpendLimitsEnabled requires msg.sender == address(this)
-            // and the key is not authorized to self-execute
-            assertEq(
-                oc.execute(abi.encode(u)),
-                bytes4(keccak256("UnauthorizedCall(bytes32,address,bytes)"))
-            );
-        }
-
-        // Test that EOA can disable spend limits with event
-        {
-            ERC7821.Call[] memory calls = new ERC7821.Call[](1);
-            calls[0].to = address(0);
-            calls[0].data = abi.encodeWithSelector(
-                GuardedExecutor.setSpendLimitsEnabled.selector, k.keyHash, false
-            );
-
-            u.nonce = d.d.getNonce(0);
-            u.executionData = abi.encode(calls);
-            u.signature = _eoaSig(d.privateKey, u);
-
-            vm.expectEmit(true, true, true, true);
-            emit GuardedExecutor.SpendLimitsEnabledSet(k.keyHash, false);
-
-            assertEq(oc.execute(abi.encode(u)), 0);
-            assertFalse(d.d.spendLimitsEnabled(k.keyHash), "Spend limits should be disabled");
-        }
-
-        // Test that when disabled, spend limits are not enforced (can spend beyond limit)
-        {
-            ERC7821.Call[] memory calls = new ERC7821.Call[](1);
-            // Try to transfer 2 ether (exceeds the 1 ether daily limit)
-            calls[0] = _transferCall2(token, address(0xb0b), 2 ether);
-
-            u.nonce = d.d.getNonce(0);
-            u.executionData = abi.encode(calls);
-            u.signature = _sig(k, u);
-
-            // Should succeed even though it exceeds the limit
-            assertEq(oc.execute(abi.encode(u)), 0);
-            assertEq(_balanceOf(token, address(0xb0b)), 2 ether);
-        }
-
-        // Test that EOA can re-enable spend limits with event
-        {
-            ERC7821.Call[] memory calls = new ERC7821.Call[](1);
-            calls[0].to = address(0);
-            calls[0].data = abi.encodeWithSelector(
-                GuardedExecutor.setSpendLimitsEnabled.selector, k.keyHash, true
-            );
-
-            u.nonce = d.d.getNonce(0);
-            u.executionData = abi.encode(calls);
-            u.signature = _eoaSig(d.privateKey, u);
-
-            vm.expectEmit(true, true, true, true);
-            emit GuardedExecutor.SpendLimitsEnabledSet(k.keyHash, true);
-
-            assertEq(oc.execute(abi.encode(u)), 0);
-            assertTrue(d.d.spendLimitsEnabled(k.keyHash), "Spend limits should be enabled again");
-        }
-
-        // Test that when re-enabled, spend limits are enforced again
-        {
-            ERC7821.Call[] memory calls = new ERC7821.Call[](1);
-            // Try to transfer 2 ether again (exceeds the 1 ether daily limit)
-            calls[0] = _transferCall2(token, address(0xb0b), 2 ether);
-
-            u.nonce = d.d.getNonce(0);
-            u.executionData = abi.encode(calls);
-            u.signature = _sig(k, u);
-
-            // Should fail now that limits are enforced
-            assertEq(oc.execute(abi.encode(u)), bytes4(keccak256("ExceededSpendLimit(address)")));
-        }
-    }
-
     function testSpends(bytes32) public {
         Orchestrator.Intent memory u;
         DelegatedEOA memory d = _randomEIP7702DelegatedEOA();
diff --git a/test/UpgradeTest.t.sol b/test/UpgradeTest.t.sol
diff --git a/test/utils/mocks/MockAccount.sol b/test/utils/mocks/MockAccount.sol
