From 63b4fff7fb54fd0e4926e988b4a56e50522f00a9 Mon Sep 17 00:00:00 2001 From: 0age <0age@protonmail.com> Date: Wed, 6 Mar 2024 14:51:25 -0500 Subject: [PATCH] modify NoSpecifiedOrdersAvailable behavior --- reference/lib/ReferenceOrderCombiner.sol | 20 +- src/core/lib/OrderCombiner.sol | 437 +++++++++++---------- test/foundry/new/helpers/FuzzMutations.sol | 13 +- 3 files changed, 247 insertions(+), 223 deletions(-) diff --git a/reference/lib/ReferenceOrderCombiner.sol b/reference/lib/ReferenceOrderCombiner.sol index 3b5e896..1d7c6da 100644 --- a/reference/lib/ReferenceOrderCombiner.sol +++ b/reference/lib/ReferenceOrderCombiner.sol @@ -403,7 +403,13 @@ contract ReferenceOrderCombiner is } // Apply criteria resolvers to each order as applicable. - _applyCriteriaResolvers(advancedOrders, ordersToExecute, criteriaResolvers); + _applyCriteriaResolvers( + advancedOrders, + ordersToExecute, + criteriaResolvers + ); + + bool someOrderAvailable; // Iterate over each order to check authorization status (for restricted // orders), generate orders (for contract orders), and emit events (for @@ -500,6 +506,13 @@ contract ReferenceOrderCombiner is spentItems, receivedItems ); + + someOrderAvailable = true; + } + + // Revert if no orders are available. + if (!someOrderAvailable) { + revert NoSpecifiedOrdersAvailable(); } } @@ -625,11 +638,6 @@ contract ReferenceOrderCombiner is ); } - // Revert if no orders are available. - if (executions.length == 0) { - revert NoSpecifiedOrdersAvailable(); - } - // Perform final checks and compress executions into standard and batch. availableOrders = _performFinalChecksAndExecuteOrders( advancedOrders, diff --git a/src/core/lib/OrderCombiner.sol b/src/core/lib/OrderCombiner.sol index a2b8d65..0df5e2b 100644 --- a/src/core/lib/OrderCombiner.sol +++ b/src/core/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 // recipient + ) - // 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(); } } } @@ -709,11 +737,6 @@ contract OrderCombiner is OrderFulfiller, FulfillmentApplier { } } - // Revert if no orders are available. - if (executions.length == 0) { - _revertNoSpecifiedOrdersAvailable(); - } - // Perform final checks and return. availableOrders = _performFinalChecksAndExecuteOrders( advancedOrders, executions, orderHashes, recipient, containsNonOpen diff --git a/test/foundry/new/helpers/FuzzMutations.sol b/test/foundry/new/helpers/FuzzMutations.sol index 1352ccd..bc374ab 100644 --- a/test/foundry/new/helpers/FuzzMutations.sol +++ b/test/foundry/new/helpers/FuzzMutations.sol @@ -3472,21 +3472,14 @@ contract FuzzMutations is Test, FuzzExecutor { MutationState memory /* mutationState */ ) external { // This mutation works by wiping out all the orders. Seaport reverts if - // `_executeAvailableFulfillments` finishes its loop and produces no - // executions. - + // `_validateOrdersAndPrepareToFulfill` finishes its loop and produces + // no unskipped orders. for (uint256 i; i < context.executionState.orders.length; i++) { AdvancedOrder memory order = context.executionState.orders[i]; - order.parameters.consideration = new ConsiderationItem[](0); - order.parameters.totalOriginalConsiderationItems = 0; + order.parameters.endTime = 0; _signOrValidateMutatedOrder(context, i); } - context.executionState.offerFulfillments = new FulfillmentComponent[][]( - 0 - ); - context.executionState.considerationFulfillments = - new FulfillmentComponent[][](0); exec(context); }