Merge pull request #160 from matter-labs/sb-port-1-4-1-fixes

Port 1 4 1 fixes to the short term fee model

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";

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";
 

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";

ethereum/contracts/zksync/Config.sol

@@ -18,14 +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 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;
-
 // TODO: change constant to the real root hash of empty Merkle tree (SMA-184)
 bytes32 constant DEFAULT_L2_LOGS_TREE_ROOT_HASH = bytes32(0);
 

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

ethereum/contracts/zksync/facets/Admin.sol

@@ -2,20 +2,22 @@
 
 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";
 
 /// @title Admin Contract controls access rights for contract management.
 /// @author Matter Labs
 /// @custom:security-contact [email protected]
 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");
 
@@ -100,9 +96,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);
@@ -112,8 +106,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();
 
@@ -123,8 +116,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();
 

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
@@ -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");
         _;
     }
 

ethereum/contracts/zksync/facets/Executor.sol

@@ -9,7 +9,10 @@ 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";
 
 /// @title zkSync Executor contract capable of processing events emitted in the zkSync protocol.
 /// @author Matter Labs
@@ -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,14 +176,11 @@ 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
-    ) 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");
@@ -232,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.
@@ -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
@@ -415,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,