Convert signing library to a mixin (#399)

This PR converts the `GPv2Siging` library to a mixin (i.e. abstract contract). This allows a couple things:
- The domain separator does not need to be passed around everywhere
- It can have storage for `presign` scheme

### Test Plan

Adjusted tests still pass.

Author: nlordell

hardhat.config.ts

@@ -65,6 +65,9 @@ export default {
     ],
   },
   networks: {
+    hardhat: {
+      blockGasLimit: 12.5e6,
+    },
     mainnet: {
       ...sharedNetworkConfig,
       url: `https://mainnet.infura.io/v3/${INFURA_KEY}`,

src/contracts/GPv2Settlement.sol

@@ -13,35 +13,15 @@ import "./libraries/GPv2Interaction.sol";
 import "./libraries/GPv2Order.sol";
 import "./libraries/GPv2Trade.sol";
 import "./libraries/GPv2TradeExecution.sol";
+import "./mixins/GPv2Signing.sol";
 
 /// @title Gnosis Protocol v2 Settlement Contract
 /// @author Gnosis Developers
-contract GPv2Settlement is ReentrancyGuard, StorageAccessible {
+contract GPv2Settlement is GPv2Signing, ReentrancyGuard, StorageAccessible {
     using GPv2Order for bytes;
-    using GPv2Signing for GPv2Signing.RecoveredOrder;
     using GPv2TradeExecution for GPv2TradeExecution.Data;
     using SafeMath for uint256;
 
-    /// @dev The EIP-712 domain type hash used for computing the domain
-    /// separator.
-    bytes32 private constant DOMAIN_TYPE_HASH =
-        keccak256(
-            "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"
-        );
-
-    /// @dev The EIP-712 domain name used for computing the domain separator.
-    bytes32 private constant DOMAIN_NAME = keccak256("Gnosis Protocol");
-
-    /// @dev The EIP-712 domain version used for computing the domain separator.
-    bytes32 private constant DOMAIN_VERSION = keccak256("v2");
-
-    /// @dev The domain separator used for signing orders that gets mixed in
-    /// making signatures for different domains incompatible. This domain
-    /// separator is computed following the EIP-712 standard and has replay
-    /// protection mixed in so that signed orders are only valid for specific
-    /// GPv2 contracts.
-    bytes32 public immutable domainSeparator;
-
     /// @dev The authenticator is used to determine who can call the settle function.
     /// That is, only authorised solvers have the ability to invoke settlements.
     /// Any valid authenticator implements an isSolver method called by the onlySolver
@@ -86,24 +66,6 @@ contract GPv2Settlement is ReentrancyGuard, StorageAccessible {
 
     constructor(GPv2Authentication authenticator_) {
         authenticator = authenticator_;
-
-        // NOTE: Currently, the only way to get the chain ID in solidity is
-        // using assembly.
-        uint256 chainId;
-        // solhint-disable-next-line no-inline-assembly
-        assembly {
-            chainId := chainid()
-        }
-
-        domainSeparator = keccak256(
-            abi.encode(
-                DOMAIN_TYPE_HASH,
-                DOMAIN_NAME,
-                DOMAIN_VERSION,
-                chainId,
-                address(this)
-            )
-        );
         allowanceManager = new GPv2AllowanceManager();
     }
 
@@ -198,18 +160,13 @@ contract GPv2Settlement is ReentrancyGuard, StorageAccessible {
         uint256[] calldata clearingPrices,
         GPv2Trade.Data[] calldata trades
     ) internal returns (GPv2TradeExecution.Data[] memory executedTrades) {
-        GPv2Signing.RecoveredOrder memory recoveredOrder =
-            GPv2Signing.allocateRecoveredOrder();
+        RecoveredOrder memory recoveredOrder = allocateRecoveredOrder();
 
         executedTrades = new GPv2TradeExecution.Data[](trades.length);
         for (uint256 i = 0; i < trades.length; i++) {
             GPv2Trade.Data calldata trade = trades[i];
 
-            recoveredOrder.recoverOrderFromTrade(
-                domainSeparator,
-                tokens,
-                trade
-            );
+            recoverOrderFromTrade(recoveredOrder, tokens, trade);
             computeTradeExecution(
                 recoveredOrder,
                 clearingPrices[trade.sellTokenIndex],
@@ -234,7 +191,7 @@ contract GPv2Settlement is ReentrancyGuard, StorageAccessible {
     /// @param feeDiscount The discount to apply to the final executed fees.
     /// @param executedTrade Memory location for computed executed trade data.
     function computeTradeExecution(
-        GPv2Signing.RecoveredOrder memory recoveredOrder,
+        RecoveredOrder memory recoveredOrder,
         uint256 sellPrice,
         uint256 buyPrice,
         uint256 executedAmount,

src/contracts/libraries/GPv2Trade.sol

@@ -2,15 +2,14 @@
 pragma solidity ^0.7.6;
 
 import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
+import "../mixins/GPv2Signing.sol";
 import "./GPv2Order.sol";
-import "./GPv2Signing.sol";
 
 /// @title Gnosis Protocol v2 Trade Library.
 /// @author Gnosis Developers
 library GPv2Trade {
     using GPv2Order for GPv2Order.Data;
     using GPv2Order for bytes;
-    using GPv2Signing for GPv2Order.Data;
 
     /// @dev A struct representing a trade to be executed as part a batch
     /// settlement.

src/contracts/libraries/GPv2Signing.sol renamed to src/contracts/mixins/GPv2Signing.sol

@@ -2,12 +2,12 @@
 pragma solidity ^0.7.6;
 
 import "../interfaces/GPv2EIP1271.sol";
-import "./GPv2Order.sol";
-import "./GPv2Trade.sol";
+import "../libraries/GPv2Order.sol";
+import "../libraries/GPv2Trade.sol";
 
 /// @title Gnosis Protocol v2 Signing Library.
 /// @author Gnosis Developers
-library GPv2Signing {
+abstract contract GPv2Signing {
     using GPv2Order for GPv2Order.Data;
     using GPv2Order for bytes;
 
@@ -22,6 +22,46 @@ library GPv2Signing {
     /// @dev Signing scheme used for recovery.
     enum Scheme {Eip712, EthSign, Eip1271}
 
+    /// @dev The EIP-712 domain type hash used for computing the domain
+    /// separator.
+    bytes32 private constant DOMAIN_TYPE_HASH =
+        keccak256(
+            "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"
+        );
+
+    /// @dev The EIP-712 domain name used for computing the domain separator.
+    bytes32 private constant DOMAIN_NAME = keccak256("Gnosis Protocol");
+
+    /// @dev The EIP-712 domain version used for computing the domain separator.
+    bytes32 private constant DOMAIN_VERSION = keccak256("v2");
+
+    /// @dev The domain separator used for signing orders that gets mixed in
+    /// making signatures for different domains incompatible. This domain
+    /// separator is computed following the EIP-712 standard and has replay
+    /// protection mixed in so that signed orders are only valid for specific
+    /// GPv2 contracts.
+    bytes32 public immutable domainSeparator;
+
+    constructor() {
+        // NOTE: Currently, the only way to get the chain ID in solidity is
+        // using assembly.
+        uint256 chainId;
+        // solhint-disable-next-line no-inline-assembly
+        assembly {
+            chainId := chainid()
+        }
+
+        domainSeparator = keccak256(
+            abi.encode(
+                DOMAIN_TYPE_HASH,
+                DOMAIN_NAME,
+                DOMAIN_VERSION,
+                chainId,
+                address(this)
+            )
+        );
+    }
+
     /// @dev Returns an empty recovered order with a pre-allocated buffer for
     /// packing the unique identifier.
     ///
@@ -38,27 +78,19 @@ library GPv2Signing {
     /// trade.
     ///
     /// @param recoveredOrder Memory location used for writing the recovered order data.
-    /// @param domainSeparator The domain separator used for signing the order.
     /// @param tokens The list of tokens included in the settlement. The token
     /// indices in the trade parameters map to tokens in this array.
     /// @param trade The trade data to recover the order data from.
     function recoverOrderFromTrade(
         RecoveredOrder memory recoveredOrder,
-        bytes32 domainSeparator,
         IERC20[] calldata tokens,
         GPv2Trade.Data calldata trade
     ) internal view {
         GPv2Order.Data memory order = recoveredOrder.data;
 
-        GPv2Signing.Scheme signingScheme =
-            GPv2Trade.extractOrder(trade, tokens, order);
+        Scheme signingScheme = GPv2Trade.extractOrder(trade, tokens, order);
         (bytes32 orderDigest, address owner) =
-            recoverOrderSigner(
-                order,
-                domainSeparator,
-                signingScheme,
-                trade.signature
-            );
+            recoverOrderSigner(order, signingScheme, trade.signature);
 
         recoveredOrder.uid.packOrderUidParams(
             orderDigest,
@@ -74,36 +106,22 @@ library GPv2Signing {
     /// @dev Recovers an order's signer from the specified order and signature.
     ///
     /// @param order The order to recover a signature for.
-    /// @param domainSeparator The domain separator used for signing the order.
     /// @param signingScheme The signing scheme.
     /// @param signature The signature bytes.
     /// @return orderDigest The computed order hash.
     /// @return owner The recovered address from the specified signature.
     function recoverOrderSigner(
         GPv2Order.Data memory order,
-        bytes32 domainSeparator,
         Scheme signingScheme,
         bytes calldata signature
     ) internal view returns (bytes32 orderDigest, address owner) {
         orderDigest = order.hash();
         if (signingScheme == Scheme.Eip712) {
-            owner = recoverEip712Signer(
-                signature,
-                domainSeparator,
-                orderDigest
-            );
+            owner = recoverEip712Signer(signature, orderDigest);
         } else if (signingScheme == Scheme.EthSign) {
-            owner = recoverEthsignSigner(
-                signature,
-                domainSeparator,
-                orderDigest
-            );
+            owner = recoverEthsignSigner(signature, orderDigest);
         } else if (signingScheme == Scheme.Eip1271) {
-            owner = recoverEip1271Signer(
-                signature,
-                domainSeparator,
-                orderDigest
-            );
+            owner = recoverEip1271Signer(signature, orderDigest);
         }
     }
 
@@ -169,15 +187,13 @@ library GPv2Signing {
     ///
     /// @param encodedSignature Calldata pointing to tightly packed signature
     /// bytes.
-    /// @param domainSeparator The domain separator used for signing the order.
     /// @param orderDigest The EIP-712 signing digest derived from the order
     /// parameters.
     /// @return owner The address of the signer.
     function recoverEip712Signer(
         bytes calldata encodedSignature,
-        bytes32 domainSeparator,
         bytes32 orderDigest
-    ) internal pure returns (address owner) {
+    ) internal view returns (address owner) {
         (bytes32 r, bytes32 s, uint8 v) =
             decodeEcdsaSignature(encodedSignature);
 
@@ -209,15 +225,13 @@ library GPv2Signing {
     ///
     /// @param encodedSignature Calldata pointing to tightly packed signature
     /// bytes.
-    /// @param domainSeparator The domain separator used for signing the order.
     /// @param orderDigest The EIP-712 signing digest derived from the order
     /// parameters.
     /// @return owner The address of the signer.
     function recoverEthsignSigner(
         bytes calldata encodedSignature,
-        bytes32 domainSeparator,
         bytes32 orderDigest
-    ) internal pure returns (address owner) {
+    ) internal view returns (address owner) {
         (bytes32 r, bytes32 s, uint8 v) =
             decodeEcdsaSignature(encodedSignature);
 
@@ -258,7 +272,6 @@ library GPv2Signing {
     /// cover the full length of the decoded signature.
     function recoverEip1271Signer(
         bytes calldata encodedSignature,
-        bytes32 domainSeparator,
         bytes32 orderDigest
     ) internal view returns (address owner) {
         // NOTE: Use assembly to read the verifier address from the encoded

src/contracts/test/GPv2SettlementTestInterface.sol

@@ -4,7 +4,6 @@ pragma abicoder v2;
 
 import "../GPv2Settlement.sol";
 import "../libraries/GPv2Interaction.sol";
-import "../libraries/GPv2Signing.sol";
 import "../libraries/GPv2Trade.sol";
 import "../libraries/GPv2TradeExecution.sol";
 
@@ -25,7 +24,7 @@ contract GPv2SettlementTestInterface is GPv2Settlement {
     }
 
     function computeTradeExecutionMemoryTest() external returns (uint256 mem) {
-        GPv2Signing.RecoveredOrder memory recoveredOrder;
+        RecoveredOrder memory recoveredOrder;
         GPv2TradeExecution.Data memory executedTrade;
 
         // NOTE: Solidity stores the free memory pointer at address 0x40. Read

src/contracts/test/GPv2SigningTestInterface.sol

@@ -3,31 +3,19 @@ pragma solidity ^0.7.6;
 pragma abicoder v2;
 
 import "../libraries/GPv2Order.sol";
-import "../libraries/GPv2Signing.sol";
 import "../libraries/GPv2Trade.sol";
+import "../mixins/GPv2Signing.sol";
 
-contract GPv2SigningTestInterface {
-    using GPv2Signing for GPv2Order.Data;
-    using GPv2Signing for GPv2Signing.RecoveredOrder;
-
-    bytes32 public constant DOMAIN_SEPARATOR =
-        keccak256(
-            abi.encode(
-                keccak256("EIP712Domain(string name)"),
-                keccak256("test")
-            )
-        );
-
+contract GPv2SigningTestInterface is GPv2Signing {
     function recoverOrderFromTradeTest(
         IERC20[] calldata tokens,
         GPv2Trade.Data calldata trade
     )
         external
         view
-        returns (GPv2Signing.RecoveredOrder memory recoveredOrder, uint256 mem)
+        returns (RecoveredOrder memory recoveredOrder, uint256 mem)
     {
-        bytes32 domainSeparator = DOMAIN_SEPARATOR;
-        recoveredOrder = GPv2Signing.allocateRecoveredOrder();
+        recoveredOrder = allocateRecoveredOrder();
 
         // NOTE: Solidity stores the free memory pointer at address 0x40. Read
         // it before and after calling `processOrder` to ensure that there are
@@ -37,7 +25,7 @@ contract GPv2SigningTestInterface {
             mem := mload(0x40)
         }
 
-        recoveredOrder.recoverOrderFromTrade(domainSeparator, tokens, trade);
+        recoverOrderFromTrade(recoveredOrder, tokens, trade);
 
         // solhint-disable-next-line no-inline-assembly
         assembly {
@@ -50,10 +38,6 @@ contract GPv2SigningTestInterface {
         GPv2Signing.Scheme signingScheme,
         bytes calldata signature
     ) external view returns (address owner) {
-        (, owner) = order.recoverOrderSigner(
-            DOMAIN_SEPARATOR,
-            signingScheme,
-            signature
-        );
+        (, owner) = recoverOrderSigner(order, signingScheme, signature);
     }
 }

test/GPv2Settlement.test.ts

@@ -52,26 +52,6 @@ describe("GPv2Settlement", () => {
     testDomain = domain(chainId, settlement.address);
   });
 
-  describe("domainSeparator", () => {
-    it("should have an EIP-712 domain separator", async () => {
-      expect(await settlement.domainSeparator()).to.equal(
-        ethers.utils._TypedDataEncoder.hashDomain(testDomain),
-      );
-    });
-
-    it("should have a different replay protection for each deployment", async () => {
-      const GPv2Settlement = await ethers.getContractFactory(
-        "GPv2SettlementTestInterface",
-        deployer,
-      );
-      const settlement2 = await GPv2Settlement.deploy(authenticator.address);
-
-      expect(await settlement.domainSeparator()).to.not.equal(
-        await settlement2.domainSeparator(),
-      );
-    });
-  });
-
   describe("authenticator", () => {
     it("should be set to the authenticator the contract was initialized with", async () => {
       expect(await settlement.authenticator()).to.equal(authenticator.address);