From 1c42b7c7d7768afe4998931a88bbc1f4ca5e157f Mon Sep 17 00:00:00 2001 From: vimageDE Date: Tue, 19 Nov 2024 17:07:42 +0100 Subject: [PATCH] Comments and documentation to deposits --- src/TheCompact.sol | 49 ++++++++++++++++++-------- src/interfaces/ITheCompact.sol | 4 ++- src/lib/ConstructorLogic.sol | 4 +-- src/lib/DepositLogic.sol | 16 ++++++--- src/lib/DirectDepositLogic.sol | 32 +++++++++++++++-- src/lib/IdLib.sol | 63 +++++++++++++++++++++++++++++++--- src/lib/RegistrationLib.sol | 32 +++++++++++++---- src/types/ResetPeriod.sol | 1 + src/types/Scope.sol | 1 + 9 files changed, 166 insertions(+), 36 deletions(-) diff --git a/src/TheCompact.sol b/src/TheCompact.sol index 0fc8a18..dbe8587 100644 --- a/src/TheCompact.sol +++ b/src/TheCompact.sol @@ -25,36 +25,54 @@ import { ISignatureTransfer } from "permit2/src/interfaces/ISignatureTransfer.so * This contract has not yet been properly tested, audited, or reviewed. */ contract TheCompact is ITheCompact, ERC6909, TheCompactLogic { + // NOTIFY READERS OF THE NATSPEC FOR THE FUNCTIONS IN THE INTERFACE function deposit(address allocator) external payable returns (uint256) { - return _performBasicNativeTokenDeposit(allocator); + // COMPLETED PR + return _performBasicNativeTokenDeposit(allocator); // DirectDepositLogic.sol } + // THE CLAIM REGISTERED IS NOT A SECRET IN THIS CASE, SO COULD WE VERIFY THE CLAIMHASH? function depositAndRegister(address allocator, bytes32 claimHash, bytes32 typehash) external payable returns (uint256 id) { - id = _performBasicNativeTokenDeposit(allocator); + // COMPLETED PR + id = _performBasicNativeTokenDeposit(allocator); // DirectDepositLogic.sol - _registerWithDefaults(claimHash, typehash); + _registerWithDefaults(claimHash, typehash); // RegistrationLogic.sol } + // COULD ALSO BE USED FOR NATIVE TOKENS TO REDUCE NUMBER OF FUNCTIONS - 'AMOUNT' COULD BE USED TO VERIFY THE INTENDED AMOUNT OF NATIVE TOKENS TO BE DEPOSITED function deposit(address token, address allocator, uint256 amount) external returns (uint256) { - return _performBasicERC20Deposit(token, allocator, amount); + // Completed PR + return _performBasicERC20Deposit(token, allocator, amount); // DirectDepositLogic.sol } + // THE CLAIM REGISTERED IS NOT A SECRET IN THIS CASE, SO COULD WE VERIFY THE CLAIMHASH? function depositAndRegister(address token, address allocator, uint256 amount, bytes32 claimHash, bytes32 typehash) external returns (uint256 id) { - id = _performBasicERC20Deposit(token, allocator, amount); + // Completed PR + id = _performBasicERC20Deposit(token, allocator, amount); // DirectDepositLogic.sol - _registerWithDefaults(claimHash, typehash); + _registerWithDefaults(claimHash, typehash); // RegistrationLogic.sol } + // HAVING THE RECIPIENT AS A PARAMETER ALLOWS US TO OUTSOURCE depositAndRegister FUNCTIONS TO HELPER CONTRACTS + // IF WE USE A CALLBACK FOR THE USER TO SEND IN THE TOKENS (LIKE IN UNISWAP V3), WE SAVE ON GAS BY SKIPPING APPROVAL FOR THIS CONTRACT. + // THE CONTRACT FEELS A LITTLE LIKE UNISWAP V3 CORE AND PERIPHERY CONTRACTS WERE COMBINED IN ONE. COULD BE INTERESTING TO EXPLORE + // WHERE TO SEPERATE THE LOGIC TO KEEP IT EASY TO READ AND MODULAR AT THE SAME TIME. function deposit(address allocator, ResetPeriod resetPeriod, Scope scope, address recipient) external payable returns (uint256) { - return _performCustomNativeTokenDeposit(allocator, resetPeriod, scope, recipient); + // Completed PR + // COULD ELIMINATE A LOT OF THE FUNCTIONS IN THE DEPOSIT CONTRACTS BY HANDELING DEFAULT PARAMETERS IN THIS CONTRACT. COULD INCREASE READABILITY. + return _performCustomNativeTokenDeposit(allocator, resetPeriod, scope, recipient); // DirectDepositLogic.sol } function deposit(address token, address allocator, ResetPeriod resetPeriod, Scope scope, uint256 amount, address recipient) external returns (uint256) { - return _performCustomERC20Deposit(token, allocator, resetPeriod, scope, amount, recipient); + // Completed PR + // COULD ELIMINATE A LOT OF THE FUNCTIONS IN THE DEPOSIT CONTRACTS BY HANDELING DEFAULT PARAMETERS IN THIS CONTRACT. COULD INCREASE READABILITY. + return _performCustomERC20Deposit(token, allocator, resetPeriod, scope, amount, recipient); // DirectDepositLogic.sol } - function deposit(uint256[2][] calldata idsAndAmounts, address recipient) external payable returns (bool) { - _processBatchDeposit(idsAndAmounts, recipient); + // IN HERE, WE ARE ACTUALLY COMBINING NATIVE AND ERC20 TOKENS IN THE SAME FUNCTION, WE CAN DO THE SAME ABOVE. + function deposit(uint256[2][] calldata idsAndAmounts, address recipient) external payable returns (bool) { + // Completed PR + _processBatchDeposit(idsAndAmounts, recipient); // DirectDepositLogic.sol return true; } @@ -62,10 +80,10 @@ contract TheCompact is ITheCompact, ERC6909, TheCompactLogic { function depositAndRegister(uint256[2][] calldata idsAndAmounts, bytes32[2][] calldata claimHashesAndTypehashes, uint256 duration) external payable returns (bool) { _processBatchDeposit(idsAndAmounts, msg.sender); - return _registerBatch(claimHashesAndTypehashes, duration); + return _registerBatch(claimHashesAndTypehashes, duration); // NEED TO LOOK INTO THIS } - function deposit( + function deposit( // RENAME TO depositViaPermit2 FOR CLEAR SEPARATION? address token, uint256, // amount uint256, // nonce @@ -76,11 +94,12 @@ contract TheCompact is ITheCompact, ERC6909, TheCompactLogic { Scope, //scope address recipient, bytes calldata signature - ) external returns (uint256) { - return _depositViaPermit2(token, recipient, signature); + ) external returns (uint256) { // Completed PR + return _depositViaPermit2(token, recipient, signature); // DepositViaPermit2Logic.sol } - function depositAndRegister( + // ITS HARD TO FOLLOW A PATTERN HERE... DEPOSIT AND REGISTER VIA PERMIT2 HAS SUCH A DIFFERENT INTERNAL PROCESS COMPARED TO ONLY DEPOSITING (NOT JUST THE REGISTERING PART ADDED). + function depositAndRegister( // RENAME TO depositAndRegisterViaPermit2 FOR CLEAR SEPARATION? address token, uint256, // amount uint256, // nonce diff --git a/src/interfaces/ITheCompact.sol b/src/interfaces/ITheCompact.sol index c3056fa..6c10b3a 100644 --- a/src/interfaces/ITheCompact.sol +++ b/src/interfaces/ITheCompact.sol @@ -57,6 +57,8 @@ interface ITheCompact { */ event AllocatorRegistered(uint96 allocatorId, address allocator); + // COULD WE ADD ALL THE CUSTOM ERRORS HERE FOR CLARITY, (EVEN THOUGH THE CONTRACT REVERTS IN ASSEMBLY)? + /** * @notice External payable function for depositing native tokens into a resource lock * and receiving back ERC6909 tokens representing the underlying locked balance controlled @@ -143,7 +145,7 @@ interface ITheCompact { /** * @notice External payable function for depositing multiple tokens in a single transaction. - * The first entry in idsAndAmounts can optionally represent native tokens by providing the + * The first entry (and ONLY the first entry) in idsAndAmounts can optionally represent native tokens by providing the * null address and an amount matching msg.value. For ERC20 tokens, the caller must directly * approve The Compact to transfer sufficient amounts on its behalf. The ERC6909 token amounts * received by the recipient are derived from the differences between starting and ending diff --git a/src/lib/ConstructorLogic.sol b/src/lib/ConstructorLogic.sol index a74858b..29c7573 100644 --- a/src/lib/ConstructorLogic.sol +++ b/src/lib/ConstructorLogic.sol @@ -26,7 +26,7 @@ contract ConstructorLogic is Tstorish { using IdLib for uint256; // Address of the Permit2 contract, optionally used for depositing ERC20 tokens. - address private constant _PERMIT2 = 0x000000000022D473030F116dDEE9F6B43aC78BA3; + address private constant _PERMIT2 = 0x000000000022D473030F116dDEE9F6B43aC78BA3; // BETTER AS IMMUTABLE TO BE MORE FLEXIBLE WITH OTHER CHAINS // Storage slot used for the reentrancy guard, whether using TSTORE or SSTORE. uint256 private constant _REENTRANCY_GUARD_SLOT = 0x929eee149b4bd21268; @@ -41,7 +41,7 @@ contract ConstructorLogic is Tstorish { MetadataRenderer private immutable _METADATA_RENDERER; // Whether Permit2 was deployed on the chain at construction time. - bool private immutable _PERMIT2_INITIALLY_DEPLOYED; + bool private immutable _PERMIT2_INITIALLY_DEPLOYED; // IF WE MAKE _PERMIT2 AN IMMUTABLE, WE CAN JUST CHECK IF _PERMIT2 IS NOT ADDRESS ZERO /** * @notice Constructor that initializes immutable variables and deploys the metadata diff --git a/src/lib/DepositLogic.sol b/src/lib/DepositLogic.sol index a14cc8c..d858bde 100644 --- a/src/lib/DepositLogic.sol +++ b/src/lib/DepositLogic.sol @@ -14,7 +14,7 @@ contract DepositLogic is ConstructorLogic { using SafeTransferLib for address; // Storage slot seed for ERC6909 state, used in computing balance slots. - uint256 private constant _ERC6909_MASTER_SLOT_SEED = 0xedcaa89a82293940; + uint256 private constant _ERC6909_MASTER_SLOT_SEED = 0xedcaa89a82293940; // WHERE IS THIS COMING FROM? // keccak256(bytes("Transfer(address,address,address,uint256,uint256)")). uint256 private constant _TRANSFER_EVENT_SIGNATURE = 0x1b3d7edb2e9c0b0e7c525b20aaaef0f5940d2ed71663c7d39266ecafac728859; @@ -36,7 +36,7 @@ contract DepositLogic is ConstructorLogic { // Revert if the balance hasn't increased. assembly ("memory-safe") { if iszero(lt(initialBalance, tokenBalance)) { - // revert InvalidDepositBalanceChange() + // revert InvalidDepositBalanceChange() // COULD PROVIDE INFORMATION ABOUT INIT AND NEW TOKEN BALANCE IN THE ERROR mstore(0, 0x426d8dcf) revert(0x1c, 0x04) } @@ -60,9 +60,14 @@ contract DepositLogic is ConstructorLogic { function _deposit(address to, uint256 id, uint256 amount) internal { assembly ("memory-safe") { // Compute the recipient's balance slot using the master slot seed. - mstore(0x20, _ERC6909_MASTER_SLOT_SEED) - mstore(0x14, to) - mstore(0x00, id) + mstore(0x20, _ERC6909_MASTER_SLOT_SEED) // length of 64 bits + mstore(0x14, to) // Length of 160 bits + mstore(0x00, id) // length of 256 bits + // -----------SLOT 1----------- -----------SLOT 2----------- + // master: | - 256 bytes - | [0000000000000000000][--64 bits--] + // to: | - 160 bytes - [[0000] | [---160 bits---]] + // id: | [---------256 bits---------] | - 256 bytes - + let toBalanceSlot := keccak256(0x00, 0x40) // Load current balance and compute new balance. @@ -87,6 +92,7 @@ contract DepositLogic is ConstructorLogic { mstore(0x00, caller()) mstore(0x20, amount) log4(0, 0x40, _TRANSFER_EVENT_SIGNATURE, 0, shr(0x60, shl(0x60, to)), id) + // IS IT REQUIRED TO SANITIZE AN INTERNAL INPUT? } } } diff --git a/src/lib/DirectDepositLogic.sol b/src/lib/DirectDepositLogic.sol index 721ee5b..37a6f1a 100644 --- a/src/lib/DirectDepositLogic.sol +++ b/src/lib/DirectDepositLogic.sol @@ -33,6 +33,7 @@ contract DirectDepositLogic is DepositLogic { * default reset period (ten minutes) and scope (multichain) will be used for the resource * lock. The ERC6909 token amount received by the caller will match the amount of native * tokens sent with the transaction. + * @dev will revert if the allocator address was not previously registered * @param allocator The address of the allocator. * @return id The ERC6909 token identifier of the associated resource lock. */ @@ -46,7 +47,7 @@ contract DirectDepositLogic is DepositLogic { /** * @notice Internal function for depositing multiple tokens in a single transaction. The - * first entry in idsAndAmounts can optionally represent native tokens by providing the null + * first entry (and ONLY the first entry) in idsAndAmounts can optionally represent native tokens by providing the null * address and an amount matching msg.value. For ERC20 tokens, the caller must directly * approve The Compact to transfer sufficient amounts on its behalf. The ERC6909 token amounts * received by the recipient are derived from the differences between starting and ending @@ -74,14 +75,29 @@ contract DirectDepositLogic is DepositLogic { // Load the first ID from idsAndAmounts. id := calldataload(idsAndAmountsOffset) + // Example of how the calldata is structured and the difference between the array pointer and .offset in assembly. + // + // The example calldata for this function has the following inputs: + // idsAndAmounts = [[1, 2],[3, 4]], recipient = 0x3482f06bbd1cc4ae5b13b3f63a5373752a7819e0 + // + // 0x5943672c| Function selector + // 0...0000000000000000000000000000000000000040| Pointer to idsAndAmounts + // 0...3482f06bbd1cc4ae5b13b3f63a5373752a7819e0| Recipient + // 0...0000000000000000000000000000000000000002| Length of idsAndAmounts <--- Pointer to idsAndAmounts + // 0...0000000000000000000000000000000000000001| First ID <--- idsAndAmounts.offset points here + // 0...0000000000000000000000000000000000000002| First Amount + // 0...0000000000000000000000000000000000000003| Second ID + // 0...0000000000000000000000000000000000000004| Second Amount + // Determine if token encoded in first ID is the null address. firstUnderlyingTokenIsNative := iszero(shr(96, shl(96, id))) + // shift 96 to left to clear [1 bit scope][3 bits resetPeriod][92 bits allocatorId], so only [160 bits token] remains // Revert if: // * the array is empty // * the callvalue is zero but the first token is native // * the callvalue is nonzero but the first token is non-native - // * the first token is non-native and the callvalue doesn't equal the first amount + // * the first token is native and the callvalue doesn't equal the first amount if or(iszero(totalIds), or(eq(firstUnderlyingTokenIsNative, iszero(callvalue())), and(firstUnderlyingTokenIsNative, iszero(eq(callvalue(), calldataload(add(idsAndAmountsOffset, 0x20))))))) { // revert InvalidBatchDepositStructure() mstore(0, 0xca0fc08e) @@ -103,6 +119,7 @@ contract DirectDepositLogic is DepositLogic { // Iterate over remaining IDs and amounts. unchecked { for (uint256 i = firstUnderlyingTokenIsNative.asUint256(); i < totalIds; ++i) { + // LOVE THAT BOOL TO UINT USAGE // Navigate to the current ID and amount pair in calldata. uint256[2] calldata idAndAmount = idsAndAmounts[i]; @@ -110,6 +127,15 @@ contract DirectDepositLogic is DepositLogic { id = idAndAmount[0]; amount = idAndAmount[1]; + // ADDITIONAL CHECK TO REVERT IF ID OR AMOUNT IS 0 + assembly ("memory-safe") { + if or(iszero(id), iszero(amount)) { + // revert InvalidBatchDepositStructure() + mstore(0, 0xca0fc08e) + revert(0x1c, 0x04) + } + } + // Derive new allocator ID from current resource lock ID. newAllocatorId = id.toAllocatorId(); @@ -138,6 +164,8 @@ contract DirectDepositLogic is DepositLogic { * default reset period (ten minutes) and scope (multichain) will be used for the resource * lock. The ERC6909 token amount received by the caller will match the amount of native * tokens sent with the transaction. + * @dev will revert if the allocator address was not previously registered + * @dev will revert if the token is address 0 (which is used to represent native tokens) * @param allocator The address of the allocator. * @return id The ERC6909 token identifier of the associated resource lock. */ diff --git a/src/lib/IdLib.sol b/src/lib/IdLib.sol index 83d2db8..b170c31 100644 --- a/src/lib/IdLib.sol +++ b/src/lib/IdLib.sol @@ -38,7 +38,7 @@ library IdLib { error AllocatorAlreadyRegistered(uint96 allocatorId, address allocator); // Storage slot seed for mapping allocator IDs to allocator addresses. - uint256 private constant _ALLOCATOR_BY_ALLOCATOR_ID_SLOT_SEED = 0x000044036fc77deaed2300000000000000000000000; + uint256 private constant _ALLOCATOR_BY_ALLOCATOR_ID_SLOT_SEED = 0x000044036fc77deaed2300000000000000000000000; // WHERE IS THIS COMING FROM? // keccak256(bytes("AllocatorRegistered(uint96,address)")). uint256 private constant _ALLOCATOR_REGISTERED_EVENT_SIGNATURE = 0xc54dcaa67a8fd7b4a9aa6fd57351934c792613d5ec1acbd65274270e6de8f7e4; @@ -96,11 +96,20 @@ library IdLib { * @return id The derived resource lock ID. */ function toIdIfRegistered(address token, Scope scope, ResetPeriod resetPeriod, address allocator) internal view returns (uint256 id) { - // Derive the allocator ID for the provided allocator address. + // Derive the allocator ID for the provided allocator address. Revert if not registered uint96 allocatorId = allocator.toAllocatorIdIfRegistered(); // Derive resource lock ID (pack scope, reset period, allocator ID, & token). id = ((scope.asUint256() << 255) | (resetPeriod.asUint256() << 252) | (allocatorId.asUint256() << 160) | token.asUint256()); + // [ 1 bit ][ 3 bits ][ 92 bits ][ 160 bits ] + // [ scope ][resetPeriod ][ allocatorId ][ token ] + // + // The scope is an enum with two choices, so 1 bit in size + // The reset period is an enum with eight choices, so 3 bits in size + // The allocator id is compacted to 92 bits (the first 4 bits of the uint96 are not set) + // The token is an address (160 bits) + // + // Combined this fits in 256 bits } /** @@ -164,6 +173,7 @@ library IdLib { assembly ("memory-safe") { // NOTE: consider an SLOAD bypass for a fully compact allocator if iszero(sload(or(_ALLOCATOR_BY_ALLOCATOR_ID_SLOT_SEED, allocatorId))) { + // ARE WE FINE TO JUST CHECK IF NON ZERO, IF PREVIOUSLY WE CHECK FOR THE ACTUAL VALUE? mstore(0, _NO_ALLOCATOR_REGISTERED_ERROR_SIGNATURE) mstore(0x20, allocatorId) revert(0x1c, 0x24) @@ -277,7 +287,7 @@ library IdLib { * allocator, but is represented by a uint96 as solidity only supports uint values * for multiples of 8 bits. * @param id The resource lock ID to extract from. - * @return allocatorId The allocator ID (bits 160-251). + * @return allocatorId The allocator ID (bits 4-96). */ function toAllocatorId(uint256 id) internal pure returns (uint96 allocatorId) { assembly ("memory-safe") { @@ -344,12 +354,57 @@ library IdLib { // Look up final value in the sequence. compactFlag := and(shr(and(sub(72, and(y, 127)), not(3)), 0xfedcba9876543210000), 15) + + // Example allocator address: 0x00000000044442D64A0BE733A5f2a3187BFA8234 + // including 32 bytes padding: 0x000000000000000000000000|00000000044442D64A0BE733A5f2a3187BFA8234 + // shl(96, allocator) 0x00000000044442D64A0BE733A5f2a3187BFA8234|000000000000000000000000 + // x := shr(168, [...]) 0x000000000000000000000000000000000000000000|00000000044442D64A0BE7 // => THIS IS WRONG, WE EXTRACT THE UPPERMOST 88 BITS, INSTEAD OF 72 + // => CAN INPUT IN AN INTERNAL FUNCTION BE DIRTY IN SOLIDITY? + + // We now have the uppermost 88 (? SHOULD BE 72) bits of the address as "x". We now work with these, and propagate the highest set bit + // These are the orig bits: 0000 0000 0000 0000 0000 0000 0000 0000 0000 0100 0100 0100 0100 0010 1101 0110 0100 1010 0000 1011 (1110 0111 - Beccause 88 Bits instead of 72) + // shr(1, x) 0000 0000 0000 0000 0000 0000 0000 0000 0000 0010 0010 0010 0010 0001 0110 1011 0010 0101 0000 0101 + // x := or(x, [...]) 0000 0000 0000 0000 0000 0000 0000 0000 0000 0110 0110 0110 0110 0011 1111 1111 0110 1111 0000 1111 + // shr(2, x) 0000 0000 0000 0000 0000 0000 0000 0000 0000 0001 1001 1001 1001 1000 1111 1111 1101 1011 1100 0011 + // x := or(x, [...]) 0000 0000 0000 0000 0000 0000 0000 0000 0000 0111 1111 1111 1111 1011 1111 1111 1111 1111 1100 1111 + // ... + // x := or(x, shr(32, x)) 0000 0000 0000 0000 0000 0000 0000 0000 0000 0111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 + // => THE 32 BIT SHIFT IS NOT SUFFICIENT FOR ADDRESSES LIKE: 0x800000000000000000|3BE733A5f2a3187BFA8234 + + // We now count the set bits to derive the most significant bit in the last byte (using the parallel bit counting SWAR ?? algorithm) + // shr(1, x) 0000 0000 0000 0000 0000 0000 0000 0000 0000 0011 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 + // now adding 0x5555...555 0x5555555555555555 in bits is: + // 0000 0000 0000 0000 0101 0101 0101 0101 0101 0101 0101 0101 0101 0101 0101 0101 0101 0101 0101 0101 + // and([...], 0x5555...555) 0000 0000 0000 0000 0000 0000 0000 0000 0000 0001 0101 0101 0101 0101 0101 0101 0101 0101 0101 0101 | + // y := sub(x, [...]) 0000 0000 0000 0000 0000 0000 0000 0000 0000 0110 1010 1010 1010 1010 1010 1010 1010 1010 1010 1010 | (1010 1010) + // + // shr(2, y) 0000 0000 0000 0000 0000 0000 0000 0000 0000 0001 1010 1010 1010 1010 1010 1010 1010 1010 1010 1010 | + // now adding 0x3333...333 0x3333333333333333 in bits is: + // 0000 0000 0000 0000 0011 0011 0011 0011 0011 0011 0011 0011 0011 0011 0011 0011 0011 0011 0011 0011 + // and([...], 0x3333...333) 0000 0000 0000 0000 0000 0000 0000 0000 0000 0001 0010 0010 0010 0010 0010 0010 0010 0010 0010 0010 | + // ... + // y := 0000 0000 0000 0000 0000 0000 0000 0000 0000 0011 0100 0100 0100 0100 0100 0100 0100 0100 0100 0100 | (0100 0100) + // + // ... + // y := 0000 0000 0000 0000 0000 0000 0000 0000 0000 0011 0000 1000 0000 1000 0000 1000 0000 1000 0000 1000 | (0000 1000) + // + // ... + // y := 0000 0000 0000 0000 0000 0000 0000 0000 0000 0011 0000 1011 0001 0000 0001 0000 0001 0000 0001 0000 | (0001 0000) + // + // ... + // y := 0000 0000 0000 0000 0000 0000 0000 0000 0000 0011 0000 1011 0001 0011 0001 1011 0010 0000 0010 0000 | (0010 0000) + // + // ... + // y := 0000 0000 0000 0000 0000 0000 0000 0000 0000 0011 0000 1011 0001 0011 0001 1011 0010 0011 0010 1011 | (0011 0011) + // + // ... + // Final compact flag: uint8(2) (FALSE), FIXING THE FIRST LINE (shr 168 -> 184) FIXED IT, RESULT IS NOW 6 } } /** * @notice Internal pure function for computing an allocator's ID from their address. - * Combines the compact flag (4 bits) with the last 88 bits of the address. + * Combines the compact flag (4 bits) with the last 88 bits of the address. // WHY IS THE LAST 88 BITS OF THE ADDRESS + THE compactFlag SUFFICIENT TO BE UNIQUE? * @param allocator The address to compute the ID for. * @return allocatorId The computed allocator ID. */ diff --git a/src/lib/RegistrationLib.sol b/src/lib/RegistrationLib.sol index 20f37fb..148dfff 100644 --- a/src/lib/RegistrationLib.sol +++ b/src/lib/RegistrationLib.sol @@ -40,19 +40,20 @@ library RegistrationLib { let m := mload(0x40) // Pack data for deriving active registration storage slot. - mstore(add(m, 0x14), sponsor) - mstore(m, _ACTIVE_REGISTRATIONS_SCOPE) - mstore(add(m, 0x34), claimHash) - mstore(add(m, 0x54), typehash) + mstore(add(m, 0x14), sponsor) // 160 bits length at 160 bits + 96 empty bits = 256-416 bits + mstore(m, _ACTIVE_REGISTRATIONS_SCOPE) // 32 bits length at 0 bits + 224 empty bits = 224-256 bits + mstore(add(m, 0x34), claimHash) // 256 bits length at 416 bits + 0 empty bits = 416-672 bits + mstore(add(m, 0x54), typehash) // 256 bits length at 672 bits + 0 empty bits = 672-928 bits // Derive and load active registration storage slot to get current expiration. - let cutoffSlot := keccak256(add(m, 0x1c), 0x58) + let cutoffSlot := keccak256(add(m, 0x1c), 0x58) // hash bits 224-928 // Compute new expiration based on current timestamp and supplied duration. let expires := add(timestamp(), duration) - // Ensure new expiration does not exceed current and duration does not exceed 30 days. + // Ensure new expiration is not earlier than current and duration does not exceed 30 days. if or(lt(expires, sload(cutoffSlot)), gt(duration, 0x278d00)) { + // LETS MAYBE MOVE THE HARDCODED 30 days max EXPIRATION TO A CONSTANT FOR EASY ADAPTATION? // revert InvalidRegistrationDuration(uint256 duration) mstore(0, 0x1f9a96f4) mstore(0x20, duration) @@ -69,6 +70,23 @@ library RegistrationLib { mstore(add(m, 0x74), expires) log2(add(m, 0x34), 0x60, _COMPACT_REGISTERED_SIGNATURE, shr(0x60, shl(0x60, sponsor))) } + + // // SOLIDITY CODE: + // // gas efficiency assembly code: 25_259 gas + // // gas efficiency solidity code: 26_537 gas + // bytes32 cutoffSlot = keccak256(abi.encodePacked(bytes4(bytes32(_ACTIVE_REGISTRATIONS_SCOPE)), sponsor, claimHash, typehash)); + // uint256 currentExpiration; + // assembly ("memory-safe") { + // currentExpiration := sload(cutoffSlot) + // } + // uint256 expires = block.timestamp + duration; + // if (expires < currentExpiration || duration > 30 days) { + // revert InvalidRegistrationDuration(duration); + // } + // assembly ("memory-safe") { + // sstore(cutoffSlot, expires) + // } + // emit CompactRegistered(sponsor, claimHash, typehash, expires); } /** @@ -90,7 +108,7 @@ library RegistrationLib { * @param typehash The EIP-712 typehash associated with the claim hash. */ function registerAsCallerWithDefaultDuration(bytes32 claimHash, bytes32 typehash) internal { - msg.sender.registerCompactWithSpecificDuration(claimHash, typehash, uint256(0x258).asStubborn()); + msg.sender.registerCompactWithSpecificDuration(claimHash, typehash, uint256(0x258).asStubborn()); // LETS MAYBE MOVE THE HARDCODED 10 MINUTES DEFAULT TO A CONSTANT FOR EASY ADAPTATION? } /** diff --git a/src/types/ResetPeriod.sol b/src/types/ResetPeriod.sol index 5bf0eca..31772c3 100644 --- a/src/types/ResetPeriod.sol +++ b/src/types/ResetPeriod.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.27; +// This enum cannot be increased. Else, the first bit will be cut off in the token id enum ResetPeriod { OneSecond, FifteenSeconds, diff --git a/src/types/Scope.sol b/src/types/Scope.sol index 842e8ef..e0a54eb 100644 --- a/src/types/Scope.sol +++ b/src/types/Scope.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.27; +// This enum cannot be increased. Else, the first bit will be cut off in the token id enum Scope { Multichain, ChainSpecific