From af4a94bc4a6608b36f0d8a7f4b5e97825e9ffdb4 Mon Sep 17 00:00:00 2001 From: mgretzke Date: Thu, 15 Jan 2026 14:03:54 +0100 Subject: [PATCH 01/11] initial allocation logic redesign --- snapshots/ERC7683Allocator_open.json | 2 +- snapshots/ERC7683Allocator_openFor.json | 2 +- snapshots/OnChainAllocatorTest.json | 15 +- src/allocators/OnChainAllocator.sol | 288 +++++++++++++++++++++--- src/interfaces/IOnChainAllocator.sol | 3 + test/OnChainAllocator.t.sol | 1 + 6 files changed, 265 insertions(+), 46 deletions(-) diff --git a/snapshots/ERC7683Allocator_open.json b/snapshots/ERC7683Allocator_open.json index 484f0b9..220f720 100644 --- a/snapshots/ERC7683Allocator_open.json +++ b/snapshots/ERC7683Allocator_open.json @@ -1,3 +1,3 @@ { - "open_simpleOrder": "168841" + "open_simpleOrder": "169186" } \ No newline at end of file diff --git a/snapshots/ERC7683Allocator_openFor.json b/snapshots/ERC7683Allocator_openFor.json index 1bed8e9..db2161e 100644 --- a/snapshots/ERC7683Allocator_openFor.json +++ b/snapshots/ERC7683Allocator_openFor.json @@ -1,3 +1,3 @@ { - "openFor_simpleOrder_userHimself": "172266" + "openFor_simpleOrder_userHimself": "172581" } \ No newline at end of file diff --git a/snapshots/OnChainAllocatorTest.json b/snapshots/OnChainAllocatorTest.json index f5ae948..502a3e7 100644 --- a/snapshots/OnChainAllocatorTest.json +++ b/snapshots/OnChainAllocatorTest.json @@ -1,9 +1,10 @@ { - "allocateFor_success_withRegistration": "134197", - "allocate_and_delete_expired_allocation": "66376", - "allocate_erc20": "129647", - "allocate_native": "129407", - "allocate_second_erc20": "97659", - "onchain_execute_double": "346191", - "onchain_execute_single": "219927" + "allocateFor_success_withRegistration": "134514", + "allocate_and_delete_expired_allocation": "76330", + "allocate_erc20": "129965", + "allocate_native": "129725", + "allocate_second_erc20": "95213", + "authorizeClaim_success_single_allocation": "32597", + "onchain_execute_double": "325233", + "onchain_execute_single": "220720" } \ No newline at end of file diff --git a/src/allocators/OnChainAllocator.sol b/src/allocators/OnChainAllocator.sol index 45d5d62..6dcb7c0 100644 --- a/src/allocators/OnChainAllocator.sol +++ b/src/allocators/OnChainAllocator.sol @@ -32,6 +32,16 @@ contract OnChainAllocator is IOnChainAllocator, Utility { mapping(bytes32 tokenHash => Allocation[] allocations) internal _allocations; + struct BalanceExpiration { + uint32 nextExpiration; + uint224 amount; + } + + mapping(bytes32 tokenHash => uint32 nextExpiration) internal _nextExpirationPointer; + /// @notice Similar to mapping(bytes32 tokenHash => mapping(uint32 expiration => BalanceExpiration balances)). + mapping(bytes32 TokenHashWithExpiration => BalanceExpiration balances) internal _balancesByExpiration; + mapping(bytes32 claimHash => uint32 normalizedExpiration) internal _allocatedClaims; + /// @notice Mapping of user addresses to their current nonce for replay protection. /// @dev The actual nonce will be a combination of the next free nonce and the user address. mapping(address user => uint96 nonce) public nonces; @@ -228,6 +238,8 @@ contract OnChainAllocator is IOnChainAllocator, Utility { uint32 expires, bytes32 claimHash ) private returns (Lock[] memory) { + // External allocation requires to normalize the expiration time + expires = _normalizeExpiration(expires); // Store the allocation for (uint256 i = 0; i < registeredAmounts.length; i++) { // Update the allocations with the actual registered amounts @@ -235,9 +247,12 @@ contract OnChainAllocator is IOnChainAllocator, Utility { commitments[i].amount = amount; // Store the allocation - _storeAllocation(commitments[i].lockTag, commitments[i].token, amount, recipient, expires, claimHash); + _storeAllocatedBalance(commitments[i].lockTag, commitments[i].token, recipient, amount, expires); } + // Store the claim + _storeClaim(claimHash, expires); + return commitments; } @@ -296,6 +311,9 @@ contract OnChainAllocator is IOnChainAllocator, Utility { (bytes32 claimHash, Lock[] memory commitments) = AL.executeAllocation(nonce, recipient, idsAndAmounts, arbiter, expires, typehash, witness); + // External allocation requires to normalize the expiration time + expires = _normalizeExpiration(expires); + // Allocate the claim for (uint256 i = 0; i < commitments.length; i++) { // Check the amount fits in the supported range @@ -303,15 +321,13 @@ contract OnChainAllocator is IOnChainAllocator, Utility { revert InvalidAmount(commitments[i].amount); } - _storeAllocation( - commitments[i].lockTag, - commitments[i].token, - uint224(commitments[i].amount), - recipient, - expires, - claimHash + // Store the allocation + _storeAllocatedBalance( + commitments[i].lockTag, commitments[i].token, recipient, uint224(commitments[i].amount), expires ); } + // Store the claim + _storeClaim(claimHash, expires); return (claimHash, commitments); } @@ -325,7 +341,7 @@ contract OnChainAllocator is IOnChainAllocator, Utility { // Check unlocked balance bytes32 tokenHash = _getTokenHash(id_, from_); - uint256 allocatedBalance = _allocatedBalance(tokenHash); + (uint256 allocatedBalance,,) = _readAllocatedBalance(tokenHash, type(uint32).max, false); uint256 fullAmount = amount_ + allocatedBalance; if (balance < fullAmount) { @@ -343,18 +359,34 @@ contract OnChainAllocator is IOnChainAllocator, Utility { uint256, /*nonce*/ // A parameter to enforce replay protection, scoped to allocator. uint256, /*expires*/ // The time at which the claim expires. uint256[2][] calldata idsAndAmounts, // The allocated token IDs and amounts. - bytes calldata /*allocatorData*/ // Arbitrary data provided by the arbiter. + bytes calldata allocatorData // Arbitrary data provided by the arbiter. ) external virtual onlyCompact returns (bytes4) { + // TODO: Allow allocatorData to be used to submit the previous expiration pointer + (bool verified, uint32 normalizedExpiration) = _verifyClaim(claimHash); + if (!verified) { + revert InvalidClaim(claimHash); + } + + // Delete the claim + delete _allocatedClaims[claimHash]; + + // If allocatorData is provided, ensure the length matches the expectation + if (allocatorData.length != 0 && allocatorData.length != 32 * idsAndAmounts.length) { + revert InvalidHint(allocatorData.length, 32 * idsAndAmounts.length); + } + + // Delete the allocations for (uint256 i = 0; i < idsAndAmounts.length; i++) { bytes32 tokenHash = _getTokenHash(idsAndAmounts[i][0], sponsor); - if (_verifyClaim(tokenHash, claimHash)) { - // Continue even if the claim is already verified to delete the other allocations. - continue; + uint32 hint = 0; + if (allocatorData.length != 0) { + assembly ("memory-safe") { + hint := shr(224, calldataload(add(allocatorData.offset, mul(i, 4 /* uint32 */ )))) + } } - - // claim could not be verified - revert InvalidClaim(claimHash); + // The amount is at this point verified to be within the range of uint224.max + _deleteAllocatedBalance(tokenHash, normalizedExpiration, uint224(idsAndAmounts[i][1]), hint); } return this.authorizeClaim.selector; @@ -364,7 +396,7 @@ contract OnChainAllocator is IOnChainAllocator, Utility { function isClaimAuthorized( bytes32 claimHash, address, /*arbiter*/ // The account tasked with verifying and submitting the claim. - address sponsor, // The account sponsoring the claim. + address, /*sponsor*/ // The account sponsoring the claim. uint256, /*nonce*/ // A parameter to enforce replay protection, scoped to allocator. uint256 expires, // The time at which the claim expires. uint256[2][] calldata idsAndAmounts, // The allocated token IDs and amounts. @@ -378,15 +410,9 @@ contract OnChainAllocator is IOnChainAllocator, Utility { if (idsAndAmounts.length == 0) { return false; } - bytes32 tokenHash = _getTokenHash(idsAndAmounts[0][0], sponsor); - Allocation[] memory allocations = _allocations[tokenHash]; - for (uint256 j = 0; j < allocations.length; j++) { - if (allocations[j].claimHash == claimHash) { - return true; - } - } + (bool verified,) = _verifyClaim(claimHash); - return false; + return verified; } function _allocate( @@ -411,17 +437,21 @@ contract OnChainAllocator is IOnChainAllocator, Utility { uint256 minResetPeriod = type(uint256).max; for (uint256 i = 0; i < commitments.length; i++) { minResetPeriod = _checkInput(commitments[i], sponsor, expires, minResetPeriod); - bytes32 tokenHash = _checkBalance(sponsor, commitments[i]); + (bytes32 tokenHash, uint32 previousExpirationPointer, uint32 nextExpirationPointer) = + _checkBalance(sponsor, commitments[i], expires); // Store the allocation uint224 amount = uint224(commitments[i].amount); - _storeAllocation(tokenHash, amount, expires, claimHash); + _storeAllocatedBalance(tokenHash, amount, expires, previousExpirationPointer, nextExpirationPointer); } // Ensure expiration is not bigger then the smallest reset period if (expires >= block.timestamp + minResetPeriod) { revert InvalidExpiration(expires, block.timestamp + minResetPeriod); } + // Store the claim + _storeClaim(claimHash, expires); + return (claimHash, nonce); } @@ -457,11 +487,16 @@ contract OnChainAllocator is IOnChainAllocator, Utility { return minResetPeriod; } - function _checkBalance(address sponsor, Lock calldata commitment) private returns (bytes32 tokenHash) { + function _checkBalance(address sponsor, Lock calldata commitment, uint32 expires) + private + returns (bytes32 tokenHash, uint32 previousExpirationPointer, uint32 nextExpirationPointer) + { // Check the balance of the recipient is sufficient tokenHash = _getTokenHash(commitment.lockTag, commitment.token, sponsor); uint256 balance = settledBalanceOf(sponsor, AL.toId(commitment.lockTag, commitment.token)); - uint256 allocatedBalance = _allocatedBalance(tokenHash); + uint256 allocatedBalance; + (allocatedBalance, previousExpirationPointer, nextExpirationPointer) = + _readAllocatedBalance(tokenHash, expires, false); uint256 requiredBalance = allocatedBalance + commitment.amount; if (requiredBalance > balance) { revert InsufficientBalance( @@ -470,23 +505,201 @@ contract OnChainAllocator is IOnChainAllocator, Utility { } } - function _storeAllocation( + function _normalizeExpiration(uint32 expires) private view returns (uint32 normalizedExpires) { + uint256 timeRemaining = expires - block.timestamp; + if (timeRemaining < 10 minutes) { + // No rounding - max size of 600 unique expirations + normalizedExpires = expires; + } else if (timeRemaining < 1 hours + 5 minutes) { + // total of 55 minutes + // Round up to the nearest 10 seconds - max size of 330 unique expirations + normalizedExpires = (expires / 10 seconds) * 10 seconds + 10 seconds; + } else if (timeRemaining < 1 days) { + // total of 1,375 minutes + // Round up to the nearest minute - max size of 1375 unique expirations + normalizedExpires = (expires / 1 minutes) * 1 minutes + 1 minutes; + } else if (timeRemaining < 1 weeks + 1 hours) { + // total of 8,700 minutes + // Round up to the nearest 10 minutes - max size of 870 unique expirations + normalizedExpires = (expires / 10 minutes) * 10 minutes + 10 minutes; + } else { + // < 30 days - total of 33,060 minutes + // Round up to the nearest hour - max size of 551 unique expirations + normalizedExpires = (expires / 1 hours) * 1 hours + 1 hours; + } + // Total max of 3,726 unique expirations => 7,824,600 max gas costs for cold storage reads + } + + function _storeClaim(bytes32 claimHash, uint32 normalizedExpiration) private { + uint256 currentExpiration = _allocatedClaims[claimHash]; + if (currentExpiration > 0) { + revert InvalidClaim(claimHash); + } + _allocatedClaims[claimHash] = normalizedExpiration; + } + + function _readAllocatedBalance(bytes32 tokenHash, uint32 normalizedExpiration, bool onlyReturnPointers) + private + returns (uint256 allocatedBalance, uint32 previousExpirationPointer, uint32 nextExpirationPointer) + { + uint32 nextExpiration = _nextExpirationPointer[tokenHash]; + if (nextExpiration == 0) { + // No allocated balance detected + allocatedBalance = 0; + previousExpirationPointer = 0; + nextExpirationPointer = type(uint32).max; + return (allocatedBalance, previousExpirationPointer, nextExpirationPointer); + } else { + // Other allocated balances detected. Accumulate non expired balances. + + // Loop through the expired balances and remove them + while (nextExpiration <= block.timestamp) { + // Found expired balance, remove it + bytes32 pointer = _generatePointer(tokenHash, nextExpiration); + nextExpiration = _balancesByExpiration[pointer].nextExpiration; + delete _balancesByExpiration[pointer]; + } + + // Read the balances that expire before the ongoing allocation + while (normalizedExpiration > nextExpiration) { + // Cache the previous expiration pointer + previousExpirationPointer = nextExpiration; + + bytes32 pointer = _generatePointer(tokenHash, nextExpiration); + BalanceExpiration memory balance = _balancesByExpiration[pointer]; + allocatedBalance += balance.amount; + nextExpiration = balance.nextExpiration; + } + + // Cache the next expiration pointer + nextExpirationPointer = nextExpiration; + + if (onlyReturnPointers) { + // Return the pointers early without an accurate allocation balance. + return (type(uint256).max, previousExpirationPointer, nextExpirationPointer); + } + + // Read the balances that expire after the ongoing allocation + while (nextExpiration < type(uint32).max) { + bytes32 pointer = _generatePointer(tokenHash, nextExpiration); + BalanceExpiration memory balance = _balancesByExpiration[pointer]; + allocatedBalance += balance.amount; + nextExpiration = balance.nextExpiration; + } + + // nextExpiration of uint32.max indicates the end of the list + } + } + + function _storeAllocatedBalance( bytes12 lockTag, address token, - uint224 amount, address recipient, - uint32 expires, - bytes32 claimHash + uint224 amount, + uint32 normalizedExpiration ) private { bytes32 tokenHash = _getTokenHash(lockTag, token, recipient); - _storeAllocation(tokenHash, amount, expires, claimHash); + (, uint32 previousExpirationPointer, uint32 nextExpirationPointer) = + _readAllocatedBalance(tokenHash, normalizedExpiration, true); + _storeAllocatedBalance( + tokenHash, amount, normalizedExpiration, previousExpirationPointer, nextExpirationPointer + ); + } + + function _storeAllocatedBalance( + bytes32 tokenHash, + uint224 amount, + uint32 normalizedExpiration, + uint32 previousExpirationPointer, + uint32 nextExpirationPointer + ) private { + bytes32 pointer = _generatePointer(tokenHash, normalizedExpiration); + + // Check if if there is already an allocation for the same expiration + if (normalizedExpiration == nextExpirationPointer) { + // Update the amount of the existing allocation + _balancesByExpiration[pointer].amount += amount; // Will overflow if amount becomes greater than uint224.max + return; + } + + // Check if there are any balances expiring earlier than the new allocation + if (previousExpirationPointer == 0) { + // Update the _nextExpirationPointer pointer to the new allocation + _nextExpirationPointer[tokenHash] = normalizedExpiration; + } + + // Create the new allocation + _balancesByExpiration[pointer] = BalanceExpiration({nextExpiration: nextExpirationPointer, amount: amount}); + } + + function _generatePointer(bytes32 tokenHash, uint32 expiration) private pure returns (bytes32 pointer) { + // Make sure the expiration is the most significant 32 bits + assembly ("memory-safe") { + pointer := or(shl(32, tokenHash), expiration) + } } - function _storeAllocation(bytes32 tokenHash, uint224 amount, uint32 expires, bytes32 claimHash) private { - Allocation memory allocation = Allocation({expires: expires, amount: amount, claimHash: claimHash}); - _allocations[tokenHash].push(allocation); + function _verifyClaim(bytes32 claimHash) private view returns (bool verified, uint32 normalizedExpiration) { + // Check if the claim is allocated + normalizedExpiration = _allocatedClaims[claimHash]; + return (normalizedExpiration != 0, normalizedExpiration); + } + + function _deleteAllocatedBalance(bytes32 tokenHash, uint32 normalizedExpiration, uint224 amount, uint32 hint) + private + { + bytes32 pointer = _generatePointer(tokenHash, normalizedExpiration); + BalanceExpiration memory balance = _balancesByExpiration[pointer]; + + // Check if there is more allocated balance at the expiration than the amount to delete + if (balance.amount > amount) { + _balancesByExpiration[pointer].amount -= amount; + return; + } + + // Delete the full allocation + delete _balancesByExpiration[pointer]; + + // Find the previous expiration to update the pointers + + uint32 previousExpirationPointer = _nextExpirationPointer[tokenHash]; + + // Check if the allocation is the earliest expiring + if (previousExpirationPointer == normalizedExpiration) { + // TODO: CHANGE THIS TO BRANCHLESS ASSEMBLY CODE BY MULTIPLYING + // Check if another allocation exists after this one + if (balance.nextExpiration == type(uint32).max) { + // Earliest and latest allocation: delete the pointer + delete _nextExpirationPointer[tokenHash]; + } else { + // Update the pointer to the next expiration + _nextExpirationPointer[tokenHash] = balance.nextExpiration; + } + + return; + } + + // Check if a valid position hint was provided + if (hint != 0) { + pointer = _generatePointer(tokenHash, hint); + // Verify the hint is valid + if (_balancesByExpiration[pointer].nextExpiration == normalizedExpiration) { + // Valid hint detected. Skip the detection loop. + previousExpirationPointer = normalizedExpiration; + } + } + + // Loop through the previously expiring balances to find the previous pointer + while (previousExpirationPointer < normalizedExpiration) { + pointer = _generatePointer(tokenHash, previousExpirationPointer); + previousExpirationPointer = _balancesByExpiration[pointer].nextExpiration; + } + + // Update the next expiration pointer of the previous expiration + _balancesByExpiration[pointer].nextExpiration = balance.nextExpiration; } + /* function _allocatedBalance(bytes32 tokenHash) private returns (uint256 allocatedBalance) { // using assembly to only read the allocated balance + expiration slot and skipping the claimHash slot assembly ("memory-safe") { @@ -577,6 +790,7 @@ contract OnChainAllocator is IOnChainAllocator, Utility { } } } + */ function _getAndUpdateNonce(address calling, address sponsor) internal returns (uint256 nonce) { assembly ("memory-safe") { diff --git a/src/interfaces/IOnChainAllocator.sol b/src/interfaces/IOnChainAllocator.sol index b79ff16..e946392 100644 --- a/src/interfaces/IOnChainAllocator.sol +++ b/src/interfaces/IOnChainAllocator.sol @@ -44,6 +44,9 @@ interface IOnChainAllocator is IOnChainAllocation { /// @notice Thrown if the provided commitments are empty error InvalidCommitments(); + /// @notice Thrown if the provided allocatorData is invalid + error InvalidHint(uint256 allocatorDataLength, uint256 expectedAllocatorDataLength); + /// @notice Registers an allocation for a set of tokens /// @param commitments The commitments of the allocations /// @param arbiter The arbiter of the allocation diff --git a/test/OnChainAllocator.t.sol b/test/OnChainAllocator.t.sol index c822c86..7214941 100644 --- a/test/OnChainAllocator.t.sol +++ b/test/OnChainAllocator.t.sol @@ -887,6 +887,7 @@ contract OnChainAllocatorTest is Test, TestHelper { // call from Compact contract address vm.prank(address(compact)); bytes4 sel = allocator.authorizeClaim(claimHash, arbiter, user, 1, defaultExpiration, idsAndAmounts, ''); + vm.snapshotGasLastCall('authorizeClaim_success_single_allocation'); assertEq(sel, IAllocator.authorizeClaim.selector); // check deletion of the allocation From 9282c63a9308ff47bd1cb3df095f0f409f3d5179 Mon Sep 17 00:00:00 2001 From: mgretzke Date: Thu, 15 Jan 2026 16:49:16 +0100 Subject: [PATCH 02/11] bug fixes --- src/allocators/OnChainAllocator.sol | 119 ++++----------------------- src/interfaces/IOnChainAllocator.sol | 5 +- 2 files changed, 18 insertions(+), 106 deletions(-) diff --git a/src/allocators/OnChainAllocator.sol b/src/allocators/OnChainAllocator.sol index 6dcb7c0..2be7cf4 100644 --- a/src/allocators/OnChainAllocator.sol +++ b/src/allocators/OnChainAllocator.sol @@ -30,13 +30,6 @@ contract OnChainAllocator is IOnChainAllocator, Utility { /// @notice The unique identifier for this allocator within The Compact protocol uint96 public immutable ALLOCATOR_ID; - mapping(bytes32 tokenHash => Allocation[] allocations) internal _allocations; - - struct BalanceExpiration { - uint32 nextExpiration; - uint224 amount; - } - mapping(bytes32 tokenHash => uint32 nextExpiration) internal _nextExpirationPointer; /// @notice Similar to mapping(bytes32 tokenHash => mapping(uint32 expiration => BalanceExpiration balances)). mapping(bytes32 TokenHashWithExpiration => BalanceExpiration balances) internal _balancesByExpiration; @@ -531,7 +524,7 @@ contract OnChainAllocator is IOnChainAllocator, Utility { } function _storeClaim(bytes32 claimHash, uint32 normalizedExpiration) private { - uint256 currentExpiration = _allocatedClaims[claimHash]; + uint32 currentExpiration = _allocatedClaims[claimHash]; if (currentExpiration > 0) { revert InvalidClaim(claimHash); } @@ -542,14 +535,15 @@ contract OnChainAllocator is IOnChainAllocator, Utility { private returns (uint256 allocatedBalance, uint32 previousExpirationPointer, uint32 nextExpirationPointer) { - uint32 nextExpiration = _nextExpirationPointer[tokenHash]; - if (nextExpiration == 0) { + uint32 originalNextExpiration = _nextExpirationPointer[tokenHash]; + if (originalNextExpiration == 0) { // No allocated balance detected allocatedBalance = 0; previousExpirationPointer = 0; nextExpirationPointer = type(uint32).max; return (allocatedBalance, previousExpirationPointer, nextExpirationPointer); } else { + uint32 nextExpiration = originalNextExpiration; // Other allocated balances detected. Accumulate non expired balances. // Loop through the expired balances and remove them @@ -559,6 +553,18 @@ contract OnChainAllocator is IOnChainAllocator, Utility { nextExpiration = _balancesByExpiration[pointer].nextExpiration; delete _balancesByExpiration[pointer]; } + // Check if the next expiration pointer has changed during the loop. If so, update the pointer. + if (nextExpiration != originalNextExpiration) { + assembly ("memory-safe") { + // Set nextExpiration to 0 if this was the last allocation (nextExpiration == type(uint32).max) + let p := mul(nextExpiration, lt(nextExpiration, 0xffffffff)) + // Store p in _nextExpirationPointer[tokenHash] + mstore(0x00, tokenHash) + mstore(0x20, _nextExpirationPointer.slot) + let pointer := keccak256(0x00, 0x40) + sstore(pointer, p) + } + } // Read the balances that expire before the ongoing allocation while (normalizedExpiration > nextExpiration) { @@ -699,99 +705,6 @@ contract OnChainAllocator is IOnChainAllocator, Utility { _balancesByExpiration[pointer].nextExpiration = balance.nextExpiration; } - /* - function _allocatedBalance(bytes32 tokenHash) private returns (uint256 allocatedBalance) { - // using assembly to only read the allocated balance + expiration slot and skipping the claimHash slot - assembly ("memory-safe") { - // no previous cached balance, calculate the allocated balance - mstore(0x00, tokenHash) - mstore(0x20, _allocations.slot) - // retrieve the array length slot - let arrayLengthSlot := keccak256(0x00, 0x40) - let origLength := sload(arrayLengthSlot) - let length := origLength - // retrieve the arrays content slot - mstore(0x00, arrayLengthSlot) - let contentSlot := keccak256(0x00, 0x20) - for { let i := 0 } lt(i, length) {} { - let slot := add(contentSlot, mul(i, 2)) // 0x40 to skip the claimHash slot - let content := sload(slot) - let expiration := shr(224, shl(224, content)) - if lt(expiration, timestamp()) { - // allocation expired, remove it - let lastSlot := add(contentSlot, mul(sub(length, 1), 2)) - if iszero(eq(slot, lastSlot)) { - // is not the last allocation of the array - let contentLast1 := sload(lastSlot) - let contentLast2 := sload(add(lastSlot, 1)) - sstore(slot, contentLast1) - sstore(add(slot, 1), contentLast2) - } - // remove the last allocation - length := sub(length, 1) - sstore(lastSlot, 0) - sstore(add(lastSlot, 1), 0) - - // repeat the loop at the same index - continue - } - - let amount := shr(32, content) - allocatedBalance := add(allocatedBalance, amount) - - // jump to the next allocation - i := add(i, 1) - } - - if lt(length, origLength) { - // update the array length - sstore(arrayLengthSlot, length) - } - } - } - - function _verifyClaim(bytes32 tokenHash, bytes32 claimHash) private returns (bool verified) { - // using assembly to only read the claimHash slot and skip the expires/amount slot - assembly ("memory-safe") { - mstore(0x00, tokenHash) - mstore(0x20, _allocations.slot) - let lengthSlot := keccak256(0x00, 0x40) - let length := sload(lengthSlot) - mstore(0x00, lengthSlot) - let contentSlot := keccak256(0x00, 0x20) - for { let i := 0 } lt(i, length) { i := add(i, 1) } { - // Each allocation occupies two consecutive slots: - // first: packed expires/amount; second: claimHash - let first := add(contentSlot, mul(i, 2)) - let second := add(first, 1) - if eq(sload(second), claimHash) { - // Swap-and-pop delete - let lastFirst := add(contentSlot, mul(sub(length, 1), 2)) - let lastSecond := add(lastFirst, 1) - if iszero(eq(first, lastFirst)) { - let contentLast1 := sload(lastFirst) - let contentLast2 := sload(lastSecond) - sstore(first, contentLast1) - sstore(second, contentLast2) - } - - sstore(lastFirst, 0) - sstore(lastSecond, 0) - - // update the array length - sstore(lengthSlot, sub(length, 1)) - - // We return at the first match, no matter if the allocated amounts match. - // If the claim includes the same token multiple times (amounts mismatch), - // we will enter this function again until all entries are deleted. - verified := 1 - break - } - } - } - } - */ - function _getAndUpdateNonce(address calling, address sponsor) internal returns (uint256 nonce) { assembly ("memory-safe") { sponsor := mul(sponsor, iszero(calling)) diff --git a/src/interfaces/IOnChainAllocator.sol b/src/interfaces/IOnChainAllocator.sol index e946392..4daa0e1 100644 --- a/src/interfaces/IOnChainAllocator.sol +++ b/src/interfaces/IOnChainAllocator.sol @@ -8,10 +8,9 @@ import {Lock} from '@uniswap/the-compact/types/EIP712Types.sol'; /// @title IOnChainAllocator /// @notice Interface for the on-chain token allocator that prevents double-spending in a fully decentralized manner interface IOnChainAllocator is IOnChainAllocation { - struct Allocation { - uint32 expires; + struct BalanceExpiration { + uint32 nextExpiration; uint224 amount; - bytes32 claimHash; } /// @notice Thrown if the allocator is not successfully registered From e53d97c46add9284f646ee0467874d59ee0c52f0 Mon Sep 17 00:00:00 2001 From: mgretzke Date: Thu, 15 Jan 2026 17:09:26 +0100 Subject: [PATCH 03/11] more fixes --- snapshots/ERC7683Allocator_open.json | 2 +- snapshots/ERC7683Allocator_openFor.json | 2 +- snapshots/OnChainAllocatorTest.json | 16 ++++++++-------- src/allocators/OnChainAllocator.sol | 8 +++++--- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/snapshots/ERC7683Allocator_open.json b/snapshots/ERC7683Allocator_open.json index 220f720..d019f5e 100644 --- a/snapshots/ERC7683Allocator_open.json +++ b/snapshots/ERC7683Allocator_open.json @@ -1,3 +1,3 @@ { - "open_simpleOrder": "169186" + "open_simpleOrder": "169184" } \ No newline at end of file diff --git a/snapshots/ERC7683Allocator_openFor.json b/snapshots/ERC7683Allocator_openFor.json index db2161e..fa5656c 100644 --- a/snapshots/ERC7683Allocator_openFor.json +++ b/snapshots/ERC7683Allocator_openFor.json @@ -1,3 +1,3 @@ { - "openFor_simpleOrder_userHimself": "172581" + "openFor_simpleOrder_userHimself": "172579" } \ No newline at end of file diff --git a/snapshots/OnChainAllocatorTest.json b/snapshots/OnChainAllocatorTest.json index 502a3e7..20102e3 100644 --- a/snapshots/OnChainAllocatorTest.json +++ b/snapshots/OnChainAllocatorTest.json @@ -1,10 +1,10 @@ { - "allocateFor_success_withRegistration": "134514", - "allocate_and_delete_expired_allocation": "76330", - "allocate_erc20": "129965", - "allocate_native": "129725", - "allocate_second_erc20": "95213", - "authorizeClaim_success_single_allocation": "32597", - "onchain_execute_double": "325233", - "onchain_execute_single": "220720" + "allocateFor_success_withRegistration": "134512", + "allocate_and_delete_expired_allocation": "76600", + "allocate_erc20": "129963", + "allocate_native": "129723", + "allocate_second_erc20": "95573", + "authorizeClaim_success_single_allocation": "32596", + "onchain_execute_double": "325229", + "onchain_execute_single": "220718" } \ No newline at end of file diff --git a/src/allocators/OnChainAllocator.sol b/src/allocators/OnChainAllocator.sol index 2be7cf4..f4cb1ce 100644 --- a/src/allocators/OnChainAllocator.sol +++ b/src/allocators/OnChainAllocator.sol @@ -354,7 +354,6 @@ contract OnChainAllocator is IOnChainAllocator, Utility { uint256[2][] calldata idsAndAmounts, // The allocated token IDs and amounts. bytes calldata allocatorData // Arbitrary data provided by the arbiter. ) external virtual onlyCompact returns (bytes4) { - // TODO: Allow allocatorData to be used to submit the previous expiration pointer (bool verified, uint32 normalizedExpiration) = _verifyClaim(claimHash); if (!verified) { revert InvalidClaim(claimHash); @@ -364,8 +363,8 @@ contract OnChainAllocator is IOnChainAllocator, Utility { delete _allocatedClaims[claimHash]; // If allocatorData is provided, ensure the length matches the expectation - if (allocatorData.length != 0 && allocatorData.length != 32 * idsAndAmounts.length) { - revert InvalidHint(allocatorData.length, 32 * idsAndAmounts.length); + if (allocatorData.length != 0 && allocatorData.length != 4 * idsAndAmounts.length) { + revert InvalidHint(allocatorData.length, 4 * idsAndAmounts.length); } // Delete the allocations @@ -632,6 +631,9 @@ contract OnChainAllocator is IOnChainAllocator, Utility { if (previousExpirationPointer == 0) { // Update the _nextExpirationPointer pointer to the new allocation _nextExpirationPointer[tokenHash] = normalizedExpiration; + } else { + bytes32 previousPointer = _generatePointer(tokenHash, previousExpirationPointer); + _balancesByExpiration[previousPointer].nextExpiration = normalizedExpiration; } // Create the new allocation From 9167b1c99ce39f4b8b46c9832df3f50c1829c541 Mon Sep 17 00:00:00 2001 From: mgretzke Date: Fri, 16 Jan 2026 14:22:23 +0100 Subject: [PATCH 04/11] Tests and external view function for normalized expiration --- snapshots/HybridAllocatorTest.json | 2 +- snapshots/OnChainAllocatorTest.json | 8 +- src/allocators/OnChainAllocator.sol | 6 + src/interfaces/IOnChainAllocator.sol | 5 + src/test/OnChainAllocationCaller.sol | 8 +- test/OnChainAllocator.t.sol | 275 +++++++++++++++++++++++++++ 6 files changed, 295 insertions(+), 9 deletions(-) diff --git a/snapshots/HybridAllocatorTest.json b/snapshots/HybridAllocatorTest.json index 7693eb8..bcbdf5e 100644 --- a/snapshots/HybridAllocatorTest.json +++ b/snapshots/HybridAllocatorTest.json @@ -6,5 +6,5 @@ "allocateAndRegister_nativeToken_emptyAmountInput": "139058", "allocateAndRegister_second_erc20Token": "114865", "allocateAndRegister_second_nativeToken": "104858", - "hybrid_execute_single": "174737" + "hybrid_execute_single": "174786" } \ No newline at end of file diff --git a/snapshots/OnChainAllocatorTest.json b/snapshots/OnChainAllocatorTest.json index 20102e3..79c1188 100644 --- a/snapshots/OnChainAllocatorTest.json +++ b/snapshots/OnChainAllocatorTest.json @@ -1,10 +1,10 @@ { "allocateFor_success_withRegistration": "134512", - "allocate_and_delete_expired_allocation": "76600", + "allocate_and_delete_expired_allocation": "79470", "allocate_erc20": "129963", "allocate_native": "129723", - "allocate_second_erc20": "95573", + "allocate_second_erc20": "98377", "authorizeClaim_success_single_allocation": "32596", - "onchain_execute_double": "325229", - "onchain_execute_single": "220718" + "onchain_execute_double": "325278", + "onchain_execute_single": "220767" } \ No newline at end of file diff --git a/src/allocators/OnChainAllocator.sol b/src/allocators/OnChainAllocator.sol index f4cb1ce..174a7da 100644 --- a/src/allocators/OnChainAllocator.sol +++ b/src/allocators/OnChainAllocator.sol @@ -407,6 +407,12 @@ contract OnChainAllocator is IOnChainAllocator, Utility { return verified; } + /// @inheritdoc IOnChainAllocator + function getNormalizedExpirationForClaim(bytes32 claimHash) external view returns (uint32 normalizedExpiration) { + normalizedExpiration = _allocatedClaims[claimHash]; + return normalizedExpiration; + } + function _allocate( address sponsor, Lock[] calldata commitments, diff --git a/src/interfaces/IOnChainAllocator.sol b/src/interfaces/IOnChainAllocator.sol index 4daa0e1..040b7a3 100644 --- a/src/interfaces/IOnChainAllocator.sol +++ b/src/interfaces/IOnChainAllocator.sol @@ -97,4 +97,9 @@ interface IOnChainAllocator is IOnChainAllocation { bytes32 typehash, bytes32 witness ) external payable returns (bytes32 claimHash, uint256[] memory registeredAmounts, uint256 nonce); + + /// @notice Returns the normalized expiration for a claim + /// @param claimHash The hash of the claim + /// @return normalizedExpiration The normalized expiration for the claim + function getNormalizedExpirationForClaim(bytes32 claimHash) external view returns (uint32 normalizedExpiration); } diff --git a/src/test/OnChainAllocationCaller.sol b/src/test/OnChainAllocationCaller.sol index 1db8bff..8dd9048 100644 --- a/src/test/OnChainAllocationCaller.sol +++ b/src/test/OnChainAllocationCaller.sol @@ -21,12 +21,12 @@ contract OnChainAllocationCaller { bytes32 typehash, bytes32 witness, uint8 todo - ) external { + ) external returns (bytes32 claimHash) { uint256 nonce; if (todo == 0) { // Correctly deposit and register nonce = ALLOCATOR.prepareAllocation(recipient, idsAndAmounts, arbiter, expires, typehash, witness, ''); - ITheCompact(COMPACT).batchDepositAndRegisterFor( + (claimHash,) = ITheCompact(COMPACT).batchDepositAndRegisterFor( recipient, idsAndAmounts, arbiter, nonce, expires, typehash, witness ); } else if (todo == 1) { @@ -35,7 +35,7 @@ contract OnChainAllocationCaller { ITheCompact(COMPACT).batchDeposit(idsAndAmounts, recipient); } else if (todo == 2) { // Do not prepare, but deposit and register - ITheCompact(COMPACT).batchDepositAndRegisterFor( + (claimHash,) = ITheCompact(COMPACT).batchDepositAndRegisterFor( recipient, idsAndAmounts, arbiter, nonce, expires, typehash, witness ); } else if (todo == 3) { @@ -43,7 +43,7 @@ contract OnChainAllocationCaller { } else { // Correctly deposit and register nonce = ALLOCATOR.prepareAllocation(recipient, idsAndAmounts, arbiter, expires, typehash, witness, ''); - ITheCompact(COMPACT).batchDepositAndRegisterFor( + (claimHash,) = ITheCompact(COMPACT).batchDepositAndRegisterFor( recipient, idsAndAmounts, arbiter, nonce, expires, typehash, witness ); ALLOCATOR.executeAllocation(recipient, idsAndAmounts, arbiter, expires, typehash, witness, ''); diff --git a/test/OnChainAllocator.t.sol b/test/OnChainAllocator.t.sol index 7214941..2e70bff 100644 --- a/test/OnChainAllocator.t.sol +++ b/test/OnChainAllocator.t.sol @@ -30,6 +30,7 @@ import {BatchClaimComponent, Component} from '@uniswap/the-compact/types/Compone import {AllocatorLib} from 'src/allocators/lib/AllocatorLib.sol'; import {OnChainAllocationCaller} from 'src/test/OnChainAllocationCaller.sol'; +import {console} from 'forge-std/console.sol'; import {DeployTheCompact} from 'test/util/DeployTheCompact.sol'; import {TestHelper} from 'test/util/TestHelper.sol'; @@ -2175,6 +2176,280 @@ contract OnChainAllocatorTest is Test, TestHelper { 'Malicious recipient should still have his balance in TheCompact' ); } + + /* --------------------------------------------------------------------- */ + /* Allocation Bombing Protection Tests */ + /* --------------------------------------------------------------------- */ + + /** + * @notice Comprehensive test for allocation bombing mitigation. + * @dev The vulnerability was: + * - Attacker creates ~66k allocations on behalf of a user with unique expirations + * - Operations loop through all allocations, exceeding block gas limit (DoS) + * + * The fix: + * - External allocations have expiration normalized (rounded up to buckets) + * - Max ~3,726 unique expirations possible + * - Amounts accumulate per bucket, not per allocation + * + * This test verifies all critical operations remain usable after worst-case attack: + * 1. attest() - reads allocated balance (traverses list) + * 2. allocate() with late expiration - inserts at end (traverses full list) + * 3. authorizeClaim() - deletes allocation (traverses to find previous pointer) + * 4. allocate() with early expiration - inserts at beginning (O(1)) + */ + /// forge-config: default.isolate = false + function test_allocationBombing_comprehensive() public { + // Setup: Create a victim with deposited funds + address victim = makeAddr('victim'); + uint256 depositAmount = 1000 ether; + usdc.mint(victim, depositAmount); + + vm.startPrank(victim); + usdc.approve(address(compact), depositAmount); + bytes12 lockTag = _toLockTag(address(allocator), Scope.Multichain, ResetPeriod.OneDay); + uint256 id = compact.depositERC20(address(usdc), lockTag, depositAmount, victim); + vm.stopPrank(); + + // ========================================================================= + // PHASE 1: Simulate attack with 66k allocations + // ========================================================================= + // We use pauseGasMetering to simulate a real-world attack where allocations + // are created across many transactions (bypassing single-tx gas limits) + uint256 numAllocations = 66_000; + uint256 amountPerAllocation = 1; + bytes32 lastClaimHash; + + vm.pauseGasMetering(); + + for (uint256 i = 0; i < numAllocations; i++) { + // Each allocation 1 second apart, spanning ~18 hours + // Without normalization: 66k unique entries (DoS) + // With normalization: ~2388 buckets + uint32 expiration = uint32(block.timestamp + 1 hours + i); + + uint256[2][] memory idsAndAmounts = new uint256[2][](1); + idsAndAmounts[0][0] = id; + idsAndAmounts[0][1] = amountPerAllocation; + + usdc.mint(address(allocationCaller), amountPerAllocation); + vm.prank(address(allocationCaller)); + usdc.approve(address(compact), amountPerAllocation); + + lastClaimHash = allocationCaller.onChainAllocation( + victim, idsAndAmounts, arbiter, expiration, BATCH_COMPACT_TYPEHASH, bytes32(0), 0 + ); + } + + vm.resumeGasMetering(); + + // ========================================================================= + // PHASE 2: Test attest() - must traverse list to read allocated balance + // ========================================================================= + uint256 gasBefore = gasleft(); + bytes4 result = allocator.attest(address(0), victim, address(0), id, 0); + uint256 attestGas = gasBefore - gasleft(); + + assertEq(result, allocator.attest.selector, 'attest should succeed'); + console.log('Gas: attest() with 66k allocations', attestGas); + assertLt(attestGas, 10_000_000, 'attest gas should be under 10M'); + + // ========================================================================= + // PHASE 3: Test allocate() with LATE expiration - must traverse to insert at end + // ========================================================================= + // This is the worst case: inserting after all existing allocations + // requires traversing the entire linked list to find insertion point + Lock[] memory lateCommitments = new Lock[](1); + lateCommitments[0] = Lock({lockTag: lockTag, token: address(usdc), amount: 1 ether}); + // Expiration after all existing allocations (1 hour + 66k seconds + buffer) + uint32 lateExpiration = uint32(block.timestamp + 1 hours + numAllocations + 1000); + + vm.startPrank(victim); + gasBefore = gasleft(); + (bytes32 lateClaimHash,) = + allocator.allocate(lateCommitments, arbiter, lateExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0)); + uint256 allocateLateGas = gasBefore - gasleft(); + vm.stopPrank(); + + console.log('Gas: allocate() late expiration (insert at end)', allocateLateGas); + assertLt(allocateLateGas, 10_000_000, 'allocate late gas should be under 10M'); + + // ========================================================================= + // PHASE 4: Test authorizeClaim() - must traverse to delete late allocation + // ========================================================================= + // Deleting an allocation at the end requires traversing to find the previous pointer + uint256[2][] memory claimIdsAndAmounts = new uint256[2][](1); + claimIdsAndAmounts[0][0] = id; + claimIdsAndAmounts[0][1] = 1 ether; + + // Build the allocator data hint (0 = no hint, force full traversal) + bytes memory noHint = new bytes(4); + + gasBefore = gasleft(); + vm.prank(address(compact)); + allocator.authorizeClaim(lateClaimHash, arbiter, victim, 0, lateExpiration, claimIdsAndAmounts, noHint); + uint256 authorizeClaimGas = gasBefore - gasleft(); + + console.log('Gas: authorizeClaim() delete late allocation', authorizeClaimGas); + assertLt(authorizeClaimGas, 10_000_000, 'authorizeClaim gas should be under 10M'); + + // ========================================================================= + // PHASE 5: Test allocate() with EARLY expiration - should be O(1) + // ========================================================================= + // This is the best case: inserting at the beginning of the list + Lock[] memory earlyCommitments = new Lock[](1); + earlyCommitments[0] = Lock({lockTag: lockTag, token: address(usdc), amount: 1 ether}); + // Expiration before all existing allocations + uint32 earlyExpiration = uint32(block.timestamp + 11 minutes); + + vm.startPrank(victim); + gasBefore = gasleft(); + allocator.allocate(earlyCommitments, arbiter, earlyExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0)); + uint256 allocateEarlyGas = gasBefore - gasleft(); + vm.stopPrank(); + + console.log('Gas: allocate() early expiration (insert at beginning)', allocateEarlyGas); + // Both early and late allocations need to traverse the full list to sum allocated balance + // (for _checkBalance), so they have similar gas costs. The key is both are under block limit. + assertLt(allocateEarlyGas, 10_000_000, 'allocate early gas should be under 10M'); + + // ========================================================================= + // PHASE 6: Test authorizeClaim() with late expiration and hint + // ========================================================================= + // This should be O(1) because the hint is provided + + vm.prank(victim); + (lateClaimHash,) = + allocator.allocate(lateCommitments, arbiter, lateExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0)); + + // Build the allocator data hint (0 = no hint, force full traversal) + uint32 lastAllocationBombingExpiration = allocator.getNormalizedExpirationForClaim(lastClaimHash); + assertGt(lastAllocationBombingExpiration, 0, 'Last allocation bombing expiration should be greater than 0'); + bytes memory hint = new bytes(4); + assembly ("memory-safe") { + mstore(add(hint, 0x20), shl(224, lastAllocationBombingExpiration)) + } + + gasBefore = gasleft(); + vm.prank(address(compact)); + allocator.authorizeClaim(lateClaimHash, arbiter, victim, 0, lateExpiration, claimIdsAndAmounts, hint); + uint256 authorizeClaimWithHintGas = gasBefore - gasleft(); + + console.log('Gas: authorizeClaim() delete late allocation with hint', authorizeClaimWithHintGas); + assertLt(authorizeClaimWithHintGas, 1_000_000, 'authorizeClaim with hint gas should be under 1M'); + + // ========================================================================= + // Summary: All operations completed within reasonable gas limits + // ========================================================================= + console.log(''); + console.log('=== Allocation Bombing Protection Summary ==='); + console.log('Total allocations created', numAllocations); + console.log('attest() gas', attestGas); + console.log('allocate() late (worst case) gas', allocateLateGas); + console.log('authorizeClaim() delete late gas', authorizeClaimGas); + console.log('allocate() early (best case) gas', allocateEarlyGas); + console.log('authorizeClaim() delete late with hint gas', authorizeClaimWithHintGas); + } + + /** + * @notice Test that allocations with same normalized expiration accumulate amounts. + * @dev Verifies that multiple allocations bucketed to the same expiration + * don't create separate entries but instead accumulate in one entry. + */ + /// forge-config: default.isolate = false + function test_allocationBombing_amountsAccumulate() public { + address victim = makeAddr('victim2'); + uint256 depositAmount = 100 ether; + usdc.mint(victim, depositAmount); + + vm.startPrank(victim); + usdc.approve(address(compact), depositAmount); + bytes12 lockTag = _toLockTag(address(allocator), Scope.Multichain, ResetPeriod.OneDay); + uint256 id = compact.depositERC20(address(usdc), lockTag, depositAmount, victim); + vm.stopPrank(); + + // Create multiple allocations that will normalize to the same expiration + // Expirations within 10-second window (in the 10min-1h range) bucket together + uint256 numAllocations = 5; + uint256 amountPerAllocation = 1 ether; + uint32 baseExpiration = uint32(block.timestamp + 30 minutes); + + for (uint256 i = 0; i < numAllocations; i++) { + // All within same 10-second bucket + uint32 expiration = baseExpiration + uint32(i * 2); // 0, 2, 4, 6, 8 seconds apart + + uint256[2][] memory idsAndAmounts = new uint256[2][](1); + idsAndAmounts[0][0] = id; + idsAndAmounts[0][1] = amountPerAllocation; + + usdc.mint(address(allocationCaller), amountPerAllocation); + vm.prank(address(allocationCaller)); + usdc.approve(address(compact), amountPerAllocation); + + allocationCaller.onChainAllocation( + victim, idsAndAmounts, arbiter, expiration, BATCH_COMPACT_TYPEHASH, bytes32(0), 0 + ); + } + + // Now try to transfer more than available (should fail) + // Total allocated: 5 * 1 ether = 5 ether + // Note: batchDepositAndRegisterFor also deposits 1 ether per allocation to victim's balance + // Balance: 100 (initial) + 5 (from allocations) = 105 ether + // Available: 105 - 5 = 100 ether + uint256 attemptTransfer = 101 ether; // More than available + + vm.expectRevert(); + allocator.attest(address(0), victim, address(0), id, attemptTransfer); + + // But transferring up to 100 ether should work + uint256 validTransfer = 100 ether; + bytes4 result = allocator.attest(address(0), victim, address(0), id, validTransfer); + assertEq(result, allocator.attest.selector, 'Valid transfer should succeed'); + } + + /** + * @notice Test that expired allocations are cleaned up during reads. + * @dev Verifies the lazy cleanup mechanism works correctly. + */ + /// forge-config: default.isolate = false + function test_allocationBombing_expiredAllocationsCleanedUp() public { + address victim = makeAddr('victim3'); + uint256 depositAmount = 100 ether; + usdc.mint(victim, depositAmount); + + vm.startPrank(victim); + usdc.approve(address(compact), depositAmount); + bytes12 lockTag = _toLockTag(address(allocator), Scope.Multichain, ResetPeriod.OneDay); + uint256 id = compact.depositERC20(address(usdc), lockTag, depositAmount, victim); + vm.stopPrank(); + + // Create allocation that expires soon + uint32 shortExpiration = uint32(block.timestamp + 5 minutes); + uint256 allocatedAmount = 50 ether; + + uint256[2][] memory idsAndAmounts = new uint256[2][](1); + idsAndAmounts[0][0] = id; + idsAndAmounts[0][1] = allocatedAmount; + + usdc.mint(address(allocationCaller), allocatedAmount); + vm.prank(address(allocationCaller)); + usdc.approve(address(compact), allocatedAmount); + + allocationCaller.onChainAllocation( + victim, idsAndAmounts, arbiter, shortExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), 0 + ); + + // Before expiration: can only transfer 50 ether + bytes4 result = allocator.attest(address(0), victim, address(0), id, 50 ether); + assertEq(result, allocator.attest.selector); + + // Warp past expiration + vm.warp(shortExpiration + 1); + + // After expiration: can transfer full 100 ether (allocation cleaned up) + result = allocator.attest(address(0), victim, address(0), id, 100 ether); + assertEq(result, allocator.attest.selector, 'Should be able to transfer full amount after expiration'); + } } /* ============================================================================ From d16d75de28a5aceb6f74e73137c75dcc7cdca083 Mon Sep 17 00:00:00 2001 From: mgretzke Date: Fri, 16 Jan 2026 18:01:37 +0100 Subject: [PATCH 05/11] bytes32 tokenHash changed to bytes28 --- snapshots/ERC7683Allocator_open.json | 2 +- snapshots/ERC7683Allocator_openFor.json | 2 +- snapshots/OnChainAllocatorTest.json | 16 +-- src/allocators/OnChainAllocator.sol | 125 ++++++++++++------------ test/OnChainAllocator.t.sol | 83 ++++++++++++++++ 5 files changed, 157 insertions(+), 71 deletions(-) diff --git a/snapshots/ERC7683Allocator_open.json b/snapshots/ERC7683Allocator_open.json index d019f5e..e81443f 100644 --- a/snapshots/ERC7683Allocator_open.json +++ b/snapshots/ERC7683Allocator_open.json @@ -1,3 +1,3 @@ { - "open_simpleOrder": "169184" + "open_simpleOrder": "169193" } \ No newline at end of file diff --git a/snapshots/ERC7683Allocator_openFor.json b/snapshots/ERC7683Allocator_openFor.json index fa5656c..7cde8e8 100644 --- a/snapshots/ERC7683Allocator_openFor.json +++ b/snapshots/ERC7683Allocator_openFor.json @@ -1,3 +1,3 @@ { - "openFor_simpleOrder_userHimself": "172579" + "openFor_simpleOrder_userHimself": "172545" } \ No newline at end of file diff --git a/snapshots/OnChainAllocatorTest.json b/snapshots/OnChainAllocatorTest.json index 79c1188..fd723df 100644 --- a/snapshots/OnChainAllocatorTest.json +++ b/snapshots/OnChainAllocatorTest.json @@ -1,10 +1,10 @@ { - "allocateFor_success_withRegistration": "134512", - "allocate_and_delete_expired_allocation": "79470", - "allocate_erc20": "129963", - "allocate_native": "129723", - "allocate_second_erc20": "98377", - "authorizeClaim_success_single_allocation": "32596", - "onchain_execute_double": "325278", - "onchain_execute_single": "220767" + "allocateFor_success_withRegistration": "134506", + "allocate_and_delete_expired_allocation": "79359", + "allocate_erc20": "129957", + "allocate_native": "129717", + "allocate_second_erc20": "98317", + "authorizeClaim_success_single_allocation": "32600", + "onchain_execute_double": "325290", + "onchain_execute_single": "220773" } \ No newline at end of file diff --git a/src/allocators/OnChainAllocator.sol b/src/allocators/OnChainAllocator.sol index 174a7da..6db4810 100644 --- a/src/allocators/OnChainAllocator.sol +++ b/src/allocators/OnChainAllocator.sol @@ -30,8 +30,8 @@ contract OnChainAllocator is IOnChainAllocator, Utility { /// @notice The unique identifier for this allocator within The Compact protocol uint96 public immutable ALLOCATOR_ID; - mapping(bytes32 tokenHash => uint32 nextExpiration) internal _nextExpirationPointer; - /// @notice Similar to mapping(bytes32 tokenHash => mapping(uint32 expiration => BalanceExpiration balances)). + mapping(bytes28 tokenHash => uint32 nextExpiration) internal _nextExpirationPointer; + /// @notice Similar to mapping(bytes28 tokenHash => mapping(uint32 expiration => BalanceExpiration balances)). mapping(bytes32 TokenHashWithExpiration => BalanceExpiration balances) internal _balancesByExpiration; mapping(bytes32 claimHash => uint32 normalizedExpiration) internal _allocatedClaims; @@ -333,7 +333,7 @@ contract OnChainAllocator is IOnChainAllocator, Utility { uint256 balance = ERC6909(AL.THE_COMPACT).balanceOf(from_, id_); // Check unlocked balance - bytes32 tokenHash = _getTokenHash(id_, from_); + bytes28 tokenHash = _getTokenHash(id_, from_); (uint256 allocatedBalance,,) = _readAllocatedBalance(tokenHash, type(uint32).max, false); uint256 fullAmount = amount_ + allocatedBalance; @@ -369,7 +369,7 @@ contract OnChainAllocator is IOnChainAllocator, Utility { // Delete the allocations for (uint256 i = 0; i < idsAndAmounts.length; i++) { - bytes32 tokenHash = _getTokenHash(idsAndAmounts[i][0], sponsor); + bytes28 tokenHash = _getTokenHash(idsAndAmounts[i][0], sponsor); uint32 hint = 0; if (allocatorData.length != 0) { @@ -435,7 +435,7 @@ contract OnChainAllocator is IOnChainAllocator, Utility { uint256 minResetPeriod = type(uint256).max; for (uint256 i = 0; i < commitments.length; i++) { minResetPeriod = _checkInput(commitments[i], sponsor, expires, minResetPeriod); - (bytes32 tokenHash, uint32 previousExpirationPointer, uint32 nextExpirationPointer) = + (bytes28 tokenHash, uint32 previousExpirationPointer, uint32 nextExpirationPointer) = _checkBalance(sponsor, commitments[i], expires); // Store the allocation @@ -487,7 +487,7 @@ contract OnChainAllocator is IOnChainAllocator, Utility { function _checkBalance(address sponsor, Lock calldata commitment, uint32 expires) private - returns (bytes32 tokenHash, uint32 previousExpirationPointer, uint32 nextExpirationPointer) + returns (bytes28 tokenHash, uint32 previousExpirationPointer, uint32 nextExpirationPointer) { // Check the balance of the recipient is sufficient tokenHash = _getTokenHash(commitment.lockTag, commitment.token, sponsor); @@ -536,70 +536,73 @@ contract OnChainAllocator is IOnChainAllocator, Utility { _allocatedClaims[claimHash] = normalizedExpiration; } - function _readAllocatedBalance(bytes32 tokenHash, uint32 normalizedExpiration, bool onlyReturnPointers) + function _readAllocatedBalance(bytes28 tokenHash, uint32 normalizedExpiration, bool onlyReturnPointers) private returns (uint256 allocatedBalance, uint32 previousExpirationPointer, uint32 nextExpirationPointer) { uint32 originalNextExpiration = _nextExpirationPointer[tokenHash]; + + // Check if there are any allocated balances if (originalNextExpiration == 0) { // No allocated balance detected allocatedBalance = 0; previousExpirationPointer = 0; nextExpirationPointer = type(uint32).max; + return (allocatedBalance, previousExpirationPointer, nextExpirationPointer); - } else { - uint32 nextExpiration = originalNextExpiration; - // Other allocated balances detected. Accumulate non expired balances. - - // Loop through the expired balances and remove them - while (nextExpiration <= block.timestamp) { - // Found expired balance, remove it - bytes32 pointer = _generatePointer(tokenHash, nextExpiration); - nextExpiration = _balancesByExpiration[pointer].nextExpiration; - delete _balancesByExpiration[pointer]; - } - // Check if the next expiration pointer has changed during the loop. If so, update the pointer. - if (nextExpiration != originalNextExpiration) { - assembly ("memory-safe") { - // Set nextExpiration to 0 if this was the last allocation (nextExpiration == type(uint32).max) - let p := mul(nextExpiration, lt(nextExpiration, 0xffffffff)) - // Store p in _nextExpirationPointer[tokenHash] - mstore(0x00, tokenHash) - mstore(0x20, _nextExpirationPointer.slot) - let pointer := keccak256(0x00, 0x40) - sstore(pointer, p) - } - } + } - // Read the balances that expire before the ongoing allocation - while (normalizedExpiration > nextExpiration) { - // Cache the previous expiration pointer - previousExpirationPointer = nextExpiration; + uint32 nextExpiration = originalNextExpiration; + // Other allocated balances detected. Accumulate non expired balances. - bytes32 pointer = _generatePointer(tokenHash, nextExpiration); - BalanceExpiration memory balance = _balancesByExpiration[pointer]; - allocatedBalance += balance.amount; - nextExpiration = balance.nextExpiration; + // Loop through the expired balances and remove them + while (nextExpiration <= block.timestamp) { + // Found expired balance, remove it + bytes32 pointer = _generatePointer(tokenHash, nextExpiration); + nextExpiration = _balancesByExpiration[pointer].nextExpiration; + delete _balancesByExpiration[pointer]; + } + // Check if the next expiration pointer has changed during the loop. If so, update the pointer. + if (nextExpiration != originalNextExpiration) { + assembly ("memory-safe") { + // Set nextExpiration to 0 if this was the last allocation (nextExpiration == type(uint32).max) + let p := mul(nextExpiration, lt(nextExpiration, 0xffffffff)) + // Store p in _nextExpirationPointer[tokenHash] + mstore(0x00, tokenHash) + mstore(0x20, _nextExpirationPointer.slot) + let pointer := keccak256(0x00, 0x40) + sstore(pointer, p) } + } - // Cache the next expiration pointer - nextExpirationPointer = nextExpiration; + // Read the balances that expire before the ongoing allocation + while (normalizedExpiration > nextExpiration) { + // Cache the previous expiration pointer + previousExpirationPointer = nextExpiration; - if (onlyReturnPointers) { - // Return the pointers early without an accurate allocation balance. - return (type(uint256).max, previousExpirationPointer, nextExpirationPointer); - } + bytes32 pointer = _generatePointer(tokenHash, nextExpiration); + BalanceExpiration memory balance = _balancesByExpiration[pointer]; + allocatedBalance += balance.amount; + nextExpiration = balance.nextExpiration; + } - // Read the balances that expire after the ongoing allocation - while (nextExpiration < type(uint32).max) { - bytes32 pointer = _generatePointer(tokenHash, nextExpiration); - BalanceExpiration memory balance = _balancesByExpiration[pointer]; - allocatedBalance += balance.amount; - nextExpiration = balance.nextExpiration; - } + // Cache the next expiration pointer + nextExpirationPointer = nextExpiration; + + if (onlyReturnPointers) { + // Return the pointers early without an accurate allocation balance. + return (type(uint256).max, previousExpirationPointer, nextExpirationPointer); + } - // nextExpiration of uint32.max indicates the end of the list + // Read the balances that expire after the ongoing allocation + while (nextExpiration < type(uint32).max) { + bytes32 pointer = _generatePointer(tokenHash, nextExpiration); + BalanceExpiration memory balance = _balancesByExpiration[pointer]; + allocatedBalance += balance.amount; + nextExpiration = balance.nextExpiration; } + + // nextExpiration of uint32.max indicates the end of the list } function _storeAllocatedBalance( @@ -609,7 +612,7 @@ contract OnChainAllocator is IOnChainAllocator, Utility { uint224 amount, uint32 normalizedExpiration ) private { - bytes32 tokenHash = _getTokenHash(lockTag, token, recipient); + bytes28 tokenHash = _getTokenHash(lockTag, token, recipient); (, uint32 previousExpirationPointer, uint32 nextExpirationPointer) = _readAllocatedBalance(tokenHash, normalizedExpiration, true); _storeAllocatedBalance( @@ -618,7 +621,7 @@ contract OnChainAllocator is IOnChainAllocator, Utility { } function _storeAllocatedBalance( - bytes32 tokenHash, + bytes28 tokenHash, uint224 amount, uint32 normalizedExpiration, uint32 previousExpirationPointer, @@ -646,10 +649,10 @@ contract OnChainAllocator is IOnChainAllocator, Utility { _balancesByExpiration[pointer] = BalanceExpiration({nextExpiration: nextExpirationPointer, amount: amount}); } - function _generatePointer(bytes32 tokenHash, uint32 expiration) private pure returns (bytes32 pointer) { + function _generatePointer(bytes28 tokenHash, uint32 expiration) private pure returns (bytes32 pointer) { // Make sure the expiration is the most significant 32 bits assembly ("memory-safe") { - pointer := or(shl(32, tokenHash), expiration) + pointer := or(tokenHash, expiration) } } @@ -659,7 +662,7 @@ contract OnChainAllocator is IOnChainAllocator, Utility { return (normalizedExpiration != 0, normalizedExpiration); } - function _deleteAllocatedBalance(bytes32 tokenHash, uint32 normalizedExpiration, uint224 amount, uint32 hint) + function _deleteAllocatedBalance(bytes28 tokenHash, uint32 normalizedExpiration, uint224 amount, uint32 hint) private { bytes32 pointer = _generatePointer(tokenHash, normalizedExpiration); @@ -736,20 +739,20 @@ contract OnChainAllocator is IOnChainAllocator, Utility { } } - function _getTokenHash(bytes12 lockTag, address token, address sponsor) private pure returns (bytes32 tokenHash) { + function _getTokenHash(bytes12 lockTag, address token, address sponsor) private pure returns (bytes28 tokenHash) { assembly ("memory-safe") { mstore(0x00, lockTag) mstore(0x0c, shl(96, token)) mstore(0x20, sponsor) - tokenHash := keccak256(0x00, 0x40) + tokenHash := and(keccak256(0x00, 0x40), 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF00000000) } } - function _getTokenHash(uint256 id, address sponsor) private pure returns (bytes32 tokenHash) { + function _getTokenHash(uint256 id, address sponsor) private pure returns (bytes28 tokenHash) { assembly ("memory-safe") { mstore(0x00, id) mstore(0x20, sponsor) - tokenHash := keccak256(0x00, 0x40) + tokenHash := and(keccak256(0x00, 0x40), 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF00000000) } } } diff --git a/test/OnChainAllocator.t.sol b/test/OnChainAllocator.t.sol index 2e70bff..659124c 100644 --- a/test/OnChainAllocator.t.sol +++ b/test/OnChainAllocator.t.sol @@ -2351,6 +2351,89 @@ contract OnChainAllocatorTest is Test, TestHelper { console.log('authorizeClaim() delete late with hint gas', authorizeClaimWithHintGas); } + /** + * @notice Test that BalanceExpiration struct is stored in a single storage slot. + * @dev The struct contains: + * - uint32 nextExpiration (4 bytes) + * - uint224 amount (28 bytes) + * Total: 32 bytes = 1 slot + * + * Storage layout (from forge inspect): + * - _balancesByExpiration is at slot 1 + * - For mapping(bytes32 => struct), slot = keccak256(key, baseSlot) + * + * Struct packing in storage (right-aligned): + * - Bits 0-31: nextExpiration (uint32) + * - Bits 32-255: amount (uint224) + */ + function test_balanceExpirationFitsInSingleSlot() public { + // Create an allocation within 10 minutes (no normalization) + usdc.mint(user, 10 ether); + vm.startPrank(user); + usdc.approve(address(compact), 10 ether); + bytes12 lockTag = _toLockTag(address(allocator), Scope.Multichain, ResetPeriod.OneDay); + compact.depositERC20(address(usdc), lockTag, 10 ether, user); + + Lock[] memory commitments = new Lock[](1); + uint224 allocatedAmount = 1 ether; + commitments[0] = Lock({lockTag: lockTag, token: address(usdc), amount: allocatedAmount}); + + // Use expiration within 10 minutes to avoid normalization + uint32 expiration = uint32(block.timestamp + 5 minutes); + allocator.allocate(commitments, arbiter, expiration, BATCH_COMPACT_TYPEHASH, bytes32(0)); + vm.stopPrank(); + + // Compute tokenHash using same logic as _getTokenHash(lockTag, token, sponsor) + // Memory layout: lockTag(12) || token(20) || zeros(12) || sponsor(20) = 64 bytes + address token = address(usdc); + address sponsor = user; + bytes28 tokenHash; + assembly ("memory-safe") { + let ptr := mload(0x40) + mstore(ptr, lockTag) + mstore(add(ptr, 0x0c), shl(96, token)) + mstore(add(ptr, 0x20), sponsor) + // Mask to keep only high 28 bytes (clear low 4 bytes) + tokenHash := and(keccak256(ptr, 0x40), 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF00000000) + } + + // Compute pointer: tokenHash | expiration + // tokenHash is bytes28 (left-aligned with 4 zero bytes on right) + // expiration is uint32 (right-aligned) + bytes32 pointer; + assembly ("memory-safe") { + pointer := or(tokenHash, expiration) + } + + // Compute storage slot: keccak256(pointer, baseSlot) + // _balancesByExpiration is at slot 1 + bytes32 storageSlot; + assembly ("memory-safe") { + mstore(0x00, pointer) + mstore(0x20, 1) // slot 1 + storageSlot := keccak256(0x00, 0x40) + } + + // Read the raw storage slot + bytes32 slotValue = vm.load(address(allocator), storageSlot); + + // Verify struct packing: + // - Low 32 bits (4 bytes): nextExpiration + // - High 224 bits (28 bytes): amount + uint32 storedNextExpiration = uint32(uint256(slotValue)); + uint224 storedAmount = uint224(uint256(slotValue) >> 32); + + // nextExpiration should be type(uint32).max (end of list marker) + assertEq(storedNextExpiration, type(uint32).max, 'nextExpiration should be max (end of list)'); + + // amount should match what we allocated + assertEq(storedAmount, allocatedAmount, 'amount should match allocated amount'); + + // Verify both values are in the same slot by checking the combined value + uint256 expectedSlotValue = (uint256(allocatedAmount) << 32) | uint256(type(uint32).max); + assertEq(uint256(slotValue), expectedSlotValue, 'Both fields should be packed in single slot'); + } + /** * @notice Test that allocations with same normalized expiration accumulate amounts. * @dev Verifies that multiple allocations bucketed to the same expiration From 6db83df6500915f15539a21fa212de21ecafec72 Mon Sep 17 00:00:00 2001 From: mgretzke Date: Wed, 21 Jan 2026 13:17:00 +0100 Subject: [PATCH 06/11] optimized _storeAllocatedBalance and _readAllocatedBalance function --- snapshots/ERC7683Allocator_open.json | 2 +- snapshots/ERC7683Allocator_openFor.json | 2 +- snapshots/OnChainAllocatorTest.json | 16 +-- src/allocators/OnChainAllocator.sol | 139 +++++++++++++++++++++++- test/OnChainAllocator.t.sol | 2 +- 5 files changed, 149 insertions(+), 12 deletions(-) diff --git a/snapshots/ERC7683Allocator_open.json b/snapshots/ERC7683Allocator_open.json index e81443f..8147bd5 100644 --- a/snapshots/ERC7683Allocator_open.json +++ b/snapshots/ERC7683Allocator_open.json @@ -1,3 +1,3 @@ { - "open_simpleOrder": "169193" + "open_simpleOrder": "168833" } \ No newline at end of file diff --git a/snapshots/ERC7683Allocator_openFor.json b/snapshots/ERC7683Allocator_openFor.json index 7cde8e8..338c56a 100644 --- a/snapshots/ERC7683Allocator_openFor.json +++ b/snapshots/ERC7683Allocator_openFor.json @@ -1,3 +1,3 @@ { - "openFor_simpleOrder_userHimself": "172545" + "openFor_simpleOrder_userHimself": "172227" } \ No newline at end of file diff --git a/snapshots/OnChainAllocatorTest.json b/snapshots/OnChainAllocatorTest.json index fd723df..2cecc2a 100644 --- a/snapshots/OnChainAllocatorTest.json +++ b/snapshots/OnChainAllocatorTest.json @@ -1,10 +1,10 @@ { - "allocateFor_success_withRegistration": "134506", - "allocate_and_delete_expired_allocation": "79359", - "allocate_erc20": "129957", - "allocate_native": "129717", - "allocate_second_erc20": "98317", - "authorizeClaim_success_single_allocation": "32600", - "onchain_execute_double": "325290", - "onchain_execute_single": "220773" + "allocateFor_success_withRegistration": "134116", + "allocate_and_delete_expired_allocation": "78914", + "allocate_erc20": "129567", + "allocate_native": "129327", + "allocate_second_erc20": "97822", + "authorizeClaim_success_single_allocation": "32584", + "onchain_execute_double": "324623", + "onchain_execute_single": "220449" } \ No newline at end of file diff --git a/src/allocators/OnChainAllocator.sol b/src/allocators/OnChainAllocator.sol index 6db4810..4e3dea0 100644 --- a/src/allocators/OnChainAllocator.sol +++ b/src/allocators/OnChainAllocator.sol @@ -23,6 +23,9 @@ import {Utility} from '@uniswap/the-compact/utility/Utility.sol'; /// @dev Users can open orders for themselves or for others by providing a signature or the tokens directly. /// @custom:security-contact security@uniswap.org contract OnChainAllocator is IOnChainAllocator, Utility { + uint32 private constant _UINT32_MAX = 0xffffffff; + bytes28 private constant _BYTES28_SELECTOR = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffff; + /// @notice The chain id at the time of deployment uint256 private immutable _INITIAL_CHAIN_ID; /// @notice The EIP-712 domain separator for The Compact protocol, used for signature verification @@ -526,6 +529,11 @@ contract OnChainAllocator is IOnChainAllocator, Utility { normalizedExpires = (expires / 1 hours) * 1 hours + 1 hours; } // Total max of 3,726 unique expirations => 7,824,600 max gas costs for cold storage reads + + // Sanitize expiration + assembly ("memory-safe") { + normalizedExpires := and(normalizedExpires, _UINT32_MAX) + } } function _storeClaim(bytes32 claimHash, uint32 normalizedExpiration) private { @@ -536,6 +544,133 @@ contract OnChainAllocator is IOnChainAllocator, Utility { _allocatedClaims[claimHash] = normalizedExpiration; } + function _readAllocatedBalance(bytes28 tokenHash, uint32 normalizedExpiration, bool onlyReturnPointers) + private + returns (uint256 allocatedBalance, uint32 previousExpirationPointer, uint32 nextExpirationPointer) + { + assembly ("memory-safe") { + mstore(0x00, tokenHash) + mstore(0x20, _nextExpirationPointer.slot) + let _nextExpirationPointerSlot := keccak256(0x00, 0x40) + let originalNextExpiration := sload(_nextExpirationPointerSlot) + // Check if there are any allocated balances + if iszero(originalNextExpiration) { + // No allocated balance detected + nextExpirationPointer := _UINT32_MAX + } + if iszero(nextExpirationPointer) { + // Other allocated balances detected. Accumulate non expired balances + let nextExpiration := originalNextExpiration + + // Store _balancesByExpiration.slot in advance to skip repeating calls in loops + mstore(0x20, _balancesByExpiration.slot) + + // Loop through the expired balances and remove them + for {} iszero(gt(nextExpiration, timestamp())) {} { + // Found expired balance, remove it + mstore(0x00, or(tokenHash, nextExpiration)) + // Previously stored _balancesByExpiration.slot in 0x20 + let pointer := keccak256(0x00, 0x40) + nextExpiration := and(sload(pointer), _UINT32_MAX) + // Delete + sstore(pointer, 0) + } + // Check if the next expiration pointer has changed during the loop. If so, update the pointer. + if iszero(eq(nextExpiration, originalNextExpiration)) { + // Set nextExpiration to 0 if this was the last allocation (nextExpiration == type(uint32).max) + let nextExpirationFixed := mul(nextExpiration, lt(nextExpiration, _UINT32_MAX)) + // Store nextExpirationFixed in _nextExpirationPointer[tokenHash] + sstore(_nextExpirationPointerSlot, nextExpirationFixed) + } + + // Read the balances that expire before the ongoing allocation + for {} gt(normalizedExpiration, nextExpiration) {} { + // Cache the previous expiration pointer + previousExpirationPointer := nextExpiration + mstore(0x00, or(tokenHash, nextExpiration)) + // Previously stored _balancesByExpiration.slot in 0x20 + let balanceStruct := sload(keccak256(0x00, 0x40)) + allocatedBalance := add(allocatedBalance, shr(32, balanceStruct)) + nextExpiration := and(balanceStruct, _UINT32_MAX) + } + // Cache the next expiration pointer + nextExpirationPointer := nextExpiration + + // Check if not only the pointers are requested + if iszero(onlyReturnPointers) { + for {} lt(nextExpiration, _UINT32_MAX) {} { + mstore(0x00, or(tokenHash, nextExpiration)) + // Previously stored _balancesByExpiration.slot in 0x20 + let balanceStruct := sload(keccak256(0x00, 0x40)) + allocatedBalance := add(allocatedBalance, shr(32, balanceStruct)) + nextExpiration := and(balanceStruct, _UINT32_MAX) + } + } + } + } + + return (allocatedBalance, previousExpirationPointer, nextExpirationPointer); + } + + function _storeAllocatedBalance( + bytes28 tokenHash, + uint224 amount, + uint32 normalizedExpiration, + uint32 previousExpirationPointer, + uint32 nextExpirationPointer + ) private { + assembly ("memory-safe") { + for {} true {} { + // Check if if there is already an allocation for the same expiration + if eq(normalizedExpiration, nextExpirationPointer) { + // Update the amount of the existing allocation and exit + mstore(0x00, or(tokenHash, normalizedExpiration)) + mstore(0x20, _balancesByExpiration.slot) + let pointer := keccak256(0x00, 0x40) + let balanceStruct := sload(pointer) + let currentAllocation := shr(32, balanceStruct) + let newAllocatedAmount := add(currentAllocation, amount) + if lt(newAllocatedAmount, currentAllocation) { + // Revert for overflow + revert(0x00, 0x00) + } + let balanceStructWithoutAmount := and(balanceStruct, _UINT32_MAX) + sstore(pointer, or(shl(32, newAllocatedAmount), balanceStructWithoutAmount)) + + break + } + + // Create and store the new allocation + mstore(0x00, or(tokenHash, normalizedExpiration)) + mstore(0x20, _balancesByExpiration.slot) + sstore(keccak256(0x00, 0x40), or(shl(32, amount), nextExpirationPointer)) + + // Check if there are any balances expiring earlier than the new allocation + if previousExpirationPointer { + // Insert the allocation into the linked list by updating the pointer of the earlier expiring allocation + mstore(0x00, or(tokenHash, previousExpirationPointer)) + // Previously stored _balancesByExpiration.slot in 0x20 + let pointer := keccak256(0x00, 0x40) + let balanceStructWithoutExpiration := and(sload(pointer), _BYTES28_SELECTOR) + // Write the updated struct to storage + sstore(pointer, or(balanceStructWithoutExpiration, normalizedExpiration)) + + break + } + + // At this point, there must not be any balances expiring earlier than the new allocation + + // Update the _nextExpirationPointer pointer to the new allocation + mstore(0x00, tokenHash) + mstore(0x20, _nextExpirationPointer.slot) + sstore(keccak256(0x00, 0x40), normalizedExpiration) + + break + } + } + } + + /* function _readAllocatedBalance(bytes28 tokenHash, uint32 normalizedExpiration, bool onlyReturnPointers) private returns (uint256 allocatedBalance, uint32 previousExpirationPointer, uint32 nextExpirationPointer) @@ -604,6 +739,7 @@ contract OnChainAllocator is IOnChainAllocator, Utility { // nextExpiration of uint32.max indicates the end of the list } + */ function _storeAllocatedBalance( bytes12 lockTag, @@ -625,7 +761,8 @@ contract OnChainAllocator is IOnChainAllocator, Utility { uint224 amount, uint32 normalizedExpiration, uint32 previousExpirationPointer, - uint32 nextExpirationPointer + uint32 nextExpirationPointer, + bool deleteThis ) private { bytes32 pointer = _generatePointer(tokenHash, normalizedExpiration); diff --git a/test/OnChainAllocator.t.sol b/test/OnChainAllocator.t.sol index 659124c..dfb8116 100644 --- a/test/OnChainAllocator.t.sol +++ b/test/OnChainAllocator.t.sol @@ -2216,7 +2216,7 @@ contract OnChainAllocatorTest is Test, TestHelper { // ========================================================================= // We use pauseGasMetering to simulate a real-world attack where allocations // are created across many transactions (bypassing single-tx gas limits) - uint256 numAllocations = 66_000; + uint256 numAllocations = 100; uint256 amountPerAllocation = 1; bytes32 lastClaimHash; From ea6c6e8c9e9649998ce6da9b78cbc9047f622248 Mon Sep 17 00:00:00 2001 From: mgretzke Date: Wed, 21 Jan 2026 13:23:25 +0100 Subject: [PATCH 07/11] _deleteAllocatedBalance optimization --- snapshots/OnChainAllocatorTest.json | 2 +- src/allocators/OnChainAllocator.sol | 76 +++++++++++++++++++++++++++++ test/OnChainAllocator.t.sol | 2 +- 3 files changed, 78 insertions(+), 2 deletions(-) diff --git a/snapshots/OnChainAllocatorTest.json b/snapshots/OnChainAllocatorTest.json index 2cecc2a..6f2bef1 100644 --- a/snapshots/OnChainAllocatorTest.json +++ b/snapshots/OnChainAllocatorTest.json @@ -4,7 +4,7 @@ "allocate_erc20": "129567", "allocate_native": "129327", "allocate_second_erc20": "97822", - "authorizeClaim_success_single_allocation": "32584", + "authorizeClaim_success_single_allocation": "32255", "onchain_execute_double": "324623", "onchain_execute_single": "220449" } \ No newline at end of file diff --git a/src/allocators/OnChainAllocator.sol b/src/allocators/OnChainAllocator.sol index 4e3dea0..17590e9 100644 --- a/src/allocators/OnChainAllocator.sol +++ b/src/allocators/OnChainAllocator.sol @@ -756,6 +756,7 @@ contract OnChainAllocator is IOnChainAllocator, Utility { ); } + /* function _storeAllocatedBalance( bytes28 tokenHash, uint224 amount, @@ -785,6 +786,7 @@ contract OnChainAllocator is IOnChainAllocator, Utility { // Create the new allocation _balancesByExpiration[pointer] = BalanceExpiration({nextExpiration: nextExpirationPointer, amount: amount}); } + */ function _generatePointer(bytes28 tokenHash, uint32 expiration) private pure returns (bytes32 pointer) { // Make sure the expiration is the most significant 32 bits @@ -802,6 +804,79 @@ contract OnChainAllocator is IOnChainAllocator, Utility { function _deleteAllocatedBalance(bytes28 tokenHash, uint32 normalizedExpiration, uint224 amount, uint32 hint) private { + assembly ("memory-safe") { + for {} true {} { + mstore(0x00, or(tokenHash, normalizedExpiration)) + mstore(0x20, _balancesByExpiration.slot) + let balancesByExpirationPointer := keccak256(0x00, 0x40) + let balanceStruct := sload(balancesByExpirationPointer) + let allocation := shr(32, balanceStruct) + let nextExpiration := and(balanceStruct, _UINT32_MAX) + + // Check if there is more allocated balance at the expiration than the amount to delete + if gt(allocation, amount) { + sstore(balancesByExpirationPointer, or(shl(32, sub(allocation, amount)), nextExpiration)) + break + } + + // Delete the full allocation + sstore(balancesByExpirationPointer, 0) + + // Find the previous expiration to update the pointers + mstore(0x00, tokenHash) + mstore(0x20, _nextExpirationPointer.slot) + let _nextExpirationPointerSlot := keccak256(0x00, 0x40) + let previousExpiration := sload(_nextExpirationPointerSlot) + + // Check if the allocation is the earliest expiring + if eq(previousExpiration, normalizedExpiration) { + // Check if another allocation exists after this one. If so, update the pointer, else delete + nextExpiration := mul(nextExpiration, lt(nextExpiration, _UINT32_MAX)) + sstore(_nextExpirationPointerSlot, nextExpiration) + break + } + + // Store _balancesByExpiration.slot in advance to skip repeating calls + mstore(0x20, _balancesByExpiration.slot) + + let previousBalanceStruct + if hint { + mstore(0x00, or(tokenHash, and(hint, _UINT32_MAX))) // sanitizes hint + balancesByExpirationPointer := keccak256(0x00, 0x40) + previousBalanceStruct := sload(balancesByExpirationPointer) + // Verify the hint is valid + let validHint := eq(normalizedExpiration, and(previousBalanceStruct, _UINT32_MAX)) + + // Branchless: validHint ? normalizedExpiration : previousExpiration + // Setting previousExpiration = normalizedExpiration makes loop condition false, skipping it + previousExpiration := + or(mul(previousExpiration, iszero(validHint)), mul(normalizedExpiration, validHint)) + } + + // Loop through the previously expiring balances to find the previous pointer + for {} lt(previousExpiration, normalizedExpiration) {} { + mstore(0x00, or(tokenHash, previousExpiration)) + balancesByExpirationPointer := keccak256(0x00, 0x40) + previousBalanceStruct := sload(balancesByExpirationPointer) + // Iterate previousExpiration to nextExpiration + previousExpiration := and(previousBalanceStruct, _UINT32_MAX) + } + + // Update the next expiration pointer of the previous expiration + sstore(balancesByExpirationPointer, or(and(previousBalanceStruct, _BYTES28_SELECTOR), nextExpiration)) + + break + } + } + } + + /* + function _deleteAllocatedBalance( + bytes28 tokenHash, + uint32 normalizedExpiration, + uint224 amount, + uint32 hint + ) private { bytes32 pointer = _generatePointer(tokenHash, normalizedExpiration); BalanceExpiration memory balance = _balancesByExpiration[pointer]; @@ -852,6 +927,7 @@ contract OnChainAllocator is IOnChainAllocator, Utility { // Update the next expiration pointer of the previous expiration _balancesByExpiration[pointer].nextExpiration = balance.nextExpiration; } + */ function _getAndUpdateNonce(address calling, address sponsor) internal returns (uint256 nonce) { assembly ("memory-safe") { diff --git a/test/OnChainAllocator.t.sol b/test/OnChainAllocator.t.sol index dfb8116..659124c 100644 --- a/test/OnChainAllocator.t.sol +++ b/test/OnChainAllocator.t.sol @@ -2216,7 +2216,7 @@ contract OnChainAllocatorTest is Test, TestHelper { // ========================================================================= // We use pauseGasMetering to simulate a real-world attack where allocations // are created across many transactions (bypassing single-tx gas limits) - uint256 numAllocations = 100; + uint256 numAllocations = 66_000; uint256 amountPerAllocation = 1; bytes32 lastClaimHash; From f02bebf1f9a3ab7aac39d0e82cf6befb8288d98d Mon Sep 17 00:00:00 2001 From: mgretzke Date: Wed, 21 Jan 2026 13:34:20 +0100 Subject: [PATCH 08/11] cleanup --- src/allocators/OnChainAllocator.sol | 162 ---------------------------- 1 file changed, 162 deletions(-) diff --git a/src/allocators/OnChainAllocator.sol b/src/allocators/OnChainAllocator.sol index 17590e9..956bde0 100644 --- a/src/allocators/OnChainAllocator.sol +++ b/src/allocators/OnChainAllocator.sol @@ -670,77 +670,6 @@ contract OnChainAllocator is IOnChainAllocator, Utility { } } - /* - function _readAllocatedBalance(bytes28 tokenHash, uint32 normalizedExpiration, bool onlyReturnPointers) - private - returns (uint256 allocatedBalance, uint32 previousExpirationPointer, uint32 nextExpirationPointer) - { - uint32 originalNextExpiration = _nextExpirationPointer[tokenHash]; - - // Check if there are any allocated balances - if (originalNextExpiration == 0) { - // No allocated balance detected - allocatedBalance = 0; - previousExpirationPointer = 0; - nextExpirationPointer = type(uint32).max; - - return (allocatedBalance, previousExpirationPointer, nextExpirationPointer); - } - - uint32 nextExpiration = originalNextExpiration; - // Other allocated balances detected. Accumulate non expired balances. - - // Loop through the expired balances and remove them - while (nextExpiration <= block.timestamp) { - // Found expired balance, remove it - bytes32 pointer = _generatePointer(tokenHash, nextExpiration); - nextExpiration = _balancesByExpiration[pointer].nextExpiration; - delete _balancesByExpiration[pointer]; - } - // Check if the next expiration pointer has changed during the loop. If so, update the pointer. - if (nextExpiration != originalNextExpiration) { - assembly ("memory-safe") { - // Set nextExpiration to 0 if this was the last allocation (nextExpiration == type(uint32).max) - let p := mul(nextExpiration, lt(nextExpiration, 0xffffffff)) - // Store p in _nextExpirationPointer[tokenHash] - mstore(0x00, tokenHash) - mstore(0x20, _nextExpirationPointer.slot) - let pointer := keccak256(0x00, 0x40) - sstore(pointer, p) - } - } - - // Read the balances that expire before the ongoing allocation - while (normalizedExpiration > nextExpiration) { - // Cache the previous expiration pointer - previousExpirationPointer = nextExpiration; - - bytes32 pointer = _generatePointer(tokenHash, nextExpiration); - BalanceExpiration memory balance = _balancesByExpiration[pointer]; - allocatedBalance += balance.amount; - nextExpiration = balance.nextExpiration; - } - - // Cache the next expiration pointer - nextExpirationPointer = nextExpiration; - - if (onlyReturnPointers) { - // Return the pointers early without an accurate allocation balance. - return (type(uint256).max, previousExpirationPointer, nextExpirationPointer); - } - - // Read the balances that expire after the ongoing allocation - while (nextExpiration < type(uint32).max) { - bytes32 pointer = _generatePointer(tokenHash, nextExpiration); - BalanceExpiration memory balance = _balancesByExpiration[pointer]; - allocatedBalance += balance.amount; - nextExpiration = balance.nextExpiration; - } - - // nextExpiration of uint32.max indicates the end of the list - } - */ - function _storeAllocatedBalance( bytes12 lockTag, address token, @@ -756,38 +685,6 @@ contract OnChainAllocator is IOnChainAllocator, Utility { ); } - /* - function _storeAllocatedBalance( - bytes28 tokenHash, - uint224 amount, - uint32 normalizedExpiration, - uint32 previousExpirationPointer, - uint32 nextExpirationPointer, - bool deleteThis - ) private { - bytes32 pointer = _generatePointer(tokenHash, normalizedExpiration); - - // Check if if there is already an allocation for the same expiration - if (normalizedExpiration == nextExpirationPointer) { - // Update the amount of the existing allocation - _balancesByExpiration[pointer].amount += amount; // Will overflow if amount becomes greater than uint224.max - return; - } - - // Check if there are any balances expiring earlier than the new allocation - if (previousExpirationPointer == 0) { - // Update the _nextExpirationPointer pointer to the new allocation - _nextExpirationPointer[tokenHash] = normalizedExpiration; - } else { - bytes32 previousPointer = _generatePointer(tokenHash, previousExpirationPointer); - _balancesByExpiration[previousPointer].nextExpiration = normalizedExpiration; - } - - // Create the new allocation - _balancesByExpiration[pointer] = BalanceExpiration({nextExpiration: nextExpirationPointer, amount: amount}); - } - */ - function _generatePointer(bytes28 tokenHash, uint32 expiration) private pure returns (bytes32 pointer) { // Make sure the expiration is the most significant 32 bits assembly ("memory-safe") { @@ -870,65 +767,6 @@ contract OnChainAllocator is IOnChainAllocator, Utility { } } - /* - function _deleteAllocatedBalance( - bytes28 tokenHash, - uint32 normalizedExpiration, - uint224 amount, - uint32 hint - ) private { - bytes32 pointer = _generatePointer(tokenHash, normalizedExpiration); - BalanceExpiration memory balance = _balancesByExpiration[pointer]; - - // Check if there is more allocated balance at the expiration than the amount to delete - if (balance.amount > amount) { - _balancesByExpiration[pointer].amount -= amount; - return; - } - - // Delete the full allocation - delete _balancesByExpiration[pointer]; - - // Find the previous expiration to update the pointers - - uint32 previousExpirationPointer = _nextExpirationPointer[tokenHash]; - - // Check if the allocation is the earliest expiring - if (previousExpirationPointer == normalizedExpiration) { - // TODO: CHANGE THIS TO BRANCHLESS ASSEMBLY CODE BY MULTIPLYING - // Check if another allocation exists after this one - if (balance.nextExpiration == type(uint32).max) { - // Earliest and latest allocation: delete the pointer - delete _nextExpirationPointer[tokenHash]; - } else { - // Update the pointer to the next expiration - _nextExpirationPointer[tokenHash] = balance.nextExpiration; - } - - return; - } - - // Check if a valid position hint was provided - if (hint != 0) { - pointer = _generatePointer(tokenHash, hint); - // Verify the hint is valid - if (_balancesByExpiration[pointer].nextExpiration == normalizedExpiration) { - // Valid hint detected. Skip the detection loop. - previousExpirationPointer = normalizedExpiration; - } - } - - // Loop through the previously expiring balances to find the previous pointer - while (previousExpirationPointer < normalizedExpiration) { - pointer = _generatePointer(tokenHash, previousExpirationPointer); - previousExpirationPointer = _balancesByExpiration[pointer].nextExpiration; - } - - // Update the next expiration pointer of the previous expiration - _balancesByExpiration[pointer].nextExpiration = balance.nextExpiration; - } - */ - function _getAndUpdateNonce(address calling, address sponsor) internal returns (uint256 nonce) { assembly ("memory-safe") { sponsor := mul(sponsor, iszero(calling)) From 0f5aeda3c9eca1b924f70637e5cbe919e0a40dbe Mon Sep 17 00:00:00 2001 From: mgretzke Date: Thu, 22 Jan 2026 16:03:24 +0100 Subject: [PATCH 09/11] improved hints to support same transaction allocations --- src/allocators/OnChainAllocator.sol | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/allocators/OnChainAllocator.sol b/src/allocators/OnChainAllocator.sol index 956bde0..3e42e5d 100644 --- a/src/allocators/OnChainAllocator.sol +++ b/src/allocators/OnChainAllocator.sol @@ -544,6 +544,9 @@ contract OnChainAllocator is IOnChainAllocator, Utility { _allocatedClaims[claimHash] = normalizedExpiration; } + // TODO: Think about if the users expirations MUST also be normalized to protect fillers + // TODO: Think about if the hint can still be used, if the nextExpiration retrieved from it is smaller then the target expiration + function _readAllocatedBalance(bytes28 tokenHash, uint32 normalizedExpiration, bool onlyReturnPointers) private returns (uint256 allocatedBalance, uint32 previousExpirationPointer, uint32 nextExpirationPointer) @@ -741,13 +744,19 @@ contract OnChainAllocator is IOnChainAllocator, Utility { mstore(0x00, or(tokenHash, and(hint, _UINT32_MAX))) // sanitizes hint balancesByExpirationPointer := keccak256(0x00, 0x40) previousBalanceStruct := sload(balancesByExpirationPointer) - // Verify the hint is valid - let validHint := eq(normalizedExpiration, and(previousBalanceStruct, _UINT32_MAX)) - - // Branchless: validHint ? normalizedExpiration : previousExpiration + let hintNextExpiration := and(previousBalanceStruct, _UINT32_MAX) + // Verify the hints next pointer is valid. + // It must be greater then the head pointer, as well as smaller or equal to the target expiration. + // Ideally the hints next pointer is equal to the target expiration. This will skip the next loop completely + let validHint := + and( + gt(hintNextExpiration, previousExpiration), iszero(gt(hintNextExpiration, normalizedExpiration)) + ) + + // Branchless: validHint ? hintNextExpiration : previousExpiration // Setting previousExpiration = normalizedExpiration makes loop condition false, skipping it previousExpiration := - or(mul(previousExpiration, iszero(validHint)), mul(normalizedExpiration, validHint)) + or(mul(previousExpiration, iszero(validHint)), mul(hintNextExpiration, validHint)) } // Loop through the previously expiring balances to find the previous pointer From 51d99c83f126aa634db09aa85a68ec69a7885702 Mon Sep 17 00:00:00 2001 From: mgretzke Date: Thu, 22 Jan 2026 16:51:17 +0100 Subject: [PATCH 10/11] normalize expirations also for the users own allocation --- snapshots/ERC7683Allocator_open.json | 2 +- snapshots/ERC7683Allocator_openFor.json | 2 +- snapshots/OnChainAllocatorTest.json | 10 +++++----- src/allocators/OnChainAllocator.sol | 13 +++++++------ 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/snapshots/ERC7683Allocator_open.json b/snapshots/ERC7683Allocator_open.json index 8147bd5..7cc5356 100644 --- a/snapshots/ERC7683Allocator_open.json +++ b/snapshots/ERC7683Allocator_open.json @@ -1,3 +1,3 @@ { - "open_simpleOrder": "168833" + "open_simpleOrder": "168974" } \ No newline at end of file diff --git a/snapshots/ERC7683Allocator_openFor.json b/snapshots/ERC7683Allocator_openFor.json index 338c56a..a671cea 100644 --- a/snapshots/ERC7683Allocator_openFor.json +++ b/snapshots/ERC7683Allocator_openFor.json @@ -1,3 +1,3 @@ { - "openFor_simpleOrder_userHimself": "172227" + "openFor_simpleOrder_userHimself": "172365" } \ No newline at end of file diff --git a/snapshots/OnChainAllocatorTest.json b/snapshots/OnChainAllocatorTest.json index 6f2bef1..c259017 100644 --- a/snapshots/OnChainAllocatorTest.json +++ b/snapshots/OnChainAllocatorTest.json @@ -1,9 +1,9 @@ { - "allocateFor_success_withRegistration": "134116", - "allocate_and_delete_expired_allocation": "78914", - "allocate_erc20": "129567", - "allocate_native": "129327", - "allocate_second_erc20": "97822", + "allocateFor_success_withRegistration": "134235", + "allocate_and_delete_expired_allocation": "79077", + "allocate_erc20": "129686", + "allocate_native": "129446", + "allocate_second_erc20": "97973", "authorizeClaim_success_single_allocation": "32255", "onchain_execute_double": "324623", "onchain_execute_single": "220449" diff --git a/src/allocators/OnChainAllocator.sol b/src/allocators/OnChainAllocator.sol index 3e42e5d..6761a65 100644 --- a/src/allocators/OnChainAllocator.sol +++ b/src/allocators/OnChainAllocator.sol @@ -431,6 +431,8 @@ contract OnChainAllocator is IOnChainAllocator, Utility { revert InvalidExpiration(expires, block.timestamp); } + uint32 normalizedExpiration = _normalizeExpiration(expires); + nonce = _getAndUpdateNonce(address(0), sponsor); // address(0) as caller allows anyone to relay bytes32 commitmentsHash = AL.getCommitmentsHash(commitments); claimHash = AL.getClaimHash(arbiter, sponsor, nonce, expires, commitmentsHash, witness, typehash); @@ -439,11 +441,13 @@ contract OnChainAllocator is IOnChainAllocator, Utility { for (uint256 i = 0; i < commitments.length; i++) { minResetPeriod = _checkInput(commitments[i], sponsor, expires, minResetPeriod); (bytes28 tokenHash, uint32 previousExpirationPointer, uint32 nextExpirationPointer) = - _checkBalance(sponsor, commitments[i], expires); + _checkBalance(sponsor, commitments[i], normalizedExpiration); // Store the allocation uint224 amount = uint224(commitments[i].amount); - _storeAllocatedBalance(tokenHash, amount, expires, previousExpirationPointer, nextExpirationPointer); + _storeAllocatedBalance( + tokenHash, amount, normalizedExpiration, previousExpirationPointer, nextExpirationPointer + ); } // Ensure expiration is not bigger then the smallest reset period if (expires >= block.timestamp + minResetPeriod) { @@ -451,7 +455,7 @@ contract OnChainAllocator is IOnChainAllocator, Utility { } // Store the claim - _storeClaim(claimHash, expires); + _storeClaim(claimHash, normalizedExpiration); return (claimHash, nonce); } @@ -544,9 +548,6 @@ contract OnChainAllocator is IOnChainAllocator, Utility { _allocatedClaims[claimHash] = normalizedExpiration; } - // TODO: Think about if the users expirations MUST also be normalized to protect fillers - // TODO: Think about if the hint can still be used, if the nextExpiration retrieved from it is smaller then the target expiration - function _readAllocatedBalance(bytes28 tokenHash, uint32 normalizedExpiration, bool onlyReturnPointers) private returns (uint256 allocatedBalance, uint32 previousExpirationPointer, uint32 nextExpirationPointer) From 0b24992a7fe986af2b1be217d84abbea4abdcb3b Mon Sep 17 00:00:00 2001 From: mgretzke Date: Fri, 23 Jan 2026 11:54:49 +0100 Subject: [PATCH 11/11] functions ordered by visibility --- src/allocators/OnChainAllocator.sol | 250 ++++++++++++++-------------- 1 file changed, 125 insertions(+), 125 deletions(-) diff --git a/src/allocators/OnChainAllocator.sol b/src/allocators/OnChainAllocator.sol index 6761a65..a325662 100644 --- a/src/allocators/OnChainAllocator.sol +++ b/src/allocators/OnChainAllocator.sol @@ -227,31 +227,6 @@ contract OnChainAllocator is IOnChainAllocator, Utility { return (claimHash, registeredAmounts, nonce); } - function _updateCommitmentsAndStoreAllocation( - address recipient, - uint256[] memory registeredAmounts, - Lock[] memory commitments, - uint32 expires, - bytes32 claimHash - ) private returns (Lock[] memory) { - // External allocation requires to normalize the expiration time - expires = _normalizeExpiration(expires); - // Store the allocation - for (uint256 i = 0; i < registeredAmounts.length; i++) { - // Update the allocations with the actual registered amounts - uint224 amount = uint224(registeredAmounts[i]); - commitments[i].amount = amount; - - // Store the allocation - _storeAllocatedBalance(commitments[i].lockTag, commitments[i].token, recipient, amount, expires); - } - - // Store the claim - _storeClaim(claimHash, expires); - - return commitments; - } - /// @inheritdoc IOnChainAllocation function prepareAllocation( address recipient, @@ -295,39 +270,6 @@ contract OnChainAllocator is IOnChainAllocator, Utility { emit Allocated(recipient, commitments, nonce, expiration, claimHash); } - function _executeAllocation( - uint256 nonce, - address recipient, - uint256[2][] calldata idsAndAmounts, - address arbiter, - uint32 expires, - bytes32 typehash, - bytes32 witness - ) private returns (bytes32, Lock[] memory) { - (bytes32 claimHash, Lock[] memory commitments) = - AL.executeAllocation(nonce, recipient, idsAndAmounts, arbiter, expires, typehash, witness); - - // External allocation requires to normalize the expiration time - expires = _normalizeExpiration(expires); - - // Allocate the claim - for (uint256 i = 0; i < commitments.length; i++) { - // Check the amount fits in the supported range - if (commitments[i].amount > type(uint224).max) { - revert InvalidAmount(commitments[i].amount); - } - - // Store the allocation - _storeAllocatedBalance( - commitments[i].lockTag, commitments[i].token, recipient, uint224(commitments[i].amount), expires - ); - } - // Store the claim - _storeClaim(claimHash, expires); - - return (claimHash, commitments); - } - /// @inheritdoc IAllocator function attest(address, address from_, address, uint256 id_, uint256 amount_) external returns (bytes4) { // Can be called by anyone, as this will only clean up expired allocations. @@ -460,36 +402,62 @@ contract OnChainAllocator is IOnChainAllocator, Utility { return (claimHash, nonce); } - function _checkInput(Lock calldata commitment, address sponsor, uint32 expires, uint256 minResetPeriod) - internal - view - returns (uint256) - { - // Check the allocator id fits this allocator - if (AL.splitAllocatorId(commitment.lockTag) != ALLOCATOR_ID) { - revert InvalidAllocator(AL.splitAllocatorId(commitment.lockTag), ALLOCATOR_ID); - } + function _executeAllocation( + uint256 nonce, + address recipient, + uint256[2][] calldata idsAndAmounts, + address arbiter, + uint32 expires, + bytes32 typehash, + bytes32 witness + ) private returns (bytes32, Lock[] memory) { + (bytes32 claimHash, Lock[] memory commitments) = + AL.executeAllocation(nonce, recipient, idsAndAmounts, arbiter, expires, typehash, witness); - // Check the amount fits in the supported range - if (commitment.amount > type(uint224).max) { - revert InvalidAmount(commitment.amount); - } + // External allocation requires to normalize the expiration time + expires = _normalizeExpiration(expires); - // Get the reset period for the token id - uint256 duration = AL.toSeconds(commitment.lockTag); - if (duration < minResetPeriod) { - minResetPeriod = duration; + // Allocate the claim + for (uint256 i = 0; i < commitments.length; i++) { + // Check the amount fits in the supported range + if (commitments[i].amount > type(uint224).max) { + revert InvalidAmount(commitments[i].amount); + } + + // Store the allocation + _storeAllocatedBalance( + commitments[i].lockTag, commitments[i].token, recipient, uint224(commitments[i].amount), expires + ); } + // Store the claim + _storeClaim(claimHash, expires); - // Ensure no forcedWithdrawal is active for the token id - (, uint256 forcedWithdrawal) = ITheCompact(AL.THE_COMPACT).getForcedWithdrawalStatus( - sponsor, AL.toId(commitment.lockTag, commitment.token) - ); - if (forcedWithdrawal != 0 && forcedWithdrawal <= expires) { - revert ForceWithdrawalAvailable(expires, forcedWithdrawal); + return (claimHash, commitments); + } + + function _updateCommitmentsAndStoreAllocation( + address recipient, + uint256[] memory registeredAmounts, + Lock[] memory commitments, + uint32 expires, + bytes32 claimHash + ) private returns (Lock[] memory) { + // External allocation requires to normalize the expiration time + expires = _normalizeExpiration(expires); + // Store the allocation + for (uint256 i = 0; i < registeredAmounts.length; i++) { + // Update the allocations with the actual registered amounts + uint224 amount = uint224(registeredAmounts[i]); + commitments[i].amount = amount; + + // Store the allocation + _storeAllocatedBalance(commitments[i].lockTag, commitments[i].token, recipient, amount, expires); } - return minResetPeriod; + // Store the claim + _storeClaim(claimHash, expires); + + return commitments; } function _checkBalance(address sponsor, Lock calldata commitment, uint32 expires) @@ -510,36 +478,6 @@ contract OnChainAllocator is IOnChainAllocator, Utility { } } - function _normalizeExpiration(uint32 expires) private view returns (uint32 normalizedExpires) { - uint256 timeRemaining = expires - block.timestamp; - if (timeRemaining < 10 minutes) { - // No rounding - max size of 600 unique expirations - normalizedExpires = expires; - } else if (timeRemaining < 1 hours + 5 minutes) { - // total of 55 minutes - // Round up to the nearest 10 seconds - max size of 330 unique expirations - normalizedExpires = (expires / 10 seconds) * 10 seconds + 10 seconds; - } else if (timeRemaining < 1 days) { - // total of 1,375 minutes - // Round up to the nearest minute - max size of 1375 unique expirations - normalizedExpires = (expires / 1 minutes) * 1 minutes + 1 minutes; - } else if (timeRemaining < 1 weeks + 1 hours) { - // total of 8,700 minutes - // Round up to the nearest 10 minutes - max size of 870 unique expirations - normalizedExpires = (expires / 10 minutes) * 10 minutes + 10 minutes; - } else { - // < 30 days - total of 33,060 minutes - // Round up to the nearest hour - max size of 551 unique expirations - normalizedExpires = (expires / 1 hours) * 1 hours + 1 hours; - } - // Total max of 3,726 unique expirations => 7,824,600 max gas costs for cold storage reads - - // Sanitize expiration - assembly ("memory-safe") { - normalizedExpires := and(normalizedExpires, _UINT32_MAX) - } - } - function _storeClaim(bytes32 claimHash, uint32 normalizedExpiration) private { uint32 currentExpiration = _allocatedClaims[claimHash]; if (currentExpiration > 0) { @@ -689,19 +627,6 @@ contract OnChainAllocator is IOnChainAllocator, Utility { ); } - function _generatePointer(bytes28 tokenHash, uint32 expiration) private pure returns (bytes32 pointer) { - // Make sure the expiration is the most significant 32 bits - assembly ("memory-safe") { - pointer := or(tokenHash, expiration) - } - } - - function _verifyClaim(bytes32 claimHash) private view returns (bool verified, uint32 normalizedExpiration) { - // Check if the claim is allocated - normalizedExpiration = _allocatedClaims[claimHash]; - return (normalizedExpiration != 0, normalizedExpiration); - } - function _deleteAllocatedBalance(bytes28 tokenHash, uint32 normalizedExpiration, uint224 amount, uint32 hint) private { @@ -800,6 +725,81 @@ contract OnChainAllocator is IOnChainAllocator, Utility { } } + function _checkInput(Lock calldata commitment, address sponsor, uint32 expires, uint256 minResetPeriod) + internal + view + returns (uint256) + { + // Check the allocator id fits this allocator + if (AL.splitAllocatorId(commitment.lockTag) != ALLOCATOR_ID) { + revert InvalidAllocator(AL.splitAllocatorId(commitment.lockTag), ALLOCATOR_ID); + } + + // Check the amount fits in the supported range + if (commitment.amount > type(uint224).max) { + revert InvalidAmount(commitment.amount); + } + + // Get the reset period for the token id + uint256 duration = AL.toSeconds(commitment.lockTag); + if (duration < minResetPeriod) { + minResetPeriod = duration; + } + + // Ensure no forcedWithdrawal is active for the token id + (, uint256 forcedWithdrawal) = ITheCompact(AL.THE_COMPACT).getForcedWithdrawalStatus( + sponsor, AL.toId(commitment.lockTag, commitment.token) + ); + if (forcedWithdrawal != 0 && forcedWithdrawal <= expires) { + revert ForceWithdrawalAvailable(expires, forcedWithdrawal); + } + + return minResetPeriod; + } + + function _verifyClaim(bytes32 claimHash) private view returns (bool verified, uint32 normalizedExpiration) { + // Check if the claim is allocated + normalizedExpiration = _allocatedClaims[claimHash]; + return (normalizedExpiration != 0, normalizedExpiration); + } + + function _normalizeExpiration(uint32 expires) private view returns (uint32 normalizedExpires) { + uint256 timeRemaining = expires - block.timestamp; + if (timeRemaining < 10 minutes) { + // No rounding - max size of 600 unique expirations + normalizedExpires = expires; + } else if (timeRemaining < 1 hours + 5 minutes) { + // total of 55 minutes + // Round up to the nearest 10 seconds - max size of 330 unique expirations + normalizedExpires = (expires / 10 seconds) * 10 seconds + 10 seconds; + } else if (timeRemaining < 1 days) { + // total of 1,375 minutes + // Round up to the nearest minute - max size of 1375 unique expirations + normalizedExpires = (expires / 1 minutes) * 1 minutes + 1 minutes; + } else if (timeRemaining < 1 weeks + 1 hours) { + // total of 8,700 minutes + // Round up to the nearest 10 minutes - max size of 870 unique expirations + normalizedExpires = (expires / 10 minutes) * 10 minutes + 10 minutes; + } else { + // < 30 days - total of 33,060 minutes + // Round up to the nearest hour - max size of 551 unique expirations + normalizedExpires = (expires / 1 hours) * 1 hours + 1 hours; + } + // Total max of 3,726 unique expirations => 7,824,600 max gas costs for cold storage reads + + // Sanitize expiration + assembly ("memory-safe") { + normalizedExpires := and(normalizedExpires, _UINT32_MAX) + } + } + + function _generatePointer(bytes28 tokenHash, uint32 expiration) private pure returns (bytes32 pointer) { + // Make sure the expiration is the most significant 32 bits + assembly ("memory-safe") { + pointer := or(tokenHash, expiration) + } + } + function _getTokenHash(bytes12 lockTag, address token, address sponsor) private pure returns (bytes28 tokenHash) { assembly ("memory-safe") { mstore(0x00, lockTag)