Merge pull request #2118 from matter-labs/vb-smart-contracts-v8

smart contracts v8

contracts/contracts/AdditionalZkSync.sol

@@ -47,7 +47,7 @@ contract AdditionalZkSync is Storage, Config, Events, ReentrancyGuard {
         address _nftCreatorAddress,
         uint32 _nftSerialId,
         bytes32 _nftContentHash,
-        uint256[] memory _proof
+        uint256[] calldata _proof
     ) external {
         require(_accountId <= MAX_ACCOUNT_ID, "e");
         require(_accountId != SPECIAL_ACCOUNT_ID, "v");
@@ -74,7 +74,7 @@ contract AdditionalZkSync is Storage, Config, Events, ReentrancyGuard {
         if (_tokenId <= MAX_FUNGIBLE_TOKEN_ID) {
             bytes22 packedBalanceKey = packAddressAndTokenId(_owner, uint16(_tokenId));
             increaseBalanceToWithdraw(packedBalanceKey, _amount);
-            emit WithdrawalPending(uint16(_tokenId), _amount);
+            emit WithdrawalPending(uint16(_tokenId), _owner, _amount);
         } else {
             require(_amount != 0, "Z"); // Unsupported nft amount
             Operations.WithdrawNFT memory withdrawNftOp = Operations.WithdrawNFT(
@@ -91,12 +91,12 @@ contract AdditionalZkSync is Storage, Config, Events, ReentrancyGuard {
         performedExodus[_accountId][_tokenId] = true;
     }
 
-    function cancelOutstandingDepositsForExodusMode(uint64 _n, bytes[] memory _depositsPubdata) external {
+    function cancelOutstandingDepositsForExodusMode(uint64 _n, bytes[] calldata _depositsPubdata) external {
         require(exodusMode, "8"); // exodus mode not active
         uint64 toProcess = Utils.minU64(totalOpenPriorityRequests, _n);
         require(toProcess > 0, "9"); // no deposits to process
         uint64 currentDepositIdx = 0;
-        for (uint64 id = firstPriorityRequestId; id < firstPriorityRequestId + toProcess; id++) {
+        for (uint64 id = firstPriorityRequestId; id < firstPriorityRequestId + toProcess; ++id) {
             if (priorityRequests[id].opType == Operations.OpType.Deposit) {
                 bytes memory depositPubdata = _depositsPubdata[currentDepositIdx];
                 require(Utils.hashBytesToBytes20(depositPubdata) == priorityRequests[id].hashedPubData, "a");
@@ -114,16 +114,20 @@ contract AdditionalZkSync is Storage, Config, Events, ReentrancyGuard {
 
     uint256 internal constant SECURITY_COUNCIL_THRESHOLD = $$(SECURITY_COUNCIL_THRESHOLD);
 
-    function approvedCutUpgradeNoticePeriod(address addr) internal {
+    /// @notice processing new approval of decrease upgrade notice period time to zero
+    /// @param addr address of the account that approved the reduction of the upgrade notice period to zero
+    /// NOTE: does NOT revert if the address is not a security council member or number of approvals is already sufficient
+    function approveCutUpgradeNoticePeriod(address addr) internal {
         address payable[SECURITY_COUNCIL_MEMBERS_NUMBER] memory SECURITY_COUNCIL_MEMBERS = [
             $(SECURITY_COUNCIL_MEMBERS)
         ];
         for (uint256 id = 0; id < SECURITY_COUNCIL_MEMBERS_NUMBER; ++id) {
             if (SECURITY_COUNCIL_MEMBERS[id] == addr && !securityCouncilApproves[id]) {
                 securityCouncilApproves[id] = true;
-                numberOfApprovalsFromSecurityCouncil++;
+                numberOfApprovalsFromSecurityCouncil += 1;
+                emit ApproveCutUpgradeNoticePeriod(addr);
 
-                if (numberOfApprovalsFromSecurityCouncil == SECURITY_COUNCIL_THRESHOLD) {
+                if (numberOfApprovalsFromSecurityCouncil >= SECURITY_COUNCIL_THRESHOLD) {
                     if (approvedUpgradeNoticePeriod > 0) {
                         approvedUpgradeNoticePeriod = 0;
                         emit NoticePeriodChange(approvedUpgradeNoticePeriod);
@@ -135,23 +139,23 @@ contract AdditionalZkSync is Storage, Config, Events, ReentrancyGuard {
         }
     }
 
-    function cutUpgradeNoticePeriod() external {
-        requireActive();
-        require(upgradeStartTimestamp != 0);
+    /// @notice approve to decrease upgrade notice period time to zero
+    /// NOTE: сan only be called after the start of the upgrade
+    function cutUpgradeNoticePeriod(bytes32 targetsHash) external {
+        require(upgradeStartTimestamp != 0, "p1");
+        require(getUpgradeTargetsHash() == targetsHash, "p3"); // given targets are not in the active upgrade
 
-        approvedCutUpgradeNoticePeriod(msg.sender);
+        approveCutUpgradeNoticePeriod(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 {
-        requireActive();
-        require(upgradeStartTimestamp != 0);
+        require(upgradeStartTimestamp != 0, "p2");
 
-        address gatekeeper = 0x38A43F4330f24fe920F943409709fc9A6084C939;
-        (, bytes memory newTarget0) = gatekeeper.call(abi.encodeWithSignature("nextTargets(uint256)", 0));
-        (, bytes memory newTarget1) = gatekeeper.call(abi.encodeWithSignature("nextTargets(uint256)", 1));
-        (, bytes memory newTarget2) = gatekeeper.call(abi.encodeWithSignature("nextTargets(uint256)", 2));
-
-        bytes32 targetsHash = keccak256(abi.encodePacked(newTarget0, newTarget1, newTarget2));
+        bytes32 targetsHash = getUpgradeTargetsHash();
+        // 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",
@@ -162,10 +166,27 @@ contract AdditionalZkSync is Storage, Config, Events, ReentrancyGuard {
 
         for (uint256 i = 0; i < signatures.length; ++i) {
             address recoveredAddress = Utils.recoverAddressFromEthSignature(signatures[i], messageHash);
-            approvedCutUpgradeNoticePeriod(recoveredAddress);
+            require(recoveredAddress != address(0x00), "p4"); // invalid signature
+            approveCutUpgradeNoticePeriod(recoveredAddress);
         }
     }
 
+    /// @return hash of the concatenation of targets for which there is an upgrade
+    /// NOTE: revert if upgrade is not active at this moment
+    function getUpgradeTargetsHash() internal returns (bytes32) {
+        // 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));
+        (, bytes memory newTarget2) = gatekeeper.staticcall(abi.encodeWithSignature("nextTargets(uint256)", 2));
+
+        address newTargetAddress0 = abi.decode(newTarget0, (address));
+        address newTargetAddress1 = abi.decode(newTarget1, (address));
+        address newTargetAddress2 = abi.decode(newTarget2, (address));
+
+        return keccak256(abi.encodePacked(newTargetAddress0, newTargetAddress1, newTargetAddress2));
+    }
+
     /// @notice Set data for changing pubkey hash using onchain authorization.
     ///         Transaction author (msg.sender) should be L2 account address
     /// @notice New pubkey hash can be reset, to do that user should send two transactions:
@@ -192,7 +213,7 @@ contract AdditionalZkSync is Storage, Config, Events, ReentrancyGuard {
     }
 
     /// @notice Reverts unverified blocks
-    function revertBlocks(StoredBlockInfo[] memory _blocksToRevert) external {
+    function revertBlocks(StoredBlockInfo[] calldata _blocksToRevert) external {
         requireActive();
 
         governance.requireActiveValidator(msg.sender);

contracts/contracts/Create2Factory.sol

@@ -0,0 +1,40 @@
+pragma solidity ^0.7.0;
+
+contract Create2Factory {
+    /**
+     * @dev Deploys a contract using `CREATE2`. The address where the contract
+     * will be deployed can be known in advance via {computeAddress}. Note that
+     * a contract cannot be deployed twice using the same salt.
+     */
+    function deploy(bytes32 salt, bytes memory bytecode) public returns (address) {
+        address addr;
+        // solhint-disable-next-line no-inline-assembly
+        assembly {
+            addr := create2(0, add(bytecode, 0x20), mload(bytecode), salt)
+        }
+        require(addr != address(0), "Create2: Failed on deploy");
+        return addr;
+    }
+
+    /**
+     * @dev Returns the address where a contract will be stored if deployed via {deploy}. Any change in the `bytecode`
+     * or `salt` will result in a new destination address.
+     */
+    function computeAddress(bytes32 salt, bytes memory bytecode) external view returns (address) {
+        return computeAddress(salt, bytecode, address(this));
+    }
+
+    /**
+     * @dev Returns the address where a contract will be stored if deployed via {deploy} from a contract located at
+     * `deployer`. If `deployer` is this contract's address, returns the same value as {computeAddress}.
+     */
+    function computeAddress(
+        bytes32 salt,
+        bytes memory bytecodeHash,
+        address deployer
+    ) public pure returns (address) {
+        bytes32 bytecodeHashHash = keccak256(bytecodeHash);
+        bytes32 _data = keccak256(abi.encodePacked(bytes1(0xff), deployer, salt, bytecodeHashHash));
+        return address(bytes20(_data << 96));
+    }
+}

contracts/contracts/Events.sol

@@ -18,7 +18,7 @@ interface Events {
     event Withdrawal(uint16 indexed tokenId, uint128 amount);
 
     /// @notice Event emitted when user funds are withdrawn from the zkSync state but not from contract
-    event WithdrawalPending(uint16 indexed tokenId, uint128 amount);
+    event WithdrawalPending(uint16 indexed tokenId, address recepient, uint128 amount);
 
     /// @notice Event emitted when user NFT is withdrawn from the zkSync state and contract
     event WithdrawalNFT(uint32 indexed tokenId);
@@ -65,6 +65,9 @@ interface Events {
         uint128 amount
     );
 
+    /// @notice Approve cut of upgrade notice period by addr
+    event ApproveCutUpgradeNoticePeriod(address addr);
+
     /// @notice Notice period changed
     event NoticePeriodChange(uint256 newNoticePeriod);
 }

contracts/contracts/Governance.sol

@@ -197,8 +197,8 @@ contract Governance is Config {
         emit NFTFactoryRegisteredCreator(_creatorAccountId, _creatorAddress, msg.sender);
     }
 
-    //@notice Set default factory for our contract. This factory will be used to mint an NFT token that has no factory
-    //@param _factory Address of NFT factory
+    /// @notice Set default factory for our contract. This factory will be used to mint an NFT token that has no factory
+    /// @param _factory Address of NFT factory
     function setDefaultNFTFactory(address _factory) external {
         requireGovernor(msg.sender);
         require(address(_factory) != address(0), "mb1"); // Factory should be non zero
@@ -209,11 +209,23 @@ contract Governance is Config {
 
     function getNFTFactory(uint32 _creatorAccountId, address _creatorAddress) external view returns (NFTFactory) {
         NFTFactory _factory = nftFactories[_creatorAccountId][_creatorAddress];
-        if (address(_factory) == address(0)) {
+        // even if the factory is undefined or has been destroyed, the user can mint NFT
+        if (address(_factory) == address(0) || !isContract(address(_factory))) {
             require(address(defaultFactory) != address(0), "fs"); // NFTFactory does not set
             return defaultFactory;
         } else {
             return _factory;
         }
     }
+
+    /// @return whether the address is a contract or not
+    /// NOTE: for smart contracts that called `selfdestruct` will return a negative result
+    function isContract(address _address) internal view returns (bool) {
+        uint256 contractSize;
+        assembly {
+            contractSize := extcodesize(_address)
+        }
+
+        return contractSize != 0;
+    }
 }

contracts/contracts/IERC20.sol

@@ -2,8 +2,7 @@
 pragma solidity ^0.7.0;
 
 /**
- * @dev Interface of the ERC20 standard as defined in the EIP. Does not include
- * the optional functions; to access them see {ERC20Detailed}.
+ * @dev Interface of the ERC20 standard as defined in the EIP.
  */
 interface IERC20 {
     /**
@@ -19,11 +18,9 @@ interface IERC20 {
     /**
      * @dev Moves `amount` tokens from the caller's account to `recipient`.
      *
-     * Returns a boolean value indicating whether the operation succeeded.
-     *
      * Emits a {Transfer} event.
      */
-    function transfer(address recipient, uint256 amount) external returns (bool);
+    function transfer(address recipient, uint256 amount) external;
 
     /**
      * @dev Returns the remaining number of tokens that `spender` will be
@@ -55,15 +52,13 @@ interface IERC20 {
      * allowance mechanism. `amount` is then deducted from the caller's
      * allowance.
      *
-     * Returns a boolean value indicating whether the operation succeeded.
-     *
      * Emits a {Transfer} event.
      */
     function transferFrom(
         address sender,
         address recipient,
         uint256 amount
-    ) external returns (bool);
+    ) external;
 
     /**
      * @dev Emitted when `value` tokens are moved from one account (`from`) to
@@ -79,3 +74,35 @@ interface IERC20 {
      */
     event Approval(address indexed owner, address indexed spender, uint256 value);
 }
+
+/// @dev Interface of the ERC20 standard as defined in the EIP.
+/// 1. Implements only `transfer` and `transferFrom` methods
+/// 2. These methods return a boolean value which indicates success of the transaction
+/// Note: It is assumed that the interface applies to those `ERC20` tokens whose code exactly matches the standard.
+/// Note: Used to perform transfers for tokens that explicitly return a boolean value
+/// (if the token returns any other data or does not return at all, then the function call will reverted)
+interface ITrustedTransfarableERC20 {
+    /**
+     * @dev Moves `amount` tokens from the caller's account to `recipient`.
+     *
+     * Returns a boolean value indicating whether the operation succeeded.
+     *
+     * Emits a {Transfer} event.
+     */
+    function transfer(address recipient, uint256 amount) external returns (bool);
+
+    /**
+     * @dev Moves `amount` tokens from `sender` to `recipient` using the
+     * allowance mechanism. `amount` is then deducted from the caller's
+     * allowance.
+     *
+     * Returns a boolean value indicating whether the operation succeeded.
+     *
+     * Emits a {Transfer} event.
+     */
+    function transferFrom(
+        address sender,
+        address recipient,
+        uint256 amount
+    ) external returns (bool);
+}

contracts/contracts/TokenGovernance.sol

@@ -2,19 +2,20 @@
 
 pragma solidity ^0.7.0;
 
+import "./ReentrancyGuard.sol";
 import "./Governance.sol";
 import "./IERC20.sol";
 import "./Utils.sol";
 
 /// @title Token Governance Contract
 /// @author Matter Labs
 /// @notice Contract is used to allow anyone to add new ERC20 tokens to zkSync given sufficient payment
-contract TokenGovernance {
+contract TokenGovernance is ReentrancyGuard {
     /// @notice Token lister added or removed (see `tokenLister`)
     event TokenListerUpdate(address indexed tokenLister, bool isActive);
 
     /// @notice Listing fee token set
-    event ListingFeeTokenUpdate(IERC20 indexed newListingFeeToken);
+    event ListingFeeTokenUpdate(ITrustedTransfarableERC20 indexed newListingFeeToken);
 
     /// @notice Listing fee set
     event ListingFeeUpdate(uint256 newListingFee);
@@ -29,7 +30,7 @@ contract TokenGovernance {
     Governance public governance;
 
     /// @notice Token used to collect listing fee for addition of new token to zkSync network
-    IERC20 public listingFeeToken;
+    ITrustedTransfarableERC20 public listingFeeToken;
 
     /// @notice Token listing fee
     uint256 public listingFee;
@@ -45,11 +46,13 @@ contract TokenGovernance {
 
     constructor(
         Governance _governance,
-        IERC20 _listingFeeToken,
+        ITrustedTransfarableERC20 _listingFeeToken,
         uint256 _listingFee,
         uint16 _listingCap,
         address _treasury
     ) {
+        initializeReentrancyGuard();
+
         governance = _governance;
         listingFeeToken = _listingFeeToken;
         listingFee = _listingFee;
@@ -65,11 +68,11 @@ contract TokenGovernance {
     /// @notice Adds new ERC20 token to zkSync network.
     /// @notice If caller is not present in the `tokenLister` map payment of `listingFee` in `listingFeeToken` should be made.
     /// @notice NOTE: before calling this function make sure to approve `listingFeeToken` transfer for this contract.
-    function addToken(address _token) external {
+    function addToken(address _token) external nonReentrant {
         require(governance.totalTokens() < listingCap, "can't add more tokens"); // Impossible to add more tokens using this contract
-        if (!tokenLister[msg.sender]) {
+        if (!tokenLister[msg.sender] && listingFee > 0) {
             // Collect fees
-            bool feeTransferOk = Utils.transferFromERC20(listingFeeToken, msg.sender, treasury, listingFee);
+            bool feeTransferOk = listingFeeToken.transferFrom(msg.sender, treasury, listingFee);
             require(feeTransferOk, "fee transfer failed"); // Failed to receive payment for token addition.
         }
         governance.addToken(_token);
@@ -79,7 +82,7 @@ contract TokenGovernance {
 
     /// @notice Set new listing token and fee
     /// @notice Can be called only by zkSync governor
-    function setListingFeeToken(IERC20 _newListingFeeToken, uint256 _newListingFee) external {
+    function setListingFeeToken(ITrustedTransfarableERC20 _newListingFeeToken, uint256 _newListingFee) external {
         governance.requireGovernor(msg.sender);
         listingFeeToken = _newListingFeeToken;
         listingFee = _newListingFee;