fix: timelock policy create proposal using erc4337 with no-op calldata

src/policies/TimelockPolicy.sol

@@ -10,10 +10,7 @@ import {
     MODULE_TYPE_POLICY,
     MODULE_TYPE_STATELESS_VALIDATOR,
     MODULE_TYPE_STATELESS_VALIDATOR_WITH_SENDER,
-    SIG_VALIDATION_SUCCESS_UINT,
-    SIG_VALIDATION_FAILED_UINT,
-    ERC1271_MAGICVALUE,
-    ERC1271_INVALID
+    SIG_VALIDATION_FAILED_UINT
 } from "src/types/Constants.sol";
 
 /**
@@ -39,11 +36,15 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW
         ProposalStatus status;
         uint48 validAfter; // Timestamp when proposal becomes executable
         uint48 validUntil; // Timestamp when proposal expires
+        uint256 epoch; // Epoch when proposal was created
     }
 
     // Storage: id => wallet => config
     mapping(bytes32 => mapping(address => TimelockConfig)) public timelockConfig;
 
+    // Storage: id => wallet => epoch (persists across uninstall/reinstall)
+    mapping(bytes32 => mapping(address => uint256)) public currentEpoch;
+
     // Storage: userOpKey => id => wallet => proposal
     // userOpKey = keccak256(abi.encode(account, keccak256(callData), nonce))
     mapping(bytes32 => mapping(bytes32 => mapping(address => Proposal))) public proposals;
@@ -66,6 +67,8 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW
     error ProposalExpired(uint256 validUntil, uint256 currentTime);
     error ProposalNotPending();
     error OnlyAccount();
+    error ProposalFromPreviousEpoch();
+    error ParametersTooLarge();
 
     /**
      * @notice Install the timelock policy
@@ -80,6 +83,13 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW
 
         if (delay == 0) revert InvalidDelay();
         if (expirationPeriod == 0) revert InvalidExpirationPeriod();
+        // Prevent uint48 overflow in createProposal: uint48(block.timestamp) + delay + expirationPeriod
+        if (uint256(delay) + uint256(expirationPeriod) > type(uint48).max - block.timestamp) {
+            revert ParametersTooLarge();
+        }
+
+        // Increment epoch to invalidate any proposals from previous installations
+        currentEpoch[id][msg.sender]++;
 
         timelockConfig[id][msg.sender] =
             TimelockConfig({delay: delay, expirationPeriod: expirationPeriod, initialized: true});
@@ -131,9 +141,9 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW
             revert ProposalAlreadyExists();
         }
 
-        // Create proposal (stored by userOpKey)
+        // Create proposal (stored by userOpKey) with current epoch
         proposals[userOpKey][id][account] =
-            Proposal({status: ProposalStatus.Pending, validAfter: validAfter, validUntil: validUntil});
+            Proposal({status: ProposalStatus.Pending, validAfter: validAfter, validUntil: validUntil, epoch: currentEpoch[id][account]});
 
         emit ProposalCreated(account, id, userOpKey, validAfter, validUntil);
     }
@@ -219,14 +229,15 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW
             return SIG_VALIDATION_FAILED_UINT; // Proposal already exists
         }
 
-        // Create proposal
+        // Create proposal with current epoch
         proposals[userOpKey][id][account] =
-            Proposal({status: ProposalStatus.Pending, validAfter: validAfter, validUntil: validUntil});
+            Proposal({status: ProposalStatus.Pending, validAfter: validAfter, validUntil: validUntil, epoch: currentEpoch[id][account]});
 
         emit ProposalCreated(account, id, userOpKey, validAfter, validUntil);
 
-        // Return failure to prevent execution (this was just proposal creation)
-        return SIG_VALIDATION_FAILED_UINT;
+        // Return success (validationData = 0) to allow the proposal creation to persist
+        // EntryPoint treats validationData == 0 as valid (no time range check)
+        return _packValidationData(0, 0);
     }
 
     /**
@@ -244,6 +255,9 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW
         // Check proposal exists and is pending
         if (proposal.status != ProposalStatus.Pending) return SIG_VALIDATION_FAILED_UINT;
 
+        // Check proposal is from current epoch (not a stale proposal from previous installation)
+        if (proposal.epoch != currentEpoch[id][account]) return SIG_VALIDATION_FAILED_UINT;
+
         // Mark as executed
         proposal.status = ProposalStatus.Executed;
 
@@ -290,20 +304,22 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW
      */
     function _isNoOpERC7579Execute(bytes calldata callData) internal view returns (bool) {
         // execute(bytes32 mode, bytes calldata executionCalldata)
-        // Need: 4 (selector) + 32 (mode) + 32 (offset) + 32 (length) + data
-        if (callData.length < 68) return false;
+        // ABI layout: 4 (selector) + 32 (mode) + 32 (offset) + 32 (length) + data
+        if (callData.length < 100) return false;
 
-        // Decode the offset to executionCalldata (should be 32)
+        // Offset to executionCalldata: 2 head slots (mode + offset) = 64
         uint256 offset = uint256(bytes32(callData[36:68]));
-        if (offset != 32) return false;
+        if (offset != 64) return false;
 
         // Decode the length of executionCalldata
-        if (callData.length < 100) return false;
         uint256 execDataLength = uint256(bytes32(callData[68:100]));
 
-        // For single execution mode, executionCalldata format is:
-        // target (20 bytes) + value (32 bytes) + calldata (variable)
-        if (execDataLength < 52) return false;
+        // ERC-7579 single execution uses compact format (no length prefix):
+        // executionCalldata = abi.encodePacked(target, value, calldata)
+        // target (20 bytes) + value (32 bytes) = 52 bytes with no inner calldata
+        if (execDataLength != 52) return false;
+
+        if (callData.length < 152) return false;
 
         // Extract target address (first 20 bytes of executionCalldata)
         address target = address(bytes20(callData[100:120]));
@@ -315,26 +331,14 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW
         uint256 value = uint256(bytes32(callData[120:152]));
 
         // Value must be 0
-        if (value != 0) return false;
-
-        // Check calldata length (remaining bytes should indicate empty calldata)
-        // executionCalldata = target(20) + value(32) + calldataLength(32) + calldata
-        if (callData.length < 184) {
-            // If we don't have enough for calldata length field, it's malformed
-            return false;
-        }
-
-        uint256 innerCalldataLength = uint256(bytes32(callData[152:184]));
-
-        // Inner calldata must be empty
-        return innerCalldataLength == 0;
+        return value == 0;
     }
 
     /**
      * @notice Check if executeUserOp call is a no-op
      * @dev Valid: executeUserOp("", bytes32)
      */
-    function _isNoOpExecuteUserOp(bytes calldata callData) internal view returns (bool) {
+    function _isNoOpExecuteUserOp(bytes calldata callData) internal pure returns (bool) {
         // executeUserOp(bytes calldata userOp, bytes32 userOpHash)
         // Format: 4 (selector) + 32 (userOp offset) + 32 (userOpHash) + 32 (userOp length) + userOp data
         if (callData.length < 100) return false;
@@ -368,37 +372,33 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW
 
     /**
      * @notice Check signature against timelock policy (for ERC-1271)
-     * @param id The policy ID
-     * @return validationData 0 if valid, 1 if invalid
+     * @dev TimelockPolicy does not support ERC-1271 signature validation - always reverts
      */
-    function checkSignaturePolicy(bytes32 id, address, bytes32 hash, bytes calldata sig)
+    function checkSignaturePolicy(bytes32, address, bytes32, bytes calldata)
         external
-        view
+        pure
         override
         returns (uint256)
     {
-        bytes4 result = _validateSignaturePolicy(id, msg.sender, hash, sig);
-        return result == ERC1271_MAGICVALUE ? 0 : 1;
+        revert("TimelockPolicy: signature validation not supported");
     }
 
-    function validateSignatureWithData(bytes32, bytes calldata, bytes calldata data)
+    function validateSignatureWithData(bytes32, bytes calldata, bytes calldata)
         external
         pure
         override(IStatelessValidator)
         returns (bool)
     {
-        (uint48 delay, uint48 expirationPeriod) = abi.decode(data, (uint48, uint48));
-        return delay != 0 && expirationPeriod != 0;
+        revert("TimelockPolicy: stateless signature validation not supported");
     }
 
-    function validateSignatureWithDataWithSender(address, bytes32, bytes calldata, bytes calldata data)
+    function validateSignatureWithDataWithSender(address, bytes32, bytes calldata, bytes calldata)
         external
         pure
         override(IStatelessValidatorWithSender)
         returns (bool)
     {
-        (uint48 delay, uint48 expirationPeriod) = abi.decode(data, (uint48, uint48));
-        return delay != 0 && expirationPeriod != 0;
+        revert("TimelockPolicy: stateless signature validation not supported");
     }
 
     // ==================== Internal Shared Logic ====================
@@ -425,23 +425,6 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW
         return _handleProposalExecutionInternal(id, userOp, account);
     }
 
-    /**
-     * @notice Internal function to validate signature policy
-     * @dev Shared logic for both installed and stateless validator modes
-     */
-    function _validateSignaturePolicy(bytes32 id, address account, bytes32 hash, bytes calldata sig)
-        internal
-        view
-        returns (bytes4)
-    {
-        TimelockConfig storage config = timelockConfig[id][account];
-        if (!config.initialized) return ERC1271_INVALID;
-
-        // For signature validation, we're more permissive
-        // Timelock is primarily for userOp execution
-        return ERC1271_MAGICVALUE;
-    }
-
     /**
      * @notice Get proposal details
      * @param account The account address

test/TimelockPolicy.t.sol

@@ -107,21 +107,33 @@ contract TimelockPolicyTest is PolicyTestBase, StatelessValidatorTestBase, State
         return statelessValidationSignature(bytes32(0), valid);
     }
 
-    // Override stateless validator tests to use proper data parameter
+    // Override stateless validator tests - TimelockPolicy reverts for stateless validation
     function testStatlessValidatorFail() external override {
         IStatelessValidator validatorModule = IStatelessValidator(address(module));
 
         bytes32 message = keccak256(abi.encodePacked("TEST_MESSAGE"));
         (, bytes memory sig) = statelessValidationSignature(message, false);
 
-        // For TimelockPolicy, validation fails if delay or expirationPeriod is 0
-        bytes memory invalidData = abi.encode(uint48(0), uint48(0));
+        bytes memory data = abi.encode(uint48(0), uint48(0));
 
         vm.startPrank(WALLET);
-        bool result = validatorModule.validateSignatureWithData(message, sig, invalidData);
+        vm.expectRevert("TimelockPolicy: stateless signature validation not supported");
+        validatorModule.validateSignatureWithData(message, sig, data);
         vm.stopPrank();
+    }
+
+    function testStatelessValidatorSuccess() external override {
+        IStatelessValidator validatorModule = IStatelessValidator(address(module));
+
+        bytes32 message = keccak256(abi.encodePacked("TEST_MESSAGE"));
+        (, bytes memory sig) = statelessValidationSignature(message, true);
+
+        bytes memory validData = abi.encode(delay, expirationPeriod);
 
-        assertFalse(result);
+        vm.startPrank(WALLET);
+        vm.expectRevert("TimelockPolicy: stateless signature validation not supported");
+        validatorModule.validateSignatureWithData(message, sig, validData);
+        vm.stopPrank();
     }
 
     function testStatelessValidatorWithSenderFail() external override {
@@ -130,14 +142,26 @@ contract TimelockPolicyTest is PolicyTestBase, StatelessValidatorTestBase, State
         bytes32 message = keccak256(abi.encodePacked("TEST_MESSAGE"));
         (address caller, bytes memory sig) = statelessValidationSignatureWithSender(message, false);
 
-        // For TimelockPolicy, validation fails if delay or expirationPeriod is 0
-        bytes memory invalidData = abi.encode(uint48(0), uint48(0));
+        bytes memory data = abi.encode(uint48(0), uint48(0));
 
         vm.startPrank(WALLET);
-        bool result = validatorModule.validateSignatureWithDataWithSender(caller, message, sig, invalidData);
+        vm.expectRevert("TimelockPolicy: stateless signature validation not supported");
+        validatorModule.validateSignatureWithDataWithSender(caller, message, sig, data);
         vm.stopPrank();
+    }
+
+    function testStatelessValidatorWithSenderSuccess() external override {
+        IStatelessValidatorWithSender validatorModule = IStatelessValidatorWithSender(address(module));
 
-        assertFalse(result);
+        bytes32 message = keccak256(abi.encodePacked("TEST_MESSAGE"));
+        (address caller, bytes memory sig) = statelessValidationSignatureWithSender(message, true);
+
+        bytes memory validData = abi.encode(delay, expirationPeriod);
+
+        vm.startPrank(WALLET);
+        vm.expectRevert("TimelockPolicy: stateless signature validation not supported");
+        validatorModule.validateSignatureWithDataWithSender(caller, message, sig, validData);
+        vm.stopPrank();
     }
 
     // Override the checkUserOpPolicy tests because TimelockPolicy has special behavior
@@ -184,22 +208,32 @@ contract TimelockPolicyTest is PolicyTestBase, StatelessValidatorTestBase, State
         assertEq(validationResult, 1);
     }
 
-    // Override signature policy test because TimelockPolicy always passes for installed accounts
-    function testPolicyCheckSignaturePolicyFail() public payable override {
+    // Override signature policy tests - TimelockPolicy reverts for signature validation
+    function testPolicyCheckSignaturePolicySuccess() public payable override {
         TimelockPolicy policyModule = TimelockPolicy(address(module));
+        vm.startPrank(WALLET);
+        policyModule.onInstall(abi.encodePacked(policyId(), installData()));
+        vm.stopPrank();
+
+        bytes32 testHash = keccak256(abi.encodePacked("TEST_HASH"));
+        (address sender, bytes memory sigData) = validSignatureData(testHash);
 
-        // Don't install for this wallet
-        address nonInstalledWallet = address(0xBEEF);
+        vm.startPrank(WALLET);
+        vm.expectRevert("TimelockPolicy: signature validation not supported");
+        policyModule.checkSignaturePolicy(policyId(), sender, testHash, sigData);
+        vm.stopPrank();
+    }
+
+    function testPolicyCheckSignaturePolicyFail() public payable override {
+        TimelockPolicy policyModule = TimelockPolicy(address(module));
 
         bytes32 testHash = keccak256(abi.encodePacked("TEST_HASH"));
         (address sender, bytes memory sigData) = invalidSignatureData(testHash);
 
-        vm.startPrank(nonInstalledWallet);
-        uint256 result = policyModule.checkSignaturePolicy(policyId(), sender, testHash, sigData);
+        vm.startPrank(WALLET);
+        vm.expectRevert("TimelockPolicy: signature validation not supported");
+        policyModule.checkSignaturePolicy(policyId(), sender, testHash, sigData);
         vm.stopPrank();
-
-        // Should fail for non-installed account
-        assertFalse(result == 0);
     }
 
     // Additional TimelockPolicy-specific tests
@@ -284,13 +318,50 @@ contract TimelockPolicyTest is PolicyTestBase, StatelessValidatorTestBase, State
         uint256 result = policyModule.checkUserOpPolicy(policyId(), userOp);
         vm.stopPrank();
 
-        // Should return failure (1) because this was proposal creation, not execution
-        assertEq(result, 1);
+        // Returns success (validationData = 0) - valid indefinitely per ERC-4337
+        // This allows proposal creation via UserOp without external caller
+        assertEq(result, 0);
 
         // Verify proposal was created
         (TimelockPolicy.ProposalStatus status,,) =
             policyModule.getProposal(WALLET, proposalCallData, proposalNonce, policyId(), WALLET);
 
         assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending));
     }
+
+    // Test that stale proposals from previous installations cannot be executed
+    function testStaleProposalNotExecutableAfterReinstall() public {
+        TimelockPolicy policyModule = TimelockPolicy(address(module));
+        vm.startPrank(WALLET);
+        policyModule.onInstall(abi.encodePacked(policyId(), installData()));
+        vm.stopPrank();
+
+        PackedUserOperation memory userOp = validUserOp();
+
+        // Create a proposal
+        vm.startPrank(WALLET);
+        policyModule.createProposal(policyId(), WALLET, userOp.callData, userOp.nonce);
+        vm.stopPrank();
+
+        // Fast forward past delay
+        vm.warp(block.timestamp + delay + 1);
+
+        // Uninstall the policy
+        vm.startPrank(WALLET);
+        policyModule.onUninstall(abi.encodePacked(policyId(), ""));
+        vm.stopPrank();
+
+        // Reinstall the policy
+        vm.startPrank(WALLET);
+        policyModule.onInstall(abi.encodePacked(policyId(), installData()));
+        vm.stopPrank();
+
+        // Try to execute the stale proposal - should fail
+        vm.startPrank(WALLET);
+        uint256 validationResult = policyModule.checkUserOpPolicy(policyId(), userOp);
+        vm.stopPrank();
+
+        // Should fail (return 1 = SIG_VALIDATION_FAILED_UINT) because proposal is from previous epoch
+        assertEq(validationResult, 1);
+    }
 }

test/base/PolicyTestBase.sol

@@ -98,7 +98,7 @@ abstract contract PolicyTestBase is ModuleTestBase {
         assertFalse(validationResult == 0);
     }
 
-    function testPolicyCheckSignaturePolicySuccess() public payable {
+    function testPolicyCheckSignaturePolicySuccess() public payable virtual {
         IPolicy policyModule = IPolicy(address(module));
         vm.startPrank(WALLET);
         policyModule.onInstall(abi.encodePacked(policyId(), installData()));