chore: rebase fixes onto v1.0.x

.github/workflows/test.yml

@@ -1,4 +1,4 @@
-name: account-contracts Test CI
+name: test
 
 on: [pull_request, workflow_dispatch]
 

.gitignore

@@ -13,4 +13,8 @@ lcov.info
 
 # deployments
 broadcast/**/run-latest.json
-broadcast/**/dry-run/**/*
+broadcast/**/dry-run/**/*
+
+# misc
+.DS_Store
+**/.DS_Store

README.md

@@ -1,5 +1,15 @@
 # Modular Account
 
+[![gh_ci_badge]][gh_ci_link]
+[![tg_badge]][tg_link]
+
+[gh_ci_badge]: https://github.com/alchemyplatform/modular-account/actions/workflows/test.yml/badge.svg
+[gh_ci_link]: https://github.com/alchemyplatform/modular-account/actions/workflows/test.yml
+[tg_badge]: https://img.shields.io/endpoint?color=neon&logo=telegram&label=chat&url=https://mogyo.ro/quart-apis/tgmembercount?chat_id=modular_account_standards
+[tg_link]: https://t.me/modular_account_standards
+
+![](./img/ma.png)
+
 Alchemy's Modular Account is a maximally modular, upgradeable smart contract account that is compatible with [ERC-4337](https://eips.ethereum.org/EIPS/eip-4337) and [ERC-6900](https://eips.ethereum.org/EIPS/eip-6900).
 
 ## Overview

deployments/mainnets/arb-mainnet.md

@@ -23,4 +23,5 @@ Chain Id: 42161
 
 | Version | Address | Explorer Link | Salt | Deploy Script Run |
 | ------- | ------------------------------------------ | ---------------------------------------------------------------------------------- | ---------------------------- | ------------------------------------------------------------- |
+| v1.0.1  | 0x0000003E0000a96de4058e1E02a62FaaeCf23d8d | [explorer](https://arbiscan.io/address/0x0000003E0000a96de4058e1E02a62FaaeCf23d8d) | `0x4e59b44847b379578588920ca78fbf26c0b4956c1689983b8c7f38000288670c` | [run](../../broadcast/Deploy.s.sol/42161/run-1708463537.json) |
 | v1.0.0  | 0x000000e30a00f600823700E975f1b1ac387f0017 | [explorer](https://arbiscan.io/address/0x000000e30a00f600823700E975f1b1ac387f0017) | `0x27f40fd3b6cb45339dbcecac` | [run](../../broadcast/Deploy.s.sol/42161/run-1707341559.json) |

deployments/mainnets/base-mainnet.md

@@ -23,4 +23,5 @@ Chain Id: 8453
 
 | Version | Address | Explorer Link | Salt | Deploy Script Run |
 | ------- | ------------------------------------------ | ----------------------------------------------------------------------------------- | ---------------------------- | ------------------------------------------------------------ |
+| v1.0.1  | 0x0000003E0000a96de4058e1E02a62FaaeCf23d8d | [explorer](https://basescan.org/address/0x0000003E0000a96de4058e1E02a62FaaeCf23d8d) | `0x4e59b44847b379578588920ca78fbf26c0b4956c1689983b8c7f38000288670c` | [run](../../broadcast/Deploy.s.sol/8453/run-1708463659.json) |
 | v1.0.0  | 0x000000e30a00f600823700E975f1b1ac387f0017 | [explorer](https://basescan.org/address/0x000000e30a00f600823700E975f1b1ac387f0017) | `0x27f40fd3b6cb45339dbcecac` | [run](../../broadcast/Deploy.s.sol/8453/run-1707342085.json) |

deployments/mainnets/eth-mainnet.md

@@ -23,4 +23,5 @@ Chain Id: 1
 
 | Version | Address | Explorer Link | Salt | Deploy Script Run |
 | ------- | ------------------------------------------ | ----------------------------------------------------------------------------------- | ---------------------------- | --------------------------------------------------------- |
+| v1.0.1  | 0x0000003E0000a96de4058e1E02a62FaaeCf23d8d | [explorer](https://etherscan.io/address/0x0000003E0000a96de4058e1E02a62FaaeCf23d8d) | `0x4e59b44847b379578588920ca78fbf26c0b4956c1689983b8c7f38000288670c` | [run](../../broadcast/Deploy.s.sol/1/run-1708463498.json) |
 | v1.0.0  | 0x000000e30a00f600823700E975f1b1ac387f0017 | [explorer](https://etherscan.io/address/0x000000e30a00f600823700E975f1b1ac387f0017) | `0x27f40fd3b6cb45339dbcecac` | [run](../../broadcast/Deploy.s.sol/1/run-1707343636.json) |

deployments/mainnets/opt-mainnet.md

@@ -23,4 +23,5 @@ Chain Id: 10
 
 | Version | Address | Explorer Link | Salt | Deploy Script Run |
 | ------- | ------------------------------------------ | ---------------------------------------------------------------------------------------------- | ---------------------------- | ---------------------------------------------------------- |
+| v1.0.1  | 0x0000003E0000a96de4058e1E02a62FaaeCf23d8d | [explorer](https://optimistic.etherscan.io/address/0x0000003E0000a96de4058e1E02a62FaaeCf23d8d) | `0x4e59b44847b379578588920ca78fbf26c0b4956c1689983b8c7f38000288670c` | [run](../../broadcast/Deploy.s.sol/10/run-1708463520.json) |
 | v1.0.0  | 0x000000e30a00f600823700E975f1b1ac387f0017 | [explorer](https://optimistic.etherscan.io/address/0x000000e30a00f600823700E975f1b1ac387f0017) | `0x27f40fd3b6cb45339dbcecac` | [run](../../broadcast/Deploy.s.sol/10/run-1707342541.json) |

deployments/mainnets/polygon-mainnet.md

@@ -26,5 +26,6 @@ Chain Id: 137
 
 | Version | Address | Explorer Link | Salt | Deploy Script Run |
 | --------------- | ------------------------------------------ | -------------------------------------------------------------------------------------- | -------------------------------------------------------------------- | ----------------------------------------------------------- |
+| v1.0.1          | 0x0000003E0000a96de4058e1E02a62FaaeCf23d8d | [explorer](https://polygonscan.com/address/0x0000003E0000a96de4058e1E02a62FaaeCf23d8d) | `0x4e59b44847b379578588920ca78fbf26c0b4956c1689983b8c7f38000288670c` | [run](../../broadcast/Deploy.s.sol/137/run-1708463552.json) |
 | v1.0.0          | 0x000000e30a00f600823700E975f1b1ac387f0017 | [explorer](https://polygonscan.com/address/0x000000e30a00f600823700E975f1b1ac387f0017) | `0x27f40fd3b6cb45339dbcecac`                                         | [run](../../broadcast/Deploy.s.sol/137/run-1707339917.json) |
 | v1.0.0-alpha.1  | 0x000000AAF83f4cbd58193D30643025ffD6C9e724 | [explorer](https://polygonscan.com/address/0x000000AAF83f4cbd58193D30643025ffD6C9e724) | `0x4e59b44847b379578588920ca78fbf26c0b4956cf3b65a380cd6110000b01942` | [run](../../broadcast/Deploy.s.sol/137/run-1706829813.json) |

deployments/testnets/arb-sepolia.md

@@ -23,4 +23,5 @@ Chain Id: 421614
 
 | Version | Address | Explorer Link | Salt | Deploy Script Run |
 | ------- | ------------------------------------------ | ------------------------------------------------------------------------------------------ | ---------------------------- | -------------------------------------------------------------- |
+| v1.0.1  | 0x0000003E0000a96de4058e1E02a62FaaeCf23d8d | [explorer](https://sepolia.arbiscan.io/address/0x0000003E0000a96de4058e1E02a62FaaeCf23d8d) | `0x4e59b44847b379578588920ca78fbf26c0b4956c1689983b8c7f38000288670c` | [run](../../broadcast/Deploy.s.sol/421614/run-1708463395.json) |
 | v1.0.0  | 0x000000e30a00f600823700E975f1b1ac387f0017 | [explorer](https://sepolia.arbiscan.io/address/0x000000e30a00f600823700E975f1b1ac387f0017) | `0x27f40fd3b6cb45339dbcecac` | [run](../../broadcast/Deploy.s.sol/421614/run-1707339376.json) |

deployments/testnets/opt-sepolia.md

@@ -23,4 +23,5 @@ Chain Id: 11155420
 
 | Version | Address | Explorer Link | Salt | Deploy Script Run |
 | ------- | ------------------------------------------ | ---------------------------------------------------------------------------------------------------- | ---------------------------- | ---------------------------------------------------------------- |
+| v1.0.1  | 0x0000003E0000a96de4058e1E02a62FaaeCf23d8d | [explorer](https://sepolia-optimism.etherscan.io/address/0x0000003E0000a96de4058e1E02a62FaaeCf23d8d) | `0x4e59b44847b379578588920ca78fbf26c0b4956c1689983b8c7f38000288670c` | [run](../../broadcast/Deploy.s.sol/11155420/run-1708463381.json) |
 | v1.0.0  | 0x000000e30a00f600823700E975f1b1ac387f0017 | [explorer](https://sepolia-optimism.etherscan.io/address/0x000000e30a00f600823700E975f1b1ac387f0017) | `0x27f40fd3b6cb45339dbcecac` | [run](../../broadcast/Deploy.s.sol/11155420/run-1707338797.json) |

deployments/testnets/polygon-mumbai.md

@@ -23,4 +23,5 @@ Chain Id: 80001
 
 | Version | Address | Explorer Link | Salt | Deploy Script Run |
 | ------- | ------------------------------------------ | ------------------------------------------------------------------------------------------- | ---------------------------- | ---------------------------------------------------------------- |
+| v1.0.1  | 0x0000003E0000a96de4058e1E02a62FaaeCf23d8d | [explorer](https://mumbai.polygonscan.com/address/0x0000003E0000a96de4058e1E02a62FaaeCf23d8d) | `0x4e59b44847b379578588920ca78fbf26c0b4956c1689983b8c7f38000288670c` | [run](../../broadcast/Deploy.s.sol/80001/run-1708463413.json) |
 | v1.0.0  | 0x000000e30a00f600823700E975f1b1ac387f0017 | [explorer](https://mumbai.polygonscan.com/address/0x000000e30a00f600823700E975f1b1ac387f0017) | `0x27f40fd3b6cb45339dbcecac` | [run](../../broadcast/Deploy.s.sol/80001/run-1707331580.json) |

src/plugins/session/ISessionKeyPlugin.sol

@@ -76,6 +76,7 @@ interface ISessionKeyPlugin {
     error InvalidToken(address token);
     error LengthMismatch();
     error NativeTokenSpendLimitExceeded(address account, address sessionKey);
+    error PermissionsCheckFailed();
     error SessionKeyNotFound(address sessionKey);
 
     // ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━┓

src/plugins/session/SessionKeyPlugin.sol

@@ -35,7 +35,9 @@ import {Call, IStandardExecutor} from "../../interfaces/IStandardExecutor.sol";
 import {
     AssociatedLinkedListSet, AssociatedLinkedListSetLib
 } from "../../libraries/AssociatedLinkedListSetLib.sol";
-import {SetValue, SENTINEL_VALUE, SIG_VALIDATION_FAILED} from "../../libraries/Constants.sol";
+import {
+    SetValue, SENTINEL_VALUE, SIG_VALIDATION_PASSED, SIG_VALIDATION_FAILED
+} from "../../libraries/Constants.sol";
 import {BasePlugin} from "../BasePlugin.sol";
 import {ISessionKeyPlugin} from "./ISessionKeyPlugin.sol";
 import {SessionKeyPermissions} from "./permissions/SessionKeyPermissions.sol";
@@ -53,7 +55,7 @@ contract SessionKeyPlugin is ISessionKeyPlugin, SessionKeyPermissions, BasePlugi
     using AssociatedLinkedListSetLib for AssociatedLinkedListSet;
 
     string internal constant _NAME = "Session Key Plugin";
-    string internal constant _VERSION = "1.0.0";
+    string internal constant _VERSION = "1.0.1";
     string internal constant _AUTHOR = "Alchemy";
 
     // Constants used in the manifest
@@ -182,15 +184,20 @@ contract SessionKeyPlugin is ISessionKeyPlugin, SessionKeyPermissions, BasePlugi
             bytes32 hash = userOpHash.toEthSignedMessageHash();
 
             (address recoveredSig, ECDSA.RecoverError err) = hash.tryRecover(userOp.signature);
-            if (err == ECDSA.RecoverError.NoError) {
-                if (
-                    _sessionKeys.contains(msg.sender, CastLib.toSetValue(sessionKey)) && sessionKey == recoveredSig
-                ) {
-                    return _checkUserOpPermissions(userOp, calls, sessionKey);
-                }
-                return SIG_VALIDATION_FAILED;
+            if (err != ECDSA.RecoverError.NoError) {
+                revert InvalidSignature(sessionKey);
+            }
+
+            if (!_sessionKeys.contains(msg.sender, CastLib.toSetValue(sessionKey))) {
+                revert PermissionsCheckFailed();
+            }
+
+            uint256 validation = _checkUserOpPermissions(userOp, calls, sessionKey);
+            if (uint160(validation) != uint160(SIG_VALIDATION_PASSED)) {
+                revert PermissionsCheckFailed();
             }
-            revert InvalidSignature(sessionKey);
+            // return SIG_VALIDATION_FAILED on sig validation failure only, all other failure modes should revert
+            return validation | (sessionKey == recoveredSig ? SIG_VALIDATION_PASSED : SIG_VALIDATION_FAILED);
         }
         revert NotImplemented(msg.sig, functionId);
     }

src/plugins/session/permissions/SessionKeyPermissions.sol

@@ -140,12 +140,10 @@ abstract contract SessionKeyPermissions is ISessionKeyPlugin, SessionKeyPermissi
             address userOpPaymaster = address(bytes20(userOp.paymasterAndData));
             validationSuccess = validationSuccess && (userOpPaymaster == sessionKeyData.requiredPaymaster);
         }
-        // Validation return data is 1 in the case of an invalid signature,
-        // otherwise a packed struct of the aggregator address (0 here), and two
-        // 6-byte timestamps indicating the start and end times at which the op
-        // is valid.
+        // A packed struct of: SIG_VALIDATION_PASSED or SIG_VALIDATION_FAILED, and two
+        // 6-byte timestamps indicating the start and end times at which the op is valid.
         return uint160(validationSuccess ? SIG_VALIDATION_PASSED : SIG_VALIDATION_FAILED)
-            | (uint256(validUntil) << 160) | (uint256(currentValidAfter) << (208));
+            | (uint256(validUntil) << 160) | (uint256(currentValidAfter) << 208);
     }
 
     /// @dev Checks permissions on a per-call basis. Should be run during user op validation once per `Call` struct

test/plugin/session/SessionKeyPluginWithMultiOwner.t.sol

@@ -372,7 +372,7 @@ contract SessionKeyPluginWithMultiOwnerTest is Test {
         assertEq(result, 0);
     }
 
-    function testFuzz_sessionKey_userOpValidation_mismathcedSig(uint8 sessionKeysSeed, uint64 signerSeed) public {
+    function testFuzz_sessionKey_userOpValidation_mismatchedSig(uint8 sessionKeysSeed, uint64 signerSeed) public {
         _createSessionKeys(sessionKeysSeed);
 
         (address signer, uint256 signerPrivate) =
@@ -405,12 +405,11 @@ contract SessionKeyPluginWithMultiOwnerTest is Test {
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(signerPrivate, userOpHash.toEthSignedMessageHash());
         userOp.signature = abi.encodePacked(r, s, v);
 
-        vm.prank(address(account1));
-        uint256 result = sessionKeyPlugin.userOpValidationFunction(
+        vm.startPrank(address(account1));
+        vm.expectRevert(ISessionKeyPlugin.PermissionsCheckFailed.selector);
+        sessionKeyPlugin.userOpValidationFunction(
             uint8(ISessionKeyPlugin.FunctionId.USER_OP_VALIDATION_SESSION_KEY), userOp, userOpHash
         );
-
-        assertEq(result, 1);
     }
 
     function testFuzz_sessionKey_userOpValidation_invalidSig(uint8 sessionKeysSeed, uint64 signerSeed) public {

test/plugin/session/permissions/SessionKeyERC20SpendLimits.t.sol

@@ -220,7 +220,7 @@ contract SessionKeyERC20SpendLimitsTest is Test {
         _runSessionKeyUserOp(
             calls,
             sessionKey1Private,
-            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error")
+            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA23 reverted (or OOG)")
         );
     }
 

test/plugin/session/permissions/SessionKeyGasLimits.t.sol

@@ -166,7 +166,7 @@ contract SessionKeyGasLimitsTest is Test {
             1,
             200_000 wei,
             sessionKey1Private,
-            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error")
+            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA23 reverted (or OOG)")
         );
     }
 
@@ -206,7 +206,7 @@ contract SessionKeyGasLimitsTest is Test {
             1_000 gwei,
             0.6 ether,
             sessionKey1Private,
-            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error")
+            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA23 reverted (or OOG)")
         );
     }
 
@@ -223,7 +223,7 @@ contract SessionKeyGasLimitsTest is Test {
             100_000, 300_000, 1_000 gwei, 0.4 ether, sessionKey1Private, uint256(nonceKey << 64)
         );
 
-        vm.expectRevert(abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error"));
+        vm.expectRevert(abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA23 reverted (or OOG)"));
         entryPoint.handleOps(userOps, beneficiary);
     }
 
@@ -240,7 +240,7 @@ contract SessionKeyGasLimitsTest is Test {
             2_000 gwei,
             1.2 ether,
             sessionKey1Private,
-            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error")
+            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA23 reverted (or OOG)")
         );
     }
 
@@ -289,7 +289,7 @@ contract SessionKeyGasLimitsTest is Test {
 
         // Run the user ops
         // The second op (index 1) should be the one that fails signature validation.
-        vm.expectRevert(abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 1, "AA24 signature error"));
+        vm.expectRevert(abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 1, "AA23 reverted (or OOG)"));
         entryPoint.handleOps(userOps, beneficiary);
 
         // The lack of usage update should be reflected in the limits
@@ -492,7 +492,7 @@ contract SessionKeyGasLimitsTest is Test {
         // actually usable.
         skip(1 days + 1 minutes);
 
-        vm.expectRevert(abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 2, "AA24 signature error"));
+        vm.expectRevert(abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 2, "AA23 reverted (or OOG)"));
         entryPoint.handleOps(userOps, beneficiary);
     }
 

test/plugin/session/permissions/SessionKeyNativeTokenSpendLimits.t.sol

@@ -187,7 +187,7 @@ contract SessionKeyNativeTokenSpendLimitsTest is Test {
         _runSessionKeyUserOp(
             calls,
             sessionKey1Private,
-            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error")
+            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA23 reverted (or OOG)")
         );
 
         // Run a user op that spends 0 wei, should succeed
@@ -233,7 +233,7 @@ contract SessionKeyNativeTokenSpendLimitsTest is Test {
         _runSessionKeyUserOp(
             calls,
             sessionKey1Private,
-            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error")
+            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA23 reverted (or OOG)")
         );
     }
 
@@ -258,7 +258,7 @@ contract SessionKeyNativeTokenSpendLimitsTest is Test {
         _runSessionKeyUserOp(
             calls,
             sessionKey1Private,
-            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error")
+            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA23 reverted (or OOG)")
         );
     }
 
@@ -302,7 +302,7 @@ contract SessionKeyNativeTokenSpendLimitsTest is Test {
         _runSessionKeyUserOp(
             calls,
             sessionKey1Private,
-            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error")
+            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA23 reverted (or OOG)")
         );
 
         // Assert that the limit is NOT updated
@@ -360,7 +360,7 @@ contract SessionKeyNativeTokenSpendLimitsTest is Test {
         _runSessionKeyUserOp(
             calls,
             sessionKey1Private,
-            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error")
+            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA23 reverted (or OOG)")
         );
 
         // Assert that the limit is NOT updated
@@ -702,7 +702,7 @@ contract SessionKeyNativeTokenSpendLimitsTest is Test {
         _runSessionKeyUserOp(
             calls,
             sessionKey1Private,
-            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error")
+            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA23 reverted (or OOG)")
         );
 
         // Assert that limits are NOT updated