Merge pull request #30 from ProjectOpenSea/fix-authorizeOrder-skip

revert on failed status update after authorizeOrder call
ProjectOpenSea · Feb 28, 2024 · 2ae3888 · 2ae3888
2 parents 1b0919f + f50f9bb
commit 2ae3888
Showing 1 changed file with 39 additions and 2 deletions.
diff --git a/src/core/lib/OrderCombiner.sol b/src/core/lib/OrderCombiner.sol
@@ -495,7 +495,11 @@ contract OrderCombiner is OrderFulfiller, FulfillmentApplier {
                 // Update order status as long as there is some fraction available.
                 if (advancedOrder.parameters.orderType != OrderType.CONTRACT) {
                     if (!_checkRestrictedAdvancedOrderAuthorization(
-                        advancedOrder, orderHashes, orderHash, (i >> OneWordShift) - 1, revertOnInvalid
+                        advancedOrder,
+                        orderHashes,
+                        orderHash,
+                        (i >> OneWordShift) - 1,
+                        revertOnInvalid
                     )) {
                         assembly {
                             mstore(add(orderHashes, i), 0)
@@ -510,7 +514,10 @@ contract OrderCombiner is OrderFulfiller, FulfillmentApplier {
                         orderHash,
                         advancedOrder.numerator,
                         advancedOrder.denominator,
-                        revertOnInvalid
+                        _revertOnFailedUpdate(
+                            advancedOrder.parameters,
+                            revertOnInvalid
+                        )
                     )) {
                         assembly {
                             mstore(add(orderHashes, i), 0)
@@ -1181,4 +1188,34 @@ contract OrderCombiner is OrderFulfiller, FulfillmentApplier {
                 )
         }
     }
+
+    /**
+     * @dev Internal view function to determine whether a status update failure
+     *      should cause a revert or allow a skipped order. The call must revert
+     *      if an `authorizeOrder` call has been successfully performed and the
+     *      status update cannot be performed, regardless of whether the order
+     *      could be otherwise marked as skipped. Note that a revert is not
+     *      required on a failed update if the call originates from the zone, as
+     *      no `authorizeOrder` call is performed in that case.
+     *
+     * @param orderParameters The order parameters in question.
+     * @param revertOnInvalid A boolean indicating whether the call should
+     *                        revert for non-restricted order types.
+     *
+     * @return revertOnFailedUpdate A boolean indicating whether the order
+     *                              should revert on a failed status update.
+     */
+    function _revertOnFailedUpdate(
+        OrderParameters memory orderParameters,
+        bool revertOnInvalid
+    ) internal view returns (bool revertOnFailedUpdate) {
+        OrderType orderType = orderParameters.orderType;
+        address zone = orderParameters.zone;
+        assembly {
+            revertOnFailedUpdate := or(
+                revertOnInvalid,
+                and(gt(orderType, 1), iszero(eq(caller(), zone)))
+            )
+        }
+    }
 }