Added returned error selectors

1inch · Sep 22, 2023 · 20917ea · 20917ea
1 parent 4198114
commit 20917ea
Show file tree

Hide file tree

Showing 5 changed files with 139 additions and 10 deletions.
diff --git a/contracts/OrderLib.sol b/contracts/OrderLib.sol
@@ -26,6 +26,12 @@ import "./helpers/AmountCalculator.sol";
     error WrongGetter();
     /// @dev Error to be thrown when the call to get the amount fails.
     error GetAmountCallFailed();
+    /// @dev Error to be thrown when the extension data of an order is missing.
+    error MissingOrderExtension();
+    /// @dev Error to be thrown when the order has an unexpected extension.
+    error UnexpectedOrderExtension();
+    /// @dev Error to be thrown when the order extension is invalid.
+    error ExtensionInvalid();
 
     /// @dev The typehash of the order struct.
     bytes32 constant internal _LIMIT_ORDER_TYPEHASH = keccak256(
@@ -150,15 +156,16 @@ import "./helpers/AmountCalculator.sol";
       * @param order The order to validate against.
       * @param extension The extension associated with the order.
       * @return True if the extension is valid, false otherwise.
+      * @return The error selector if the extension is invalid, 0x00000000 otherwise.
       */
-    function validateExtension(IOrderMixin.Order calldata order, bytes calldata extension) internal pure returns(bool) {
+    function isValidExtension(IOrderMixin.Order calldata order, bytes calldata extension) internal pure returns(bool, bytes4) {
         if (order.makerTraits.hasExtension()) {
-            if (extension.length == 0) return false;
+            if (extension.length == 0) return (false, MissingOrderExtension.selector);
             // Lowest 160 bits of the order salt must be equal to the lowest 160 bits of the extension hash
-            if (uint256(keccak256(extension)) & type(uint160).max != order.salt & type(uint160).max) return false;
+            if (uint256(keccak256(extension)) & type(uint160).max != order.salt & type(uint160).max) return (false, ExtensionInvalid.selector);
         } else {
-            if (extension.length > 0) return false;
+            if (extension.length > 0) return (false, UnexpectedOrderExtension.selector);
         }
-        return true;
+        return (true, 0x00000000);
     }
 }
diff --git a/contracts/OrderMixin.sol b/contracts/OrderMixin.sol
@@ -175,7 +175,8 @@ abstract contract OrderMixin is IOrderMixin, EIP712, OnlyWethReceiver, Predicate
         bytes calldata extension,
         bytes calldata interaction
     ) public payable returns(uint256 makingAmount, uint256 takingAmount, bytes32 orderHash) {
-        if (!order.validateExtension(extension)) revert InvalidExtension();
+        (bool isValid, bytes4 validationResult) = order.isValidExtension(extension);
+        if (!isValid) revert InvalidExtension(validationResult);
         orderHash = order.hash(_domainSeparatorV4());
 
         // Check signature and apply order permit only on the first fill
@@ -253,7 +254,8 @@ abstract contract OrderMixin is IOrderMixin, EIP712, OnlyWethReceiver, Predicate
         if (permit.length > 0) {
             IERC20(order.takerAsset.get()).safePermit(permit);
         }
-        if(!order.validateExtension(extension)) revert InvalidExtension();
+        (bool isValid, bytes4 validationResult) = order.isValidExtension(extension);
+        if (!isValid) revert InvalidExtension(validationResult);
         orderHash = order.hash(_domainSeparatorV4());
 
         // Check signature and apply order permit only on the first fill

diff --git a/contracts/helpers/ETHOrders.sol b/contracts/helpers/ETHOrders.sol
@@ -19,7 +19,7 @@ contract ETHOrders is IPostInteraction, OnlyWethReceiver {
 
     error AccessDenied();
     error InvalidOrder();
-    error InvalidExtension();
+    error InvalidExtension(bytes4 errorSelector);
     error NotEnoughBalance();
     error ExistingOrder();
 
@@ -65,7 +65,8 @@ contract ETHOrders is IPostInteraction, OnlyWethReceiver {
      */
     function ethOrderDeposit(IOrderMixin.Order calldata order, bytes calldata extension) external payable returns(bytes32 orderHash) {
         if (!order.makerTraits.needPostInteractionCall()) revert InvalidOrder();
-        if (!order.validateExtension(extension)) revert InvalidExtension();
+        (bool isValid, bytes4 validationResult) = order.isValidExtension(extension);
+        if (!isValid) revert InvalidExtension(validationResult);
         if (order.maker.get() != address(this)) revert AccessDenied();
         if (order.getReceiver() != msg.sender) revert AccessDenied();
         if (order.makingAmount != msg.value) revert InvalidOrder();

diff --git a/contracts/interfaces/IOrderMixin.sol b/contracts/interfaces/IOrderMixin.sol
@@ -37,7 +37,7 @@ interface IOrderMixin {
     error MismatchArraysLengths();
     error InvalidPermit2Transfer();
     error SimulationResults(bool success, bytes res);
-    error InvalidExtension();
+    error InvalidExtension(bytes4 errorSelector);
 
     /**
      * @notice Emitted when order gets filled

diff --git a/test/LimitOrderProtocol.js b/test/LimitOrderProtocol.js
@@ -722,6 +722,50 @@ describe('LimitOrderProtocol', function () {
             await expect(ethOrders.connect(addr1).ethOrderDeposit(order, order.extension, { value: ether('0.3') }))
                 .to.be.revertedWithCustomError(ethOrders, 'InvalidOrder');
         });
+
+        it('Invalid extension (empty extension)', async function () {
+            const { dai, weth, ethOrders } = await loadFixture(deployContractsAndInit);
+
+            const order = buildOrder(
+                {
+                    maker: ethOrders.address,
+                    receiver: addr1.address,
+                    makerAsset: weth.address,
+                    takerAsset: dai.address,
+                    makingAmount: ether('0.3'),
+                    takingAmount: ether('300'),
+                },
+                {
+                    postInteraction: ethOrders.address,
+                },
+            );
+
+            const errorSelector = ethers.utils.keccak256(ethers.utils.toUtf8Bytes('MissingOrderExtension()')).slice(0, 10);
+            await expect(ethOrders.connect(addr1).ethOrderDeposit(order, [], { value: ether('0.3') }))
+                .to.be.revertedWithCustomError(ethOrders, 'InvalidExtension').withArgs(errorSelector);
+        });
+
+        it('Invalid extension (mismatched extension)', async function () {
+            const { dai, weth, ethOrders } = await loadFixture(deployContractsAndInit);
+
+            const order = buildOrder(
+                {
+                    maker: ethOrders.address,
+                    receiver: addr1.address,
+                    makerAsset: weth.address,
+                    takerAsset: dai.address,
+                    makingAmount: ether('0.3'),
+                    takingAmount: ether('300'),
+                },
+                {
+                    postInteraction: ethOrders.address,
+                },
+            );
+
+            const errorSelector = ethers.utils.keccak256(ethers.utils.toUtf8Bytes('ExtensionInvalid()')).slice(0, 10);
+            await expect(ethOrders.connect(addr1).ethOrderDeposit(order, order.extension.slice(0, -6), { value: ether('0.3') }))
+                .to.be.revertedWithCustomError(ethOrders, 'InvalidExtension').withArgs(errorSelector);
+        });
     });
 
     describe('Remaining invalidator', function () {
@@ -1120,6 +1164,81 @@ describe('LimitOrderProtocol', function () {
             await expect(swap.fillOrderExt(order, r, vs, 1, 1, order.extension))
                 .to.be.revertedWithCustomError(swap, 'PredicateIsNotTrue');
         });
+
+        it('should fail with invalid extension (empty extension)', async function () {
+            const { dai, weth, swap, chainId, arbitraryPredicate } = await loadFixture(deployContractsAndInit);
+
+            const arbitraryCall = swap.interface.encodeFunctionData('arbitraryStaticCall', [
+                arbitraryPredicate.address,
+                arbitraryPredicate.interface.encodeFunctionData('copyArg', [1]),
+            ]);
+            const predicate = swap.interface.encodeFunctionData('lt', [10, arbitraryCall]);
+
+            const order = buildOrder(
+                {
+                    makerAsset: dai.address,
+                    takerAsset: weth.address,
+                    makingAmount: 1,
+                    takingAmount: 1,
+                    maker: addr1.address,
+                },
+                {
+                    predicate,
+                },
+            );
+
+            const { r, vs } = compactSignature(await signOrder(order, chainId, swap.address, addr1));
+            const errorSelector = ethers.utils.keccak256(ethers.utils.toUtf8Bytes('MissingOrderExtension()')).slice(0, 10);
+            await expect(swap.fillOrderExt(order, r, vs, 1, 1, []))
+                .to.be.revertedWithCustomError(swap, 'InvalidExtension').withArgs(errorSelector);
+        });
+
+        it('should fail with invalid extension (mismatched extension)', async function () {
+            const { dai, weth, swap, chainId, arbitraryPredicate } = await loadFixture(deployContractsAndInit);
+
+            const arbitraryCall = swap.interface.encodeFunctionData('arbitraryStaticCall', [
+                arbitraryPredicate.address,
+                arbitraryPredicate.interface.encodeFunctionData('copyArg', [1]),
+            ]);
+            const predicate = swap.interface.encodeFunctionData('lt', [10, arbitraryCall]);
+
+            const order = buildOrder(
+                {
+                    makerAsset: dai.address,
+                    takerAsset: weth.address,
+                    makingAmount: 1,
+                    takingAmount: 1,
+                    maker: addr1.address,
+                },
+                {
+                    predicate,
+                },
+            );
+
+            const { r, vs } = compactSignature(await signOrder(order, chainId, swap.address, addr1));
+            const errorSelector = ethers.utils.keccak256(ethers.utils.toUtf8Bytes('ExtensionInvalid()')).slice(0, 10);
+            await expect(swap.fillOrderExt(order, r, vs, 1, 1, order.extension.slice(0, -6)))
+                .to.be.revertedWithCustomError(swap, 'InvalidExtension').withArgs(errorSelector);
+        });
+
+        it('should fail with invalid extension (unexpected extension)', async function () {
+            const { dai, weth, swap, chainId } = await loadFixture(deployContractsAndInit);
+
+            const order = buildOrder(
+                {
+                    makerAsset: dai.address,
+                    takerAsset: weth.address,
+                    makingAmount: 1,
+                    takingAmount: 1,
+                    maker: addr1.address,
+                },
+            );
+
+            const { r, vs } = compactSignature(await signOrder(order, chainId, swap.address, addr1));
+            const errorSelector = ethers.utils.keccak256(ethers.utils.toUtf8Bytes('UnexpectedOrderExtension()')).slice(0, 10);
+            await expect(swap.fillOrderExt(order, r, vs, 1, 1, ethers.utils.toUtf8Bytes('test')))
+                .to.be.revertedWithCustomError(swap, 'InvalidExtension').withArgs(errorSelector);
+        });
     });
 
     describe('Expiration', function () {