From d0173f0494ad84aaf71855f6a6c2f2fd79f284b5 Mon Sep 17 00:00:00 2001
From: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
Date: Fri, 9 Aug 2024 07:20:43 +0000
Subject: [PATCH 01/10] chore: migrate GPv2Signing domain separator tests to
 Foundry

---
 test/GPv2Signing.test.ts               | 19 ------------
 test/GPv2Signing/DomainSeparator.t.sol | 27 +++++++++++++++++
 test/GPv2Signing/Helper.sol            | 33 +++++++-------------
 test/libraries/Eip712.sol              | 42 ++++++++++++++++++++++++++
 4 files changed, 80 insertions(+), 41 deletions(-)
 create mode 100644 test/GPv2Signing/DomainSeparator.t.sol

diff --git a/test/GPv2Signing.test.ts b/test/GPv2Signing.test.ts
index c6d8ec08..1a7c9b4b 100644
--- a/test/GPv2Signing.test.ts
+++ b/test/GPv2Signing.test.ts
@@ -38,25 +38,6 @@ describe("GPv2Signing", () => {
     testDomain = domain(chainId, signing.address);
   });
 
-  describe("domainSeparator", () => {
-    it("should have an EIP-712 domain separator", async () => {
-      expect(await signing.domainSeparator()).to.equal(
-        ethers.utils._TypedDataEncoder.hashDomain(testDomain),
-      );
-    });
-
-    it("should have a different replay protection for each deployment", async () => {
-      const GPv2Signing = await ethers.getContractFactory(
-        "GPv2SigningTestInterface",
-      );
-      const signing2 = await GPv2Signing.deploy();
-
-      expect(await signing.domainSeparator()).to.not.equal(
-        await signing2.domainSeparator(),
-      );
-    });
-  });
-
   describe("setPreSignature", () => {
     const [owner, nonOwner] = traders;
     const orderUid = packOrderUidParams({
diff --git a/test/GPv2Signing/DomainSeparator.t.sol b/test/GPv2Signing/DomainSeparator.t.sol
new file mode 100644
index 00000000..32949567
--- /dev/null
+++ b/test/GPv2Signing/DomainSeparator.t.sol
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: LGPL-3.0-or-later
+pragma solidity ^0.8;
+
+import {Helper} from "./Helper.sol";
+
+import {Harness, Helper} from "./Helper.sol";
+import {Eip712} from "test/libraries/Eip712.sol";
+import {Order as OrderLib} from "test/libraries/Order.sol";
+
+contract DomainSeparator is Helper {
+    function test_TYPE_HASH_matches_the_EIP_712_order_type_hash() public view {
+        bytes32 expectedDomainSeparator = Eip712.hashStruct(
+            Eip712.EIP712Domain({
+                name: "Gnosis Protocol",
+                version: "v2",
+                chainId: block.chainid,
+                verifyingContract: address(executor)
+            })
+        );
+        assertEq(executor.domainSeparator(), expectedDomainSeparator);
+    }
+
+    function test_should_have_a_different_replay_protection_for_each_deployment() public {
+        Harness signing = new Harness();
+        assertNotEq(executor.domainSeparator(), signing.domainSeparator());
+    }
+}
diff --git a/test/GPv2Signing/Helper.sol b/test/GPv2Signing/Helper.sol
index 1ff214f6..98e07eac 100644
--- a/test/GPv2Signing/Helper.sol
+++ b/test/GPv2Signing/Helper.sol
@@ -1,30 +1,19 @@
 // SPDX-License-Identifier: LGPL-3.0-or-later
 pragma solidity ^0.8;
 
-import {IERC20} from "src/contracts/interfaces/IERC20.sol";
-import {GPv2Order} from "src/contracts/libraries/GPv2Order.sol";
-import {GPv2Trade} from "src/contracts/libraries/GPv2Trade.sol";
-import {GPv2Signing} from "src/contracts/mixins/GPv2Signing.sol";
+import {Test} from "forge-std/Test.sol";
 
-// solhint-disable func-name-mixedcase
-contract Harness is GPv2Signing {
-    constructor(bytes32 _domainSeparator) {
-        domainSeparator = _domainSeparator;
-    }
+import {GPv2SigningTestInterface} from "test/src/GPv2SigningTestInterface.sol";
 
-    function exposed_recoverOrderFromTrade(
-        RecoveredOrder memory recoveredOrder,
-        IERC20[] calldata tokens,
-        GPv2Trade.Data calldata trade
-    ) external view {
-        recoverOrderFromTrade(recoveredOrder, tokens, trade);
-    }
+// TODO: move the content of `GPv2SigningTestInterface` here once all tests have
+// been removed.
+// solhint-disable-next-line no-empty-blocks
+contract Harness is GPv2SigningTestInterface {}
+
+contract Helper is Test {
+    Harness internal executor;
 
-    function exposed_recoverOrderSigner(GPv2Order.Data memory order, Scheme signingScheme, bytes calldata signature)
-        external
-        view
-        returns (bytes32 orderDigest, address owner)
-    {
-        (orderDigest, owner) = recoverOrderSigner(order, signingScheme, signature);
+    function setUp() public {
+        executor = new Harness();
     }
 }
diff --git a/test/libraries/Eip712.sol b/test/libraries/Eip712.sol
index b387f701..985ace84 100644
--- a/test/libraries/Eip712.sol
+++ b/test/libraries/Eip712.sol
@@ -4,6 +4,13 @@ pragma solidity ^0.8;
 import {GPv2Order} from "src/contracts/libraries/GPv2Order.sol";
 
 library Eip712 {
+    struct EIP712Domain {
+        string name;
+        string version;
+        uint256 chainId;
+        address verifyingContract;
+    }
+
     // This is the struct representing an order that is signed by the user using
     // EIP-712.
     struct Order {
@@ -21,6 +28,25 @@ library Eip712 {
         string buyTokenBalance;
     }
 
+    // Ideally, this would be replaced by type(EIP712Domain).typehash.
+    // Progress tracking for this Solidity feature is here:
+    // https://github.com/ethereum/solidity/issues/14157
+    function EIP712DOMAIN_TYPE_HASH() internal pure returns (bytes32) {
+        return keccak256(
+            bytes(
+                string.concat(
+                    // Should reflect the definition of the struct `EIP712Domain`.
+                    "EIP712Domain(",
+                    "string name,",
+                    "string version,",
+                    "uint256 chainId,",
+                    "address verifyingContract",
+                    ")"
+                )
+            )
+        );
+    }
+
     // Ideally, this would be replaced by type(Order).typehash.
     // Progress tracking for this Solidity feature is here:
     // https://github.com/ethereum/solidity/issues/14157
@@ -96,6 +122,22 @@ library Eip712 {
         });
     }
 
+    function hashStruct(EIP712Domain memory domain) internal pure returns (bytes32) {
+        // Ideally, this would be replaced by `domain.hashStruct()`.
+        // Progress tracking for this Solidity feature is here:
+        // https://github.com/ethereum/solidity/issues/14208
+        return keccak256(
+            // Note: dynamic types are hashed.
+            abi.encode(
+                EIP712DOMAIN_TYPE_HASH(),
+                keccak256(bytes(domain.name)),
+                keccak256(bytes(domain.version)),
+                domain.chainId,
+                domain.verifyingContract
+            )
+        );
+    }
+
     function hashStruct(Order memory order) internal pure returns (bytes32) {
         // Ideally, this would be replaced by `order.hashStruct()`.
         // Progress tracking for this Solidity feature is here:

From c7d4149ac49a7c9708559245984d059d0019f3db Mon Sep 17 00:00:00 2001
From: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
Date: Fri, 9 Aug 2024 08:14:56 +0000
Subject: [PATCH 02/10] Fix unused import

---
 test/GPv2Signing/DomainSeparator.t.sol | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/GPv2Signing/DomainSeparator.t.sol b/test/GPv2Signing/DomainSeparator.t.sol
index 32949567..64c1dab7 100644
--- a/test/GPv2Signing/DomainSeparator.t.sol
+++ b/test/GPv2Signing/DomainSeparator.t.sol
@@ -5,7 +5,6 @@ import {Helper} from "./Helper.sol";
 
 import {Harness, Helper} from "./Helper.sol";
 import {Eip712} from "test/libraries/Eip712.sol";
-import {Order as OrderLib} from "test/libraries/Order.sol";
 
 contract DomainSeparator is Helper {
     function test_TYPE_HASH_matches_the_EIP_712_order_type_hash() public view {

From bc7f235db4dc7d7014a485305c1e6fc3cad59479 Mon Sep 17 00:00:00 2001
From: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
Date: Mon, 12 Aug 2024 06:58:16 +0000
Subject: [PATCH 03/10] Remove duplicated import

---
 test/GPv2Signing/DomainSeparator.t.sol | 2 --
 1 file changed, 2 deletions(-)

diff --git a/test/GPv2Signing/DomainSeparator.t.sol b/test/GPv2Signing/DomainSeparator.t.sol
index 64c1dab7..ff359c38 100644
--- a/test/GPv2Signing/DomainSeparator.t.sol
+++ b/test/GPv2Signing/DomainSeparator.t.sol
@@ -1,8 +1,6 @@
 // SPDX-License-Identifier: LGPL-3.0-or-later
 pragma solidity ^0.8;
 
-import {Helper} from "./Helper.sol";
-
 import {Harness, Helper} from "./Helper.sol";
 import {Eip712} from "test/libraries/Eip712.sol";
 

From 07218923726518edeed494e6efce09ab8c0c107a Mon Sep 17 00:00:00 2001
From: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
Date: Mon, 12 Aug 2024 07:14:31 +0000
Subject: [PATCH 04/10] chore: migrate GPv2Signing set pre-signature tests to
 Foundry

---
 src/ts/sign.ts                         |  5 ---
 test/GPv2Signing.test.ts               | 47 ---------------------
 test/GPv2Signing/SetPreSignature.t.sol | 56 ++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 52 deletions(-)
 create mode 100644 test/GPv2Signing/SetPreSignature.t.sol

diff --git a/src/ts/sign.ts b/src/ts/sign.ts
index 401270f4..a41e04f2 100644
--- a/src/ts/sign.ts
+++ b/src/ts/sign.ts
@@ -24,11 +24,6 @@ export const EIP1271_MAGICVALUE = ethers.utils.hexDataSlice(
   4,
 );
 
-/**
- * Marker value indicating a presignature is set.
- */
-export const PRE_SIGNED = ethers.utils.id("GPv2Signing.Scheme.PreSign");
-
 /**
  * The signing scheme used to sign the order.
  */
diff --git a/test/GPv2Signing.test.ts b/test/GPv2Signing.test.ts
index 1a7c9b4b..87de48f8 100644
--- a/test/GPv2Signing.test.ts
+++ b/test/GPv2Signing.test.ts
@@ -6,7 +6,6 @@ import {
   EIP1271_MAGICVALUE,
   OrderBalance,
   OrderKind,
-  PRE_SIGNED,
   SettlementEncoder,
   SigningScheme,
   TypedDataDomain,
@@ -14,7 +13,6 @@ import {
   domain,
   encodeEip1271SignatureData,
   hashOrder,
-  packOrderUidParams,
   signOrder,
 } from "../src/ts";
 
@@ -38,51 +36,6 @@ describe("GPv2Signing", () => {
     testDomain = domain(chainId, signing.address);
   });
 
-  describe("setPreSignature", () => {
-    const [owner, nonOwner] = traders;
-    const orderUid = packOrderUidParams({
-      orderDigest: ethers.constants.HashZero,
-      owner: owner.address,
-      validTo: 0xffffffff,
-    });
-
-    it("should set the pre-signature", async () => {
-      await signing.connect(owner).setPreSignature(orderUid, true);
-      expect(await signing.preSignature(orderUid)).to.equal(PRE_SIGNED);
-    });
-
-    it("should unset the pre-signature", async () => {
-      await signing.connect(owner).setPreSignature(orderUid, true);
-      await signing.connect(owner).setPreSignature(orderUid, false);
-      expect(await signing.preSignature(orderUid)).to.equal(
-        ethers.constants.Zero,
-      );
-    });
-
-    it("should emit a PreSignature event", async () => {
-      await expect(signing.connect(owner).setPreSignature(orderUid, true))
-        .to.emit(signing, "PreSignature")
-        .withArgs(owner.address, orderUid, true);
-
-      await expect(signing.connect(owner).setPreSignature(orderUid, false))
-        .to.emit(signing, "PreSignature")
-        .withArgs(owner.address, orderUid, false);
-    });
-
-    it("should emit a PreSignature event even if storage doesn't change", async () => {
-      await signing.connect(owner).setPreSignature(orderUid, true);
-      await expect(signing.connect(owner).setPreSignature(orderUid, true))
-        .to.emit(signing, "PreSignature")
-        .withArgs(owner.address, orderUid, true);
-    });
-
-    it("should revert if the order owner is not the transaction sender", async () => {
-      await expect(
-        signing.connect(nonOwner).setPreSignature(orderUid, true),
-      ).to.be.revertedWith("cannot presign order");
-    });
-  });
-
   describe("recoverOrderFromTrade", () => {
     it("should round-trip encode order data", async () => {
       // NOTE: Pay extra attention to use all bytes for each field, and that
diff --git a/test/GPv2Signing/SetPreSignature.t.sol b/test/GPv2Signing/SetPreSignature.t.sol
new file mode 100644
index 00000000..9f422b10
--- /dev/null
+++ b/test/GPv2Signing/SetPreSignature.t.sol
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: LGPL-3.0-or-later
+pragma solidity ^0.8;
+
+import {GPv2Signing} from "src/contracts/mixins/GPv2Signing.sol";
+
+import {Helper} from "./Helper.sol";
+import {Order} from "test/libraries/Order.sol";
+
+contract SetPreSignature is Helper {
+    address private immutable owner = makeAddr("GPv2Signing.SetPreSignature owner");
+    bytes private orderUid =
+        Order.computeOrderUid(keccak256("GPv2Signing.SetPreSignature order hash"), owner, type(uint32).max);
+
+    // Same constant as in GPv2Signing.PRE_SIGNED.
+    uint256 private constant PRE_SIGNED = uint256(keccak256("GPv2Signing.Scheme.PreSign"));
+    uint256 private constant NOT_SIGNED = 0;
+
+    function test_should_set_the_pre_signature() public {
+        vm.prank(owner);
+        executor.setPreSignature(orderUid, true);
+        assertEq(executor.preSignature(orderUid), PRE_SIGNED);
+    }
+
+    function test_should_unset_the_pre_signature() public {
+        vm.prank(owner);
+        executor.setPreSignature(orderUid, true);
+        vm.prank(owner);
+        executor.setPreSignature(orderUid, false);
+        assertEq(executor.preSignature(orderUid), NOT_SIGNED);
+    }
+
+    function test_should_emit_a_pre_signature_event() public {
+        vm.prank(owner);
+        vm.expectEmit(address(executor));
+        emit GPv2Signing.PreSignature(owner, orderUid, true);
+        executor.setPreSignature(orderUid, true);
+
+        vm.prank(owner);
+        vm.expectEmit(address(executor));
+        emit GPv2Signing.PreSignature(owner, orderUid, false);
+        executor.setPreSignature(orderUid, false);
+    }
+
+    function test_should_emit_a_PreSignature_event_even_if_storage_does_not_change() public {
+        emit GPv2Signing.PreSignature(owner, orderUid, true);
+        vm.prank(owner);
+        vm.expectEmit(address(executor));
+        emit GPv2Signing.PreSignature(owner, orderUid, true);
+        executor.setPreSignature(orderUid, true);
+    }
+
+    function test_reverts_if_the_order_owner_is_not_the_transaction_sender() public {
+        vm.expectRevert("GPv2: cannot presign order");
+        executor.setPreSignature(orderUid, true);
+    }
+}

From 56ff59800f58202a230d9cfb453fdb308031a7a8 Mon Sep 17 00:00:00 2001
From: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
Date: Mon, 12 Aug 2024 15:09:46 +0000
Subject: [PATCH 05/10] chore: migrate GPv2Signing recoverOrderFromTrade tests
 to Foundry

---
 test/GPv2Signing.test.ts                     | 111 +------------------
 test/GPv2Signing/Helper.sol                  |   2 +
 test/GPv2Signing/RecoverOrderFromTrade.t.sol |  76 +++++++++++++
 test/libraries/OrderFuzz.sol                 |  46 ++++++++
 4 files changed, 126 insertions(+), 109 deletions(-)
 create mode 100644 test/GPv2Signing/RecoverOrderFromTrade.t.sol
 create mode 100644 test/libraries/OrderFuzz.sol

diff --git a/test/GPv2Signing.test.ts b/test/GPv2Signing.test.ts
index 87de48f8..b73d22a2 100644
--- a/test/GPv2Signing.test.ts
+++ b/test/GPv2Signing.test.ts
@@ -4,8 +4,6 @@ import { artifacts, ethers, waffle } from "hardhat";
 
 import {
   EIP1271_MAGICVALUE,
-  OrderBalance,
-  OrderKind,
   SettlementEncoder,
   SigningScheme,
   TypedDataDomain,
@@ -16,8 +14,8 @@ import {
   signOrder,
 } from "../src/ts";
 
-import { decodeOrder, encodeOrder } from "./encoding";
-import { fillBytes, fillUint, SAMPLE_ORDER } from "./testHelpers";
+import { encodeOrder } from "./encoding";
+import { SAMPLE_ORDER } from "./testHelpers";
 
 describe("GPv2Signing", () => {
   const [deployer, ...traders] = waffle.provider.getWallets();
@@ -37,111 +35,6 @@ describe("GPv2Signing", () => {
   });
 
   describe("recoverOrderFromTrade", () => {
-    it("should round-trip encode order data", async () => {
-      // NOTE: Pay extra attention to use all bytes for each field, and that
-      // they all have different values to make sure the are correctly
-      // round-tripped.
-      const order = {
-        sellToken: fillBytes(20, 0x01),
-        buyToken: fillBytes(20, 0x02),
-        receiver: fillBytes(20, 0x03),
-        sellAmount: fillUint(256, 0x04),
-        buyAmount: fillUint(256, 0x05),
-        validTo: fillUint(32, 0x06).toNumber(),
-        appData: fillBytes(32, 0x07),
-        feeAmount: fillUint(256, 0x08),
-        kind: OrderKind.BUY,
-        partiallyFillable: true,
-        sellTokenBalance: OrderBalance.EXTERNAL,
-        buyTokenBalance: OrderBalance.INTERNAL,
-      };
-      const tradeExecution = {
-        executedAmount: fillUint(256, 0x09),
-      };
-
-      const encoder = new SettlementEncoder(testDomain);
-      await encoder.signEncodeTrade(
-        order,
-        traders[0],
-        SigningScheme.EIP712,
-        tradeExecution,
-      );
-
-      const { data: encodedOrder } = await signing.recoverOrderFromTradeTest(
-        encoder.tokens,
-        encoder.trades[0],
-      );
-      expect(decodeOrder(encodedOrder)).to.deep.equal(order);
-    });
-
-    it("should compute order unique identifier", async () => {
-      const encoder = new SettlementEncoder(testDomain);
-      await encoder.signEncodeTrade(
-        SAMPLE_ORDER,
-        traders[0],
-        SigningScheme.EIP712,
-      );
-
-      const { uid: orderUid } = await signing.recoverOrderFromTradeTest(
-        encoder.tokens,
-        encoder.trades[0],
-      );
-      expect(orderUid).to.equal(
-        computeOrderUid(testDomain, SAMPLE_ORDER, traders[0].address),
-      );
-    });
-
-    it("should recover the owner for all signing schemes", async () => {
-      const artifact = await artifacts.readArtifact("EIP1271Verifier");
-      const verifier = await waffle.deployMockContract(deployer, artifact.abi);
-      await verifier.mock.isValidSignature.returns(EIP1271_MAGICVALUE);
-
-      const sampleOrderUid = computeOrderUid(
-        testDomain,
-        SAMPLE_ORDER,
-        traders[2].address,
-      );
-      await signing.connect(traders[2]).setPreSignature(sampleOrderUid, true);
-
-      const encoder = new SettlementEncoder(testDomain);
-      await encoder.signEncodeTrade(
-        SAMPLE_ORDER,
-        traders[0],
-        SigningScheme.EIP712,
-      );
-      await encoder.signEncodeTrade(
-        SAMPLE_ORDER,
-        traders[1],
-        SigningScheme.ETHSIGN,
-      );
-      encoder.encodeTrade(SAMPLE_ORDER, {
-        scheme: SigningScheme.EIP1271,
-        data: {
-          verifier: verifier.address,
-          signature: "0x",
-        },
-      });
-      encoder.encodeTrade(SAMPLE_ORDER, {
-        scheme: SigningScheme.PRESIGN,
-        data: traders[2].address,
-      });
-
-      const owners = [
-        traders[0].address,
-        traders[1].address,
-        verifier.address,
-        traders[2].address,
-      ];
-
-      for (const [i, trade] of encoder.trades.entries()) {
-        const { owner } = await signing.recoverOrderFromTradeTest(
-          encoder.tokens,
-          trade,
-        );
-        expect(owner).to.equal(owners[i]);
-      }
-    });
-
     describe("uid uniqueness", () => {
       it("invalid EVM transaction encoding does not change order hash", async () => {
         // The variables for an EVM transaction are encoded in multiples of 32
diff --git a/test/GPv2Signing/Helper.sol b/test/GPv2Signing/Helper.sol
index 98e07eac..d89c606e 100644
--- a/test/GPv2Signing/Helper.sol
+++ b/test/GPv2Signing/Helper.sol
@@ -12,8 +12,10 @@ contract Harness is GPv2SigningTestInterface {}
 
 contract Helper is Test {
     Harness internal executor;
+    bytes32 internal domainSeparator;
 
     function setUp() public {
         executor = new Harness();
+        domainSeparator = executor.domainSeparator();
     }
 }
diff --git a/test/GPv2Signing/RecoverOrderFromTrade.t.sol b/test/GPv2Signing/RecoverOrderFromTrade.t.sol
new file mode 100644
index 00000000..41dd93e9
--- /dev/null
+++ b/test/GPv2Signing/RecoverOrderFromTrade.t.sol
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: LGPL-3.0-or-later
+pragma solidity ^0.8;
+
+import {Vm} from "forge-std/Test.sol";
+
+import {EIP1271Verifier, GPv2EIP1271} from "src/contracts/mixins/GPv2Signing.sol";
+import {GPv2Order, GPv2Signing} from "src/contracts/mixins/GPv2Signing.sol";
+
+import {Helper} from "./Helper.sol";
+import {Order} from "test/libraries/Order.sol";
+
+import {OrderFuzz} from "test/libraries/OrderFuzz.sol";
+import {Sign} from "test/libraries/Sign.sol";
+import {SettlementEncoder} from "test/libraries/encoders/SettlementEncoder.sol";
+
+contract RecoverOrderFromTrade is Helper {
+    using SettlementEncoder for SettlementEncoder.State;
+    using Sign for EIP1271Verifier;
+
+    Vm.Wallet private trader;
+
+    constructor() {
+        trader = vm.createWallet("GPv2Signing.RecoverOrderFromTrade: trader");
+    }
+
+    function test_should_round_trip_encode_order_data_and_unique_identifier(
+        OrderFuzz.Params memory params,
+        uint256 executedAmount
+    ) public {
+        GPv2Order.Data memory order = OrderFuzz.order(params);
+
+        SettlementEncoder.State storage encoder = SettlementEncoder.makeSettlementEncoder();
+        encoder.signEncodeTrade(vm, trader, order, domainSeparator, GPv2Signing.Scheme.Eip712, executedAmount);
+
+        GPv2Signing.RecoveredOrder memory recovered =
+            executor.recoverOrderFromTradeTest(encoder.tokens(), encoder.trades[0]);
+        assertEq(abi.encode(recovered.data), abi.encode(order));
+        assertEq(recovered.uid, Order.computeOrderUid(order, domainSeparator, trader.addr));
+    }
+
+    function test_should_recover_the_order_for_all_signing_schemes(OrderFuzz.Params memory params) public {
+        GPv2Order.Data memory order = OrderFuzz.order(params);
+
+        address traderPreSign = makeAddr("trader pre-sign");
+        EIP1271Verifier traderEip1271 = EIP1271Verifier(makeAddr("eip1271 verifier"));
+        Vm.Wallet memory traderEip712 = vm.createWallet("trader eip712");
+        Vm.Wallet memory traderEthsign = vm.createWallet("trader ethsign");
+
+        bytes memory uidPreSign = Order.computeOrderUid(order, domainSeparator, traderPreSign);
+        vm.prank(traderPreSign);
+        executor.setPreSignature(uidPreSign, true);
+
+        vm.mockCallRevert(address(traderEip1271), hex"", "unexpected call to mock contract");
+        vm.mockCall(
+            address(traderEip1271),
+            abi.encodePacked(EIP1271Verifier.isValidSignature.selector),
+            abi.encode(GPv2EIP1271.MAGICVALUE)
+        );
+
+        SettlementEncoder.State storage encoder = SettlementEncoder.makeSettlementEncoder();
+        encoder.encodeTrade(order, Sign.preSign(traderPreSign), 0);
+        encoder.encodeTrade(order, Sign.sign(traderEip1271, hex""), 0);
+        encoder.signEncodeTrade(vm, traderEip712, order, domainSeparator, GPv2Signing.Scheme.Eip712, 0);
+        encoder.signEncodeTrade(vm, traderEthsign, order, domainSeparator, GPv2Signing.Scheme.EthSign, 0);
+
+        GPv2Signing.RecoveredOrder memory recovered;
+        recovered = executor.recoverOrderFromTradeTest(encoder.tokens(), encoder.trades[0]);
+        assertEq(recovered.owner, traderPreSign);
+        recovered = executor.recoverOrderFromTradeTest(encoder.tokens(), encoder.trades[1]);
+        assertEq(recovered.owner, address(traderEip1271));
+        recovered = executor.recoverOrderFromTradeTest(encoder.tokens(), encoder.trades[2]);
+        assertEq(recovered.owner, traderEip712.addr);
+        recovered = executor.recoverOrderFromTradeTest(encoder.tokens(), encoder.trades[3]);
+        assertEq(recovered.owner, traderEthsign.addr);
+    }
+}
diff --git a/test/libraries/OrderFuzz.sol b/test/libraries/OrderFuzz.sol
new file mode 100644
index 00000000..3fa0f4eb
--- /dev/null
+++ b/test/libraries/OrderFuzz.sol
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: LGPL-3.0-or-later
+pragma solidity ^0.8;
+
+import {GPv2Order, IERC20} from "src/contracts/libraries/GPv2Order.sol";
+import {GPv2Trade} from "src/contracts/libraries/GPv2Trade.sol";
+
+import {Order} from "./Order.sol";
+
+library OrderFuzz {
+    using GPv2Order for GPv2Order.Data;
+    using GPv2Order for bytes;
+    using GPv2Trade for uint256;
+
+    struct Params {
+        address sellToken;
+        address buyToken;
+        address receiver;
+        uint256 sellAmount;
+        uint256 buyAmount;
+        uint32 validTo;
+        bytes32 appData;
+        uint256 feeAmount;
+        bytes32 flagsPick;
+    }
+
+    function order(Params memory params) internal pure returns (GPv2Order.Data memory) {
+        Order.Flags[] memory allFlags = Order.ALL_FLAGS();
+        // `flags` isn't exactly random, but for fuzzing purposes it should be
+        // more than enough.
+        Order.Flags memory flags = allFlags[uint256(params.flagsPick) % allFlags.length];
+        return GPv2Order.Data({
+            sellToken: IERC20(params.sellToken),
+            buyToken: IERC20(params.buyToken),
+            receiver: params.receiver,
+            sellAmount: params.sellAmount,
+            buyAmount: params.buyAmount,
+            validTo: params.validTo,
+            appData: params.appData,
+            feeAmount: params.feeAmount,
+            partiallyFillable: flags.partiallyFillable,
+            kind: flags.kind,
+            sellTokenBalance: flags.sellTokenBalance,
+            buyTokenBalance: flags.buyTokenBalance
+        });
+    }
+}

From 28168278888747872ce1918d54924e6cbdfd8855 Mon Sep 17 00:00:00 2001
From: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
Date: Mon, 12 Aug 2024 15:35:10 +0000
Subject: [PATCH 06/10] Add explanatory comment on domain separator struct

---
 test/libraries/Eip712.sol | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/test/libraries/Eip712.sol b/test/libraries/Eip712.sol
index 985ace84..82e3f11a 100644
--- a/test/libraries/Eip712.sol
+++ b/test/libraries/Eip712.sol
@@ -4,6 +4,10 @@ pragma solidity ^0.8;
 import {GPv2Order} from "src/contracts/libraries/GPv2Order.sol";
 
 library Eip712 {
+    /// EIP-712 domain information (a.k.a. "domain separator"). The salt
+    /// parameter is intentionally omitted as there is no need to disambiguate
+    /// with the available information. More details can be found at:
+    /// https://eips.ethereum.org/EIPS/eip-712#definition-of-domainseparator
     struct EIP712Domain {
         string name;
         string version;

From 585703de65f95a6ed60bf53bdfee3142969e3339 Mon Sep 17 00:00:00 2001
From: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
Date: Mon, 12 Aug 2024 15:39:34 +0000
Subject: [PATCH 07/10] Reuse existing PRE_SIGNED variable from library

---
 test/GPv2Signing/SetPreSignature.t.sol | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/test/GPv2Signing/SetPreSignature.t.sol b/test/GPv2Signing/SetPreSignature.t.sol
index 9f422b10..19e6e21b 100644
--- a/test/GPv2Signing/SetPreSignature.t.sol
+++ b/test/GPv2Signing/SetPreSignature.t.sol
@@ -5,20 +5,17 @@ import {GPv2Signing} from "src/contracts/mixins/GPv2Signing.sol";
 
 import {Helper} from "./Helper.sol";
 import {Order} from "test/libraries/Order.sol";
+import {Sign} from "test/libraries/Sign.sol";
 
 contract SetPreSignature is Helper {
     address private immutable owner = makeAddr("GPv2Signing.SetPreSignature owner");
     bytes private orderUid =
         Order.computeOrderUid(keccak256("GPv2Signing.SetPreSignature order hash"), owner, type(uint32).max);
 
-    // Same constant as in GPv2Signing.PRE_SIGNED.
-    uint256 private constant PRE_SIGNED = uint256(keccak256("GPv2Signing.Scheme.PreSign"));
-    uint256 private constant NOT_SIGNED = 0;
-
     function test_should_set_the_pre_signature() public {
         vm.prank(owner);
         executor.setPreSignature(orderUid, true);
-        assertEq(executor.preSignature(orderUid), PRE_SIGNED);
+        assertEq(executor.preSignature(orderUid), Sign.PRE_SIGNED);
     }
 
     function test_should_unset_the_pre_signature() public {
@@ -26,7 +23,7 @@ contract SetPreSignature is Helper {
         executor.setPreSignature(orderUid, true);
         vm.prank(owner);
         executor.setPreSignature(orderUid, false);
-        assertEq(executor.preSignature(orderUid), NOT_SIGNED);
+        assertEq(executor.preSignature(orderUid), 0);
     }
 
     function test_should_emit_a_pre_signature_event() public {

From 84eb7fc3cad25aaf1233e206abfc09c92918f5bc Mon Sep 17 00:00:00 2001
From: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
Date: Mon, 12 Aug 2024 15:50:31 +0000
Subject: [PATCH 08/10] Fix missing setting of pre-signature at start of test

Co-authored-by: mfw78 <53399572+mfw78@users.noreply.github.com>
---
 test/GPv2Signing/SetPreSignature.t.sol | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/GPv2Signing/SetPreSignature.t.sol b/test/GPv2Signing/SetPreSignature.t.sol
index 19e6e21b..e653a1dd 100644
--- a/test/GPv2Signing/SetPreSignature.t.sol
+++ b/test/GPv2Signing/SetPreSignature.t.sol
@@ -39,7 +39,8 @@ contract SetPreSignature is Helper {
     }
 
     function test_should_emit_a_PreSignature_event_even_if_storage_does_not_change() public {
-        emit GPv2Signing.PreSignature(owner, orderUid, true);
+        vm.prank(owner);
+        executor.setPreSignature(orderUid, true);
         vm.prank(owner);
         vm.expectEmit(address(executor));
         emit GPv2Signing.PreSignature(owner, orderUid, true);

From 6e618dc32a63737497b71618a84f1b1df413e8eb Mon Sep 17 00:00:00 2001
From: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
Date: Wed, 14 Aug 2024 09:24:00 +0000
Subject: [PATCH 09/10] Clean up imports

---
 test/GPv2Signing/RecoverOrderFromTrade.t.sol | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/test/GPv2Signing/RecoverOrderFromTrade.t.sol b/test/GPv2Signing/RecoverOrderFromTrade.t.sol
index 41dd93e9..96256ebb 100644
--- a/test/GPv2Signing/RecoverOrderFromTrade.t.sol
+++ b/test/GPv2Signing/RecoverOrderFromTrade.t.sol
@@ -3,12 +3,10 @@ pragma solidity ^0.8;
 
 import {Vm} from "forge-std/Test.sol";
 
-import {EIP1271Verifier, GPv2EIP1271} from "src/contracts/mixins/GPv2Signing.sol";
-import {GPv2Order, GPv2Signing} from "src/contracts/mixins/GPv2Signing.sol";
+import {EIP1271Verifier, GPv2EIP1271, GPv2Order, GPv2Signing} from "src/contracts/mixins/GPv2Signing.sol";
 
 import {Helper} from "./Helper.sol";
 import {Order} from "test/libraries/Order.sol";
-
 import {OrderFuzz} from "test/libraries/OrderFuzz.sol";
 import {Sign} from "test/libraries/Sign.sol";
 import {SettlementEncoder} from "test/libraries/encoders/SettlementEncoder.sol";

From 8dd66e72b535831ad7e569dea6bc14f3257e4544 Mon Sep 17 00:00:00 2001
From: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
Date: Thu, 15 Aug 2024 12:41:23 +0000
Subject: [PATCH 10/10] Merge fuzzed order library into order library

---
 test/GPv2Signing/RecoverOrderFromTrade.t.sol |  9 ++--
 test/libraries/Order.sol                     | 34 +++++++++++++++
 test/libraries/OrderFuzz.sol                 | 46 --------------------
 3 files changed, 38 insertions(+), 51 deletions(-)
 delete mode 100644 test/libraries/OrderFuzz.sol

diff --git a/test/GPv2Signing/RecoverOrderFromTrade.t.sol b/test/GPv2Signing/RecoverOrderFromTrade.t.sol
index 96256ebb..42cabf14 100644
--- a/test/GPv2Signing/RecoverOrderFromTrade.t.sol
+++ b/test/GPv2Signing/RecoverOrderFromTrade.t.sol
@@ -7,7 +7,6 @@ import {EIP1271Verifier, GPv2EIP1271, GPv2Order, GPv2Signing} from "src/contract
 
 import {Helper} from "./Helper.sol";
 import {Order} from "test/libraries/Order.sol";
-import {OrderFuzz} from "test/libraries/OrderFuzz.sol";
 import {Sign} from "test/libraries/Sign.sol";
 import {SettlementEncoder} from "test/libraries/encoders/SettlementEncoder.sol";
 
@@ -22,10 +21,10 @@ contract RecoverOrderFromTrade is Helper {
     }
 
     function test_should_round_trip_encode_order_data_and_unique_identifier(
-        OrderFuzz.Params memory params,
+        Order.Fuzzed memory params,
         uint256 executedAmount
     ) public {
-        GPv2Order.Data memory order = OrderFuzz.order(params);
+        GPv2Order.Data memory order = Order.fuzz(params);
 
         SettlementEncoder.State storage encoder = SettlementEncoder.makeSettlementEncoder();
         encoder.signEncodeTrade(vm, trader, order, domainSeparator, GPv2Signing.Scheme.Eip712, executedAmount);
@@ -36,8 +35,8 @@ contract RecoverOrderFromTrade is Helper {
         assertEq(recovered.uid, Order.computeOrderUid(order, domainSeparator, trader.addr));
     }
 
-    function test_should_recover_the_order_for_all_signing_schemes(OrderFuzz.Params memory params) public {
-        GPv2Order.Data memory order = OrderFuzz.order(params);
+    function test_should_recover_the_order_for_all_signing_schemes(Order.Fuzzed memory params) public {
+        GPv2Order.Data memory order = Order.fuzz(params);
 
         address traderPreSign = makeAddr("trader pre-sign");
         EIP1271Verifier traderEip1271 = EIP1271Verifier(makeAddr("eip1271 verifier"));
diff --git a/test/libraries/Order.sol b/test/libraries/Order.sol
index 1eedf725..393735f8 100644
--- a/test/libraries/Order.sol
+++ b/test/libraries/Order.sol
@@ -17,6 +17,19 @@ library Order {
         bool partiallyFillable;
     }
 
+    /// All parameters needed to generated a valid fuzzed order.
+    struct Fuzzed {
+        address sellToken;
+        address buyToken;
+        address receiver;
+        uint256 sellAmount;
+        uint256 buyAmount;
+        uint32 validTo;
+        bytes32 appData;
+        uint256 feeAmount;
+        bytes32 flagsPick;
+    }
+
     // I wish I could declare the following as constants and export them as part
     // of the library. However, "Only constants of value type and byte array
     // type are implemented." and "Library cannot have non-constant state
@@ -145,4 +158,25 @@ library Order {
         orderUid = new bytes(GPv2Order.UID_LENGTH);
         orderUid.packOrderUidParams(orderHash, owner, validTo);
     }
+
+    function fuzz(Fuzzed memory params) internal pure returns (GPv2Order.Data memory) {
+        Order.Flags[] memory allFlags = Order.ALL_FLAGS();
+        // `flags` isn't exactly random, but for fuzzing purposes it should be
+        // more than enough.
+        Order.Flags memory flags = allFlags[uint256(params.flagsPick) % allFlags.length];
+        return GPv2Order.Data({
+            sellToken: IERC20(params.sellToken),
+            buyToken: IERC20(params.buyToken),
+            receiver: params.receiver,
+            sellAmount: params.sellAmount,
+            buyAmount: params.buyAmount,
+            validTo: params.validTo,
+            appData: params.appData,
+            feeAmount: params.feeAmount,
+            partiallyFillable: flags.partiallyFillable,
+            kind: flags.kind,
+            sellTokenBalance: flags.sellTokenBalance,
+            buyTokenBalance: flags.buyTokenBalance
+        });
+    }
 }
diff --git a/test/libraries/OrderFuzz.sol b/test/libraries/OrderFuzz.sol
deleted file mode 100644
index 3fa0f4eb..00000000
--- a/test/libraries/OrderFuzz.sol
+++ /dev/null
@@ -1,46 +0,0 @@
-// SPDX-License-Identifier: LGPL-3.0-or-later
-pragma solidity ^0.8;
-
-import {GPv2Order, IERC20} from "src/contracts/libraries/GPv2Order.sol";
-import {GPv2Trade} from "src/contracts/libraries/GPv2Trade.sol";
-
-import {Order} from "./Order.sol";
-
-library OrderFuzz {
-    using GPv2Order for GPv2Order.Data;
-    using GPv2Order for bytes;
-    using GPv2Trade for uint256;
-
-    struct Params {
-        address sellToken;
-        address buyToken;
-        address receiver;
-        uint256 sellAmount;
-        uint256 buyAmount;
-        uint32 validTo;
-        bytes32 appData;
-        uint256 feeAmount;
-        bytes32 flagsPick;
-    }
-
-    function order(Params memory params) internal pure returns (GPv2Order.Data memory) {
-        Order.Flags[] memory allFlags = Order.ALL_FLAGS();
-        // `flags` isn't exactly random, but for fuzzing purposes it should be
-        // more than enough.
-        Order.Flags memory flags = allFlags[uint256(params.flagsPick) % allFlags.length];
-        return GPv2Order.Data({
-            sellToken: IERC20(params.sellToken),
-            buyToken: IERC20(params.buyToken),
-            receiver: params.receiver,
-            sellAmount: params.sellAmount,
-            buyAmount: params.buyAmount,
-            validTo: params.validTo,
-            appData: params.appData,
-            feeAmount: params.feeAmount,
-            partiallyFillable: flags.partiallyFillable,
-            kind: flags.kind,
-            sellTokenBalance: flags.sellTokenBalance,
-            buyTokenBalance: flags.buyTokenBalance
-        });
-    }
-}