Skip to content

Commit

Permalink
Merge pull request #37 from ProjectOpenSea/reference-fixes
Browse files Browse the repository at this point in the history
bring reference closer in line with optimized
  • Loading branch information
0age authored Feb 28, 2024
2 parents ed0ae6e + a482291 commit 9621516
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 195 deletions.
40 changes: 25 additions & 15 deletions reference/lib/ReferenceCriteriaResolution.sol
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,8 @@ contract ReferenceCriteriaResolution is CriteriaResolutionErrors {
// Retrieve length of orders array and place on stack.
arraySize = ordersToExecute.length;

// Iterate over each order to execute, skipping contract orders.
// Iterate over each order to execute.
for (uint256 i = 0; i < arraySize; ++i) {
if (advancedOrders[i].parameters.orderType == OrderType.CONTRACT) {
continue;
}

// Retrieve the order to execute.
OrderToExecute memory orderToExecute = ordersToExecute[i];

Expand All @@ -180,7 +176,12 @@ contract ReferenceCriteriaResolution is CriteriaResolutionErrors {
if (
_isItemWithCriteria(orderToExecute.spentItems[j].itemType)
) {
revert UnresolvedOfferCriteria(i, j);
if (
advancedOrders[i].parameters.orderType != OrderType.CONTRACT ||
orderToExecute.spentItems[j].identifier != 0
) {
revert UnresolvedOfferCriteria(i, j);
}
}
}

Expand All @@ -195,7 +196,12 @@ contract ReferenceCriteriaResolution is CriteriaResolutionErrors {
orderToExecute.receivedItems[j].itemType
)
) {
revert UnresolvedConsiderationCriteria(i, j);
if (
advancedOrders[i].parameters.orderType != OrderType.CONTRACT ||
orderToExecute.receivedItems[j].identifier != 0
) {
revert UnresolvedConsiderationCriteria(i, j);
}
}
}
}
Expand Down Expand Up @@ -317,12 +323,6 @@ contract ReferenceCriteriaResolution is CriteriaResolutionErrors {
}
}

// Skip validating criteria resolvers for considerationItems on contract
// orders, since contract offerers may resolve wildcard items themselves.
if (advancedOrder.parameters.orderType == OrderType.CONTRACT) {
return;
}

// Validate Criteria on order has been resolved

// Read consideration length from memory and place on stack.
Expand All @@ -336,7 +336,12 @@ contract ReferenceCriteriaResolution is CriteriaResolutionErrors {
advancedOrder.parameters.consideration[i].itemType
)
) {
revert UnresolvedConsiderationCriteria(0, i);
if (
advancedOrder.parameters.orderType != OrderType.CONTRACT ||
advancedOrder.parameters.consideration[i].identifierOrCriteria != 0
) {
revert UnresolvedConsiderationCriteria(0, i);
}
}
}

Expand All @@ -349,7 +354,12 @@ contract ReferenceCriteriaResolution is CriteriaResolutionErrors {
if (
_isItemWithCriteria(advancedOrder.parameters.offer[i].itemType)
) {
revert UnresolvedOfferCriteria(0, i);
if (
advancedOrder.parameters.orderType != OrderType.CONTRACT ||
advancedOrder.parameters.offer[i].identifierOrCriteria != 0
) {
revert UnresolvedOfferCriteria(0, i);
}
}
}
}
Expand Down
50 changes: 42 additions & 8 deletions reference/lib/ReferenceOrderCombiner.sol
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,9 @@ contract ReferenceOrderCombiner is
// Apply criteria resolvers to each order as applicable.
_applyCriteriaResolvers(advancedOrders, ordersToExecute, criteriaResolvers);

// Emit an event for each order signifying that it has been fulfilled.
// Iterate over each order.
// Iterate over each order to check authorization status (for restricted
// orders), generate orders (for contract orders), and emit events (for
// all available orders) signifying that they have been fulfilled.
for (uint256 i = 0; i < advancedOrders.length; ++i) {
// Do not emit an event if no order hash is present.
if (orderHashes[i] == bytes32(0)) {
Expand Down Expand Up @@ -452,24 +453,25 @@ contract ReferenceOrderCombiner is
orderHashes[i],
storedFractions[i].storedNumerator,
storedFractions[i].storedDenominator,
orderValidationParams.revertOnInvalid
_revertOnFailedUpdate(
orderParameters,
orderValidationParams.revertOnInvalid
)
)
) {
orderHashes[i] = bytes32(0);
ordersToExecute[i].numerator = 0;
continue;
}
} else {
(
bytes32 orderHash,
OrderToExecute memory orderToExecute
) = _getGeneratedOrder(
bytes32 orderHash = _getGeneratedOrder(
ordersToExecute[i],
orderParameters,
advancedOrders[i].extraData,
orderValidationParams.revertOnInvalid
);

orderHashes[i] = orderHash;
ordersToExecute[i] = orderToExecute;

if (orderHashes[i] == bytes32(0)) {
continue;
Expand Down Expand Up @@ -1107,4 +1109,36 @@ contract ReferenceOrderCombiner is
// Return executions.
return executions;
}

/**
* @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;
return (
revertOnInvalid || (
(
orderType == OrderType.FULL_RESTRICTED ||
orderType == OrderType.PARTIAL_RESTRICTED
) && zone != msg.sender
)
);
}
}
4 changes: 2 additions & 2 deletions reference/lib/ReferenceOrderFulfiller.sol
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,14 @@ contract ReferenceOrderFulfiller is
true
);
} else {
(bytes32 orderHash, OrderToExecute memory orderToExecute) = _getGeneratedOrder(
bytes32 orderHash = _getGeneratedOrder(
orderValidation.orderToExecute,
advancedOrder.parameters,
advancedOrder.extraData,
true
);

orderValidation.orderHash = orderHash;
orderValidation.orderToExecute = orderToExecute;
}

// Transfer each item contained in the order.
Expand Down
Loading

0 comments on commit 9621516

Please sign in to comment.