diff --git a/snapshots/TheCompactTest.json b/snapshots/TheCompactTest.json index 306bb13..2d205c1 100644 --- a/snapshots/TheCompactTest.json +++ b/snapshots/TheCompactTest.json @@ -1,42 +1,42 @@ { "basicTransfer": "56867", - "basicWithdrawal": "59810", - "batchClaim": "112477", - "batchClaimRegisteredWithDeposit": "112477", - "batchClaimRegisteredWithDepositWithWitness": "113236", - "batchClaimWithWitness": "113230", - "batchDepositAndRegisterViaPermit2": "221877", - "batchDepositAndRegisterWithWitnessViaPermit2": "221855", + "basicWithdrawal": "59992", + "batchClaim": "112512", + "batchClaimRegisteredWithDeposit": "112512", + "batchClaimRegisteredWithDepositWithWitness": "113271", + "batchClaimWithWitness": "113265", + "batchDepositAndRegisterViaPermit2": "222059", + "batchDepositAndRegisterWithWitnessViaPermit2": "222037", "batchTransfer": "81344", - "batchWithdrawal": "99729", - "claim": "56982", - "claimAndWithdraw": "73258", - "claimWithWitness": "59479", - "depositAndRegisterViaPermit2": "124247", - "depositBatchSingleERC20": "67868", - "depositBatchSingleNative": "28171", - "depositBatchViaPermit2NativeAndERC20": "129569", - "depositBatchViaPermit2SingleERC20": "104723", - "depositERC20AndURI": "67094", - "depositERC20Basic": "67120", - "depositERC20ViaPermit2AndURI": "98289", + "batchWithdrawal": "100093", + "claim": "56974", + "claimAndWithdraw": "73432", + "claimWithWitness": "59471", + "depositAndRegisterViaPermit2": "124429", + "depositBatchSingleERC20": "68050", + "depositBatchSingleNative": "28353", + "depositBatchViaPermit2NativeAndERC20": "129751", + "depositBatchViaPermit2SingleERC20": "104905", + "depositERC20AndURI": "67276", + "depositERC20Basic": "67302", + "depositERC20ViaPermit2AndURI": "98471", "depositETHAndURI": "26754", "depositETHBasic": "28391", - "qualifiedBatchClaim": "114099", - "qualifiedBatchClaimWithWitness": "113637", - "qualifiedClaim": "60414", - "qualifiedClaimWithWitness": "59106", - "qualifiedSplitBatchClaim": "141456", - "qualifiedSplitBatchClaimWithWitness": "141522", + "qualifiedBatchClaim": "114134", + "qualifiedBatchClaimWithWitness": "113672", + "qualifiedClaim": "60406", + "qualifiedClaimWithWitness": "59098", + "qualifiedSplitBatchClaim": "141486", + "qualifiedSplitBatchClaimWithWitness": "141552", "qualifiedSplitClaim": "86977", "qualifiedSplitClaimWithWitness": "87452", "register": "25379", - "splitBatchClaim": "140758", - "splitBatchClaimWithWitness": "140719", - "splitBatchTransfer": "110582", - "splitBatchWithdrawal": "139745", + "splitBatchClaim": "140788", + "splitBatchClaimWithWitness": "140749", + "splitBatchTransfer": "110652", + "splitBatchWithdrawal": "140361", "splitClaim": "86751", "splitClaimWithWitness": "86232", - "splitTransfer": "82439", - "splitWithdrawal": "93363" + "splitTransfer": "82482", + "splitWithdrawal": "93770" } \ No newline at end of file diff --git a/src/interfaces/ITheCompact.sol b/src/interfaces/ITheCompact.sol index 8028420..d7eaad2 100644 --- a/src/interfaces/ITheCompact.sol +++ b/src/interfaces/ITheCompact.sol @@ -567,4 +567,5 @@ interface ITheCompact { error InvalidDepositBalanceChange(); error Permit2CallFailed(); error InvalidRegistrationDuration(uint256 duration); + error ReentrantCall(address existingCaller); } diff --git a/src/lib/ComponentLib.sol b/src/lib/ComponentLib.sol index d31f842..7599abc 100644 --- a/src/lib/ComponentLib.sol +++ b/src/lib/ComponentLib.sol @@ -44,23 +44,8 @@ library ComponentLib { * @return Whether the transfer was successfully processed. */ function processSplitTransfer(SplitTransfer calldata transfer, function(address, address, uint256, uint256) internal returns (bool) operation) internal returns (bool) { - // Navigate to the split components in calldata. - SplitComponent[] calldata recipients = transfer.recipients; - - // Retrieve the resource lock ID and the total number of components. - uint256 id = transfer.id; - uint256 totalSplits = recipients.length; - - unchecked { - // Iterate over each additional component in calldata. - for (uint256 i = 0; i < totalSplits; ++i) { - // Navigate to location of the component in calldata. - SplitComponent calldata component = recipients[i]; - - // Perform the transfer or withdrawal for the portion. - operation(msg.sender, component.claimant, id, component.amount); - } - } + // Process the transfer for each split component. + _processSplitTransferComponents(transfer.recipients, transfer.id, operation); return true; } @@ -104,9 +89,6 @@ library ComponentLib { * @return Whether the transfer was successfully processed. */ function performSplitBatchTransfer(SplitBatchTransfer calldata transfer, function(address, address, uint256, uint256) internal returns (bool) operation) internal returns (bool) { - // Declare a variable for tracking the id of each component. - uint256 id; - // Navigate to the split batch components array in calldata. SplitByIdComponent[] calldata transfers = transfer.transfers; @@ -119,23 +101,8 @@ library ComponentLib { // Navigate to location of the component in calldata. SplitByIdComponent calldata component = transfers[i]; - // Retrieve the id for the component. - id = component.id; - - // Navigate to location of component portions in calldata. - SplitComponent[] calldata portions = component.portions; - - // Retrieve the total number of portions on the component. - uint256 totalPortions = portions.length; - - // Iterate over each portion in calldata. - for (uint256 j = 0; j < totalPortions; ++j) { - // Navigate to the location of the portion in calldata. - SplitComponent calldata portion = portions[j]; - - // Perform the transfer or withdrawal for the portion. - operation(msg.sender, portion.claimant, id, portion.amount); - } + // Process transfer for each split component in the set. + _processSplitTransferComponents(component.portions, component.id, operation); } } @@ -249,20 +216,13 @@ library ComponentLib { // Revert if the batch is empty. uint256 totalClaims = claims.length; - assembly ("memory-safe") { - if iszero(totalClaims) { - // revert InvalidBatchAllocation() - mstore(0, 0x3a03d3bb) - revert(0x1c, 0x04) - } - } // Process first claim and initialize error tracking. // NOTE: many of the bounds checks on these array accesses can be skipped as an optimization BatchClaimComponent calldata component = claims[0]; uint256 id = component.id; uint256 amount = component.amount; - uint256 errorBuffer = component.allocatedAmount.allocationExceededOrScopeNotMultichain(amount, id, sponsorDomainSeparator).asUint256(); + uint256 errorBuffer = (totalClaims == 0).or(component.allocatedAmount.allocationExceededOrScopeNotMultichain(amount, id, sponsorDomainSeparator)).asUint256(); // Execute transfer or withdrawal for first claim. operation(sponsor, claimant, id, amount); @@ -278,21 +238,7 @@ library ComponentLib { operation(sponsor, claimant, id, amount); } - // If any errors occurred, identify specific failures and revert. - if (errorBuffer.asBool()) { - for (uint256 i = 0; i < totalClaims; ++i) { - component = claims[i]; - component.amount.withinAllocated(component.allocatedAmount); - id = component.id; - sponsorDomainSeparator.ensureValidScope(component.id); - } - - assembly ("memory-safe") { - // revert InvalidBatchAllocation() - mstore(0, 0x3a03d3bb) - revert(0x1c, 0x04) - } - } + _revertWithInvalidBatchAllocationIfError(errorBuffer); } return true; @@ -358,18 +304,8 @@ library ComponentLib { claimComponent.portions.verifyAndProcessSplitComponents(sponsor, id, claimComponent.allocatedAmount, operation); } - // If any errors occurred, identify specific scope failures and revert. - if (errorBuffer.asBool()) { - for (uint256 i = 0; i < totalClaims; ++i) { - sponsorDomainSeparator.ensureValidScope(claims[i].id); - } - - assembly ("memory-safe") { - // revert InvalidBatchAllocation() - mstore(0, 0x3a03d3bb) - revert(0x1c, 0x04) - } - } + // Revert if any errors occurred. + _revertWithInvalidBatchAllocationIfError(errorBuffer); } return true; @@ -429,4 +365,44 @@ library ComponentLib { return true; } + + /** + * @notice Private function for performing a set of split transfers or withdrawals + * given an array of split components and an ID for an associated resource lock. + * Executes the transfer or withdrawal operation targeting multiple recipients. + * @param recipients A SplitComponent struct array containing split transfer details. + * @param id The ERC6909 token identifier of the resource lock. + * @param operation Function pointer to either _release or _withdraw for executing the claim. + */ + function _processSplitTransferComponents(SplitComponent[] calldata recipients, uint256 id, function(address, address, uint256, uint256) internal returns (bool) operation) private { + // Retrieve the total number of components. + uint256 totalSplits = recipients.length; + + unchecked { + // Iterate over each additional component in calldata. + for (uint256 i = 0; i < totalSplits; ++i) { + // Navigate to location of the component in calldata. + SplitComponent calldata component = recipients[i]; + + // Perform the transfer or withdrawal for the portion. + operation(msg.sender, component.claimant, id, component.amount); + } + } + } + + /** + * @notice Private pure function that reverts with an InvalidBatchAllocation error + * if an error buffer is nonzero. + * @param errorBuffer The error buffer to check. + */ + function _revertWithInvalidBatchAllocationIfError(uint256 errorBuffer) private pure { + // Revert if any errors occurred. + assembly ("memory-safe") { + if errorBuffer { + // revert InvalidBatchAllocation() + mstore(0, 0x3a03d3bb) + revert(0x1c, 0x04) + } + } + } } diff --git a/src/lib/ConstructorLogic.sol b/src/lib/ConstructorLogic.sol index 04ae01b..a74858b 100644 --- a/src/lib/ConstructorLogic.sol +++ b/src/lib/ConstructorLogic.sol @@ -57,10 +57,27 @@ contract ConstructorLogic is Tstorish { /** * @notice Internal function to set the reentrancy guard using either TSTORE or SSTORE. - * Called as part of functions that require reentrancy protection. + * Called as part of functions that require reentrancy protection. Reverts if called + * again before the reentrancy guard has been cleared. + * @dev Note that the caller is set to the value; this enables external contracts to + * ascertain the account originating the ongoing call while handling the call as long + * as exttload is available. */ function _setReentrancyGuard() internal { - _setTstorish(_REENTRANCY_GUARD_SLOT, 1); + uint256 entered = _getTstorish(_REENTRANCY_GUARD_SLOT); + + assembly ("memory-safe") { + if entered { + // revert ReentrantCall(address existingCaller) + mstore(0, 0xf57c448b) + mstore(0x20, entered) + revert(0x1c, 0x24) + } + + entered := caller() + } + + _setTstorish(_REENTRANCY_GUARD_SLOT, entered); } /**