refactor: Validation logic updates (#21)

* refactor: MinPercentageToAsk * refactor: New validation codes for ERC1155 with static calls * test: Adjustments post-refactor
LooksRare · Sep 26, 2022 · d8e4328 · d8e4328
1 parent 0899fb3
commit d8e4328
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 18 deletions.
diff --git a/contracts/orderValidation/OrderValidatorV1.sol b/contracts/orderValidation/OrderValidatorV1.sol
@@ -218,12 +218,13 @@ contract OrderValidatorV1 {
         // Return if order is bid since there is no protection for minPercentageToAsk
         if (!makerOrder.isOrderAsk) return ORDER_EXPECTED_TO_BE_VALID;
 
+        uint256 minNetPriceToAsk = (makerOrder.minPercentageToAsk * makerOrder.price);
+
         uint256 finalSellerAmount = makerOrder.price;
         uint256 protocolFee = (makerOrder.price * IExecutionStrategy(makerOrder.strategy).viewProtocolFee()) / 10000;
         finalSellerAmount -= protocolFee;
 
-        if ((finalSellerAmount * 10000) < (makerOrder.minPercentageToAsk * makerOrder.price))
-            return MIN_NET_RATIO_ABOVE_PROTOCOL_FEE;
+        if ((finalSellerAmount * 10000) < minNetPriceToAsk) return MIN_NET_RATIO_ABOVE_PROTOCOL_FEE;
 
         (address receiver, uint256 royaltyAmount) = royaltyFeeRegistry.royaltyInfo(
             makerOrder.collection,
@@ -233,7 +234,7 @@ contract OrderValidatorV1 {
         if (receiver != address(0) && royaltyAmount != 0) {
             // Royalty registry logic
             finalSellerAmount -= royaltyAmount;
-            if ((finalSellerAmount * 10000) < (makerOrder.minPercentageToAsk * makerOrder.price))
+            if ((finalSellerAmount * 10000) < minNetPriceToAsk)
                 return MIN_NET_RATIO_ABOVE_ROYALTY_FEE_REGISTRY_AND_PROTOCOL_FEE;
         } else {
             // ERC2981 logic
@@ -250,7 +251,7 @@ contract OrderValidatorV1 {
 
                 if (receiver != address(0)) {
                     finalSellerAmount -= royaltyAmount;
-                    if ((finalSellerAmount * 10000) < (makerOrder.minPercentageToAsk * makerOrder.price))
+                    if ((finalSellerAmount * 10000) < minNetPriceToAsk)
                         return MIN_NET_RATIO_ABOVE_ROYALTY_FEE_ERC2981_AND_PROTOCOL_FEE;
                 }
             }
@@ -398,11 +399,19 @@ contract OrderValidatorV1 {
         uint256 tokenId,
         uint256 amount
     ) internal view returns (uint256 validationCode) {
-        // @dev ERC1155 balanceOf doesn't revert if tokenId doesn't exist
-        if ((IERC1155(collection).balanceOf(user, tokenId)) < amount)
-            return ERC1155_BALANCE_TOKEN_ID_INFERIOR_TO_AMOUNT;
-        bool isApprovedAll = IERC1155(collection).isApprovedForAll(user, transferManager);
-        if (!isApprovedAll) return ERC1155_NO_APPROVAL_FOR_ALL;
+        (bool success, bytes memory data) = collection.staticcall(
+            abi.encodeWithSelector(IERC1155.balanceOf.selector, user, tokenId)
+        );
+
+        if (!success) return ERC1155_BALANCE_OF_DOES_NOT_EXIST;
+        if (abi.decode(data, (uint256)) < amount) return ERC1155_BALANCE_OF_TOKEN_ID_INFERIOR_TO_AMOUNT;
+
+        (success, data) = collection.staticcall(
+            abi.encodeWithSelector(IERC1155.isApprovedForAll.selector, user, transferManager)
+        );
+
+        if (!success) return ERC1155_IS_APPROVED_FOR_ALL_DOES_NOT_EXIST;
+        if (!abi.decode(data, (bool))) return ERC1155_NO_APPROVAL_FOR_ALL;
     }
 
     /**

diff --git a/contracts/orderValidation/ValidationCodeConstants.sol b/contracts/orderValidation/ValidationCodeConstants.sol
@@ -27,5 +27,7 @@ uint256 constant ERC20_APPROVAL_INFERIOR_TO_PRICE = 712;
 uint256 constant ERC721_TOKEN_ID_DOES_NOT_EXIST = 721;
 uint256 constant ERC721_TOKEN_ID_NOT_IN_BALANCE = 722;
 uint256 constant ERC721_NO_APPROVAL_FOR_ALL_OR_TOKEN_ID = 723;
-uint256 constant ERC1155_BALANCE_TOKEN_ID_INFERIOR_TO_AMOUNT = 731;
-uint256 constant ERC1155_NO_APPROVAL_FOR_ALL = 732;
+uint256 constant ERC1155_BALANCE_OF_DOES_NOT_EXIST = 731;
+uint256 constant ERC1155_BALANCE_OF_TOKEN_ID_INFERIOR_TO_AMOUNT = 732;
+uint256 constant ERC1155_IS_APPROVED_FOR_ALL_DOES_NOT_EXIST = 733;
+uint256 constant ERC1155_NO_APPROVAL_FOR_ALL = 734;
diff --git a/contracts/orderValidation/ValidationCodes.md b/contracts/orderValidation/ValidationCodes.md
@@ -41,5 +41,7 @@
 | 721  | ERC721_TOKEN_ID_DOES_NOT_EXIST                            |
 | 722  | ERC721_TOKEN_ID_NOT_IN_BALANCE                            |
 | 723  | ERC721_NO_APPROVAL_FOR_ALL_OR_TOKEN_ID                    |
-| 731  | ERC1155_BALANCE_TOKEN_ID_INFERIOR_TO_AMOUNT               |
-| 732  | ERC1155_NO_APPROVAL_FOR_ALL                               |
+| 731  | ERC1155_BALANCE_OF_DOES_NOT_EXIST                         |
+| 732  | ERC1155_BALANCE_OF_TOKEN_ID_INFERIOR_TO_AMOUNT            |
+| 733  | ERC1155_IS_APPROVED_FOR_ALL_DOES_NOT_EXIST                |
+| 734  | ERC1155_NO_APPROVAL_FOR_ALL                               |
diff --git a/test/helpers/configErrorCodes.ts b/test/helpers/configErrorCodes.ts
@@ -24,5 +24,7 @@ export const ERC20_APPROVAL_INFERIOR_TO_PRICE = 712;
 export const ERC721_TOKEN_ID_DOES_NOT_EXIST = 721;
 export const ERC721_TOKEN_ID_NOT_IN_BALANCE = 722;
 export const ERC721_NO_APPROVAL_FOR_ALL_OR_TOKEN_ID = 723;
-export const ERC1155_BALANCE_TOKEN_ID_INFERIOR_TO_AMOUNT = 731;
-export const ERC1155_NO_APPROVAL_FOR_ALL = 732;
+export const ERC1155_BALANCE_OF_DOES_NOT_EXIST = 731;
+export const ERC1155_BALANCE_OF_TOKEN_ID_INFERIOR_TO_AMOUNT = 732;
+export const ERC1155_IS_APPROVED_FOR_ALL_DOES_NOT_EXIST = 733;
+export const ERC1155_NO_APPROVAL_FOR_ALL = 734;
diff --git a/test/orderValidatorV1.test.ts b/test/orderValidatorV1.test.ts
@@ -8,7 +8,7 @@ import { setUp } from "./test-setup";
 import { tokenSetUp } from "./token-set-up";
 import {
   CUSTOM_TRANSFER_MANAGER,
-  ERC1155_BALANCE_TOKEN_ID_INFERIOR_TO_AMOUNT,
+  ERC1155_BALANCE_OF_TOKEN_ID_INFERIOR_TO_AMOUNT,
   ERC1155_NO_APPROVAL_FOR_ALL,
   ERC20_APPROVAL_INFERIOR_TO_PRICE,
   ERC20_BALANCE_INFERIOR_TO_PRICE,
@@ -252,12 +252,18 @@ describe("OrderValidatorV1 (additional tests)", () => {
       verifyingContract: looksRareExchange.address,
     });
 
-    assert.equal(await orderValidatorV1.checkOrderValidity(makerAskOrder), ERC1155_BALANCE_TOKEN_ID_INFERIOR_TO_AMOUNT);
+    assert.equal(
+      await orderValidatorV1.checkOrderValidity(makerAskOrder),
+      ERC1155_BALANCE_OF_TOKEN_ID_INFERIOR_TO_AMOUNT
+    );
 
     // 2. Signer has a balanceOf(tokenId) inferior to order amount
     // Mints 1 tokenId=0 (balance = 1)
     await mockERC1155.mint(makerAskUser.address, tokenId, constants.One, defaultAbiCoder.encode([], []));
-    assert.equal(await orderValidatorV1.checkOrderValidity(makerAskOrder), ERC1155_BALANCE_TOKEN_ID_INFERIOR_TO_AMOUNT);
+    assert.equal(
+      await orderValidatorV1.checkOrderValidity(makerAskOrder),
+      ERC1155_BALANCE_OF_TOKEN_ID_INFERIOR_TO_AMOUNT
+    );
 
     // Mints 1 tokenId=0 (balance = 2)
     await mockERC1155.mint(makerAskUser.address, tokenId, constants.One, defaultAbiCoder.encode([], []));