Merge pull request #1188 from thesandboxgame/feat/marketplace-no-zero…

…-maker

feat: check that order.maker != zero

packages/marketplace/contracts/exchange/ExchangeCore.sol

@@ -5,18 +5,18 @@ pragma solidity 0.8.21;
 import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 import {LibFill} from "./libraries/LibFill.sol";
 import {IAssetMatcher} from "../interfaces/IAssetMatcher.sol";
-import {TransferExecutor, LibTransfer} from "../transfer-manager/TransferExecutor.sol";
-import {LibDeal, LibAsset} from "../transfer-manager/lib/LibDeal.sol";
+import {TransferExecutor} from "../transfer-manager/TransferExecutor.sol";
 import {LibFeeSide} from "../transfer-manager/lib/LibFeeSide.sol";
-import {LibOrderDataGeneric, LibOrder} from "./libraries/LibOrderDataGeneric.sol";
+import {LibDeal} from "../transfer-manager/lib/LibDeal.sol";
+import {LibAsset} from "../lib-asset/LibAsset.sol";
+import {LibOrder} from "../lib-order/LibOrder.sol";
+import {LibPart} from "../lib-part/LibPart.sol";
 import {ITransferManager} from "../transfer-manager/interfaces/ITransferManager.sol";
 import {IOrderValidator} from "../interfaces/IOrderValidator.sol";
 
 /// @notice ExchangeCore contract
 /// @dev contains the main functions for the marketplace
 abstract contract ExchangeCore is Initializable, TransferExecutor, ITransferManager {
-    using LibTransfer for address payable;
-
     // a list of left/right orders that match each other
     // left and right are symmetrical except for fees that are taken from left side first.
     struct ExchangeMatch {
@@ -134,13 +134,14 @@ abstract contract ExchangeCore is Initializable, TransferExecutor, ITransferMana
         LibOrder.Order memory orderRight,
         bytes memory signatureRight
     ) internal view {
+        // validate must force order.maker != address(0)
         orderValidator.validate(orderLeft, signatureLeft, sender);
         orderValidator.validate(orderRight, signatureRight, sender);
         if (orderLeft.taker != address(0)) {
-            if (orderRight.maker != address(0)) require(orderRight.maker == orderLeft.taker, "leftOrder.taker failed");
+            require(orderRight.maker == orderLeft.taker, "leftOrder.taker failed");
         }
         if (orderRight.taker != address(0)) {
-            if (orderLeft.maker != address(0)) require(orderRight.taker == orderLeft.maker, "rightOrder.taker failed");
+            require(orderRight.taker == orderLeft.maker, "rightOrder.taker failed");
         }
     }
 
@@ -150,114 +151,65 @@ abstract contract ExchangeCore is Initializable, TransferExecutor, ITransferMana
     /// @param orderRight the right order of the match
     function _matchAndTransfer(
         address sender,
-        LibOrder.Order memory orderLeft,
-        LibOrder.Order memory orderRight
+        LibOrder.Order calldata orderLeft,
+        LibOrder.Order calldata orderRight
     ) internal {
         (LibAsset.AssetType memory makeMatch, LibAsset.AssetType memory takeMatch) = _matchAssets(
             orderLeft,
             orderRight
         );
 
-        (
-            LibOrderDataGeneric.GenericOrderData memory leftOrderData,
-            LibOrderDataGeneric.GenericOrderData memory rightOrderData,
-            LibFill.FillResult memory newFill
-        ) = _parseOrdersSetFillEmitMatch(sender, orderLeft, orderRight);
-
+        LibFill.FillResult memory newFill = _parseOrdersSetFillEmitMatch(sender, orderLeft, orderRight);
         doTransfers(
             LibDeal.DealSide({
                 asset: LibAsset.Asset({assetType: makeMatch, value: newFill.leftValue}),
-                payouts: leftOrderData.payouts,
+                payouts: _payToMaker(orderLeft),
                 from: orderLeft.maker
             }),
             LibDeal.DealSide({
                 asset: LibAsset.Asset(takeMatch, newFill.rightValue),
-                payouts: rightOrderData.payouts,
+                payouts: _payToMaker(orderRight),
                 from: orderRight.maker
             }),
             LibFeeSide.getFeeSide(makeMatch.assetClass, takeMatch.assetClass)
         );
     }
 
+    /// @notice create a payout array that pays to maker 100%
+    /// @param order the order from which the maker is taken
+    /// @return an array with just one entry that pays to order.maker
+    function _payToMaker(LibOrder.Order memory order) internal pure returns (LibPart.Part[] memory) {
+        LibPart.Part[] memory payout = new LibPart.Part[](1);
+        payout[0].account = order.maker;
+        payout[0].value = 10000;
+        return payout;
+    }
+
     /// @notice parse orders with LibOrderDataGeneric parse() to get the order data, then create a new fill with setFillEmitMatch()
     /// @param sender the message sender
     /// @param orderLeft left order
     /// @param orderRight right order
-    /// @return leftOrderData generic order data from left order
-    /// @return rightOrderData generic order data from right order
     /// @return newFill fill result
     function _parseOrdersSetFillEmitMatch(
         address sender,
-        LibOrder.Order memory orderLeft,
-        LibOrder.Order memory orderRight
-    )
-        internal
-        returns (
-            LibOrderDataGeneric.GenericOrderData memory leftOrderData,
-            LibOrderDataGeneric.GenericOrderData memory rightOrderData,
-            LibFill.FillResult memory newFill
-        )
-    {
+        LibOrder.Order calldata orderLeft,
+        LibOrder.Order calldata orderRight
+    ) internal returns (LibFill.FillResult memory newFill) {
         bytes32 leftOrderKeyHash = LibOrder.hashKey(orderLeft);
         bytes32 rightOrderKeyHash = LibOrder.hashKey(orderRight);
 
-        if (orderLeft.maker == address(0)) {
-            orderLeft.maker = sender;
-        }
-        if (orderRight.maker == address(0)) {
-            orderRight.maker = sender;
-        }
-
-        leftOrderData = LibOrderDataGeneric.parse(orderLeft);
-        rightOrderData = LibOrderDataGeneric.parse(orderRight);
-
-        newFill = _setFillEmitMatch(
-            sender,
-            orderLeft,
-            orderRight,
-            leftOrderKeyHash,
-            rightOrderKeyHash,
-            leftOrderData.isMakeFill,
-            rightOrderData.isMakeFill
-        );
-    }
-
-    ///    @notice calculates fills for the matched orders and set them in "fills" mapping
-    ///    @param sender the message sender
-    ///    @param orderLeft left order of the match
-    ///    @param orderRight right order of the match
-    ///    @param leftMakeFill true if the left orders uses make-side fills, false otherwise
-    ///    @param rightMakeFill true if the right orders uses make-side fills, false otherwise
-    ///    @return newFill returns change in orders' fills by the match
-    function _setFillEmitMatch(
-        address sender,
-        LibOrder.Order memory orderLeft,
-        LibOrder.Order memory orderRight,
-        bytes32 leftOrderKeyHash,
-        bytes32 rightOrderKeyHash,
-        bool leftMakeFill,
-        bool rightMakeFill
-    ) internal returns (LibFill.FillResult memory newFill) {
         uint256 leftOrderFill = _getOrderFill(orderLeft.salt, leftOrderKeyHash);
         uint256 rightOrderFill = _getOrderFill(orderRight.salt, rightOrderKeyHash);
-        newFill = LibFill.fillOrder(orderLeft, orderRight, leftOrderFill, rightOrderFill, leftMakeFill, rightMakeFill);
+        newFill = LibFill.fillOrder(orderLeft, orderRight, leftOrderFill, rightOrderFill);
 
         require(newFill.rightValue > 0 && newFill.leftValue > 0, "nothing to fill");
 
         if (orderLeft.salt != 0) {
-            if (leftMakeFill) {
-                fills[leftOrderKeyHash] = leftOrderFill + newFill.leftValue;
-            } else {
-                fills[leftOrderKeyHash] = leftOrderFill + newFill.rightValue;
-            }
+            fills[leftOrderKeyHash] = leftOrderFill + newFill.rightValue;
         }
 
         if (orderRight.salt != 0) {
-            if (rightMakeFill) {
-                fills[rightOrderKeyHash] = rightOrderFill + newFill.rightValue;
-            } else {
-                fills[rightOrderKeyHash] = rightOrderFill + newFill.leftValue;
-            }
+            fills[rightOrderKeyHash] = rightOrderFill + newFill.leftValue;
         }
 
         emit Match({

packages/marketplace/contracts/exchange/OrderValidator.sol

@@ -44,36 +44,37 @@ contract OrderValidator is IOrderValidator, Initializable, EIP712Upgradeable, Wh
     /// @param order order to be validated
     /// @param signature signature of order
     /// @param sender order sender
-    function validate(LibOrder.Order memory order, bytes memory signature, address sender) public view {
+    function validate(LibOrder.Order calldata order, bytes memory signature, address sender) public view {
+        require(order.maker != address(0), "no maker");
+
         LibOrder.validateOrderTime(order);
 
         address makeToken = abi.decode(order.makeAsset.assetType.data, (address));
         verifyWhiteList(makeToken);
 
         if (order.salt == 0) {
-            if (order.maker != address(0)) {
-                require(sender == order.maker, "maker is not tx sender");
-            }
-        } else {
-            if (sender != order.maker) {
-                bytes32 hash = LibOrder.hash(order);
-                // if maker is contract checking ERC1271 signature
-                if (order.maker.isContract()) {
-                    require(
-                        IERC1271Upgradeable(order.maker).isValidSignature(_hashTypedDataV4(hash), signature) ==
-                            MAGICVALUE,
-                        "contract order signature verification error"
-                    );
-                } else {
-                    // if maker is not contract then checking ECDSA signature
-                    if (_hashTypedDataV4(hash).recover(signature) != order.maker) {
-                        revert("order signature verification error");
-                    } else {
-                        require(order.maker != address(0), "no maker");
-                    }
-                }
-            }
+            require(sender == order.maker, "maker is not tx sender");
+            // No partial fill the order is reusable forever
+            return;
+        }
+
+        if (sender == order.maker) {
+            return;
         }
+
+        bytes32 hash = LibOrder.hash(order);
+        // if maker is contract checking ERC1271 signature
+        if (order.maker.isContract()) {
+            require(
+                IERC1271Upgradeable(order.maker).isValidSignature(_hashTypedDataV4(hash), signature) == MAGICVALUE,
+                "contract order signature verification error"
+            );
+            return;
+        }
+
+        // if maker is not contract then checking ECDSA signature
+        address recovered = _hashTypedDataV4(hash).recover(signature);
+        require(recovered == order.maker, "order signature verification error");
     }
 
     /// @notice if ERC20 token is accepted

packages/marketplace/contracts/exchange/libraries/LibFill.sol

@@ -1,7 +1,8 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.21;
 
-import {LibOrder, LibMath} from "../../lib-order/LibOrder.sol";
+import {LibOrder} from "../../lib-order/LibOrder.sol";
+import {LibMath} from "./LibMath.sol";
 
 /// @title This library provides `fillOrder` function.
 /// @notice It calculates fill of both orders (part of the Order that can be filled).
@@ -11,46 +12,42 @@ library LibFill {
         uint256 rightValue;
     }
 
-    struct IsMakeFill {
-        bool leftMake;
-        bool rightMake;
-    }
-
     /// @notice Should return filled values
     /// @param leftOrder left order
     /// @param rightOrder right order
     /// @param leftOrderFill current fill of the left order (0 if order is unfilled)
     /// @param rightOrderFill current fill of the right order (0 if order is unfilled)
-    /// @param leftIsMakeFill true if left orders fill is calculated from the make side, false if from the take side
-    /// @param rightIsMakeFill true if right orders fill is calculated from the make side, false if from the take side
     /// @dev We have 3 cases, 1st: left order should be fully filled
     /// @dev 2nd: right order should be fully filled or 3d: both should be fully filled if required values are the same
     /// @return the fill result of both orders
     function fillOrder(
-        LibOrder.Order memory leftOrder,
-        LibOrder.Order memory rightOrder,
+        LibOrder.Order calldata leftOrder,
+        LibOrder.Order calldata rightOrder,
         uint256 leftOrderFill,
-        uint256 rightOrderFill,
-        bool leftIsMakeFill,
-        bool rightIsMakeFill
+        uint256 rightOrderFill
     ) internal pure returns (FillResult memory) {
-        (uint256 leftMakeValue, uint256 leftTakeValue) = LibOrder.calculateRemaining(
-            leftOrder,
-            leftOrderFill,
-            leftIsMakeFill
-        );
-        (uint256 rightMakeValue, uint256 rightTakeValue) = LibOrder.calculateRemaining(
-            rightOrder,
-            rightOrderFill,
-            rightIsMakeFill
-        );
+        (uint256 leftMakeValue, uint256 leftTakeValue) = calculateRemaining(leftOrder, leftOrderFill);
+        (uint256 rightMakeValue, uint256 rightTakeValue) = calculateRemaining(rightOrder, rightOrderFill);
 
         if (rightTakeValue > leftMakeValue) {
             return fillLeft(leftMakeValue, leftTakeValue, rightOrder.makeAsset.value, rightOrder.takeAsset.value);
         }
         return fillRight(leftOrder.makeAsset.value, leftOrder.takeAsset.value, rightMakeValue, rightTakeValue);
     }
 
+    /// @notice calculate the remaining fill from orders
+    /// @param order order that we will calculate the remaining fill
+    /// @param fill to be subtracted
+    /// @return makeValue remaining fill from make side
+    /// @return takeValue remaining fill from take side
+    function calculateRemaining(
+        LibOrder.Order calldata order,
+        uint256 fill
+    ) internal pure returns (uint256 makeValue, uint256 takeValue) {
+        takeValue = order.takeAsset.value - fill;
+        makeValue = LibMath.safeGetPartialAmountFloor(order.makeAsset.value, order.takeAsset.value, takeValue);
+    }
+
     function fillRight(
         uint256 leftMakeValue,
         uint256 leftTakeValue,