diff --git a/src/Permit3.sol b/src/Permit3.sol index 94edf8f..7a5064e 100644 --- a/src/Permit3.sol +++ b/src/Permit3.sol @@ -361,6 +361,11 @@ contract Permit3 is IPermit3, MultiTokenPermit, NonceManager { revert ZeroAccount(); } + // Prevent setting timestamps in the future for all timestamp-storing modes + if (block.timestamp < timestamp) { + revert InvalidTimestamp(timestamp, uint48(block.timestamp)); + } + Allowance memory allowed = allowances[owner][p.tokenKey][p.account]; // Validate lock status before processing @@ -465,7 +470,7 @@ contract Permit3 is IPermit3, MultiTokenPermit, NonceManager { Allowance memory allowed, AllowanceOrTransfer memory p, uint48 timestamp - ) private view { + ) private pure { // Handle amount increase if specified if (p.amountDelta > 0) { if (allowed.amount != MAX_ALLOWANCE) { @@ -477,11 +482,6 @@ contract Permit3 is IPermit3, MultiTokenPermit, NonceManager { } } - // Prevent setting timestamps in the future - if (block.timestamp < timestamp) { - revert InvalidTimestamp(timestamp, uint48(block.timestamp)); - } - // Update expiration and timestamp based on precedence rules if (timestamp > allowed.timestamp) { allowed.expiration = p.modeOrExpiration; diff --git a/test/Permit3Edge.t.sol b/test/Permit3Edge.t.sol index 1ef78e1..774d4eb 100644 --- a/test/Permit3Edge.t.sol +++ b/test/Permit3Edge.t.sol @@ -712,6 +712,59 @@ contract Permit3EdgeTest is Test { ); } + function test_lockTimestampInFuture() public { + // A Lock permit must not be allowed to store a future timestamp. Otherwise the + // lock would be effectively un-unlockable, since Unlock requires a timestamp + // strictly greater than the stored lock timestamp. + + // Create a permit to lock the allowance with a future timestamp + PermitInputs memory lockInputs; + lockInputs.permits = new IPermit3.AllowanceOrTransfer[](1); + lockInputs.permits[0] = IPermit3.AllowanceOrTransfer({ + modeOrExpiration: uint48(IPermit3.PermitType.Lock), // Lock mode (2) + tokenKey: bytes32(uint256(uint160(address(token)))), + account: spender, + amountDelta: 0 // Not used for lock + }); + + lockInputs.chainPermits = IPermit3.ChainPermits({ chainId: uint64(block.chainid), permits: lockInputs.permits }); + + TestParams memory lockParams; + lockParams.salt = bytes32(uint256(0x446)); + lockParams.deadline = uint48(block.timestamp + 1 hours); + lockParams.timestamp = uint48(block.timestamp + 1000); // Future timestamp + + // Sign the lock permit + bytes32 lockDataHash = permit3Tester.hashChainPermits(lockInputs.chainPermits); + bytes32 lockSignedHash = keccak256( + abi.encode( + permit3.SIGNED_PERMIT3_TYPEHASH(), + owner, + lockParams.salt, + lockParams.deadline, + lockParams.timestamp, + lockDataHash + ) + ); + + bytes32 lockDigest = _getDigest(lockSignedHash); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(ownerPrivateKey, lockDigest); + lockParams.signature = abi.encodePacked(r, s, v); + + // Should revert with InvalidTimestamp - a Lock cannot store a future timestamp + vm.expectRevert( + abi.encodeWithSelector(IPermit.InvalidTimestamp.selector, lockParams.timestamp, uint48(block.timestamp)) + ); + permit3.permit( + owner, + lockParams.salt, + lockParams.deadline, + lockParams.timestamp, + lockInputs.chainPermits.permits, + lockParams.signature + ); + } + function test_lockAndUnlockAllowance() public { // First set up an initial allowance vm.prank(owner); @@ -767,6 +820,12 @@ contract Permit3EdgeTest is Test { assertEq(expiration, 2); // LOCKED_ALLOWANCE is 2 assertEq(ts, lockParams.timestamp); + // Remember the lock timestamp before advancing the clock + uint48 lockTimestamp = lockParams.timestamp; + + // Advance the chain so the unlock can use a newer (but not future) timestamp + vm.warp(block.timestamp + 100); + // Now create a permit to unlock with a newer timestamp PermitInputs memory unlockInputs; unlockInputs.permits = new IPermit3.AllowanceOrTransfer[](1); @@ -783,7 +842,7 @@ contract Permit3EdgeTest is Test { TestParams memory unlockParams; unlockParams.salt = bytes32(uint256(0x777)); unlockParams.deadline = uint48(block.timestamp + 1 hours); - unlockParams.timestamp = uint48(block.timestamp + 100); // Newer timestamp + unlockParams.timestamp = uint48(block.timestamp); // Newer than lock timestamp, not in the future // Sign the unlock permit bytes32 unlockDataHash = permit3Tester.hashChainPermits(unlockInputs.chainPermits); @@ -817,7 +876,7 @@ contract Permit3EdgeTest is Test { assertEq(amount, 0); // Amount remains unchanged by unlock operation assertEq(expiration, 0); // No expiration (unlocked) // Note: timestamp should remain from lock operation since unlock only changes expiration - assertEq(ts, uint48(block.timestamp)); // Timestamp remains from lock operation + assertEq(ts, lockTimestamp); // Timestamp remains from lock operation } function test_attemptUnlockWithOlderTimestamp() public { @@ -935,7 +994,7 @@ contract Permit3EdgeTest is Test { TestParams memory params; params.salt = bytes32(uint256(0xaaa)); params.deadline = uint48(block.timestamp + 1 hours); - params.timestamp = uint48(block.timestamp + 100); + params.timestamp = uint48(block.timestamp); PermitInputs memory inputs; inputs.permits = new IPermit3.AllowanceOrTransfer[](1); @@ -979,7 +1038,7 @@ contract Permit3EdgeTest is Test { TestParams memory params; params.salt = bytes32(uint256(0xbbb)); params.deadline = uint48(block.timestamp + 1 hours); - params.timestamp = uint48(block.timestamp + 100); + params.timestamp = uint48(block.timestamp); PermitInputs memory inputs; inputs.permits = new IPermit3.AllowanceOrTransfer[](1); @@ -1070,7 +1129,7 @@ contract Permit3EdgeTest is Test { TestParams memory params; params.salt = bytes32(uint256(0xddd)); params.deadline = uint48(block.timestamp + 1 hours); - params.timestamp = uint48(block.timestamp + 100); + params.timestamp = uint48(block.timestamp); PermitInputs memory inputs; inputs.permits = new IPermit3.AllowanceOrTransfer[](1); @@ -1288,8 +1347,11 @@ contract Permit3EdgeTest is Test { vm.prank(owner); permit3.lockdown(pairs); - // Create unlock permit with newer timestamp - uint48 newerTimestamp = uint48(block.timestamp) + 1; + // Advance the clock so the unlock can use a newer (but not future) timestamp than the lock + vm.warp(block.timestamp + 1); + + // Create unlock permit with newer timestamp (now equal to the current block time) + uint48 newerTimestamp = uint48(block.timestamp); PermitInputs memory inputs; inputs.permits = new IPermit3.AllowanceOrTransfer[](1);