fix: improve session key gas estimation

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";
@@ -182,15 +184,19 @@ 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) > 0) {
+                revert PermissionsCheckFailed();
             }
-            revert InvalidSignature(sessionKey);
+            return validation | (sessionKey == recoveredSig ? SIG_VALIDATION_PASSED : SIG_VALIDATION_FAILED);
         }
         revert NotImplemented(msg.sig, functionId);
     }

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

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

@@ -139,7 +139,7 @@ contract SessionKeyPermissionsTest is Test {
             sessionKey1Private,
             abi.encodeCall(Counter.increment, ()),
             0 wei,
-            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error")
+            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA23 reverted (or OOG)")
         );
 
         // Call should fail before removing the allowlist
@@ -198,7 +198,7 @@ contract SessionKeyPermissionsTest is Test {
             sessionKey1Private,
             abi.encodeCall(Counter.increment, ()),
             0 wei,
-            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error")
+            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA23 reverted (or OOG)")
         );
 
         assertEq(counter2.number(), 1);
@@ -223,7 +223,7 @@ contract SessionKeyPermissionsTest is Test {
             sessionKey1Private,
             abi.encodeCall(Counter.increment, ()),
             0 wei,
-            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error")
+            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA23 reverted (or OOG)")
         );
 
         assertEq(counter1.number(), 1);
@@ -284,7 +284,7 @@ contract SessionKeyPermissionsTest is Test {
             sessionKey1Private,
             abi.encodeCall(Counter.setNumber, (5)),
             0 wei,
-            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error")
+            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA23 reverted (or OOG)")
         );
 
         assertEq(counter1.number(), 2);
@@ -314,7 +314,7 @@ contract SessionKeyPermissionsTest is Test {
             sessionKey1Private,
             abi.encodeCall(Counter.increment, ()),
             0 wei,
-            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error")
+            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA23 reverted (or OOG)")
         );
 
         assertEq(counter1.number(), 1);
@@ -328,8 +328,12 @@ contract SessionKeyPermissionsTest is Test {
     }
 
     function testFuzz_sessionKeyTimeRange(uint48 startTime, uint48 endTime) public {
-        bytes[] memory updates = new bytes[](1);
+        bytes[] memory updates = new bytes[](2);
         updates[0] = abi.encodeCall(ISessionKeyPermissionsUpdates.updateTimeRange, (startTime, endTime));
+        updates[1] = abi.encodeCall(
+            ISessionKeyPermissionsUpdates.setAccessListType,
+            (ISessionKeyPlugin.ContractAccessControlType.ALLOW_ALL_ACCESS)
+        );
 
         vm.prank(owner1);
         SessionKeyPlugin(address(account1)).updateKeyPermissions(sessionKey1, updates);
@@ -395,7 +399,7 @@ contract SessionKeyPermissionsTest is Test {
             sessionKey1Private,
             abi.encodeCall(Counter.increment, ()),
             0 wei,
-            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA24 signature error")
+            abi.encodeWithSelector(IEntryPoint.FailedOp.selector, 0, "AA23 reverted (or OOG)")
         );
 
         // Attempting to use the new key should succeed
@@ -504,15 +508,16 @@ contract SessionKeyPermissionsTest is Test {
         vm.prank(owner1);
         SessionKeyPlugin(address(account1)).updateKeyPermissions(sessionKey1, updates2);
 
-        vm.prank(address(entryPoint));
-        validationData = account1.validateUserOp(userOp, userOpHash, 0);
+        vm.startPrank(address(entryPoint));
 
         if (requiredPaymaster == providedPaymaster || requiredPaymaster == address(0)) {
             // Assert that validation passes
+            validationData = account1.validateUserOp(userOp, userOpHash, 0);
             assertEq(uint160(validationData), 0);
         } else {
             // Assert that validation fails
-            assertEq(uint160(validationData), 1);
+            vm.expectRevert(ISessionKeyPlugin.PermissionsCheckFailed.selector);
+            validationData = account1.validateUserOp(userOp, userOpHash, 0);
         }
     }