Merge pull request #1299 from matter-labs/vd-zks-321-add-events-for-w…

…ithdrawal-status

Event failed onchain withdrawal

contracts/contracts/Events.sol

@@ -14,8 +14,8 @@ interface Events {
     /// @notice Event emitted when a block is verified
     event BlockVerification(uint32 indexed blockNumber);
 
-    /// @notice Event emitted when user funds are withdrawn from the account
-    event OnchainWithdrawal(address indexed owner, uint16 indexed tokenId, uint128 amount);
+    /// @notice Event emitted when user funds are withdrawn from the zkSync contract
+    event OnchainWithdrawal(address indexed owner, uint16 indexed tokenId, uint128 amount, bool success);
 
     /// @notice Event emitted when user funds are withdrawn from the rollup
     event RollupWithdrawal(address indexed owner, uint16 indexed tokenId, uint128 amount);

contracts/contracts/Operations.sol

@@ -75,7 +75,7 @@ library Operations {
     }
 
     /// Serialize deposit pubdata
-    function writeDepositPubdata(Deposit memory op) internal pure returns (bytes memory buf) {
+    function writeDepositPubdataForPriorityQueue(Deposit memory op) internal pure returns (bytes memory buf) {
         buf = abi.encodePacked(
             uint8(OpType.Deposit),
             bytes4(0), // accountId (ignored) (update when ACCOUNT_ID_BYTES is changed)
@@ -87,7 +87,7 @@ library Operations {
 
     /// @notice Write deposit pubdata for priority queue check.
     function checkDepositInPriorityQueue(Deposit memory op, bytes20 hashedPubdata) internal pure returns (bool) {
-        return Utils.hashBytesToBytes20(writeDepositPubdata(op)) == hashedPubdata;
+        return Utils.hashBytesToBytes20(writeDepositPubdataForPriorityQueue(op)) == hashedPubdata;
     }
 
     // FullExit pubdata
@@ -114,7 +114,7 @@ library Operations {
         require(offset == PACKED_FULL_EXIT_PUBDATA_BYTES, "n"); // reading invalid full exit pubdata size
     }
 
-    function writeFullExitPubdata(FullExit memory op) internal pure returns (bytes memory buf) {
+    function writeFullExitPubdataForPriorityQueue(FullExit memory op) internal pure returns (bytes memory buf) {
         buf = abi.encodePacked(
             uint8(OpType.FullExit),
             op.accountId, // accountId
@@ -125,7 +125,7 @@ library Operations {
     }
 
     function checkFullExitInPriorityQueue(FullExit memory op, bytes20 hashedPubdata) internal pure returns (bool) {
-        return Utils.hashBytesToBytes20(writeFullExitPubdata(op)) == hashedPubdata;
+        return Utils.hashBytesToBytes20(writeFullExitPubdataForPriorityQueue(op)) == hashedPubdata;
     }
 
     // PartialExit pubdata

contracts/contracts/SafeMath.sol

@@ -42,7 +42,7 @@ library SafeMath {
      * - Subtraction cannot overflow.
      */
     function sub(uint256 a, uint256 b) internal pure returns (uint256) {
-        return sub(a, b, "SafeMath: subtraction overflow");
+        return sub(a, b, "v");
     }
 
     /**
@@ -61,7 +61,7 @@ library SafeMath {
         uint256 b,
         string memory errorMessage
     ) internal pure returns (uint256) {
-        require(b <= a, "v");
+        require(b <= a, errorMessage);
         uint256 c = a - b;
 
         return c;
@@ -102,7 +102,7 @@ library SafeMath {
      * - The divisor cannot be zero.
      */
     function div(uint256 a, uint256 b) internal pure returns (uint256) {
-        return div(a, b, "SafeMath: division by zero");
+        return div(a, b, "x");
     }
 
     /**
@@ -124,7 +124,7 @@ library SafeMath {
         string memory errorMessage
     ) internal pure returns (uint256) {
         // Solidity only automatically asserts when dividing by 0
-        require(b > 0, "x");
+        require(b > 0, errorMessage);
         uint256 c = a / b;
         // assert(a == b * c + a % b); // There is no case in which this doesn't hold
 
@@ -143,7 +143,7 @@ library SafeMath {
      * - The divisor cannot be zero.
      */
     function mod(uint256 a, uint256 b) internal pure returns (uint256) {
-        return mod(a, b, "SafeMath: modulo by zero");
+        return mod(a, b, "y");
     }
 
     /**
@@ -164,7 +164,7 @@ library SafeMath {
         uint256 b,
         string memory errorMessage
     ) internal pure returns (uint256) {
-        require(b != 0, "y");
+        require(b != 0, errorMessage);
         return a % b;
     }
 }

contracts/contracts/SafeMathUInt128.sol

@@ -42,7 +42,7 @@ library SafeMathUInt128 {
      * - Subtraction cannot overflow.
      */
     function sub(uint128 a, uint128 b) internal pure returns (uint128) {
-        return sub(a, b, "SafeMath: subtraction overflow");
+        return sub(a, b, "aa");
     }
 
     /**
@@ -61,7 +61,7 @@ library SafeMathUInt128 {
         uint128 b,
         string memory errorMessage
     ) internal pure returns (uint128) {
-        require(b <= a, "aa");
+        require(b <= a, errorMessage);
         uint128 c = a - b;
 
         return c;
@@ -102,7 +102,7 @@ library SafeMathUInt128 {
      * - The divisor cannot be zero.
      */
     function div(uint128 a, uint128 b) internal pure returns (uint128) {
-        return div(a, b, "SafeMath: division by zero");
+        return div(a, b, "ac");
     }
 
     /**
@@ -124,7 +124,7 @@ library SafeMathUInt128 {
         string memory errorMessage
     ) internal pure returns (uint128) {
         // Solidity only automatically asserts when dividing by 0
-        require(b > 0, "ac");
+        require(b > 0, errorMessage);
         uint128 c = a / b;
         // assert(a == b * c + a % b); // There is no case in which this doesn't hold
 
@@ -143,7 +143,7 @@ library SafeMathUInt128 {
      * - The divisor cannot be zero.
      */
     function mod(uint128 a, uint128 b) internal pure returns (uint128) {
-        return mod(a, b, "SafeMath: modulo by zero");
+        return mod(a, b, "ad");
     }
 
     /**
@@ -164,7 +164,7 @@ library SafeMathUInt128 {
         uint128 b,
         string memory errorMessage
     ) internal pure returns (uint128) {
-        require(b != 0, "ad");
+        require(b != 0, errorMessage);
         return a % b;
     }
 }

contracts/contracts/Utils.sol

@@ -56,18 +56,6 @@ library Utils {
         return callSuccess && returnedSuccess;
     }
 
-    /// @notice Sends ETH
-    /// @param _to Address of recipient
-    /// @param _amount Amount of tokens to transfer
-    /// @return bool flag indicating that transfer is successful
-    function sendETHNoRevert(address payable _to, uint256 _amount) internal returns (bool) {
-        // TODO: Use constant from Config
-        uint256 WITHDRAWAL_GAS_LIMIT = 50000;
-
-        (bool callSuccess, ) = _to.call{gas: WITHDRAWAL_GAS_LIMIT, value: _amount}("");
-        return callSuccess;
-    }
-
     /// @notice Recovers signer's address from ethereum signature for given message
     /// @param _signature 65 bytes concatenated. R (32) + S (32) + V (1)
     /// @param _messageHash signed message hash.

contracts/contracts/ZkSync.sol

@@ -273,7 +273,7 @@ contract ZkSync is UpgradeableMaster, Storage, Config, Events, ReentrancyGuard {
                 tokenId: tokenId,
                 amount: 0 // unknown at this point
             });
-        bytes memory pubData = Operations.writeFullExitPubdata(op);
+        bytes memory pubData = Operations.writeFullExitPubdataForPriorityQueue(op);
         addPriorityRequest(Operations.OpType.FullExit, pubData);
 
         // User must fill storage slot of balancesToWithdraw(msg.sender, tokenId) with nonzero value
@@ -329,7 +329,6 @@ contract ZkSync is UpgradeableMaster, Storage, Config, Events, ReentrancyGuard {
         // Check that we commit blocks after last committed block
         require(storedBlockHashes[totalBlocksCommitted] == hashStoredBlockInfo(_lastCommittedBlockData), "ax"); // incorrect previous block data
 
-        uint64 committedPriorityRequests = 0;
         for (uint32 i = 0; i < _newBlocksData.length; ++i) {
             _lastCommittedBlockData = commitOneBlock(_lastCommittedBlockData, _newBlocksData[i]);
 
@@ -357,7 +356,7 @@ contract ZkSync is UpgradeableMaster, Storage, Config, Events, ReentrancyGuard {
         bool sent = false;
         if (_tokenId == 0) {
             address payable toPayable = address(uint160(_recipient));
-            sent = Utils.sendETHNoRevert(toPayable, _amount);
+            sent = sendETHNoRevert(toPayable, _amount);
         } else {
             address tokenAddr = governance.tokenAddresses(_tokenId);
             try this.withdrawERC20Guarded{gas: WITHDRAWAL_GAS_LIMIT}(IERC20(tokenAddr), _recipient, _amount, _amount) {
@@ -366,11 +365,10 @@ contract ZkSync is UpgradeableMaster, Storage, Config, Events, ReentrancyGuard {
                 sent = false;
             }
         }
-        if (sent) {
-            emit OnchainWithdrawal(_recipient, _tokenId, _amount);
-        } else {
+        if (!sent) {
             increaseBalanceToWithdraw(packedBalanceKey, _amount);
         }
+        emit OnchainWithdrawal(_recipient, _tokenId, _amount, sent);
     }
 
     /// @dev Executes one block
@@ -575,7 +573,7 @@ contract ZkSync is UpgradeableMaster, Storage, Config, Events, ReentrancyGuard {
                 tokenId: _tokenId,
                 amount: _amount
             });
-        bytes memory pubData = Operations.writeDepositPubdata(op);
+        bytes memory pubData = Operations.writeDepositPubdataForPriorityQueue(op);
         addPriorityRequest(Operations.OpType.Deposit, pubData);
 
         emit OnchainDeposit(msg.sender, _tokenId, _amount, _owner);
@@ -593,7 +591,7 @@ contract ZkSync is UpgradeableMaster, Storage, Config, Events, ReentrancyGuard {
         bytes22 packedBalanceKey = packAddressAndTokenId(_to, _token);
         uint128 balance = balancesToWithdraw[packedBalanceKey].balanceToWithdraw;
         balancesToWithdraw[packedBalanceKey].balanceToWithdraw = balance.sub(_amount);
-        emit OnchainWithdrawal(_to, _token, _amount);
+        emit OnchainWithdrawal(_to, _token, _amount, true);
     }
 
     function emitDepositCommitEvent(uint32 _blockNumber, Operations.Deposit memory depositData) internal {
@@ -872,4 +870,13 @@ contract ZkSync is UpgradeableMaster, Storage, Config, Events, ReentrancyGuard {
         uint128 balance = balancesToWithdraw[_packedBalanceKey].balanceToWithdraw;
         balancesToWithdraw[_packedBalanceKey] = BalanceToWithdraw(balance.add(_amount), FILLED_GAS_RESERVE_VALUE);
     }
+
+    /// @notice Sends ETH
+    /// @param _to Address of recipient
+    /// @param _amount Amount of tokens to transfer
+    /// @return bool flag indicating that transfer is successful
+    function sendETHNoRevert(address payable _to, uint256 _amount) internal returns (bool) {
+        (bool callSuccess, ) = _to.call{gas: WITHDRAWAL_GAS_LIMIT, value: _amount}("");
+        return callSuccess;
+    }
 }

contracts/contracts/dev-contracts/ZKSyncSignatureUnitTest.sol

@@ -7,23 +7,15 @@ pragma experimental ABIEncoderV2;
 import "../ZkSync.sol";
 
 contract ZKSyncSignatureUnitTest is ZkSync {
-    function changePubkeySignatureCheck(
-        bytes calldata _signature,
-        bytes20 _newPkHash,
-        uint32 _nonce,
-        address _ethAddress,
-        uint24 _accountId
-    ) external pure returns (bool) {
-        Operations.ChangePubKey memory changePk;
-        changePk.owner = _ethAddress;
-        changePk.nonce = _nonce;
-        changePk.pubKeyHash = _newPkHash;
-        changePk.accountId = _accountId;
-        bytes memory witness = abi.encodePacked(bytes1(0x01), _signature, bytes32(0));
-        return verifyChangePubkeyECRECOVER(witness, changePk);
+    function changePubkeySignatureCheck(Operations.ChangePubKey memory _changePk, bytes memory _witness)
+        external
+        pure
+        returns (bool)
+    {
+        return verifyChangePubkeyECRECOVER(_witness, _changePk);
     }
 
-    function testRecoverAddressFromEthSignature(bytes calldata _signature, bytes32 _messageHash)
+    function testRecoverAddressFromEthSignature(bytes memory _signature, bytes32 _messageHash)
         external
         pure
         returns (address)

contracts/contracts/dev-contracts/ZkSyncProcessOpUnitTest.sol

@@ -14,9 +14,9 @@ contract ZkSyncProcessOpUnitTest is ZkSync {
         bytes memory offsetsCommitment
     ) external {
         (bytes32 resOpHash, uint64 resPriorOps, bytes memory resOffsetsCommitment) = collectOnchainOps(_newBlockData);
-        require(resOpHash == processableOperationsHash, "hash");
-        require(resPriorOps == priorityOperationsProcessed, "prop");
-        require(keccak256(resOffsetsCommitment) == keccak256(offsetsCommitment), "offComm");
+        require(resOpHash == processableOperationsHash, "h");
+        require(resPriorOps == priorityOperationsProcessed, "p");
+        require(keccak256(resOffsetsCommitment) == keccak256(offsetsCommitment), "o");
     }
 
     function commitPriorityRequests() external {

core/bin/zksync_core/src/eth_watch/client.rs

@@ -15,7 +15,6 @@ use zksync_types::{ethereum::CompleteWithdrawalsTx, Address, Nonce, PriorityOp,
 
 struct ContractTopics {
     new_priority_request: Hash,
-    complete_withdrawals_event: Hash,
 }
 
 impl ContractTopics {
@@ -25,11 +24,6 @@ impl ContractTopics {
                 .event("NewPriorityRequest")
                 .expect("main contract abi error")
                 .signature(),
-
-            complete_withdrawals_event: zksync_contract
-                .event("PendingWithdrawalsComplete")
-                .expect("main contract abi error")
-                .signature(),
         }
     }
 }
@@ -41,15 +35,8 @@ pub trait EthClient {
         from: BlockNumber,
         to: BlockNumber,
     ) -> anyhow::Result<Vec<PriorityOp>>;
-    async fn get_complete_withdrawals_event(
-        &self,
-        from: BlockNumber,
-        to: BlockNumber,
-    ) -> anyhow::Result<Vec<CompleteWithdrawalsTx>>;
     async fn block_number(&self) -> anyhow::Result<u64>;
     async fn get_auth_fact(&self, address: Address, nonce: Nonce) -> anyhow::Result<Vec<u8>>;
-    async fn get_first_pending_withdrawal_index(&self) -> anyhow::Result<u32>;
-    async fn get_number_of_pending_withdrawals(&self) -> anyhow::Result<u32>;
 }
 
 pub struct EthHttpClient {
@@ -116,24 +103,6 @@ impl EthClient for EthHttpClient {
         result
     }
 
-    async fn get_complete_withdrawals_event(
-        &self,
-        from: BlockNumber,
-        to: BlockNumber,
-    ) -> anyhow::Result<Vec<CompleteWithdrawalsTx>> {
-        let start = Instant::now();
-
-        let result = self
-            .get_events(from, to, vec![self.topics.complete_withdrawals_event])
-            .await;
-
-        metrics::histogram!(
-            "eth_watcher.get_complete_withdrawals_event",
-            start.elapsed()
-        );
-        result
-    }
-
     async fn block_number(&self) -> anyhow::Result<u64> {
         Ok(self.web3.eth().block_number().await?.as_u64())
     }
@@ -150,35 +119,4 @@ impl EthClient for EthHttpClient {
             .await
             .map_err(|e| format_err!("Failed to query contract authFacts: {}", e))
     }
-
-    async fn get_first_pending_withdrawal_index(&self) -> anyhow::Result<u32> {
-        self.zksync_contract
-            .query(
-                "firstPendingWithdrawalIndex",
-                (),
-                None,
-                Options::default(),
-                None,
-            )
-            .await
-            .map_err(|e| {
-                format_err!(
-                    "Failed to query contract firstPendingWithdrawalIndex: {}",
-                    e
-                )
-            })
-    }
-
-    async fn get_number_of_pending_withdrawals(&self) -> anyhow::Result<u32> {
-        self.zksync_contract
-            .query(
-                "numberOfPendingWithdrawals",
-                (),
-                None,
-                Options::default(),
-                None,
-            )
-            .await
-            .map_err(|e| format_err!("Failed to query contract numberOfPendingWithdrawals: {}", e))
-    }
 }