diff --git a/src/lib/BasicOrderFulfiller.sol b/src/lib/BasicOrderFulfiller.sol index a673cfc..fff9a7e 100644 --- a/src/lib/BasicOrderFulfiller.sol +++ b/src/lib/BasicOrderFulfiller.sol @@ -91,9 +91,9 @@ import { OrderFulfilled_offer_body_offset, OrderFulfilled_offer_head_offset, OrderFulfilled_offer_length_baseOffset, - OrderFulfilled_offer_length_baseOffset_relativeTo_OrderFulfilled_baseOffset, - OrderFulfilled_offer_itemType_baseOffset_relativeTo_OrderFulfilled_baseOffset, - OrderFulfilled_offer_token_baseOffset_relativeTo_OrderFulfilled_baseOffset, + OrderFulfilled_offer_length_offset_relativeTo_baseOffset, + OrderFulfilled_offer_itemType_offset_relativeTo_baseOffset, + OrderFulfilled_offer_token_offset_relativeTo_baseOffset, OrderFulfilled_post_memory_region_reservedBytes, OrderFulfilled_selector, ReceivedItem_amount_offset, @@ -1012,7 +1012,7 @@ contract BasicOrderFulfiller is OrderValidator { mstore( add( eventDataPtr, - OrderFulfilled_offer_length_baseOffset_relativeTo_OrderFulfilled_baseOffset + OrderFulfilled_offer_length_offset_relativeTo_baseOffset ), 1 ) @@ -1021,7 +1021,7 @@ contract BasicOrderFulfiller is OrderValidator { mstore( add( eventDataPtr, - OrderFulfilled_offer_itemType_baseOffset_relativeTo_OrderFulfilled_baseOffset + OrderFulfilled_offer_itemType_offset_relativeTo_baseOffset ), offeredItemType ) @@ -1032,7 +1032,7 @@ contract BasicOrderFulfiller is OrderValidator { calldatacopy( add( eventDataPtr, - OrderFulfilled_offer_token_baseOffset_relativeTo_OrderFulfilled_baseOffset + OrderFulfilled_offer_token_offset_relativeTo_baseOffset ), BasicOrder_offerToken_cdPtr, ThreeWords @@ -1074,11 +1074,14 @@ contract BasicOrderFulfiller is OrderValidator { mstore(FreeMemoryPointerSlot, add( eventDataPtr, - // reserve extra 3 words to be used by `authorizeOrder` and - // `validatateOrder` if pre-post exection hook to the zone is - // required. These 3 memory slots will be used for the extra data/context - // and order hashes of the calldata. - add(dataSize, OrderFulfilled_post_memory_region_reservedBytes) + // Reserve extra 3 words to be used by `authorizeOrder` and + // `validatateOrder` if pre-post exection hook to the zone + // is required. These 3 memory slots will be used for the + // extra data/context and order hashes of the calldata. + add( + dataSize, + OrderFulfilled_post_memory_region_reservedBytes + ) ) ) } diff --git a/src/lib/ConsiderationDecoder.sol b/src/lib/ConsiderationDecoder.sol index 638cf55..4bc64ed 100644 --- a/src/lib/ConsiderationDecoder.sol +++ b/src/lib/ConsiderationDecoder.sol @@ -890,8 +890,8 @@ contract ConsiderationDecoder { // 1. offerOffset // 2. considerationOffset // 3. offerLength & considerationLength might occupy just one word - // if offerOffset & considerationOffset would point to the same offset - // and the arrays have length 0. + // if offerOffset & considerationOffset would point to the same + // offset and the arrays have length 0. invalidEncoding := lt(returndatasize(), ThreeWords) let offsetOffer @@ -907,7 +907,8 @@ contract ConsiderationDecoder { offsetOffer := mload(0) offsetConsideration := mload(OneWord) - // If valid length, check that offsets word boundaries are within returndata. + // If valid length, check that offsets word boundaries are + // within returndata. let invalidOfferOffset := gt( add(offsetOffer, OneWord), @@ -932,7 +933,7 @@ contract ConsiderationDecoder { considerationLength := mload(OneWord) { - // Calculate end offsets of offer & consideration arrays. + // Derive end offsets for offer & consideration arrays. let offerEndOffset := add( add(offsetOffer, OneWord), diff --git a/src/lib/ConsiderationEncoder.sol b/src/lib/ConsiderationEncoder.sol index 811a86b..0f7965c 100644 --- a/src/lib/ConsiderationEncoder.sol +++ b/src/lib/ConsiderationEncoder.sol @@ -555,11 +555,13 @@ contract ConsiderationEncoder { /** * @dev Takes an order hash and BasicOrderParameters struct (from calldata) - * and encodes it as `authorizeOrder` calldata. Note that memory data is reused - * from `OrderFulfilled` event data, and the rest of the calldata is prefixed and - * postfixed to this memory region. Importantly the memory region before the - * `OrderFulfilled`'s spent and received items are overwritten to. So this - * function will need to be modified if the layout of that event data changes. + * and encodes it as `authorizeOrder` calldata. Note that memory data + * is reused from `OrderFulfilled` event data, and the rest of the + * calldata is prefixed and postfixed to this memory region. Note that + * the memory region before the spent and received items on the + * `OrderFulfilled` event are overwritten, which implies that this + * function will need to be modified should the layout of that event + * data change in the future. * * @param orderHash The order hash. * diff --git a/src/lib/FulfillmentApplier.sol b/src/lib/FulfillmentApplier.sol index 405e230..6a030fb 100644 --- a/src/lib/FulfillmentApplier.sol +++ b/src/lib/FulfillmentApplier.sol @@ -112,7 +112,11 @@ contract FulfillmentApplier is FulfillmentApplicationErrors { // Validate & aggregate consideration items to new Execution object. _aggregateValidFulfillmentConsiderationItems( - advancedOrders, considerationComponents, considerationExecution + advancedOrders, + considerationComponents, + considerationExecution, + address(0), + bytes32(0) ); // Retrieve the consideration item from the execution struct. @@ -120,20 +124,15 @@ contract FulfillmentApplier is FulfillmentApplicationErrors { // Skip aggregating offer items if no consideration items are available. if (considerationItem.amount == 0) { - // Set the offerer and recipient to null address and the item type - // to a non-native item type if the execution amount is zero. This - // will cause the execution item to be skipped. - considerationExecution.offerer = address(0); - considerationExecution.item.recipient = payable(0); - considerationExecution.item.itemType = ItemType.ERC20; return considerationExecution; } - // Recipient does not need to be specified because it will always be set - // to that of the consideration. // Validate & aggregate offer items to Execution object. _aggregateValidFulfillmentOfferItems( - advancedOrders, offerComponents, execution + advancedOrders, + offerComponents, + execution, + considerationItem.recipient ); ReceivedItem memory executionItem = execution.item; @@ -215,11 +214,8 @@ contract FulfillmentApplier is FulfillmentApplicationErrors { executionItem.amount = considerationItem.amount; } - // Reuse consideration recipient. - executionItem.recipient = considerationItem.recipient; - // Return the final execution that will be triggered for relevant items. - return execution; // Execution(considerationItem, offerer, conduitKey); + return execution; // Execution(executionItem, offerer, conduitKey); } /** @@ -262,35 +258,24 @@ contract FulfillmentApplier is FulfillmentApplicationErrors { // If the fulfillment components are offer components... if (side == Side.OFFER) { - // Set the supplied recipient on the execution item. - item.recipient = payable(recipient); - // Return execution for aggregated items provided by offerer. _aggregateValidFulfillmentOfferItems( - advancedOrders, fulfillmentComponents, execution + advancedOrders, + fulfillmentComponents, + execution, + payable(recipient) ); } else { // Otherwise, fulfillment components are consideration // components. Return execution for aggregated items provided by // the fulfiller. _aggregateValidFulfillmentConsiderationItems( - advancedOrders, fulfillmentComponents, execution + advancedOrders, + fulfillmentComponents, + execution, + msg.sender, + fulfillerConduitKey ); - - // Set the caller as the offerer on the execution. - execution.offerer = msg.sender; - - // Set fulfiller conduit key as the conduit key on execution. - execution.conduitKey = fulfillerConduitKey; - } - - // Set the offerer and recipient to null address and the item type - // to a non-native item type if the execution amount is zero. This - // will cause the execution item to be skipped. - if (item.amount == 0) { - execution.offerer = address(0); - item.recipient = payable(0); - item.itemType = ItemType.ERC20; } } } @@ -305,11 +290,13 @@ contract FulfillmentApplier is FulfillmentApplicationErrors { * indicating the order index and item index of each * candidate offer item for aggregation. * @param execution The execution to apply the aggregation to. + * @param recipient The intended recipient for the received item. */ function _aggregateValidFulfillmentOfferItems( AdvancedOrder[] memory advancedOrders, FulfillmentComponent[] memory offerComponents, - Execution memory execution + Execution memory execution, + address payable recipient ) internal pure { assembly { // Declare a variable for the final aggregated item amount. @@ -335,11 +322,10 @@ contract FulfillmentApplier is FulfillmentApplicationErrors { // Increment position in considerationComponents head. fulfillmentHeadPtr := add(fulfillmentHeadPtr, OneWord) - // Retrieve the order index using the fulfillment pointer. - let orderIndex := mload(mload(fulfillmentHeadPtr)) - // Ensure that the order index is not out of range. - if iszero(lt(orderIndex, mload(advancedOrders))) { + if iszero( + lt(mload(mload(fulfillmentHeadPtr)), mload(advancedOrders)) + ) { throwInvalidFulfillmentComponentData() } @@ -349,7 +335,7 @@ contract FulfillmentApplier is FulfillmentApplicationErrors { // Calculate head position of advancedOrders[orderIndex] add( add(advancedOrders, OneWord), - shl(OneWordShift, orderIndex) + shl(OneWordShift, mload(mload(fulfillmentHeadPtr))) ) ) @@ -442,6 +428,12 @@ contract FulfillmentApplier is FulfillmentApplicationErrors { mload(add(offerItemPtr, Common_identifier_offset)) ) + // Set the recipient on the received item. + mstore( + add(receivedItem, ReceivedItem_recipient_offset), + recipient + ) + // Set offerer on returned execution using order pointer. mstore( add(execution, Execution_offerer_offset), @@ -554,7 +546,7 @@ contract FulfillmentApplier is FulfillmentApplicationErrors { * using supplied directives on which component items are candidates * for aggregation, skipping items on orders that are not available. * Note that this function depends on memory layout affected by an - * earlier call to _validateOrdersAndPrepareToFulfill. The memory for + * earlier call to _validateOrdersAndPrepareToFulfill. The memory for * the consideration arrays needs to be updated before calling * _aggregateValidFulfillmentConsiderationItems. * _validateOrdersAndPrepareToFulfill is called in _matchAdvancedOrders @@ -567,11 +559,17 @@ contract FulfillmentApplier is FulfillmentApplicationErrors { * of each candidate consideration item for * aggregation. * @param execution The execution to apply the aggregation to. + * @param offerer The address of the offerer to set on the + * execution. + * @param conduitKey A bytes32 value indicating the conduit key + * to set on the execution. */ function _aggregateValidFulfillmentConsiderationItems( AdvancedOrder[] memory advancedOrders, FulfillmentComponent[] memory considerationComponents, - Execution memory execution + Execution memory execution, + address offerer, + bytes32 conduitKey ) internal pure { // Utilize assembly in order to efficiently aggregate the items. assembly { @@ -724,6 +722,18 @@ contract FulfillmentApplier is FulfillmentApplicationErrors { ) ) + // Set provided offerer on the execution. + mstore( + add(execution, Execution_offerer_offset), + offerer + ) + + // Set provided conduitKey on the execution. + mstore( + add(execution, Execution_conduit_offset), + conduitKey + ) + // Calculate the hash of (itemType, token, identifier, // recipient). This is run after amount is set to zero, so // there will be one blank word after identifier included in diff --git a/src/lib/OrderCombiner.sol b/src/lib/OrderCombiner.sol index d6dbece..7ad669a 100644 --- a/src/lib/OrderCombiner.sol +++ b/src/lib/OrderCombiner.sol @@ -218,250 +218,267 @@ contract OrderCombiner is OrderFulfiller, FulfillmentApplier { // Ensure this function cannot be triggered during a reentrant call. _setReentrancyGuard(true); // Native tokens accepted during execution. - // Declare an error buffer indicating status of any native offer items. - // Native tokens may only be provided as part of contract orders or when - // fulfilling via matchOrders or matchAdvancedOrders; if bits indicating - // these conditions are not met have been set, throw. - uint256 invalidNativeOfferItemErrorBuffer; - - // Use assembly to set the value for the second bit of the error buffer. - assembly { - /** - * Use the 231st bit of the error buffer to indicate whether the - * current function is not matchAdvancedOrders or matchOrders. - * - * sig func - * ----------------------------------------------------------------- - * 1010100000010111010001000 0 000100 matchOrders - * 1111001011010001001010110 0 010010 matchAdvancedOrders - * 1110110110011000101001010 1 110100 fulfillAvailableOrders - * 1000011100100000000110110 1 000001 fulfillAvailableAdvancedOrders - * ^ 7th bit - */ - invalidNativeOfferItemErrorBuffer := - and(NonMatchSelector_MagicMask, calldataload(0)) - } - // Declare "terminal memory offset" variable for use in efficient loops. uint256 terminalMemoryOffset; - unchecked { - // Read length of orders array and place on the stack. - uint256 totalOrders = advancedOrders.length; + { + // Declare an error buffer indicating status of any native offer + // items. Native tokens may only be provided as part of contract + // orders or when fulfilling via matchOrders or matchAdvancedOrders; + // throw if bits indicating the conditions aren't met have been set. + uint256 invalidNativeOfferItemErrorBuffer; + + // Use assembly to set the value for the second bit of error buffer. + assembly { + /** + * Use the 231st bit of the error buffer to indicate whether the + * current function is not matchAdvancedOrders or matchOrders. + * + * sig func + * ------------------------------------------------------------- + * 1010100000010111010001000 0 000100 matchOrders + * 1111001011010001001010110 0 010010 matchAdvancedOrders + * 1110110110011000101001010 1 110100 fulfillAvailableOrders + * 1000011100100000000110110 1 000001 fulfillAvailableAdvanced + * ^ 7th bit + */ + invalidNativeOfferItemErrorBuffer := + and(NonMatchSelector_MagicMask, calldataload(0)) + } - // Track the order hash for each order being fulfilled. - orderHashes = new bytes32[](totalOrders); + unchecked { + // Read length of orders array and place on the stack. + uint256 totalOrders = advancedOrders.length; - // Determine the memory offset to terminate on during loops. - terminalMemoryOffset = (totalOrders + 1) << OneWordShift; - } + // Track the order hash for each order being fulfilled. + orderHashes = new bytes32[](totalOrders); - // Skip overflow checks as all for loops are indexed starting at zero. - unchecked { - // Declare variable to track if an order is not a contract order. - bool isNonContract; + // Determine the memory offset to terminate on during loops. + terminalMemoryOffset = (totalOrders + 1) << OneWordShift; + } - // Iterate over each order. - for (uint256 i = OneWord; i < terminalMemoryOffset; i += OneWord) { - // Retrieve order using pointer libraries to bypass out-of-range - // check & cast function to avoid additional memory allocation. - AdvancedOrder memory advancedOrder = ( - _getReadAdvancedOrderByOffset()(advancedOrders, i) - ); + // Skip overflow checks as for loops are indexed starting at zero. + unchecked { + // Declare variable to track if order is not a contract order. + bool isNonContract; + + // Iterate over each order. + for ( + uint256 i = OneWord; + i < terminalMemoryOffset; + i += OneWord + ) { + // Retrieve order via pointer to bypass out-of-range check & + // cast function to avoid additional memory allocation. + AdvancedOrder memory advancedOrder = ( + _getReadAdvancedOrderByOffset()(advancedOrders, i) + ); - // Validate it, update status, and determine fraction to fill. - (bytes32 orderHash, uint256 numerator, uint256 denominator) = - _validateOrder(advancedOrder, revertOnInvalid); + // Validate it, update status, & determine fraction to fill. + ( + bytes32 orderHash, + uint256 numerator, + uint256 denominator + ) = _validateOrder(advancedOrder, revertOnInvalid); - advancedOrder.numerator = uint120(numerator); + // Update the numerator on the order in question. + advancedOrder.numerator = uint120(numerator); - // Do not track hash or adjust prices if order is not fulfilled. - if (numerator == 0) { - // Continue iterating through the remaining orders. - continue; - } + // Do not track hash or adjust prices if order is skipped. + if (numerator == 0) { + // Continue iterating through the remaining orders. + continue; + } - advancedOrder.denominator = uint120(denominator); + // Update the denominator on the order in question. + advancedOrder.denominator = uint120(denominator); - // Otherwise, track the order hash in question. - assembly { - mstore(add(orderHashes, i), orderHash) - } + // Otherwise, track the order hash in question. + assembly { + mstore(add(orderHashes, i), orderHash) + } - // Place the start time for the order on the stack. - uint256 startTime = advancedOrder.parameters.startTime; + // Place the start time for the order on the stack. + uint256 startTime = advancedOrder.parameters.startTime; - // Place the end time for the order on the stack. - uint256 endTime = advancedOrder.parameters.endTime; + // Place the end time for the order on the stack. + uint256 endTime = advancedOrder.parameters.endTime; - { - // Determine the order type, used to check for eligibility - // for native token offer items as well as for the presence - // of restricted and contract orders (or non-open orders). - OrderType orderType = advancedOrder.parameters.orderType; - - // Utilize assembly to efficiently check for order types. - // Note that these checks expect that there are no order - // types beyond the current set (0-4) and will need to be - // modified if more order types are added. - assembly { - // Assign the variable indicating if the order is not a - // contract order. - isNonContract := lt(orderType, 4) + { + // Determine order type, used to check for eligibility + // for native token offer items as well as for presence + // of restricted and contract orders or non-open orders. + OrderType orderType = ( + advancedOrder.parameters.orderType + ); - // Update the variable indicating if the order is not an - // open order, remaining set if it has been set already. - containsNonOpen := or(containsNonOpen, gt(orderType, 1)) + // Utilize assembly to efficiently check order types. + // Note these checks expect that there are no order + // types beyond current set (0-4) and will need to be + // modified if more order types are added. + assembly { + // Assign the variable indicating if the order is + // not a contract order. + isNonContract := lt(orderType, 4) + + // Update the variable indicating if order is not an + // open order & keep set if it has been set already. + containsNonOpen := or( + containsNonOpen, + gt(orderType, 1) + ) + } } - } - // Retrieve array of offer items for the order in question. - OfferItem[] memory offer = advancedOrder.parameters.offer; + // Retrieve array of offer items for the order in question. + OfferItem[] memory offer = advancedOrder.parameters.offer; - // Read length of offer array and place on the stack. - uint256 totalOfferItems = offer.length; + // Read length of offer array and place on the stack. + uint256 totalOfferItems = offer.length; - // Iterate over each offer item on the order. - for (uint256 j = 0; j < totalOfferItems; ++j) { - // Retrieve the offer item. - OfferItem memory offerItem = offer[j]; + // Iterate over each offer item on the order. + for (uint256 j = 0; j < totalOfferItems; ++j) { + // Retrieve the offer item. + OfferItem memory offerItem = offer[j]; - // If the offer item is for the native token and the order - // type is not a contract order type, set the first bit of - // the error buffer to true. - assembly { - invalidNativeOfferItemErrorBuffer := - or( - invalidNativeOfferItemErrorBuffer, - lt(mload(offerItem), isNonContract) - ) - } + // If the offer item is for the native token and the + // order type is not a contract order type, set the + // first bit of the error buffer to true. + assembly { + invalidNativeOfferItemErrorBuffer := + or( + invalidNativeOfferItemErrorBuffer, + lt(mload(offerItem), isNonContract) + ) + } - // Apply order fill fraction to offer item end amount. - uint256 endAmount = _getFraction( - numerator, denominator, offerItem.endAmount - ); + // Apply order fill fraction to offer item end amount. + uint256 endAmount = _getFraction( + numerator, denominator, offerItem.endAmount + ); + + // Reuse same fraction if start & end amounts are equal. + if (offerItem.startAmount == offerItem.endAmount) { + // Apply derived amount to both start & end amount. + offerItem.startAmount = endAmount; + } else { + // Apply order fill fraction to item start amount. + offerItem.startAmount = _getFraction( + numerator, denominator, offerItem.startAmount + ); + } - // Reuse same fraction if start and end amounts are equal. - if (offerItem.startAmount == offerItem.endAmount) { - // Apply derived amount to both start and end amount. - offerItem.startAmount = endAmount; - } else { - // Apply order fill fraction to offer item start amount. - offerItem.startAmount = _getFraction( - numerator, denominator, offerItem.startAmount + // Adjust offer amount using current time; round down. + uint256 currentAmount = _locateCurrentAmount( + offerItem.startAmount, + endAmount, + startTime, + endTime, + _runTimeConstantFalse() // round down ); + + // Update amounts in memory to match the current amount. + // Note the end amount is used to track spent amounts. + offerItem.startAmount = currentAmount; + offerItem.endAmount = currentAmount; } - // Adjust offer amount using current time; round down. - uint256 currentAmount = _locateCurrentAmount( - offerItem.startAmount, - endAmount, - startTime, - endTime, - _runTimeConstantFalse() // round down + // Retrieve consideration item array for order in question. + ConsiderationItem[] memory consideration = ( + advancedOrder.parameters.consideration ); - // Update amounts in memory to match the current amount. - // Note that the end amount is used to track spent amounts. - offerItem.startAmount = currentAmount; - offerItem.endAmount = currentAmount; - } - - // Retrieve array of consideration items for order in question. - ConsiderationItem[] memory consideration = ( - advancedOrder.parameters.consideration - ); - - // Read length of consideration array and place on the stack. - uint256 totalConsiderationItems = consideration.length; + // Read length of consideration array and place on stack. + uint256 totalConsiderationItems = consideration.length; - // Iterate over each consideration item on the order. - for (uint256 j = 0; j < totalConsiderationItems; ++j) { - // Retrieve the consideration item. - ConsiderationItem memory considerationItem = - (consideration[j]); + // Iterate over each consideration item on the order. + for (uint256 j = 0; j < totalConsiderationItems; ++j) { + // Retrieve the consideration item. + ConsiderationItem memory considerationItem = + (consideration[j]); - // Apply fraction to consideration item end amount. - uint256 endAmount = _getFraction( - numerator, denominator, considerationItem.endAmount - ); + // Apply fraction to consideration item end amount. + uint256 endAmount = _getFraction( + numerator, denominator, considerationItem.endAmount + ); - // Reuse same fraction if start and end amounts are equal. - if ( - considerationItem.startAmount - == considerationItem.endAmount - ) { - // Apply derived amount to both start and end amount. - considerationItem.startAmount = endAmount; - } else { - // Apply fraction to consideration item start amount. - considerationItem.startAmount = _getFraction( - numerator, - denominator, + // Reuse same fraction if start & end amounts are equal. + if ( considerationItem.startAmount + == considerationItem.endAmount + ) { + // Apply derived amount to both start & end amount. + considerationItem.startAmount = endAmount; + } else { + // Apply fraction to item start amount. + considerationItem.startAmount = _getFraction( + numerator, + denominator, + considerationItem.startAmount + ); + } + + // Adjust amount using current time; round up. + uint256 currentAmount = ( + _locateCurrentAmount( + considerationItem.startAmount, + endAmount, + startTime, + endTime, + _runTimeConstantTrue() // round up + ) ); - } - // Adjust consideration amount using current time; round up. - uint256 currentAmount = ( - _locateCurrentAmount( - considerationItem.startAmount, - endAmount, - startTime, - endTime, - _runTimeConstantTrue() // round up - ) - ); + // Set the start amount as equal to the current amount. + considerationItem.startAmount = currentAmount; - // Set the start amount as equal to the current amount. - considerationItem.startAmount = currentAmount; + // Utilize assembly to manually "shift" the recipient + // value, then copy the start amount to the recipient. + // Note that this sets up the memory layout that is + // subsequently relied upon by + // _aggregateValidFulfillmentConsiderationItems as well + // as during comparison to generated contract orders. + assembly { + // Derive pointer to the recipient using the item + // pointer along with the offset to the recipient. + let considerationItemRecipientPtr := + add( + considerationItem, + ConsiderationItem_recipient_offset + ) - // Utilize assembly to manually "shift" the recipient value, - // then to copy the start amount to the recipient. - // Note that this sets up the memory layout that is - // subsequently relied upon by - // _aggregateValidFulfillmentConsiderationItems as well as - // during comparison against generated contract orders. - assembly { - // Derive the pointer to the recipient using the item - // pointer along with the offset to the recipient. - let considerationItemRecipientPtr := - add( - considerationItem, - ConsiderationItem_recipient_offset // recipient + // Write recipient to endAmount, as endAmount is not + // used from this point on and can be repurposed to + // fit the layout of a ReceivedItem. + mstore( + add( + considerationItem, + // Note that this value used to be endAmount + ReceivedItem_recipient_offset + ), + mload(considerationItemRecipientPtr) ) - // Write recipient to endAmount, as endAmount is not - // used from this point on and can be repurposed to fit - // the layout of a ReceivedItem. - mstore( - add( - considerationItem, - ReceivedItem_recipient_offset // old endAmount - ), - mload(considerationItemRecipientPtr) - ) - - // Write startAmount to recipient, as recipient is not - // used from this point on and can be repurposed to - // track received amounts. - mstore(considerationItemRecipientPtr, currentAmount) + // Write startAmount to recipient, as recipient is + // not used from this point on and can be repurposed + // to track received amounts. + mstore(considerationItemRecipientPtr, currentAmount) + } } } } - } - // If the first bit is set, a native offer item was encountered on an - // order that is not a contract order. If the 231st bit is set in the - // error buffer, the current function is not matchOrders or - // matchAdvancedOrders. If the value is 1 + (1 << 230), then both the - // 1st and 231st bits were set; in that case, revert with an error. - if ( - invalidNativeOfferItemErrorBuffer - == NonMatchSelector_InvalidErrorValue - ) { - _revertInvalidNativeOfferItem(); + // If the first bit is set, a native offer item was encountered on + // an order that is not a contract order. If the 231st bit is set in + // the error buffer, the current function is not matchOrders or + // matchAdvancedOrders. If the value is 1 + (1 << 230), then both + // 1st and 231st bits were set; in that case, revert with an error. + if ( + invalidNativeOfferItemErrorBuffer + == NonMatchSelector_InvalidErrorValue + ) { + _revertInvalidNativeOfferItem(); + } } // Apply criteria resolvers to each order as applicable. @@ -475,6 +492,9 @@ contract OrderCombiner is OrderFulfiller, FulfillmentApplier { // Declare stack variable outside of the loop to track order hash. bytes32 orderHash; + // Track whether any orders are still available for fulfillment. + bool someOrderAvailable = false; + // Iterate over each order. for (uint256 i = OneWord; i < terminalMemoryOffset; i += OneWord) { // Retrieve order hash, bypassing out-of-range check. @@ -595,6 +615,14 @@ contract OrderCombiner is OrderFulfiller, FulfillmentApplier { orderParameters.offer, orderParameters.consideration ); + + // Set the flag indicating that some order is available. + someOrderAvailable = true; + } + + // Revert if no orders are available. + if (!someOrderAvailable) { + _revertNoSpecifiedOrdersAvailable(); } } } @@ -682,73 +710,31 @@ contract OrderCombiner is OrderFulfiller, FulfillmentApplier { // Skip overflow checks as all for loops are indexed starting at zero. unchecked { - // Track number of filtered executions. - uint256 totalFilteredExecutions = 0; - // Iterate over each offer fulfillment. - for (uint256 i = 0; i < totalOfferFulfillments;) { - // Derive aggregated execution corresponding with fulfillment. - Execution memory execution = _aggregateAvailable( + for (uint256 i = 0; i < totalOfferFulfillments; ++i) { + // Derive aggregated execution corresponding with fulfillment + // and assign it to the executions array. + executions[i] = _aggregateAvailable( advancedOrders, Side.OFFER, offerFulfillments[i], fulfillerConduitKey, recipient ); - - // If the execution is filterable... - if (_isFilterableExecution(execution)) { - // Increment total filtered executions. - ++totalFilteredExecutions; - } else { - // Otherwise, assign the execution to the executions array. - executions[i - totalFilteredExecutions] = execution; - } - - // Increment iterator. - ++i; } // Iterate over each consideration fulfillment. - for (uint256 i = 0; i < totalConsiderationFulfillments;) { - // Derive aggregated execution corresponding with fulfillment. - Execution memory execution = _aggregateAvailable( + for (uint256 i = 0; i < totalConsiderationFulfillments; ++i) { + // Derive aggregated execution corresponding with fulfillment + // and assign it to the executions array. + executions[i + totalOfferFulfillments] = _aggregateAvailable( advancedOrders, Side.CONSIDERATION, considerationFulfillments[i], fulfillerConduitKey, address(0) // unused ); - - // If the execution is filterable... - if (_isFilterableExecution(execution)) { - // Increment total filtered executions. - ++totalFilteredExecutions; - } else { - // Otherwise, assign the execution to the executions array. - executions[i + totalOfferFulfillments - - totalFilteredExecutions] = execution; - } - - // Increment iterator. - ++i; } - - // If some number of executions have been filtered... - if (totalFilteredExecutions != 0) { - // reduce the total length of the executions array. - assembly { - mstore( - executions, - sub(mload(executions), totalFilteredExecutions) - ) - } - } - } - - // Revert if no orders are available. - if (executions.length == 0) { - _revertNoSpecifiedOrdersAvailable(); } // Perform final checks and return. @@ -815,46 +801,49 @@ contract OrderCombiner is OrderFulfiller, FulfillmentApplier { _getReadExecutionByOffset()(executions, i) ); - // Retrieve the associated received item. + // Retrieve the associated received item and amount. ReceivedItem memory item = execution.item; + uint256 amount = item.amount; + + // Transfer the item specified by the execution as long as the + // execution is not a zero-amount execution (which can occur if + // the corresponding fulfillment contained only items on orders + // that are unavailable or are out of range of the respective + // item array). + if (amount != 0) { + // Utilize assembly to check for native token balance. + assembly { + // Ensure a sufficient native balance if relevant. + if and( + iszero(mload(item)), // itemType == ItemType.NATIVE + // item.amount > address(this).balance + gt(amount, selfbalance()) + ) { + // Store left-padded selector with push4, + // mem[28:32] = selector + mstore( + 0, + InsufficientNativeTokensSupplied_error_selector + ) - // If execution transfers native tokens, check available value. - assembly { - // Ensure sufficient available balance for native transfers. - if and( - iszero(mload(item)), // itemType == ItemType.NATIVE - // item.amount > address(this).balance - gt( - mload( - add( - item, - ReceivedItem_amount_offset - ) - ), - selfbalance() - ) - ) { - // Store left-padded selector with push4, - // mem[28:32] = selector - mstore( - 0, - InsufficientNativeTokensSupplied_error_selector - ) - - // revert(abi.encodeWithSignature( - // "InsufficientNativeTokensSupplied()" - // )) - revert( - Error_selector_offset, - InsufficientNativeTokensSupplied_error_length - ) + // revert(abi.encodeWithSignature( + // "InsufficientNativeTokensSupplied()" + // )) + revert( + Error_selector_offset, + InsufficientNativeTokensSupplied_error_length + ) + } } - } - // Transfer the item specified by the execution. - _transfer( - item, execution.offerer, execution.conduitKey, accumulator - ); + // Transfer the item specified by the execution. + _transfer( + item, + execution.offerer, + execution.conduitKey, + accumulator + ); + } } } @@ -1159,41 +1148,19 @@ contract OrderCombiner is OrderFulfiller, FulfillmentApplier { // Skip overflow checks as all for loops are indexed starting at zero. unchecked { - // Track number of filtered executions. - uint256 totalFilteredExecutions = 0; - // Iterate over each fulfillment. for (uint256 i = 0; i < totalFulfillments; ++i) { /// Retrieve the fulfillment in question. Fulfillment memory fulfillment = fulfillments[i]; - // Derive the execution corresponding with the fulfillment. - Execution memory execution = _applyFulfillment( + // Derive the execution corresponding with the fulfillment and + // assign it to the executions array. + executions[i] = _applyFulfillment( advancedOrders, fulfillment.offerComponents, fulfillment.considerationComponents, i ); - - // If the execution is filterable... - if (_isFilterableExecution(execution)) { - // Increment total filtered executions. - ++totalFilteredExecutions; - } else { - // Otherwise, assign the execution to the executions array. - executions[i - totalFilteredExecutions] = execution; - } - } - - // If some number of executions have been filtered... - if (totalFilteredExecutions != 0) { - // reduce the total length of the executions array. - assembly { - mstore( - executions, - sub(mload(executions), totalFilteredExecutions) - ) - } } } @@ -1206,45 +1173,6 @@ contract OrderCombiner is OrderFulfiller, FulfillmentApplier { return executions; } - /** - * @dev Internal pure function to determine whether a given execution is - * filterable and may be removed from the executions array. The offerer - * and the recipient must be the same address and the item type cannot - * indicate a native token transfer. - * - * @param execution The execution to check for filterability. - * - * @return filterable A boolean indicating whether the execution in question - * can be filtered from the executions array. - */ - function _isFilterableExecution(Execution memory execution) - internal - pure - returns (bool filterable) - { - // Utilize assembly to efficiently determine if execution is filterable. - assembly { - // Retrieve the received item referenced by the execution. - let item := mload(execution) - - // Determine whether the execution is filterable. - filterable := - and( - // Determine if offerer and recipient are the same address. - eq( - // Retrieve recipient's address from the received item. - mload(add(item, ReceivedItem_recipient_offset)), - // Retrieve the offerer's address from the execution. - mload(add(execution, Execution_offerer_offset)) - ), - // Determine if received item's item type is non-zero, - // thereby indicating that the execution does not involve - // native tokens. - iszero(iszero(mload(item))) - ) - } - } - /** * @dev Internal view function to determine whether a status update failure * should cause a revert or allow a skipped order. The call must revert diff --git a/src/lib/OrderValidator.sol b/src/lib/OrderValidator.sol index c9fee7c..e150b8e 100644 --- a/src/lib/OrderValidator.sol +++ b/src/lib/OrderValidator.sol @@ -112,7 +112,7 @@ contract OrderValidator is Executor, ZoneInteraction { ); unchecked { - // If the order is not already validated, verify the supplied signature. + // If the order is not already validated, verify supplied signature. if (!orderStatus.isValidated) { _verifySignature( offerer, @@ -125,8 +125,8 @@ contract OrderValidator is Executor, ZoneInteraction { CalldataPointer .wrap(BasicOrder_signature_cdPtr) .readMaskedUint256() + - // Add the BasicOrderParameters struct offset to the - // relative pointer. + // Add the BasicOrderParameters struct offset to + // the relative pointer. BasicOrder_basicOrderParameters_cd_offset ) ) @@ -517,7 +517,7 @@ contract OrderValidator is Executor, ZoneInteraction { out := _a } - // Determine the amount to scale down the new filled fraction. + // Determine amount to scale down the new filled fraction. let scaleDown := gcd(filledNumerator, denominator) // Ensure that the divisor is at least one.