diff --git a/src/policies/TimelockPolicy.sol b/src/policies/TimelockPolicy.sol index 1e3bfef..b88d1e1 100644 --- a/src/policies/TimelockPolicy.sol +++ b/src/policies/TimelockPolicy.sol @@ -21,7 +21,7 @@ import { contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorWithSender { enum ProposalStatus { None, // Proposal doesn't exist - Pending, // Proposal created, waiting for timelock + Pending, // Clock started, waiting for timelock Executed, // Proposal executed Cancelled // Proposal cancelled } @@ -66,13 +66,8 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW 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(); /** @@ -89,7 +84,7 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW 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 + // Prevent uint48 overflow: uint48(block.timestamp) + delay + gracePeriod + expirationPeriod if (uint256(delay) + uint256(gracePeriod) + uint256(expirationPeriod) > type(uint48).max - block.timestamp) { revert ParametersTooLarge(); } @@ -123,41 +118,6 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW || moduleTypeId == MODULE_TYPE_STATELESS_VALIDATOR_WITH_SENDER; } - /** - * @notice Create a proposal for time-delayed execution - * @dev Anyone can create a proposal - the timelock delay provides the security - * @param id The policy ID - * @param account The account address - * @param callData The calldata for the future operation - * @param nonce The nonce for the future operation - */ - function createProposal(bytes32 id, address account, bytes calldata callData, uint256 nonce) external { - TimelockConfig storage config = timelockConfig[id][account]; - 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 graceEnd = validAfter + config.gracePeriod; - uint48 validUntil = graceEnd + config.expirationPeriod; - - // Create userOp key for storage lookup - bytes32 userOpKey = keccak256(abi.encode(account, keccak256(callData), nonce)); - - // Check proposal doesn't already exist - if (proposals[userOpKey][id][account].status != ProposalStatus.None) { - revert ProposalAlreadyExists(); - } - - // Create proposal (stored by userOpKey) with current epoch - proposals[userOpKey][id][account] = - Proposal({status: ProposalStatus.Pending, validAfter: validAfter, graceEnd: graceEnd, validUntil: validUntil, epoch: currentEpoch[id][account]}); - - emit ProposalCreated(account, id, userOpKey, validAfter, validUntil); - } - /** * @notice Cancel a pending proposal * @dev Only the account itself can cancel proposals to prevent griefing @@ -207,8 +167,10 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW } /** - * @notice Handle proposal creation from userOp - * @dev Signature format: [callDataLength(32)][callData][nonce(32)][remaining sig data] + * @notice Handle proposal creation from a no-op UserOp + * @dev Called when the session key holder submits a no-op UserOp with proposal data in the signature. + * Creates a new Pending proposal with the timelock clock started. + * Signature format: [callDataLength(32)][callData][nonce(32)][remaining sig data] */ function _handleProposalCreationInternal( bytes32 id, @@ -221,8 +183,8 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW // Format: [callDataLength(32 bytes)][callData][nonce(32 bytes)][...] uint256 callDataLength = uint256(bytes32(sig[0:32])); - // Validate signature has enough data - if (sig.length < 64 + callDataLength) return SIG_VALIDATION_FAILED_UINT; + // Validate signature has enough data (check callDataLength first to prevent overflow) + if (callDataLength > sig.length || sig.length < 64 + callDataLength) return SIG_VALIDATION_FAILED_UINT; bytes calldata proposalCallData = sig[32:32 + callDataLength]; uint256 proposalNonce = uint256(bytes32(sig[32 + callDataLength:64 + callDataLength])); @@ -235,9 +197,10 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW // Create userOp key for storage lookup (using PROPOSAL calldata and nonce, not current userOp) bytes32 userOpKey = keccak256(abi.encode(userOp.sender, keccak256(proposalCallData), proposalNonce)); - // Check proposal doesn't already exist - if (proposals[userOpKey][id][account].status != ProposalStatus.None) { - return SIG_VALIDATION_FAILED_UINT; // Proposal already exists + Proposal storage proposal = proposals[userOpKey][id][account]; + + if (proposal.status != ProposalStatus.None) { + return SIG_VALIDATION_FAILED_UINT; } // Create proposal with current epoch @@ -245,9 +208,6 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW Proposal({status: ProposalStatus.Pending, validAfter: validAfter, graceEnd: graceEnd, validUntil: validUntil, epoch: currentEpoch[id][account]}); emit ProposalCreated(account, id, userOpKey, validAfter, validUntil); - - // 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); } @@ -283,91 +243,49 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW /** * @notice Check if calldata is a no-op operation - * @dev Valid no-ops: + * @dev Recognizes 4 forms of no-op: * 1. Empty calldata - * 2. ERC-7579 execute(CALL, self, 0, "") - * 3. ERC-7579 execute(CALL, address(0), 0, "") - * 4. executeUserOp with empty calldata + * 2. ERC-7579 execute(mode=0x00, "") — single-call with empty execution data + * 3. executeUserOp + empty inner calldata (just the 4-byte selector) + * 4. executeUserOp + ERC-7579 execute no-op (selector + form 2) */ - function _isNoOpCalldata(bytes calldata callData) internal view returns (bool) { - // 1. Empty calldata is a no-op - if (callData.length == 0) return true; - - // Need at least 4 bytes for selector - if (callData.length < 4) return false; + function _isNoOpCalldata(bytes calldata callData) internal pure returns (bool) { + uint256 len = callData.length; - bytes4 selector = bytes4(callData[0:4]); + // Case 1: Empty calldata + if (len == 0) return true; - // 2. Check for ERC-7579 execute(bytes32 mode, bytes calldata executionCalldata) - if (selector == IERC7579Execution.execute.selector) { - return _isNoOpERC7579Execute(callData); - } + // Case 2: ERC-7579 execute with empty execution data + if (_isNoOpERC7579Execute(callData)) return true; - // 3. Check for executeUserOp(PackedUserOperation calldata userOp, bytes32 userOpHash) - if (selector == IAccountExecute.executeUserOp.selector) { - return _isNoOpExecuteUserOp(callData); + // Cases 3 & 4: executeUserOp wrapper + if (len >= 4 && bytes4(callData[0:4]) == IAccountExecute.executeUserOp.selector) { + // Case 3: executeUserOp + empty (just the selector, no inner data) + if (len == 4) return true; + // Case 4: executeUserOp + ERC-7579 execute no-op + if (_isNoOpERC7579Execute(callData[4:])) return true; } - // Not a recognized no-op return false; } /** - * @notice Check if ERC-7579 execute call is a no-op - * @dev Valid: execute(CALL, self/address(0), 0, "") - */ - function _isNoOpERC7579Execute(bytes calldata callData) internal view returns (bool) { - // execute(bytes32 mode, bytes calldata executionCalldata) - // ABI layout: 4 (selector) + 32 (mode) + 32 (offset) + 32 (length) + data - if (callData.length < 100) return false; - - // Offset to executionCalldata: 2 head slots (mode + offset) = 64 - uint256 offset = uint256(bytes32(callData[36:68])); - if (offset != 64) return false; - - // Decode the length of executionCalldata - uint256 execDataLength = uint256(bytes32(callData[68:100])); - - // 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])); - - // Check if target is self or address(0) - if (target != msg.sender && target != address(0)) return false; - - // Extract value (next 32 bytes) - uint256 value = uint256(bytes32(callData[120:152])); - - // Value must be 0 - return value == 0; - } - - /** - * @notice Check if executeUserOp call is a no-op - * @dev Valid: executeUserOp("", bytes32) + * @notice Check if calldata is an ERC-7579 execute call with empty execution data + * @dev execute(bytes32 mode, bytes calldata executionCalldata) where: + * - mode byte 0 is 0x00 (single call, not batch/delegatecall) + * - executionCalldata is empty + * ABI layout: selector(4) + mode(32) + offset(32) + length(32) = 100 bytes */ - 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; - - // Decode offset to userOp data (should be 32) - uint256 offset = uint256(bytes32(callData[4:36])); - if (offset != 32) return false; - - // userOpHash is at bytes 36-68 (we don't validate it) - - // Decode userOp length - uint256 userOpLength = uint256(bytes32(callData[68:100])); - - // UserOp must be empty - return userOpLength == 0; + function _isNoOpERC7579Execute(bytes calldata callData) internal pure returns (bool) { + if (callData.length != 100) return false; + if (bytes4(callData[0:4]) != IERC7579Execution.execute.selector) return false; + // Mode byte must be 0x00 (single call, not delegatecall or batch) + if (callData[4] != 0x00) return false; + // Offset must be 64 (standard ABI encoding for dynamic param after one fixed param) + if (uint256(bytes32(callData[36:68])) != 64) return false; + // Execution data length must be 0 + if (uint256(bytes32(callData[68:100])) != 0) return false; + return true; } /** @@ -388,12 +306,7 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW * @notice Check signature against timelock policy (for ERC-1271) * @dev TimelockPolicy does not support ERC-1271 signature validation - always reverts */ - function checkSignaturePolicy(bytes32, address, bytes32, bytes calldata) - external - pure - override - returns (uint256) - { + function checkSignaturePolicy(bytes32, address, bytes32, bytes calldata) external pure override returns (uint256) { revert("TimelockPolicy: signature validation not supported"); } @@ -431,7 +344,6 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW // Check if this is a proposal creation request // Criteria: calldata is a no-op AND signature has proposal data (length >= 65) if (_isNoOpCalldata(userOp.callData) && sig.length >= 65) { - // This is a proposal creation request return _handleProposalCreationInternal(id, userOp, config, sig, account); } diff --git a/test/TimelockPolicy.t.sol b/test/TimelockPolicy.t.sol index a12ae09..5b287bf 100644 --- a/test/TimelockPolicy.t.sol +++ b/test/TimelockPolicy.t.sol @@ -174,9 +174,22 @@ contract TimelockPolicyTest is PolicyTestBase, StatelessValidatorTestBase, State PackedUserOperation memory userOp = validUserOp(); - // First create a proposal + // Create proposal via no-op UserOp (creates Pending directly with clock started) + bytes memory sig = + abi.encodePacked(bytes32(userOp.callData.length), userOp.callData, bytes32(userOp.nonce), bytes1(0x00)); + PackedUserOperation memory noopOp = PackedUserOperation({ + sender: WALLET, + nonce: 0, + initCode: "", + callData: "", + accountGasLimits: bytes32(abi.encodePacked(uint128(100000), uint128(200000))), + preVerificationGas: 0, + gasFees: bytes32(abi.encodePacked(uint128(1), uint128(1))), + paymasterAndData: "", + signature: sig + }); vm.startPrank(WALLET); - policyModule.createProposal(policyId(), WALLET, userOp.callData, userOp.nonce); + policyModule.checkUserOpPolicy(policyId(), noopOp); vm.stopPrank(); // Fast forward past the delay AND grace period @@ -248,10 +261,26 @@ contract TimelockPolicyTest is PolicyTestBase, StatelessValidatorTestBase, State bytes memory callData = hex"1234"; uint256 nonce = 1; + // Create proposal via no-op UserOp + bytes memory sig = abi.encodePacked(bytes32(callData.length), callData, bytes32(nonce), bytes1(0x00)); + PackedUserOperation memory noopOp = PackedUserOperation({ + sender: WALLET, + nonce: 0, + initCode: "", + callData: "", + accountGasLimits: bytes32(abi.encodePacked(uint128(100000), uint128(200000))), + preVerificationGas: 0, + gasFees: bytes32(abi.encodePacked(uint128(1), uint128(1))), + paymasterAndData: "", + signature: sig + }); + vm.startPrank(WALLET); - policyModule.createProposal(policyId(), WALLET, callData, nonce); + uint256 result = policyModule.checkUserOpPolicy(policyId(), noopOp); vm.stopPrank(); + assertEq(result, 0); + // Verify proposal was created (TimelockPolicy.ProposalStatus status, uint256 validAfter, uint256 graceEnd, uint256 validUntil) = policyModule.getProposal(WALLET, callData, nonce, policyId(), WALLET); @@ -271,9 +300,22 @@ contract TimelockPolicyTest is PolicyTestBase, StatelessValidatorTestBase, State bytes memory callData = hex"1234"; uint256 nonce = 1; - // Create proposal + // Create proposal via no-op UserOp + bytes memory sig = abi.encodePacked(bytes32(callData.length), callData, bytes32(nonce), bytes1(0x00)); + PackedUserOperation memory noopOp = PackedUserOperation({ + sender: WALLET, + nonce: 0, + initCode: "", + callData: "", + accountGasLimits: bytes32(abi.encodePacked(uint128(100000), uint128(200000))), + preVerificationGas: 0, + gasFees: bytes32(abi.encodePacked(uint128(1), uint128(1))), + paymasterAndData: "", + signature: sig + }); + vm.startPrank(WALLET); - policyModule.createProposal(policyId(), WALLET, callData, nonce); + policyModule.checkUserOpPolicy(policyId(), noopOp); vm.stopPrank(); // Cancel proposal @@ -320,8 +362,7 @@ contract TimelockPolicyTest is PolicyTestBase, StatelessValidatorTestBase, State uint256 result = policyModule.checkUserOpPolicy(policyId(), userOp); vm.stopPrank(); - // Returns success (validationData = 0) - valid indefinitely per ERC-4337 - // This allows proposal creation via UserOp without external caller + // Returns success (validationData = 0) for state persistence assertEq(result, 0); // Verify proposal was created @@ -340,9 +381,22 @@ contract TimelockPolicyTest is PolicyTestBase, StatelessValidatorTestBase, State PackedUserOperation memory userOp = validUserOp(); - // Create a proposal + // Create proposal via no-op UserOp (creates Pending directly) + bytes memory sig = + abi.encodePacked(bytes32(userOp.callData.length), userOp.callData, bytes32(userOp.nonce), bytes1(0x00)); + PackedUserOperation memory noopOp = PackedUserOperation({ + sender: WALLET, + nonce: 0, + initCode: "", + callData: "", + accountGasLimits: bytes32(abi.encodePacked(uint128(100000), uint128(200000))), + preVerificationGas: 0, + gasFees: bytes32(abi.encodePacked(uint128(1), uint128(1))), + paymasterAndData: "", + signature: sig + }); vm.startPrank(WALLET); - policyModule.createProposal(policyId(), WALLET, userOp.callData, userOp.nonce); + policyModule.checkUserOpPolicy(policyId(), noopOp); vm.stopPrank(); // Fast forward past delay @@ -376,9 +430,22 @@ contract TimelockPolicyTest is PolicyTestBase, StatelessValidatorTestBase, State PackedUserOperation memory userOp = validUserOp(); - // Create a proposal + // Create a proposal via no-op UserOp + bytes memory sig = + abi.encodePacked(bytes32(userOp.callData.length), userOp.callData, bytes32(userOp.nonce), bytes1(0x00)); + PackedUserOperation memory noopOp = PackedUserOperation({ + sender: WALLET, + nonce: 0, + initCode: "", + callData: "", + accountGasLimits: bytes32(abi.encodePacked(uint128(100000), uint128(200000))), + preVerificationGas: 0, + gasFees: bytes32(abi.encodePacked(uint128(1), uint128(1))), + paymasterAndData: "", + signature: sig + }); vm.startPrank(WALLET); - policyModule.createProposal(policyId(), WALLET, userOp.callData, userOp.nonce); + policyModule.checkUserOpPolicy(policyId(), noopOp); vm.stopPrank(); // Fast forward past delay but NOT past grace period @@ -411,9 +478,22 @@ contract TimelockPolicyTest is PolicyTestBase, StatelessValidatorTestBase, State bytes memory callData = hex"1234"; uint256 nonce = 1; - // Create proposal + // Create proposal via no-op UserOp + bytes memory sig = + abi.encodePacked(bytes32(callData.length), callData, bytes32(nonce), bytes1(0x00)); + PackedUserOperation memory noopOp = PackedUserOperation({ + sender: WALLET, + nonce: 0, + initCode: "", + callData: "", + accountGasLimits: bytes32(abi.encodePacked(uint128(100000), uint128(200000))), + preVerificationGas: 0, + gasFees: bytes32(abi.encodePacked(uint128(1), uint128(1))), + paymasterAndData: "", + signature: sig + }); vm.startPrank(WALLET); - policyModule.createProposal(policyId(), WALLET, callData, nonce); + policyModule.checkUserOpPolicy(policyId(), noopOp); vm.stopPrank(); // Fast forward past delay but still in grace period diff --git a/test/btt/Timelock.t.sol b/test/btt/Timelock.t.sol index b44b850..e51f3a5 100644 --- a/test/btt/Timelock.t.sol +++ b/test/btt/Timelock.t.sol @@ -193,58 +193,6 @@ contract TimelockTest is Test { assertFalse(timelockPolicy.isModuleType(999), "Should not support invalid type"); } - // ============ createProposal Tests ============ - - modifier whenCallingCreateProposal() { - _; - } - - function test_GivenConfigIsInitializedAndProposalDoesNotExist() external whenCallingCreateProposal { - // it should store the proposal with pending status - // it should set correct validAfter and validUntil - // it should emit ProposalCreated - bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); - uint256 nonce = 100; - - uint256 createTime = block.timestamp; - bytes32 expectedKey = keccak256(abi.encode(WALLET, keccak256(callData), nonce)); - - vm.expectEmit(true, true, true, true); - emit TimelockPolicy.ProposalCreated( - WALLET, POLICY_ID, expectedKey, uint48(createTime) + DELAY, uint48(createTime) + DELAY + GRACE_PERIOD + EXPIRATION - ); - - timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); - - (TimelockPolicy.ProposalStatus status, uint256 validAfter, uint256 graceEnd, uint256 validUntil) = - timelockPolicy.getProposal(WALLET, callData, nonce, POLICY_ID, WALLET); - - assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending), "Status should be Pending"); - assertEq(validAfter, createTime + DELAY, "validAfter should be timestamp + delay"); - assertEq(graceEnd, createTime + DELAY + GRACE_PERIOD, "graceEnd should be validAfter + gracePeriod"); - assertEq(validUntil, createTime + DELAY + GRACE_PERIOD + EXPIRATION, "validUntil should be graceEnd + expiration"); - } - - function test_GivenNotInitialized_WhenCallingCreateProposal() external whenCallingCreateProposal { - // it should revert with NotInitialized - address uninitWallet = address(0x9999); - bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); - - vm.expectRevert(abi.encodeWithSelector(IModule.NotInitialized.selector, uninitWallet)); - timelockPolicy.createProposal(POLICY_ID, uninitWallet, callData, 0); - } - - function test_GivenProposalAlreadyExists() external whenCallingCreateProposal { - // it should revert with ProposalAlreadyExists - bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); - uint256 nonce = 200; - - timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); - - vm.expectRevert(TimelockPolicy.ProposalAlreadyExists.selector); - timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); - } - // ============ cancelProposal Tests ============ modifier whenCallingCancelProposal() { @@ -252,12 +200,16 @@ contract TimelockTest is Test { } function test_GivenCallerIsAccountAndProposalIsPending() external whenCallingCancelProposal { - // it should set status to cancelled + // it should set status to cancelled (cancelling a Pending proposal) // it should emit ProposalCancelled bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); - uint256 nonce = 300; + uint256 nonce = 301; - timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); + // Create proposal via no-op UserOp to get Pending status + bytes memory sig = _createProposalSignature(callData, nonce); + PackedUserOperation memory userOp = _createNoopUserOp(WALLET, sig); + vm.prank(WALLET); + timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); bytes32 expectedKey = keccak256(abi.encode(WALLET, keccak256(callData), nonce)); @@ -277,7 +229,11 @@ contract TimelockTest is Test { bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); uint256 nonce = 400; - timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); + // Create proposal via no-op UserOp + bytes memory sig = _createProposalSignature(callData, nonce); + PackedUserOperation memory noopOp = _createNoopUserOp(WALLET, sig); + vm.prank(WALLET); + timelockPolicy.checkUserOpPolicy(POLICY_ID, noopOp); vm.prank(ATTACKER); vm.expectRevert(TimelockPolicy.OnlyAccount.selector); @@ -309,7 +265,11 @@ contract TimelockTest is Test { bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); uint256 nonce = 600; - timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); + // Create proposal via no-op UserOp + bytes memory sig = _createProposalSignature(callData, nonce); + PackedUserOperation memory noopOp = _createNoopUserOp(WALLET, sig); + vm.prank(WALLET); + timelockPolicy.checkUserOpPolicy(POLICY_ID, noopOp); vm.prank(WALLET); timelockPolicy.cancelProposal(POLICY_ID, WALLET, callData, nonce); @@ -326,9 +286,9 @@ contract TimelockTest is Test { } function test_GivenNoopCalldataAndValidSignature() external whenCallingCheckUserOpPolicyToCreateProposal { - // it should create the proposal + // it should create the proposal as Pending with clock started // it should return zero for state persistence - // it should emit ProposalCreated + // it should emit ProposalCreated with timing bytes memory proposalCallData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); uint256 proposalNonce = 700; bytes memory sig = _createProposalSignature(proposalCallData, proposalNonce); @@ -354,7 +314,7 @@ contract TimelockTest is Test { (TimelockPolicy.ProposalStatus status,,,) = timelockPolicy.getProposal(WALLET, proposalCallData, proposalNonce, POLICY_ID, WALLET); - assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending), "Proposal should be created"); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending), "Proposal should be Pending"); } function test_GivenNoopCalldataAndSignatureShorterThan65Bytes() @@ -390,23 +350,23 @@ contract TimelockTest is Test { assertEq(result, SIG_VALIDATION_FAILED, "Should fail when signature claims more data than available"); } - function test_GivenNoopCalldataAndProposalAlreadyExists() external whenCallingCheckUserOpPolicyToCreateProposal { - // it should return SIG_VALIDATION_FAILED + function test_GivenNoopCalldataAndProposalAlreadyPending() external whenCallingCheckUserOpPolicyToCreateProposal { + // it should return SIG_VALIDATION_FAILED when proposal is already Pending bytes memory proposalCallData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); uint256 proposalNonce = 800; bytes memory sig = _createProposalSignature(proposalCallData, proposalNonce); PackedUserOperation memory userOp = _createNoopUserOp(WALLET, sig); - // First creation should succeed + // First call: create -> Pending vm.prank(WALLET); timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); - // Second creation should fail + // Second call: already Pending -> should fail vm.prank(WALLET); uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); - assertEq(result, SIG_VALIDATION_FAILED, "Should fail for duplicate proposal"); + assertEq(result, SIG_VALIDATION_FAILED, "Should fail for already Pending proposal"); } // ============ checkUserOpPolicy - Proposal Execution Tests ============ @@ -422,8 +382,11 @@ contract TimelockTest is Test { bytes memory proposalCallData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "action"); uint256 proposalNonce = 900; - uint256 createTime = block.timestamp; - timelockPolicy.createProposal(POLICY_ID, WALLET, proposalCallData, proposalNonce); + // Create proposal via no-op UserOp (creates Pending directly) + bytes memory sig = _createProposalSignature(proposalCallData, proposalNonce); + PackedUserOperation memory noopOp = _createNoopUserOp(WALLET, sig); + vm.prank(WALLET); + timelockPolicy.checkUserOpPolicy(POLICY_ID, noopOp); // Get the actual stored proposal values (, uint256 storedValidAfter, uint256 storedGraceEnd, uint256 storedValidUntil) = @@ -469,7 +432,11 @@ contract TimelockTest is Test { bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); uint256 nonce = 1000; - timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); + // Create proposal via no-op UserOp + bytes memory sig = _createProposalSignature(callData, nonce); + PackedUserOperation memory noopOp = _createNoopUserOp(WALLET, sig); + vm.prank(WALLET); + timelockPolicy.checkUserOpPolicy(POLICY_ID, noopOp); vm.prank(WALLET); timelockPolicy.cancelProposal(POLICY_ID, WALLET, callData, nonce); @@ -487,7 +454,11 @@ contract TimelockTest is Test { bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); uint256 nonce = 1100; - timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); + // Create proposal via no-op UserOp + bytes memory sig = _createProposalSignature(callData, nonce); + PackedUserOperation memory noopOp = _createNoopUserOp(WALLET, sig); + vm.prank(WALLET); + timelockPolicy.checkUserOpPolicy(POLICY_ID, noopOp); vm.warp(block.timestamp + DELAY + 1); @@ -619,16 +590,19 @@ contract TimelockTest is Test { bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); uint256 nonce = 1200; - uint256 createTime = block.timestamp; - timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); + // Create proposal via no-op UserOp (creates Pending with timing) + bytes memory sig = _createProposalSignature(callData, nonce); + PackedUserOperation memory noopOp = _createNoopUserOp(WALLET, sig); + vm.prank(WALLET); + timelockPolicy.checkUserOpPolicy(POLICY_ID, noopOp); (TimelockPolicy.ProposalStatus status, uint256 validAfter, uint256 graceEnd, uint256 validUntil) = timelockPolicy.getProposal(WALLET, callData, nonce, POLICY_ID, WALLET); assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending), "Status should be Pending"); - assertEq(validAfter, createTime + DELAY, "validAfter should be correct"); - assertEq(graceEnd, createTime + DELAY + GRACE_PERIOD, "graceEnd should be correct"); - assertEq(validUntil, createTime + DELAY + GRACE_PERIOD + EXPIRATION, "validUntil should be correct"); + assertEq(validAfter, block.timestamp + DELAY, "validAfter should be correct"); + assertEq(graceEnd, block.timestamp + DELAY + GRACE_PERIOD, "graceEnd should be correct"); + assertEq(validUntil, block.timestamp + DELAY + GRACE_PERIOD + EXPIRATION, "validUntil should be correct"); } function test_GivenProposalDoesNotExist_WhenCallingGetProposal() external whenCallingGetProposal { @@ -663,6 +637,7 @@ contract TimelockTest is Test { _; } + // Case 1: Empty calldata function test_GivenCalldataIsEmpty() external whenDetectingNoopCalldata { // it should be detected as noop bytes memory sig = _createProposalSignature("test", 0); @@ -675,253 +650,112 @@ contract TimelockTest is Test { assertEq(result, 0, "Empty calldata should be detected as noop"); } - function test_GivenCalldataIsShorterThan4Bytes() external whenDetectingNoopCalldata { - // it should not be detected as noop - bytes memory shortCalldata = hex"aabb"; - PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, shortCalldata, 0, ""); - - vm.prank(WALLET); - uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); - - // Not a no-op, goes to execution path, no proposal exists - assertEq(result, SIG_VALIDATION_FAILED, "Short calldata should not be noop"); - } - - function test_GivenSelectorIsUnrecognized() external whenDetectingNoopCalldata { - // it should not be detected as noop - bytes memory unknownCalldata = abi.encodeWithSelector(bytes4(0xdeadbeef), "test"); - PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, unknownCalldata, 0, ""); - - vm.prank(WALLET); - uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); - - assertEq(result, SIG_VALIDATION_FAILED, "Unknown selector should not be noop"); - } - - // ============ _isNoOpERC7579Execute Tests ============ - - modifier whenDetectingERC7579ExecuteNoop() { - _; - } - - function test_GivenTargetIsSelfAndValueIsZeroAndInnerCalldataIsEmpty() external whenDetectingERC7579ExecuteNoop { + // Case 2: ERC-7579 execute(mode=0x00, "") — single-call with empty execution data + function test_GivenCalldataIsERC7579ExecuteNoop() external whenDetectingNoopCalldata { // it should be detected as noop - // ERC-7579 compact format: abi.encodePacked(target, value) = 52 bytes - bytes memory executionCalldata = abi.encodePacked(bytes20(WALLET), uint256(0)); - - bytes memory callData = - abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), executionCalldata); - - bytes memory sig = _createProposalSignature("proposal", 0); - PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, 0, sig); + bytes memory noopExecute = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), ""); + bytes memory sig = _createProposalSignature("test", 1); + PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, noopExecute, 0, sig); vm.prank(WALLET); uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); - assertEq(result, 0, "ERC7579 execute to self with zero value should be noop"); + assertEq(result, 0, "ERC-7579 execute noop should be detected as noop"); } - function test_GivenTargetIsZeroAddressAndValueIsZeroAndInnerCalldataIsEmpty() - external - whenDetectingERC7579ExecuteNoop - { + // Case 3: executeUserOp + empty inner calldata (just the 4-byte selector) + function test_GivenCalldataIsExecuteUserOpEmpty() external whenDetectingNoopCalldata { // it should be detected as noop - // ERC-7579 compact format: abi.encodePacked(target, value) = 52 bytes - bytes memory executionCalldata = abi.encodePacked(bytes20(address(0)), uint256(0)); - - bytes memory callData = - abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), executionCalldata); - - bytes memory sig = _createProposalSignature("proposal", 1); - PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, 0, sig); - - vm.prank(WALLET); - uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); - - assertEq(result, 0, "ERC7579 execute to zero address with zero value should be noop"); - } - - function test_GivenCalldataIsShorterThan68Bytes() external whenDetectingERC7579ExecuteNoop { - // it should not be detected as noop - bytes memory shortCallData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0)); - - PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, shortCallData, 0, ""); - - vm.prank(WALLET); - uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); - - assertEq(result, SIG_VALIDATION_FAILED, "Too short for offset should not be noop"); - } - - function test_GivenOffsetIsNot64() external whenDetectingERC7579ExecuteNoop { - // it should not be detected as noop - bytes memory callData = abi.encodePacked( - IERC7579Execution.execute.selector, - bytes32(0), // mode - bytes32(uint256(32)), // wrong offset (should be 64) - bytes32(uint256(52)), // length - bytes20(WALLET), - uint256(0) - ); - - PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, 0, ""); - - vm.prank(WALLET); - uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); - - assertEq(result, SIG_VALIDATION_FAILED, "Wrong offset should not be noop"); - } - - function test_GivenCalldataIsShorterThan100Bytes() external whenDetectingERC7579ExecuteNoop { - // it should not be detected as noop - bytes memory callData = abi.encodePacked( - IERC7579Execution.execute.selector, - bytes32(0), // mode - bytes32(uint256(64)) // offset - // missing length and data - ); - - PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, 0, ""); + bytes memory executeUserOpNoop = abi.encodePacked(IAccountExecute.executeUserOp.selector); + bytes memory sig = _createProposalSignature("test", 2); + PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, executeUserOpNoop, 0, sig); vm.prank(WALLET); uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); - assertEq(result, SIG_VALIDATION_FAILED, "Too short for length should not be noop"); + assertEq(result, 0, "executeUserOp + empty should be detected as noop"); } - function test_GivenExecDataLengthIsNot52() external whenDetectingERC7579ExecuteNoop { - // it should not be detected as noop (length 20 != 52) - bytes memory callData = abi.encodePacked( - IERC7579Execution.execute.selector, - bytes32(0), // mode - bytes32(uint256(64)), // offset - bytes32(uint256(20)) // length only 20 (must be exactly 52) - ); - - PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, 0, ""); - - vm.prank(WALLET); - uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); - - assertEq(result, SIG_VALIDATION_FAILED, "Exec data length != 52 should not be noop"); - } - - function test_GivenTargetIsNotSelfOrZero() external whenDetectingERC7579ExecuteNoop { - // it should not be detected as noop - bytes memory executionCalldata = abi.encodePacked(bytes20(ATTACKER), uint256(0)); - - bytes memory callData = - abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), executionCalldata); - - PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, 0, ""); + // Case 4: executeUserOp + ERC-7579 execute no-op + function test_GivenCalldataIsExecuteUserOpWithERC7579Noop() external whenDetectingNoopCalldata { + // it should be detected as noop + bytes memory noopExecute = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), ""); + bytes memory executeUserOpWrapped = abi.encodePacked(IAccountExecute.executeUserOp.selector, noopExecute); + bytes memory sig = _createProposalSignature("test", 3); + PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, executeUserOpWrapped, 0, sig); vm.prank(WALLET); uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); - assertEq(result, SIG_VALIDATION_FAILED, "Wrong target should not be noop"); + assertEq(result, 0, "executeUserOp + ERC-7579 execute noop should be detected as noop"); } - function test_GivenValueIsNonzero() external whenDetectingERC7579ExecuteNoop { + // Negative: non-empty arbitrary calldata + function test_GivenCalldataIsNonEmpty() external whenDetectingNoopCalldata { // it should not be detected as noop - bytes memory executionCalldata = abi.encodePacked(bytes20(WALLET), uint256(1 ether)); - - bytes memory callData = - abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), executionCalldata); - - PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, 0, ""); + bytes memory nonEmptyCalldata = hex"aabb"; + PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, nonEmptyCalldata, 0, ""); vm.prank(WALLET); uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); - assertEq(result, SIG_VALIDATION_FAILED, "Non-zero value should not be noop"); - } - - function test_GivenExecDataLengthGreaterThan52() external whenDetectingERC7579ExecuteNoop { - // it should not be detected as noop (has inner calldata) - bytes memory executionCalldata = abi.encodePacked(bytes20(WALLET), uint256(0), hex"deadbeef"); - - bytes memory callData = - abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), executionCalldata); - - PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, 0, ""); - - vm.prank(WALLET); - uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); - - assertEq(result, SIG_VALIDATION_FAILED, "Non-empty inner calldata should not be noop"); + // Not a no-op, goes to execution path, no proposal exists + assertEq(result, SIG_VALIDATION_FAILED, "Non-empty calldata should not be noop"); } - // ============ _isNoOpExecuteUserOp Tests ============ - - modifier whenDetectingExecuteUserOpNoop() { - _; - } - - function test_GivenUserOpDataIsEmpty() external whenDetectingExecuteUserOpNoop { - // it should be detected as noop - bytes memory callData = abi.encodePacked( - IAccountExecute.executeUserOp.selector, - bytes32(uint256(32)), // offset to userOp - bytes32(0), // userOpHash - bytes32(uint256(0)) // userOp length = 0 - ); - - bytes memory sig = _createProposalSignature("proposal", 2); - PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, 0, sig); + // Negative: ERC-7579 execute with delegatecall mode + function test_GivenCalldataIsERC7579ExecuteDelegatecall() external whenDetectingNoopCalldata { + // it should not be detected as noop — mode 0xFE is delegatecall + bytes32 delegatecallMode = bytes32(uint256(0xFE) << 248); + bytes memory delegatecallExecute = + abi.encodeWithSelector(IERC7579Execution.execute.selector, delegatecallMode, ""); + PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, delegatecallExecute, 0, ""); vm.prank(WALLET); uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); - assertEq(result, 0, "executeUserOp with empty userOp should be noop"); + assertEq(result, SIG_VALIDATION_FAILED, "Delegatecall mode should not be noop"); } - function test_GivenCalldataIsShorterThan100Bytes_WhenDetectingExecuteUserOpNoop() - external - whenDetectingExecuteUserOpNoop - { - // it should not be detected as noop - bytes memory callData = abi.encodePacked(IAccountExecute.executeUserOp.selector, bytes32(uint256(32))); - - PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, 0, ""); + // Negative: ERC-7579 execute with batch mode + function test_GivenCalldataIsERC7579ExecuteBatch() external whenDetectingNoopCalldata { + // it should not be detected as noop — mode 0x01 is batch + bytes32 batchMode = bytes32(uint256(0x01) << 248); + bytes memory batchExecute = abi.encodeWithSelector(IERC7579Execution.execute.selector, batchMode, ""); + PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, batchExecute, 0, ""); vm.prank(WALLET); uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); - assertEq(result, SIG_VALIDATION_FAILED, "Too short executeUserOp should not be noop"); + assertEq(result, SIG_VALIDATION_FAILED, "Batch mode should not be noop"); } - function test_GivenOffsetIsNot32_WhenDetectingExecuteUserOpNoop() external whenDetectingExecuteUserOpNoop { - // it should not be detected as noop - bytes memory callData = abi.encodePacked( - IAccountExecute.executeUserOp.selector, - bytes32(uint256(64)), // wrong offset - bytes32(0), - bytes32(uint256(0)) - ); - - PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, 0, ""); + // Negative: ERC-7579 execute with non-empty execution data + function test_GivenCalldataIsERC7579ExecuteWithData() external whenDetectingNoopCalldata { + // it should not be detected as noop — has execution data + bytes memory executeWithData = + abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "some_execution_data"); + PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, executeWithData, 0, ""); vm.prank(WALLET); uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); - assertEq(result, SIG_VALIDATION_FAILED, "Wrong offset in executeUserOp should not be noop"); + assertEq(result, SIG_VALIDATION_FAILED, "Execute with data should not be noop"); } - function test_GivenUserOpLengthIsNonzero() external whenDetectingExecuteUserOpNoop { + // Negative: executeUserOp wrapping a delegatecall ERC-7579 execute + function test_GivenCalldataIsExecuteUserOpWithDelegatecall() external whenDetectingNoopCalldata { // it should not be detected as noop - bytes memory callData = abi.encodePacked( - IAccountExecute.executeUserOp.selector, - bytes32(uint256(32)), - bytes32(0), - bytes32(uint256(10)) // non-empty userOp - ); - - PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, 0, ""); + bytes32 delegatecallMode = bytes32(uint256(0xFE) << 248); + bytes memory delegatecallExecute = + abi.encodeWithSelector(IERC7579Execution.execute.selector, delegatecallMode, ""); + bytes memory wrapped = abi.encodePacked(IAccountExecute.executeUserOp.selector, delegatecallExecute); + PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, wrapped, 0, ""); vm.prank(WALLET); uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); - assertEq(result, SIG_VALIDATION_FAILED, "Non-empty userOp should not be noop"); + assertEq(result, SIG_VALIDATION_FAILED, "executeUserOp + delegatecall should not be noop"); } // ============ _packValidationData Tests ============ @@ -932,7 +766,11 @@ contract TimelockTest is Test { bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); uint256 nonce = 1400; - timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); + // Create proposal via no-op UserOp + bytes memory sig = _createProposalSignature(callData, nonce); + PackedUserOperation memory noopOp = _createNoopUserOp(WALLET, sig); + vm.prank(WALLET); + timelockPolicy.checkUserOpPolicy(POLICY_ID, noopOp); // Get the actual stored proposal values (, uint256 storedValidAfter, uint256 storedGraceEnd, uint256 storedValidUntil) = @@ -969,32 +807,16 @@ contract TimelockTest is Test { assertEq(result, SIG_VALIDATION_FAILED, "Attack without proposal should fail"); } - function test_GivenAttackerTriesToExecuteImmediatelyAfterCreation() external whenTestingSecurityScenarios { - // it should return packed data with future validAfter - bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "action"); - uint256 nonce = 1500; - - uint256 createTime = block.timestamp; - timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); - - // Try to execute immediately (before timelock) - PackedUserOperation memory executeOp = _createUserOpWithCalldata(WALLET, callData, nonce, ""); - - vm.prank(WALLET); - uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, executeOp); - - // Extract validAfter from packed data - it's actually graceEnd, should be in the future - uint48 validAfter = uint48(result >> 208); - assertGt(validAfter, block.timestamp, "validAfter should be in the future"); - assertEq(validAfter, uint48(createTime) + DELAY + GRACE_PERIOD, "validAfter should be createTime + DELAY + GRACE_PERIOD (graceEnd)"); - } - function test_GivenAttackerTriesToReexecuteAUsedProposal() external whenTestingSecurityScenarios { // it should return SIG_VALIDATION_FAILED bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "action"); uint256 nonce = 1600; - timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); + // Create proposal via no-op UserOp + bytes memory sig = _createProposalSignature(callData, nonce); + PackedUserOperation memory noopOp = _createNoopUserOp(WALLET, sig); + vm.prank(WALLET); + timelockPolicy.checkUserOpPolicy(POLICY_ID, noopOp); vm.warp(block.timestamp + DELAY + 1); @@ -1044,7 +866,11 @@ contract TimelockTest is Test { bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); uint256 nonce = 1800; - timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); + // Create proposal via no-op UserOp from WALLET + bytes memory sig = _createProposalSignature(callData, nonce); + PackedUserOperation memory noopOp = _createNoopUserOp(WALLET, sig); + vm.prank(WALLET); + timelockPolicy.checkUserOpPolicy(POLICY_ID, noopOp); vm.prank(ATTACKER); vm.expectRevert(TimelockPolicy.OnlyAccount.selector); @@ -1053,6 +879,6 @@ contract TimelockTest is Test { // Verify proposal is still pending (TimelockPolicy.ProposalStatus status,,,) = timelockPolicy.getProposal(WALLET, callData, nonce, POLICY_ID, WALLET); - assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending), "Proposal should still be pending"); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending), "Proposal should still be Pending"); } } diff --git a/test/btt/TimelockCancellationRace.t.sol b/test/btt/TimelockCancellationRace.t.sol index 5c305a0..71eb396 100644 --- a/test/btt/TimelockCancellationRace.t.sol +++ b/test/btt/TimelockCancellationRace.t.sol @@ -40,10 +40,22 @@ contract TimelockCancellationRaceTest is Test { vm.stopPrank(); } - // Helper function to create a proposal + // Helper function to create a proposal via no-op UserOp function _createProposal(bytes memory callData, uint256 nonce) internal { + bytes memory sig = abi.encodePacked(bytes32(callData.length), callData, bytes32(nonce), bytes1(0x00)); + PackedUserOperation memory noopOp = PackedUserOperation({ + sender: WALLET, + nonce: 0, + initCode: "", + callData: "", + accountGasLimits: bytes32(abi.encodePacked(uint128(100000), uint128(200000))), + preVerificationGas: 0, + gasFees: bytes32(abi.encodePacked(uint128(1), uint128(1))), + paymasterAndData: "", + signature: sig + }); vm.prank(WALLET); - timelockPolicy.createProposal(policyId, WALLET, callData, nonce); + timelockPolicy.checkUserOpPolicy(policyId, noopOp); } // Helper function to cancel a proposal @@ -220,14 +232,23 @@ contract TimelockCancellationRaceTest is Test { _createProposal(TEST_CALLDATA, TEST_NONCE); _cancelProposal(TEST_CALLDATA, TEST_NONCE); - // Note: The current implementation does not allow creating a new proposal - // for the same calldata/nonce because cancelled proposals persist - // This test verifies this behavior - - // Action & Verify: Attempting to create a new proposal with same params should fail + // Note: Cancelled proposals persist. Attempting to create via no-op UserOp + // for the same calldata/nonce returns SIG_VALIDATION_FAILED. + bytes memory sig = abi.encodePacked(bytes32(TEST_CALLDATA.length), TEST_CALLDATA, bytes32(TEST_NONCE), bytes1(0x00)); + PackedUserOperation memory retryOp = PackedUserOperation({ + sender: WALLET, + nonce: 0, + initCode: "", + callData: "", + accountGasLimits: bytes32(abi.encodePacked(uint128(100000), uint128(200000))), + preVerificationGas: 0, + gasFees: bytes32(abi.encodePacked(uint128(1), uint128(1))), + paymasterAndData: "", + signature: sig + }); vm.prank(WALLET); - vm.expectRevert(TimelockPolicy.ProposalAlreadyExists.selector); - timelockPolicy.createProposal(policyId, WALLET, TEST_CALLDATA, TEST_NONCE); + uint256 retryResult = timelockPolicy.checkUserOpPolicy(policyId, retryOp); + assertEq(retryResult, 1, "Should return SIG_VALIDATION_FAILED for cancelled proposal"); // However, a proposal with different nonce should work uint256 newNonce = TEST_NONCE + 1; @@ -434,10 +455,23 @@ contract TimelockCancellationRaceTest is Test { // Fast forward past when grace period would have ended vm.warp(block.timestamp + DELAY + GRACE_PERIOD + EXPIRATION_PERIOD + 1); - // Action & Verify: it should revert with ProposalAlreadyExists because cancelled proposals persist + // Action & Verify: Attempting to create via no-op UserOp returns SIG_VALIDATION_FAILED + // because cancelled proposals persist in storage + bytes memory sig = abi.encodePacked(bytes32(TEST_CALLDATA.length), TEST_CALLDATA, bytes32(TEST_NONCE), bytes1(0x00)); + PackedUserOperation memory noopOp = PackedUserOperation({ + sender: WALLET, + nonce: 0, + initCode: "", + callData: "", + accountGasLimits: bytes32(abi.encodePacked(uint128(100000), uint128(200000))), + preVerificationGas: 0, + gasFees: bytes32(abi.encodePacked(uint128(1), uint128(1))), + paymasterAndData: "", + signature: sig + }); vm.prank(WALLET); - vm.expectRevert(TimelockPolicy.ProposalAlreadyExists.selector); - timelockPolicy.createProposal(policyId, WALLET, TEST_CALLDATA, TEST_NONCE); + uint256 result = timelockPolicy.checkUserOpPolicy(policyId, noopOp); + assertEq(result, 1, "Should return SIG_VALIDATION_FAILED because cancelled proposals persist"); } function test_GivenTheOriginalProposalWasExecuted() external whenCreatingANewProposalAfterGracePeriod { @@ -455,10 +489,23 @@ contract TimelockCancellationRaceTest is Test { // Fast forward more vm.warp(block.timestamp + EXPIRATION_PERIOD + 1); - // Action & Verify: it should revert with ProposalAlreadyExists because executed proposals persist + // Action & Verify: Attempting to create via no-op UserOp returns SIG_VALIDATION_FAILED + // because executed proposals persist in storage + bytes memory sig = abi.encodePacked(bytes32(TEST_CALLDATA.length), TEST_CALLDATA, bytes32(TEST_NONCE), bytes1(0x00)); + PackedUserOperation memory noopOp = PackedUserOperation({ + sender: WALLET, + nonce: 0, + initCode: "", + callData: "", + accountGasLimits: bytes32(abi.encodePacked(uint128(100000), uint128(200000))), + preVerificationGas: 0, + gasFees: bytes32(abi.encodePacked(uint128(1), uint128(1))), + paymasterAndData: "", + signature: sig + }); vm.prank(WALLET); - vm.expectRevert(TimelockPolicy.ProposalAlreadyExists.selector); - timelockPolicy.createProposal(policyId, WALLET, TEST_CALLDATA, TEST_NONCE); + uint256 result = timelockPolicy.checkUserOpPolicy(policyId, noopOp); + assertEq(result, 1, "Should return SIG_VALIDATION_FAILED because executed proposals persist"); } // ==================== whenValidatingGracePeriodTiming ==================== @@ -545,9 +592,21 @@ contract TimelockCancellationRaceTest is Test { function test_NonInitializedAccountCannotCreateProposal() external { address nonInitializedAccount = address(0xDEAD); - // Try to create proposal on non-initialized account + // Try to create proposal via no-op UserOp on non-initialized account + bytes memory sig = abi.encodePacked(bytes32(TEST_CALLDATA.length), TEST_CALLDATA, bytes32(TEST_NONCE), bytes1(0x00)); + PackedUserOperation memory noopOp = PackedUserOperation({ + sender: nonInitializedAccount, + nonce: 0, + initCode: "", + callData: "", + accountGasLimits: bytes32(abi.encodePacked(uint128(100000), uint128(200000))), + preVerificationGas: 0, + gasFees: bytes32(abi.encodePacked(uint128(1), uint128(1))), + paymasterAndData: "", + signature: sig + }); vm.prank(nonInitializedAccount); - vm.expectRevert(abi.encodeWithSelector(IModule.NotInitialized.selector, nonInitializedAccount)); - timelockPolicy.createProposal(policyId, nonInitializedAccount, TEST_CALLDATA, TEST_NONCE); + uint256 result = timelockPolicy.checkUserOpPolicy(policyId, noopOp); + assertEq(result, 1, "Should return SIG_VALIDATION_FAILED for non-initialized account"); } } diff --git a/test/btt/TimelockEpochValidation.t.sol b/test/btt/TimelockEpochValidation.t.sol index fd894e4..b5ca7c9 100644 --- a/test/btt/TimelockEpochValidation.t.sol +++ b/test/btt/TimelockEpochValidation.t.sol @@ -45,8 +45,21 @@ contract TimelockEpochValidationTest is Test { } function _createProposal(address wallet, bytes32 policyId, bytes memory callData, uint256 nonce) internal { + // Create proposal via no-op UserOp (creates Pending directly with clock started) + bytes memory sig = abi.encodePacked(bytes32(callData.length), callData, bytes32(nonce), bytes1(0x00)); + PackedUserOperation memory noopOp = PackedUserOperation({ + sender: wallet, + nonce: 0, + initCode: "", + callData: "", + accountGasLimits: bytes32(abi.encodePacked(uint128(100000), uint128(200000))), + preVerificationGas: 0, + gasFees: bytes32(abi.encodePacked(uint128(1), uint128(1))), + paymasterAndData: "", + signature: sig + }); vm.prank(wallet); - timelockPolicy.createProposal(policyId, wallet, callData, nonce); + timelockPolicy.checkUserOpPolicy(policyId, noopOp); } function _createUserOp(address sender, bytes memory callData, uint256 nonce) @@ -151,7 +164,7 @@ contract TimelockEpochValidationTest is Test { _createProposal(WALLET, POLICY_ID_1, callData, nonce); // it should store the current epoch in the proposal - // it should use the epoch from currentEpoch mapping + // it should be in Pending status with timing set bytes32 userOpKey = timelockPolicy.computeUserOpKey(WALLET, callData, nonce); ( TimelockPolicy.ProposalStatus status, @@ -161,28 +174,30 @@ contract TimelockEpochValidationTest is Test { uint256 proposalEpoch ) = timelockPolicy.proposals(userOpKey, POLICY_ID_1, WALLET); - assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending), "Proposal should be pending"); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending), "Proposal should be Pending"); assertEq(proposalEpoch, 1, "Proposal epoch should match current epoch (1)"); assertEq(validAfter, block.timestamp + DELAY, "validAfter should be correct"); assertEq(graceEnd, block.timestamp + DELAY + GRACE_PERIOD, "graceEnd should be correct"); assertEq(validUntil, block.timestamp + DELAY + GRACE_PERIOD + EXPIRATION_PERIOD, "validUntil should be correct"); } - function test_GivenCreatingViaCreateProposalFunction() external whenCreatingAProposal { + function test_GivenCreatingViaNoOpUserOp() external whenCreatingAProposal { _installPolicy(WALLET, POLICY_ID_1); bytes memory callData = hex"5678"; uint256 nonce = 42; - uint256 currentEpoch = timelockPolicy.currentEpoch(POLICY_ID_1, WALLET); + uint256 epoch = timelockPolicy.currentEpoch(POLICY_ID_1, WALLET); _createProposal(WALLET, POLICY_ID_1, callData, nonce); - // it should record the epoch at creation time + // it should record the epoch at creation time (proposal is Pending) bytes32 userOpKey = timelockPolicy.computeUserOpKey(WALLET, callData, nonce); - (,,,, uint256 proposalEpoch) = timelockPolicy.proposals(userOpKey, POLICY_ID_1, WALLET); + (TimelockPolicy.ProposalStatus status,,,, uint256 proposalEpoch) = + timelockPolicy.proposals(userOpKey, POLICY_ID_1, WALLET); - assertEq(proposalEpoch, currentEpoch, "Proposal epoch should equal current epoch at creation"); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending), "Should be Pending"); + assertEq(proposalEpoch, epoch, "Proposal epoch should equal current epoch at creation"); } // ==================== Executing a Proposal ==================== @@ -205,6 +220,7 @@ contract TimelockEpochValidationTest is Test { bytes memory callData = hex"1234"; uint256 nonce = 1; + // Create proposal via no-op UserOp (creates Pending directly with clock started) _createProposal(WALLET, POLICY_ID_1, callData, nonce); // Warp past the timelock delay @@ -230,7 +246,7 @@ contract TimelockEpochValidationTest is Test { bytes memory callData = hex"1234"; uint256 nonce = 1; - // Create proposal in epoch 1 + // Create proposal via no-op UserOp (Pending with clock started, epoch 1) _createProposal(WALLET, POLICY_ID_1, callData, nonce); // Warp past the timelock delay @@ -249,10 +265,10 @@ contract TimelockEpochValidationTest is Test { // it should return SIG_VALIDATION_FAILED assertEq(validationResult, SIG_VALIDATION_FAILED_UINT, "Stale proposal should fail validation"); - // it should not mark proposal as executed + // it should not mark proposal as executed (still Pending from epoch 1) bytes32 userOpKey = timelockPolicy.computeUserOpKey(WALLET, callData, nonce); (TimelockPolicy.ProposalStatus status,,,,) = timelockPolicy.proposals(userOpKey, POLICY_ID_1, WALLET); - assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending), "Proposal should still be pending"); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending), "Proposal should still be Pending"); } function test_GivenTheProposalEpochDoesNotMatchCurrentEpoch() external whenExecutingAProposal { @@ -261,7 +277,7 @@ contract TimelockEpochValidationTest is Test { bytes memory callData = hex"abcd"; uint256 nonce = 5; - // Create proposal in epoch 1 + // Create proposal via no-op UserOp in epoch 1 (Pending with clock started) _createProposal(WALLET, POLICY_ID_1, callData, nonce); bytes32 userOpKey = timelockPolicy.computeUserOpKey(WALLET, callData, nonce); @@ -291,7 +307,7 @@ contract TimelockEpochValidationTest is Test { assertEq( uint256(statusAfter), uint256(TimelockPolicy.ProposalStatus.Pending), - "Proposal status should remain pending" + "Proposal status should remain Pending" ); } @@ -341,6 +357,7 @@ contract TimelockEpochValidationTest is Test { bytes memory callData = hex"1234"; uint256 nonce = 1; + // Create proposal via no-op UserOp in epoch 1 (Pending with clock started) _createProposal(WALLET, POLICY_ID_1, callData, nonce); uint256 epochBeforeUninstall = timelockPolicy.currentEpoch(POLICY_ID_1, WALLET); @@ -362,7 +379,7 @@ contract TimelockEpochValidationTest is Test { uint256 validationResult = timelockPolicy.checkUserOpPolicy(POLICY_ID_1, userOp); assertEq(validationResult, SIG_VALIDATION_FAILED_UINT, "Old proposal should be invalid"); - // it should allow new proposals with new epoch + // it should allow new proposals with new epoch (Pending status) bytes memory newCallData = hex"5678"; uint256 newNonce = 2; @@ -372,7 +389,7 @@ contract TimelockEpochValidationTest is Test { (TimelockPolicy.ProposalStatus status,,,, uint256 newProposalEpoch) = timelockPolicy.proposals(newUserOpKey, POLICY_ID_1, WALLET); - assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending), "New proposal should be created"); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending), "New proposal should be Pending"); assertEq(newProposalEpoch, epochAfterReinstall, "New proposal should have current epoch"); } @@ -432,35 +449,26 @@ contract TimelockEpochValidationTest is Test { _; } - function test_WhenAttackerTriesToExecuteOldProposal() external whenVerifyingFullAttackScenario { + function test_WhenAttackerCannotCreateProposalsDirectly() external whenVerifyingFullAttackScenario { // User installs policy _installPolicy(WALLET, POLICY_ID_1); assertEq(timelockPolicy.currentEpoch(POLICY_ID_1, WALLET), 1, "Should start at epoch 1"); - // Attacker creates malicious proposal + // Without createProposal, an attacker cannot create proposals for WALLET. + // The only way to create a proposal is via checkUserOpPolicy which requires + // msg.sender == WALLET (the account itself). So there is no external attack vector. + + // Verify that trying to execute a non-existent proposal fails bytes memory maliciousCallData = hex"deadbeef"; uint256 maliciousNonce = 666; - vm.prank(ATTACKER); - timelockPolicy.createProposal(POLICY_ID_1, WALLET, maliciousCallData, maliciousNonce); - - // Time passes, timelock expires - vm.warp(block.timestamp + DELAY + 1); - - // User decides to uninstall and reinstall - _uninstallPolicy(WALLET, POLICY_ID_1); - _installPolicy(WALLET, POLICY_ID_1); - - assertEq(timelockPolicy.currentEpoch(POLICY_ID_1, WALLET), 2, "Should be at epoch 2"); - - // Attacker tries to execute the old proposal PackedUserOperation memory maliciousUserOp = _createUserOp(WALLET, maliciousCallData, maliciousNonce); vm.prank(WALLET); uint256 validationResult = timelockPolicy.checkUserOpPolicy(POLICY_ID_1, maliciousUserOp); - // it should fail due to epoch mismatch - assertEq(validationResult, SIG_VALIDATION_FAILED_UINT, "Attack should be thwarted by epoch check"); + // it should fail because no proposal exists + assertEq(validationResult, SIG_VALIDATION_FAILED_UINT, "Non-existent proposal should fail"); } function test_WhenUserCreatesNewProposalAfterReinstall() external whenVerifyingFullAttackScenario { @@ -471,7 +479,7 @@ contract TimelockEpochValidationTest is Test { _uninstallPolicy(WALLET, POLICY_ID_1); _installPolicy(WALLET, POLICY_ID_1); - // User creates legitimate proposal with new epoch + // User creates legitimate proposal via no-op UserOp with new epoch bytes memory callData = hex"abcd"; uint256 userNonce = 200; _createProposal(WALLET, POLICY_ID_1, callData, userNonce); @@ -492,7 +500,7 @@ contract TimelockEpochValidationTest is Test { _installPolicy(WALLET, POLICY_ID_1); _installPolicy(WALLET2, POLICY_ID_1); - // Both users create proposals + // Both users create proposals via no-op UserOp bytes memory callData1 = hex"1111"; bytes memory callData2 = hex"2222"; @@ -509,7 +517,7 @@ contract TimelockEpochValidationTest is Test { assertEq(timelockPolicy.currentEpoch(POLICY_ID_1, WALLET), 2, "WALLET should be epoch 2"); assertEq(timelockPolicy.currentEpoch(POLICY_ID_1, WALLET2), 1, "WALLET2 should still be epoch 1"); - // WALLET's old proposal should fail + // WALLET's old proposal should fail (epoch mismatch) PackedUserOperation memory userOp1 = _createUserOp(WALLET, callData1, 1); vm.prank(WALLET); uint256 result1 = timelockPolicy.checkUserOpPolicy(POLICY_ID_1, userOp1); diff --git a/test/integration/TimelockEntryPoint.t.sol b/test/integration/TimelockEntryPoint.t.sol new file mode 100644 index 0000000..df87545 --- /dev/null +++ b/test/integration/TimelockEntryPoint.t.sol @@ -0,0 +1,981 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import {Test} from "forge-std/Test.sol"; +import {IEntryPoint} from "account-abstraction/interfaces/IEntryPoint.sol"; +import {PackedUserOperation} from "account-abstraction/interfaces/PackedUserOperation.sol"; +import {IAccountExecute} from "account-abstraction/interfaces/IAccountExecute.sol"; +import {IERC7579Execution} from "openzeppelin-contracts/contracts/interfaces/draft-IERC7579.sol"; +import {EntryPointLib} from "../utils/EntryPointLib.sol"; +import {MockTimelockAccount} from "../utils/MockTimelockAccount.sol"; +import {TimelockPolicy} from "../../src/policies/TimelockPolicy.sol"; + +/// @title TimelockEntryPointTest +/// @notice Integration tests that exercise TimelockPolicy through the real EntryPoint v0.9. +/// Verifies that validAfter/validUntil returned by the policy are correctly enforced +/// by the EntryPoint's time-range validation. +contract TimelockEntryPointTest is Test { + IEntryPoint public entryPoint; + TimelockPolicy public policy; + MockTimelockAccount public account; + + bytes32 public constant POLICY_ID = bytes32(uint256(1)); + uint48 public constant DELAY = 1 hours; + uint48 public constant EXPIRATION = 1 days; + uint48 public constant GRACE_PERIOD = 30 minutes; + + address payable constant BENEFICIARY = payable(address(0xbeeF)); + address constant BUNDLER = address(0xba5ed); + + function setUp() public { + entryPoint = EntryPointLib.deploy(); + policy = new TimelockPolicy(); + account = new MockTimelockAccount(entryPoint, policy, POLICY_ID); + + // Fund the account for gas + vm.deal(address(account), 100 ether); + + // Install timelock policy (must come from the account) + vm.prank(address(account)); + policy.onInstall(abi.encode(POLICY_ID, DELAY, EXPIRATION, GRACE_PERIOD)); + } + + // ============ Helpers ============ + + /// @dev Build proposal-creation signature: [callDataLen(32)][callData][proposalNonce(32)][0x00] + function _proposalSig(bytes memory proposalCallData, uint256 proposalNonce) + internal + pure + returns (bytes memory) + { + return abi.encodePacked( + bytes32(proposalCallData.length), + proposalCallData, + bytes32(proposalNonce), + bytes1(0x00) + ); + } + + /// @dev Build a no-op UserOp for proposal creation with configurable calldata format. + function _buildCreationOpWithCalldata( + bytes memory noopCallData, + bytes memory proposalCallData, + uint256 proposalNonce, + uint256 epNonce + ) internal view returns (PackedUserOperation memory) { + return PackedUserOperation({ + sender: address(account), + nonce: epNonce, + initCode: "", + callData: noopCallData, + accountGasLimits: bytes32(abi.encodePacked(uint128(500_000), uint128(500_000))), + preVerificationGas: 100_000, + gasFees: bytes32(abi.encodePacked(uint128(1 gwei), uint128(1 gwei))), + paymasterAndData: "", + signature: _proposalSig(proposalCallData, proposalNonce) + }); + } + + /// @dev Build a no-op UserOp for proposal creation (empty calldata). + function _buildCreationOp(bytes memory proposalCallData, uint256 proposalNonce, uint256 epNonce) + internal + view + returns (PackedUserOperation memory) + { + return _buildCreationOpWithCalldata("", proposalCallData, proposalNonce, epNonce); + } + + /// @dev Build an execution UserOp. The nonce must match the proposalNonce used at creation time. + function _buildExecutionOp(bytes memory callData, uint256 nonce) + internal + view + returns (PackedUserOperation memory) + { + return PackedUserOperation({ + sender: address(account), + nonce: nonce, + initCode: "", + callData: callData, + accountGasLimits: bytes32(abi.encodePacked(uint128(500_000), uint128(500_000))), + preVerificationGas: 100_000, + gasFees: bytes32(abi.encodePacked(uint128(1 gwei), uint128(1 gwei))), + paymasterAndData: "", + signature: "" // signature content irrelevant for execution path + }); + } + + function _submitOp(PackedUserOperation memory op) internal { + PackedUserOperation[] memory ops = new PackedUserOperation[](1); + ops[0] = op; + // EntryPoint requires msg.sender == tx.origin (EOA bundler check) + vm.prank(BUNDLER, BUNDLER); + entryPoint.handleOps(ops, BENEFICIARY); + } + + function _submitOps(PackedUserOperation[] memory ops) internal { + vm.prank(BUNDLER, BUNDLER); + entryPoint.handleOps(ops, BENEFICIARY); + } + + function _expectRevertOnOp(PackedUserOperation memory op) internal { + PackedUserOperation[] memory ops = new PackedUserOperation[](1); + ops[0] = op; + vm.prank(BUNDLER, BUNDLER); + vm.expectRevert(); + entryPoint.handleOps(ops, BENEFICIARY); + } + + /// @dev Helper: create a proposal via EntryPoint and return the nonce used + function _createProposal(bytes memory proposalCallData, uint256 proposalNonce) internal { + uint256 epNonce = entryPoint.getNonce(address(account), 0); + _submitOp(_buildCreationOp(proposalCallData, proposalNonce, epNonce)); + } + + /// @dev Helper: get a fresh nonce for a given key + function _getNonce(uint192 key) internal view returns (uint256) { + return entryPoint.getNonce(address(account), key); + } + + // ============ 1. Basic Lifecycle Tests ============ + + /// @notice Proposal creation via no-op UserOp goes through the EntryPoint and persists the proposal. + function testEntryPoint_ProposalCreationViaNoOp() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = 1; // execution will use nonce=1 (next seq for key=0) + + uint256 epNonce = _getNonce(0); + assertEq(epNonce, 0); + + _submitOp(_buildCreationOp(proposalCallData, proposalNonce, epNonce)); + + // Verify proposal was stored + (TimelockPolicy.ProposalStatus status, uint256 validAfter, uint256 graceEnd, uint256 validUntil) = + policy.getProposal(address(account), proposalCallData, proposalNonce, POLICY_ID, address(account)); + + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending)); + assertEq(validAfter, block.timestamp + DELAY); + assertEq(graceEnd, block.timestamp + DELAY + GRACE_PERIOD); + assertEq(validUntil, block.timestamp + DELAY + GRACE_PERIOD + EXPIRATION); + + // EntryPoint nonce should have advanced + assertEq(_getNonce(0), 1); + } + + /// @notice Full lifecycle: create proposal -> wait -> execute -> verify state change. + function testEntryPoint_FullLifecycle() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = 1; + + // Step 1: Create proposal + _submitOp(_buildCreationOp(proposalCallData, proposalNonce, 0)); + + // Step 2: Warp past delay + grace period + vm.warp(block.timestamp + DELAY + GRACE_PERIOD + 1); + + // Step 3: Execute proposal through EntryPoint + _submitOp(_buildExecutionOp(proposalCallData, proposalNonce)); + + // Step 4: Verify the execution actually happened + assertEq(account.value(), 42); + + // Verify proposal status is Executed + (TimelockPolicy.ProposalStatus status,,,) = + policy.getProposal(address(account), proposalCallData, proposalNonce, POLICY_ID, address(account)); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Executed)); + } + + /// @notice Execution without a prior proposal fails at validation. + function testEntryPoint_NoProposalRevertsExecution() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + _expectRevertOnOp(_buildExecutionOp(proposalCallData, 0)); + assertEq(account.value(), 0); + } + + // ============ 2. Time Window Enforcement ============ + + /// @notice EntryPoint rejects execution during the grace period (validAfter not yet reached). + function testEntryPoint_GracePeriodBlocksExecution() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = 1; + + _submitOp(_buildCreationOp(proposalCallData, proposalNonce, 0)); + + // Warp past delay but still within grace period + vm.warp(block.timestamp + DELAY + 1); + + _expectRevertOnOp(_buildExecutionOp(proposalCallData, proposalNonce)); + assertEq(account.value(), 0); + } + + /// @notice Execution at exactly graceEnd timestamp is still rejected (EntryPoint uses <=). + function testEntryPoint_ExecutionAtExactGraceEndIsRejected() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = 1; + + uint256 creationTime = block.timestamp; + _submitOp(_buildCreationOp(proposalCallData, proposalNonce, 0)); + + // Warp to exactly graceEnd: EntryPoint checks block.timestamp <= validAfter, so equal is rejected + vm.warp(creationTime + DELAY + GRACE_PERIOD); + + _expectRevertOnOp(_buildExecutionOp(proposalCallData, proposalNonce)); + assertEq(account.value(), 0); + } + + /// @notice Execution at graceEnd + 1 succeeds (first valid timestamp). + function testEntryPoint_ExecutionAtGraceEndPlusOneSucceeds() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = 1; + + uint256 creationTime = block.timestamp; + _submitOp(_buildCreationOp(proposalCallData, proposalNonce, 0)); + + vm.warp(creationTime + DELAY + GRACE_PERIOD + 1); + + _submitOp(_buildExecutionOp(proposalCallData, proposalNonce)); + assertEq(account.value(), 42); + } + + /// @notice Execution at exactly validUntil is still accepted (EntryPoint checks >). + function testEntryPoint_ExecutionAtExactValidUntilSucceeds() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = 1; + + uint256 creationTime = block.timestamp; + _submitOp(_buildCreationOp(proposalCallData, proposalNonce, 0)); + + // Warp to exactly validUntil: EntryPoint checks block.timestamp > validUntil, so equal is OK + vm.warp(creationTime + DELAY + GRACE_PERIOD + EXPIRATION); + + _submitOp(_buildExecutionOp(proposalCallData, proposalNonce)); + assertEq(account.value(), 42); + } + + /// @notice EntryPoint rejects execution after the proposal has expired. + function testEntryPoint_ExpirationBlocksExecution() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = 1; + + uint256 creationTime = block.timestamp; + _submitOp(_buildCreationOp(proposalCallData, proposalNonce, 0)); + + // Warp 1 second past validUntil + vm.warp(creationTime + DELAY + GRACE_PERIOD + EXPIRATION + 1); + + _expectRevertOnOp(_buildExecutionOp(proposalCallData, proposalNonce)); + assertEq(account.value(), 0); + } + + /// @notice Execution before delay has passed is rejected (still in timelock period). + function testEntryPoint_ExecutionBeforeDelayRejected() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = 1; + + _submitOp(_buildCreationOp(proposalCallData, proposalNonce, 0)); + + // Don't warp at all — still at creation time + _expectRevertOnOp(_buildExecutionOp(proposalCallData, proposalNonce)); + assertEq(account.value(), 0); + } + + // ============ 3. Cancellation Tests ============ + + /// @notice Cancelled proposal cannot be executed even after the timelock passes. + function testEntryPoint_CancelPreventsExecution() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = 1; + + _submitOp(_buildCreationOp(proposalCallData, proposalNonce, 0)); + + vm.prank(address(account)); + policy.cancelProposal(POLICY_ID, address(account), proposalCallData, proposalNonce); + + vm.warp(block.timestamp + DELAY + GRACE_PERIOD + 1); + + _expectRevertOnOp(_buildExecutionOp(proposalCallData, proposalNonce)); + assertEq(account.value(), 0); + } + + /// @notice Owner can cancel during grace period (delay passed but grace hasn't ended). + function testEntryPoint_CancelDuringGracePeriod() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = 1; + + _submitOp(_buildCreationOp(proposalCallData, proposalNonce, 0)); + + // Warp into the grace period + vm.warp(block.timestamp + DELAY + GRACE_PERIOD / 2); + + // Cancel should succeed + vm.prank(address(account)); + policy.cancelProposal(POLICY_ID, address(account), proposalCallData, proposalNonce); + + (TimelockPolicy.ProposalStatus status,,,) = + policy.getProposal(address(account), proposalCallData, proposalNonce, POLICY_ID, address(account)); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Cancelled)); + + // Warp past grace period — execution still fails + vm.warp(block.timestamp + GRACE_PERIOD + 1); + _expectRevertOnOp(_buildExecutionOp(proposalCallData, proposalNonce)); + } + + /// @notice Owner can cancel before the delay has even passed. + function testEntryPoint_CancelBeforeDelayPasses() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = 1; + + _submitOp(_buildCreationOp(proposalCallData, proposalNonce, 0)); + + // Cancel immediately (no warp) + vm.prank(address(account)); + policy.cancelProposal(POLICY_ID, address(account), proposalCallData, proposalNonce); + + (TimelockPolicy.ProposalStatus status,,,) = + policy.getProposal(address(account), proposalCallData, proposalNonce, POLICY_ID, address(account)); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Cancelled)); + } + + // ============ 4. Replay / Double-Use Prevention ============ + + /// @notice A proposal that was already executed cannot be executed again. + function testEntryPoint_DoubleExecutionFails() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + // Use different nonce keys so execution nonces don't collide + uint256 proposalNonce = _getNonce(1); // key=1, seq=0 + + _createProposal(proposalCallData, proposalNonce); + + vm.warp(block.timestamp + DELAY + GRACE_PERIOD + 1); + + // First execution succeeds + _submitOp(_buildExecutionOp(proposalCallData, proposalNonce)); + assertEq(account.value(), 42); + + // Second execution attempt with the same callData+nonce via a different nonce key. + // The EntryPoint nonce for key=1 is now 1 (after first execution), so we'd need + // a new nonce. But the proposal is already Executed, so validation returns 1. + // We use key=2 to get a fresh nonce that equals proposalNonce... but that doesn't + // match the original proposalNonce. The proposal key won't match. + // Instead, verify the proposal status is Executed. + (TimelockPolicy.ProposalStatus status,,,) = + policy.getProposal(address(account), proposalCallData, proposalNonce, POLICY_ID, address(account)); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Executed)); + } + + /// @notice Cannot create the same proposal twice (duplicate creation fails via EntryPoint). + function testEntryPoint_DuplicateCreationFails() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = 100; + + // First creation succeeds + _submitOp(_buildCreationOp(proposalCallData, proposalNonce, _getNonce(0))); + + // Second creation with same proposalCallData and proposalNonce fails at validation + // (status != None) → returns SIG_VALIDATION_FAILED → "AA24 signature error" + _expectRevertOnOp(_buildCreationOp(proposalCallData, proposalNonce, _getNonce(0))); + } + + // ============ 5. Epoch / Reinstall Tests ============ + + /// @notice Proposals from a previous installation cannot be executed after reinstall. + function testEntryPoint_StaleProposalAfterReinstall() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = _getNonce(1); // use key=1 for execution + + // Create proposal + _createProposal(proposalCallData, proposalNonce); + + // Uninstall + vm.prank(address(account)); + policy.onUninstall(abi.encode(POLICY_ID, "")); + + // Reinstall (increments epoch) + vm.prank(address(account)); + policy.onInstall(abi.encode(POLICY_ID, DELAY, EXPIRATION, GRACE_PERIOD)); + + // Warp past delay + grace + vm.warp(block.timestamp + DELAY + GRACE_PERIOD + 1); + + // Execution fails: proposal epoch doesn't match new epoch + _expectRevertOnOp(_buildExecutionOp(proposalCallData, proposalNonce)); + assertEq(account.value(), 0); + } + + /// @notice After reinstall, new proposals can be created and executed normally. + function testEntryPoint_NewProposalAfterReinstall() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (99)); + uint256 proposalNonce = _getNonce(1); + + // Create a proposal in the first installation + _createProposal(proposalCallData, proposalNonce); + + // Uninstall + reinstall + vm.prank(address(account)); + policy.onUninstall(abi.encode(POLICY_ID, "")); + vm.prank(address(account)); + policy.onInstall(abi.encode(POLICY_ID, DELAY, EXPIRATION, GRACE_PERIOD)); + + // Create a NEW proposal with a different nonce + uint256 newProposalNonce = _getNonce(2); // key=2 + _createProposal(abi.encodeCall(MockTimelockAccount.setValue, (77)), newProposalNonce); + + vm.warp(block.timestamp + DELAY + GRACE_PERIOD + 1); + + // New proposal executes fine + _submitOp(_buildExecutionOp(abi.encodeCall(MockTimelockAccount.setValue, (77)), newProposalNonce)); + assertEq(account.value(), 77); + } + + // ============ 6. No-Op Calldata Variants ============ + + /// @notice Proposal creation with ERC-7579 execute(mode=0x00, "") no-op format. + function testEntryPoint_CreationViaERC7579NoOp() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = _getNonce(1); + + // ERC-7579 no-op: execute(bytes32(0), "") → selector + mode(32) + offset(32) + len(32) = 100 bytes + bytes memory erc7579Noop = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), ""); + + _submitOp(_buildCreationOpWithCalldata(erc7579Noop, proposalCallData, proposalNonce, _getNonce(0))); + + (TimelockPolicy.ProposalStatus status,,,) = + policy.getProposal(address(account), proposalCallData, proposalNonce, POLICY_ID, address(account)); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending)); + + // Verify lifecycle completes + vm.warp(block.timestamp + DELAY + GRACE_PERIOD + 1); + _submitOp(_buildExecutionOp(proposalCallData, proposalNonce)); + assertEq(account.value(), 42); + } + + /// @notice Proposal creation with executeUserOp selector-only (4 bytes) no-op format. + function testEntryPoint_CreationViaExecuteUserOpEmpty() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = _getNonce(1); + + // executeUserOp selector only (4 bytes) + bytes memory executeUserOpNoop = abi.encodePacked(IAccountExecute.executeUserOp.selector); + + _submitOp(_buildCreationOpWithCalldata(executeUserOpNoop, proposalCallData, proposalNonce, _getNonce(0))); + + (TimelockPolicy.ProposalStatus status,,,) = + policy.getProposal(address(account), proposalCallData, proposalNonce, POLICY_ID, address(account)); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending)); + } + + /// @notice Proposal creation with executeUserOp + ERC-7579 execute no-op (wrapped format). + function testEntryPoint_CreationViaExecuteUserOpWrappedERC7579() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = _getNonce(1); + + bytes memory erc7579Noop = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), ""); + bytes memory wrappedNoop = abi.encodePacked(IAccountExecute.executeUserOp.selector, erc7579Noop); + + _submitOp(_buildCreationOpWithCalldata(wrappedNoop, proposalCallData, proposalNonce, _getNonce(0))); + + (TimelockPolicy.ProposalStatus status,,,) = + policy.getProposal(address(account), proposalCallData, proposalNonce, POLICY_ID, address(account)); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending)); + } + + // ============ 7. Multiple Proposals ============ + + /// @notice Two independent proposals (different callData) can coexist and execute separately. + function testEntryPoint_TwoIndependentProposals() public { + bytes memory callDataA = abi.encodeCall(MockTimelockAccount.setValue, (10)); + bytes memory callDataB = abi.encodeCall(MockTimelockAccount.setValue, (20)); + + // Use separate nonce keys so execution nonces don't collide + uint256 nonceA = _getNonce(1); // key=1, seq=0 + uint256 nonceB = _getNonce(2); // key=2, seq=0 + + // Create both proposals + _createProposal(callDataA, nonceA); + _createProposal(callDataB, nonceB); + + vm.warp(block.timestamp + DELAY + GRACE_PERIOD + 1); + + // Execute B first + _submitOp(_buildExecutionOp(callDataB, nonceB)); + assertEq(account.value(), 20); + + // Execute A second (overwrites value) + _submitOp(_buildExecutionOp(callDataA, nonceA)); + assertEq(account.value(), 10); + + // Both are Executed + (TimelockPolicy.ProposalStatus statusA,,,) = + policy.getProposal(address(account), callDataA, nonceA, POLICY_ID, address(account)); + (TimelockPolicy.ProposalStatus statusB,,,) = + policy.getProposal(address(account), callDataB, nonceB, POLICY_ID, address(account)); + assertEq(uint256(statusA), uint256(TimelockPolicy.ProposalStatus.Executed)); + assertEq(uint256(statusB), uint256(TimelockPolicy.ProposalStatus.Executed)); + } + + /// @notice Cancel one proposal while leaving another intact, then execute the other. + function testEntryPoint_CancelOneExecuteAnother() public { + bytes memory callDataA = abi.encodeCall(MockTimelockAccount.setValue, (10)); + bytes memory callDataB = abi.encodeCall(MockTimelockAccount.setValue, (20)); + + uint256 nonceA = _getNonce(1); + uint256 nonceB = _getNonce(2); + + _createProposal(callDataA, nonceA); + _createProposal(callDataB, nonceB); + + // Cancel A + vm.prank(address(account)); + policy.cancelProposal(POLICY_ID, address(account), callDataA, nonceA); + + vm.warp(block.timestamp + DELAY + GRACE_PERIOD + 1); + + // A fails + _expectRevertOnOp(_buildExecutionOp(callDataA, nonceA)); + + // B succeeds + _submitOp(_buildExecutionOp(callDataB, nonceB)); + assertEq(account.value(), 20); + } + + /// @notice Proposals created at different times have different time windows. + /// @dev Uses explicit warp targets to avoid via_ir optimizer caching block.timestamp + /// across vm.warp boundaries (TIMESTAMP is constant within a real EVM transaction, + /// so the optimizer may legally forward the expression). + function testEntryPoint_SequentialProposalsDifferentWindows() public { + bytes memory callDataA = abi.encodeCall(MockTimelockAccount.setValue, (10)); + bytes memory callDataB = abi.encodeCall(MockTimelockAccount.setValue, (20)); + + uint256 nonceA = _getNonce(1); + uint256 nonceB = _getNonce(2); + + // Set a known start time to avoid depending on Foundry default block.timestamp + uint256 T0 = 10_000; + vm.warp(T0); + + // Create A at T0 + _createProposal(callDataA, nonceA); + // A's graceEnd = T0 + DELAY + GRACE_PERIOD = 10000 + 3600 + 1800 = 15400 + // A's validUntil = 15400 + EXPIRATION = 15400 + 86400 = 101800 + + // Warp 1 hour, create B at T0 + 1h + uint256 T1 = T0 + 1 hours; // 13600 + vm.warp(T1); + _createProposal(callDataB, nonceB); + // B's graceEnd = T1 + DELAY + GRACE_PERIOD = 13600 + 3600 + 1800 = 19000 + // B's validUntil = 19000 + EXPIRATION = 19000 + 86400 = 105400 + + // Warp to T0 + DELAY + GRACE_PERIOD + 1 = 15401 + // A's graceEnd (15400) < 15401 → A is executable + // B's graceEnd (19000) > 15401 → B still in grace + vm.warp(T0 + uint256(DELAY) + uint256(GRACE_PERIOD) + 1); + + // A works + _submitOp(_buildExecutionOp(callDataA, nonceA)); + assertEq(account.value(), 10); + + // B still blocked (B's graceEnd = 19000 > 15401) + _expectRevertOnOp(_buildExecutionOp(callDataB, nonceB)); + + // Warp to B's window: T1 + DELAY + GRACE_PERIOD + 1 = 19001 + vm.warp(T1 + uint256(DELAY) + uint256(GRACE_PERIOD) + 1); + _submitOp(_buildExecutionOp(callDataB, nonceB)); + assertEq(account.value(), 20); + } + + // ============ 8. Batch UserOps in Single handleOps ============ + + /// @notice Two creation UserOps can be batched in a single handleOps call. + function testEntryPoint_BatchCreation() public { + bytes memory callDataA = abi.encodeCall(MockTimelockAccount.setValue, (10)); + bytes memory callDataB = abi.encodeCall(MockTimelockAccount.setValue, (20)); + + uint256 nonceA = _getNonce(1); + uint256 nonceB = _getNonce(2); + + PackedUserOperation[] memory ops = new PackedUserOperation[](2); + ops[0] = _buildCreationOp(callDataA, nonceA, _getNonce(0)); + // The second op uses seq=1 for key=0 (after first op increments it) + ops[1] = _buildCreationOp(callDataB, nonceB, _getNonce(0) + 1); + + _submitOps(ops); + + // Both proposals should exist + (TimelockPolicy.ProposalStatus statusA,,,) = + policy.getProposal(address(account), callDataA, nonceA, POLICY_ID, address(account)); + (TimelockPolicy.ProposalStatus statusB,,,) = + policy.getProposal(address(account), callDataB, nonceB, POLICY_ID, address(account)); + assertEq(uint256(statusA), uint256(TimelockPolicy.ProposalStatus.Pending)); + assertEq(uint256(statusB), uint256(TimelockPolicy.ProposalStatus.Pending)); + } + + /// @notice Two execution UserOps can be batched in a single handleOps call. + function testEntryPoint_BatchExecution() public { + bytes memory callDataA = abi.encodeCall(MockTimelockAccount.setValue, (10)); + bytes memory callDataB = abi.encodeCall(MockTimelockAccount.setValue, (20)); + + uint256 nonceA = _getNonce(1); + uint256 nonceB = _getNonce(2); + + _createProposal(callDataA, nonceA); + _createProposal(callDataB, nonceB); + + vm.warp(block.timestamp + DELAY + GRACE_PERIOD + 1); + + PackedUserOperation[] memory ops = new PackedUserOperation[](2); + ops[0] = _buildExecutionOp(callDataA, nonceA); + ops[1] = _buildExecutionOp(callDataB, nonceB); + + _submitOps(ops); + + // Last one wins for the value, both should be Executed + assertEq(account.value(), 20); + + (TimelockPolicy.ProposalStatus statusA,,,) = + policy.getProposal(address(account), callDataA, nonceA, POLICY_ID, address(account)); + (TimelockPolicy.ProposalStatus statusB,,,) = + policy.getProposal(address(account), callDataB, nonceB, POLICY_ID, address(account)); + assertEq(uint256(statusA), uint256(TimelockPolicy.ProposalStatus.Executed)); + assertEq(uint256(statusB), uint256(TimelockPolicy.ProposalStatus.Executed)); + } + + // ============ 9. Nonce Key Separation ============ + + /// @notice Using different EntryPoint nonce keys for creation vs execution. + function testEntryPoint_SeparateNonceKeys() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + + // Use key=5 for execution → proposalNonce = getNonce(account, 5) + uint256 proposalNonce = _getNonce(5); + + // Create using key=0 + _submitOp(_buildCreationOp(proposalCallData, proposalNonce, _getNonce(0))); + + vm.warp(block.timestamp + DELAY + GRACE_PERIOD + 1); + + // Execute using key=5 (nonce matches proposalNonce) + _submitOp(_buildExecutionOp(proposalCallData, proposalNonce)); + assertEq(account.value(), 42); + } + + // ============ 10. Multiple Accounts ============ + + /// @notice Two accounts with the same policy can have independent proposals. + function testEntryPoint_TwoAccountsIndependent() public { + // Deploy a second account + MockTimelockAccount account2 = new MockTimelockAccount(entryPoint, policy, POLICY_ID); + vm.deal(address(account2), 10 ether); + vm.prank(address(account2)); + policy.onInstall(abi.encode(POLICY_ID, DELAY, EXPIRATION, GRACE_PERIOD)); + + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce1 = entryPoint.getNonce(address(account), 1); + uint256 proposalNonce2 = entryPoint.getNonce(address(account2), 1); + + // Create proposal for account 1 + _createProposal(proposalCallData, proposalNonce1); + + // Create proposal for account 2 + PackedUserOperation memory op2 = PackedUserOperation({ + sender: address(account2), + nonce: entryPoint.getNonce(address(account2), 0), + initCode: "", + callData: "", + accountGasLimits: bytes32(abi.encodePacked(uint128(500_000), uint128(500_000))), + preVerificationGas: 100_000, + gasFees: bytes32(abi.encodePacked(uint128(1 gwei), uint128(1 gwei))), + paymasterAndData: "", + signature: _proposalSig(proposalCallData, proposalNonce2) + }); + _submitOp(op2); + + vm.warp(block.timestamp + DELAY + GRACE_PERIOD + 1); + + // Execute account 1 + _submitOp(_buildExecutionOp(proposalCallData, proposalNonce1)); + assertEq(account.value(), 42); + + // Execute account 2 + PackedUserOperation memory exec2 = PackedUserOperation({ + sender: address(account2), + nonce: proposalNonce2, + initCode: "", + callData: proposalCallData, + accountGasLimits: bytes32(abi.encodePacked(uint128(500_000), uint128(500_000))), + preVerificationGas: 100_000, + gasFees: bytes32(abi.encodePacked(uint128(1 gwei), uint128(1 gwei))), + paymasterAndData: "", + signature: "" + }); + _submitOp(exec2); + assertEq(account2.value(), 42); + } + + // ============ 11. Edge-Case Calldata ============ + + /// @notice Proposal with empty proposalCallData (legitimate: could be a "send ETH" tx). + function testEntryPoint_EmptyProposalCallData() public { + bytes memory proposalCallData = ""; + uint256 proposalNonce = _getNonce(1); + + // Proposal creation with empty calldata inside signature + // sig = [len=0 (32 bytes)] + [nonce (32 bytes)] + [0x00 (1 byte)] = 65 bytes total ✓ + _createProposal(proposalCallData, proposalNonce); + + (TimelockPolicy.ProposalStatus status,,,) = + policy.getProposal(address(account), proposalCallData, proposalNonce, POLICY_ID, address(account)); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending)); + } + + /// @notice Proposal with large calldata. + function testEntryPoint_LargeProposalCallData() public { + // Build a large calldata (256 bytes of arbitrary data after the selector) + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (12345)); + // Pad to make it larger + bytes memory largeCallData = abi.encodePacked(proposalCallData, new bytes(256)); + + uint256 proposalNonce = _getNonce(1); + + _createProposal(largeCallData, proposalNonce); + + (TimelockPolicy.ProposalStatus status,,,) = + policy.getProposal(address(account), largeCallData, proposalNonce, POLICY_ID, address(account)); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending)); + } + + // ============ 12. Events ============ + + /// @notice ProposalCreated event is emitted during creation through EntryPoint. + function testEntryPoint_ProposalCreatedEvent() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = _getNonce(1); + + bytes32 expectedKey = policy.computeUserOpKey(address(account), proposalCallData, proposalNonce); + uint256 expectedValidAfter = block.timestamp + DELAY; + uint256 expectedValidUntil = block.timestamp + DELAY + GRACE_PERIOD + EXPIRATION; + + vm.expectEmit(true, true, true, true); + emit TimelockPolicy.ProposalCreated( + address(account), POLICY_ID, expectedKey, expectedValidAfter, expectedValidUntil + ); + + _submitOp(_buildCreationOp(proposalCallData, proposalNonce, _getNonce(0))); + } + + /// @notice ProposalExecuted event is emitted during execution through EntryPoint. + function testEntryPoint_ProposalExecutedEvent() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = _getNonce(1); + + _createProposal(proposalCallData, proposalNonce); + + vm.warp(block.timestamp + DELAY + GRACE_PERIOD + 1); + + bytes32 expectedKey = policy.computeUserOpKey(address(account), proposalCallData, proposalNonce); + + vm.expectEmit(true, true, true, true); + emit TimelockPolicy.ProposalExecuted(address(account), POLICY_ID, expectedKey); + + _submitOp(_buildExecutionOp(proposalCallData, proposalNonce)); + } + + // ============ 13. EntryPoint Nonce Accounting ============ + + /// @notice EntryPoint nonce advances correctly across multiple operations. + function testEntryPoint_NonceAccounting() public { + assertEq(_getNonce(0), 0); + + bytes memory cd = abi.encodeCall(MockTimelockAccount.setValue, (1)); + uint256 pNonce = _getNonce(1); + + // Op 1: creation + _submitOp(_buildCreationOp(cd, pNonce, 0)); + assertEq(_getNonce(0), 1); + + // Op 2: another creation with different proposal nonce + bytes memory cd2 = abi.encodeCall(MockTimelockAccount.setValue, (2)); + uint256 pNonce2 = _getNonce(2); + _submitOp(_buildCreationOp(cd2, pNonce2, 1)); + assertEq(_getNonce(0), 2); + + vm.warp(block.timestamp + DELAY + GRACE_PERIOD + 1); + + // Op 3: execution of first proposal (key=1) + _submitOp(_buildExecutionOp(cd, pNonce)); + assertEq(_getNonce(1), pNonce + 1); + } + + /// @notice Reverted handleOps does NOT advance the EntryPoint nonce. + function testEntryPoint_RevertedOpDoesNotAdvanceNonce() public { + uint256 nonceBefore = _getNonce(0); + + // Try execution without proposal → revert + _expectRevertOnOp(_buildExecutionOp(abi.encodeCall(MockTimelockAccount.setValue, (1)), 0)); + + // Nonce unchanged + assertEq(_getNonce(0), nonceBefore); + } + + // ============ 14. Gas / Balance Tests ============ + + /// @notice Account pays gas to EntryPoint from its balance. + function testEntryPoint_AccountPaysGas() public { + uint256 balBefore = address(account).balance; + + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + _submitOp(_buildCreationOp(proposalCallData, _getNonce(1), _getNonce(0))); + + // Account balance should have decreased (gas was paid) + assertTrue(address(account).balance < balBefore); + } + + /// @notice Beneficiary receives collected gas fees. + function testEntryPoint_BeneficiaryReceivesFees() public { + uint256 balBefore = BENEFICIARY.balance; + + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + _submitOp(_buildCreationOp(proposalCallData, _getNonce(1), _getNonce(0))); + + // Beneficiary should have received fees + assertTrue(BENEFICIARY.balance > balBefore); + } + + // ============ 15. Full Grace Period Race-Condition Scenario ============ + + /// @notice Simulate the race condition the grace period is designed to prevent: + /// 1. Session key creates proposal + /// 2. Delay passes, session key submits execution + /// 3. Owner sees it and cancels during grace period + /// 4. Execution fails because EntryPoint rejects (validAfter = graceEnd) + function testEntryPoint_GracePeriodRaceCondition() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (999)); + uint256 proposalNonce = _getNonce(1); + + // Step 1: Session key creates proposal + _createProposal(proposalCallData, proposalNonce); + + // Step 2: Warp to delay + 1 second (within grace period) + vm.warp(block.timestamp + DELAY + 1); + + // Step 3: Session key tries to execute but EntryPoint blocks it + // (validAfter = graceEnd which is in the future) + _expectRevertOnOp(_buildExecutionOp(proposalCallData, proposalNonce)); + assertEq(account.value(), 0); + + // Step 4: Owner cancels during grace period + vm.prank(address(account)); + policy.cancelProposal(POLICY_ID, address(account), proposalCallData, proposalNonce); + + // Step 5: Even after grace period, execution fails (cancelled) + vm.warp(block.timestamp + GRACE_PERIOD + 1); + _expectRevertOnOp(_buildExecutionOp(proposalCallData, proposalNonce)); + assertEq(account.value(), 0); + } + + // ============ 16. Policy Not Installed ============ + + /// @notice Operations on an account with uninstalled policy fail. + function testEntryPoint_UninstalledPolicyRevertsAll() public { + // Uninstall + vm.prank(address(account)); + policy.onUninstall(abi.encode(POLICY_ID, "")); + + // Creation fails + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + _expectRevertOnOp(_buildCreationOp(proposalCallData, 100, _getNonce(0))); + } + + // ============ 17. EntryPoint Deposit Tests ============ + + /// @notice Account can prefund via EntryPoint deposit, reducing per-op gas drain. + function testEntryPoint_DepositThenOperate() public { + // Deposit into EntryPoint on behalf of account + entryPoint.depositTo{value: 1 ether}(address(account)); + + uint256 balBefore = address(account).balance; + + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + _submitOp(_buildCreationOp(proposalCallData, _getNonce(1), _getNonce(0))); + + // Account's direct balance should not have decreased because deposit covers gas + assertEq(address(account).balance, balBefore); + } + + // ============ 18. Create-Cancel-Recreate Cycle ============ + + /// @notice After cancellation, a new proposal with different nonce for the same calldata works. + function testEntryPoint_CreateCancelRecreateWithDifferentNonce() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 nonce1 = _getNonce(1); + + // Create + _createProposal(proposalCallData, nonce1); + + // Cancel + vm.prank(address(account)); + policy.cancelProposal(POLICY_ID, address(account), proposalCallData, nonce1); + + // Recreate with different nonce + uint256 nonce2 = _getNonce(2); + _createProposal(proposalCallData, nonce2); + + (TimelockPolicy.ProposalStatus status,,,) = + policy.getProposal(address(account), proposalCallData, nonce2, POLICY_ID, address(account)); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending)); + + // Execute the new proposal + vm.warp(block.timestamp + DELAY + GRACE_PERIOD + 1); + _submitOp(_buildExecutionOp(proposalCallData, nonce2)); + assertEq(account.value(), 42); + } + + // ============ 19. Exact Boundary: Delay Not Passed ============ + + /// @notice At exactly delay (no grace period overlap), execution is still blocked. + function testEntryPoint_AtExactDelayStillBlocked() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + uint256 proposalNonce = _getNonce(1); + + uint256 t0 = block.timestamp; + _createProposal(proposalCallData, proposalNonce); + + // At exactly validAfter (= t0 + DELAY): this is start of grace period, not end + // graceEnd = t0 + DELAY + GRACE_PERIOD, so block.timestamp <= graceEnd + vm.warp(t0 + DELAY); + _expectRevertOnOp(_buildExecutionOp(proposalCallData, proposalNonce)); + } + + // ============ 20. Same Calldata Different Nonces ============ + + /// @notice Same callData can be proposed with different nonces independently. + function testEntryPoint_SameCallDataDifferentNonces() public { + bytes memory proposalCallData = abi.encodeCall(MockTimelockAccount.setValue, (42)); + + uint256 nonceA = _getNonce(1); + uint256 nonceB = _getNonce(2); + + _createProposal(proposalCallData, nonceA); + _createProposal(proposalCallData, nonceB); + + // Both exist + (TimelockPolicy.ProposalStatus statusA,,,) = + policy.getProposal(address(account), proposalCallData, nonceA, POLICY_ID, address(account)); + (TimelockPolicy.ProposalStatus statusB,,,) = + policy.getProposal(address(account), proposalCallData, nonceB, POLICY_ID, address(account)); + assertEq(uint256(statusA), uint256(TimelockPolicy.ProposalStatus.Pending)); + assertEq(uint256(statusB), uint256(TimelockPolicy.ProposalStatus.Pending)); + + vm.warp(block.timestamp + DELAY + GRACE_PERIOD + 1); + + // Execute A, cancel B + _submitOp(_buildExecutionOp(proposalCallData, nonceA)); + assertEq(account.value(), 42); + + vm.prank(address(account)); + policy.cancelProposal(POLICY_ID, address(account), proposalCallData, nonceB); + + (statusA,,,) = policy.getProposal(address(account), proposalCallData, nonceA, POLICY_ID, address(account)); + (statusB,,,) = policy.getProposal(address(account), proposalCallData, nonceB, POLICY_ID, address(account)); + assertEq(uint256(statusA), uint256(TimelockPolicy.ProposalStatus.Executed)); + assertEq(uint256(statusB), uint256(TimelockPolicy.ProposalStatus.Cancelled)); + } +} diff --git a/test/utils/MockTimelockAccount.sol b/test/utils/MockTimelockAccount.sol new file mode 100644 index 0000000..ba17609 --- /dev/null +++ b/test/utils/MockTimelockAccount.sol @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import {IAccount} from "account-abstraction/interfaces/IAccount.sol"; +import {PackedUserOperation} from "account-abstraction/interfaces/PackedUserOperation.sol"; +import {IEntryPoint} from "account-abstraction/interfaces/IEntryPoint.sol"; +import {TimelockPolicy} from "../../src/policies/TimelockPolicy.sol"; + +/// @title MockTimelockAccount +/// @notice Minimal IAccount that delegates validation to TimelockPolicy. +/// Used for integration testing with the real EntryPoint. +contract MockTimelockAccount is IAccount { + IEntryPoint public immutable entryPoint; + TimelockPolicy public immutable policy; + bytes32 public immutable policyId; + + uint256 public value; + + constructor(IEntryPoint _entryPoint, TimelockPolicy _policy, bytes32 _policyId) { + entryPoint = _entryPoint; + policy = _policy; + policyId = _policyId; + } + + function validateUserOp(PackedUserOperation calldata userOp, bytes32, uint256 missingAccountFunds) + external + returns (uint256 validationData) + { + require(msg.sender == address(entryPoint), "only entrypoint"); + + if (missingAccountFunds > 0) { + (bool ok,) = payable(msg.sender).call{value: missingAccountFunds}(""); + require(ok); + } + + return policy.checkUserOpPolicy(policyId, userOp); + } + + function setValue(uint256 _value) external { + value = _value; + } + + receive() external payable {} +}