Backmerge 1.3.0 libs.0 (safe-fndn#383)

rmeissner · mmv08 · web-flow · commit ee9295730765 · 2022-05-16T18:53:30.000+02:00
* move signMessageLib out of examples and deploy it (safe-fndn#333) * fix signmessagelib deployment * add polygon config * add bsc to config * add arbitrum config * add fantom testnet configuration * Extended SignMessageLib tests (safe-fndn#347) * add test for using invalid operation with signmessagelib * extract singed messages mapping storage slot to a constant * add test for SignMessageLib storage manipulations * update lockfile * update description of test that checks storage slots * change nonce visibility to internal * use EOA acc to test DELEGATECALL enforcement * test desc correction * test desc correction 2 * Feature: Storage tests for GnosisSafeStorage and GnosisSafe contracts (safe-fndn#348) * gnosissafestorage tests wip * trying to hack the matcher * add mastercopy storage tests * test disc fix * formatting improvement * use startsWith in test, change mastercopy to singleton * Support custom singleton factory (safe-fndn#344) * PoC of custom singleton factory * Adjust readme * Bump node version * Use npm packages * Update solidity coverage * Update changelog§ (safe-fndn#350) * Improve readme * Update factory * fix tests naming (safe-fndn#401) * Remove duplicate SignMessage contract Co-authored-by: Mikhail Mikheev <mmvsha73@gmail.com>
diff --git a/.env.sample b/.env.sample
@@ -3,4 +3,6 @@ MNEMONIC=""
 INFURA_KEY=""
 # Used for custom network
 NODE_URL=""
-ETHERSCAN_API_KEY=""
+ETHERSCAN_API_KEY=""
+# Use the Safe singleton factory for singleton deployment. This is required if EIP-155 is enforce on a chain.
+# CUSTOM_DETERMINISTIC_DEPLOYMENT="true"
diff --git a/.nvmrc b/.nvmrc
@@ -1 +1 @@
-v16.7.0
+v16.7.0
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,77 @@
 
 This changelog only contains changes starting from version 1.3.0
 
+# Version 1.3.0-libs.0
+
+## Compiler settings
+
+Solidity compiler: [0.7.6](https://github.com/ethereum/solidity/releases/tag/v0.7.6) (more info see issue [#251](https://github.com/gnosis/safe-contracts/issues/251))
+
+Solidity optimizer: `disabled`
+
+## Expected addresses with [Deterministic Deployment Proxy](https://github.com/Arachnid/deterministic-deployment-proxy) (default)
+
+### Core contracts
+- `GnosisSafe` at `0xd9Db270c1B5E3Bd161E8c8503c55cEABeE709552`
+- `GnosisSafeL2` at `0x3E5c63644E683549055b9Be8653de26E0B4CD36E`
+### Factory contracts
+- `GnosisSafeProxyFactory` at `0xa6B71E26C5e0845f74c812102Ca7114b6a896AB2`
+### Handler contracts
+- `DefaultCallbackHandler` at `0x1AC114C2099aFAf5261731655Dc6c306bFcd4Dbd`
+- `CompatibilityFallbackHandler` at `0xf48f2B2d2a534e402487b3ee7C18c33Aec0Fe5e4`
+### Lib contracts
+- `MultiSend` at `0xA238CBeb142c10Ef7Ad8442C6D1f9E89e07e7761`
+- `MultiSendCallOnly` at `0x40A2aCCbd92BCA938b02010E17A5b8929b49130D`
+- `CreateCall` at `0x7cbB62EaA69F79e6873cD1ecB2392971036cFAa4`
+- `SignMessageLib` at `0xA65387F16B013cf2Af4605Ad8aA5ec25a2cbA3a2`
+### Storage reader contracts
+- `SimulateTxAccessor` at `0x59AD6735bCd8152B84860Cb256dD9e96b85F69Da`
+
+## Expected addresses with [Safe Singleton Factory](https://github.com/gnosis/safe-singleton-factory)
+
+### Core contracts
+- `GnosisSafe` at `0x69f4D1788e39c87893C980c06EdF4b7f686e2938`
+- `GnosisSafeL2` at `0xfb1bffC9d739B8D520DaF37dF666da4C687191EA`
+### Factory contracts
+- `GnosisSafeProxyFactory` at `0xC22834581EbC8527d974F8a1c97E1bEA4EF910BC`
+### Handler contracts
+- `DefaultCallbackHandler` at `0x3d8E605B02032A941Cfe26897Ca94d77a5BC24b3`
+- `CompatibilityFallbackHandler` at `0x017062a1dE2FE6b99BE3d9d37841FeD19F573804`
+### Lib contracts
+- `MultiSend` at `0x998739BFdAAdde7C933B942a68053933098f9EDa`
+- `MultiSendCallOnly` at `0xA1dabEF33b3B82c7814B6D82A79e50F4AC44102B`
+- `CreateCall` at `0xB19D6FFc2182150F8Eb585b79D4ABcd7C5640A9d`
+- `SignMessageLib` at `0x98FFBBF51bb33A056B08ddf711f289936AafF717`
+### Storage reader contracts
+- `SimulateTxAccessor` at `0x727a77a074D1E6c4530e814F89E618a3298FC044`
+
+## Changes 
+
+### Deployment process
+
+To support deployment to networks that require replay protection support for the [Safe Singleton Factory](https://github.com/gnosis/safe-singleton-factory) has been added. This will result in an additional set of deterministic addresses which are listed above.
+
+### Libraries
+
+The following libraries have been marked as production ready.
+
+#### SignMessageLib
+
+File: [`contracts/libraries/SignMessage.sol`](https://github.com/gnosis/safe-contracts/blob/e57df14ea96dc7dabf93f041c7531f2ab6755c76/contracts/libraries/SignMessage.sol)
+
+Expected behaviour:
+
+The library is meant as a compatibility tool for the removed `signMessage` function from the pre-1.3.0 Safe contracts. It has the same signature and assumes the same storage layout as the previous Safe contract versions. After calling this function with a massage, the hash of that message should be marked as executed in the `signedMessages` mapping.
+
+#### GnosisSafeStorage
+
+
+File: [`contracts/libraries/GnosisSafeStorage.sol`](https://github.com/gnosis/safe-contracts/blob/e57df14ea96dc7dabf93f041c7531f2ab6755c76/contracts/libraries/GnosisSafeStorage.sol)
+
+Expected behaviour:
+
+The contract contains the basic storage layout of the `GnosisSafe.sol` contract and can be used by library contracts to access the storage variables.
+
 # Version 1.3.0
 
 ## Compiler settings
diff --git a/README.md b/README.md
@@ -64,6 +64,14 @@ The resulting addresses should be on all networks the same.
 
 Note: Address will vary if contract code is changed or a different Solidity version is used.
 
+#### Replay protection (EIP-155)
+
+Some networks require replay protection. This is not possible with the default deployment process as it relies on a presigned transaction without replay protection (see https://github.com/Arachnid/deterministic-deployment-proxy). 
+
+It is possible to enable deployment via a different determinisitic deployment proxy (https://github.com/gnosis/safe-singleton-factory). To enable this the `CUSTOM_DETERMINISTIC_DEPLOYMENT` env var has to be set to `true` (see `.env.sample`). To make sure that the latest version of this package is install, make sure to run `yarn add @gnosis.pm/safe-singleton-factory` before deployment.
+
+Note: This will result in different addresses compared to the default deployment process.
+
 ### Verify contract
 
 This command will use the deployment artifacts to compile the contracts and compare them to the onchain code
diff --git a/contracts/examples/libraries/Migrate_1_3_0_to_1_2_0.sol b/contracts/examples/libraries/Migrate_1_3_0_to_1_2_0.sol
@@ -25,7 +25,7 @@ contract Migration is GnosisSafeStorage {
         require(address(this) != migrationSingleton, "Migration should only be called via delegatecall");
         // Master copy address cannot be null.
         singleton = safe120Singleton;
-        domainSeparator = keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, this));
+        _deprecatedDomainSeparator = keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, this));
         emit ChangedMasterCopy(singleton);
     }
 }
diff --git a/contracts/libraries/GnosisSafeStorage.sol b/contracts/libraries/GnosisSafeStorage.sol
@@ -14,8 +14,8 @@ contract GnosisSafeStorage {
     uint256 internal threshold;
 
     // From /GnosisSafe.sol
-    bytes32 internal nonce;
-    bytes32 internal domainSeparator;
+    uint256 internal nonce;
+    bytes32 internal _deprecatedDomainSeparator;
     mapping(bytes32 => uint256) internal signedMessages;
     mapping(address => mapping(bytes32 => uint256)) internal approvedHashes;
 }
diff --git a/contracts/libraries/SignMessageLib.sol b/contracts/libraries/SignMessageLib.sol
diff --git a/hardhat.config.ts b/hardhat.config.ts
@@ -1,10 +1,11 @@
+import type { HardhatUserConfig, HttpNetworkUserConfig } from "hardhat/types";
 import "@nomiclabs/hardhat-etherscan";
 import "@nomiclabs/hardhat-waffle";
 import "solidity-coverage";
 import "hardhat-deploy";
 import dotenv from "dotenv";
-import type { HardhatUserConfig, HttpNetworkUserConfig } from "hardhat/types";
 import yargs from "yargs";
+import { getSingletonFactoryInfo } from "@gnosis.pm/safe-singleton-factory";
 
 const argv = yargs
   .option("network", {
@@ -16,7 +17,7 @@ const argv = yargs
 
 // Load environment variables.
 dotenv.config();
-const { NODE_URL, INFURA_KEY, MNEMONIC, ETHERSCAN_API_KEY, PK, SOLIDITY_VERSION, SOLIDITY_SETTINGS } = process.env;
+const { NODE_URL, INFURA_KEY, MNEMONIC, ETHERSCAN_API_KEY, PK, SOLIDITY_VERSION, SOLIDITY_SETTINGS, CUSTOM_DETERMINISTIC_DEPLOYMENT } = process.env;
 
 const DEFAULT_MNEMONIC =
   "candy maple cake sugar pudding cream honey rich smooth crumble sweet treat";
@@ -39,10 +40,23 @@ if (["mainnet", "rinkeby", "kovan", "goerli", "ropsten", "mumbai", "polygon"].in
 import "./src/tasks/local_verify"
 import "./src/tasks/deploy_contracts"
 import "./src/tasks/show_codesize"
+import { BigNumber } from "@ethersproject/bignumber";
 
 const primarySolidityVersion = SOLIDITY_VERSION || "0.7.6"
 const soliditySettings = !!SOLIDITY_SETTINGS ? JSON.parse(SOLIDITY_SETTINGS) : undefined
 
+const deterministicDeployment = CUSTOM_DETERMINISTIC_DEPLOYMENT == "true" ?
+  (network: string) => {
+    const info = getSingletonFactoryInfo(parseInt(network))
+    if (!info) return undefined
+    return {
+      factory: info.address,
+      deployer: info.signerAddress,
+      funding: BigNumber.from(info.gasLimit).mul(BigNumber.from(info.gasPrice)).toString(),
+      signedTx: info.transaction
+    }
+  } : undefined
+
 const userConfig: HardhatUserConfig = {
   paths: {
     artifacts: "build/artifacts",
@@ -103,7 +117,20 @@ const userConfig: HardhatUserConfig = {
       ...sharedNetworkConfig,
       url: `https://volta-rpc.energyweb.org`,
     },
+    bsc: {
+      ...sharedNetworkConfig,
+      url: `https://bsc-dataseed.binance.org/`,
+    },
+    arbitrum: {
+      ...sharedNetworkConfig,
+      url: `https://arb1.arbitrum.io/rpc`,
+    },
+    fantomTestnet: {
+      ...sharedNetworkConfig,
+      url: `https://rpc.testnet.fantom.network/`,
+    }
   },
+  deterministicDeployment,
   namedAccounts: {
     deployer: 0,
   },
diff --git a/package.json b/package.json
@@ -19,6 +19,7 @@
     "test": "hardhat test",
     "coverage": "hardhat coverage",
     "benchmark": "yarn test benchmark/*.ts",
+    "deploy-custom": "rm -rf deployments/custom && yarn deploy-all custom",
     "deploy-all": "hardhat deploy-contracts --network",
     "deploy": "hardhat deploy --network",
     "lint": "yarn lint:sol && yarn lint:ts",
@@ -44,16 +45,17 @@
   },
   "devDependencies": {
     "@gnosis.pm/mock-contract": "^4.0.0",
+    "@gnosis.pm/safe-singleton-factory": "^1.0.3",
     "@nomiclabs/hardhat-ethers": "^2.0.0",
     "@nomiclabs/hardhat-etherscan": "^2.1.0",
     "@nomiclabs/hardhat-waffle": "^2.0.0",
     "@openzeppelin/contracts": "^3.4.0",
     "@types/chai": "^4.2.14",
     "@types/mocha": "^8.2.0",
     "@types/node": "^14.14.21",
+    "@types/yargs": "^15.0.10",
     "@typescript-eslint/eslint-plugin": "^4.7.0",
     "@typescript-eslint/parser": "^4.7.0",
-    "@types/yargs": "^15.0.10",
     "argv": "^0.0.2",
     "chai": "^4.2.0",
     "debug": "^4.2.0",
@@ -66,14 +68,14 @@
     "ethereum-waffle": "^3.3.0",
     "ethers": "^5.1.4",
     "hardhat": "^2.2.1",
-    "hardhat-deploy": "^0.7.5",
+    "hardhat-deploy": "0.9.2",
     "husky": "^5.1.3",
     "prettier": "^2.1.2",
     "prettier-plugin-solidity": "^1.0.0-alpha.60",
     "solc": "0.7.6",
     "solhint": "^3.3.2",
     "solhint-plugin-prettier": "^0.0.5",
-    "solidity-coverage": "^0.7.16",
+    "solidity-coverage": "^0.7.17",
     "ts-node": "^9.1.1",
     "typescript": "^4.2.4",
     "yargs": "^16.1.1"
diff --git a/src/deploy/deploy_libraries.ts b/src/deploy/deploy_libraries.ts
@@ -8,6 +8,7 @@ const deploy: DeployFunction = async function (
   const { deployer } = await getNamedAccounts();
   const { deploy } = deployments;
 
+
   await deploy("CreateCall", {
     from: deployer,
     args: [],
diff --git a/test/core/GnosisSafe.Storage.spec.ts b/test/core/GnosisSafe.Storage.spec.ts
@@ -0,0 +1,25 @@
+import { expect } from "chai";
+import hre from "hardhat";
+import "@nomiclabs/hardhat-ethers";
+import { getContractStorageLayout } from "../utils/storage";
+
+describe("GnosisSafe", async () => {
+  it("follows storage layout defined by GnosisSafeStorage library", async () => {
+    const gnosisSafeStorageLayout = await getContractStorageLayout(
+      hre,
+      "GnosisSafeStorage"
+    );
+    const gnosisSafeSingletonStorageLayout = await getContractStorageLayout(
+      hre,
+      "GnosisSafe"
+    );
+    
+    // Chai doesn't have built-in matcher for deep object equality
+    // For the sake of simplicity I decided just to convert the object to a string and compare the strings
+    expect(
+      JSON.stringify(gnosisSafeSingletonStorageLayout).startsWith(
+        JSON.stringify(gnosisSafeStorageLayout)
+      )
+    ).to.be.true;
+  });
+});
diff --git a/test/libraries/GnosisSafeStorage.spec.ts b/test/libraries/GnosisSafeStorage.spec.ts
@@ -0,0 +1,52 @@
+import { expect } from "chai";
+import hre from "hardhat";
+import "@nomiclabs/hardhat-ethers";
+import { getContractStorageLayout } from "../utils/storage";
+
+const EXPECTED_LAYOUT = [
+  { name: "singleton", slot: "0", offset: 0, type: "t_address" },
+  {
+    name: "modules",
+    slot: "1",
+    offset: 0,
+    type: "t_mapping(t_address,t_address)",
+  },
+  {
+    name: "owners",
+    slot: "2",
+    offset: 0,
+    type: "t_mapping(t_address,t_address)",
+  },
+  { name: "ownerCount", slot: "3", offset: 0, type: "t_uint256" },
+  { name: "threshold", slot: "4", offset: 0, type: "t_uint256" },
+  { name: "nonce", slot: "5", offset: 0, type: "t_uint256" },
+  {
+    name: "_deprecatedDomainSeparator",
+    slot: "6",
+    offset: 0,
+    type: "t_bytes32",
+  },
+  {
+    name: "signedMessages",
+    slot: "7",
+    offset: 0,
+    type: "t_mapping(t_bytes32,t_uint256)",
+  },
+  {
+    name: "approvedHashes",
+    slot: "8",
+    offset: 0,
+    type: "t_mapping(t_address,t_mapping(t_bytes32,t_uint256))",
+  },
+];
+
+describe("GnosisSafeStorage", async () => {
+  it("follows the expected storage layout", async () => {
+    const gnosisSafeStorageLayout = await getContractStorageLayout(
+      hre,
+      "GnosisSafeStorage"
+    );
+
+    expect(gnosisSafeStorageLayout).to.deep.eq(EXPECTED_LAYOUT);
+  });
+});
diff --git a/test/libraries/SignMessageLib.spec.ts b/test/libraries/SignMessageLib.spec.ts
@@ -43,5 +43,52 @@ describe("SignMessageLib", async () => {
                 await safe.signedMessages(messageHash)
             ).to.be.eq(1)
         })
+
+        it('can be used only via DELEGATECALL opcode', async () => {
+            const { lib } = await setupTests()
+
+            expect(lib.signMessage("0xbaddad")).to.revertedWith("function selector was not recognized and there's no fallback function");
+        })
+
+        it("changes the expected storage slot without touching the most important ones", async () => {
+          const { safe, lib } = await setupTests();
+
+          const SIGNED_MESSAGES_MAPPING_STORAGE_SLOT = 7
+          const message = "no rugpull, funds must be safu";
+          const eip191MessageHash = hre.ethers.utils.hashMessage(message);
+          const safeInternalMsgHash = calculateSafeMessageHash(
+            safe,
+            hre.ethers.utils.hashMessage(message),
+            await chainId()
+          );
+          const expectedStorageSlot = hre.ethers.utils.keccak256(
+            hre.ethers.utils.defaultAbiCoder.encode(["bytes32", "uint256"], [safeInternalMsgHash, SIGNED_MESSAGES_MAPPING_STORAGE_SLOT])
+          );
+
+          const masterCopyAddressBeforeSigning = await hre.ethers.provider.getStorageAt(safe.address, 0);
+          const ownerCountBeforeSigning = await hre.ethers.provider.getStorageAt(safe.address, 3);
+          const thresholdBeforeSigning = await hre.ethers.provider.getStorageAt(safe.address, 4);
+          const nonceBeforeSigning = await hre.ethers.provider.getStorageAt(safe.address, 5);
+          const msgStorageSlotBeforeSigning = await hre.ethers.provider.getStorageAt(safe.address, expectedStorageSlot);
+          
+          expect(nonceBeforeSigning).to.be.eq(`0x${"0".padStart(64, "0")}`);
+          expect(await safe.signedMessages(safeInternalMsgHash)).to.be.eq(0);
+          expect(msgStorageSlotBeforeSigning).to.be.eq(`0x${"0".padStart(64, "0")}`);
+          
+          await executeContractCallWithSigners(safe, lib, "signMessage", [eip191MessageHash], [user1, user2], true);
+          
+          const masterCopyAddressAfterSigning = await hre.ethers.provider.getStorageAt(safe.address, 0);
+          const ownerCountAfterSigning = await hre.ethers.provider.getStorageAt(safe.address, 3);
+          const thresholdAfterSigning = await hre.ethers.provider.getStorageAt(safe.address, 4);
+          const nonceAfterSigning = await hre.ethers.provider.getStorageAt(safe.address, 5);
+          const msgStorageSlotAfterSigning = await hre.ethers.provider.getStorageAt(safe.address, expectedStorageSlot);
+          
+          expect(await safe.signedMessages(safeInternalMsgHash)).to.be.eq(1);
+          expect(masterCopyAddressBeforeSigning).to.be.eq(masterCopyAddressAfterSigning);
+          expect(thresholdBeforeSigning).to.be.eq(thresholdAfterSigning);
+          expect(ownerCountBeforeSigning).to.be.eq(ownerCountAfterSigning);
+          expect(nonceAfterSigning).to.be.eq(`0x${"1".padStart(64, "0")}`);
+          expect(msgStorageSlotAfterSigning).to.be.eq(`0x${"1".padStart(64, "0")}`);
+        });
     })
 })
diff --git a/test/utils/storage.ts b/test/utils/storage.ts
diff --git a/yarn.lock b/yarn.lock

Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ contract Migration is GnosisSafeStorage {`
`25`	`25`	`require(address(this) != migrationSingleton, "Migration should only be called via delegatecall");`
`26`	`26`	`// Master copy address cannot be null.`
`27`	`27`	`singleton = safe120Singleton;`
`28`		`- domainSeparator = keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, this));`
	`28`	`+ _deprecatedDomainSeparator = keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, this));`
`29`	`29`	`emit ChangedMasterCopy(singleton);`
`30`	`30`	`}`
`31`	`31`	`}`