More fixes

Atandanny · Dec 27, 2021 · cdae0a6 · cdae0a6
1 parent 3ac4b1f
commit cdae0a6
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 10 deletions.
diff --git a/contracts/contracts/AdditionalZkSync.sol b/contracts/contracts/AdditionalZkSync.sol
@@ -121,7 +121,7 @@ contract AdditionalZkSync is Storage, Config, Events, ReentrancyGuard {
         for (uint256 id = 0; id < SECURITY_COUNCIL_MEMBERS_NUMBER; ++id) {
             if (SECURITY_COUNCIL_MEMBERS[id] == addr && !securityCouncilApproves[id]) {
                 securityCouncilApproves[id] = true;
-                numberOfApprovalsFromSecurityCouncil++;
+                numberOfApprovalsFromSecurityCouncil += 1;
 
                 if (numberOfApprovalsFromSecurityCouncil == SECURITY_COUNCIL_THRESHOLD) {
                     if (approvedUpgradeNoticePeriod > 0) {
@@ -135,15 +135,21 @@ contract AdditionalZkSync is Storage, Config, Events, ReentrancyGuard {
         }
     }
 
+    /// @notice approve to decrease upgrade notice period time to zero
+    /// NOTE: сan only be called after the start of the upgrade
     function cutUpgradeNoticePeriod() external {
         require(upgradeStartTimestamp != 0);
 
         approvedCutUpgradeNoticePeriod(msg.sender);
     }
 
+    /// @notice approve to decrease upgrade notice period time to zero by signatures
+    /// NOTE: Can accept many signatures at a time, thus it is possible
+    /// to completely cut the upgrade notice period in one transaction
     function cutUpgradeNoticePeriodBySignature(bytes[] calldata signatures) external {
         require(upgradeStartTimestamp != 0);
 
+        // Get the addresses of contracts that are being prepared for the upgrade.
         address gatekeeper = $(UPGRADE_GATEKEEPER_ADDRESS);
         (, bytes memory newTarget0) = gatekeeper.staticcall(abi.encodeWithSignature("nextTargets(uint256)", 0));
         (, bytes memory newTarget1) = gatekeeper.staticcall(abi.encodeWithSignature("nextTargets(uint256)", 1));
@@ -154,6 +160,7 @@ contract AdditionalZkSync is Storage, Config, Events, ReentrancyGuard {
         address newTargetAddress2 = abi.decode(newTarget2, (address));
 
         bytes32 targetsHash = keccak256(abi.encodePacked(newTargetAddress0, newTargetAddress1, newTargetAddress2));
+        // The Message includes a hash of the addresses of the contracts to which the upgrade will take place to prevent reuse signature.
         bytes32 messageHash = keccak256(
             abi.encodePacked(
                 "\x19Ethereum Signed Message:\n110",

diff --git a/contracts/contracts/ZkSync.sol b/contracts/contracts/ZkSync.sol
@@ -162,12 +162,12 @@ contract ZkSync is UpgradeableMaster, Storage, Config, Events, ReentrancyGuard {
     }
 
     function cutUpgradeNoticePeriod() external {
-        /// All functions delegated to additional contract should NOT be nonReentrant
+        // All functions delegated to additional contract should NOT be nonReentrant
         delegateAdditional();
     }
 
     function cutUpgradeNoticePeriodBySignature(bytes[] calldata signatures) external {
-        /// All functions delegated to additional contract should NOT be nonReentrant
+        // All functions delegated to additional contract should NOT be nonReentrant
         delegateAdditional();
     }
 
@@ -200,16 +200,20 @@ contract ZkSync is UpgradeableMaster, Storage, Config, Events, ReentrancyGuard {
     /// @dev Canceling may take several separate transactions to be completed
     /// @param _n number of requests to process
     function cancelOutstandingDepositsForExodusMode(uint64 _n, bytes[] memory _depositsPubdata) external {
-        /// All functions delegated to additional contract should NOT be nonReentrant
+        // All functions delegated to additional contract should NOT be nonReentrant
         delegateAdditional();
     }
 
     /// @notice Deposit ETH to Layer 2 - transfer ether from user into contract, validate it, register deposit
     /// @param _zkSyncAddress The receiver Layer 2 address
     function depositETH(address _zkSyncAddress) external payable {
         require(_zkSyncAddress != SPECIAL_ACCOUNT_ADDRESS, "P");
-        require(msg.value > 0, "M");
+        require(msg.value > 0, "M"); // Zero-value deposits are forbidden by zkSync rollup logic
         requireActive();
+        // It is safe to convert `msg.value` to `ui128` without additional checks:
+        // - 1 Ether is 10^18 Wei
+        // - Total supply of Ether is 118,019,446 (as of December 27, 2021)
+        // - 2^128 > 10^38 > (Total supply of Ether) * 10^18
         registerDeposit(0, uint128(msg.value), _zkSyncAddress);
     }
 
@@ -282,7 +286,7 @@ contract ZkSync is UpgradeableMaster, Storage, Config, Events, ReentrancyGuard {
 
         // `amount = min(balance, _amount)` and `transferERC20` include check that `amount <= balance` so it's safe.
         pendingBalances[packedBalanceKey].balanceToWithdraw = balance - amount;
-        emit Withdrawal(_token, amount);
+        emit Withdrawal(tokenId, amount);
     }
 
     /// @notice  Withdraws NFT from zkSync contract to the owner
@@ -367,7 +371,7 @@ contract ZkSync is UpgradeableMaster, Storage, Config, Events, ReentrancyGuard {
 
     /// @dev Process one block commit using previous block StoredBlockInfo,
     /// @dev returns new block StoredBlockInfo
-    /// @dev NOTE: does not change storage! (only emit events)
+    /// @dev NOTE: does not change storage (only emit events)!
     function commitOneBlock(StoredBlockInfo memory _previousBlock, CommitBlockInfo memory _newBlock)
         internal
         view
@@ -593,7 +597,7 @@ contract ZkSync is UpgradeableMaster, Storage, Config, Events, ReentrancyGuard {
 
     /// @notice Reverts unverified blocks
     function revertBlocks(StoredBlockInfo[] memory _blocksToRevert) external {
-        /// All functions delegated to additional contract should NOT be nonReentrant
+        // All functions delegated to additional contract should NOT be nonReentrant
         delegateAdditional();
     }
 
@@ -636,7 +640,7 @@ contract ZkSync is UpgradeableMaster, Storage, Config, Events, ReentrancyGuard {
         bytes32 _nftContentHash,
         uint256[] memory _proof
     ) external {
-        /// All functions delegated to additional should NOT be nonReentrant
+        // All functions delegated to additional should NOT be nonReentrant
         delegateAdditional();
     }
 
@@ -648,7 +652,7 @@ contract ZkSync is UpgradeableMaster, Storage, Config, Events, ReentrancyGuard {
     /// @param _pubkeyHash New pubkey hash
     /// @param _nonce Nonce of the change pubkey L2 transaction
     function setAuthPubkeyHash(bytes calldata _pubkeyHash, uint32 _nonce) external {
-        /// All functions delegated to additional contract should NOT be nonReentrant
+        // All functions delegated to additional contract should NOT be nonReentrant
         delegateAdditional();
     }