From f50a63aa9ed5e1aed19df1a39a81e005b8ed119d Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Fri, 11 Nov 2022 10:26:57 -0800 Subject: [PATCH] Fix slow compilation with `via-ir` (#217) * refactor: change stdChains to a mapping * ci: add a test contract to compile Test in CI * fix: no more stack too deep (once https://github.com/foundry-rs/foundry/pull/3659 is merged) * ci: more granular build results * fix: add ABIEncoderV2 pragma to compilation test * refactor: cleanup code, more compilation tests * build: add view modifiers to silence compiler warnings * ci/refactor: more via-ir benchmarks, now in test dir * chore: udpate chain names * chore: fix test name * ci: reorder checks, add 0.6.12 --- .github/workflows/ci.yml | 43 ++++++-- foundry.toml | 3 +- src/StdCheats.sol | 116 +++++++-------------- src/Vm.sol | 7 ++ test/StdCheats.t.sol | 26 +++-- test/StdUtils.t.sol | 5 - test/compilation/CompilationScript.sol | 10 ++ test/compilation/CompilationScriptBase.sol | 10 ++ test/compilation/CompilationTest.sol | 10 ++ test/compilation/CompilationTestBase.sol | 10 ++ 10 files changed, 138 insertions(+), 102 deletions(-) create mode 100644 test/compilation/CompilationScript.sol create mode 100644 test/compilation/CompilationScriptBase.sol create mode 100644 test/compilation/CompilationTest.sol create mode 100644 test/compilation/CompilationTestBase.sol diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9971ff9e..e6becd09 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,12 +21,43 @@ jobs: - name: Install dependencies run: forge install - - name: Check backward compatibility - run: | - forge build --skip test --use solc:0.8.0 - forge build --skip test --use solc:0.7.6 - forge build --skip test --use solc:0.7.0 - forge build --skip test --use solc:0.6.2 + # Backwards compatibility checks. + - name: Check compatibility with 0.8.0 + if: always() + run: forge build --skip test --use solc:0.8.0 + + - name: Check compatibility with 0.7.6 + if: always() + run: forge build --skip test --use solc:0.7.6 + + - name: Check compatibility with 0.7.0 + if: always() + run: forge build --skip test --use solc:0.7.0 + + - name: Check compatibility with 0.6.12 + if: always() + run: forge build --skip test --use solc:0.6.12 + + - name: Check compatibility with 0.6.2 + if: always() + run: forge build --skip test --use solc:0.6.2 + + # via-ir compilation time checks. + - name: Measure compilation time of Test with 0.8.17 --via-ir + if: always() + run: forge build --skip test --contracts test/compilation/CompilationTest.sol --use solc:0.8.17 --via-ir + + - name: Measure compilation time of TestBase with 0.8.17 --via-ir + if: always() + run: forge build --skip test --contracts test/compilation/CompilationTestBase.sol --use solc:0.8.17 --via-ir + + - name: Measure compilation time of Script with 0.8.17 --via-ir + if: always() + run: forge build --skip test --contracts test/compilation/CompilationScript.sol --use solc:0.8.17 --via-ir + + - name: Measure compilation time of ScriptBase with 0.8.17 --via-ir + if: always() + run: forge build --skip test --contracts test/compilation/CompilationScriptBase.sol --use solc:0.8.17 --via-ir test: runs-on: ubuntu-latest diff --git a/foundry.toml b/foundry.toml index 4f93dbfb..13495fd2 100644 --- a/foundry.toml +++ b/foundry.toml @@ -2,11 +2,10 @@ fs_permissions = [{ access = "read-write", path = "./"}] [rpc_endpoints] -# We intentionally use both dashes and underscores in the key names to ensure both are supported. # The RPC URLs below match the StdChains URLs but append a trailing slash for testing. mainnet = "https://api.mycryptoapi.com/eth/" optimism_goerli = "https://goerli.optimism.io/" -arbitrum-one-goerli = "https://goerli-rollup.arbitrum.io/rpc/" +arbitrum_one_goerli = "https://goerli-rollup.arbitrum.io/rpc/" [fmt] # These are all the `forge fmt` defaults. diff --git a/src/StdCheats.sol b/src/StdCheats.sol index 4441b2a3..e5018e13 100644 --- a/src/StdCheats.sol +++ b/src/StdCheats.sol @@ -26,27 +26,8 @@ abstract contract StdCheatsSafe { string rpcUrl; } - struct Chains { - Chain Anvil; - Chain Hardhat; - Chain Mainnet; - Chain Goerli; - Chain Sepolia; - Chain Optimism; - Chain OptimismGoerli; - Chain ArbitrumOne; - Chain ArbitrumOneGoerli; - Chain ArbitrumNova; - Chain Polygon; - Chain PolygonMumbai; - Chain Avalanche; - Chain AvalancheFuji; - Chain BnbSmartChain; - Chain BnbSmartChainTestnet; - Chain GnosisChain; - } - - Chains stdChains; + // Maps from a chain's name (matching what's in the `foundry.toml` file) to chain data. + mapping(string => Chain) internal stdChains; // Data structures to parse Transaction objects from the broadcast artifact // that conform to EIP1559. The Raw structs is what is parsed from the JSON @@ -227,63 +208,33 @@ abstract contract StdCheatsSafe { function _constructor() private returns (uint256) { // Initialize `stdChains` with the defaults. - stdChains = Chains({ - Anvil: Chain("Anvil", 31337, "http://127.0.0.1:8545"), - Hardhat: Chain("Hardhat", 31337, "http://127.0.0.1:8545"), - Mainnet: Chain("Mainnet", 1, "https://api.mycryptoapi.com/eth"), - Goerli: Chain("Goerli", 5, "https://goerli.infura.io/v3/84842078b09946638c03157f83405213"), // Default Infura key from ethers.js: https://github.com/ethers-io/ethers.js/blob/c80fcddf50a9023486e9f9acb1848aba4c19f7b6/packages/providers/src.ts/infura-provider.ts - Sepolia: Chain("Sepolia", 11155111, "https://rpc.sepolia.dev"), - Optimism: Chain("Optimism", 10, "https://mainnet.optimism.io"), - OptimismGoerli: Chain("OptimismGoerli", 420, "https://goerli.optimism.io"), - ArbitrumOne: Chain("ArbitrumOne", 42161, "https://arb1.arbitrum.io/rpc"), - ArbitrumOneGoerli: Chain("ArbitrumOneGoerli", 421613, "https://goerli-rollup.arbitrum.io/rpc"), - ArbitrumNova: Chain("ArbitrumNova", 42170, "https://nova.arbitrum.io/rpc"), - Polygon: Chain("Polygon", 137, "https://polygon-rpc.com"), - PolygonMumbai: Chain("PolygonMumbai", 80001, "https://rpc-mumbai.matic.today"), - Avalanche: Chain("Avalanche", 43114, "https://api.avax.network/ext/bc/C/rpc"), - AvalancheFuji: Chain("AvalancheFuji", 43113, "https://api.avax-test.network/ext/bc/C/rpc"), - BnbSmartChain: Chain("BnbSmartChain", 56, "https://bsc-dataseed1.binance.org"), - BnbSmartChainTestnet: Chain("BnbSmartChainTestnet", 97, "https://data-seed-prebsc-1-s1.binance.org:8545"), - GnosisChain: Chain("GnosisChain", 100, "https://rpc.gnosischain.com") - }); + stdChains["anvil"] = Chain("Anvil", 31337, "http://127.0.0.1:8545"); + stdChains["hardhat"] = Chain("Hardhat", 31337, "http://127.0.0.1:8545"); + stdChains["mainnet"] = Chain("Mainnet", 1, "https://api.mycryptoapi.com/eth"); + stdChains["goerli"] = Chain("Goerli", 5, "https://goerli.infura.io/v3/6770454bc6ea42c58aac12978531b93f"); + stdChains["sepolia"] = Chain("Sepolia", 11155111, "https://rpc.sepolia.dev"); + stdChains["optimism"] = Chain("Optimism", 10, "https://mainnet.optimism.io"); + stdChains["optimism_goerli"] = Chain("Optimism Goerli", 420, "https://goerli.optimism.io"); + stdChains["arbitrum_one"] = Chain("Arbitrum One", 42161, "https://arb1.arbitrum.io/rpc"); + stdChains["arbitrum_one_goerli"] = Chain("Arbitrum One Goerli", 421613, "https://goerli-rollup.arbitrum.io/rpc"); + stdChains["arbitrum_nova"] = Chain("Arbitrum Nova", 42170, "https://nova.arbitrum.io/rpc"); + stdChains["polygon"] = Chain("Polygon", 137, "https://polygon-rpc.com"); + stdChains["polygon_mumbai"] = Chain("Polygon Mumbai", 80001, "https://rpc-mumbai.matic.today"); + stdChains["avalanche"] = Chain("Avalanche", 43114, "https://api.avax.network/ext/bc/C/rpc"); + stdChains["avalanche_fuji"] = Chain("Avalanche Fuji", 43113, "https://api.avax-test.network/ext/bc/C/rpc"); + stdChains["bnb_smart_chain"] = Chain("BNB Smart Chain", 56, "https://bsc-dataseed1.binance.org"); + stdChains["bnb_smart_chain_testnet"] = Chain("BNB Smart Chain Testnet", 97, "https://data-seed-prebsc-1-s1.binance.org:8545");// forgefmt: disable-line + stdChains["gnosis_chain"] = Chain("Gnosis Chain", 100, "https://rpc.gnosischain.com"); // Loop over RPC URLs in the config file to replace the default RPC URLs - (string[2][] memory rpcs) = vm.rpcUrls(); + Vm.Rpc[] memory rpcs = vm.rpcUrlStructs(); for (uint256 i = 0; i < rpcs.length; i++) { - (string memory name, string memory rpcUrl) = (rpcs[i][0], rpcs[i][1]); - // forgefmt: disable-start - if (isEqual(name, "anvil")) stdChains.Anvil.rpcUrl = rpcUrl; - else if (isEqual(name, "hardhat")) stdChains.Hardhat.rpcUrl = rpcUrl; - else if (isEqual(name, "mainnet")) stdChains.Mainnet.rpcUrl = rpcUrl; - else if (isEqual(name, "goerli")) stdChains.Goerli.rpcUrl = rpcUrl; - else if (isEqual(name, "sepolia")) stdChains.Sepolia.rpcUrl = rpcUrl; - else if (isEqual(name, "optimism")) stdChains.Optimism.rpcUrl = rpcUrl; - else if (isEqual(name, "optimism_goerli", "optimism-goerli")) stdChains.OptimismGoerli.rpcUrl = rpcUrl; - else if (isEqual(name, "arbitrum_one", "arbitrum-one")) stdChains.ArbitrumOne.rpcUrl = rpcUrl; - else if (isEqual(name, "arbitrum_one_goerli", "arbitrum-one-goerli")) stdChains.ArbitrumOneGoerli.rpcUrl = rpcUrl; - else if (isEqual(name, "arbitrum_nova", "arbitrum-nova")) stdChains.ArbitrumNova.rpcUrl = rpcUrl; - else if (isEqual(name, "polygon")) stdChains.Polygon.rpcUrl = rpcUrl; - else if (isEqual(name, "polygon_mumbai", "polygon-mumbai")) stdChains.PolygonMumbai.rpcUrl = rpcUrl; - else if (isEqual(name, "avalanche")) stdChains.Avalanche.rpcUrl = rpcUrl; - else if (isEqual(name, "avalanche_fuji", "avalanche-fuji")) stdChains.AvalancheFuji.rpcUrl = rpcUrl; - else if (isEqual(name, "bnb_smart_chain", "bnb-smart-chain")) stdChains.BnbSmartChain.rpcUrl = rpcUrl; - else if (isEqual(name, "bnb_smart_chain_testnet", "bnb-smart-chain-testnet")) stdChains.BnbSmartChainTestnet.rpcUrl = rpcUrl; - else if (isEqual(name, "gnosis_chain", "gnosis-chain")) stdChains.GnosisChain.rpcUrl = rpcUrl; - // forgefmt: disable-end + stdChains[rpcs[i].name].rpcUrl = rpcs[i].url; } return 0; } - function isEqual(string memory a, string memory b) private pure returns (bool) { - return keccak256(abi.encode(a)) == keccak256(abi.encode(b)); - } - - function isEqual(string memory a, string memory b, string memory c) private pure returns (bool) { - return - keccak256(abi.encode(a)) == keccak256(abi.encode(b)) || keccak256(abi.encode(a)) == keccak256(abi.encode(c)); - } - - function assumeNoPrecompiles(address addr) internal virtual { + function assumeNoPrecompiles(address addr) internal view virtual { // Assembly required since `block.chainid` was introduced in 0.8.0. uint256 chainId; assembly { @@ -292,7 +243,7 @@ abstract contract StdCheatsSafe { assumeNoPrecompiles(addr, chainId); } - function assumeNoPrecompiles(address addr, uint256 chainId) internal virtual { + function assumeNoPrecompiles(address addr, uint256 chainId) internal view virtual { // Note: For some chains like Optimism these are technically predeploys (i.e. bytecode placed at a specific // address), but the same rationale for excluding them applies so we include those too. @@ -300,13 +251,13 @@ abstract contract StdCheatsSafe { vm.assume(addr < address(0x1) || addr > address(0x9)); // forgefmt: disable-start - if (chainId == stdChains.Optimism.chainId || chainId == stdChains.OptimismGoerli.chainId) { + if (chainId == stdChains["optimism"].chainId || chainId == stdChains["optimism_goerli"].chainId) { // https://github.com/ethereum-optimism/optimism/blob/eaa371a0184b56b7ca6d9eb9cb0a2b78b2ccd864/op-bindings/predeploys/addresses.go#L6-L21 vm.assume(addr < address(0x4200000000000000000000000000000000000000) || addr > address(0x4200000000000000000000000000000000000800)); - } else if (chainId == stdChains.ArbitrumOne.chainId || chainId == stdChains.ArbitrumOneGoerli.chainId) { + } else if (chainId == stdChains["arbitrum_one"].chainId || chainId == stdChains["arbitrum_one_goerli"].chainId) { // https://developer.arbitrum.io/useful-addresses#arbitrum-precompiles-l2-same-on-all-arb-chains vm.assume(addr < address(0x0000000000000000000000000000000000000064) || addr > address(0x0000000000000000000000000000000000000068)); - } else if (chainId == stdChains.Avalanche.chainId || chainId == stdChains.AvalancheFuji.chainId) { + } else if (chainId == stdChains["avalanche"].chainId || chainId == stdChains["avalanche_fuji"].chainId) { // https://github.com/ava-labs/subnet-evm/blob/47c03fd007ecaa6de2c52ea081596e0a88401f58/precompile/params.go#L18-L59 vm.assume(addr < address(0x0100000000000000000000000000000000000000) || addr > address(0x01000000000000000000000000000000000000ff)); vm.assume(addr < address(0x0200000000000000000000000000000000000000) || addr > address(0x02000000000000000000000000000000000000FF)); @@ -315,7 +266,12 @@ abstract contract StdCheatsSafe { // forgefmt: disable-end } - function readEIP1559ScriptArtifact(string memory path) internal virtual returns (EIP1559ScriptArtifact memory) { + function readEIP1559ScriptArtifact(string memory path) + internal + view + virtual + returns (EIP1559ScriptArtifact memory) + { string memory data = vm.readFile(path); bytes memory parsedData = vm.parseJson(data); RawEIP1559ScriptArtifact memory rawArtifact = abi.decode(parsedData, (RawEIP1559ScriptArtifact)); @@ -367,14 +323,14 @@ abstract contract StdCheatsSafe { return txDetail; } - function readTx1559s(string memory path) internal virtual returns (Tx1559[] memory) { + function readTx1559s(string memory path) internal view virtual returns (Tx1559[] memory) { string memory deployData = vm.readFile(path); bytes memory parsedDeployData = vm.parseJson(deployData, ".transactions"); RawTx1559[] memory rawTxs = abi.decode(parsedDeployData, (RawTx1559[])); return rawToConvertedEIPTx1559s(rawTxs); } - function readTx1559(string memory path, uint256 index) internal virtual returns (Tx1559 memory) { + function readTx1559(string memory path, uint256 index) internal view virtual returns (Tx1559 memory) { string memory deployData = vm.readFile(path); string memory key = string(abi.encodePacked(".transactions[", vm.toString(index), "]")); bytes memory parsedDeployData = vm.parseJson(deployData, key); @@ -383,14 +339,14 @@ abstract contract StdCheatsSafe { } // Analogous to readTransactions, but for receipts. - function readReceipts(string memory path) internal virtual returns (Receipt[] memory) { + function readReceipts(string memory path) internal view virtual returns (Receipt[] memory) { string memory deployData = vm.readFile(path); bytes memory parsedDeployData = vm.parseJson(deployData, ".receipts"); RawReceipt[] memory rawReceipts = abi.decode(parsedDeployData, (RawReceipt[])); return rawToConvertedReceipts(rawReceipts); } - function readReceipt(string memory path, uint256 index) internal virtual returns (Receipt memory) { + function readReceipt(string memory path, uint256 index) internal view virtual returns (Receipt memory) { string memory deployData = vm.readFile(path); string memory key = string(abi.encodePacked(".receipts[", vm.toString(index), "]")); bytes memory parsedDeployData = vm.parseJson(deployData, key); diff --git a/src/Vm.sol b/src/Vm.sol index 6b2343e1..d6828c52 100644 --- a/src/Vm.sol +++ b/src/Vm.sol @@ -16,6 +16,11 @@ interface VmSafe { address emitter; } + struct Rpc { + string name; + string url; + } + // Loads a storage slot from an address (who, slot) function load(address, bytes32) external view returns (bytes32); // Signs data, (privateKey, digest) => (v, r, s) @@ -191,6 +196,8 @@ interface VmSafe { function rpcUrl(string calldata) external view returns (string memory); // Returns all rpc urls and their aliases `[alias, url][]` function rpcUrls() external view returns (string[2][] memory); + // Returns all rpc urls and their aliases as structs. + function rpcUrlStructs() external view returns (Rpc[] memory); // If the condition is false, discard this run's fuzz inputs and generate new ones. function assume(bool) external pure; } diff --git a/test/StdCheats.t.sol b/test/StdCheats.t.sol index 0580c2d7..4bf629a0 100644 --- a/test/StdCheats.t.sol +++ b/test/StdCheats.t.sol @@ -186,7 +186,7 @@ contract StdCheatsTest is Test { assertEq(txDetail.value, 0); } - function testReadEIP1559Transaction() public { + function testReadEIP1559Transaction() public view { string memory root = vm.projectRoot(); string memory path = string.concat(root, "/test/fixtures/broadcast.log.json"); uint256 index = 0; @@ -194,7 +194,7 @@ contract StdCheatsTest is Test { transaction; } - function testReadEIP1559Transactions() public { + function testReadEIP1559Transactions() public view { string memory root = vm.projectRoot(); string memory path = string.concat(root, "/test/fixtures/broadcast.log.json"); Tx1559[] memory transactions = readTx1559s(path); @@ -212,7 +212,7 @@ contract StdCheatsTest is Test { ); } - function testReadReceipts() public { + function testReadReceipts() public view { string memory root = vm.projectRoot(); string memory path = string.concat(root, "/test/fixtures/broadcast.log.json"); Receipt[] memory receipts = readReceipts(path); @@ -229,14 +229,14 @@ contract StdCheatsTest is Test { function testChainRpcInitialization() public { // RPCs specified in `foundry.toml` should be updated. - assertEq(stdChains.Mainnet.rpcUrl, "https://api.mycryptoapi.com/eth/"); - assertEq(stdChains.OptimismGoerli.rpcUrl, "https://goerli.optimism.io/"); - assertEq(stdChains.ArbitrumOneGoerli.rpcUrl, "https://goerli-rollup.arbitrum.io/rpc/"); + assertEq(stdChains["mainnet"].rpcUrl, "https://api.mycryptoapi.com/eth/"); + assertEq(stdChains["optimism_goerli"].rpcUrl, "https://goerli.optimism.io/"); + assertEq(stdChains["arbitrum_one_goerli"].rpcUrl, "https://goerli-rollup.arbitrum.io/rpc/"); // Other RPCs should remain unchanged. - assertEq(stdChains.Anvil.rpcUrl, "http://127.0.0.1:8545"); - assertEq(stdChains.Hardhat.rpcUrl, "http://127.0.0.1:8545"); - assertEq(stdChains.Sepolia.rpcUrl, "https://rpc.sepolia.dev"); + assertEq(stdChains["anvil"].rpcUrl, "http://127.0.0.1:8545"); + assertEq(stdChains["hardhat"].rpcUrl, "http://127.0.0.1:8545"); + assertEq(stdChains["sepolia"].rpcUrl, "https://rpc.sepolia.dev"); } // Ensure we can connect to the default RPC URL for each chain. @@ -247,6 +247,14 @@ contract StdCheatsTest is Test { vm.createSelectFork(rpcUrl); } } + + function testAssumeNoPrecompiles(address addr) external { + assumeNoPrecompiles(addr, stdChains["optimism_goerli"].chainId); + assertTrue( + addr < address(1) || (addr > address(9) && addr < address(0x4200000000000000000000000000000000000000)) + || addr > address(0x4200000000000000000000000000000000000800) + ); + } } contract Bar { diff --git a/test/StdUtils.t.sol b/test/StdUtils.t.sol index 4782d82b..c1ed6d62 100644 --- a/test/StdUtils.t.sol +++ b/test/StdUtils.t.sol @@ -88,9 +88,4 @@ contract StdUtilsTest is Test { address create2Address = computeCreate2Address(salt, initcodeHash, deployer); assertEq(create2Address, 0xB147a5d25748fda14b463EB04B111027C290f4d3); } - - function testAssumeNoPrecompilesL1(address addr) external { - assumeNoPrecompiles(addr, stdChains.Mainnet.chainId); - assertTrue(addr < address(1) || addr > address(9)); - } } diff --git a/test/compilation/CompilationScript.sol b/test/compilation/CompilationScript.sol new file mode 100644 index 00000000..e205cfff --- /dev/null +++ b/test/compilation/CompilationScript.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: MIT +pragma solidity >=0.6.2 <0.9.0; + +pragma experimental ABIEncoderV2; + +import "../../src/Script.sol"; + +// The purpose of this contract is to benchmark compilation time to avoid accidentally introducing +// a change that results in very long compilation times with via-ir. See https://github.com/foundry-rs/forge-std/issues/207 +contract CompilationScript is Script {} diff --git a/test/compilation/CompilationScriptBase.sol b/test/compilation/CompilationScriptBase.sol new file mode 100644 index 00000000..ce8e0e95 --- /dev/null +++ b/test/compilation/CompilationScriptBase.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: MIT +pragma solidity >=0.6.2 <0.9.0; + +pragma experimental ABIEncoderV2; + +import "../../src/Script.sol"; + +// The purpose of this contract is to benchmark compilation time to avoid accidentally introducing +// a change that results in very long compilation times with via-ir. See https://github.com/foundry-rs/forge-std/issues/207 +contract CompilationScriptBase is ScriptBase {} diff --git a/test/compilation/CompilationTest.sol b/test/compilation/CompilationTest.sol new file mode 100644 index 00000000..9beeafeb --- /dev/null +++ b/test/compilation/CompilationTest.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: MIT +pragma solidity >=0.6.2 <0.9.0; + +pragma experimental ABIEncoderV2; + +import "../../src/Test.sol"; + +// The purpose of this contract is to benchmark compilation time to avoid accidentally introducing +// a change that results in very long compilation times with via-ir. See https://github.com/foundry-rs/forge-std/issues/207 +contract CompilationTest is Test {} diff --git a/test/compilation/CompilationTestBase.sol b/test/compilation/CompilationTestBase.sol new file mode 100644 index 00000000..e993535b --- /dev/null +++ b/test/compilation/CompilationTestBase.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: MIT +pragma solidity >=0.6.2 <0.9.0; + +pragma experimental ABIEncoderV2; + +import "../../src/Test.sol"; + +// The purpose of this contract is to benchmark compilation time to avoid accidentally introducing +// a change that results in very long compilation times with via-ir. See https://github.com/foundry-rs/forge-std/issues/207 +contract CompilationTestBase is TestBase {}