From 518bfff51085743dc85d2824d823aaf4bb3e82d8 Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Tue, 28 Nov 2023 16:11:26 +0100 Subject: [PATCH 01/10] Add the ability to reinitialize erc20 token (#102) --- .github/workflows/ci.yml | 16 ++- ethereum/package.json | 4 +- ethereum/scripts/utils.ts | 48 +++++++ ethereum/src.ts/deploy.ts | 50 +------ ethereum/yarn.lock | 18 +-- zksync/contracts/bridge/L2StandardERC20.sol | 40 +++++- zksync/package.json | 7 +- zksync/src/utils.ts | 9 +- zksync/test/erc20.test.ts | 146 ++++++++++++++++++++ zksync/yarn.lock | 18 +-- 10 files changed, 282 insertions(+), 74 deletions(-) create mode 100644 zksync/test/erc20.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 524b178bb..f83017697 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -211,7 +211,7 @@ jobs: run: forge test test-hardhat-l2: - needs: [build-l2, lint-l2] + needs: [build-l1, build-l2, lint-l2] runs-on: ubuntu-latest defaults: @@ -237,6 +237,20 @@ jobs: - name: Install dependencies run: yarn install + - name: Install L1 dependencies + working-directory: ethereum + run: yarn install + + - name: Restore L1 artifacts cache + uses: actions/cache/restore@v3 + with: + fail-on-cache-miss: true + key: artifacts-${{ github.sha }} + path: | + ethereum/artifacts + ethereum/cache + ethereum/typechain + - name: Restore artifacts cache uses: actions/cache/restore@v3 with: diff --git a/ethereum/package.json b/ethereum/package.json index 1f5711401..8b0854e9c 100644 --- a/ethereum/package.json +++ b/ethereum/package.json @@ -9,8 +9,8 @@ "@nomiclabs/hardhat-etherscan": "^3.1.0", "@nomiclabs/hardhat-solpp": "^2.0.0", "@nomiclabs/hardhat-waffle": "^2.0.0", - "@openzeppelin/contracts": "4.8.0", - "@openzeppelin/contracts-upgradeable": "4.8.0", + "@openzeppelin/contracts": "4.9.2", + "@openzeppelin/contracts-upgradeable": "4.9.2", "@typechain/ethers-v5": "^2.0.0", "@types/argparse": "^1.0.36", "@types/chai": "^4.2.21", diff --git a/ethereum/scripts/utils.ts b/ethereum/scripts/utils.ts index 82596007b..b5e9cf372 100644 --- a/ethereum/scripts/utils.ts +++ b/ethereum/scripts/utils.ts @@ -151,3 +151,51 @@ export function getTokens(network: string): L1Token[] { }) ); } + +export interface DeployedAddresses { + ZkSync: { + MailboxFacet: string; + AdminFacet: string; + ExecutorFacet: string; + GettersFacet: string; + Verifier: string; + DiamondInit: string; + DiamondUpgradeInit: string; + DefaultUpgrade: string; + DiamondProxy: string; + }; + Bridges: { + ERC20BridgeImplementation: string; + ERC20BridgeProxy: string; + WethBridgeImplementation: string; + WethBridgeProxy: string; + }; + Governance: string; + ValidatorTimeLock: string; + Create2Factory: string; +} + +export function deployedAddressesFromEnv(): DeployedAddresses { + return { + ZkSync: { + MailboxFacet: getAddressFromEnv("CONTRACTS_MAILBOX_FACET_ADDR"), + AdminFacet: getAddressFromEnv("CONTRACTS_ADMIN_FACET_ADDR"), + ExecutorFacet: getAddressFromEnv("CONTRACTS_EXECUTOR_FACET_ADDR"), + GettersFacet: getAddressFromEnv("CONTRACTS_GETTERS_FACET_ADDR"), + DiamondInit: getAddressFromEnv("CONTRACTS_DIAMOND_INIT_ADDR"), + DiamondUpgradeInit: getAddressFromEnv("CONTRACTS_DIAMOND_UPGRADE_INIT_ADDR"), + DefaultUpgrade: getAddressFromEnv("CONTRACTS_DEFAULT_UPGRADE_ADDR"), + DiamondProxy: getAddressFromEnv("CONTRACTS_DIAMOND_PROXY_ADDR"), + Verifier: getAddressFromEnv("CONTRACTS_VERIFIER_ADDR"), + }, + Bridges: { + ERC20BridgeImplementation: getAddressFromEnv("CONTRACTS_L1_ERC20_BRIDGE_IMPL_ADDR"), + ERC20BridgeProxy: getAddressFromEnv("CONTRACTS_L1_ERC20_BRIDGE_PROXY_ADDR"), + WethBridgeImplementation: getAddressFromEnv("CONTRACTS_L1_WETH_BRIDGE_IMPL_ADDR"), + WethBridgeProxy: getAddressFromEnv("CONTRACTS_L1_WETH_BRIDGE_PROXY_ADDR"), + }, + Create2Factory: getAddressFromEnv("CONTRACTS_CREATE2_FACTORY_ADDR"), + ValidatorTimeLock: getAddressFromEnv("CONTRACTS_VALIDATOR_TIMELOCK_ADDR"), + Governance: getAddressFromEnv("CONTRACTS_GOVERNANCE_ADDR"), + }; +} diff --git a/ethereum/src.ts/deploy.ts b/ethereum/src.ts/deploy.ts index 22416fbe4..e99e92414 100644 --- a/ethereum/src.ts/deploy.ts +++ b/ethereum/src.ts/deploy.ts @@ -11,6 +11,7 @@ import { L1WethBridgeFactory } from "../typechain/L1WethBridgeFactory"; import { ValidatorTimelockFactory } from "../typechain/ValidatorTimelockFactory"; import { SingletonFactoryFactory } from "../typechain/SingletonFactoryFactory"; import { TransparentUpgradeableProxyFactory } from "../typechain/TransparentUpgradeableProxyFactory"; +import type { DeployedAddresses } from "../scripts/utils"; import { readSystemContractsBytecode, hashL2Bytecode, @@ -19,6 +20,7 @@ import { getNumberFromEnv, readBatchBootloaderBytecode, getTokens, + deployedAddressesFromEnv, } from "../scripts/utils"; import { deployViaCreate2 } from "./deploy-utils"; import { IGovernanceFactory } from "../typechain/IGovernanceFactory"; @@ -26,60 +28,12 @@ import { IGovernanceFactory } from "../typechain/IGovernanceFactory"; const L2_BOOTLOADER_BYTECODE_HASH = hexlify(hashL2Bytecode(readBatchBootloaderBytecode())); const L2_DEFAULT_ACCOUNT_BYTECODE_HASH = hexlify(hashL2Bytecode(readSystemContractsBytecode("DefaultAccount"))); -export interface DeployedAddresses { - ZkSync: { - MailboxFacet: string; - AdminFacet: string; - ExecutorFacet: string; - GettersFacet: string; - Verifier: string; - DiamondInit: string; - DiamondUpgradeInit: string; - DefaultUpgrade: string; - DiamondProxy: string; - }; - Bridges: { - ERC20BridgeImplementation: string; - ERC20BridgeProxy: string; - WethBridgeImplementation: string; - WethBridgeProxy: string; - }; - Governance: string; - ValidatorTimeLock: string; - Create2Factory: string; -} - export interface DeployerConfig { deployWallet: Wallet; ownerAddress?: string; verbose?: boolean; } -export function deployedAddressesFromEnv(): DeployedAddresses { - return { - ZkSync: { - MailboxFacet: getAddressFromEnv("CONTRACTS_MAILBOX_FACET_ADDR"), - AdminFacet: getAddressFromEnv("CONTRACTS_ADMIN_FACET_ADDR"), - ExecutorFacet: getAddressFromEnv("CONTRACTS_EXECUTOR_FACET_ADDR"), - GettersFacet: getAddressFromEnv("CONTRACTS_GETTERS_FACET_ADDR"), - DiamondInit: getAddressFromEnv("CONTRACTS_DIAMOND_INIT_ADDR"), - DiamondUpgradeInit: getAddressFromEnv("CONTRACTS_DIAMOND_UPGRADE_INIT_ADDR"), - DefaultUpgrade: getAddressFromEnv("CONTRACTS_DEFAULT_UPGRADE_ADDR"), - DiamondProxy: getAddressFromEnv("CONTRACTS_DIAMOND_PROXY_ADDR"), - Verifier: getAddressFromEnv("CONTRACTS_VERIFIER_ADDR"), - }, - Bridges: { - ERC20BridgeImplementation: getAddressFromEnv("CONTRACTS_L1_ERC20_BRIDGE_IMPL_ADDR"), - ERC20BridgeProxy: getAddressFromEnv("CONTRACTS_L1_ERC20_BRIDGE_PROXY_ADDR"), - WethBridgeImplementation: getAddressFromEnv("CONTRACTS_L1_WETH_BRIDGE_IMPL_ADDR"), - WethBridgeProxy: getAddressFromEnv("CONTRACTS_L1_WETH_BRIDGE_PROXY_ADDR"), - }, - Create2Factory: getAddressFromEnv("CONTRACTS_CREATE2_FACTORY_ADDR"), - ValidatorTimeLock: getAddressFromEnv("CONTRACTS_VALIDATOR_TIMELOCK_ADDR"), - Governance: getAddressFromEnv("CONTRACTS_GOVERNANCE_ADDR"), - }; -} - export class Deployer { public addresses: DeployedAddresses; private deployWallet: Wallet; diff --git a/ethereum/yarn.lock b/ethereum/yarn.lock index cadb8a168..57a0a1e2d 100644 --- a/ethereum/yarn.lock +++ b/ethereum/yarn.lock @@ -866,15 +866,15 @@ "@types/sinon-chai" "^3.2.3" "@types/web3" "1.0.19" -"@openzeppelin/contracts-upgradeable@4.8.0": - version "4.8.0" - resolved "https://registry.yarnpkg.com/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-4.8.0.tgz#26688982f46969018e3ed3199e72a07c8d114275" - integrity sha512-5GeFgqMiDlqGT8EdORadp1ntGF0qzWZLmEY7Wbp/yVhN7/B3NNzCxujuI77ktlyG81N3CUZP8cZe3ZAQ/cW10w== - -"@openzeppelin/contracts@4.8.0": - version "4.8.0" - resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-4.8.0.tgz#6854c37df205dd2c056bdfa1b853f5d732109109" - integrity sha512-AGuwhRRL+NaKx73WKRNzeCxOCOCxpaqF+kp8TJ89QzAipSwZy/NoflkWaL9bywXFRhIzXt8j38sfF7KBKCPWLw== +"@openzeppelin/contracts-upgradeable@4.9.2": + version "4.9.2" + resolved "https://registry.yarnpkg.com/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-4.9.2.tgz#a817c75688f8daede420052fbcb34e52482e769e" + integrity sha512-siviV3PZV/fHfPaoIC51rf1Jb6iElkYWnNYZ0leO23/ukXuvOyoC/ahy8jqiV7g+++9Nuo3n/rk5ajSN/+d/Sg== + +"@openzeppelin/contracts@4.9.2": + version "4.9.2" + resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-4.9.2.tgz#1cb2d5e4d3360141a17dbc45094a8cad6aac16c1" + integrity sha512-mO+y6JaqXjWeMh9glYVzVu8HYPGknAAnWyxTRhGeckOruyXQMNnlcW6w/Dx9ftLeIQk6N+ZJFuVmTwF7lEIFrg== "@pkgr/utils@^2.3.1": version "2.4.2" diff --git a/zksync/contracts/bridge/L2StandardERC20.sol b/zksync/contracts/bridge/L2StandardERC20.sol index 28bf8497f..b54ef8585 100644 --- a/zksync/contracts/bridge/L2StandardERC20.sol +++ b/zksync/contracts/bridge/L2StandardERC20.sol @@ -3,12 +3,15 @@ pragma solidity 0.8.20; import "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/draft-ERC20PermitUpgradeable.sol"; +import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; +import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Upgrade.sol"; + import "./interfaces/IL2StandardToken.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev /// @notice The ERC20 token implementation, that is used in the "default" ERC20 bridge -contract L2StandardERC20 is ERC20PermitUpgradeable, IL2StandardToken { +contract L2StandardERC20 is ERC20PermitUpgradeable, IL2StandardToken, ERC1967Upgrade { /// @dev Describes whether there is a specific getter in the token. /// @notice Used to explicitly separate which getters the token has and which it does not. /// @notice Different tokens in L1 can implement or not implement getter function as `name`/`symbol`/`decimals`, @@ -97,11 +100,46 @@ contract L2StandardERC20 is ERC20PermitUpgradeable, IL2StandardToken { emit BridgeInitialize(_l1Address, decodedName, decodedSymbol, decimals_); } + /// @notice A method to be called by the governor to update the token's metadata. + /// @param _availableGetters The getters that the token has. + /// @param _newName The new name of the token. + /// @param _newSymbol The new symbol of the token. + /// @param _newDecimals The new decimals of the token. + /// @param _version The version of the token that will be initialized. + /// @dev The _version must be exactly the version higher by 1 than the current version. This is needed + /// to ensure that the governor can not accidentally disable future reinitialization of the token. + function reinitializeToken( + ERC20Getters calldata _availableGetters, + string memory _newName, + string memory _newSymbol, + uint8 _newDecimals, + uint8 _version + ) external onlyNextVersion(_version) reinitializer(_version) { + // It is expected that this token is deployed as a beacon proxy, so we'll + // allow the governor of the beacon to reinitialize the token. + address beaconAddress = _getBeacon(); + require(msg.sender == UpgradeableBeacon(beaconAddress).owner(), "tt"); + + __ERC20_init_unchained(_newName, _newSymbol); + __ERC20Permit_init(_newName); + decimals_ = _newDecimals; + availableGetters = _availableGetters; + + emit BridgeInitialize(l1Address, _newName, _newSymbol, _newDecimals); + } + modifier onlyBridge() { require(msg.sender == l2Bridge, "xnt"); // Only L2 bridge can call this method _; } + modifier onlyNextVersion(uint8 _version) { + // The version should be incremented by 1. Otherwise, the governor risks disabling + // future reinitialization of the token by providing too large a version. + require(_version == _getInitializedVersion() + 1, "v"); + _; + } + /// @dev Mint tokens to a given account. /// @param _to The account that will receive the created tokens. /// @param _amount The amount that will be created. diff --git a/zksync/package.json b/zksync/package.json index fbd13450e..db329a540 100644 --- a/zksync/package.json +++ b/zksync/package.json @@ -14,8 +14,8 @@ "@nomiclabs/hardhat-ethers": "^2.0.0", "@nomiclabs/hardhat-etherscan": "^3.1.7", "@nomiclabs/hardhat-solpp": "^2.0.0", - "@openzeppelin/contracts": "4.6.0", - "@openzeppelin/contracts-upgradeable": "4.6.0", + "@openzeppelin/contracts": "4.9.2", + "@openzeppelin/contracts-upgradeable": "4.9.2", "@typechain/ethers-v5": "^2.0.0", "@types/chai": "^4.2.21", "@types/chai-as-promised": "^7.1.4", @@ -58,7 +58,8 @@ "deploy-force-deploy-upgrader": "ts-node src/deployForceDeployUpgrader.ts", "publish-bridge-preimages": "ts-node src/publish-bridge-preimages.ts", "deploy-l2-weth": "ts-node src/deployL2Weth.ts", - "upgrade-l2-erc20-contract": "ts-node src/upgradeL2BridgeImpl.ts" + "upgrade-l2-erc20-contract": "ts-node src/upgradeL2BridgeImpl.ts", + "test": "hardhat test --network localhost" }, "dependencies": { "dotenv": "^16.0.3" diff --git a/zksync/src/utils.ts b/zksync/src/utils.ts index bee4d7cd2..a237f6228 100644 --- a/zksync/src/utils.ts +++ b/zksync/src/utils.ts @@ -1,7 +1,7 @@ import { artifacts } from "hardhat"; import { Interface } from "ethers/lib/utils"; -import { deployedAddressesFromEnv } from "../../ethereum/src.ts/deploy"; +import { deployedAddressesFromEnv } from "../../ethereum/scripts/utils"; import { IZkSyncFactory } from "../../ethereum/typechain/IZkSyncFactory"; import type { BytesLike, Wallet } from "ethers"; @@ -21,6 +21,13 @@ export function applyL1ToL2Alias(address: string): string { return ethers.utils.hexlify(ethers.BigNumber.from(address).add(L1_TO_L2_ALIAS_OFFSET).mod(ADDRESS_MODULO)); } +export function unapplyL1ToL2Alias(address: string): string { + // We still add ADDRESS_MODULO to avoid negative numbers + return ethers.utils.hexlify( + ethers.BigNumber.from(address).sub(L1_TO_L2_ALIAS_OFFSET).add(ADDRESS_MODULO).mod(ADDRESS_MODULO) + ); +} + export function hashL2Bytecode(bytecode: ethers.BytesLike): Uint8Array { // For getting the consistent length we first convert the bytecode to UInt8Array const bytecodeAsArray = ethers.utils.arrayify(bytecode); diff --git a/zksync/test/erc20.test.ts b/zksync/test/erc20.test.ts new file mode 100644 index 000000000..a3f92fe64 --- /dev/null +++ b/zksync/test/erc20.test.ts @@ -0,0 +1,146 @@ +import { Deployer } from "@matterlabs/hardhat-zksync-deploy"; +import { expect } from "chai"; +import { ethers } from "ethers"; +import * as hre from "hardhat"; +import { Provider, Wallet } from "zksync-web3"; +import { hashBytecode } from "zksync-web3/build/src/utils"; +import { unapplyL1ToL2Alias } from "../src/utils"; +import { L2ERC20BridgeFactory, L2StandardERC20Factory } from "../typechain"; +import type { L2ERC20Bridge, L2StandardERC20 } from "../typechain"; + +const richAccount = [ + { + address: "0x36615Cf349d7F6344891B1e7CA7C72883F5dc049", + privateKey: "0x7726827caac94a7f9e1b160f7ea819f172f7b6f9d2a97f992c38edeab82d4110", + }, + { + address: "0xa61464658AfeAf65CccaaFD3a512b69A83B77618", + privateKey: "0xac1e735be8536c6534bb4f17f06f6afc73b2b5ba84ac2cfb12f7461b20c0bbe3", + }, + { + address: "0x0D43eB5B8a47bA8900d84AA36656c92024e9772e", + privateKey: "0xd293c684d884d56f8d6abd64fc76757d3664904e309a0645baf8522ab6366d9e", + }, +]; + +describe("ERC20Bridge", function () { + const provider = new Provider(hre.config.networks.localhost.url); + const deployerWallet = new Wallet(richAccount[0].privateKey, provider); + const governorWallet = new Wallet(richAccount[1].privateKey, provider); + + // We need to emulate a L1->L2 transaction from the L1 bridge to L2 counterpart. + // It is a bit easier to use EOA and it is sufficient for the tests. + const l1BridgeWallet = new Wallet(richAccount[2].privateKey, provider); + + // We won't actually deploy an L1 token in these tests, but we need some address for it. + const L1_TOKEN_ADDRESS = "0x1111000000000000000000000000000000001111"; + + let erc20Bridge: L2ERC20Bridge; + let erc20Token: L2StandardERC20; + + before("Deploy token and bridge", async function () { + const deployer = new Deployer(hre, deployerWallet); + + // While we formally don't need to deploy the token and the beacon proxy, it is a neat way to have the bytecode published + const l2TokenImplAddress = await deployer.deploy(await deployer.loadArtifact("L2StandardERC20")); + const l2Erc20TokenBeacon = await deployer.deploy(await deployer.loadArtifact("UpgradeableBeacon"), [ + l2TokenImplAddress.address, + ]); + await deployer.deploy(await deployer.loadArtifact("BeaconProxy"), [l2Erc20TokenBeacon.address, "0x"]); + + const beaconProxyBytecodeHash = hashBytecode((await deployer.loadArtifact("BeaconProxy")).bytecode); + + const erc20BridgeImpl = await deployer.deploy(await deployer.loadArtifact("L2ERC20Bridge")); + const bridgeInitializeData = erc20BridgeImpl.interface.encodeFunctionData("initialize", [ + unapplyL1ToL2Alias(l1BridgeWallet.address), + beaconProxyBytecodeHash, + governorWallet.address, + ]); + + const erc20BridgeProxy = await deployer.deploy(await deployer.loadArtifact("TransparentUpgradeableProxy"), [ + erc20BridgeImpl.address, + governorWallet.address, + bridgeInitializeData, + ]); + + erc20Bridge = L2ERC20BridgeFactory.connect(erc20BridgeProxy.address, deployerWallet); + }); + + it("Should finalize deposit ERC20 deposit", async function () { + const erc20BridgeWithL1Bridge = L2ERC20BridgeFactory.connect(erc20Bridge.address, l1BridgeWallet); + + const l1Depositor = ethers.Wallet.createRandom(); + const l2Receiver = ethers.Wallet.createRandom(); + + const tx = await ( + await erc20BridgeWithL1Bridge.finalizeDeposit( + // Depositor and l2Receiver can be any here + l1Depositor.address, + l2Receiver.address, + L1_TOKEN_ADDRESS, + 100, + encodedTokenData("TestToken", "TT", 18) + ) + ).wait(); + + const l2TokenAddress = tx.events.find((event) => event.event === "FinalizeDeposit").args.l2Token; + + // Checking the correctness of the balance: + erc20Token = L2StandardERC20Factory.connect(l2TokenAddress, deployerWallet); + expect(await erc20Token.balanceOf(l2Receiver.address)).to.equal(100); + expect(await erc20Token.totalSupply()).to.equal(100); + expect(await erc20Token.name()).to.equal("TestToken"); + expect(await erc20Token.symbol()).to.equal("TT"); + expect(await erc20Token.decimals()).to.equal(18); + }); + + it("Governance should be able to reinitialize the token", async () => { + const erc20TokenWithGovernor = L2StandardERC20Factory.connect(erc20Token.address, governorWallet); + + await ( + await erc20TokenWithGovernor.reinitializeToken( + { + ignoreName: false, + ignoreSymbol: false, + ignoreDecimals: false, + }, + "TestTokenNewName", + "TTN", + 12, + 2 + ) + ).wait(); + + expect(await erc20Token.name()).to.equal("TestTokenNewName"); + expect(await erc20Token.symbol()).to.equal("TTN"); + expect(await erc20Token.decimals()).to.equal(12); + }); + + it("Governance should not be able to skip initializer versions", async () => { + const erc20TokenWithGovernor = L2StandardERC20Factory.connect(erc20Token.address, governorWallet); + + await expect( + erc20TokenWithGovernor.reinitializeToken( + { + ignoreName: false, + ignoreSymbol: false, + ignoreDecimals: false, + }, + "TestTokenNewName", + "TTN", + 12, + 20, + { gasLimit: 10000000 } + ) + ).to.be.reverted; + }); +}); + +function encodedTokenData(name: string, symbol: string, decimals: number) { + const abiCoder = ethers.utils.defaultAbiCoder; + const encodedName = abiCoder.encode(["string"], [name]); + const encodedSymbol = abiCoder.encode(["string"], [symbol]); + const encodedDecimals = abiCoder.encode(["uint8"], [decimals]); + + return abiCoder.encode(["bytes", "bytes", "bytes"], [encodedName, encodedSymbol, encodedDecimals]); +} diff --git a/zksync/yarn.lock b/zksync/yarn.lock index 4482a1724..b0dfaae4e 100644 --- a/zksync/yarn.lock +++ b/zksync/yarn.lock @@ -857,15 +857,15 @@ fs-extra "^7.0.1" solpp "^0.11.5" -"@openzeppelin/contracts-upgradeable@4.6.0": - version "4.6.0" - resolved "https://registry.yarnpkg.com/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-4.6.0.tgz#1bf55f230f008554d4c6fe25eb165b85112108b0" - integrity sha512-5OnVuO4HlkjSCJO165a4i2Pu1zQGzMs//o54LPrwUgxvEO2P3ax1QuaSI0cEHHTveA77guS0PnNugpR2JMsPfA== - -"@openzeppelin/contracts@4.6.0": - version "4.6.0" - resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-4.6.0.tgz#c91cf64bc27f573836dba4122758b4743418c1b3" - integrity sha512-8vi4d50NNya/bQqCmaVzvHNmwHvS0OBKb7HNtuNwEE3scXWrP31fKQoGxNMT+KbzmrNZzatE3QK5p2gFONI/hg== +"@openzeppelin/contracts-upgradeable@4.9.2": + version "4.9.2" + resolved "https://registry.yarnpkg.com/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-4.9.2.tgz#a817c75688f8daede420052fbcb34e52482e769e" + integrity sha512-siviV3PZV/fHfPaoIC51rf1Jb6iElkYWnNYZ0leO23/ukXuvOyoC/ahy8jqiV7g+++9Nuo3n/rk5ajSN/+d/Sg== + +"@openzeppelin/contracts@4.9.2": + version "4.9.2" + resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-4.9.2.tgz#1cb2d5e4d3360141a17dbc45094a8cad6aac16c1" + integrity sha512-mO+y6JaqXjWeMh9glYVzVu8HYPGknAAnWyxTRhGeckOruyXQMNnlcW6w/Dx9ftLeIQk6N+ZJFuVmTwF7lEIFrg== "@pkgr/utils@^2.3.1": version "2.4.2" From a07e7b0a7e5cb3e9148e99270ec6cb43c1db4cdb Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Tue, 9 Jan 2024 09:46:34 +0100 Subject: [PATCH 02/10] Add missing documentation (#127) --- ethereum/contracts/zksync/Verifier.sol | 7 +- ethereum/contracts/zksync/facets/Admin.sol | 36 +++----- ethereum/contracts/zksync/facets/Executor.sol | 21 ++--- ethereum/contracts/zksync/facets/Getters.sol | 91 ++++++++----------- ethereum/contracts/zksync/facets/Mailbox.sol | 60 ++---------- .../contracts/zksync/interfaces/IAdmin.sol | 22 +++++ .../contracts/zksync/interfaces/IBase.sol | 1 + .../contracts/zksync/interfaces/IExecutor.sol | 19 ++++ .../contracts/zksync/interfaces/IGetters.sol | 40 +++++++- .../zksync/interfaces/ILegacyGetters.sol | 16 ++++ .../contracts/zksync/interfaces/IMailbox.sol | 50 ++++++++++ .../contracts/zksync/interfaces/IVerifier.sol | 5 + tools/data/verifier_contract_template.txt | 7 +- 13 files changed, 225 insertions(+), 150 deletions(-) diff --git a/ethereum/contracts/zksync/Verifier.sol b/ethereum/contracts/zksync/Verifier.sol index 0924ee2e7..99419d5ed 100644 --- a/ethereum/contracts/zksync/Verifier.sol +++ b/ethereum/contracts/zksync/Verifier.sol @@ -255,8 +255,7 @@ contract Verifier is IVerifier { uint256 internal constant G2_ELEMENTS_1_Y1 = 0x04fc6369f7110fe3d25156c1bb9a72859cf2a04641f99ba4ee413c80da6a5fe4; uint256 internal constant G2_ELEMENTS_1_Y2 = 0x22febda3c0c0632a56475b4214e5615e11e6dd3f96e6cea2854a87d4dacc5e55; - /// @notice Calculates a keccak256 hash of the runtime loaded verification keys. - /// @return vkHash The keccak256 hash of the loaded verification keys. + /// @inheritdoc IVerifier function verificationKeyHash() external pure returns (bytes32 vkHash) { _loadVerificationKey(); @@ -340,9 +339,7 @@ contract Verifier is IVerifier { } } - /// @dev Verifies a zk-SNARK proof. - /// @return A boolean value indicating whether the zk-SNARK proof is valid. - /// Note: The function may revert execution instead of returning false in some cases. + /// @inheritdoc IVerifier function verify( uint256[] calldata, // _publicInputs uint256[] calldata, // _proof diff --git a/ethereum/contracts/zksync/facets/Admin.sol b/ethereum/contracts/zksync/facets/Admin.sol index 0990a13cf..422a70a64 100644 --- a/ethereum/contracts/zksync/facets/Admin.sol +++ b/ethereum/contracts/zksync/facets/Admin.sol @@ -7,15 +7,17 @@ import "../libraries/Diamond.sol"; import {L2_TX_MAX_GAS_LIMIT} from "../Config.sol"; import "./Base.sol"; +// While formally the following import is not used, it is needed to inherit documentation from it +import {IBase} from "../interfaces/IBase.sol"; + /// @title Admin Contract controls access rights for contract management. /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev contract AdminFacet is Base, IAdmin { + /// @inheritdoc IBase string public constant override getName = "AdminFacet"; - /// @notice Starts the transfer of governor rights. Only the current governor can propose a new pending one. - /// @notice New governor can accept governor rights by calling `acceptGovernor` function. - /// @param _newPendingGovernor Address of the new governor + /// @inheritdoc IAdmin function setPendingGovernor(address _newPendingGovernor) external onlyGovernor { // Save previous value into the stack to put it into the event later address oldPendingGovernor = s.pendingGovernor; @@ -24,7 +26,7 @@ contract AdminFacet is Base, IAdmin { emit NewPendingGovernor(oldPendingGovernor, _newPendingGovernor); } - /// @notice Accepts transfer of governor rights. Only pending governor can accept the role. + /// @inheritdoc IAdmin function acceptGovernor() external { address pendingGovernor = s.pendingGovernor; require(msg.sender == pendingGovernor, "n4"); // Only proposed by current governor address can claim the governor rights @@ -37,9 +39,7 @@ contract AdminFacet is Base, IAdmin { emit NewGovernor(previousGovernor, pendingGovernor); } - /// @notice Starts the transfer of admin rights. Only the current governor or admin can propose a new pending one. - /// @notice New admin can accept admin rights by calling `acceptAdmin` function. - /// @param _newPendingAdmin Address of the new admin + /// @inheritdoc IAdmin function setPendingAdmin(address _newPendingAdmin) external onlyGovernor { // Save previous value into the stack to put it into the event later address oldPendingAdmin = s.pendingAdmin; @@ -48,7 +48,7 @@ contract AdminFacet is Base, IAdmin { emit NewPendingAdmin(oldPendingAdmin, _newPendingAdmin); } - /// @notice Accepts transfer of admin rights. Only pending admin can accept the role. + /// @inheritdoc IAdmin function acceptAdmin() external { address pendingAdmin = s.pendingAdmin; require(msg.sender == pendingAdmin, "n4"); // Only proposed by current admin address can claim the admin rights @@ -61,24 +61,20 @@ contract AdminFacet is Base, IAdmin { emit NewAdmin(previousAdmin, pendingAdmin); } - /// @notice Change validator status (active or not active) - /// @param _validator Validator address - /// @param _active Active flag + /// @inheritdoc IAdmin function setValidator(address _validator, bool _active) external onlyGovernorOrAdmin { s.validators[_validator] = _active; emit ValidatorStatusUpdate(_validator, _active); } - /// @notice Change zk porter availability - /// @param _zkPorterIsAvailable The availability of zk porter shard + /// @inheritdoc IAdmin function setPorterAvailability(bool _zkPorterIsAvailable) external onlyGovernor { // Change the porter availability s.zkPorterIsAvailable = _zkPorterIsAvailable; emit IsPorterAvailableStatusUpdate(_zkPorterIsAvailable); } - /// @notice Change the max L2 gas limit for L1 -> L2 transactions - /// @param _newPriorityTxMaxGasLimit The maximum number of L2 gas that a user can request for L1 -> L2 transactions + /// @inheritdoc IAdmin function setPriorityTxMaxGasLimit(uint256 _newPriorityTxMaxGasLimit) external onlyGovernor { require(_newPriorityTxMaxGasLimit <= L2_TX_MAX_GAS_LIMIT, "n5"); @@ -91,9 +87,7 @@ contract AdminFacet is Base, IAdmin { UPGRADE EXECUTION //////////////////////////////////////////////////////////////*/ - /// @notice Executes a proposed governor upgrade - /// @dev Only the current governor can execute the upgrade - /// @param _diamondCut The diamond cut parameters to be executed + /// @inheritdoc IAdmin function executeUpgrade(Diamond.DiamondCutData calldata _diamondCut) external onlyGovernor { Diamond.diamondCut(_diamondCut); emit ExecuteUpgrade(_diamondCut); @@ -103,8 +97,7 @@ contract AdminFacet is Base, IAdmin { CONTRACT FREEZING //////////////////////////////////////////////////////////////*/ - /// @notice Instantly pause the functionality of all freezable facets & their selectors - /// @dev Only the governance mechanism may freeze Diamond Proxy + /// @inheritdoc IAdmin function freezeDiamond() external onlyGovernor { Diamond.DiamondStorage storage diamondStorage = Diamond.getDiamondStorage(); @@ -114,8 +107,7 @@ contract AdminFacet is Base, IAdmin { emit Freeze(); } - /// @notice Unpause the functionality of all freezable facets & their selectors - /// @dev Both the governor and its owner can unfreeze Diamond Proxy + /// @inheritdoc IAdmin function unfreezeDiamond() external onlyGovernorOrAdmin { Diamond.DiamondStorage storage diamondStorage = Diamond.getDiamondStorage(); diff --git a/ethereum/contracts/zksync/facets/Executor.sol b/ethereum/contracts/zksync/facets/Executor.sol index 633061b18..e9e33932c 100644 --- a/ethereum/contracts/zksync/facets/Executor.sol +++ b/ethereum/contracts/zksync/facets/Executor.sol @@ -11,6 +11,9 @@ import {UnsafeBytes} from "../../common/libraries/UnsafeBytes.sol"; import {VerifierParams} from "../Storage.sol"; import {L2_BOOTLOADER_ADDRESS, L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR, L2_SYSTEM_CONTEXT_SYSTEM_CONTRACT_ADDR, L2_KNOWN_CODE_STORAGE_SYSTEM_CONTRACT_ADDR} from "../../common/L2ContractAddresses.sol"; +// While formally the following import is not used, it is needed to inherit documentation from it +import {IBase} from "../interfaces/IBase.sol"; + /// @title zkSync Executor contract capable of processing events emitted in the zkSync protocol. /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -18,6 +21,7 @@ contract ExecutorFacet is Base, IExecutor { using UncheckedMath for uint256; using PriorityQueue for PriorityQueue.Queue; + /// @inheritdoc IBase string public constant override getName = "ExecutorFacet"; /// @dev Process one batch commit using the previous batch StoredBatchInfo @@ -172,10 +176,7 @@ contract ExecutorFacet is Base, IExecutor { } } - /// @notice Commit batch - /// @notice 1. Checks timestamp. - /// @notice 2. Process L2 logs. - /// @notice 3. Store batch commitments. + /// @inheritdoc IExecutor function commitBatches( StoredBatchInfo memory _lastCommittedBatchData, CommitBatchInfo[] calldata _newBatchesData @@ -285,9 +286,7 @@ contract ExecutorFacet is Base, IExecutor { s.l2LogsRootHashes[currentBatchNumber] = _storedBatch.l2LogsTreeRoot; } - /// @notice Execute batches, complete priority operations and process withdrawals. - /// @notice 1. Processes all pending operations (Complete priority requests) - /// @notice 2. Finalizes batch on Ethereum + /// @inheritdoc IExecutor function executeBatches(StoredBatchInfo[] calldata _batchesData) external nonReentrant onlyValidator { uint256 nBatches = _batchesData.length; for (uint256 i = 0; i < nBatches; i = i.uncheckedInc()) { @@ -306,8 +305,7 @@ contract ExecutorFacet is Base, IExecutor { } } - /// @notice Batches commitment verification. - /// @notice Only verifies batch commitments without any other processing + /// @inheritdoc IExecutor function proveBatches( StoredBatchInfo calldata _prevBatch, StoredBatchInfo[] calldata _committedBatches, @@ -393,10 +391,7 @@ contract ExecutorFacet is Base, IExecutor { ) >> PUBLIC_INPUT_SHIFT; } - /// @notice Reverts unexecuted batches - /// @param _newLastBatch batch number after which batches should be reverted - /// NOTE: Doesn't delete the stored data about batches, but only decreases - /// counters that are responsible for the number of batches + /// @inheritdoc IExecutor function revertBatches(uint256 _newLastBatch) external nonReentrant onlyValidator { require(s.totalBatchesCommitted > _newLastBatch, "v1"); // The last committed batch is less than new last batch require(_newLastBatch >= s.totalBatchesExecuted, "v2"); // Already executed batches cannot be reverted diff --git a/ethereum/contracts/zksync/facets/Getters.sol b/ethereum/contracts/zksync/facets/Getters.sol index 2a181b138..04ed1535c 100644 --- a/ethereum/contracts/zksync/facets/Getters.sol +++ b/ethereum/contracts/zksync/facets/Getters.sol @@ -9,6 +9,9 @@ import "../../common/libraries/UncheckedMath.sol"; import "../interfaces/IGetters.sol"; import "../interfaces/ILegacyGetters.sol"; +// While formally the following import is not used, it is needed to inherit documentation from it +import {IBase} from "../interfaces/IBase.sol"; + /// @title Getters Contract implements functions for getting contract state from outside the batchchain. /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -16,122 +19,115 @@ contract GettersFacet is Base, IGetters, ILegacyGetters { using UncheckedMath for uint256; using PriorityQueue for PriorityQueue.Queue; + /// @inheritdoc IBase string public constant override getName = "GettersFacet"; /*////////////////////////////////////////////////////////////// CUSTOM GETTERS //////////////////////////////////////////////////////////////*/ - /// @return The address of the verifier smart contract + /// @inheritdoc IGetters function getVerifier() external view returns (address) { return address(s.verifier); } - /// @return The address of the current governor + /// @inheritdoc IGetters function getGovernor() external view returns (address) { return s.governor; } - /// @return The address of the pending governor + /// @inheritdoc IGetters function getPendingGovernor() external view returns (address) { return s.pendingGovernor; } - /// @return The total number of batches that were committed + /// @inheritdoc IGetters function getTotalBatchesCommitted() external view returns (uint256) { return s.totalBatchesCommitted; } - /// @return The total number of batches that were committed & verified + /// @inheritdoc IGetters function getTotalBatchesVerified() external view returns (uint256) { return s.totalBatchesVerified; } - /// @return The total number of batches that were committed & verified & executed + /// @inheritdoc IGetters function getTotalBatchesExecuted() external view returns (uint256) { return s.totalBatchesExecuted; } - /// @return The total number of priority operations that were added to the priority queue, including all processed ones + /// @inheritdoc IGetters function getTotalPriorityTxs() external view returns (uint256) { return s.priorityQueue.getTotalPriorityTxs(); } - /// @notice Returns zero if and only if no operations were processed from the queue - /// @notice Reverts if there are no unprocessed priority transactions - /// @return Index of the oldest priority operation that wasn't processed yet + /// @inheritdoc IGetters function getFirstUnprocessedPriorityTx() external view returns (uint256) { return s.priorityQueue.getFirstUnprocessedPriorityTx(); } - /// @return The number of priority operations currently in the queue + /// @inheritdoc IGetters function getPriorityQueueSize() external view returns (uint256) { return s.priorityQueue.getSize(); } - /// @return The first unprocessed priority operation from the queue + /// @inheritdoc IGetters function priorityQueueFrontOperation() external view returns (PriorityOperation memory) { return s.priorityQueue.front(); } - /// @return Whether the address has a validator access + /// @inheritdoc IGetters function isValidator(address _address) external view returns (bool) { return s.validators[_address]; } - /// @return Merkle root of the tree with L2 logs for the selected batch + /// @inheritdoc IGetters function l2LogsRootHash(uint256 _batchNumber) external view returns (bytes32) { return s.l2LogsRootHashes[_batchNumber]; } - /// @notice For unfinalized (non executed) batches may change - /// @dev returns zero for non-committed batches - /// @return The hash of committed L2 batch. + /// @inheritdoc IGetters function storedBatchHash(uint256 _batchNumber) external view returns (bytes32) { return s.storedBatchHashes[_batchNumber]; } - /// @return Bytecode hash of bootloader program. + /// @inheritdoc IGetters function getL2BootloaderBytecodeHash() external view returns (bytes32) { return s.l2BootloaderBytecodeHash; } - /// @return Bytecode hash of default account (bytecode for EOA). + /// @inheritdoc IGetters function getL2DefaultAccountBytecodeHash() external view returns (bytes32) { return s.l2DefaultAccountBytecodeHash; } - /// @return Verifier parameters. + /// @inheritdoc IGetters function getVerifierParams() external view returns (VerifierParams memory) { return s.verifierParams; } - /// @return The current protocol version + /// @inheritdoc IGetters function getProtocolVersion() external view returns (uint256) { return s.protocolVersion; } - /// @return The upgrade system contract transaction hash, 0 if the upgrade is not initialized + /// @inheritdoc IGetters function getL2SystemContractsUpgradeTxHash() external view returns (bytes32) { return s.l2SystemContractsUpgradeTxHash; } - /// @return The L2 batch number in which the upgrade transaction was processed. - /// @dev It is equal to 0 in the following two cases: - /// - No upgrade transaction has ever been processed. - /// - The upgrade transaction has been processed and the batch with such transaction has been - /// executed (i.e. finalized). + /// @inheritdoc IGetters function getL2SystemContractsUpgradeBatchNumber() external view returns (uint256) { return s.l2SystemContractsUpgradeBatchNumber; } - /// @return Whether the diamond is frozen or not + /// @inheritdoc IGetters function isDiamondStorageFrozen() external view returns (bool) { Diamond.DiamondStorage storage ds = Diamond.getDiamondStorage(); return ds.isFrozen; } - /// @return isFreezable Whether the facet can be frozen by the governor or always accessible + /// @inheritdoc IGetters function isFacetFreezable(address _facet) external view returns (bool isFreezable) { Diamond.DiamondStorage storage ds = Diamond.getDiamondStorage(); @@ -144,21 +140,19 @@ contract GettersFacet is Base, IGetters, ILegacyGetters { } } - /// @return The maximum number of L2 gas that a user can request for L1 -> L2 transactions + /// @inheritdoc IGetters function getPriorityTxMaxGasLimit() external view returns (uint256) { return s.priorityTxMaxGasLimit; } - /// @return Whether the selector can be frozen by the governor or always accessible + /// @inheritdoc IGetters function isFunctionFreezable(bytes4 _selector) external view returns (bool) { Diamond.DiamondStorage storage ds = Diamond.getDiamondStorage(); require(ds.selectorToFacet[_selector].facetAddress != address(0), "g2"); return ds.selectorToFacet[_selector].isFreezable; } - /// @return Whether a withdrawal has been finalized. - /// @param _l2BatchNumber The L2 batch number within which the withdrawal happened. - /// @param _l2MessageIndex The index of the L2->L1 message denoting the withdrawal. + /// @inheritdoc IGetters function isEthWithdrawalFinalized(uint256 _l2BatchNumber, uint256 _l2MessageIndex) external view returns (bool) { return s.isEthWithdrawalFinalized[_l2BatchNumber][_l2MessageIndex]; } @@ -167,7 +161,7 @@ contract GettersFacet is Base, IGetters, ILegacyGetters { DIAMOND LOUPE //////////////////////////////////////////////////////////////*/ - /// @return result All facet addresses and their function selectors + /// @inheritdoc IGetters function facets() external view returns (Facet[] memory result) { Diamond.DiamondStorage storage ds = Diamond.getDiamondStorage(); @@ -182,19 +176,19 @@ contract GettersFacet is Base, IGetters, ILegacyGetters { } } - /// @return NON-sorted array with function selectors supported by a specific facet + /// @inheritdoc IGetters function facetFunctionSelectors(address _facet) external view returns (bytes4[] memory) { Diamond.DiamondStorage storage ds = Diamond.getDiamondStorage(); return ds.facetToSelectors[_facet].selectors; } - /// @return NON-sorted array of facet addresses supported on diamond + /// @inheritdoc IGetters function facetAddresses() external view returns (address[] memory) { Diamond.DiamondStorage storage ds = Diamond.getDiamondStorage(); return ds.facets; } - /// @return Facet address associated with a selector. Zero if the selector is not added to the diamond + /// @inheritdoc IGetters function facetAddress(bytes4 _selector) external view returns (address) { Diamond.DiamondStorage storage ds = Diamond.getDiamondStorage(); return ds.selectorToFacet[_selector].facetAddress; @@ -204,38 +198,27 @@ contract GettersFacet is Base, IGetters, ILegacyGetters { DEPRECATED METHODS //////////////////////////////////////////////////////////////*/ - /// @return The total number of batches that were committed - /// @dev It is a *deprecated* method, please use `getTotalBatchesCommitted` instead + /// @inheritdoc ILegacyGetters function getTotalBlocksCommitted() external view returns (uint256) { return s.totalBatchesCommitted; } - /// @return The total number of batches that were committed & verified - /// @dev It is a *deprecated* method, please use `getTotalBatchesVerified` instead. + /// @inheritdoc ILegacyGetters function getTotalBlocksVerified() external view returns (uint256) { return s.totalBatchesVerified; } - /// @return The total number of batches that were committed & verified & executed - /// @dev It is a *deprecated* method, please use `getTotalBatchesExecuted` instead. + /// @inheritdoc ILegacyGetters function getTotalBlocksExecuted() external view returns (uint256) { return s.totalBatchesExecuted; } - /// @notice For unfinalized (non executed) batches may change - /// @dev It is a *deprecated* method, please use `storedBatchHash` instead. - /// @dev returns zero for non-committed batches - /// @return The hash of committed L2 batch. + /// @inheritdoc ILegacyGetters function storedBlockHash(uint256 _batchNumber) external view returns (bytes32) { return s.storedBatchHashes[_batchNumber]; } - /// @return The L2 batch number in which the upgrade transaction was processed. - /// @dev It is a *deprecated* method, please use `getL2SystemContractsUpgradeBatchNumber` instead. - /// @dev It is equal to 0 in the following two cases: - /// - No upgrade transaction has ever been processed. - /// - The upgrade transaction has been processed and the batch with such transaction has been - /// executed (i.e. finalized). + /// @inheritdoc ILegacyGetters function getL2SystemContractsUpgradeBlockNumber() external view returns (uint256) { return s.l2SystemContractsUpgradeBatchNumber; } diff --git a/ethereum/contracts/zksync/facets/Mailbox.sol b/ethereum/contracts/zksync/facets/Mailbox.sol index c161ed664..bd2d040e2 100644 --- a/ethereum/contracts/zksync/facets/Mailbox.sol +++ b/ethereum/contracts/zksync/facets/Mailbox.sol @@ -17,6 +17,9 @@ import {Base} from "./Base.sol"; import {REQUIRED_L2_GAS_PRICE_PER_PUBDATA, FAIR_L2_GAS_PRICE, L1_GAS_PER_PUBDATA_BYTE, L2_L1_LOGS_TREE_DEFAULT_LEAF_HASH, PRIORITY_OPERATION_L2_TX_TYPE, PRIORITY_EXPIRATION, MAX_NEW_FACTORY_DEPS} from "../Config.sol"; import {L2_BOOTLOADER_ADDRESS, L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR, L2_ETH_TOKEN_SYSTEM_CONTRACT_ADDR} from "../../common/L2ContractAddresses.sol"; +// While formally the following import is not used, it is needed to inherit documentation from it +import {IBase} from "../interfaces/IBase.sol"; + /// @title zkSync Mailbox contract providing interfaces for L1 <-> L2 interaction. /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -24,14 +27,10 @@ contract MailboxFacet is Base, IMailbox { using UncheckedMath for uint256; using PriorityQueue for PriorityQueue.Queue; + /// @inheritdoc IBase string public constant override getName = "MailboxFacet"; - /// @notice Prove that a specific arbitrary-length message was sent in a specific L2 batch number - /// @param _batchNumber The executed L2 batch number in which the message appeared - /// @param _index The position in the L2 logs Merkle tree of the l2Log that was sent with the message - /// @param _message Information about the sent message: sender address, the message itself, tx index in the L2 batch where the message was sent - /// @param _proof Merkle proof for inclusion of L2 log that was sent with the message - /// @return Whether the proof is valid + /// @inheritdoc IMailbox function proveL2MessageInclusion( uint256 _batchNumber, uint256 _index, @@ -41,12 +40,7 @@ contract MailboxFacet is Base, IMailbox { return _proveL2LogInclusion(_batchNumber, _index, _L2MessageToLog(_message), _proof); } - /// @notice Prove that a specific L2 log was sent in a specific L2 batch - /// @param _batchNumber The executed L2 batch number in which the log appeared - /// @param _index The position of the l2log in the L2 logs Merkle tree - /// @param _log Information about the sent log - /// @param _proof Merkle proof for inclusion of the L2 log - /// @return Whether the proof is correct and L2 log is included in batch + /// @inheritdoc IMailbox function proveL2LogInclusion( uint256 _batchNumber, uint256 _index, @@ -56,15 +50,7 @@ contract MailboxFacet is Base, IMailbox { return _proveL2LogInclusion(_batchNumber, _index, _log, _proof); } - /// @notice Prove that the L1 -> L2 transaction was processed with the specified status. - /// @param _l2TxHash The L2 canonical transaction hash - /// @param _l2BatchNumber The L2 batch number where the transaction was processed - /// @param _l2MessageIndex The position in the L2 logs Merkle tree of the l2Log that was sent with the message - /// @param _l2TxNumberInBatch The L2 transaction number in the batch, in which the log was sent - /// @param _merkleProof The Merkle proof of the processing L1 -> L2 transaction - /// @param _status The execution status of the L1 -> L2 transaction (true - success & 0 - fail) - /// @return Whether the proof is correct and the transaction was actually executed with provided status - /// NOTE: It may return `false` for incorrect proof, but it doesn't mean that the L1 -> L2 transaction has an opposite status! + /// @inheritdoc IMailbox function proveL1ToL2TransactionStatus( bytes32 _l2TxHash, uint256 _l2BatchNumber, @@ -144,11 +130,7 @@ contract MailboxFacet is Base, IMailbox { }); } - /// @notice Estimates the cost in Ether of requesting execution of an L2 transaction from L1 - /// @param _gasPrice expected L1 gas price at which the user requests the transaction execution - /// @param _l2GasLimit Maximum amount of L2 gas that transaction can consume during execution on L2 - /// @param _l2GasPerPubdataByteLimit The maximum amount of L2 gas that the operator may charge the user for a single byte of pubdata. - /// @return The estimated ETH spent on L2 gas for the transaction + /// @inheritdoc IMailbox function l2TransactionBaseCost( uint256 _gasPrice, uint256 _l2GasLimit, @@ -169,12 +151,7 @@ contract MailboxFacet is Base, IMailbox { return Math.max(FAIR_L2_GAS_PRICE, minL2GasPriceETH); } - /// @notice Finalize the withdrawal and release funds - /// @param _l2BatchNumber The L2 batch number where the withdrawal was processed - /// @param _l2MessageIndex The position in the L2 logs Merkle tree of the l2Log that was sent with the message - /// @param _l2TxNumberInBatch The L2 transaction number in a batch, in which the log was sent - /// @param _message The L2 withdraw data, stored in an L2 -> L1 message - /// @param _merkleProof The Merkle proof of the inclusion L2 -> L1 message about withdrawal initialization + /// @inheritdoc IMailbox function finalizeEthWithdrawal( uint256 _l2BatchNumber, uint256 _l2MessageIndex, @@ -201,24 +178,7 @@ contract MailboxFacet is Base, IMailbox { emit EthWithdrawalFinalized(_l1WithdrawReceiver, _amount); } - /// @notice Request execution of L2 transaction from L1. - /// @param _contractL2 The L2 receiver address - /// @param _l2Value `msg.value` of L2 transaction - /// @param _calldata The input of the L2 transaction - /// @param _l2GasLimit Maximum amount of L2 gas that transaction can consume during execution on L2 - /// @param _l2GasPerPubdataByteLimit The maximum amount L2 gas that the operator may charge the user for single byte of pubdata. - /// @param _factoryDeps An array of L2 bytecodes that will be marked as known on L2 - /// @param _refundRecipient The address on L2 that will receive the refund for the transaction. - /// @dev If the L2 deposit finalization transaction fails, the `_refundRecipient` will receive the `_l2Value`. - /// Please note, the contract may change the refund recipient's address to eliminate sending funds to addresses out of control. - /// - If `_refundRecipient` is a contract on L1, the refund will be sent to the aliased `_refundRecipient`. - /// - If `_refundRecipient` is set to `address(0)` and the sender has NO deployed bytecode on L1, the refund will be sent to the `msg.sender` address. - /// - If `_refundRecipient` is set to `address(0)` and the sender has deployed bytecode on L1, the refund will be sent to the aliased `msg.sender` address. - /// @dev The address aliasing of L1 contracts as refund recipient on L2 is necessary to guarantee that the funds are controllable, - /// since address aliasing to the from address for the L2 tx will be applied if the L1 `msg.sender` is a contract. - /// Without address aliasing for L1 contracts as refund recipients they would not be able to make proper L2 tx requests - /// through the Mailbox to use or withdraw the funds from L2, and the funds would be lost. - /// @return canonicalTxHash The hash of the requested L2 transaction. This hash can be used to follow the transaction status + /// @inheritdoc IMailbox function requestL2Transaction( address _contractL2, uint256 _l2Value, diff --git a/ethereum/contracts/zksync/interfaces/IAdmin.sol b/ethereum/contracts/zksync/interfaces/IAdmin.sol index 15edb7ec1..529d60b0a 100644 --- a/ethereum/contracts/zksync/interfaces/IAdmin.sol +++ b/ethereum/contracts/zksync/interfaces/IAdmin.sol @@ -7,24 +7,46 @@ import "./IBase.sol"; import {Diamond} from "../libraries/Diamond.sol"; interface IAdmin is IBase { + /// @notice Starts the transfer of governor rights. Only the current governor can propose a new pending one. + /// @notice New governor can accept governor rights by calling `acceptGovernor` function. + /// @param _newPendingGovernor Address of the new governor function setPendingGovernor(address _newPendingGovernor) external; + /// @notice Accepts transfer of governor rights. Only pending governor can accept the role. function acceptGovernor() external; + /// @notice Starts the transfer of admin rights. Only the current governor or admin can propose a new pending one. + /// @notice New admin can accept admin rights by calling `acceptAdmin` function. + /// @param _newPendingAdmin Address of the new admin function setPendingAdmin(address _newPendingAdmin) external; + /// @notice Accepts transfer of admin rights. Only pending admin can accept the role. function acceptAdmin() external; + /// @notice Change validator status (active or not active) + /// @param _validator Validator address + /// @param _active Active flag function setValidator(address _validator, bool _active) external; + /// @notice Change zk porter availability + /// @param _zkPorterIsAvailable The availability of zk porter shard function setPorterAvailability(bool _zkPorterIsAvailable) external; + /// @notice Change the max L2 gas limit for L1 -> L2 transactions + /// @param _newPriorityTxMaxGasLimit The maximum number of L2 gas that a user can request for L1 -> L2 transactions function setPriorityTxMaxGasLimit(uint256 _newPriorityTxMaxGasLimit) external; + /// @notice Executes a proposed governor upgrade + /// @dev Only the current governor can execute the upgrade + /// @param _diamondCut The diamond cut parameters to be executed function executeUpgrade(Diamond.DiamondCutData calldata _diamondCut) external; + /// @notice Instantly pause the functionality of all freezable facets & their selectors + /// @dev Only the governance mechanism may freeze Diamond Proxy function freezeDiamond() external; + /// @notice Unpause the functionality of all freezable facets & their selectors + /// @dev Both the governor and its owner can unfreeze Diamond Proxy function unfreezeDiamond() external; /// @notice Porter availability status changes diff --git a/ethereum/contracts/zksync/interfaces/IBase.sol b/ethereum/contracts/zksync/interfaces/IBase.sol index d77eb205c..3076ae2d1 100644 --- a/ethereum/contracts/zksync/interfaces/IBase.sol +++ b/ethereum/contracts/zksync/interfaces/IBase.sol @@ -2,5 +2,6 @@ pragma solidity 0.8.20; interface IBase { + /// @return Returns facet name. function getName() external view returns (string memory); } diff --git a/ethereum/contracts/zksync/interfaces/IExecutor.sol b/ethereum/contracts/zksync/interfaces/IExecutor.sol index 50e983945..ec0e8507b 100644 --- a/ethereum/contracts/zksync/interfaces/IExecutor.sol +++ b/ethereum/contracts/zksync/interfaces/IExecutor.sol @@ -76,19 +76,38 @@ interface IExecutor is IBase { uint256[] serializedProof; } + /// @notice Function called by the operator to commit new batches. It is responsible for: + /// - Verifying the correctness of their timestamps. + /// - Processing their L2->L1 logs. + /// - Storing batch commitments. + /// @param _lastCommittedBatchData Stored data of the last committed batch. + /// @param _newBatchesData Data of the new batches to be committed. function commitBatches( StoredBatchInfo calldata _lastCommittedBatchData, CommitBatchInfo[] calldata _newBatchesData ) external; + /// @notice Batches commitment verification. + /// @dev Only verifies batch commitments without any other processing. + /// @param _prevBatch Stored data of the last committed batch. + /// @param _committedBatches Stored data of the committed batches. + /// @param _proof The zero knowledge proof. function proveBatches( StoredBatchInfo calldata _prevBatch, StoredBatchInfo[] calldata _committedBatches, ProofInput calldata _proof ) external; + /// @notice The function called by the operator to finalize (execute) batches. It is responsible for: + /// - Processing all pending operations (commpleting priority requests). + /// - Finalizing this batch (i.e. allowing to withdraw funds from the system) + /// @param _batchesData Data of the batches to be executed. function executeBatches(StoredBatchInfo[] calldata _batchesData) external; + /// @notice Reverts unexecuted batches + /// @param _newLastBatch batch number after which batches should be reverted + /// NOTE: Doesn't delete the stored data about batches, but only decreases + /// counters that are responsible for the number of batches function revertBatches(uint256 _newLastBatch) external; /// @notice Event emitted when a batch is committed diff --git a/ethereum/contracts/zksync/interfaces/IGetters.sol b/ethereum/contracts/zksync/interfaces/IGetters.sol index 46310b556..c866835b8 100644 --- a/ethereum/contracts/zksync/interfaces/IGetters.sol +++ b/ethereum/contracts/zksync/interfaces/IGetters.sol @@ -11,48 +11,80 @@ interface IGetters is IBase { CUSTOM GETTERS //////////////////////////////////////////////////////////////*/ + /// @return The address of the verifier smart contract function getVerifier() external view returns (address); + /// @return The address of the current governor function getGovernor() external view returns (address); + /// @return The address of the pending governor function getPendingGovernor() external view returns (address); + /// @return The total number of batches that were committed function getTotalBatchesCommitted() external view returns (uint256); + /// @return The total number of batches that were committed & verified function getTotalBatchesVerified() external view returns (uint256); + /// @return The total number of batches that were committed & verified & executed function getTotalBatchesExecuted() external view returns (uint256); + /// @return The total number of priority operations that were added to the priority queue, including all processed ones function getTotalPriorityTxs() external view returns (uint256); + /// @notice Returns zero if and only if no operations were processed from the queue + /// @notice Reverts if there are no unprocessed priority transactions + /// @return Index of the oldest priority operation that wasn't processed yet function getFirstUnprocessedPriorityTx() external view returns (uint256); + /// @return The number of priority operations currently in the queue function getPriorityQueueSize() external view returns (uint256); + /// @return The first unprocessed priority operation from the queue function priorityQueueFrontOperation() external view returns (PriorityOperation memory); + /// @return Whether the address has a validator access function isValidator(address _address) external view returns (bool); - function l2LogsRootHash(uint256 _batchNumber) external view returns (bytes32 hash); + /// @return merkleRoot Merkle root of the tree with L2 logs for the selected batch + function l2LogsRootHash(uint256 _batchNumber) external view returns (bytes32 merkleRoot); + /// @notice For unfinalized (non executed) batches may change + /// @dev returns zero for non-committed batches + /// @return The hash of committed L2 batch. function storedBatchHash(uint256 _batchNumber) external view returns (bytes32); + /// @return Bytecode hash of bootloader program. function getL2BootloaderBytecodeHash() external view returns (bytes32); + /// @return Bytecode hash of default account (bytecode for EOA). function getL2DefaultAccountBytecodeHash() external view returns (bytes32); + /// @return Verifier parameters. function getVerifierParams() external view returns (VerifierParams memory); + /// @return Whether the diamond is frozen or not function isDiamondStorageFrozen() external view returns (bool); + /// @return The current protocol version function getProtocolVersion() external view returns (uint256); + /// @return The upgrade system contract transaction hash, 0 if the upgrade is not initialized function getL2SystemContractsUpgradeTxHash() external view returns (bytes32); + /// @return The L2 batch number in which the upgrade transaction was processed. + /// @dev It is equal to 0 in the following two cases: + /// - No upgrade transaction has ever been processed. + /// - The upgrade transaction has been processed and the batch with such transaction has been + /// executed (i.e. finalized). function getL2SystemContractsUpgradeBatchNumber() external view returns (uint256); + /// @return The maximum number of L2 gas that a user can request for L1 -> L2 transactions function getPriorityTxMaxGasLimit() external view returns (uint256); + /// @return Whether a withdrawal has been finalized. + /// @param _l2BatchNumber The L2 batch number within which the withdrawal happened. + /// @param _l2MessageIndex The index of the L2->L1 message denoting the withdrawal. function isEthWithdrawalFinalized(uint256 _l2BatchNumber, uint256 _l2MessageIndex) external view returns (bool); /*////////////////////////////////////////////////////////////// @@ -67,15 +99,21 @@ interface IGetters is IBase { bytes4[] selectors; } + /// @return result All facet addresses and their function selectors function facets() external view returns (Facet[] memory); + /// @return NON-sorted array with function selectors supported by a specific facet function facetFunctionSelectors(address _facet) external view returns (bytes4[] memory); + /// @return facets NON-sorted array of facet addresses supported on diamond function facetAddresses() external view returns (address[] memory facets); + /// @return facet The facet address associated with a selector. Zero if the selector is not added to the diamond function facetAddress(bytes4 _selector) external view returns (address facet); + /// @return Whether the selector can be frozen by the governor or always accessible function isFunctionFreezable(bytes4 _selector) external view returns (bool); + /// @return isFreezable Whether the facet can be frozen by the governor or always accessible function isFacetFreezable(address _facet) external view returns (bool isFreezable); } diff --git a/ethereum/contracts/zksync/interfaces/ILegacyGetters.sol b/ethereum/contracts/zksync/interfaces/ILegacyGetters.sol index faaa76e36..781796ff0 100644 --- a/ethereum/contracts/zksync/interfaces/ILegacyGetters.sol +++ b/ethereum/contracts/zksync/interfaces/ILegacyGetters.sol @@ -8,13 +8,29 @@ import "./IBase.sol"; /// @dev This interface contains getters for the zkSync contract that should not be used, /// but still are keot for backward compatibility. interface ILegacyGetters is IBase { + /// @return The total number of batches that were committed + /// @dev It is a *deprecated* method, please use `getTotalBatchesCommitted` instead function getTotalBlocksCommitted() external view returns (uint256); + /// @return The total number of batches that were committed & verified + /// @dev It is a *deprecated* method, please use `getTotalBatchesVerified` instead. function getTotalBlocksVerified() external view returns (uint256); + /// @return The total number of batches that were committed & verified & executed + /// @dev It is a *deprecated* method, please use `getTotalBatchesExecuted` instead. function getTotalBlocksExecuted() external view returns (uint256); + /// @notice For unfinalized (non executed) batches may change + /// @dev It is a *deprecated* method, please use `storedBatchHash` instead. + /// @dev returns zero for non-committed batches + /// @return The hash of committed L2 batch. function storedBlockHash(uint256 _batchNumber) external view returns (bytes32); + /// @return The L2 batch number in which the upgrade transaction was processed. + /// @dev It is a *deprecated* method, please use `getL2SystemContractsUpgradeBatchNumber` instead. + /// @dev It is equal to 0 in the following two cases: + /// - No upgrade transaction has ever been processed. + /// - The upgrade transaction has been processed and the batch with such transaction has been + /// executed (i.e. finalized). function getL2SystemContractsUpgradeBlockNumber() external view returns (uint256); } diff --git a/ethereum/contracts/zksync/interfaces/IMailbox.sol b/ethereum/contracts/zksync/interfaces/IMailbox.sol index fedd3eee7..177b900d8 100644 --- a/ethereum/contracts/zksync/interfaces/IMailbox.sol +++ b/ethereum/contracts/zksync/interfaces/IMailbox.sol @@ -87,6 +87,12 @@ interface IMailbox is IBase { address refundRecipient; } + /// @notice Prove that a specific arbitrary-length message was sent in a specific L2 batch number + /// @param _l2BatchNumber The executed L2 batch number in which the message appeared + /// @param _index The position in the L2 logs Merkle tree of the l2Log that was sent with the message + /// @param _message Information about the sent message: sender address, the message itself, tx index in the L2 batch where the message was sent + /// @param _proof Merkle proof for inclusion of L2 log that was sent with the message + /// @return Whether the proof is valid function proveL2MessageInclusion( uint256 _l2BatchNumber, uint256 _index, @@ -94,6 +100,12 @@ interface IMailbox is IBase { bytes32[] calldata _proof ) external view returns (bool); + /// @notice Prove that a specific L2 log was sent in a specific L2 batch + /// @param _l2BatchNumber The executed L2 batch number in which the log appeared + /// @param _index The position of the l2log in the L2 logs Merkle tree + /// @param _log Information about the sent log + /// @param _proof Merkle proof for inclusion of the L2 log + /// @return Whether the proof is correct and L2 log is included in batch function proveL2LogInclusion( uint256 _l2BatchNumber, uint256 _index, @@ -101,6 +113,15 @@ interface IMailbox is IBase { bytes32[] calldata _proof ) external view returns (bool); + /// @notice Prove that the L1 -> L2 transaction was processed with the specified status. + /// @param _l2TxHash The L2 canonical transaction hash + /// @param _l2BatchNumber The L2 batch number where the transaction was processed + /// @param _l2MessageIndex The position in the L2 logs Merkle tree of the l2Log that was sent with the message + /// @param _l2TxNumberInBatch The L2 transaction number in the batch, in which the log was sent + /// @param _merkleProof The Merkle proof of the processing L1 -> L2 transaction + /// @param _status The execution status of the L1 -> L2 transaction (true - success & 0 - fail) + /// @return Whether the proof is correct and the transaction was actually executed with provided status + /// NOTE: It may return `false` for incorrect proof, but it doesn't mean that the L1 -> L2 transaction has an opposite status! function proveL1ToL2TransactionStatus( bytes32 _l2TxHash, uint256 _l2BatchNumber, @@ -110,6 +131,12 @@ interface IMailbox is IBase { TxStatus _status ) external view returns (bool); + /// @notice Finalize the withdrawal and release funds + /// @param _l2BatchNumber The L2 batch number where the withdrawal was processed + /// @param _l2MessageIndex The position in the L2 logs Merkle tree of the l2Log that was sent with the message + /// @param _l2TxNumberInBatch The L2 transaction number in a batch, in which the log was sent + /// @param _message The L2 withdraw data, stored in an L2 -> L1 message + /// @param _merkleProof The Merkle proof of the inclusion L2 -> L1 message about withdrawal initialization function finalizeEthWithdrawal( uint256 _l2BatchNumber, uint256 _l2MessageIndex, @@ -118,6 +145,24 @@ interface IMailbox is IBase { bytes32[] calldata _merkleProof ) external; + /// @notice Request execution of L2 transaction from L1. + /// @param _contractL2 The L2 receiver address + /// @param _l2Value `msg.value` of L2 transaction + /// @param _calldata The input of the L2 transaction + /// @param _l2GasLimit Maximum amount of L2 gas that transaction can consume during execution on L2 + /// @param _l2GasPerPubdataByteLimit The maximum amount L2 gas that the operator may charge the user for single byte of pubdata. + /// @param _factoryDeps An array of L2 bytecodes that will be marked as known on L2 + /// @param _refundRecipient The address on L2 that will receive the refund for the transaction. + /// @dev If the L2 deposit finalization transaction fails, the `_refundRecipient` will receive the `_l2Value`. + /// Please note, the contract may change the refund recipient's address to eliminate sending funds to addresses out of control. + /// - If `_refundRecipient` is a contract on L1, the refund will be sent to the aliased `_refundRecipient`. + /// - If `_refundRecipient` is set to `address(0)` and the sender has NO deployed bytecode on L1, the refund will be sent to the `msg.sender` address. + /// - If `_refundRecipient` is set to `address(0)` and the sender has deployed bytecode on L1, the refund will be sent to the aliased `msg.sender` address. + /// @dev The address aliasing of L1 contracts as refund recipient on L2 is necessary to guarantee that the funds are controllable, + /// since address aliasing to the from address for the L2 tx will be applied if the L1 `msg.sender` is a contract. + /// Without address aliasing for L1 contracts as refund recipients they would not be able to make proper L2 tx requests + /// through the Mailbox to use or withdraw the funds from L2, and the funds would be lost. + /// @return canonicalTxHash The hash of the requested L2 transaction. This hash can be used to follow the transaction status function requestL2Transaction( address _contractL2, uint256 _l2Value, @@ -128,6 +173,11 @@ interface IMailbox is IBase { address _refundRecipient ) external payable returns (bytes32 canonicalTxHash); + /// @notice Estimates the cost in Ether of requesting execution of an L2 transaction from L1 + /// @param _gasPrice expected L1 gas price at which the user requests the transaction execution + /// @param _l2GasLimit Maximum amount of L2 gas that transaction can consume during execution on L2 + /// @param _l2GasPerPubdataByteLimit The maximum amount of L2 gas that the operator may charge the user for a single byte of pubdata. + /// @return The estimated ETH spent on L2 gas for the transaction function l2TransactionBaseCost( uint256 _gasPrice, uint256 _l2GasLimit, diff --git a/ethereum/contracts/zksync/interfaces/IVerifier.sol b/ethereum/contracts/zksync/interfaces/IVerifier.sol index b4c103e9a..7638a577c 100644 --- a/ethereum/contracts/zksync/interfaces/IVerifier.sol +++ b/ethereum/contracts/zksync/interfaces/IVerifier.sol @@ -3,11 +3,16 @@ pragma solidity 0.8.20; interface IVerifier { + /// @dev Verifies a zk-SNARK proof. + /// @return A boolean value indicating whether the zk-SNARK proof is valid. + /// Note: The function may revert execution instead of returning false in some cases. function verify( uint256[] calldata _publicInputs, uint256[] calldata _proof, uint256[] calldata _recursiveAggregationInput ) external view returns (bool); + /// @notice Calculates a keccak256 hash of the runtime loaded verification keys. + /// @return vkHash The keccak256 hash of the loaded verification keys. function verificationKeyHash() external pure returns (bytes32); } diff --git a/tools/data/verifier_contract_template.txt b/tools/data/verifier_contract_template.txt index 0f6fc1659..64236b1b1 100644 --- a/tools/data/verifier_contract_template.txt +++ b/tools/data/verifier_contract_template.txt @@ -241,8 +241,7 @@ contract Verifier is IVerifier { uint256 internal constant FR_MASK = 0x1fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; {{residue_g2_elements}} - /// @notice Calculates a keccak256 hash of the runtime loaded verification keys. - /// @return vkHash The keccak256 hash of the loaded verification keys. + /// @inheritdoc IVerifier function verificationKeyHash() external pure returns (bytes32 vkHash) { _loadVerificationKey(); @@ -275,9 +274,7 @@ contract Verifier is IVerifier { } } - /// @dev Verifies a zk-SNARK proof. - /// @return A boolean value indicating whether the zk-SNARK proof is valid. - /// Note: The function may revert execution instead of returning false in some cases. + /// @inheritdoc IVerifier function verify( uint256[] calldata, // _publicInputs uint256[] calldata, // _proof From fb9c2ed570a2e9ced9cb369cdacaa21160b6cb9d Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Tue, 9 Jan 2024 09:49:08 +0100 Subject: [PATCH 03/10] Fix comment for getting the latest unprocessed priority transaction (#137) --- ethereum/contracts/zksync/interfaces/IGetters.sol | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ethereum/contracts/zksync/interfaces/IGetters.sol b/ethereum/contracts/zksync/interfaces/IGetters.sol index c866835b8..e567db3b0 100644 --- a/ethereum/contracts/zksync/interfaces/IGetters.sol +++ b/ethereum/contracts/zksync/interfaces/IGetters.sol @@ -32,8 +32,10 @@ interface IGetters is IBase { /// @return The total number of priority operations that were added to the priority queue, including all processed ones function getTotalPriorityTxs() external view returns (uint256); - /// @notice Returns zero if and only if no operations were processed from the queue - /// @notice Reverts if there are no unprocessed priority transactions + /// @notice The function that returns the first unprocessed priority transaction. + /// @dev Returns zero if and only if no operations were processed from the queue. + /// @dev If all the transactions were processed, it will return the last processed index, so + /// in case exactly *unprocessed* transactions are needed, one should check that getPriorityQueueSize() is greater than 0. /// @return Index of the oldest priority operation that wasn't processed yet function getFirstUnprocessedPriorityTx() external view returns (uint256); From f1596c62913f94cd8cfa94f4269e0c26d405e1a5 Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Tue, 9 Jan 2024 09:50:01 +0100 Subject: [PATCH 04/10] Remove some of the implicit imports (#126) --- ethereum/contracts/bridge/L1ERC20Bridge.sol | 2 ++ ethereum/contracts/bridge/L1WethBridge.sol | 2 ++ ethereum/contracts/upgrades/BaseZkSyncUpgrade.sol | 1 + ethereum/contracts/zksync/facets/Admin.sol | 6 +++--- ethereum/contracts/zksync/facets/Base.sol | 4 ++-- ethereum/contracts/zksync/facets/Getters.sol | 13 +++++++------ ethereum/contracts/zksync/interfaces/IAdmin.sol | 3 +-- ethereum/contracts/zksync/interfaces/IGetters.sol | 2 +- .../contracts/zksync/interfaces/ILegacyGetters.sol | 2 +- ethereum/contracts/zksync/interfaces/IMailbox.sol | 2 +- ethereum/contracts/zksync/interfaces/IZkSync.sol | 8 ++++---- .../zksync/libraries/TransactionValidator.sol | 6 +++--- 12 files changed, 28 insertions(+), 23 deletions(-) diff --git a/ethereum/contracts/bridge/L1ERC20Bridge.sol b/ethereum/contracts/bridge/L1ERC20Bridge.sol index 5077aa839..984e1b365 100644 --- a/ethereum/contracts/bridge/L1ERC20Bridge.sol +++ b/ethereum/contracts/bridge/L1ERC20Bridge.sol @@ -13,6 +13,8 @@ import "./interfaces/IL2ERC20Bridge.sol"; import "./libraries/BridgeInitializationHelper.sol"; import "../zksync/interfaces/IZkSync.sol"; +import {TxStatus} from "../zksync/interfaces/IMailbox.sol"; +import "../zksync/Storage.sol"; import "../common/libraries/UnsafeBytes.sol"; import "../common/libraries/L2ContractHelper.sol"; import "../common/ReentrancyGuard.sol"; diff --git a/ethereum/contracts/bridge/L1WethBridge.sol b/ethereum/contracts/bridge/L1WethBridge.sol index 6d2839458..5dc4c6f3d 100644 --- a/ethereum/contracts/bridge/L1WethBridge.sol +++ b/ethereum/contracts/bridge/L1WethBridge.sol @@ -15,6 +15,8 @@ import "./libraries/BridgeInitializationHelper.sol"; import "../common/libraries/UnsafeBytes.sol"; import "../common/ReentrancyGuard.sol"; import "../common/libraries/L2ContractHelper.sol"; +import {TxStatus} from "../zksync/interfaces/IMailbox.sol"; +import "../zksync/Storage.sol"; import {L2_ETH_TOKEN_SYSTEM_CONTRACT_ADDR} from "../common/L2ContractAddresses.sol"; import "../vendor/AddressAliasHelper.sol"; diff --git a/ethereum/contracts/upgrades/BaseZkSyncUpgrade.sol b/ethereum/contracts/upgrades/BaseZkSyncUpgrade.sol index 93b630491..580fdb155 100644 --- a/ethereum/contracts/upgrades/BaseZkSyncUpgrade.sol +++ b/ethereum/contracts/upgrades/BaseZkSyncUpgrade.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.20; import "../zksync/facets/Base.sol"; +import "../zksync/Storage.sol"; import "../zksync/interfaces/IMailbox.sol"; import "../zksync/interfaces/IVerifier.sol"; import "../common/libraries/L2ContractHelper.sol"; diff --git a/ethereum/contracts/zksync/facets/Admin.sol b/ethereum/contracts/zksync/facets/Admin.sol index 422a70a64..5c98f16f9 100644 --- a/ethereum/contracts/zksync/facets/Admin.sol +++ b/ethereum/contracts/zksync/facets/Admin.sol @@ -2,10 +2,10 @@ pragma solidity 0.8.20; -import "../interfaces/IAdmin.sol"; -import "../libraries/Diamond.sol"; +import {IAdmin} from "../interfaces/IAdmin.sol"; +import {Diamond} from "../libraries/Diamond.sol"; import {L2_TX_MAX_GAS_LIMIT} from "../Config.sol"; -import "./Base.sol"; +import {Base} from "./Base.sol"; // While formally the following import is not used, it is needed to inherit documentation from it import {IBase} from "../interfaces/IBase.sol"; diff --git a/ethereum/contracts/zksync/facets/Base.sol b/ethereum/contracts/zksync/facets/Base.sol index af3cf72c9..51bda761b 100644 --- a/ethereum/contracts/zksync/facets/Base.sol +++ b/ethereum/contracts/zksync/facets/Base.sol @@ -2,8 +2,8 @@ pragma solidity 0.8.20; -import "../Storage.sol"; -import "../../common/ReentrancyGuard.sol"; +import {AppStorage} from "../Storage.sol"; +import {ReentrancyGuard} from "../../common/ReentrancyGuard.sol"; /// @title Base contract containing functions accessible to the other facets. /// @author Matter Labs diff --git a/ethereum/contracts/zksync/facets/Getters.sol b/ethereum/contracts/zksync/facets/Getters.sol index 04ed1535c..0c6437e69 100644 --- a/ethereum/contracts/zksync/facets/Getters.sol +++ b/ethereum/contracts/zksync/facets/Getters.sol @@ -2,12 +2,13 @@ pragma solidity 0.8.20; -import "./Base.sol"; -import "../libraries/Diamond.sol"; -import "../libraries/PriorityQueue.sol"; -import "../../common/libraries/UncheckedMath.sol"; -import "../interfaces/IGetters.sol"; -import "../interfaces/ILegacyGetters.sol"; +import {Base} from "./Base.sol"; +import {VerifierParams} from "../Storage.sol"; +import {Diamond} from "../libraries/Diamond.sol"; +import {PriorityQueue, PriorityOperation} from "../libraries/PriorityQueue.sol"; +import {UncheckedMath} from "../../common/libraries/UncheckedMath.sol"; +import {IGetters} from "../interfaces/IGetters.sol"; +import {ILegacyGetters} from "../interfaces/ILegacyGetters.sol"; // While formally the following import is not used, it is needed to inherit documentation from it import {IBase} from "../interfaces/IBase.sol"; diff --git a/ethereum/contracts/zksync/interfaces/IAdmin.sol b/ethereum/contracts/zksync/interfaces/IAdmin.sol index 529d60b0a..e4e6fd709 100644 --- a/ethereum/contracts/zksync/interfaces/IAdmin.sol +++ b/ethereum/contracts/zksync/interfaces/IAdmin.sol @@ -2,8 +2,7 @@ pragma solidity 0.8.20; -import "./IBase.sol"; - +import {IBase} from "./IBase.sol"; import {Diamond} from "../libraries/Diamond.sol"; interface IAdmin is IBase { diff --git a/ethereum/contracts/zksync/interfaces/IGetters.sol b/ethereum/contracts/zksync/interfaces/IGetters.sol index e567db3b0..2efd3aafe 100644 --- a/ethereum/contracts/zksync/interfaces/IGetters.sol +++ b/ethereum/contracts/zksync/interfaces/IGetters.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.20; -import "../libraries/PriorityQueue.sol"; +import {PriorityOperation} from "../libraries/PriorityQueue.sol"; import {VerifierParams, UpgradeState} from "../Storage.sol"; import "./IBase.sol"; diff --git a/ethereum/contracts/zksync/interfaces/ILegacyGetters.sol b/ethereum/contracts/zksync/interfaces/ILegacyGetters.sol index 781796ff0..94838584e 100644 --- a/ethereum/contracts/zksync/interfaces/ILegacyGetters.sol +++ b/ethereum/contracts/zksync/interfaces/ILegacyGetters.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.20; -import "./IBase.sol"; +import {IBase} from "./IBase.sol"; /// @author Matter Labs /// @dev This interface contains getters for the zkSync contract that should not be used, diff --git a/ethereum/contracts/zksync/interfaces/IMailbox.sol b/ethereum/contracts/zksync/interfaces/IMailbox.sol index 177b900d8..77d816799 100644 --- a/ethereum/contracts/zksync/interfaces/IMailbox.sol +++ b/ethereum/contracts/zksync/interfaces/IMailbox.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.20; import {L2Log, L2Message} from "../Storage.sol"; -import "./IBase.sol"; +import {IBase} from "./IBase.sol"; /// @dev The enum that represents the transaction execution status /// @param Failure The transaction execution failed diff --git a/ethereum/contracts/zksync/interfaces/IZkSync.sol b/ethereum/contracts/zksync/interfaces/IZkSync.sol index 020ecb034..9f58e16b6 100644 --- a/ethereum/contracts/zksync/interfaces/IZkSync.sol +++ b/ethereum/contracts/zksync/interfaces/IZkSync.sol @@ -2,9 +2,9 @@ pragma solidity 0.8.20; -import "./IMailbox.sol"; -import "./IAdmin.sol"; -import "./IExecutor.sol"; -import "./IGetters.sol"; +import {IMailbox} from "./IMailbox.sol"; +import {IAdmin} from "./IAdmin.sol"; +import {IExecutor} from "./IExecutor.sol"; +import {IGetters} from "./IGetters.sol"; interface IZkSync is IMailbox, IAdmin, IExecutor, IGetters {} diff --git a/ethereum/contracts/zksync/libraries/TransactionValidator.sol b/ethereum/contracts/zksync/libraries/TransactionValidator.sol index 9a7ddd26c..7d125913a 100644 --- a/ethereum/contracts/zksync/libraries/TransactionValidator.sol +++ b/ethereum/contracts/zksync/libraries/TransactionValidator.sol @@ -2,10 +2,10 @@ pragma solidity 0.8.20; -import "@openzeppelin/contracts/utils/math/Math.sol"; +import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; -import "../interfaces/IMailbox.sol"; -import "../Config.sol"; +import {IMailbox} from "../interfaces/IMailbox.sol"; +import {PRIORITY_TX_MAX_PUBDATA, L1_TX_INTRINSIC_L2_GAS, L1_TX_DELTA_544_ENCODING_BYTES, L1_TX_DELTA_FACTORY_DEPS_L2_GAS, L1_TX_MIN_L2_GAS_BASE, L1_TX_INTRINSIC_PUBDATA, L1_TX_DELTA_FACTORY_DEPS_PUBDATA, BATCH_OVERHEAD_L2_GAS, BATCH_OVERHEAD_PUBDATA, MAX_TRANSACTIONS_IN_BATCH, BOOTLOADER_TX_ENCODING_SPACE, L2_TX_MAX_GAS_LIMIT} from "../Config.sol"; /// @title zkSync Library for validating L1 -> L2 transactions /// @author Matter Labs From 873d925e99f4d261cffd2baa2a72a39288e18d96 Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Tue, 9 Jan 2024 09:50:18 +0100 Subject: [PATCH 05/10] Remove unused function (#125) --- ethereum/contracts/zksync/facets/Executor.sol | 5 ----- 1 file changed, 5 deletions(-) diff --git a/ethereum/contracts/zksync/facets/Executor.sol b/ethereum/contracts/zksync/facets/Executor.sol index e9e33932c..06c9a9439 100644 --- a/ethereum/contracts/zksync/facets/Executor.sol +++ b/ethereum/contracts/zksync/facets/Executor.sol @@ -410,11 +410,6 @@ contract ExecutorFacet is Base, IExecutor { emit BlocksRevert(s.totalBatchesCommitted, s.totalBatchesVerified, s.totalBatchesExecuted); } - /// @notice Returns larger of two values - function _maxU256(uint256 a, uint256 b) internal pure returns (uint256) { - return a < b ? b : a; - } - /// @dev Creates batch commitment from its data function _createBatchCommitment( CommitBatchInfo calldata _newBatchData, From b0fc9d3967b2d593db7859565dd1d64833ef7629 Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Tue, 9 Jan 2024 09:50:33 +0100 Subject: [PATCH 06/10] Fix typographical errors (#120) --- ethereum/contracts/zksync/facets/Executor.sol | 2 +- ethereum/contracts/zksync/facets/Getters.sol | 2 +- ethereum/contracts/zksync/interfaces/ILegacyGetters.sol | 2 +- .../contracts/zksync/libraries/TransactionValidator.sol | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ethereum/contracts/zksync/facets/Executor.sol b/ethereum/contracts/zksync/facets/Executor.sol index 06c9a9439..fc340eb32 100644 --- a/ethereum/contracts/zksync/facets/Executor.sol +++ b/ethereum/contracts/zksync/facets/Executor.sol @@ -233,7 +233,7 @@ contract ExecutorFacet is Base, IExecutor { // carried out within the first batch committed after the upgrade. // While the logic of the contract ensures that the s.l2SystemContractsUpgradeBatchNumber is 0 when this function is called, - // this check is added just in case. Since it is a hot read, it does not encure noticable gas cost. + // this check is added just in case. Since it is a hot read, it does not encure noticeable gas cost. require(s.l2SystemContractsUpgradeBatchNumber == 0, "ik"); // Save the batch number where the upgrade transaction was executed. diff --git a/ethereum/contracts/zksync/facets/Getters.sol b/ethereum/contracts/zksync/facets/Getters.sol index 0c6437e69..6ff706a76 100644 --- a/ethereum/contracts/zksync/facets/Getters.sol +++ b/ethereum/contracts/zksync/facets/Getters.sol @@ -13,7 +13,7 @@ import {ILegacyGetters} from "../interfaces/ILegacyGetters.sol"; // While formally the following import is not used, it is needed to inherit documentation from it import {IBase} from "../interfaces/IBase.sol"; -/// @title Getters Contract implements functions for getting contract state from outside the batchchain. +/// @title Getters Contract implements functions for getting contract state from outside the blockchain. /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev contract GettersFacet is Base, IGetters, ILegacyGetters { diff --git a/ethereum/contracts/zksync/interfaces/ILegacyGetters.sol b/ethereum/contracts/zksync/interfaces/ILegacyGetters.sol index 94838584e..db47a1148 100644 --- a/ethereum/contracts/zksync/interfaces/ILegacyGetters.sol +++ b/ethereum/contracts/zksync/interfaces/ILegacyGetters.sol @@ -6,7 +6,7 @@ import {IBase} from "./IBase.sol"; /// @author Matter Labs /// @dev This interface contains getters for the zkSync contract that should not be used, -/// but still are keot for backward compatibility. +/// but still are kept for backward compatibility. interface ILegacyGetters is IBase { /// @return The total number of batches that were committed /// @dev It is a *deprecated* method, please use `getTotalBatchesCommitted` instead diff --git a/ethereum/contracts/zksync/libraries/TransactionValidator.sol b/ethereum/contracts/zksync/libraries/TransactionValidator.sol index 7d125913a..32488a6b9 100644 --- a/ethereum/contracts/zksync/libraries/TransactionValidator.sol +++ b/ethereum/contracts/zksync/libraries/TransactionValidator.sol @@ -84,7 +84,7 @@ library TransactionValidator { // and the size of each new encoding word). costForComputation += Math.ceilDiv(_encodingLength * L1_TX_DELTA_544_ENCODING_BYTES, 544); - // Taking into the account the additional costs of providing new factory dependenies + // Taking into the account the additional costs of providing new factory dependencies costForComputation += _numberOfFactoryDependencies * L1_TX_DELTA_FACTORY_DEPS_L2_GAS; // There is a minimal amount of computational L2 gas that the transaction should cover @@ -93,10 +93,10 @@ library TransactionValidator { uint256 costForPubdata = 0; { - // Adding the intrinsic cost for the transaction, i.e. auxilary prices which cannot be easily accounted for + // Adding the intrinsic cost for the transaction, i.e. auxiliary prices which cannot be easily accounted for costForPubdata = L1_TX_INTRINSIC_PUBDATA * _l2GasPricePerPubdata; - // Taking into the account the additional costs of providing new factory dependenies + // Taking into the account the additional costs of providing new factory dependencies costForPubdata += _numberOfFactoryDependencies * L1_TX_DELTA_FACTORY_DEPS_PUBDATA * _l2GasPricePerPubdata; } From 8551b61f22580755c675e6002017dae74ac0dcda Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Tue, 9 Jan 2024 09:51:24 +0100 Subject: [PATCH 07/10] Use consistent error codes (#121) --- ethereum/contracts/zksync/facets/Base.sol | 2 +- ethereum/test/unit_tests/governance_test.spec.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ethereum/contracts/zksync/facets/Base.sol b/ethereum/contracts/zksync/facets/Base.sol index 51bda761b..e2b5fd45e 100644 --- a/ethereum/contracts/zksync/facets/Base.sol +++ b/ethereum/contracts/zksync/facets/Base.sol @@ -19,7 +19,7 @@ contract Base is ReentrancyGuard { /// @notice Checks that the message sender is an active governor or admin modifier onlyGovernorOrAdmin() { - require(msg.sender == s.governor || msg.sender == s.admin, "Only by governor or admin"); + require(msg.sender == s.governor || msg.sender == s.admin, "1k"); _; } diff --git a/ethereum/test/unit_tests/governance_test.spec.ts b/ethereum/test/unit_tests/governance_test.spec.ts index 88837a0e6..292a7944e 100644 --- a/ethereum/test/unit_tests/governance_test.spec.ts +++ b/ethereum/test/unit_tests/governance_test.spec.ts @@ -38,7 +38,7 @@ describe("Admin facet tests", function () { const revertReason = await getCallRevertReason( adminFacetTest.connect(randomSigner).setValidator(validatorAddress, true) ); - expect(revertReason).equal("Only by governor or admin"); + expect(revertReason).equal("1k"); }); it("governor successfully set porter availability", async () => { From f23f867c9f80b5544dc64103286a33adaf7e2b18 Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Tue, 9 Jan 2024 09:52:29 +0100 Subject: [PATCH 08/10] Include security contact (#122) --- ethereum/contracts/zksync/interfaces/IAdmin.sol | 3 +++ ethereum/contracts/zksync/interfaces/IBase.sol | 3 +++ ethereum/contracts/zksync/interfaces/IExecutor.sol | 3 +++ ethereum/contracts/zksync/interfaces/IGetters.sol | 3 +++ ethereum/contracts/zksync/interfaces/ILegacyGetters.sol | 1 + ethereum/contracts/zksync/interfaces/IMailbox.sol | 3 +++ ethereum/contracts/zksync/interfaces/IVerifier.sol | 3 +++ ethereum/contracts/zksync/interfaces/IZkSync.sol | 8 +++++++- 8 files changed, 26 insertions(+), 1 deletion(-) diff --git a/ethereum/contracts/zksync/interfaces/IAdmin.sol b/ethereum/contracts/zksync/interfaces/IAdmin.sol index e4e6fd709..4ef552f9b 100644 --- a/ethereum/contracts/zksync/interfaces/IAdmin.sol +++ b/ethereum/contracts/zksync/interfaces/IAdmin.sol @@ -5,6 +5,9 @@ pragma solidity 0.8.20; import {IBase} from "./IBase.sol"; import {Diamond} from "../libraries/Diamond.sol"; +/// @title The interface of the Admin Contract that controls access rights for contract management. +/// @author Matter Labs +/// @custom:security-contact security@matterlabs.dev interface IAdmin is IBase { /// @notice Starts the transfer of governor rights. Only the current governor can propose a new pending one. /// @notice New governor can accept governor rights by calling `acceptGovernor` function. diff --git a/ethereum/contracts/zksync/interfaces/IBase.sol b/ethereum/contracts/zksync/interfaces/IBase.sol index 3076ae2d1..bbe7af652 100644 --- a/ethereum/contracts/zksync/interfaces/IBase.sol +++ b/ethereum/contracts/zksync/interfaces/IBase.sol @@ -1,6 +1,9 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity 0.8.20; +/// @title The interface of the zkSync contract, responsible for the main zkSync logic. +/// @author Matter Labs +/// @custom:security-contact security@matterlabs.dev interface IBase { /// @return Returns facet name. function getName() external view returns (string memory); diff --git a/ethereum/contracts/zksync/interfaces/IExecutor.sol b/ethereum/contracts/zksync/interfaces/IExecutor.sol index ec0e8507b..379907b0f 100644 --- a/ethereum/contracts/zksync/interfaces/IExecutor.sol +++ b/ethereum/contracts/zksync/interfaces/IExecutor.sol @@ -25,6 +25,9 @@ uint256 constant L2_LOG_KEY_OFFSET = 24; /// @dev Offset used to pull Value From Log. Equal to 4 (bytes for isService) + 20 (bytes for address) + 32 (bytes for key) uint256 constant L2_LOG_VALUE_OFFSET = 56; +/// @title The interface of the zkSync Executor contract capable of processing events emitted in the zkSync protocol. +/// @author Matter Labs +/// @custom:security-contact security@matterlabs.dev interface IExecutor is IBase { /// @notice Rollup batch stored data /// @param batchNumber Rollup batch number diff --git a/ethereum/contracts/zksync/interfaces/IGetters.sol b/ethereum/contracts/zksync/interfaces/IGetters.sol index 2efd3aafe..3bfa21d7d 100644 --- a/ethereum/contracts/zksync/interfaces/IGetters.sol +++ b/ethereum/contracts/zksync/interfaces/IGetters.sol @@ -6,6 +6,9 @@ import {PriorityOperation} from "../libraries/PriorityQueue.sol"; import {VerifierParams, UpgradeState} from "../Storage.sol"; import "./IBase.sol"; +/// @title The interface of the Getters Contract that implements functions for getting contract state from outside the blockchain. +/// @author Matter Labs +/// @custom:security-contact security@matterlabs.dev interface IGetters is IBase { /*////////////////////////////////////////////////////////////// CUSTOM GETTERS diff --git a/ethereum/contracts/zksync/interfaces/ILegacyGetters.sol b/ethereum/contracts/zksync/interfaces/ILegacyGetters.sol index db47a1148..2ecc9dde8 100644 --- a/ethereum/contracts/zksync/interfaces/ILegacyGetters.sol +++ b/ethereum/contracts/zksync/interfaces/ILegacyGetters.sol @@ -7,6 +7,7 @@ import {IBase} from "./IBase.sol"; /// @author Matter Labs /// @dev This interface contains getters for the zkSync contract that should not be used, /// but still are kept for backward compatibility. +/// @custom:security-contact security@matterlabs.dev interface ILegacyGetters is IBase { /// @return The total number of batches that were committed /// @dev It is a *deprecated* method, please use `getTotalBatchesCommitted` instead diff --git a/ethereum/contracts/zksync/interfaces/IMailbox.sol b/ethereum/contracts/zksync/interfaces/IMailbox.sol index 77d816799..6fbe09c86 100644 --- a/ethereum/contracts/zksync/interfaces/IMailbox.sol +++ b/ethereum/contracts/zksync/interfaces/IMailbox.sol @@ -13,6 +13,9 @@ enum TxStatus { Success } +/// @title The interface of the zkSync Mailbox contract that provides interfaces for L1 <-> L2 interaction. +/// @author Matter Labs +/// @custom:security-contact security@matterlabs.dev interface IMailbox is IBase { /// @dev Structure that includes all fields of the L2 transaction /// @dev The hash of this structure is the "canonical L2 transaction hash" and can be used as a unique identifier of a tx diff --git a/ethereum/contracts/zksync/interfaces/IVerifier.sol b/ethereum/contracts/zksync/interfaces/IVerifier.sol index 7638a577c..1fbac964c 100644 --- a/ethereum/contracts/zksync/interfaces/IVerifier.sol +++ b/ethereum/contracts/zksync/interfaces/IVerifier.sol @@ -2,6 +2,9 @@ pragma solidity 0.8.20; +/// @title The interface of the Verifier contract, responsible for the zero knowledge proof verification. +/// @author Matter Labs +/// @custom:security-contact security@matterlabs.dev interface IVerifier { /// @dev Verifies a zk-SNARK proof. /// @return A boolean value indicating whether the zk-SNARK proof is valid. diff --git a/ethereum/contracts/zksync/interfaces/IZkSync.sol b/ethereum/contracts/zksync/interfaces/IZkSync.sol index 9f58e16b6..12f6eb6ca 100644 --- a/ethereum/contracts/zksync/interfaces/IZkSync.sol +++ b/ethereum/contracts/zksync/interfaces/IZkSync.sol @@ -7,4 +7,10 @@ import {IAdmin} from "./IAdmin.sol"; import {IExecutor} from "./IExecutor.sol"; import {IGetters} from "./IGetters.sol"; -interface IZkSync is IMailbox, IAdmin, IExecutor, IGetters {} +/// @title The interface of the zkSync contract, responsible for the main zkSync logic. +/// @author Matter Labs +/// @dev This interface combines the interfaces of all the facets of the zkSync contract. +/// @custom:security-contact security@matterlabs +interface IZkSync is IMailbox, IAdmin, IExecutor, IGetters { + +} From f258f2b9e1172ba91422be34dcef224f710c8a42 Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Tue, 9 Jan 2024 09:52:44 +0100 Subject: [PATCH 09/10] Remove unused imports (#123) --- ethereum/contracts/zksync/Config.sol | 14 -------------- ethereum/contracts/zksync/facets/Executor.sol | 4 ++-- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/ethereum/contracts/zksync/Config.sol b/ethereum/contracts/zksync/Config.sol index c33bbe8bc..87184d9b5 100644 --- a/ethereum/contracts/zksync/Config.sol +++ b/ethereum/contracts/zksync/Config.sol @@ -18,20 +18,6 @@ uint256 constant MAX_L2_TO_L1_LOGS_COMMITMENT_BYTES = 4 + L2_TO_L1_LOG_SERIALIZE /// @dev Actually equal to the `keccak256(new bytes(L2_TO_L1_LOG_SERIALIZE_SIZE))` bytes32 constant L2_L1_LOGS_TREE_DEFAULT_LEAF_HASH = 0x72abee45b59e344af8a6e520241c4744aff26ed411f4c4b00f8af09adada43ba; -/// @dev Number of bytes in a one initial storage change -/// @dev Equal to the bytes size of the tuple - (bytes32 key, bytes32 value) -uint256 constant INITIAL_STORAGE_CHANGE_SERIALIZE_SIZE = 64; - -/// @dev The maximum length of the bytes array with initial storage changes -uint256 constant MAX_INITIAL_STORAGE_CHANGES_COMMITMENT_BYTES = 4 + INITIAL_STORAGE_CHANGE_SERIALIZE_SIZE * 4765; - -/// @dev Number of bytes in a one repeated storage change -/// @dev Equal to the bytes size of the tuple - (bytes8 key, bytes32 value) -uint256 constant REPEATED_STORAGE_CHANGE_SERIALIZE_SIZE = 40; - -/// @dev The maximum length of the bytes array with repeated storage changes -uint256 constant MAX_REPEATED_STORAGE_CHANGES_COMMITMENT_BYTES = 4 + REPEATED_STORAGE_CHANGE_SERIALIZE_SIZE * 7564; - // TODO: change constant to the real root hash of empty Merkle tree (SMA-184) bytes32 constant DEFAULT_L2_LOGS_TREE_ROOT_HASH = bytes32(0); diff --git a/ethereum/contracts/zksync/facets/Executor.sol b/ethereum/contracts/zksync/facets/Executor.sol index fc340eb32..c00994d32 100644 --- a/ethereum/contracts/zksync/facets/Executor.sol +++ b/ethereum/contracts/zksync/facets/Executor.sol @@ -3,13 +3,13 @@ pragma solidity 0.8.20; import {Base} from "./Base.sol"; -import {COMMIT_TIMESTAMP_NOT_OLDER, COMMIT_TIMESTAMP_APPROXIMATION_DELTA, EMPTY_STRING_KECCAK, L2_TO_L1_LOG_SERIALIZE_SIZE, MAX_INITIAL_STORAGE_CHANGES_COMMITMENT_BYTES, MAX_REPEATED_STORAGE_CHANGES_COMMITMENT_BYTES, MAX_L2_TO_L1_LOGS_COMMITMENT_BYTES, PACKED_L2_BLOCK_TIMESTAMP_MASK, PUBLIC_INPUT_SHIFT} from "../Config.sol"; +import {COMMIT_TIMESTAMP_NOT_OLDER, COMMIT_TIMESTAMP_APPROXIMATION_DELTA, EMPTY_STRING_KECCAK, L2_TO_L1_LOG_SERIALIZE_SIZE, MAX_L2_TO_L1_LOGS_COMMITMENT_BYTES, PACKED_L2_BLOCK_TIMESTAMP_MASK, PUBLIC_INPUT_SHIFT} from "../Config.sol"; import {IExecutor, L2_LOG_ADDRESS_OFFSET, L2_LOG_KEY_OFFSET, L2_LOG_VALUE_OFFSET, SystemLogKey} from "../interfaces/IExecutor.sol"; import {PriorityQueue, PriorityOperation} from "../libraries/PriorityQueue.sol"; import {UncheckedMath} from "../../common/libraries/UncheckedMath.sol"; import {UnsafeBytes} from "../../common/libraries/UnsafeBytes.sol"; import {VerifierParams} from "../Storage.sol"; -import {L2_BOOTLOADER_ADDRESS, L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR, L2_SYSTEM_CONTEXT_SYSTEM_CONTRACT_ADDR, L2_KNOWN_CODE_STORAGE_SYSTEM_CONTRACT_ADDR} from "../../common/L2ContractAddresses.sol"; +import {L2_BOOTLOADER_ADDRESS, L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR, L2_SYSTEM_CONTEXT_SYSTEM_CONTRACT_ADDR} from "../../common/L2ContractAddresses.sol"; // While formally the following import is not used, it is needed to inherit documentation from it import {IBase} from "../interfaces/IBase.sol"; From a2cd7cedaa7a2e5871ed805c711f0f95e235095c Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Tue, 9 Jan 2024 09:53:14 +0100 Subject: [PATCH 10/10] Remove unneeded override (#124) --- ethereum/contracts/zksync/facets/Executor.sol | 2 +- ethereum/contracts/zksync/facets/Mailbox.sol | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ethereum/contracts/zksync/facets/Executor.sol b/ethereum/contracts/zksync/facets/Executor.sol index c00994d32..b34183f60 100644 --- a/ethereum/contracts/zksync/facets/Executor.sol +++ b/ethereum/contracts/zksync/facets/Executor.sol @@ -180,7 +180,7 @@ contract ExecutorFacet is Base, IExecutor { function commitBatches( StoredBatchInfo memory _lastCommittedBatchData, CommitBatchInfo[] calldata _newBatchesData - ) external override nonReentrant onlyValidator { + ) external nonReentrant onlyValidator { // Check that we commit batches after last committed batch require(s.storedBatchHashes[s.totalBatchesCommitted] == _hashStoredBatchInfo(_lastCommittedBatchData), "i"); // incorrect previous batch data require(_newBatchesData.length > 0, "No batches to commit"); diff --git a/ethereum/contracts/zksync/facets/Mailbox.sol b/ethereum/contracts/zksync/facets/Mailbox.sol index bd2d040e2..4e0e6cb4a 100644 --- a/ethereum/contracts/zksync/facets/Mailbox.sol +++ b/ethereum/contracts/zksync/facets/Mailbox.sol @@ -58,7 +58,7 @@ contract MailboxFacet is Base, IMailbox { uint16 _l2TxNumberInBatch, bytes32[] calldata _merkleProof, TxStatus _status - ) public view override returns (bool) { + ) public view returns (bool) { // Bootloader sends an L2 -> L1 log only after processing the L1 -> L2 transaction. // Thus, we can verify that the L1 -> L2 transaction was included in the L2 batch with specified status. // @@ -158,7 +158,7 @@ contract MailboxFacet is Base, IMailbox { uint16 _l2TxNumberInBatch, bytes calldata _message, bytes32[] calldata _merkleProof - ) external override nonReentrant { + ) external nonReentrant { require(!s.isEthWithdrawalFinalized[_l2BatchNumber][_l2MessageIndex], "jj"); L2Message memory l2ToL1Message = L2Message({