Bug fixes

Author: hensha256

.forge-snapshots/Base-V2DutchOrder-BaseExecuteSingleWithFee.snap

@@ -1 +1 @@
-188845
+187839

.forge-snapshots/Base-V2DutchOrder-ExecuteBatch.snap

@@ -1 +1 @@
-210669
+208628

.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputs.snap

@@ -1 +1 @@
-221076
+218396

.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap

@@ -1 +1 @@
-275385
+272068

.forge-snapshots/Base-V2DutchOrder-ExecuteBatchNativeOutput.snap

@@ -1 +1 @@
-204195
+202154

.forge-snapshots/Base-V2DutchOrder-ExecuteSingle.snap

@@ -1 +1 @@
-155052
+154046

.forge-snapshots/Base-V2DutchOrder-ExecuteSingleNativeOutput.snap

@@ -1 +1 @@
-140619
+139613

.forge-snapshots/Base-V2DutchOrder-ExecuteSingleValidation.snap

@@ -1 +1 @@
-164362
+163357

src/lib/V2DutchOrderLib.sol

@@ -90,23 +90,23 @@ library CosignerExtraDataLib {
     function hasExclusiveFiller(bytes memory extraData) internal pure returns (bool flag) {
         if (extraData.length == 0) return false;
         assembly {
-            flag := and(mload(extraData), EXCL_FILLER_FLAG_MASK)
+            flag := and(mload(add(extraData, 32)), EXCL_FILLER_FLAG_MASK)
         }
     }
 
     // "True" second bit of byte 1 signals that there is an input override
     function hasInputOverride(bytes memory extraData) internal pure returns (bool flag) {
         if (extraData.length == 0) return false;
         assembly {
-            flag := and(mload(extraData), INPUT_OVERRIDE_FLAG_MASK)
+            flag := and(mload(add(extraData, 32)), INPUT_OVERRIDE_FLAG_MASK)
         }
     }
 
     // "True" third bit of byte 1 signals that there is an output override
     function hasOutputOverrides(bytes memory extraData) internal pure returns (bool flag) {
         if (extraData.length == 0) return false;
         assembly {
-            flag := and(mload(extraData), OUTPUT_OVERRIDE_FLAG_MASK)
+            flag := and(mload(add(extraData, 32)), OUTPUT_OVERRIDE_FLAG_MASK)
         }
     }
 
@@ -116,16 +116,17 @@ library CosignerExtraDataLib {
         returns (address filler, uint256 inputOverride, uint256[] memory outputOverrides)
     {
         if (extraData.length == 0) return (filler, inputOverride, outputOverrides);
+        // The first 32 bytes are length
         // The first byte (index 0) only contains flags of whether each field is included in the bytes
-        // So we can start from index 1 to start decoding each field
-        uint256 bytesOffset = 1;
+        // So we can start from index 1 (after 32) to start decoding each field
+        uint256 bytesOffset = 33;
 
         if (hasExclusiveFiller(extraData)) {
             // an address is 20 bytes long
             require(extraData.length >= bytesOffset + 20);
             assembly {
                 // it loads a full 32 bytes, shift right 96 bits so only the address remains
-                filler := shr(mload(add(extraData, bytesOffset)), 96)
+                filler := shr(96, mload(add(extraData, bytesOffset)))
             }
             bytesOffset += 20;
         }
@@ -148,7 +149,7 @@ library CosignerExtraDataLib {
             bytesOffset += 32;
 
             // each element of the array is 32 bytes
-            require(extraData.length == bytesOffset + length * 32);
+            require(extraData.length == bytesOffset + (length - 1) * 32);
             outputOverrides = new uint256[](length);
 
             uint256 outputOverride;

src/reactors/V2DutchOrderReactor.sol

@@ -100,17 +100,19 @@ contract V2DutchOrderReactor is BaseReactor {
             order.input.startAmount = inputOverride;
         }
 
-        if (outputOverrides.length != order.outputs.length) {
-            revert InvalidOutputOverride();
-        }
-        for (uint256 i = 0; i < order.outputs.length; i++) {
-            DutchOutput memory output = order.outputs[i];
-            uint256 outputOverride = outputOverrides[i];
-            if (outputOverride != 0) {
-                if (outputOverride < output.startAmount) {
-                    revert InvalidOutputOverride();
+        if (outputOverrides.length != 0) {
+            if (outputOverrides.length != order.outputs.length) {
+                revert InvalidOutputOverride();
+            }
+            for (uint256 i = 0; i < order.outputs.length; i++) {
+                DutchOutput memory output = order.outputs[i];
+                uint256 outputOverride = outputOverrides[i];
+                if (outputOverride != 0) {
+                    if (outputOverride < output.startAmount) {
+                        revert InvalidOutputOverride();
+                    }
+                    output.startAmount = outputOverride;
                 }
-                output.startAmount = outputOverride;
             }
         }
     }

test/reactors/V2DutchOrderReactor.t.sol

@@ -6,6 +6,7 @@ import {DeployPermit2} from "../util/DeployPermit2.sol";
 import {
     V2DutchOrder,
     V2DutchOrderLib,
+    CosignerExtraDataLib,
     CosignerData,
     V2DutchOrderReactor,
     ResolvedOrder,
@@ -30,6 +31,7 @@ import {DutchOrder, BaseDutchOrderReactorTest} from "./BaseDutchOrderReactor.t.s
 contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReactorTest {
     using OrderInfoBuilder for OrderInfo;
     using V2DutchOrderLib for V2DutchOrder;
+    using CosignerExtraDataLib for bytes;
 
     uint256 constant cosignerPrivateKey = 0x99999999;
 
@@ -252,6 +254,20 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact
         assertEq(tokenIn.balanceOf(address(fillContract)), inputAmount);
     }
 
+    function testEncoding() public {
+        bytes memory encodedBytes = encodeExtraCosignerData(address(1111111111111111), 22222222222222, ArrayBuilder.fill(3, 5));
+        assertTrue(encodedBytes.hasExclusiveFiller());
+        assertTrue(encodedBytes.hasInputOverride());
+        assertTrue(encodedBytes.hasOutputOverrides());
+        (address filler, uint256 input, uint256[] memory output) = encodedBytes.decodeExtraParameters();
+        assertEq(filler, address(1111111111111111));
+        assertEq(input, 22222222222222);
+        assertEq(output.length, 3);
+        assertEq(output[0], 5);
+        assertEq(output[1], 5);
+        assertEq(output[2], 5);
+    }
+
     function testExclusivity() public {
         uint256 inputAmount = 1 ether;
         tokenIn.mint(swapper, inputAmount);
@@ -299,7 +315,7 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact
     {
         bool hasExclusiveFiller = (exclusiveFiller != address(0));
         bool hasInputOverride = (inputOverride != 0);
-        bool hasOutputOverrides = (outputOverrides.length == 0);
+        bool hasOutputOverrides = (outputOverrides.length != 0);
 
         bytes1 firstByte = 0x00;
         if (hasExclusiveFiller) firstByte |= 0x80;
@@ -309,8 +325,11 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact
         if (firstByte == 0x00) return "";
 
         extraData = abi.encodePacked(firstByte);
-        if (hasExclusiveFiller) bytes.concat(extraData, abi.encodePacked(exclusiveFiller));
-        if (hasInputOverride) bytes.concat(extraData, abi.encodePacked(inputOverride));
-        if (hasOutputOverrides) bytes.concat(extraData, abi.encode(outputOverrides));
+        if (hasExclusiveFiller) extraData = bytes.concat(extraData, abi.encodePacked(exclusiveFiller));
+        if (hasInputOverride) extraData = bytes.concat(extraData, abi.encodePacked(inputOverride));
+        if (hasOutputOverrides) {
+            extraData = bytes.concat(extraData, abi.encodePacked(outputOverrides.length));
+            extraData = bytes.concat(extraData, abi.encodePacked(outputOverrides));
+        }
     }
 }