Comments and documentation

src/TheCompact.sol

@@ -112,7 +112,7 @@ contract TheCompact is ITheCompact, ERC6909, TheCompactLogic {
         CompactCategory compactCategory,
         string calldata witness,
         bytes calldata signature
-    ) external returns (uint256) {
+    ) external returns (uint256) { // Completed PR
         return _depositAndRegisterViaPermit2(token, depositor, resetPeriod, claimHash, compactCategory, witness, signature);
     }
 
@@ -146,12 +146,16 @@ contract TheCompact is ITheCompact, ERC6909, TheCompactLogic {
         return _depositBatchAndRegisterViaPermit2(depositor, permitted, resetPeriod, claimHash, compactCategory, witness, signature);
     }
 
-    function allocatedTransfer(BasicTransfer calldata transfer) external returns (bool) {
-        return _processBasicTransfer(transfer, _release);
+    // THIS REQUIRES ALWAYS THE MSG.SENDER TO BE THE SPONSOR. WHY DO WE NOT WORK WITH A 'FROM' INPUT AND CHECK FOR AN APPROVAL?
+    function allocatedTransfer(BasicTransfer calldata transfer) external returns (bool) { // Completed PR
+        // _release is a function pointer to the _release function in the SharedLogic.sol contract
+        return _processBasicTransfer(transfer, _release); // TransferLogic.sol
     }
 
-    function allocatedWithdrawal(BasicTransfer calldata withdrawal) external returns (bool) {
-        return _processBasicTransfer(withdrawal, _withdraw);
+    // THIS REQUIRES ALWAYS THE MSG.SENDER TO BE THE SPONSOR. WHY DO WE NOT WORK WITH A 'FROM' INPUT AND CHECK FOR AN APPROVAL?
+    function allocatedWithdrawal(BasicTransfer calldata withdrawal) external returns (bool) { // Completed PR
+        // _withdraw is a function pointer to the _withdraw function in the SharedLogic.sol contract
+        return _processBasicTransfer(withdrawal, _withdraw); // TransferLogic.sol
     }
 
     function allocatedTransfer(SplitTransfer calldata transfer) external returns (bool) {
@@ -178,34 +182,34 @@ contract TheCompact is ITheCompact, ERC6909, TheCompactLogic {
         return _processSplitBatchTransfer(withdrawal, _withdraw);
     }
 
-    function enableForcedWithdrawal(uint256 id) external returns (uint256) {
-        return _enableForcedWithdrawal(id);
+    function enableForcedWithdrawal(uint256 id) external returns (uint256) { // Completed PR
+        return _enableForcedWithdrawal(id); // WithdrawalLogic.sol
     }
 
-    function disableForcedWithdrawal(uint256 id) external returns (bool) {
-        return _disableForcedWithdrawal(id);
+    function disableForcedWithdrawal(uint256 id) external returns (bool) { // Completed PR
+        return _disableForcedWithdrawal(id); // WithdrawalLogic.sol
     }
 
-    function forcedWithdrawal(uint256 id, address recipient, uint256 amount) external returns (bool) {
-        return _processForcedWithdrawal(id, recipient, amount);
+    function forcedWithdrawal(uint256 id, address recipient, uint256 amount) external returns (bool) { // Completed PR
+        return _processForcedWithdrawal(id, recipient, amount); // WithdrawalLogic.sol
     }
 
-    function register(bytes32 claimHash, bytes32 typehash, uint256 duration) external returns (bool) {
-        _register(msg.sender, claimHash, typehash, duration);
+    function register(bytes32 claimHash, bytes32 typehash, uint256 duration) external returns (bool) { // Completed PR
+        _register(msg.sender, claimHash, typehash, duration); // RegistrationLogic.sol
         return true;
     }
 
     function getRegistrationStatus(address sponsor, bytes32 claimHash, bytes32 typehash) external view returns (bool isActive, uint256 expires) {
-        expires = _getRegistrationStatus(sponsor, claimHash, typehash);
+        expires = _getRegistrationStatus(sponsor, claimHash, typehash); // RegistrationLogic.sol
         isActive = expires > block.timestamp;
-    }
+    } // Completed PR
 
     function register(bytes32[2][] calldata claimHashesAndTypehashes, uint256 duration) external returns (bool) {
         return _registerBatch(claimHashesAndTypehashes, duration);
     }
 
     function consume(uint256[] calldata nonces) external returns (bool) {
-        return _consume(nonces);
+        return _consume(nonces); // AllocatorLogic.sol
     }
 
     function __registerAllocator(address allocator, bytes calldata proof) external returns (uint96) {
@@ -216,6 +220,7 @@ contract TheCompact is ITheCompact, ERC6909, TheCompactLogic {
         return _getForcedWithdrawalStatus(account, id);
     }
 
+    // CAN WE NAME THE OUTPUT OF THIS FUNCTION TO MAKE IT CLEAR WHAT ADDRESS IS THE ALLOCATOR AND WHICH IS THE TOKEN?
     function getLockDetails(uint256 id) external view returns (address, address, ResetPeriod, Scope) {
         return _getLockDetails(id);
     }

src/interfaces/ITheCompact.sol

@@ -312,6 +312,7 @@ interface ITheCompact {
 
     /**
      * @notice Transfers ERC6909 tokens to a single recipient with allocator approval.
+     * @dev The sponsor for this transfer will always be the msg.sender.
      * @param transfer A BasicTransfer struct containing the following:
      *  -  allocatorSignature Authorization signature from the allocator.
      *  -  nonce              Parameter enforcing replay protection, scoped to the allocator.

src/lib/AllocatorLogic.sol

@@ -31,19 +31,23 @@ contract AllocatorLogic {
      * @return       Whether all nonces were successfully marked as consumed.
      */
     function _consume(uint256[] calldata nonces) internal returns (bool) {
+        // NATSPEC COMMENT STATES THIS FUNCTION IS USED IN THE CLAIM PROCESS, BUT I CAN'T FIND ANY OTHER CALLS TO IT THEN FROM TheCompact.consume()
+
         // NOTE: this may not be necessary, consider removing
         msg.sender.usingAllocatorId().mustHaveARegisteredAllocator();
 
+        // THE CALL INDEED DOES NOT SEEM TO BE NECESSARY, BUT IT IS NICE TO PREVENT SPONSORS FROM CALLING THIS FUNCTION BY MISTAKE.
+
         unchecked {
             uint256 i;
 
             assembly ("memory-safe") {
                 i := nonces.offset
             }
 
-            uint256 end = i + (nonces.length << 5);
+            uint256 end = i + (nonces.length << 5); // nonces.length << 5 = nonces.length * 32
             uint256 nonce;
-            for (; i < end; i += 0x20) {
+            for (; i < end; i += 0x20) { // loop over each nonce in the array
                 assembly ("memory-safe") {
                     nonce := calldataload(i)
                 }

src/lib/ConsumerLib.sol

@@ -9,8 +9,8 @@ pragma solidity ^0.8.27;
  */
 library ConsumerLib {
     // Storage scope identifiers for nonce buckets.
-    uint256 private constant _ALLOCATOR_NONCE_SCOPE = 0x03f37b1a;
-    uint256 private constant _SPONSOR_NONCE_SCOPE = 0x8ccd9613;
+    uint256 private constant _ALLOCATOR_NONCE_SCOPE = 0x03f37b1a; // WHERE IS THIS COMING FROM?
+    uint256 private constant _SPONSOR_NONCE_SCOPE = 0x8ccd9613; // WHERE IS THIS COMING FROM?
 
     // Error thrown when attempting to consume an already-consumed nonce.
     error InvalidNonce(address account, uint256 nonce);
@@ -74,21 +74,37 @@ library ConsumerLib {
 
             // derive the nonce bucket slot:
             // keccak256(_CONSUMER_NONCE_SCOPE ++ account ++ nonce[0:31])
-            mstore(0x20, account)
-            mstore(0x0c, scope)
-            mstore(0x40, nonce)
-            let bucketSlot := keccak256(0x28, 0x37)
+            mstore(0x20, account) // 32 bytes offset
+            mstore(0x0c, scope) // 12 bytes offset
+            mstore(0x40, nonce) // 64 bytes offset
+
+            // Memory looks now as follows:
+            //
+            // [ 40 bytes ][  4 bytes  ][  20 bytes  ][  32 bytes  ] <- memory content
+            // [    ---   ][   scope   ][   account  ][    nonce   ]
+            // [  0 bytes ][ 40 bytes  ][  44 bytes  ][  64 bytes  ] <- memory offset
+            //            _ALOC_NCE_SCPE  allocator           
+
+            // WE COULD SAFELY USE THE FIRST 56 BYTES, THIS WAY WE DO NOT HAVE TO STORE AND REWRITE THE FREE MEMORY POINTER
+
+            let bucketSlot := keccak256(0x28, 0x37) // hash from 40 bytes (0x28) to 95 bytes (55 bytes length (0x37))
 
             // Retrieve nonce bucket and check if nonce has been consumed.
             let bucketValue := sload(bucketSlot)
             let bit := shl(and(0xff, nonce), 1)
+            // We first mask the nonce with 0xff (000...0000 1111 1111) to get the last 8 bits
+            // The last 8 bits can have a value between 0 and 255.
+            // We now shift 1 (0000...0001) to the left by this value.
+            // This will result in a storage slot to be filled with up to 256 bits / nonces, rather then requiring a single slot per nonce.
+
+            // check if the bit has already been set in the bucket
             if and(bit, bucketValue) {
                 // `InvalidNonce(address,uint256)` with padding for `account`.
                 mstore(0x0c, 0xdbc205b1000000000000000000000000)
                 revert(0x1c, 0x44)
             }
 
-            // Invalidate the nonce by setting its bit.
+            // Invalidate the nonce by setting its bit. We use 'or' to set the bit additional to the existing bits/nonces.
             sstore(bucketSlot, or(bucketValue, bit))
 
             // Restore the free memory pointer.

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; // WHERE IS THIS COMING FROM?
+    uint256 private constant _ERC6909_MASTER_SLOT_SEED = 0xedcaa89a82293940; // WHERE IS THIS COMING FROM? // WE HAVE THIS TWICE, LETS STICK TO ONE SOURCE OF TRUTH
 
     // keccak256(bytes("Transfer(address,address,address,uint256,uint256)")).
     uint256 private constant _TRANSFER_EVENT_SIGNATURE = 0x1b3d7edb2e9c0b0e7c525b20aaaef0f5940d2ed71663c7d39266ecafac728859;
@@ -64,9 +64,9 @@ contract DepositLogic is ConstructorLogic {
             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 -
+            // master:  |        - 256 bits  -         | [0000000000000000000][--64 bits--]
+            // to:      |    - 160 bits  -     [[0000] | [---160 bits---]]
+            // id:      | [---------256 bits---------] |        - 256 bits  -
 
             let toBalanceSlot := keccak256(0x00, 0x40)