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";
 
 /**
@@ -32,18 +29,24 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW
     struct TimelockConfig {
         uint48 delay; // Timelock delay in seconds
         uint48 expirationPeriod; // How long after validAfter the proposal remains valid
+        uint48 gracePeriod; // Period after validAfter during which only owner can execute/cancel
         bool initialized;
     }
 
     struct Proposal {
         ProposalStatus status;
-        uint48 validAfter; // Timestamp when proposal becomes executable
+        uint48 validAfter; // Timestamp when timelock passes (grace period starts)
+        uint48 graceEnd; // Timestamp when grace period ends (public execution allowed)
         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;
@@ -56,35 +59,48 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW
 
     event ProposalCancelled(address indexed wallet, bytes32 indexed id, bytes32 indexed proposalHash);
 
-    event TimelockConfigUpdated(address indexed wallet, bytes32 indexed id, uint256 delay, uint256 expirationPeriod);
+    event TimelockConfigUpdated(
+        address indexed wallet, bytes32 indexed id, uint256 delay, uint256 expirationPeriod, uint256 gracePeriod
+    );
 
     error InvalidDelay();
     error InvalidExpirationPeriod();
+    error InvalidGracePeriod();
     error ProposalNotFound();
     error ProposalAlreadyExists();
     error TimelockNotExpired(uint256 validAfter, uint256 currentTime);
     error ProposalExpired(uint256 validUntil, uint256 currentTime);
     error ProposalNotPending();
     error OnlyAccount();
+    error ProposalFromPreviousEpoch();
+    error ParametersTooLarge();
 
     /**
      * @notice Install the timelock policy
-     * @param _data Encoded: (uint48 delay, uint48 expirationPeriod)
+     * @param _data Encoded: (uint48 delay, uint48 expirationPeriod, uint48 gracePeriod)
      */
     function _policyOninstall(bytes32 id, bytes calldata _data) internal override {
-        (uint48 delay, uint48 expirationPeriod) = abi.decode(_data, (uint48, uint48));
+        (uint48 delay, uint48 expirationPeriod, uint48 gracePeriod) = abi.decode(_data, (uint48, uint48, uint48));
 
         if (timelockConfig[id][msg.sender].initialized) {
             revert IModule.AlreadyInitialized(msg.sender);
         }
 
         if (delay == 0) revert InvalidDelay();
         if (expirationPeriod == 0) revert InvalidExpirationPeriod();
+        if (gracePeriod == 0) revert InvalidGracePeriod();
+        // Prevent uint48 overflow in createProposal: uint48(block.timestamp) + delay + gracePeriod + expirationPeriod
+        if (uint256(delay) + uint256(gracePeriod) + 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});
+            TimelockConfig({delay: delay, expirationPeriod: expirationPeriod, gracePeriod: gracePeriod, initialized: true});
 
-        emit TimelockConfigUpdated(msg.sender, id, delay, expirationPeriod);
+        emit TimelockConfigUpdated(msg.sender, id, delay, expirationPeriod, gracePeriod);
     }
 
     /**
@@ -120,8 +136,12 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW
         if (!config.initialized) revert IModule.NotInitialized(account);
 
         // Calculate proposal timing
+        // validAfter: when timelock passes (grace period starts)
+        // graceEnd: when grace period ends (public execution allowed)
+        // validUntil: when proposal expires
         uint48 validAfter = uint48(block.timestamp) + config.delay;
-        uint48 validUntil = validAfter + config.expirationPeriod;
+        uint48 graceEnd = validAfter + config.gracePeriod;
+        uint48 validUntil = graceEnd + config.expirationPeriod;
 
         // Create userOp key for storage lookup
         bytes32 userOpKey = keccak256(abi.encode(account, keccak256(callData), nonce));
@@ -131,9 +151,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, graceEnd: graceEnd, validUntil: validUntil, epoch: currentEpoch[id][account]});
 
         emit ProposalCreated(account, id, userOpKey, validAfter, validUntil);
     }
@@ -209,7 +229,8 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW
 
         // Calculate proposal timing
         uint48 validAfter = uint48(block.timestamp) + config.delay;
-        uint48 validUntil = validAfter + config.expirationPeriod;
+        uint48 graceEnd = validAfter + config.gracePeriod;
+        uint48 validUntil = graceEnd + config.expirationPeriod;
 
         // Create userOp key for storage lookup (using PROPOSAL calldata and nonce, not current userOp)
         bytes32 userOpKey = keccak256(abi.encode(userOp.sender, keccak256(proposalCallData), proposalNonce));
@@ -219,18 +240,21 @@ 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, graceEnd: graceEnd, 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);
     }
 
     /**
      * @notice Handle proposal execution from userOp
+     * @dev Returns graceEnd as validAfter to prevent execution during grace period.
+     *      This gives the owner time to cancel proposals without race conditions.
      */
     function _handleProposalExecutionInternal(bytes32 id, PackedUserOperation calldata userOp, address account)
         internal
@@ -244,13 +268,17 @@ 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;
 
         emit ProposalExecuted(account, id, userOpKey);
 
-        // Return validAfter and validUntil for EntryPoint to validate timing
-        return _packValidationData(proposal.validAfter, proposal.validUntil);
+        // Return graceEnd (not validAfter) as the earliest execution time
+        // This prevents race conditions by ensuring the owner has a grace period to cancel
+        return _packValidationData(proposal.graceEnd, proposal.validUntil);
     }
 
     /**
@@ -290,20 +318,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 +345,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 +386,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 +439,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
@@ -450,17 +447,18 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW
      * @param id The policy ID
      * @param wallet The wallet address
      * @return status The proposal status
-     * @return validAfter When the proposal becomes valid
+     * @return validAfter When the timelock passes (grace period starts)
+     * @return graceEnd When the grace period ends (public execution allowed)
      * @return validUntil When the proposal expires
      */
     function getProposal(address account, bytes calldata callData, uint256 nonce, bytes32 id, address wallet)
         external
         view
-        returns (ProposalStatus status, uint256 validAfter, uint256 validUntil)
+        returns (ProposalStatus status, uint256 validAfter, uint256 graceEnd, uint256 validUntil)
     {
         bytes32 userOpKey = keccak256(abi.encode(account, keccak256(callData), nonce));
         Proposal storage proposal = proposals[userOpKey][id][wallet];
-        return (proposal.status, proposal.validAfter, proposal.validUntil);
+        return (proposal.status, proposal.validAfter, proposal.graceEnd, proposal.validUntil);
     }
 
     /**