diff --git a/src/policies/TimelockPolicy.sol b/src/policies/TimelockPolicy.sol index ce86996..f1340ca 100644 --- a/src/policies/TimelockPolicy.sol +++ b/src/policies/TimelockPolicy.sol @@ -10,10 +10,7 @@ import { MODULE_TYPE_POLICY, MODULE_TYPE_STATELESS_VALIDATOR, MODULE_TYPE_STATELESS_VALIDATOR_WITH_SENDER, - SIG_VALIDATION_SUCCESS_UINT, - SIG_VALIDATION_FAILED_UINT, - ERC1271_MAGICVALUE, - ERC1271_INVALID + SIG_VALIDATION_FAILED_UINT } from "src/types/Constants.sol"; /** @@ -39,11 +36,15 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW ProposalStatus status; uint48 validAfter; // Timestamp when proposal becomes executable uint48 validUntil; // Timestamp when proposal expires + uint256 epoch; // Epoch when proposal was created } // Storage: id => wallet => config mapping(bytes32 => mapping(address => TimelockConfig)) public timelockConfig; + // Storage: id => wallet => epoch (persists across uninstall/reinstall) + mapping(bytes32 => mapping(address => uint256)) public currentEpoch; + // Storage: userOpKey => id => wallet => proposal // userOpKey = keccak256(abi.encode(account, keccak256(callData), nonce)) mapping(bytes32 => mapping(bytes32 => mapping(address => Proposal))) public proposals; @@ -66,6 +67,8 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW error ProposalExpired(uint256 validUntil, uint256 currentTime); error ProposalNotPending(); error OnlyAccount(); + error ProposalFromPreviousEpoch(); + error ParametersTooLarge(); /** * @notice Install the timelock policy @@ -80,6 +83,13 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW if (delay == 0) revert InvalidDelay(); if (expirationPeriod == 0) revert InvalidExpirationPeriod(); + // Prevent uint48 overflow in createProposal: uint48(block.timestamp) + delay + expirationPeriod + if (uint256(delay) + uint256(expirationPeriod) > type(uint48).max - block.timestamp) { + revert ParametersTooLarge(); + } + + // Increment epoch to invalidate any proposals from previous installations + currentEpoch[id][msg.sender]++; timelockConfig[id][msg.sender] = TimelockConfig({delay: delay, expirationPeriod: expirationPeriod, initialized: true}); @@ -131,9 +141,9 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW revert ProposalAlreadyExists(); } - // Create proposal (stored by userOpKey) + // Create proposal (stored by userOpKey) with current epoch proposals[userOpKey][id][account] = - Proposal({status: ProposalStatus.Pending, validAfter: validAfter, validUntil: validUntil}); + Proposal({status: ProposalStatus.Pending, validAfter: validAfter, validUntil: validUntil, epoch: currentEpoch[id][account]}); emit ProposalCreated(account, id, userOpKey, validAfter, validUntil); } @@ -219,14 +229,15 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW return SIG_VALIDATION_FAILED_UINT; // Proposal already exists } - // Create proposal + // Create proposal with current epoch proposals[userOpKey][id][account] = - Proposal({status: ProposalStatus.Pending, validAfter: validAfter, validUntil: validUntil}); + Proposal({status: ProposalStatus.Pending, validAfter: validAfter, validUntil: validUntil, epoch: currentEpoch[id][account]}); emit ProposalCreated(account, id, userOpKey, validAfter, validUntil); - // Return failure to prevent execution (this was just proposal creation) - return SIG_VALIDATION_FAILED_UINT; + // Return success (validationData = 0) to allow the proposal creation to persist + // EntryPoint treats validationData == 0 as valid (no time range check) + return _packValidationData(0, 0); } /** @@ -244,6 +255,9 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW // Check proposal exists and is pending if (proposal.status != ProposalStatus.Pending) return SIG_VALIDATION_FAILED_UINT; + // Check proposal is from current epoch (not a stale proposal from previous installation) + if (proposal.epoch != currentEpoch[id][account]) return SIG_VALIDATION_FAILED_UINT; + // Mark as executed proposal.status = ProposalStatus.Executed; @@ -290,20 +304,22 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW */ function _isNoOpERC7579Execute(bytes calldata callData) internal view returns (bool) { // execute(bytes32 mode, bytes calldata executionCalldata) - // Need: 4 (selector) + 32 (mode) + 32 (offset) + 32 (length) + data - if (callData.length < 68) return false; + // ABI layout: 4 (selector) + 32 (mode) + 32 (offset) + 32 (length) + data + if (callData.length < 100) return false; - // Decode the offset to executionCalldata (should be 32) + // Offset to executionCalldata: 2 head slots (mode + offset) = 64 uint256 offset = uint256(bytes32(callData[36:68])); - if (offset != 32) return false; + if (offset != 64) return false; // Decode the length of executionCalldata - if (callData.length < 100) return false; uint256 execDataLength = uint256(bytes32(callData[68:100])); - // For single execution mode, executionCalldata format is: - // target (20 bytes) + value (32 bytes) + calldata (variable) - if (execDataLength < 52) return false; + // ERC-7579 single execution uses compact format (no length prefix): + // executionCalldata = abi.encodePacked(target, value, calldata) + // target (20 bytes) + value (32 bytes) = 52 bytes with no inner calldata + if (execDataLength != 52) return false; + + if (callData.length < 152) return false; // Extract target address (first 20 bytes of executionCalldata) address target = address(bytes20(callData[100:120])); @@ -315,26 +331,14 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW uint256 value = uint256(bytes32(callData[120:152])); // Value must be 0 - if (value != 0) return false; - - // Check calldata length (remaining bytes should indicate empty calldata) - // executionCalldata = target(20) + value(32) + calldataLength(32) + calldata - if (callData.length < 184) { - // If we don't have enough for calldata length field, it's malformed - return false; - } - - uint256 innerCalldataLength = uint256(bytes32(callData[152:184])); - - // Inner calldata must be empty - return innerCalldataLength == 0; + return value == 0; } /** * @notice Check if executeUserOp call is a no-op * @dev Valid: executeUserOp("", bytes32) */ - function _isNoOpExecuteUserOp(bytes calldata callData) internal view returns (bool) { + function _isNoOpExecuteUserOp(bytes calldata callData) internal pure returns (bool) { // executeUserOp(bytes calldata userOp, bytes32 userOpHash) // Format: 4 (selector) + 32 (userOp offset) + 32 (userOpHash) + 32 (userOp length) + userOp data if (callData.length < 100) return false; @@ -368,37 +372,33 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW /** * @notice Check signature against timelock policy (for ERC-1271) - * @param id The policy ID - * @return validationData 0 if valid, 1 if invalid + * @dev TimelockPolicy does not support ERC-1271 signature validation - always reverts */ - function checkSignaturePolicy(bytes32 id, address, bytes32 hash, bytes calldata sig) + function checkSignaturePolicy(bytes32, address, bytes32, bytes calldata) external - view + pure override returns (uint256) { - bytes4 result = _validateSignaturePolicy(id, msg.sender, hash, sig); - return result == ERC1271_MAGICVALUE ? 0 : 1; + revert("TimelockPolicy: signature validation not supported"); } - function validateSignatureWithData(bytes32, bytes calldata, bytes calldata data) + function validateSignatureWithData(bytes32, bytes calldata, bytes calldata) external pure override(IStatelessValidator) returns (bool) { - (uint48 delay, uint48 expirationPeriod) = abi.decode(data, (uint48, uint48)); - return delay != 0 && expirationPeriod != 0; + revert("TimelockPolicy: stateless signature validation not supported"); } - function validateSignatureWithDataWithSender(address, bytes32, bytes calldata, bytes calldata data) + function validateSignatureWithDataWithSender(address, bytes32, bytes calldata, bytes calldata) external pure override(IStatelessValidatorWithSender) returns (bool) { - (uint48 delay, uint48 expirationPeriod) = abi.decode(data, (uint48, uint48)); - return delay != 0 && expirationPeriod != 0; + revert("TimelockPolicy: stateless signature validation not supported"); } // ==================== Internal Shared Logic ==================== @@ -425,23 +425,6 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW return _handleProposalExecutionInternal(id, userOp, account); } - /** - * @notice Internal function to validate signature policy - * @dev Shared logic for both installed and stateless validator modes - */ - function _validateSignaturePolicy(bytes32 id, address account, bytes32 hash, bytes calldata sig) - internal - view - returns (bytes4) - { - TimelockConfig storage config = timelockConfig[id][account]; - if (!config.initialized) return ERC1271_INVALID; - - // For signature validation, we're more permissive - // Timelock is primarily for userOp execution - return ERC1271_MAGICVALUE; - } - /** * @notice Get proposal details * @param account The account address diff --git a/test/TimelockPolicy.t.sol b/test/TimelockPolicy.t.sol index 55d7541..2df90c2 100644 --- a/test/TimelockPolicy.t.sol +++ b/test/TimelockPolicy.t.sol @@ -107,21 +107,33 @@ contract TimelockPolicyTest is PolicyTestBase, StatelessValidatorTestBase, State return statelessValidationSignature(bytes32(0), valid); } - // Override stateless validator tests to use proper data parameter + // Override stateless validator tests - TimelockPolicy reverts for stateless validation function testStatlessValidatorFail() external override { IStatelessValidator validatorModule = IStatelessValidator(address(module)); bytes32 message = keccak256(abi.encodePacked("TEST_MESSAGE")); (, bytes memory sig) = statelessValidationSignature(message, false); - // For TimelockPolicy, validation fails if delay or expirationPeriod is 0 - bytes memory invalidData = abi.encode(uint48(0), uint48(0)); + bytes memory data = abi.encode(uint48(0), uint48(0)); vm.startPrank(WALLET); - bool result = validatorModule.validateSignatureWithData(message, sig, invalidData); + vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + validatorModule.validateSignatureWithData(message, sig, data); vm.stopPrank(); + } + + function testStatelessValidatorSuccess() external override { + IStatelessValidator validatorModule = IStatelessValidator(address(module)); + + bytes32 message = keccak256(abi.encodePacked("TEST_MESSAGE")); + (, bytes memory sig) = statelessValidationSignature(message, true); + + bytes memory validData = abi.encode(delay, expirationPeriod); - assertFalse(result); + vm.startPrank(WALLET); + vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + validatorModule.validateSignatureWithData(message, sig, validData); + vm.stopPrank(); } function testStatelessValidatorWithSenderFail() external override { @@ -130,14 +142,26 @@ contract TimelockPolicyTest is PolicyTestBase, StatelessValidatorTestBase, State bytes32 message = keccak256(abi.encodePacked("TEST_MESSAGE")); (address caller, bytes memory sig) = statelessValidationSignatureWithSender(message, false); - // For TimelockPolicy, validation fails if delay or expirationPeriod is 0 - bytes memory invalidData = abi.encode(uint48(0), uint48(0)); + bytes memory data = abi.encode(uint48(0), uint48(0)); vm.startPrank(WALLET); - bool result = validatorModule.validateSignatureWithDataWithSender(caller, message, sig, invalidData); + vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + validatorModule.validateSignatureWithDataWithSender(caller, message, sig, data); vm.stopPrank(); + } + + function testStatelessValidatorWithSenderSuccess() external override { + IStatelessValidatorWithSender validatorModule = IStatelessValidatorWithSender(address(module)); - assertFalse(result); + bytes32 message = keccak256(abi.encodePacked("TEST_MESSAGE")); + (address caller, bytes memory sig) = statelessValidationSignatureWithSender(message, true); + + bytes memory validData = abi.encode(delay, expirationPeriod); + + vm.startPrank(WALLET); + vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + validatorModule.validateSignatureWithDataWithSender(caller, message, sig, validData); + vm.stopPrank(); } // Override the checkUserOpPolicy tests because TimelockPolicy has special behavior @@ -184,22 +208,32 @@ contract TimelockPolicyTest is PolicyTestBase, StatelessValidatorTestBase, State assertEq(validationResult, 1); } - // Override signature policy test because TimelockPolicy always passes for installed accounts - function testPolicyCheckSignaturePolicyFail() public payable override { + // Override signature policy tests - TimelockPolicy reverts for signature validation + function testPolicyCheckSignaturePolicySuccess() public payable override { TimelockPolicy policyModule = TimelockPolicy(address(module)); + vm.startPrank(WALLET); + policyModule.onInstall(abi.encodePacked(policyId(), installData())); + vm.stopPrank(); + + bytes32 testHash = keccak256(abi.encodePacked("TEST_HASH")); + (address sender, bytes memory sigData) = validSignatureData(testHash); - // Don't install for this wallet - address nonInstalledWallet = address(0xBEEF); + vm.startPrank(WALLET); + vm.expectRevert("TimelockPolicy: signature validation not supported"); + policyModule.checkSignaturePolicy(policyId(), sender, testHash, sigData); + vm.stopPrank(); + } + + function testPolicyCheckSignaturePolicyFail() public payable override { + TimelockPolicy policyModule = TimelockPolicy(address(module)); bytes32 testHash = keccak256(abi.encodePacked("TEST_HASH")); (address sender, bytes memory sigData) = invalidSignatureData(testHash); - vm.startPrank(nonInstalledWallet); - uint256 result = policyModule.checkSignaturePolicy(policyId(), sender, testHash, sigData); + vm.startPrank(WALLET); + vm.expectRevert("TimelockPolicy: signature validation not supported"); + policyModule.checkSignaturePolicy(policyId(), sender, testHash, sigData); vm.stopPrank(); - - // Should fail for non-installed account - assertFalse(result == 0); } // Additional TimelockPolicy-specific tests @@ -284,8 +318,9 @@ contract TimelockPolicyTest is PolicyTestBase, StatelessValidatorTestBase, State uint256 result = policyModule.checkUserOpPolicy(policyId(), userOp); vm.stopPrank(); - // Should return failure (1) because this was proposal creation, not execution - assertEq(result, 1); + // Returns success (validationData = 0) - valid indefinitely per ERC-4337 + // This allows proposal creation via UserOp without external caller + assertEq(result, 0); // Verify proposal was created (TimelockPolicy.ProposalStatus status,,) = @@ -293,4 +328,40 @@ contract TimelockPolicyTest is PolicyTestBase, StatelessValidatorTestBase, State assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending)); } + + // Test that stale proposals from previous installations cannot be executed + function testStaleProposalNotExecutableAfterReinstall() public { + TimelockPolicy policyModule = TimelockPolicy(address(module)); + vm.startPrank(WALLET); + policyModule.onInstall(abi.encodePacked(policyId(), installData())); + vm.stopPrank(); + + PackedUserOperation memory userOp = validUserOp(); + + // Create a proposal + vm.startPrank(WALLET); + policyModule.createProposal(policyId(), WALLET, userOp.callData, userOp.nonce); + vm.stopPrank(); + + // Fast forward past delay + vm.warp(block.timestamp + delay + 1); + + // Uninstall the policy + vm.startPrank(WALLET); + policyModule.onUninstall(abi.encodePacked(policyId(), "")); + vm.stopPrank(); + + // Reinstall the policy + vm.startPrank(WALLET); + policyModule.onInstall(abi.encodePacked(policyId(), installData())); + vm.stopPrank(); + + // Try to execute the stale proposal - should fail + vm.startPrank(WALLET); + uint256 validationResult = policyModule.checkUserOpPolicy(policyId(), userOp); + vm.stopPrank(); + + // Should fail (return 1 = SIG_VALIDATION_FAILED_UINT) because proposal is from previous epoch + assertEq(validationResult, 1); + } } diff --git a/test/base/PolicyTestBase.sol b/test/base/PolicyTestBase.sol index ca693a2..2daf0f7 100644 --- a/test/base/PolicyTestBase.sol +++ b/test/base/PolicyTestBase.sol @@ -98,7 +98,7 @@ abstract contract PolicyTestBase is ModuleTestBase { assertFalse(validationResult == 0); } - function testPolicyCheckSignaturePolicySuccess() public payable { + function testPolicyCheckSignaturePolicySuccess() public payable virtual { IPolicy policyModule = IPolicy(address(module)); vm.startPrank(WALLET); policyModule.onInstall(abi.encodePacked(policyId(), installData())); diff --git a/test/base/StatelessValidatorTestBase.sol b/test/base/StatelessValidatorTestBase.sol index 48d8239..08bf240 100644 --- a/test/base/StatelessValidatorTestBase.sol +++ b/test/base/StatelessValidatorTestBase.sol @@ -21,7 +21,7 @@ abstract contract StatelessValidatorTestBase is ModuleTestBase { assertTrue(result); } - function testStatelessValidatorSuccess() external { + function testStatelessValidatorSuccess() external virtual { IStatelessValidator validatorModule = IStatelessValidator(address(module)); bytes32 message = keccak256(abi.encodePacked("TEST_MESSAGE")); diff --git a/test/base/StatelessValidatorWithSenderTestBase.sol b/test/base/StatelessValidatorWithSenderTestBase.sol index 920b1b2..5a05801 100644 --- a/test/base/StatelessValidatorWithSenderTestBase.sol +++ b/test/base/StatelessValidatorWithSenderTestBase.sol @@ -21,7 +21,7 @@ abstract contract StatelessValidatorWithSenderTestBase is ModuleTestBase { assertTrue(result); } - function testStatelessValidatorWithSenderSuccess() external { + function testStatelessValidatorWithSenderSuccess() external virtual { IStatelessValidatorWithSender validatorModule = IStatelessValidatorWithSender(address(module)); bytes32 message = keccak256(abi.encodePacked("TEST_MESSAGE")); diff --git a/test/btt/Timelock.t.sol b/test/btt/Timelock.t.sol new file mode 100644 index 0000000..04ec804 --- /dev/null +++ b/test/btt/Timelock.t.sol @@ -0,0 +1,1053 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import {Test} from "forge-std/Test.sol"; +import {TimelockPolicy} from "../../src/policies/TimelockPolicy.sol"; +import {PackedUserOperation} from "account-abstraction/interfaces/PackedUserOperation.sol"; +import {IERC7579Execution} from "openzeppelin-contracts/contracts/interfaces/draft-IERC7579.sol"; +import {IAccountExecute} from "account-abstraction/interfaces/IAccountExecute.sol"; +import {IModule} from "../../src/interfaces/IERC7579Modules.sol"; +import { + MODULE_TYPE_POLICY, + MODULE_TYPE_STATELESS_VALIDATOR, + MODULE_TYPE_STATELESS_VALIDATOR_WITH_SENDER +} from "../../src/types/Constants.sol"; + +/** + * @title TimelockTest + * @notice BTT tests for TimelockPolicy contract + */ +contract TimelockTest is Test { + TimelockPolicy public timelockPolicy; + + address public constant WALLET = address(0x1234); + address public constant ATTACKER = address(0xdead); + bytes32 public constant POLICY_ID = bytes32(uint256(1)); + + uint48 public constant DELAY = 1 hours; + uint48 public constant EXPIRATION = 1 days; + + uint256 public constant SIG_VALIDATION_FAILED = 1; + + function setUp() public { + timelockPolicy = new TimelockPolicy(); + + // Install policy for WALLET + bytes memory installData = abi.encode(POLICY_ID, DELAY, EXPIRATION); + vm.prank(WALLET); + timelockPolicy.onInstall(installData); + } + + // ============ Helper Functions ============ + + function _createNoopUserOp(address sender, bytes memory signature) + internal + pure + returns (PackedUserOperation memory) + { + return PackedUserOperation({ + sender: sender, + nonce: 0, + initCode: "", + callData: "", + accountGasLimits: bytes32(abi.encodePacked(uint128(100000), uint128(200000))), + preVerificationGas: 0, + gasFees: bytes32(abi.encodePacked(uint128(1), uint128(1))), + paymasterAndData: "", + signature: signature + }); + } + + function _createUserOpWithCalldata(address sender, bytes memory callData, uint256 nonce, bytes memory signature) + internal + pure + returns (PackedUserOperation memory) + { + return PackedUserOperation({ + sender: sender, + nonce: nonce, + initCode: "", + callData: callData, + accountGasLimits: bytes32(abi.encodePacked(uint128(100000), uint128(200000))), + preVerificationGas: 0, + gasFees: bytes32(abi.encodePacked(uint128(1), uint128(1))), + paymasterAndData: "", + signature: signature + }); + } + + function _createProposalSignature(bytes memory proposalCallData, uint256 proposalNonce) + internal + pure + returns (bytes memory) + { + return + abi.encodePacked(bytes32(proposalCallData.length), proposalCallData, bytes32(proposalNonce), bytes1(0x00)); + } + + function _packValidationData(uint48 validAfter, uint48 validUntil) internal pure returns (uint256) { + return uint256(validAfter) << 208 | uint256(validUntil) << 160; + } + + // ============ onInstall Tests ============ + + modifier whenCallingOnInstall() { + _; + } + + function test_GivenDelayAndExpirationAreValid() external whenCallingOnInstall { + // it should store the config + // it should emit TimelockConfigUpdated + address newWallet = address(0x5555); + bytes32 newId = bytes32(uint256(2)); + + vm.expectEmit(true, true, true, true); + emit TimelockPolicy.TimelockConfigUpdated(newWallet, newId, 2 hours, 2 days); + + bytes memory installData = abi.encode(newId, uint48(2 hours), uint48(2 days)); + vm.prank(newWallet); + timelockPolicy.onInstall(installData); + + (uint48 delay, uint48 expiration, bool initialized) = timelockPolicy.timelockConfig(newId, newWallet); + assertEq(delay, 2 hours, "Delay should be stored"); + assertEq(expiration, 2 days, "Expiration should be stored"); + assertTrue(initialized, "Should be initialized"); + } + + function test_GivenAlreadyInitialized() external whenCallingOnInstall { + // it should revert with AlreadyInitialized + bytes memory installData = abi.encode(POLICY_ID, DELAY, EXPIRATION); + vm.prank(WALLET); + vm.expectRevert(abi.encodeWithSelector(IModule.AlreadyInitialized.selector, WALLET)); + timelockPolicy.onInstall(installData); + } + + function test_GivenDelayIsZero() external whenCallingOnInstall { + // it should revert with InvalidDelay + address newWallet = address(0x6666); + bytes memory installData = abi.encode(POLICY_ID, uint48(0), EXPIRATION); + vm.prank(newWallet); + vm.expectRevert(TimelockPolicy.InvalidDelay.selector); + timelockPolicy.onInstall(installData); + } + + function test_GivenExpirationIsZero() external whenCallingOnInstall { + // it should revert with InvalidExpirationPeriod + address newWallet = address(0x7777); + bytes memory installData = abi.encode(POLICY_ID, DELAY, uint48(0)); + vm.prank(newWallet); + vm.expectRevert(TimelockPolicy.InvalidExpirationPeriod.selector); + timelockPolicy.onInstall(installData); + } + + // ============ onUninstall Tests ============ + + modifier whenCallingOnUninstall() { + _; + } + + function test_GivenInitialized() external whenCallingOnUninstall { + // it should clear the config + vm.prank(WALLET); + timelockPolicy.onUninstall(abi.encode(POLICY_ID)); + + (,, bool initialized) = timelockPolicy.timelockConfig(POLICY_ID, WALLET); + assertFalse(initialized, "Config should be cleared"); + } + + function test_GivenNotInitialized() external whenCallingOnUninstall { + // it should revert with NotInitialized + address newWallet = address(0x8888); + vm.prank(newWallet); + vm.expectRevert(abi.encodeWithSelector(IModule.NotInitialized.selector, newWallet)); + timelockPolicy.onUninstall(abi.encode(POLICY_ID)); + } + + // ============ isModuleType Tests ============ + + modifier whenCallingIsModuleType() { + _; + } + + function test_GivenTypeIsPolicy() external whenCallingIsModuleType { + // it should return true + assertTrue(timelockPolicy.isModuleType(MODULE_TYPE_POLICY), "Should support policy type"); + } + + function test_GivenTypeIsStatelessValidator() external whenCallingIsModuleType { + // it should return true + assertTrue(timelockPolicy.isModuleType(MODULE_TYPE_STATELESS_VALIDATOR), "Should support stateless validator"); + } + + function test_GivenTypeIsStatelessValidatorWithSender() external whenCallingIsModuleType { + // it should return true + assertTrue( + timelockPolicy.isModuleType(MODULE_TYPE_STATELESS_VALIDATOR_WITH_SENDER), + "Should support stateless validator with sender" + ); + } + + function test_GivenTypeIsInvalid() external whenCallingIsModuleType { + // it should return false + 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 + EXPIRATION + ); + + timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); + + (TimelockPolicy.ProposalStatus status, uint256 validAfter, 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(validUntil, createTime + DELAY + EXPIRATION, "validUntil should be validAfter + 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() { + _; + } + + function test_GivenCallerIsAccountAndProposalIsPending() external whenCallingCancelProposal { + // it should set status to cancelled + // it should emit ProposalCancelled + bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); + uint256 nonce = 300; + + timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); + + bytes32 expectedKey = keccak256(abi.encode(WALLET, keccak256(callData), nonce)); + + vm.expectEmit(true, true, true, true); + emit TimelockPolicy.ProposalCancelled(WALLET, POLICY_ID, expectedKey); + + vm.prank(WALLET); + timelockPolicy.cancelProposal(POLICY_ID, WALLET, callData, nonce); + + (TimelockPolicy.ProposalStatus status,,) = + timelockPolicy.getProposal(WALLET, callData, nonce, POLICY_ID, WALLET); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Cancelled), "Status should be Cancelled"); + } + + function test_GivenCallerIsNotAccount() external whenCallingCancelProposal { + // it should revert with OnlyAccount + bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); + uint256 nonce = 400; + + timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); + + vm.prank(ATTACKER); + vm.expectRevert(TimelockPolicy.OnlyAccount.selector); + timelockPolicy.cancelProposal(POLICY_ID, WALLET, callData, nonce); + } + + function test_GivenNotInitialized_WhenCallingCancelProposal() external whenCallingCancelProposal { + // it should revert with NotInitialized + address uninitWallet = address(0xaaaa); + bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); + + vm.prank(uninitWallet); + vm.expectRevert(abi.encodeWithSelector(IModule.NotInitialized.selector, uninitWallet)); + timelockPolicy.cancelProposal(POLICY_ID, uninitWallet, callData, 0); + } + + function test_GivenProposalDoesNotExist() external whenCallingCancelProposal { + // it should revert with ProposalNotPending + bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); + uint256 nonce = 500; + + vm.prank(WALLET); + vm.expectRevert(TimelockPolicy.ProposalNotPending.selector); + timelockPolicy.cancelProposal(POLICY_ID, WALLET, callData, nonce); + } + + function test_GivenProposalIsAlreadyCancelled() external whenCallingCancelProposal { + // it should revert with ProposalNotPending + bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); + uint256 nonce = 600; + + timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); + + vm.prank(WALLET); + timelockPolicy.cancelProposal(POLICY_ID, WALLET, callData, nonce); + + vm.prank(WALLET); + vm.expectRevert(TimelockPolicy.ProposalNotPending.selector); + timelockPolicy.cancelProposal(POLICY_ID, WALLET, callData, nonce); + } + + // ============ checkUserOpPolicy - Proposal Creation Tests ============ + + modifier whenCallingCheckUserOpPolicyToCreateProposal() { + _; + } + + function test_GivenNoopCalldataAndValidSignature() external whenCallingCheckUserOpPolicyToCreateProposal { + // it should create the proposal + // it should return zero for state persistence + // it should emit ProposalCreated + bytes memory proposalCallData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); + uint256 proposalNonce = 700; + bytes memory sig = _createProposalSignature(proposalCallData, proposalNonce); + + PackedUserOperation memory userOp = _createNoopUserOp(WALLET, sig); + + bytes32 expectedKey = keccak256(abi.encode(WALLET, keccak256(proposalCallData), proposalNonce)); + + vm.expectEmit(true, true, true, true); + emit TimelockPolicy.ProposalCreated( + WALLET, + POLICY_ID, + expectedKey, + uint48(block.timestamp) + DELAY, + uint48(block.timestamp) + DELAY + EXPIRATION + ); + + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + // Proposal creation must return 0 for state persistence + assertEq(result, 0, "Should return 0 for state persistence"); + + (TimelockPolicy.ProposalStatus status,,) = + timelockPolicy.getProposal(WALLET, proposalCallData, proposalNonce, POLICY_ID, WALLET); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending), "Proposal should be created"); + } + + function test_GivenNoopCalldataAndSignatureShorterThan65Bytes() + external + whenCallingCheckUserOpPolicyToCreateProposal + { + // it should return SIG_VALIDATION_FAILED + bytes memory shortSig = new bytes(64); + PackedUserOperation memory userOp = _createNoopUserOp(WALLET, shortSig); + + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + assertEq(result, SIG_VALIDATION_FAILED, "Should fail with short signature"); + } + + function test_GivenNoopCalldataAndSignatureClaimsMoreDataThanAvailable() + external + whenCallingCheckUserOpPolicyToCreateProposal + { + // it should return SIG_VALIDATION_FAILED + bytes memory badSig = abi.encodePacked( + bytes32(uint256(1000)), // claims 1000 bytes of calldata + bytes32(0), // nonce + bytes1(0x00) // only 65 bytes total, not enough for claimed calldata + ); + + PackedUserOperation memory userOp = _createNoopUserOp(WALLET, badSig); + + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + 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 + 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 + vm.prank(WALLET); + timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + // Second creation should fail + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + assertEq(result, SIG_VALIDATION_FAILED, "Should fail for duplicate proposal"); + } + + // ============ checkUserOpPolicy - Proposal Execution Tests ============ + + modifier whenCallingCheckUserOpPolicyToExecuteProposal() { + _; + } + + function test_GivenProposalIsPendingAndTimelockPassed() external whenCallingCheckUserOpPolicyToExecuteProposal { + // it should mark proposal as executed + // it should return packed validation data with timing + // it should emit ProposalExecuted + 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); + + // Get the actual stored proposal values + (, uint256 storedValidAfter, uint256 storedValidUntil) = + timelockPolicy.getProposal(WALLET, proposalCallData, proposalNonce, POLICY_ID, WALLET); + + vm.warp(block.timestamp + DELAY + 1); + + PackedUserOperation memory executeOp = _createUserOpWithCalldata(WALLET, proposalCallData, proposalNonce, ""); + + bytes32 expectedKey = keccak256(abi.encode(WALLET, keccak256(proposalCallData), proposalNonce)); + + vm.expectEmit(true, true, true, true); + emit TimelockPolicy.ProposalExecuted(WALLET, POLICY_ID, expectedKey); + + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, executeOp); + + // Extract validAfter and validUntil from packed data + uint48 validAfter = uint48(result >> 208); + uint48 validUntil = uint48(result >> 160); + assertEq(validAfter, storedValidAfter, "validAfter should match proposal"); + assertEq(validUntil, storedValidUntil, "validUntil should match proposal"); + + (TimelockPolicy.ProposalStatus status,,) = + timelockPolicy.getProposal(WALLET, proposalCallData, proposalNonce, POLICY_ID, WALLET); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Executed), "Proposal should be executed"); + } + + function test_GivenNoProposalExists() external whenCallingCheckUserOpPolicyToExecuteProposal { + // it should return SIG_VALIDATION_FAILED + bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "attack"); + PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, 0, ""); + + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + assertEq(result, SIG_VALIDATION_FAILED, "Should fail without proposal"); + } + + function test_GivenProposalIsCancelled() external whenCallingCheckUserOpPolicyToExecuteProposal { + // it should return SIG_VALIDATION_FAILED + bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); + uint256 nonce = 1000; + + timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); + + vm.prank(WALLET); + timelockPolicy.cancelProposal(POLICY_ID, WALLET, callData, nonce); + + PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, nonce, ""); + + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + assertEq(result, SIG_VALIDATION_FAILED, "Should fail for cancelled proposal"); + } + + function test_GivenProposalIsAlreadyExecuted() external whenCallingCheckUserOpPolicyToExecuteProposal { + // it should return SIG_VALIDATION_FAILED + bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); + uint256 nonce = 1100; + + timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); + + vm.warp(block.timestamp + DELAY + 1); + + PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, nonce, ""); + + // First execution + vm.prank(WALLET); + timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + // Second execution attempt + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + assertEq(result, SIG_VALIDATION_FAILED, "Should fail for already executed proposal"); + } + + // ============ checkUserOpPolicy - Not Initialized ============ + + function test_WhenCallingCheckUserOpPolicyWithoutInitialization() external { + // it should return SIG_VALIDATION_FAILED + address uninitWallet = address(0xbbbb); + bytes memory sig = _createProposalSignature("test", 0); + PackedUserOperation memory userOp = _createNoopUserOp(uninitWallet, sig); + + vm.prank(uninitWallet); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + assertEq(result, SIG_VALIDATION_FAILED, "Should fail when not initialized"); + } + + // ============ checkSignaturePolicy Tests ============ + + modifier whenCallingCheckSignaturePolicy() { + _; + } + + function test_GivenInitialized_WhenCallingCheckSignaturePolicy() external whenCallingCheckSignaturePolicy { + // it should revert (TOB-KERNEL-20: signature validation not supported) + vm.prank(WALLET); + vm.expectRevert("TimelockPolicy: signature validation not supported"); + timelockPolicy.checkSignaturePolicy(POLICY_ID, address(0), bytes32(0), ""); + } + + function test_GivenNotInitialized_WhenCallingCheckSignaturePolicy() external whenCallingCheckSignaturePolicy { + // it should revert (TOB-KERNEL-20: signature validation not supported) + address uninitWallet = address(0xcccc); + vm.prank(uninitWallet); + vm.expectRevert("TimelockPolicy: signature validation not supported"); + timelockPolicy.checkSignaturePolicy(POLICY_ID, address(0), bytes32(0), ""); + } + + // ============ validateSignatureWithData Tests ============ + + modifier whenCallingValidateSignatureWithData() { + _; + } + + function test_GivenDelayAndExpirationAreNonzero() external whenCallingValidateSignatureWithData { + // it should revert (TOB-KERNEL-20: stateless signature validation not supported) + bytes memory data = abi.encode(uint48(1 hours), uint48(1 days)); + vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + timelockPolicy.validateSignatureWithData(bytes32(0), "", data); + } + + function test_GivenDelayIsZero_WhenCallingValidateSignatureWithData() + external + whenCallingValidateSignatureWithData + { + // it should revert (TOB-KERNEL-20: stateless signature validation not supported) + bytes memory data = abi.encode(uint48(0), uint48(1 days)); + vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + timelockPolicy.validateSignatureWithData(bytes32(0), "", data); + } + + function test_GivenExpirationIsZero_WhenCallingValidateSignatureWithData() + external + whenCallingValidateSignatureWithData + { + // it should revert (TOB-KERNEL-20: stateless signature validation not supported) + bytes memory data = abi.encode(uint48(1 hours), uint48(0)); + vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + timelockPolicy.validateSignatureWithData(bytes32(0), "", data); + } + + // ============ validateSignatureWithDataWithSender Tests ============ + + modifier whenCallingValidateSignatureWithDataWithSender() { + _; + } + + function test_GivenDelayAndExpirationAreNonzero_WhenCallingValidateSignatureWithDataWithSender() + external + whenCallingValidateSignatureWithDataWithSender + { + // it should revert (TOB-KERNEL-20: stateless signature validation not supported) + bytes memory data = abi.encode(uint48(1 hours), uint48(1 days)); + vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + timelockPolicy.validateSignatureWithDataWithSender(address(0), bytes32(0), "", data); + } + + function test_GivenDelayIsZero_WhenCallingValidateSignatureWithDataWithSender() + external + whenCallingValidateSignatureWithDataWithSender + { + // it should revert (TOB-KERNEL-20: stateless signature validation not supported) + bytes memory data = abi.encode(uint48(0), uint48(1 days)); + vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + timelockPolicy.validateSignatureWithDataWithSender(address(0), bytes32(0), "", data); + } + + function test_GivenExpirationIsZero_WhenCallingValidateSignatureWithDataWithSender() + external + whenCallingValidateSignatureWithDataWithSender + { + // it should revert (TOB-KERNEL-20: stateless signature validation not supported) + bytes memory data = abi.encode(uint48(1 hours), uint48(0)); + vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + timelockPolicy.validateSignatureWithDataWithSender(address(0), bytes32(0), "", data); + } + + // ============ getProposal Tests ============ + + modifier whenCallingGetProposal() { + _; + } + + function test_GivenProposalExists() external whenCallingGetProposal { + // it should return status validAfter and validUntil + 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); + + (TimelockPolicy.ProposalStatus status, uint256 validAfter, 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(validUntil, createTime + DELAY + EXPIRATION, "validUntil should be correct"); + } + + function test_GivenProposalDoesNotExist_WhenCallingGetProposal() external whenCallingGetProposal { + // it should return None status and zeros + bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); + + (TimelockPolicy.ProposalStatus status, uint256 validAfter, uint256 validUntil) = + timelockPolicy.getProposal(WALLET, callData, 9999, POLICY_ID, WALLET); + + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.None), "Status should be None"); + assertEq(validAfter, 0, "validAfter should be 0"); + assertEq(validUntil, 0, "validUntil should be 0"); + } + + // ============ computeUserOpKey Tests ============ + + function test_WhenCallingComputeUserOpKey() external { + // it should match manual keccak256 computation + bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); + uint256 nonce = 1300; + + bytes32 expected = keccak256(abi.encode(WALLET, keccak256(callData), nonce)); + bytes32 result = timelockPolicy.computeUserOpKey(WALLET, callData, nonce); + + assertEq(result, expected, "computeUserOpKey should match manual computation"); + } + + // ============ _isNoOpCalldata Tests ============ + + modifier whenDetectingNoopCalldata() { + _; + } + + function test_GivenCalldataIsEmpty() external whenDetectingNoopCalldata { + // it should be detected as noop + bytes memory sig = _createProposalSignature("test", 0); + PackedUserOperation memory userOp = _createNoopUserOp(WALLET, sig); + + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + // If it's a no-op, it goes to creation path and returns 0 + 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 { + // 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); + + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + assertEq(result, 0, "ERC7579 execute to self with zero value should be noop"); + } + + function test_GivenTargetIsZeroAddressAndValueIsZeroAndInnerCalldataIsEmpty() + external + whenDetectingERC7579ExecuteNoop + { + // 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, ""); + + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + assertEq(result, SIG_VALIDATION_FAILED, "Too short for length should not be 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, ""); + + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + assertEq(result, SIG_VALIDATION_FAILED, "Wrong target should not be noop"); + } + + function test_GivenValueIsNonzero() external whenDetectingERC7579ExecuteNoop { + // 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, ""); + + 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"); + } + + // ============ _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); + + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + assertEq(result, 0, "executeUserOp with empty userOp should 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, ""); + + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + assertEq(result, SIG_VALIDATION_FAILED, "Too short executeUserOp 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, ""); + + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + assertEq(result, SIG_VALIDATION_FAILED, "Wrong offset in executeUserOp should not be noop"); + } + + function test_GivenUserOpLengthIsNonzero() external whenDetectingExecuteUserOpNoop { + // 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, ""); + + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + assertEq(result, SIG_VALIDATION_FAILED, "Non-empty userOp should not be noop"); + } + + // ============ _packValidationData Tests ============ + + function test_WhenPackingValidationData() external { + // it should correctly pack validAfter and validUntil + // Test the packing by creating a proposal and checking the returned validation data + bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); + uint256 nonce = 1400; + + timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); + + // Get the actual stored proposal values + (, uint256 storedValidAfter, uint256 storedValidUntil) = + timelockPolicy.getProposal(WALLET, callData, nonce, POLICY_ID, WALLET); + + vm.warp(block.timestamp + DELAY + 1); + + PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, nonce, ""); + + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + uint256 expectedPacked = _packValidationData(uint48(storedValidAfter), uint48(storedValidUntil)); + + assertEq(result, expectedPacked, "Packed validation data should match expected"); + } + + // ============ Security Tests ============ + + modifier whenTestingSecurityScenarios() { + _; + } + + function test_GivenAttackerTriesToExecuteWithoutProposal() external whenTestingSecurityScenarios { + // it should return SIG_VALIDATION_FAILED + bytes memory maliciousCalldata = + abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "steal_funds"); + + PackedUserOperation memory attackOp = _createUserOpWithCalldata(WALLET, maliciousCalldata, 0, ""); + + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, attackOp); + + 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 - it 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, "validAfter should be createTime + DELAY"); + } + + 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); + + vm.warp(block.timestamp + DELAY + 1); + + PackedUserOperation memory executeOp = _createUserOpWithCalldata(WALLET, callData, nonce, ""); + + // First execution - should succeed + vm.prank(WALLET); + uint256 firstResult = timelockPolicy.checkUserOpPolicy(POLICY_ID, executeOp); + assertNotEq(firstResult, SIG_VALIDATION_FAILED, "First execution should succeed"); + + // Verify proposal is marked as executed + (TimelockPolicy.ProposalStatus status,,) = + timelockPolicy.getProposal(WALLET, callData, nonce, POLICY_ID, WALLET); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Executed), "Should be executed"); + + // Second execution attempt - should fail + vm.prank(WALLET); + uint256 secondResult = timelockPolicy.checkUserOpPolicy(POLICY_ID, executeOp); + assertEq(secondResult, SIG_VALIDATION_FAILED, "Re-execution should fail"); + } + + function test_GivenProposalCreationReturnsZero() external whenTestingSecurityScenarios { + // it should allow state to persist via EntryPoint + bytes memory proposalCallData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); + uint256 proposalNonce = 1700; + bytes memory sig = _createProposalSignature(proposalCallData, proposalNonce); + + PackedUserOperation memory userOp = _createNoopUserOp(WALLET, sig); + + vm.prank(WALLET); + uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp); + + // Result must be 0 for EntryPoint to not revert + assertEq(result, 0, "Proposal creation must return 0 for state persistence"); + + // Verify the proposal was actually created and persisted + (TimelockPolicy.ProposalStatus status, uint256 validAfter, uint256 validUntil) = + timelockPolicy.getProposal(WALLET, proposalCallData, proposalNonce, POLICY_ID, WALLET); + + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Pending), "Proposal state should persist"); + assertGt(validAfter, 0, "validAfter should be set"); + assertGt(validUntil, validAfter, "validUntil should be after validAfter"); + } + + function test_GivenAttackerTriesToCancelAnotherAccountsProposal() external whenTestingSecurityScenarios { + // it should revert with OnlyAccount + bytes memory callData = abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), "test"); + uint256 nonce = 1800; + + timelockPolicy.createProposal(POLICY_ID, WALLET, callData, nonce); + + vm.prank(ATTACKER); + vm.expectRevert(TimelockPolicy.OnlyAccount.selector); + timelockPolicy.cancelProposal(POLICY_ID, WALLET, callData, nonce); + + // 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"); + } +} diff --git a/test/btt/Timelock.tree b/test/btt/Timelock.tree new file mode 100644 index 0000000..6d6f7a9 --- /dev/null +++ b/test/btt/Timelock.tree @@ -0,0 +1,146 @@ +TimelockTest +├── when calling onInstall +│ ├── given delay and expiration are valid +│ │ ├── it should store the config +│ │ └── it should emit TimelockConfigUpdated +│ ├── given already initialized +│ │ └── it should revert with AlreadyInitialized +│ ├── given delay is zero +│ │ └── it should revert with InvalidDelay +│ └── given expiration is zero +│ └── it should revert with InvalidExpirationPeriod +├── when calling onUninstall +│ ├── given initialized +│ │ └── it should clear the config +│ └── given not initialized +│ └── it should revert with NotInitialized +├── when calling isModuleType +│ ├── given type is policy +│ │ └── it should return true +│ ├── given type is stateless validator +│ │ └── it should return true +│ ├── given type is stateless validator with sender +│ │ └── it should return true +│ └── given type is invalid +│ └── it should return false +├── when calling createProposal +│ ├── given config is initialized and proposal does not exist +│ │ ├── it should store the proposal with pending status +│ │ ├── it should set correct validAfter and validUntil +│ │ └── it should emit ProposalCreated +│ ├── given not initialized +│ │ └── it should revert with NotInitialized +│ └── given proposal already exists +│ └── it should revert with ProposalAlreadyExists +├── when calling cancelProposal +│ ├── given caller is account and proposal is pending +│ │ ├── it should set status to cancelled +│ │ └── it should emit ProposalCancelled +│ ├── given caller is not account +│ │ └── it should revert with OnlyAccount +│ ├── given not initialized +│ │ └── it should revert with NotInitialized +│ ├── given proposal does not exist +│ │ └── it should revert with ProposalNotPending +│ └── given proposal is already cancelled +│ └── it should revert with ProposalNotPending +├── when calling checkUserOpPolicy to create proposal +│ ├── given noop calldata and valid signature +│ │ ├── it should create the proposal +│ │ ├── it should return zero for state persistence +│ │ └── it should emit ProposalCreated +│ ├── given noop calldata and signature shorter than 65 bytes +│ │ └── it should return SIG_VALIDATION_FAILED +│ ├── given noop calldata and signature claims more data than available +│ │ └── it should return SIG_VALIDATION_FAILED +│ └── given noop calldata and proposal already exists +│ └── it should return SIG_VALIDATION_FAILED +├── when calling checkUserOpPolicy to execute proposal +│ ├── given proposal is pending and timelock passed +│ │ ├── it should mark proposal as executed +│ │ ├── it should return packed validation data with timing +│ │ └── it should emit ProposalExecuted +│ ├── given no proposal exists +│ │ └── it should return SIG_VALIDATION_FAILED +│ ├── given proposal is cancelled +│ │ └── it should return SIG_VALIDATION_FAILED +│ └── given proposal is already executed +│ └── it should return SIG_VALIDATION_FAILED +├── when calling checkUserOpPolicy without initialization +│ └── it should return SIG_VALIDATION_FAILED +├── when calling checkSignaturePolicy +│ ├── given initialized +│ │ └── it should return zero +│ └── given not initialized +│ └── it should return one +├── when calling validateSignatureWithData +│ ├── given delay and expiration are nonzero +│ │ └── it should return true +│ ├── given delay is zero +│ │ └── it should return false +│ └── given expiration is zero +│ └── it should return false +├── when calling validateSignatureWithDataWithSender +│ ├── given delay and expiration are nonzero +│ │ └── it should return true +│ ├── given delay is zero +│ │ └── it should return false +│ └── given expiration is zero +│ └── it should return false +├── when calling getProposal +│ ├── given proposal exists +│ │ └── it should return status validAfter and validUntil +│ └── given proposal does not exist +│ └── it should return None status and zeros +├── when calling computeUserOpKey +│ └── it should match manual keccak256 computation +├── when detecting noop calldata +│ ├── given calldata is empty +│ │ └── it should be detected as noop +│ ├── given calldata is shorter than 4 bytes +│ │ └── it should not be detected as noop +│ └── given selector is unrecognized +│ └── it should not be detected as noop +├── when detecting ERC7579 execute noop +│ ├── given target is self and value is zero and inner calldata is empty +│ │ └── it should be detected as noop +│ ├── given target is zero address and value is zero and inner calldata is empty +│ │ └── it should be detected as noop +│ ├── given calldata is shorter than 68 bytes +│ │ └── it should not be detected as noop +│ ├── given offset is not 32 +│ │ └── it should not be detected as noop +│ ├── given calldata is shorter than 100 bytes +│ │ └── it should not be detected as noop +│ ├── given exec data length is less than 52 +│ │ └── it should not be detected as noop +│ ├── given target is not self or zero +│ │ └── it should not be detected as noop +│ ├── given value is nonzero +│ │ └── it should not be detected as noop +│ ├── given calldata is shorter than 184 bytes +│ │ └── it should not be detected as noop +│ └── given inner calldata length is nonzero +│ └── it should not be detected as noop +├── when detecting executeUserOp noop +│ ├── given userOp data is empty +│ │ └── it should be detected as noop +│ ├── given calldata is shorter than 100 bytes +│ │ └── it should not be detected as noop +│ ├── given offset is not 32 +│ │ └── it should not be detected as noop +│ └── given userOp length is nonzero +│ └── it should not be detected as noop +├── when packing validation data +│ └── it should correctly pack validAfter and validUntil +└── when testing security scenarios + ├── given attacker tries to execute without proposal + │ └── it should return SIG_VALIDATION_FAILED + ├── given attacker tries to execute immediately after creation + │ └── it should return packed data with future validAfter + ├── given attacker tries to reexecute a used proposal + │ └── it should return SIG_VALIDATION_FAILED + ├── given proposal creation returns zero + │ └── it should allow state to persist via EntryPoint + └── given attacker tries to cancel another accounts proposal + └── it should revert with OnlyAccount diff --git a/test/btt/TimelockEpochValidation.t.sol b/test/btt/TimelockEpochValidation.t.sol new file mode 100644 index 0000000..6abe651 --- /dev/null +++ b/test/btt/TimelockEpochValidation.t.sol @@ -0,0 +1,521 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.20; + +import {Test} from "forge-std/Test.sol"; +import {TimelockPolicy} from "src/policies/TimelockPolicy.sol"; +import {PackedUserOperation} from "account-abstraction/interfaces/PackedUserOperation.sol"; + +/** + * @title TimelockEpochValidationTest + * @notice BTT tests for the epoch-based validation fix in TimelockPolicy + * @dev Tests the fix for TOB-KERNEL-2: Stale proposals persisting across reinstalls + */ +contract TimelockEpochValidationTest is Test { + TimelockPolicy public timelockPolicy; + + address constant WALLET = address(0x1234); + address constant WALLET2 = address(0x5678); + address constant ATTACKER = address(0xBAD); + + uint48 constant DELAY = 1 days; + uint48 constant EXPIRATION_PERIOD = 1 days; + + bytes32 constant POLICY_ID_1 = keccak256("POLICY_ID_1"); + bytes32 constant POLICY_ID_2 = keccak256("POLICY_ID_2"); + + uint256 constant SIG_VALIDATION_FAILED_UINT = 1; + + function setUp() public { + timelockPolicy = new TimelockPolicy(); + } + + function _installData() internal pure returns (bytes memory) { + return abi.encode(DELAY, EXPIRATION_PERIOD); + } + + function _installPolicy(address wallet, bytes32 policyId) internal { + vm.prank(wallet); + timelockPolicy.onInstall(abi.encodePacked(policyId, _installData())); + } + + function _uninstallPolicy(address wallet, bytes32 policyId) internal { + vm.prank(wallet); + timelockPolicy.onUninstall(abi.encodePacked(policyId, "")); + } + + function _createProposal(address wallet, bytes32 policyId, bytes memory callData, uint256 nonce) internal { + vm.prank(wallet); + timelockPolicy.createProposal(policyId, wallet, callData, nonce); + } + + function _createUserOp(address sender, bytes memory callData, uint256 nonce) + internal + pure + returns (PackedUserOperation memory) + { + return PackedUserOperation({ + sender: sender, + nonce: nonce, + initCode: "", + callData: callData, + accountGasLimits: bytes32(abi.encodePacked(uint128(100000), uint128(200000))), + preVerificationGas: 0, + gasFees: bytes32(abi.encodePacked(uint128(1), uint128(1))), + paymasterAndData: "", + signature: "" + }); + } + + // ==================== Installing the Policy ==================== + + modifier whenInstallingThePolicy() { + _; + } + + function test_GivenItIsTheFirstInstallation() external whenInstallingThePolicy { + // Check epoch before installation + uint256 epochBefore = timelockPolicy.currentEpoch(POLICY_ID_1, WALLET); + assertEq(epochBefore, 0, "Epoch should be 0 before first install"); + + // Install the policy + _installPolicy(WALLET, POLICY_ID_1); + + // it should set currentEpoch to 1 + uint256 epochAfter = timelockPolicy.currentEpoch(POLICY_ID_1, WALLET); + assertEq(epochAfter, 1, "Epoch should be 1 after first install"); + + // it should initialize the policy config + (uint48 delay, uint48 expirationPeriod, bool initialized) = + timelockPolicy.timelockConfig(POLICY_ID_1, WALLET); + assertTrue(initialized, "Policy should be initialized"); + assertEq(delay, DELAY, "Delay should match"); + assertEq(expirationPeriod, EXPIRATION_PERIOD, "Expiration period should match"); + } + + function test_GivenThePolicyWasPreviouslyUninstalled() external whenInstallingThePolicy { + // First installation + _installPolicy(WALLET, POLICY_ID_1); + uint256 epochAfterFirstInstall = timelockPolicy.currentEpoch(POLICY_ID_1, WALLET); + assertEq(epochAfterFirstInstall, 1, "Epoch should be 1 after first install"); + + // Uninstall + _uninstallPolicy(WALLET, POLICY_ID_1); + + // Reinstall + _installPolicy(WALLET, POLICY_ID_1); + + // it should increment the epoch counter + // it should set currentEpoch to previous epoch plus 1 + uint256 epochAfterReinstall = timelockPolicy.currentEpoch(POLICY_ID_1, WALLET); + assertEq(epochAfterReinstall, 2, "Epoch should be 2 after reinstall"); + } + + function test_GivenMultipleReinstallCyclesOccur() external whenInstallingThePolicy { + // First installation + _installPolicy(WALLET, POLICY_ID_1); + assertEq(timelockPolicy.currentEpoch(POLICY_ID_1, WALLET), 1, "Epoch should be 1"); + + // First reinstall cycle + _uninstallPolicy(WALLET, POLICY_ID_1); + _installPolicy(WALLET, POLICY_ID_1); + + // it should increment epoch on each install + assertEq(timelockPolicy.currentEpoch(POLICY_ID_1, WALLET), 2, "Epoch should be 2"); + + // Second reinstall cycle + _uninstallPolicy(WALLET, POLICY_ID_1); + _installPolicy(WALLET, POLICY_ID_1); + assertEq(timelockPolicy.currentEpoch(POLICY_ID_1, WALLET), 3, "Epoch should be 3"); + + // Third reinstall cycle + _uninstallPolicy(WALLET, POLICY_ID_1); + _installPolicy(WALLET, POLICY_ID_1); + + // it should track epoch correctly after 3 reinstalls + assertEq(timelockPolicy.currentEpoch(POLICY_ID_1, WALLET), 4, "Epoch should be 4 after 3 reinstalls"); + } + + // ==================== Creating a Proposal ==================== + + modifier whenCreatingAProposal() { + _; + } + + function test_GivenThePolicyIsInstalled() external whenCreatingAProposal { + _installPolicy(WALLET, POLICY_ID_1); + + bytes memory callData = hex"1234"; + uint256 nonce = 1; + + _createProposal(WALLET, POLICY_ID_1, callData, nonce); + + // it should store the current epoch in the proposal + // it should use the epoch from currentEpoch mapping + bytes32 userOpKey = timelockPolicy.computeUserOpKey(WALLET, callData, nonce); + ( + TimelockPolicy.ProposalStatus status, + uint48 validAfter, + uint48 validUntil, + uint256 proposalEpoch + ) = timelockPolicy.proposals(userOpKey, POLICY_ID_1, WALLET); + + 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(validUntil, block.timestamp + DELAY + EXPIRATION_PERIOD, "validUntil should be correct"); + } + + function test_GivenCreatingViaCreateProposalFunction() external whenCreatingAProposal { + _installPolicy(WALLET, POLICY_ID_1); + + bytes memory callData = hex"5678"; + uint256 nonce = 42; + + uint256 currentEpoch = timelockPolicy.currentEpoch(POLICY_ID_1, WALLET); + + _createProposal(WALLET, POLICY_ID_1, callData, nonce); + + // it should record the epoch at creation time + bytes32 userOpKey = timelockPolicy.computeUserOpKey(WALLET, callData, nonce); + (,,, uint256 proposalEpoch) = timelockPolicy.proposals(userOpKey, POLICY_ID_1, WALLET); + + assertEq(proposalEpoch, currentEpoch, "Proposal epoch should equal current epoch at creation"); + } + + // ==================== Executing a Proposal ==================== + + modifier whenExecutingAProposal() { + _; + } + + modifier givenTheProposalWasCreatedInTheCurrentEpoch() { + _; + } + + function test_GivenTheTimelockHasPassed() + external + whenExecutingAProposal + givenTheProposalWasCreatedInTheCurrentEpoch + { + _installPolicy(WALLET, POLICY_ID_1); + + bytes memory callData = hex"1234"; + uint256 nonce = 1; + + _createProposal(WALLET, POLICY_ID_1, callData, nonce); + + // Warp past the timelock delay + vm.warp(block.timestamp + DELAY + 1); + + PackedUserOperation memory userOp = _createUserOp(WALLET, callData, nonce); + + vm.prank(WALLET); + uint256 validationResult = timelockPolicy.checkUserOpPolicy(POLICY_ID_1, userOp); + + // it should return success validation data (not SIG_VALIDATION_FAILED_UINT) + assertTrue(validationResult != SIG_VALIDATION_FAILED_UINT, "Validation should succeed"); + + // it should mark proposal as executed + bytes32 userOpKey = timelockPolicy.computeUserOpKey(WALLET, callData, nonce); + (TimelockPolicy.ProposalStatus status,,,) = timelockPolicy.proposals(userOpKey, POLICY_ID_1, WALLET); + assertEq(uint256(status), uint256(TimelockPolicy.ProposalStatus.Executed), "Proposal should be executed"); + } + + function test_GivenTheProposalWasCreatedBeforeUninstallAndReinstall() external whenExecutingAProposal { + _installPolicy(WALLET, POLICY_ID_1); + + bytes memory callData = hex"1234"; + uint256 nonce = 1; + + // Create proposal in epoch 1 + _createProposal(WALLET, POLICY_ID_1, callData, nonce); + + // Warp past the timelock delay + vm.warp(block.timestamp + DELAY + 1); + + // Uninstall and reinstall (epoch becomes 2) + _uninstallPolicy(WALLET, POLICY_ID_1); + _installPolicy(WALLET, POLICY_ID_1); + + // Try to execute the stale proposal + PackedUserOperation memory userOp = _createUserOp(WALLET, callData, nonce); + + vm.prank(WALLET); + uint256 validationResult = timelockPolicy.checkUserOpPolicy(POLICY_ID_1, userOp); + + // it should return SIG_VALIDATION_FAILED + assertEq(validationResult, SIG_VALIDATION_FAILED_UINT, "Stale proposal should fail validation"); + + // it should not mark proposal as executed + 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"); + } + + function test_GivenTheProposalEpochDoesNotMatchCurrentEpoch() external whenExecutingAProposal { + _installPolicy(WALLET, POLICY_ID_1); + + bytes memory callData = hex"abcd"; + uint256 nonce = 5; + + // Create proposal in epoch 1 + _createProposal(WALLET, POLICY_ID_1, callData, nonce); + + bytes32 userOpKey = timelockPolicy.computeUserOpKey(WALLET, callData, nonce); + (,,, uint256 proposalEpoch) = timelockPolicy.proposals(userOpKey, POLICY_ID_1, WALLET); + assertEq(proposalEpoch, 1, "Proposal should be in epoch 1"); + + // Warp and do multiple reinstalls to get to epoch 3 + vm.warp(block.timestamp + DELAY + 1); + _uninstallPolicy(WALLET, POLICY_ID_1); + _installPolicy(WALLET, POLICY_ID_1); + _uninstallPolicy(WALLET, POLICY_ID_1); + _installPolicy(WALLET, POLICY_ID_1); + + assertEq(timelockPolicy.currentEpoch(POLICY_ID_1, WALLET), 3, "Current epoch should be 3"); + + // Try to execute + PackedUserOperation memory userOp = _createUserOp(WALLET, callData, nonce); + + vm.prank(WALLET); + uint256 validationResult = timelockPolicy.checkUserOpPolicy(POLICY_ID_1, userOp); + + // it should reject the stale proposal + assertEq(validationResult, SIG_VALIDATION_FAILED_UINT, "Should reject stale proposal"); + + // it should leave proposal status unchanged + (TimelockPolicy.ProposalStatus statusAfter,,,) = timelockPolicy.proposals(userOpKey, POLICY_ID_1, WALLET); + assertEq( + uint256(statusAfter), + uint256(TimelockPolicy.ProposalStatus.Pending), + "Proposal status should remain pending" + ); + } + + // ==================== Uninstalling and Reinstalling ==================== + + modifier whenUninstallingAndReinstallingThePolicy() { + _; + } + + modifier givenAProposalExistsBeforeUninstall() { + _; + } + + function test_WhenThePolicyIsUninstalled() + external + whenUninstallingAndReinstallingThePolicy + givenAProposalExistsBeforeUninstall + { + _installPolicy(WALLET, POLICY_ID_1); + + bytes memory callData = hex"1234"; + uint256 nonce = 1; + + _createProposal(WALLET, POLICY_ID_1, callData, nonce); + + uint256 epochBeforeUninstall = timelockPolicy.currentEpoch(POLICY_ID_1, WALLET); + + // Uninstall + _uninstallPolicy(WALLET, POLICY_ID_1); + + // it should delete the policy config + (,, bool initialized) = timelockPolicy.timelockConfig(POLICY_ID_1, WALLET); + assertFalse(initialized, "Policy config should be deleted"); + + // it should preserve the epoch counter + uint256 epochAfterUninstall = timelockPolicy.currentEpoch(POLICY_ID_1, WALLET); + assertEq(epochAfterUninstall, epochBeforeUninstall, "Epoch should be preserved after uninstall"); + } + + function test_WhenThePolicyIsReinstalled() + external + whenUninstallingAndReinstallingThePolicy + givenAProposalExistsBeforeUninstall + { + _installPolicy(WALLET, POLICY_ID_1); + + bytes memory callData = hex"1234"; + uint256 nonce = 1; + + _createProposal(WALLET, POLICY_ID_1, callData, nonce); + + uint256 epochBeforeUninstall = timelockPolicy.currentEpoch(POLICY_ID_1, WALLET); + + _uninstallPolicy(WALLET, POLICY_ID_1); + + // Reinstall + _installPolicy(WALLET, POLICY_ID_1); + + // it should increment the epoch + uint256 epochAfterReinstall = timelockPolicy.currentEpoch(POLICY_ID_1, WALLET); + assertEq(epochAfterReinstall, epochBeforeUninstall + 1, "Epoch should increment on reinstall"); + + // it should invalidate old proposals implicitly + vm.warp(block.timestamp + DELAY + 1); + PackedUserOperation memory userOp = _createUserOp(WALLET, callData, nonce); + + vm.prank(WALLET); + 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 + bytes memory newCallData = hex"5678"; + uint256 newNonce = 2; + + _createProposal(WALLET, POLICY_ID_1, newCallData, newNonce); + + bytes32 newUserOpKey = timelockPolicy.computeUserOpKey(WALLET, newCallData, newNonce); + (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(newProposalEpoch, epochAfterReinstall, "New proposal should have current epoch"); + } + + // ==================== Epoch Storage Per Policy ID ==================== + + modifier whenEpochIsStoredPerPolicyID() { + _; + } + + function test_GivenTwoDifferentPolicyIDsForSameWallet() external whenEpochIsStoredPerPolicyID { + // Install policy 1 + _installPolicy(WALLET, POLICY_ID_1); + assertEq(timelockPolicy.currentEpoch(POLICY_ID_1, WALLET), 1, "Policy 1 should be epoch 1"); + + // Install policy 2 + _installPolicy(WALLET, POLICY_ID_2); + assertEq(timelockPolicy.currentEpoch(POLICY_ID_2, WALLET), 1, "Policy 2 should be epoch 1"); + + // Reinstall policy 1 twice + _uninstallPolicy(WALLET, POLICY_ID_1); + _installPolicy(WALLET, POLICY_ID_1); + _uninstallPolicy(WALLET, POLICY_ID_1); + _installPolicy(WALLET, POLICY_ID_1); + + // it should track separate epochs for each policy ID + assertEq(timelockPolicy.currentEpoch(POLICY_ID_1, WALLET), 3, "Policy 1 should be epoch 3"); + + // it should not cross contaminate epochs + assertEq(timelockPolicy.currentEpoch(POLICY_ID_2, WALLET), 1, "Policy 2 should still be epoch 1"); + } + + function test_GivenSamePolicyIDForDifferentWallets() external whenEpochIsStoredPerPolicyID { + // Install for wallet 1 + _installPolicy(WALLET, POLICY_ID_1); + assertEq(timelockPolicy.currentEpoch(POLICY_ID_1, WALLET), 1, "Wallet 1 should be epoch 1"); + + // Install for wallet 2 + _installPolicy(WALLET2, POLICY_ID_1); + assertEq(timelockPolicy.currentEpoch(POLICY_ID_1, WALLET2), 1, "Wallet 2 should be epoch 1"); + + // Reinstall for wallet 1 multiple times + _uninstallPolicy(WALLET, POLICY_ID_1); + _installPolicy(WALLET, POLICY_ID_1); + _uninstallPolicy(WALLET, POLICY_ID_1); + _installPolicy(WALLET, POLICY_ID_1); + + // it should track separate epochs for each wallet + assertEq(timelockPolicy.currentEpoch(POLICY_ID_1, WALLET), 3, "Wallet 1 should be epoch 3"); + + // it should not affect other wallets epoch on reinstall + assertEq(timelockPolicy.currentEpoch(POLICY_ID_1, WALLET2), 1, "Wallet 2 should still be epoch 1"); + } + + // ==================== Full Attack Scenario ==================== + + modifier whenVerifyingFullAttackScenario() { + _; + } + + function test_WhenAttackerTriesToExecuteOldProposal() 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 + 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"); + } + + function test_WhenUserCreatesNewProposalAfterReinstall() external whenVerifyingFullAttackScenario { + // User installs policy + _installPolicy(WALLET, POLICY_ID_1); + + // User uninstalls and reinstalls + _uninstallPolicy(WALLET, POLICY_ID_1); + _installPolicy(WALLET, POLICY_ID_1); + + // User creates legitimate proposal with new epoch + bytes memory callData = hex"abcd"; + uint256 userNonce = 200; + _createProposal(WALLET, POLICY_ID_1, callData, userNonce); + + vm.warp(block.timestamp + DELAY + 1); + + PackedUserOperation memory userOp = _createUserOp(WALLET, callData, userNonce); + + vm.prank(WALLET); + uint256 validationResult = timelockPolicy.checkUserOpPolicy(POLICY_ID_1, userOp); + + // it should succeed with new epoch + assertTrue(validationResult != SIG_VALIDATION_FAILED_UINT, "User's new proposal should succeed"); + } + + function test_WhenOneUserReinstallsDoesNotAffectOther() external whenVerifyingFullAttackScenario { + // Both users install policy + _installPolicy(WALLET, POLICY_ID_1); + _installPolicy(WALLET2, POLICY_ID_1); + + // Both users create proposals + bytes memory callData1 = hex"1111"; + bytes memory callData2 = hex"2222"; + + _createProposal(WALLET, POLICY_ID_1, callData1, 1); + _createProposal(WALLET2, POLICY_ID_1, callData2, 1); + + vm.warp(block.timestamp + DELAY + 1); + + // WALLET reinstalls + _uninstallPolicy(WALLET, POLICY_ID_1); + _installPolicy(WALLET, POLICY_ID_1); + + // it should only affect that users epoch + 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 + PackedUserOperation memory userOp1 = _createUserOp(WALLET, callData1, 1); + vm.prank(WALLET); + uint256 result1 = timelockPolicy.checkUserOpPolicy(POLICY_ID_1, userOp1); + assertEq(result1, SIG_VALIDATION_FAILED_UINT, "WALLET's old proposal should fail"); + + // it should not affect other users proposals + PackedUserOperation memory userOp2 = _createUserOp(WALLET2, callData2, 1); + vm.prank(WALLET2); + uint256 result2 = timelockPolicy.checkUserOpPolicy(POLICY_ID_1, userOp2); + assertTrue(result2 != SIG_VALIDATION_FAILED_UINT, "WALLET2's proposal should still work"); + } +} diff --git a/test/btt/TimelockEpochValidation.tree b/test/btt/TimelockEpochValidation.tree new file mode 100644 index 0000000..db1132f --- /dev/null +++ b/test/btt/TimelockEpochValidation.tree @@ -0,0 +1,75 @@ +TimelockEpochValidationTest +├── when installing the policy +│ ├── given it is the first installation +│ │ ├── it should set currentEpoch to 1 +│ │ └── it should initialize the policy config +│ ├── given the policy was previously uninstalled +│ │ ├── it should increment the epoch counter +│ │ └── it should set currentEpoch to previous epoch plus 1 +│ └── given multiple reinstall cycles occur +│ ├── it should increment epoch on each install +│ └── it should track epoch correctly after 3 reinstalls +├── when creating a proposal +│ ├── given the policy is installed +│ │ ├── it should store the current epoch in the proposal +│ │ └── it should use the epoch from currentEpoch mapping +│ └── given creating via createProposal function +│ └── it should record the epoch at creation time +├── when executing a proposal +│ ├── given the proposal was created in the current epoch +│ │ ├── given the timelock has passed +│ │ │ ├── it should return success validation data +│ │ │ └── it should mark proposal as executed +│ │ └── given the proposal is pending +│ │ └── it should pass epoch validation +│ ├── given the proposal was created before uninstall and reinstall +│ │ ├── it should return SIG_VALIDATION_FAILED +│ │ └── it should not mark proposal as executed +│ └── given the proposal epoch does not match current epoch +│ ├── it should reject the stale proposal +│ └── it should leave proposal status unchanged +├── when uninstalling and reinstalling the policy +│ ├── given a proposal exists before uninstall +│ │ ├── when the policy is uninstalled +│ │ │ ├── it should delete the policy config +│ │ │ └── it should preserve the epoch counter +│ │ └── when the policy is reinstalled +│ │ ├── it should increment the epoch +│ │ ├── it should invalidate old proposals implicitly +│ │ └── it should allow new proposals with new epoch +│ ├── given multiple proposals exist before uninstall +│ │ └── when reinstalled and proposals executed +│ │ ├── it should reject all old proposals +│ │ └── it should accept new proposals created after reinstall +│ └── given an attacker tries to execute stale proposal +│ ├── when proposal was created in epoch 1 and current epoch is 2 +│ │ └── it should fail validation +│ └── when proposal was created in epoch 2 and current epoch is 5 +│ └── it should fail validation +├── when epoch is stored per policy ID +│ ├── given two different policy IDs for same wallet +│ │ ├── it should track separate epochs for each policy ID +│ │ └── it should not cross contaminate epochs +│ └── given same policy ID for different wallets +│ ├── it should track separate epochs for each wallet +│ └── it should not affect other wallets epoch on reinstall +├── when creating new proposals after reinstall +│ ├── given old proposal exists with old epoch +│ │ ├── it should allow creating new proposal with same parameters +│ │ └── it should store new proposal with current epoch +│ └── given the new proposal is executed +│ ├── it should succeed because epoch matches +│ └── it should mark new proposal as executed +└── when verifying full attack scenario + ├── given attacker creates proposal before user uninstalls + │ ├── when user uninstalls and reinstalls + │ │ ├── when attacker tries to execute old proposal + │ │ │ └── it should fail due to epoch mismatch + │ │ └── when user creates new proposal with same calldata + │ │ └── it should succeed with new epoch + │ └── when timelock passes but policy was reinstalled + │ └── it should still reject the stale proposal + └── given multiple users with same policy + └── when one user reinstalls + ├── it should only affect that users epoch + └── it should not affect other users proposals diff --git a/test/btt/TimelockSignaturePolicy.t.sol b/test/btt/TimelockSignaturePolicy.t.sol new file mode 100644 index 0000000..0df7676 --- /dev/null +++ b/test/btt/TimelockSignaturePolicy.t.sol @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.20; + +import {Test} from "forge-std/Test.sol"; +import {TimelockPolicy} from "src/policies/TimelockPolicy.sol"; + +/** + * @title TimelockSignaturePolicyTest + * @notice BTT tests for ERC-1271 signature validation with timelock + * @dev Tests that TimelockPolicy disables ERC-1271 signature validation (always reverts) + */ +contract TimelockSignaturePolicyTest is Test { + TimelockPolicy public timelockPolicy; + + address constant WALLET = address(0x1234); + + uint48 constant DELAY = 1 days; + uint48 constant EXPIRATION_PERIOD = 1 days; + + bytes32 public policyId; + bytes32 public testHash; + + function setUp() public { + timelockPolicy = new TimelockPolicy(); + policyId = keccak256(abi.encodePacked("POLICY_ID_1")); + testHash = keccak256(abi.encodePacked("TEST_HASH_TO_SIGN")); + } + + /// @notice Helper to install the policy for a wallet + function _installPolicy(address wallet) internal { + bytes memory installData = abi.encode(DELAY, EXPIRATION_PERIOD); + vm.prank(wallet); + timelockPolicy.onInstall(abi.encodePacked(policyId, installData)); + } + + // ============================================================ + // Test: checkSignaturePolicy always reverts + // ============================================================ + + function test_WhenCheckingSignaturePolicy() external { + // it should revert because signature validation is not supported + + // Install policy + _installPolicy(WALLET); + + // Try to validate a signature - should always revert + vm.prank(WALLET); + vm.expectRevert("TimelockPolicy: signature validation not supported"); + timelockPolicy.checkSignaturePolicy(policyId, address(0), testHash, ""); + } + + function test_WhenCheckingSignaturePolicyWithoutInstall() external { + // it should revert because signature validation is not supported + + // Do NOT install the policy + + // Try to validate a signature - should always revert + vm.prank(WALLET); + vm.expectRevert("TimelockPolicy: signature validation not supported"); + timelockPolicy.checkSignaturePolicy(policyId, address(0), testHash, ""); + } + + // ============================================================ + // Test: validateSignatureWithData always reverts + // ============================================================ + + function test_WhenValidatingSignatureWithData() external { + // it should revert because stateless signature validation is not supported + + bytes memory data = abi.encode(DELAY, EXPIRATION_PERIOD); + + vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + timelockPolicy.validateSignatureWithData(testHash, "", data); + } + + // ============================================================ + // Test: validateSignatureWithDataWithSender always reverts + // ============================================================ + + function test_WhenValidatingSignatureWithDataWithSender() external { + // it should revert because stateless signature validation is not supported + + bytes memory data = abi.encode(DELAY, EXPIRATION_PERIOD); + + vm.expectRevert("TimelockPolicy: stateless signature validation not supported"); + timelockPolicy.validateSignatureWithDataWithSender(WALLET, testHash, "", data); + } +} diff --git a/test/btt/TimelockSignaturePolicy.tree b/test/btt/TimelockSignaturePolicy.tree new file mode 100644 index 0000000..9a21572 --- /dev/null +++ b/test/btt/TimelockSignaturePolicy.tree @@ -0,0 +1,9 @@ +TimelockSignaturePolicyTest +├── when checking signature policy +│ └── it should revert because signature validation is not supported +├── when checking signature policy without install +│ └── it should revert because signature validation is not supported +├── when validating signature with data +│ └── it should revert because stateless signature validation is not supported +└── when validating signature with data with sender + └── it should revert because stateless signature validation is not supported