feat: check that order.maker != zero

packages/marketplace/contracts/exchange/ExchangeCore.sol

@@ -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,8 +151,8 @@ 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,
@@ -188,8 +189,8 @@ abstract contract ExchangeCore is Initializable, TransferExecutor, ITransferMana
     /// @return newFill fill result
     function _parseOrdersSetFillEmitMatch(
         address sender,
-        LibOrder.Order memory orderLeft,
-        LibOrder.Order memory orderRight
+        LibOrder.Order calldata orderLeft,
+        LibOrder.Order calldata orderRight
     )
         internal
         returns (
@@ -201,13 +202,6 @@ abstract contract ExchangeCore is Initializable, TransferExecutor, ITransferMana
         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);
 
@@ -231,8 +225,8 @@ abstract contract ExchangeCore is Initializable, TransferExecutor, ITransferMana
     ///    @return newFill returns change in orders' fills by the match
     function _setFillEmitMatch(
         address sender,
-        LibOrder.Order memory orderLeft,
-        LibOrder.Order memory orderRight,
+        LibOrder.Order calldata orderLeft,
+        LibOrder.Order calldata orderRight,
         bytes32 leftOrderKeyHash,
         bytes32 rightOrderKeyHash,
         bool leftMakeFill,

packages/marketplace/contracts/exchange/OrderValidator.sol

@@ -44,35 +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
+        if (_hashTypedDataV4(hash).recover(signature) != order.maker) {
+            revert("order signature verification error");
         }
     }
 

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

@@ -27,8 +27,8 @@ library LibFill {
     /// @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,

packages/marketplace/contracts/lib-order/LibOrder.sol

@@ -50,7 +50,7 @@ library LibOrder {
     /// @notice calculate hash key from order
     /// @param order object to be hashed
     /// @return hash key of order
-    function hashKey(Order memory order) internal pure returns (bytes32) {
+    function hashKey(Order calldata order) internal pure returns (bytes32) {
         if (order.dataType == DEFAULT_ORDER_TYPE) {
             return
                 keccak256(
@@ -79,7 +79,7 @@ library LibOrder {
     /// @notice calculate hash from order
     /// @param order object to be hashed
     /// @return hash of order
-    function hash(Order memory order) internal pure returns (bytes32) {
+    function hash(Order calldata order) internal pure returns (bytes32) {
         return
             keccak256(
                 // solhint-disable-next-line func-named-parameters

packages/marketplace/test/exchange/OrderValidator.test.ts

@@ -79,6 +79,46 @@ describe('OrderValidator.sol', function () {
       .to.not.be.reverted;
   });
 
+  it('should validate when salt is non zero and Order maker is sender', async function () {
+    const {OrderValidatorAsUser, ERC20Contract, ERC721Contract, user1} =
+      await loadFixture(deployFixtures);
+    const makerAsset = await AssetERC721(ERC721Contract, 100);
+    const takerAsset = await AssetERC20(ERC20Contract, 100);
+    const order = await OrderDefault(
+      user1,
+      makerAsset,
+      ZeroAddress,
+      takerAsset,
+      1,
+      0,
+      0
+    );
+    const signature = await signOrder(order, user1, OrderValidatorAsUser);
+
+    await expect(OrderValidatorAsUser.validate(order, signature, user1.address))
+      .to.not.be.reverted;
+  });
+  it('should not validate when maker is address zero', async function () {
+    const {OrderValidatorAsUser, ERC20Contract, ERC721Contract, user1, user2} =
+      await loadFixture(deployFixtures);
+    const makerAsset = await AssetERC721(ERC721Contract, 100);
+    const takerAsset = await AssetERC20(ERC20Contract, 100);
+    const order = await OrderDefault(
+      user1,
+      makerAsset,
+      ZeroAddress,
+      takerAsset,
+      1,
+      0,
+      0
+    );
+    order.maker = ZeroAddress;
+    const signature = await signOrder(order, user2, OrderValidatorAsUser);
+    await expect(
+      OrderValidatorAsUser.validate(order, signature, user2.address)
+    ).to.be.revertedWith('no maker');
+  });
+
   it('should not validate when sender and signature signer is not Order maker', async function () {
     const {OrderValidatorAsUser, ERC20Contract, ERC721Contract, user1, user2} =
       await loadFixture(deployFixtures);
@@ -349,4 +389,7 @@ describe('OrderValidator.sol', function () {
       )
     ).to.be.equal(false);
   });
+  // TODO:
+  // it('should check start / end', async function () {});
+  // it('should check validate through the whitelist', async function () {});
 });