Merge #1490

1490: Loadnext r=popzxc a=popzxc

Surprise!

I had some spare time recently, thus I decided to work on the important task that nobody has free hands to work on.
Meet `loadnext`: a new random-driven loadtest.

It:
- doesn't care whether the server is alive or not. At worst, it will just consider the test failed. No panics, no mindless unwraps, yay.
- does a unique set of operations for each participating account.
- sends transactions, batches, and priority operations.
- sends incorrect transactions as well as correct ones and compares the outcome to the expected one.
- has an easy-to-extend command system that allows adding new types of actions to the flow.
- has an easy-to-extend report analysis system.
- already found a bug in the server code.

Flaws:
- API commands are not implemented. I was not sure whether it makes sense to test v0.1 API, and likely it will be better to implement these commands for API v0.2 once it's ready.
- Report analysis system is in a pretty primitive state. I think that it's weird to try to understand which kind of analysis we need until we have something actually running. The system is easy to extend, so I think we will integrate the loadtest first, and then improve the report system.

I didn't write the code in a while, so it's time to get revenge for all my nitpicks 

Resolves ZKS-260


Co-authored-by: Igor Aleksanov <[email protected]>

Cargo.toml

@@ -37,6 +37,7 @@ members = [
     "core/tests/test_account",
     "core/tests/testkit",
     "core/tests/loadtest",
+    "core/tests/loadnext",
 
     # SDK section
     "sdk/zksync-rs",

changelog/core.md

@@ -30,6 +30,7 @@ All notable changes to the core components will be documented in this file.
 
 - (`zksync_api`): Internal error with tokens not listed on CoinGecko.
 - Fix wrong block info cache behavior in the `api_server`.
+- Bug with gas price limit being used instead of average gas price when storing data to DB in gas adjuster.
 - `timeout` in ETH sender main loop was replaced with `tokio::time::delay_for`.
 
 ## Release 2021-02-19

changelog/infrastructure.md

@@ -15,6 +15,8 @@ components, the logs will have the following format:
 
 ### Added
 
+- `loadnext` crate, a new implementation of the loadtest for zkSync.
+
 ### Fixed
 
 ## Release 2021-02-19

changelog/rust-sdk.md

@@ -6,6 +6,12 @@ All notable changes to `zksync_rs` will be documented in this file.
 
 ### Added
 
+- `PriorityOpHandle` structure, allowing awaiting for the priority operations execution.
+- `PriorityOpHolder::priority_op_handle` method, allowing to get `PriorityOpHandle` out of the Ethereum transaction
+  logs.
+- `mint` feature with `mint_erc20` for minting ERC-20 tokens.
+- `EthereumProvider::erc20_balance` method for getting the balance of ERC-20 token.
+
 ### Changed
 
 - Hardcode gas limit for `depositERC20` for each token.

core/bin/zksync_eth_sender/src/gas_adjuster/mod.rs

@@ -125,11 +125,20 @@ impl<DB: DatabaseInterface> GasAdjuster<DB> {
                     return;
                 }
             };
+
+            let average_gas_price = match self.statistics.get_average_price() {
+                Some(price) => price,
+                None => {
+                    // Not enough data to update anything yet.
+                    return;
+                }
+            };
+
             let result = db
                 .update_gas_price_params(
                     &mut connection,
                     self.statistics.get_limit(),
-                    self.get_average_gas_price(),
+                    average_gas_price,
                 )
                 .await;
 
@@ -156,14 +165,6 @@ impl<DB: DatabaseInterface> GasAdjuster<DB> {
     pub fn get_current_max_price(&self) -> U256 {
         self.statistics.get_limit()
     }
-
-    /// Get estimate of the average gas prices used for past transactions based on the current gas_limit.
-    pub fn get_average_gas_price(&self) -> U256 {
-        let scale_factor = parameters::limit_scale_factor();
-        let divider = U256::from((scale_factor * 100.0f64).round() as u64);
-        let multiplier = U256::from(100);
-        self.statistics.get_limit() * multiplier / divider
-    }
 }
 
 /// Helper structure responsible for collecting the data about recent transactions,
@@ -177,7 +178,7 @@ pub(super) struct GasStatistics {
 
 impl GasStatistics {
     /// Amount of entries in the gas price statistics pool.
-    const GAS_PRICE_SAMPLES_AMOUNT: usize = 10;
+    pub(crate) const GAS_PRICE_SAMPLES_AMOUNT: usize = 10;
 
     pub fn new(initial_max_price: U256) -> Self {
         Self {

core/bin/zksync_eth_sender/src/gas_adjuster/tests.rs

@@ -146,6 +146,46 @@ async fn initial_upper_gas_limit() {
     assert_eq!(scaled_gas, PRICE_LIMIT.into());
 }
 
+/// Checks whether the average gas price is stored to the database correctly.
+#[tokio::test]
+async fn average_gas_price_stored_correctly() {
+    // Initial price limit to set.
+    const PRICE_LIMIT: i64 = 1000;
+
+    let (mut ethereum, db) = eth_and_db_clients().await;
+
+    db.update_gas_price_limit(PRICE_LIMIT).await.unwrap();
+    let mut gas_adjuster: GasAdjuster<MockDatabase> = GasAdjuster::new(&db).await;
+
+    let initial_db_price = db.average_gas_price().await;
+    assert_eq!(initial_db_price, 0u64.into()); // Check just in case.
+
+    // Set the low gas price in Ethereum.
+    let ethereum_price = U256::from(1u64);
+    ethereum
+        .get_mut_mock()
+        .unwrap()
+        .set_gas_price(ethereum_price)
+        .await
+        .unwrap();
+
+    // Update the gas adjuster state once.
+    gas_adjuster.keep_updated(&ethereum, &db).await;
+
+    // Average gas price should not be changed in the DB, as we don't have enough samples.
+    let current_db_price = db.average_gas_price().await;
+    assert_eq!(current_db_price, initial_db_price);
+
+    // Now update adjuster multiple times for it to gather enough statistics.
+    for _ in 0..GasStatistics::GAS_PRICE_SAMPLES_AMOUNT {
+        gas_adjuster.keep_updated(&ethereum, &db).await;
+    }
+
+    // Finally, the stored price should become equal to the network price.
+    let current_db_price = db.average_gas_price().await;
+    assert_eq!(current_db_price, ethereum_price);
+}
+
 /// Checks the gas price limit scaling algorithm:
 /// We are successively keep requesting the gas price with the
 /// ethereum client suggesting the price far beyond the current limit

core/bin/zksync_eth_sender/src/tests/mock.rs

@@ -87,6 +87,13 @@ impl MockDatabase {
 
         assert!(is_confirmed);
     }
+
+    /// Returns the stored average gas price.
+    pub async fn average_gas_price(&self) -> U256 {
+        let eth_parameters = self.eth_parameters.read().await;
+
+        U256::from(eth_parameters.average_gas_price.unwrap_or_default() as u64)
+    }
 }
 
 #[async_trait::async_trait]

core/lib/types/src/tokens.rs

@@ -6,6 +6,9 @@ use std::{fmt, fs::read_to_string, path::PathBuf, str::FromStr};
 use thiserror::Error;
 use zksync_utils::{parse_env, UnsignedRatioSerializeAsDecimal};
 
+/// ID of the ETH token in zkSync network.
+pub use zksync_crypto::params::ETH_TOKEN_ID;
+
 // Order of the fields is important (from more specific types to less specific types)
 /// Set of values that can be interpreted as a token descriptor.
 #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, Hash)]

core/lib/types/src/tx/withdraw.rs

@@ -136,9 +136,11 @@ impl Withdraw {
     ///
     /// - `account_id` field must be within supported range.
     /// - `token` field must be within supported range.
-    /// - `amount` field must represent a packable value.
     /// - `fee` field must represent a packable value.
     /// - zkSync signature must correspond to the PubKeyHash of the account.
+    ///
+    /// Note that we don't need to check whether token amount is packable, because pubdata for this operation
+    /// contains unpacked value only.
     pub fn check_correctness(&mut self) -> bool {
         let mut valid = self.amount <= BigUint::from(u128::max_value())
             && is_fee_amount_packable(&self.fee)

core/tests/loadnext/Cargo.toml

@@ -0,0 +1,30 @@
+[package]
+name = "loadnext"
+version = "0.1.0"
+edition = "2018"
+authors = ["The Matter Labs Team <[email protected]>"]
+homepage = "https://zksync.io/"
+repository = "https://github.com/matter-labs/zksync"
+license = "Apache-2.0"
+keywords = ["blockchain", "zksync"]
+categories = ["cryptography"]
+publish = false # We don't want to publish our tests.
+
+[dependencies]
+zksync = { path = "../../../sdk/zksync-rs", version = "0.3", features = ["mint"] }
+zksync_types = { path = "../../lib/types", version = "1.0" }
+zksync_eth_signer = { path = "../../lib/eth_signer", version = "1.0" }
+vlog = { path = "../../lib/vlog", version = "1.0" }
+
+serde = { version = "1.0", features = ["derive"] }
+num = { version = "0.3.1", features = ["serde"] }
+tokio = { version = "0.2", features = ["full"] }
+futures = "0.3"
+anyhow = "1.0"
+rand = { version = "0.7", features = ["small_rng"] }
+envy = "0.4"
+hex = "0.4"
+static_assertions = "1.1"
+
+[dev-dependencies]
+zksync_test_account = { path = "../test_account", version = "1.0" }

core/tests/loadnext/README.md

@@ -0,0 +1,100 @@
+# Loadnext: the next generation loadtest for zkSync
+
+Loadnext is an utility for random stress-testing the zkSync server. It is capable of simulating the behavior of many
+independent users of zkSync network, who are sending quasi-random requests to the server.
+
+It:
+
+- doesn't care whether the server is alive or not. At worst, it will just consider the test failed. No panics, no
+  mindless unwraps, yay.
+- does a unique set of operations for each participating account.
+- sends transactions, batches, and priority operations.
+- sends incorrect transactions as well as correct ones and compares the outcome to the expected one.
+- has an easy-to-extend command system that allows adding new types of actions to the flow.
+- has an easy-to-extend report analysis system.
+
+Flaws:
+
+- It does not send API requests other than required to execute transactions.
+- So far it has pretty primitive report system.
+
+## Launch
+
+In order to launch the test in the development scenario, you must first run server and prover (it is recommended to use
+dummy prover), and then launch the test itself.
+
+```sh
+# First terminal
+zk server
+# Second terminal
+zk dummy-prover
+# Third terminal
+RUST_BACKTRACE=1 RUST_LOG=info cargo run --bin loadnext
+```
+
+Without any configuration supplied, the test will fallback to the dev defaults:
+
+- Use one of the "rich" accounts in the private local Ethereum chain.
+- Use mintable "DAI" token.
+- Connect to the localhost zkSync node and use localhost web3 API.
+
+**Note:** when running the loadtest in the localhost scenario, you **must** adjust the supported block chunks sizes.
+Edit the `etc/env/dev/chain.toml` and set `block_chunk_sizes` to `[10,32,72,156,322,654]` and `aggregated_proof_sizes`
+to `[1,4,8,18]`. Do not forget to re-compile configs after that.
+
+This is required because the loadtest relies on batches, which will not fit into smaller block sizes.
+
+## Configuration
+
+For cases when loadtest is launched outside of the localhost environment, configuration is provided via environment
+variables.
+
+The following variables are required:
+
+```sh
+# Address of the zkSync node.
+ZKSYNC_RPC_ADDR
+# Address of the Ethereum web3 API.
+WEB3_URL
+# Used Ethereum network (e.g. `rinkeby` or `localhost`).
+ETH_NETWORK
+# Ethereum private key of the wallet that has funds to perform a test (without `0x` prefix).
+MASTER_WALLET_PK
+# Amount of accounts to be used in test.
+# This option configures the "width" of the test:
+# how many concurrent operation flows will be executed.
+ACCOUNTS_AMOUNT
+# Amount of operations per account.
+# This option configures the "length" of the test:
+# how many individual operations each account of the test will execute.
+OPERATIONS_PER_ACCOUNT
+# Symbolic representation of the ERC-20 token to be used in test.
+#
+# Token must satisfy two criteria:
+# - Be supported by zkSync.
+# - Have `mint` operation.
+#
+# Note that we use ERC-20 token since we can't easily mint a lot of ETH on
+# Rinkeby or Ropsten without caring about collecting it back.
+MAIN_TOKEN
+```
+
+Optional parameters:
+
+```sh
+# Optional seed to be used in the test: normally you don't need to set the seed,
+# but you can re-use seed from previous run to reproduce the sequence of operations locally.
+# Seed must be represented as a hexadecimal string.
+SEED
+```
+
+## Infrastructure relationship
+
+This crate is meant to be independent of the existing zkSync infrastructure. It is not integrated in `zk` and does not
+rely on `zksync_config` crate, and is not meant to be.
+
+The reason is that this application is meant to be used in CI, where any kind of configuration pre-requisites makes the
+tool harder to use:
+
+- Additional tools (e.g. `zk`) must be shipped together with the application, or included into the docker container.
+- Configuration that lies in files is harder to use in CI, due some sensitive data being stored in GITHUB_SECRETS.