From 7cecea9194ff4fd2328ca93a68116cbda4b989c4 Mon Sep 17 00:00:00 2001 From: vladbochok Date: Sun, 1 Oct 2023 14:22:51 +0200 Subject: [PATCH] fix(tests): Fix coverage in Foundry tests --- ethereum/contracts/zksync/DiamondInit.sol | 72 ++++++++++--------- ethereum/package.json | 1 + ethereum/src.ts/deploy.ts | 28 ++++---- .../unit_tests/l1_erc20_bridge_test.spec.ts | 32 +++++---- .../unit_tests/l1_weth_bridge_test.spec.ts | 32 +++++---- .../test/unit_tests/l2-upgrade.test.spec.ts | 26 +++---- ethereum/test/unit_tests/mailbox_test.spec.ts | 32 +++++---- ethereum/test/unit_tests/proxy_test.spec.ts | 26 +++---- 8 files changed, 133 insertions(+), 116 deletions(-) diff --git a/ethereum/contracts/zksync/DiamondInit.sol b/ethereum/contracts/zksync/DiamondInit.sol index 9140af05e..40adeae86 100644 --- a/ethereum/contracts/zksync/DiamondInit.sol +++ b/ethereum/contracts/zksync/DiamondInit.sol @@ -17,10 +17,8 @@ import {L2_TO_L1_LOG_SERIALIZE_SIZE, EMPTY_STRING_KECCAK, DEFAULT_L2_LOGS_TREE_R /// @dev The contract is used only once to initialize the diamond proxy. /// @dev The deployment process takes care of this contract's initialization. contract DiamondInit is Base { - /// @dev Initialize the implementation to prevent any possibility of a Parity hack. - constructor() reentrancyGuardInitializer {} - - /// @notice zkSync contract initialization + /// @notice Struct that holds all data needed for initializing zkSync Diamond Proxy. + /// @dev We use struct instead of raw parameters in `initialize` function to prevent "Stack too deep" error /// @param _verifier address of Verifier contract /// @param _governor address who can manage critical updates in the contract /// @param _admin address who can manage non-critical updates in the contract @@ -33,50 +31,56 @@ contract DiamondInit is Base { /// @param _l2BootloaderBytecodeHash The hash of bootloader L2 bytecode /// @param _l2DefaultAccountBytecodeHash The hash of default account L2 bytecode /// @param _priorityTxMaxGasLimit maximum number of the L2 gas that a user can request for L1 -> L2 transactions + struct InitializeData { + IVerifier verifier; + address governor; + address admin; + bytes32 genesisBatchHash; + uint64 genesisIndexRepeatedStorageChanges; + bytes32 genesisBatchCommitment; + IAllowList allowList; + VerifierParams verifierParams; + bool zkPorterIsAvailable; + bytes32 l2BootloaderBytecodeHash; + bytes32 l2DefaultAccountBytecodeHash; + uint256 priorityTxMaxGasLimit; + } + + /// @dev Initialize the implementation to prevent any possibility of a Parity hack. + constructor() reentrancyGuardInitializer {} + + /// @notice zkSync contract initialization /// @return Magic 32 bytes, which indicates that the contract logic is expected to be used as a diamond proxy /// initializer - function initialize( - IVerifier _verifier, - address _governor, - address _admin, - bytes32 _genesisBatchHash, - uint64 _genesisIndexRepeatedStorageChanges, - bytes32 _genesisBatchCommitment, - IAllowList _allowList, - VerifierParams calldata _verifierParams, - bool _zkPorterIsAvailable, - bytes32 _l2BootloaderBytecodeHash, - bytes32 _l2DefaultAccountBytecodeHash, - uint256 _priorityTxMaxGasLimit - ) external reentrancyGuardInitializer returns (bytes32) { - require(address(_verifier) != address(0), "vt"); - require(_governor != address(0), "vy"); - require(_admin != address(0), "hc"); - require(_priorityTxMaxGasLimit <= L2_TX_MAX_GAS_LIMIT, "vu"); + function initialize(InitializeData calldata _initalizeData) external reentrancyGuardInitializer returns (bytes32) { + require(address(_initalizeData.verifier) != address(0), "vt"); + require(_initalizeData.governor != address(0), "vy"); + require(_initalizeData.admin != address(0), "hc"); + require(_initalizeData.priorityTxMaxGasLimit <= L2_TX_MAX_GAS_LIMIT, "vu"); - s.verifier = _verifier; - s.governor = _governor; - s.admin = _admin; + s.verifier = _initalizeData.verifier; + s.governor = _initalizeData.governor; + s.admin = _initalizeData.admin; // We need to initialize the state hash because it is used in the commitment of the next batch IExecutor.StoredBatchInfo memory storedBatchZero = IExecutor.StoredBatchInfo( 0, - _genesisBatchHash, - _genesisIndexRepeatedStorageChanges, + _initalizeData.genesisBatchHash, + _initalizeData.genesisIndexRepeatedStorageChanges, 0, EMPTY_STRING_KECCAK, DEFAULT_L2_LOGS_TREE_ROOT_HASH, 0, - _genesisBatchCommitment + _initalizeData.genesisBatchCommitment ); s.storedBatchHashes[0] = keccak256(abi.encode(storedBatchZero)); - s.allowList = _allowList; - s.verifierParams = _verifierParams; - s.zkPorterIsAvailable = _zkPorterIsAvailable; - s.l2BootloaderBytecodeHash = _l2BootloaderBytecodeHash; - s.l2DefaultAccountBytecodeHash = _l2DefaultAccountBytecodeHash; - s.priorityTxMaxGasLimit = _priorityTxMaxGasLimit; + s.allowList = _initalizeData.allowList; + s.verifierParams = _initalizeData.verifierParams; + s.zkPorterIsAvailable = _initalizeData.zkPorterIsAvailable; + s.l2BootloaderBytecodeHash = _initalizeData.l2BootloaderBytecodeHash; + s.l2DefaultAccountBytecodeHash = _initalizeData.l2DefaultAccountBytecodeHash; + s.priorityTxMaxGasLimit = _initalizeData.priorityTxMaxGasLimit; // While this does not provide a protection in the production, it is needed for local testing // Length of the L2Log encoding should not be equal to the length of other L2Logs' tree nodes preimages diff --git a/ethereum/package.json b/ethereum/package.json index 8abd508cc..5f06dc1f5 100644 --- a/ethereum/package.json +++ b/ethereum/package.json @@ -52,6 +52,7 @@ "test": "CONTRACT_TESTS=1 yarn hardhat test test/unit_tests/*.spec.ts --network hardhat", "test:foundry": "hardhat solpp && forge test", "test:fork": "CONTRACT_TESTS=1 TEST_CONTRACTS_FORK=1 yarn run hardhat test test/unit_tests/*.fork.ts --network hardhat", + "coverage:foundry": "hardhat solpp && forge coverage", "lint": "yarn lint:sol && yarn prettier:check", "lint:sol-contracts": "solhint --max-warnings 44 contracts/**/*.sol", "lint:sol-tests": "solhint --max-warnings 0 test/**/*.sol", diff --git a/ethereum/src.ts/deploy.ts b/ethereum/src.ts/deploy.ts index f08ddeda4..3ea44754f 100644 --- a/ethereum/src.ts/deploy.ts +++ b/ethereum/src.ts/deploy.ts @@ -106,7 +106,7 @@ export class Deployer { ) ); const genesisBatchHash = getHashFromEnv('CONTRACTS_GENESIS_ROOT'); // TODO: confusing name - const genesisRollupLeafIndex = getNumberFromEnv('CONTRACTS_GENESIS_ROLLUP_LEAF_INDEX'); + const genesisIndexRepeatedStorageChanges = getNumberFromEnv('CONTRACTS_GENESIS_ROLLUP_LEAF_INDEX'); const genesisBatchCommitment = getHashFromEnv('CONTRACTS_GENESIS_BATCH_COMMITMENT'); const verifierParams = { recursionNodeLevelVkHash: getHashFromEnv('CONTRACTS_RECURSION_NODE_LEVEL_VK_HASH'), @@ -117,18 +117,20 @@ export class Deployer { const DiamondInit = new Interface(hardhat.artifacts.readArtifactSync('DiamondInit').abi); const diamondInitCalldata = DiamondInit.encodeFunctionData('initialize', [ - this.addresses.ZkSync.Verifier, - this.ownerAddress, - this.ownerAddress, - genesisBatchHash, - genesisRollupLeafIndex, - genesisBatchCommitment, - this.addresses.AllowList, - verifierParams, - false, // isPorterAvailable - L2_BOOTLOADER_BYTECODE_HASH, - L2_DEFAULT_ACCOUNT_BYTECODE_HASH, - priorityTxMaxGasLimit + { + verifier: this.addresses.ZkSync.Verifier, + governor: this.ownerAddress, + admin: this.ownerAddress, + genesisBatchHash, + genesisIndexRepeatedStorageChanges, + genesisBatchCommitment, + allowList: this.addresses.AllowList, + verifierParams, + zkPorterIsAvailable: false, + l2BootloaderBytecodeHash: L2_BOOTLOADER_BYTECODE_HASH, + l2DefaultAccountBytecodeHash: L2_DEFAULT_ACCOUNT_BYTECODE_HASH, + priorityTxMaxGasLimit, + } ]); // @ts-ignore diff --git a/ethereum/test/unit_tests/l1_erc20_bridge_test.spec.ts b/ethereum/test/unit_tests/l1_erc20_bridge_test.spec.ts index 399e2742d..ce6a63458 100644 --- a/ethereum/test/unit_tests/l1_erc20_bridge_test.spec.ts +++ b/ethereum/test/unit_tests/l1_erc20_bridge_test.spec.ts @@ -50,22 +50,24 @@ describe(`L1ERC20Bridge tests`, function () { dummyHash.set([1, 0, 0, 1]); const dummyAddress = ethers.utils.hexlify(ethers.utils.randomBytes(20)); const diamondInitData = diamondInit.interface.encodeFunctionData('initialize', [ - dummyAddress, - await owner.getAddress(), - await owner.getAddress(), - ethers.constants.HashZero, - 0, - ethers.constants.HashZero, - allowList.address, { - recursionCircuitsSetVksHash: ethers.constants.HashZero, - recursionLeafLevelVkHash: ethers.constants.HashZero, - recursionNodeLevelVkHash: ethers.constants.HashZero - }, - false, - dummyHash, - dummyHash, - 10000000 + verifier: dummyAddress, + governor: await owner.getAddress(), + admin: await owner.getAddress(), + genesisBatchHash: ethers.constants.HashZero, + genesisIndexRepeatedStorageChanges: 0, + genesisBatchCommitment: ethers.constants.HashZero, + allowList: allowList.address, + verifierParams: { + recursionCircuitsSetVksHash: ethers.constants.HashZero, + recursionLeafLevelVkHash: ethers.constants.HashZero, + recursionNodeLevelVkHash: ethers.constants.HashZero + }, + zkPorterIsAvailable: false, + l2BootloaderBytecodeHash: dummyHash, + l2DefaultAccountBytecodeHash: dummyHash, + priorityTxMaxGasLimit: 10000000, + } ]); const facetCuts = [ diff --git a/ethereum/test/unit_tests/l1_weth_bridge_test.spec.ts b/ethereum/test/unit_tests/l1_weth_bridge_test.spec.ts index 5306aedae..f7047c25e 100644 --- a/ethereum/test/unit_tests/l1_weth_bridge_test.spec.ts +++ b/ethereum/test/unit_tests/l1_weth_bridge_test.spec.ts @@ -82,22 +82,24 @@ describe('WETH Bridge tests', () => { dummyHash.set([1, 0, 0, 1]); const dummyAddress = ethers.utils.hexlify(ethers.utils.randomBytes(20)); const diamondInitData = diamondInit.interface.encodeFunctionData('initialize', [ - dummyAddress, - await owner.getAddress(), - await owner.getAddress(), - ethers.constants.HashZero, - 0, - ethers.constants.HashZero, - allowList.address, { - recursionCircuitsSetVksHash: ethers.constants.HashZero, - recursionLeafLevelVkHash: ethers.constants.HashZero, - recursionNodeLevelVkHash: ethers.constants.HashZero - }, - false, - dummyHash, - dummyHash, - 10000000 + verifier: dummyAddress, + governor: await owner.getAddress(), + admin: await owner.getAddress(), + genesisBatchHash: ethers.constants.HashZero, + genesisIndexRepeatedStorageChanges: 0, + genesisBatchCommitment: ethers.constants.HashZero, + allowList: allowList.address, + verifierParams: { + recursionCircuitsSetVksHash: ethers.constants.HashZero, + recursionLeafLevelVkHash: ethers.constants.HashZero, + recursionNodeLevelVkHash: ethers.constants.HashZero + }, + zkPorterIsAvailable: false, + l2BootloaderBytecodeHash: dummyHash, + l2DefaultAccountBytecodeHash: dummyHash, + priorityTxMaxGasLimit: 10000000 + } ]); const facetCuts = [ diff --git a/ethereum/test/unit_tests/l2-upgrade.test.spec.ts b/ethereum/test/unit_tests/l2-upgrade.test.spec.ts index 460799510..791ba7ad6 100644 --- a/ethereum/test/unit_tests/l2-upgrade.test.spec.ts +++ b/ethereum/test/unit_tests/l2-upgrade.test.spec.ts @@ -88,18 +88,20 @@ describe('L2 upgrade test', function () { recursionNodeLevelVkHash: ethers.constants.HashZero }; const diamondInitData = diamondInit.interface.encodeFunctionData('initialize', [ - verifier, - await owner.getAddress(), - await owner.getAddress(), - ethers.constants.HashZero, - 0, - ethers.constants.HashZero, - allowList.address, - verifierParams, - false, - dummyHash, - dummyHash, - 10000000 + { + verifier, + governor: await owner.getAddress(), + admin: await owner.getAddress(), + genesisBatchHash: ethers.constants.HashZero, + genesisIndexRepeatedStorageChanges: 0, + genesisBatchCommitment: ethers.constants.HashZero, + allowList: allowList.address, + verifierParams, + zkPorterIsAvailable: false, + l2BootloaderBytecodeHash: dummyHash, + l2DefaultAccountBytecodeHash: dummyHash, + priorityTxMaxGasLimit: 10000000 + } ]); const facetCuts = [ diff --git a/ethereum/test/unit_tests/mailbox_test.spec.ts b/ethereum/test/unit_tests/mailbox_test.spec.ts index 10199c18d..ba6458c92 100644 --- a/ethereum/test/unit_tests/mailbox_test.spec.ts +++ b/ethereum/test/unit_tests/mailbox_test.spec.ts @@ -60,22 +60,24 @@ describe('Mailbox tests', function () { dummyHash.set([1, 0, 0, 1]); const dummyAddress = ethers.utils.hexlify(ethers.utils.randomBytes(20)); const diamondInitData = diamondInit.interface.encodeFunctionData('initialize', [ - dummyAddress, - dummyAddress, - dummyAddress, - ethers.constants.HashZero, - 0, - ethers.constants.HashZero, - allowList.address, { - recursionCircuitsSetVksHash: ethers.constants.HashZero, - recursionLeafLevelVkHash: ethers.constants.HashZero, - recursionNodeLevelVkHash: ethers.constants.HashZero - }, - false, - dummyHash, - dummyHash, - 10000000 + verifier: dummyAddress, + governor: dummyAddress, + admin: dummyAddress, + genesisBatchHash: ethers.constants.HashZero, + genesisIndexRepeatedStorageChanges: 0, + genesisBatchCommitment: ethers.constants.HashZero, + allowList: allowList.address, + verifierParams: { + recursionCircuitsSetVksHash: ethers.constants.HashZero, + recursionLeafLevelVkHash: ethers.constants.HashZero, + recursionNodeLevelVkHash: ethers.constants.HashZero + }, + zkPorterIsAvailable: false, + l2BootloaderBytecodeHash: dummyHash, + l2DefaultAccountBytecodeHash: dummyHash, + priorityTxMaxGasLimit: 10000000 + } ]); const facetCuts = [ diff --git a/ethereum/test/unit_tests/proxy_test.spec.ts b/ethereum/test/unit_tests/proxy_test.spec.ts index ce52454a1..ef352eaa3 100644 --- a/ethereum/test/unit_tests/proxy_test.spec.ts +++ b/ethereum/test/unit_tests/proxy_test.spec.ts @@ -68,18 +68,20 @@ describe('Diamond proxy tests', function () { recursionCircuitsSetVksHash: ethers.constants.HashZero }; const diamondInitCalldata = diamondInit.interface.encodeFunctionData('initialize', [ - '0x03752D8252d67f99888E741E3fB642803B29B155', - governorAddress, - governorAddress, - '0x02c775f0a90abf7a0e8043f2fdc38f0580ca9f9996a895d05a501bfeaa3b2e21', - 0, - '0x0000000000000000000000000000000000000000000000000000000000000000', - '0x70a0F165d6f8054d0d0CF8dFd4DD2005f0AF6B55', - dummyVerifierParams, - false, - '0x0100000000000000000000000000000000000000000000000000000000000000', - '0x0100000000000000000000000000000000000000000000000000000000000000', - 500000 // priority tx max L2 gas limit + { + verifier: '0x03752D8252d67f99888E741E3fB642803B29B155', + governor: governorAddress, + admin: governorAddress, + genesisBatchHash: '0x02c775f0a90abf7a0e8043f2fdc38f0580ca9f9996a895d05a501bfeaa3b2e21', + genesisIndexRepeatedStorageChanges: 0, + genesisBatchCommitment: ethers.constants.HashZero, + allowList: '0x70a0F165d6f8054d0d0CF8dFd4DD2005f0AF6B55', + verifierParams: dummyVerifierParams, + zkPorterIsAvailable: false, + l2BootloaderBytecodeHash: '0x0100000000000000000000000000000000000000000000000000000000000000', + l2DefaultAccountBytecodeHash: '0x0100000000000000000000000000000000000000000000000000000000000000', + priorityTxMaxGasLimit: 500000 + } ]); const diamondCutData = diamondCut(facetCuts, diamondInit.address, diamondInitCalldata);