From 383e930c9d3b29a3e825e7df4617b2d73dc79bc1 Mon Sep 17 00:00:00 2001 From: Ihor Barenblat Date: Mon, 8 Mar 2021 15:35:47 +0200 Subject: [PATCH 01/17] fee_free_account --- .../zksync_api/src/api_server/tx_sender.rs | 49 ++++++++++++------- core/lib/config/src/configs/api.rs | 5 ++ core/lib/types/src/tx/zksync_tx.rs | 14 ++++++ etc/env/base/api.toml | 2 + yarn.lock | 12 ++++- 5 files changed, 62 insertions(+), 20 deletions(-) diff --git a/core/bin/zksync_api/src/api_server/tx_sender.rs b/core/bin/zksync_api/src/api_server/tx_sender.rs index 0255e79886..0737ad0674 100644 --- a/core/bin/zksync_api/src/api_server/tx_sender.rs +++ b/core/bin/zksync_api/src/api_server/tx_sender.rs @@ -1,6 +1,7 @@ //! Helper module to submit transactions into the zkSync Network. // Built-in uses +use std::iter::FromIterator; use std::{collections::HashSet, fmt::Display, str::FromStr}; // External uses @@ -21,7 +22,7 @@ use zksync_types::{ tx::{ EthBatchSignData, EthBatchSignatures, EthSignData, SignedZkSyncTx, TxEthSignature, TxHash, }, - Address, BatchFee, Fee, Token, TokenId, TokenLike, TxFeeTypes, ZkSyncTx, H160, + AccountId, Address, BatchFee, Fee, Token, TokenId, TokenLike, TxFeeTypes, ZkSyncTx, H160, }; // Local uses @@ -44,6 +45,8 @@ pub struct TxSender { pub tokens: TokenDBCache, /// Mimimum age of the account for `ForcedExit` operations to be allowed. pub forced_exit_minimum_account_age: chrono::Duration, + /// List of account ids that are not paying fees on operations. + pub fee_free_accounts: HashSet, pub enforce_pubkey_change_fee: bool, // Limit the number of both transactions and Ethereum signatures per batch. pub max_number_of_transactions_per_batch: usize, @@ -145,6 +148,7 @@ impl TxSender { enforce_pubkey_change_fee: config.api.common.enforce_pubkey_change_fee, forced_exit_minimum_account_age, + fee_free_accounts: HashSet::from_iter(config.api.common.fee_free_accounts.clone()), max_number_of_transactions_per_batch, max_number_of_authors_per_batch, } @@ -227,26 +231,32 @@ impl TxSender { let ticker_request_sender = self.ticker_requests.clone(); if let Some((tx_type, token, address, provided_fee)) = tx_fee_info { - let should_enforce_fee = !matches!(tx_type, TxFeeTypes::ChangePubKey { .. }) - || self.enforce_pubkey_change_fee; + let is_whitelisted_initiator = self + .fee_free_accounts + .contains(&tx.get_initiator_account_id()); - let fee_allowed = - Self::token_allowed_for_fees(ticker_request_sender.clone(), token.clone()).await?; + if !is_whitelisted_initiator { + let should_enforce_fee = !matches!(tx_type, TxFeeTypes::ChangePubKey { .. }) + || self.enforce_pubkey_change_fee; - if !fee_allowed { - return Err(SubmitError::InappropriateFeeToken); - } + let fee_allowed = + Self::token_allowed_for_fees(ticker_request_sender.clone(), token.clone()) + .await?; + + if !fee_allowed { + return Err(SubmitError::InappropriateFeeToken); + } - let required_fee = - Self::ticker_request(ticker_request_sender, tx_type, address, token.clone()) - .await?; - // Converting `BitUint` to `BigInt` is safe. - let required_fee: BigDecimal = required_fee.total_fee.to_bigint().unwrap().into(); - let provided_fee: BigDecimal = provided_fee.to_bigint().unwrap().into(); - // Scaling the fee required since the price may change between signing the transaction and sending it to the server. - let scaled_provided_fee = scale_user_fee_up(provided_fee.clone()); - if required_fee >= scaled_provided_fee && should_enforce_fee { - vlog::error!( + let required_fee = + Self::ticker_request(ticker_request_sender, tx_type, address, token.clone()) + .await?; + // Converting `BitUint` to `BigInt` is safe. + let required_fee: BigDecimal = required_fee.total_fee.to_bigint().unwrap().into(); + let provided_fee: BigDecimal = provided_fee.to_bigint().unwrap().into(); + // Scaling the fee required since the price may change between signing the transaction and sending it to the server. + let scaled_provided_fee = scale_user_fee_up(provided_fee.clone()); + if required_fee >= scaled_provided_fee && should_enforce_fee { + vlog::error!( "User provided fee is too low, required: {}, provided: {} (scaled: {}); difference {}, token: {:?}", required_fee.to_string(), provided_fee.to_string(), @@ -255,7 +265,8 @@ impl TxSender { token ); - return Err(SubmitError::TxAdd(TxAddError::TxFeeTooLow)); + return Err(SubmitError::TxAdd(TxAddError::TxFeeTooLow)); + } } } diff --git a/core/lib/config/src/configs/api.rs b/core/lib/config/src/configs/api.rs index 7f19f5a8d4..37ed1f2e34 100644 --- a/core/lib/config/src/configs/api.rs +++ b/core/lib/config/src/configs/api.rs @@ -2,6 +2,8 @@ use serde::Deserialize; /// Built-in uses use std::net::SocketAddr; +// Workspace uses +use zksync_types::AccountId; // Local uses use crate::envy_load; @@ -46,6 +48,8 @@ pub struct Common { // Determines the required minimum account age for `ForcedExit` operation to be allowed. // Type of value is seconds. pub forced_exit_minimum_account_age_secs: u64, + // List of account ids that are not paying fees on operations. + pub fee_free_accounts: Vec, pub enforce_pubkey_change_fee: bool, pub max_number_of_transactions_per_batch: u64, @@ -154,6 +158,7 @@ mod tests { enforce_pubkey_change_fee: true, max_number_of_transactions_per_batch: 200, max_number_of_authors_per_batch: 10, + fee_free_accounts: vec![], }, admin: AdminApi { port: 8080, diff --git a/core/lib/types/src/tx/zksync_tx.rs b/core/lib/types/src/tx/zksync_tx.rs index 2f5b3607c6..6e3fb911a0 100644 --- a/core/lib/types/src/tx/zksync_tx.rs +++ b/core/lib/types/src/tx/zksync_tx.rs @@ -303,6 +303,20 @@ impl ZkSyncTx { } } + /// Returns the transaction initiator's account id. + /// + /// Returns `None` if transaction initiator's account id is unknown. + /// For example, `Close` operation. + pub fn get_initiator_account_id(&self) -> AccountId { + match self { + ZkSyncTx::Transfer(tx) => tx.account_id, + ZkSyncTx::Withdraw(tx) => tx.account_id, + ZkSyncTx::Close(_) => unreachable!(), + ZkSyncTx::ChangePubKey(tx) => tx.account_id, + ZkSyncTx::ForcedExit(tx) => tx.initiator_account_id, + } + } + /// Returns the unix format timestamp of the first moment when transaction execution is valid. pub fn valid_from(&self) -> u64 { match self { diff --git a/etc/env/base/api.toml b/etc/env/base/api.toml index ba15b4ee3f..8aacc73472 100644 --- a/etc/env/base/api.toml +++ b/etc/env/base/api.toml @@ -9,6 +9,8 @@ caches_size=10000 # value at least 24 hours for production. # Type of value is seconds. forced_exit_minimum_account_age_secs=0 +# List of account ids that are not paying fees on operations. +fee_free_accounts=[] # Ability to perform change pub key with zero fee enforce_pubkey_change_fee=true diff --git a/yarn.lock b/yarn.lock index ad75a65c62..c03416c0a6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -14740,8 +14740,18 @@ zksync-crypto@^0.4.2: resolved "https://registry.yarnpkg.com/zksync-crypto/-/zksync-crypto-0.4.5.tgz#2941ea8dbada9178390afdcc3c93430f31b00784" integrity sha512-XhhnNhc85BgfmLEAmA7YqrblUltfykrPxlbayGe0dQowroco+1ply3VEZbs0tSEDW5ssBwJHzRSYm3+dOCnzxQ== -"zksync@link:sdk/zksync.js": +zksync@^0.9.0: version "0.9.0" + resolved "https://registry.yarnpkg.com/zksync/-/zksync-0.9.0.tgz#7550f81a193e124464582e33062a03625a5e9b7f" + integrity sha512-5pRSsml/0fTNgkcmvTWi+Ar9+XFtdbpa6GSzx/DCCoHKvNLUUwEYIVYi5azTNnm7R1DEsJY1cWHyBhlyqI2RvA== + dependencies: + axios "^0.21.1" + websocket "^1.0.30" + websocket-as-promised "^1.1.0" + zksync-crypto "^0.4.2" + +"zksync@link:sdk/zksync.js": + version "0.10.0" dependencies: axios "^0.21.1" websocket "^1.0.30" From 38d09b52bf42c440b1297e7cdd8694a16b0e1923 Mon Sep 17 00:00:00 2001 From: Ihor Barenblat Date: Mon, 8 Mar 2021 15:47:36 +0200 Subject: [PATCH 02/17] changelog --- changelog/core.md | 1 + 1 file changed, 1 insertion(+) diff --git a/changelog/core.md b/changelog/core.md index 9552f450ff..3675d3af57 100644 --- a/changelog/core.md +++ b/changelog/core.md @@ -21,6 +21,7 @@ All notable changes to the core components will be documented in this file. operations. - (`eth_client`): Added `get_tx`, `create_contract` methods to `EthereumGateway`, `get_web3_transport` method to ETHDirectClient. +- (`api_server`): Support for accounts that don't have to pay fees (e.g. network service accounts) was added. ### Fixed From 2e49daf6561e8e265083d0fd77f152e0e841d283 Mon Sep 17 00:00:00 2001 From: Ihor Barenblat Date: Tue, 9 Mar 2021 12:10:35 +0200 Subject: [PATCH 03/17] tests for fee free accounts --- .../src/api_server/rest/v1/transactions.rs | 63 ++++++++++++++----- .../zksync_api/src/api_server/tx_sender.rs | 21 ++++--- core/lib/config/src/configs/api.rs | 2 +- core/lib/types/src/tx/zksync_tx.rs | 14 ----- etc/env/base/api.toml | 2 +- 5 files changed, 61 insertions(+), 41 deletions(-) diff --git a/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs b/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs index a2fd100ac9..7b7a88e97f 100644 --- a/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs +++ b/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs @@ -476,6 +476,19 @@ mod tests { } } + #[actix_rt::test] + #[cfg_attr( + not(feature = "api_test"), + ignore = "Use `zk test rust-api` command to perform this test" + )] + async fn test_rust_api() -> anyhow::Result<()> { + test_transactions_scope().await?; + test_bad_fee_token().await?; + test_fast_processing_flag().await?; + test_fee_free_account().await?; + Ok(()) + } + #[actix_rt::test] #[cfg_attr( not(feature = "api_test"), @@ -498,11 +511,6 @@ mod tests { Ok(()) } - #[actix_rt::test] - #[cfg_attr( - not(feature = "api_test"), - ignore = "Use `zk test rust-api` command to perform this test" - )] async fn test_transactions_scope() -> anyhow::Result<()> { let (client, server) = TestServer::new().await?; @@ -665,11 +673,6 @@ mod tests { /// - Attempt to pay fees in an inappropriate token fails for single txs. /// - Attempt to pay fees in an inappropriate token fails for single batch. /// - Batch with an inappropriate token still can be processed if the fee is covered with a common token. - #[actix_rt::test] - #[cfg_attr( - not(feature = "api_test"), - ignore = "Use `zk test rust-api` command to perform this test" - )] async fn test_bad_fee_token() -> anyhow::Result<()> { let (client, server) = TestServer::new().await?; @@ -781,16 +784,46 @@ mod tests { Ok(()) } + /// This test checks the following: + /// + /// Attempt by fee free account to pay zero fee in single tx or txs batch. + async fn test_fee_free_account() -> anyhow::Result<()> { + let (client, server) = TestServer::new().await?; + + let from = ZkSyncAccount::rand(); + from.set_account_id(Some(AccountId(0xfee))); + let to = ZkSyncAccount::rand(); + + // Submit transaction with a zero fee by the fee free account + let (tx, eth_sig) = from.sign_transfer( + TokenId(0), + "ETH", + 0u64.into(), + 0u64.into(), + &to.address, + Some(Nonce(0)), + false, + Default::default(), + ); + let transfer = ZkSyncTx::Transfer(Box::new(tx)); + client + .submit_tx( + transfer.clone(), + eth_sig.map(TxEthSignature::EthereumSignature), + None, + ) + .await + .unwrap(); + + server.stop().await; + Ok(()) + } + /// This test checks the following criteria: /// /// - Attempt to submit non-withdraw transaction with the enabled fast-processing. /// - Attempt to submit non-withdraw transaction with the disabled fast-processing. /// - Attempt to submit withdraw transaction with the enabled fast-processing. - #[actix_rt::test] - #[cfg_attr( - not(feature = "api_test"), - ignore = "Use `zk test rust-api` command to perform this test" - )] async fn test_fast_processing_flag() -> anyhow::Result<()> { let (client, server) = TestServer::new().await?; diff --git a/core/bin/zksync_api/src/api_server/tx_sender.rs b/core/bin/zksync_api/src/api_server/tx_sender.rs index 0737ad0674..38697d2bfb 100644 --- a/core/bin/zksync_api/src/api_server/tx_sender.rs +++ b/core/bin/zksync_api/src/api_server/tx_sender.rs @@ -231,9 +231,10 @@ impl TxSender { let ticker_request_sender = self.ticker_requests.clone(); if let Some((tx_type, token, address, provided_fee)) = tx_fee_info { - let is_whitelisted_initiator = self - .fee_free_accounts - .contains(&tx.get_initiator_account_id()); + let is_whitelisted_initiator = tx + .account_id() + .map(|account_id| self.fee_free_accounts.contains(&account_id)) + .unwrap_or_default(); if !is_whitelisted_initiator { let should_enforce_fee = !matches!(tx_type, TxFeeTypes::ChangePubKey { .. }) @@ -257,13 +258,13 @@ impl TxSender { let scaled_provided_fee = scale_user_fee_up(provided_fee.clone()); if required_fee >= scaled_provided_fee && should_enforce_fee { vlog::error!( - "User provided fee is too low, required: {}, provided: {} (scaled: {}); difference {}, token: {:?}", - required_fee.to_string(), - provided_fee.to_string(), - scaled_provided_fee.to_string(), - (&required_fee - &scaled_provided_fee).to_string(), - token - ); + "User provided fee is too low, required: {}, provided: {} (scaled: {}); difference {}, token: {:?}", + required_fee.to_string(), + provided_fee.to_string(), + scaled_provided_fee.to_string(), + (&required_fee - &scaled_provided_fee).to_string(), + token + ); return Err(SubmitError::TxAdd(TxAddError::TxFeeTooLow)); } diff --git a/core/lib/config/src/configs/api.rs b/core/lib/config/src/configs/api.rs index 37ed1f2e34..cd757ae099 100644 --- a/core/lib/config/src/configs/api.rs +++ b/core/lib/config/src/configs/api.rs @@ -158,7 +158,7 @@ mod tests { enforce_pubkey_change_fee: true, max_number_of_transactions_per_batch: 200, max_number_of_authors_per_batch: 10, - fee_free_accounts: vec![], + fee_free_accounts: vec![4078], }, admin: AdminApi { port: 8080, diff --git a/core/lib/types/src/tx/zksync_tx.rs b/core/lib/types/src/tx/zksync_tx.rs index 6e3fb911a0..2f5b3607c6 100644 --- a/core/lib/types/src/tx/zksync_tx.rs +++ b/core/lib/types/src/tx/zksync_tx.rs @@ -303,20 +303,6 @@ impl ZkSyncTx { } } - /// Returns the transaction initiator's account id. - /// - /// Returns `None` if transaction initiator's account id is unknown. - /// For example, `Close` operation. - pub fn get_initiator_account_id(&self) -> AccountId { - match self { - ZkSyncTx::Transfer(tx) => tx.account_id, - ZkSyncTx::Withdraw(tx) => tx.account_id, - ZkSyncTx::Close(_) => unreachable!(), - ZkSyncTx::ChangePubKey(tx) => tx.account_id, - ZkSyncTx::ForcedExit(tx) => tx.initiator_account_id, - } - } - /// Returns the unix format timestamp of the first moment when transaction execution is valid. pub fn valid_from(&self) -> u64 { match self { diff --git a/etc/env/base/api.toml b/etc/env/base/api.toml index 8aacc73472..396dd10a01 100644 --- a/etc/env/base/api.toml +++ b/etc/env/base/api.toml @@ -10,7 +10,7 @@ caches_size=10000 # Type of value is seconds. forced_exit_minimum_account_age_secs=0 # List of account ids that are not paying fees on operations. -fee_free_accounts=[] +fee_free_accounts=[4078] # Ability to perform change pub key with zero fee enforce_pubkey_change_fee=true From 92441cee6bc5ceb00de5d0e1e07d16187d821680 Mon Sep 17 00:00:00 2001 From: Ihor Barenblat Date: Tue, 9 Mar 2021 12:13:24 +0200 Subject: [PATCH 04/17] remove yarn.lock changes --- yarn.lock | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/yarn.lock b/yarn.lock index c03416c0a6..ad75a65c62 100644 --- a/yarn.lock +++ b/yarn.lock @@ -14740,18 +14740,8 @@ zksync-crypto@^0.4.2: resolved "https://registry.yarnpkg.com/zksync-crypto/-/zksync-crypto-0.4.5.tgz#2941ea8dbada9178390afdcc3c93430f31b00784" integrity sha512-XhhnNhc85BgfmLEAmA7YqrblUltfykrPxlbayGe0dQowroco+1ply3VEZbs0tSEDW5ssBwJHzRSYm3+dOCnzxQ== -zksync@^0.9.0: - version "0.9.0" - resolved "https://registry.yarnpkg.com/zksync/-/zksync-0.9.0.tgz#7550f81a193e124464582e33062a03625a5e9b7f" - integrity sha512-5pRSsml/0fTNgkcmvTWi+Ar9+XFtdbpa6GSzx/DCCoHKvNLUUwEYIVYi5azTNnm7R1DEsJY1cWHyBhlyqI2RvA== - dependencies: - axios "^0.21.1" - websocket "^1.0.30" - websocket-as-promised "^1.1.0" - zksync-crypto "^0.4.2" - "zksync@link:sdk/zksync.js": - version "0.10.0" + version "0.9.0" dependencies: axios "^0.21.1" websocket "^1.0.30" From e6f1c22f2cd57098f22505e8e1c405849392f270 Mon Sep 17 00:00:00 2001 From: Ihor Barenblat Date: Tue, 9 Mar 2021 12:54:48 +0200 Subject: [PATCH 05/17] lints --- core/lib/config/src/configs/api.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/lib/config/src/configs/api.rs b/core/lib/config/src/configs/api.rs index cd757ae099..4b46e0b9a9 100644 --- a/core/lib/config/src/configs/api.rs +++ b/core/lib/config/src/configs/api.rs @@ -158,7 +158,7 @@ mod tests { enforce_pubkey_change_fee: true, max_number_of_transactions_per_batch: 200, max_number_of_authors_per_batch: 10, - fee_free_accounts: vec![4078], + fee_free_accounts: vec![AccountId(4078)], }, admin: AdminApi { port: 8080, From 0d29bf0e3dddde918c6a322057ae2f68e8790c5e Mon Sep 17 00:00:00 2001 From: Ihor Barenblat Date: Tue, 9 Mar 2021 14:14:28 +0200 Subject: [PATCH 06/17] added fee free accounts to tests --- core/lib/config/src/configs/api.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/core/lib/config/src/configs/api.rs b/core/lib/config/src/configs/api.rs index 4b46e0b9a9..8ef683d1be 100644 --- a/core/lib/config/src/configs/api.rs +++ b/core/lib/config/src/configs/api.rs @@ -193,6 +193,7 @@ mod tests { let config = r#" API_COMMON_CACHES_SIZE="10000" API_COMMON_FORCED_EXIT_MINIMUM_ACCOUNT_AGE_SECS="0" +API_COMMON_FEE_FREE_ACCOUNTS=4078 API_COMMON_ENFORCE_PUBKEY_CHANGE_FEE=true API_COMMON_MAX_NUMBER_OF_TRANSACTIONS_PER_BATCH=200 API_COMMON_MAX_NUMBER_OF_AUTHORS_PER_BATCH=10 From 3c4cff22efc3d0df1c529f8ebd18a3cd338c1626 Mon Sep 17 00:00:00 2001 From: Ihor Barenblat Date: Wed, 10 Mar 2021 08:05:12 +0200 Subject: [PATCH 07/17] Fixed comment in test_fee_free_account --- core/bin/zksync_api/src/api_server/rest/v1/transactions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs b/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs index 7b7a88e97f..2be73c0542 100644 --- a/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs +++ b/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs @@ -786,7 +786,7 @@ mod tests { /// This test checks the following: /// - /// Attempt by fee free account to pay zero fee in single tx or txs batch. + /// Attempt by fee free account to pay zero fee in single tx. async fn test_fee_free_account() -> anyhow::Result<()> { let (client, server) = TestServer::new().await?; From 99117ce3cf9deb08f9e788f100704276e8b7caa8 Mon Sep 17 00:00:00 2001 From: Ihor Barenblat Date: Wed, 10 Mar 2021 13:18:36 +0200 Subject: [PATCH 08/17] removed test fee free account from env --- .../src/api_server/rest/v1/transactions.rs | 7 +- .../zksync_api/src/api_server/tx_sender.rs | 69 ++++++++++--------- core/lib/config/src/configs/api.rs | 4 +- etc/env/base/api.toml | 2 +- 4 files changed, 44 insertions(+), 38 deletions(-) diff --git a/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs b/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs index 2be73c0542..85424fdd20 100644 --- a/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs +++ b/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs @@ -443,7 +443,12 @@ mod tests { async fn new() -> anyhow::Result<(Client, Self)> { let (core_client, core_server) = submit_txs_loopback(); - let cfg = TestServerConfig::default(); + let mut cfg = TestServerConfig::default(); + cfg.config + .api + .common + .fee_free_accounts + .push(AccountId(0xfee)); let pool = cfg.pool.clone(); cfg.fill_database().await?; diff --git a/core/bin/zksync_api/src/api_server/tx_sender.rs b/core/bin/zksync_api/src/api_server/tx_sender.rs index 38697d2bfb..31bca6a785 100644 --- a/core/bin/zksync_api/src/api_server/tx_sender.rs +++ b/core/bin/zksync_api/src/api_server/tx_sender.rs @@ -225,49 +225,50 @@ impl TxSender { .get_ethereum_sign_message(token.clone()) .map(String::into_bytes); - let tx_fee_info = tx.get_fee_info(); + let is_whitelisted_initiator = tx + .account_id() + .map(|account_id| self.fee_free_accounts.contains(&account_id)) + .unwrap_or_default(); + + let tx_fee_info = if !is_whitelisted_initiator { + tx.get_fee_info() + } else { + None + }; let sign_verify_channel = self.sign_verify_requests.clone(); let ticker_request_sender = self.ticker_requests.clone(); if let Some((tx_type, token, address, provided_fee)) = tx_fee_info { - let is_whitelisted_initiator = tx - .account_id() - .map(|account_id| self.fee_free_accounts.contains(&account_id)) - .unwrap_or_default(); + let should_enforce_fee = !matches!(tx_type, TxFeeTypes::ChangePubKey { .. }) + || self.enforce_pubkey_change_fee; - if !is_whitelisted_initiator { - let should_enforce_fee = !matches!(tx_type, TxFeeTypes::ChangePubKey { .. }) - || self.enforce_pubkey_change_fee; + let fee_allowed = + Self::token_allowed_for_fees(ticker_request_sender.clone(), token.clone()).await?; - let fee_allowed = - Self::token_allowed_for_fees(ticker_request_sender.clone(), token.clone()) - .await?; + if !fee_allowed { + return Err(SubmitError::InappropriateFeeToken); + } - if !fee_allowed { - return Err(SubmitError::InappropriateFeeToken); - } + let required_fee = + Self::ticker_request(ticker_request_sender, tx_type, address, token.clone()) + .await?; + // Converting `BitUint` to `BigInt` is safe. + let required_fee: BigDecimal = required_fee.total_fee.to_bigint().unwrap().into(); + let provided_fee: BigDecimal = provided_fee.to_bigint().unwrap().into(); + // Scaling the fee required since the price may change between signing the transaction and sending it to the server. + let scaled_provided_fee = scale_user_fee_up(provided_fee.clone()); + if required_fee >= scaled_provided_fee && should_enforce_fee { + vlog::error!( + "User provided fee is too low, required: {}, provided: {} (scaled: {}); difference {}, token: {:?}", + required_fee.to_string(), + provided_fee.to_string(), + scaled_provided_fee.to_string(), + (&required_fee - &scaled_provided_fee).to_string(), + token + ); - let required_fee = - Self::ticker_request(ticker_request_sender, tx_type, address, token.clone()) - .await?; - // Converting `BitUint` to `BigInt` is safe. - let required_fee: BigDecimal = required_fee.total_fee.to_bigint().unwrap().into(); - let provided_fee: BigDecimal = provided_fee.to_bigint().unwrap().into(); - // Scaling the fee required since the price may change between signing the transaction and sending it to the server. - let scaled_provided_fee = scale_user_fee_up(provided_fee.clone()); - if required_fee >= scaled_provided_fee && should_enforce_fee { - vlog::error!( - "User provided fee is too low, required: {}, provided: {} (scaled: {}); difference {}, token: {:?}", - required_fee.to_string(), - provided_fee.to_string(), - scaled_provided_fee.to_string(), - (&required_fee - &scaled_provided_fee).to_string(), - token - ); - - return Err(SubmitError::TxAdd(TxAddError::TxFeeTooLow)); - } + return Err(SubmitError::TxAdd(TxAddError::TxFeeTooLow)); } } diff --git a/core/lib/config/src/configs/api.rs b/core/lib/config/src/configs/api.rs index 8ef683d1be..169d4c60ec 100644 --- a/core/lib/config/src/configs/api.rs +++ b/core/lib/config/src/configs/api.rs @@ -158,7 +158,7 @@ mod tests { enforce_pubkey_change_fee: true, max_number_of_transactions_per_batch: 200, max_number_of_authors_per_batch: 10, - fee_free_accounts: vec![AccountId(4078)], + fee_free_accounts: vec![], }, admin: AdminApi { port: 8080, @@ -193,7 +193,7 @@ mod tests { let config = r#" API_COMMON_CACHES_SIZE="10000" API_COMMON_FORCED_EXIT_MINIMUM_ACCOUNT_AGE_SECS="0" -API_COMMON_FEE_FREE_ACCOUNTS=4078 +API_COMMON_FEE_FREE_ACCOUNTS= API_COMMON_ENFORCE_PUBKEY_CHANGE_FEE=true API_COMMON_MAX_NUMBER_OF_TRANSACTIONS_PER_BATCH=200 API_COMMON_MAX_NUMBER_OF_AUTHORS_PER_BATCH=10 diff --git a/etc/env/base/api.toml b/etc/env/base/api.toml index 396dd10a01..8aacc73472 100644 --- a/etc/env/base/api.toml +++ b/etc/env/base/api.toml @@ -10,7 +10,7 @@ caches_size=10000 # Type of value is seconds. forced_exit_minimum_account_age_secs=0 # List of account ids that are not paying fees on operations. -fee_free_accounts=[4078] +fee_free_accounts=[] # Ability to perform change pub key with zero fee enforce_pubkey_change_fee=true From 69cc231355d6facd74a1016d9396ca313712a055 Mon Sep 17 00:00:00 2001 From: Ihor Barenblat Date: Wed, 10 Mar 2021 17:33:13 +0200 Subject: [PATCH 09/17] added fee_free_accounts to expected_config test --- core/lib/config/src/configs/api.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/lib/config/src/configs/api.rs b/core/lib/config/src/configs/api.rs index 169d4c60ec..d0354776ae 100644 --- a/core/lib/config/src/configs/api.rs +++ b/core/lib/config/src/configs/api.rs @@ -158,7 +158,7 @@ mod tests { enforce_pubkey_change_fee: true, max_number_of_transactions_per_batch: 200, max_number_of_authors_per_batch: 10, - fee_free_accounts: vec![], + fee_free_accounts: vec![AccountId(4078), AccountId(387)], }, admin: AdminApi { port: 8080, @@ -193,7 +193,7 @@ mod tests { let config = r#" API_COMMON_CACHES_SIZE="10000" API_COMMON_FORCED_EXIT_MINIMUM_ACCOUNT_AGE_SECS="0" -API_COMMON_FEE_FREE_ACCOUNTS= +API_COMMON_FEE_FREE_ACCOUNTS=4078,387 API_COMMON_ENFORCE_PUBKEY_CHANGE_FEE=true API_COMMON_MAX_NUMBER_OF_TRANSACTIONS_PER_BATCH=200 API_COMMON_MAX_NUMBER_OF_AUTHORS_PER_BATCH=10 From 276f11b05a91996402f85e5f66919ede58afdc68 Mon Sep 17 00:00:00 2001 From: Ihor Barenblat Date: Fri, 12 Mar 2021 10:25:55 +0200 Subject: [PATCH 10/17] TODO: ZKS-561 --- core/bin/zksync_api/src/api_server/rest/v1/transactions.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs b/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs index 85424fdd20..4c0c8b9bc0 100644 --- a/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs +++ b/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs @@ -487,6 +487,7 @@ mod tests { ignore = "Use `zk test rust-api` command to perform this test" )] async fn test_rust_api() -> anyhow::Result<()> { + // TODO: ZKS-561 test_transactions_scope().await?; test_bad_fee_token().await?; test_fast_processing_flag().await?; From 28a4ae782dc0b04f4b7012d2ebb4e4b553d1c761 Mon Sep 17 00:00:00 2001 From: Ihor Barenblat Date: Fri, 12 Mar 2021 11:16:53 +0200 Subject: [PATCH 11/17] test_fee_free_accounts fix --- .../src/api_server/rest/v1/transactions.rs | 52 ++++++++++++++----- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs b/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs index 4c0c8b9bc0..694acf2be2 100644 --- a/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs +++ b/core/bin/zksync_api/src/api_server/rest/v1/transactions.rs @@ -491,7 +491,7 @@ mod tests { test_transactions_scope().await?; test_bad_fee_token().await?; test_fast_processing_flag().await?; - test_fee_free_account().await?; + test_fee_free_accounts().await?; Ok(()) } @@ -792,34 +792,62 @@ mod tests { /// This test checks the following: /// - /// Attempt by fee free account to pay zero fee in single tx. - async fn test_fee_free_account() -> anyhow::Result<()> { + /// Fee free account can pay zero fee in single tx. + /// Not a fee free account can't pay zero fee in single tx. + async fn test_fee_free_accounts() -> anyhow::Result<()> { let (client, server) = TestServer::new().await?; - let from = ZkSyncAccount::rand(); - from.set_account_id(Some(AccountId(0xfee))); - let to = ZkSyncAccount::rand(); + let from1 = ZkSyncAccount::rand(); + from1.set_account_id(Some(AccountId(0xfee))); + let to1 = ZkSyncAccount::rand(); // Submit transaction with a zero fee by the fee free account - let (tx, eth_sig) = from.sign_transfer( + let (tx1, eth_sig1) = from1.sign_transfer( TokenId(0), "ETH", 0u64.into(), 0u64.into(), - &to.address, + &to1.address, Some(Nonce(0)), false, Default::default(), ); - let transfer = ZkSyncTx::Transfer(Box::new(tx)); + let transfer1 = ZkSyncTx::Transfer(Box::new(tx1)); client .submit_tx( - transfer.clone(), - eth_sig.map(TxEthSignature::EthereumSignature), + transfer1.clone(), + eth_sig1.map(TxEthSignature::EthereumSignature), + None, + ) + .await + .expect("fee free account transaction fails"); + + let from2 = ZkSyncAccount::rand(); + from2.set_account_id(Some(AccountId(0xbee))); + let to2 = ZkSyncAccount::rand(); + + // Submit transaction with a zero fee not by the fee free account + let (tx2, eth_sig2) = from2.sign_transfer( + TokenId(0), + "ETH", + 0u64.into(), + 0u64.into(), + &to2.address, + Some(Nonce(0)), + false, + Default::default(), + ); + let transfer2 = ZkSyncTx::Transfer(Box::new(tx2)); + client + .submit_tx( + transfer2.clone(), + eth_sig2.map(TxEthSignature::EthereumSignature), None, ) .await - .unwrap(); + .unwrap_err() + .to_string() + .contains("Transaction fee is too low"); server.stop().await; Ok(()) From f243fd9784a5ac422316e359706bc11d33180871 Mon Sep 17 00:00:00 2001 From: Ihor Barenblat <57619852+Barichek@users.noreply.github.com> Date: Fri, 12 Mar 2021 11:18:19 +0200 Subject: [PATCH 12/17] Update core/lib/config/src/configs/api.rs Co-authored-by: Igor Aleksanov --- core/lib/config/src/configs/api.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/lib/config/src/configs/api.rs b/core/lib/config/src/configs/api.rs index d0354776ae..c24b94cbf4 100644 --- a/core/lib/config/src/configs/api.rs +++ b/core/lib/config/src/configs/api.rs @@ -48,7 +48,7 @@ pub struct Common { // Determines the required minimum account age for `ForcedExit` operation to be allowed. // Type of value is seconds. pub forced_exit_minimum_account_age_secs: u64, - // List of account ids that are not paying fees on operations. + /// List of account IDs that do not have to pay fees for operations. pub fee_free_accounts: Vec, pub enforce_pubkey_change_fee: bool, From 522c06d10c8e027b7148318fde886abdfbfb35c6 Mon Sep 17 00:00:00 2001 From: Ihor Barenblat <57619852+Barichek@users.noreply.github.com> Date: Fri, 12 Mar 2021 11:18:36 +0200 Subject: [PATCH 13/17] Update etc/env/base/api.toml Co-authored-by: Igor Aleksanov --- etc/env/base/api.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/etc/env/base/api.toml b/etc/env/base/api.toml index 8aacc73472..127308c91d 100644 --- a/etc/env/base/api.toml +++ b/etc/env/base/api.toml @@ -9,7 +9,7 @@ caches_size=10000 # value at least 24 hours for production. # Type of value is seconds. forced_exit_minimum_account_age_secs=0 -# List of account ids that are not paying fees on operations. +# List of account IDs that do not have to pay fees for operations. fee_free_accounts=[] # Ability to perform change pub key with zero fee From 76a4fe2fd3f51a2e167124d7e0ac03d3368c1ed8 Mon Sep 17 00:00:00 2001 From: Ihor Barenblat Date: Fri, 12 Mar 2021 11:19:28 +0200 Subject: [PATCH 14/17] unwrap_or(false) --- core/bin/zksync_api/src/api_server/tx_sender.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/bin/zksync_api/src/api_server/tx_sender.rs b/core/bin/zksync_api/src/api_server/tx_sender.rs index 31bca6a785..bbf01c9089 100644 --- a/core/bin/zksync_api/src/api_server/tx_sender.rs +++ b/core/bin/zksync_api/src/api_server/tx_sender.rs @@ -228,7 +228,7 @@ impl TxSender { let is_whitelisted_initiator = tx .account_id() .map(|account_id| self.fee_free_accounts.contains(&account_id)) - .unwrap_or_default(); + .unwrap_or(false); let tx_fee_info = if !is_whitelisted_initiator { tx.get_fee_info() From 5317836191b978a87cbec7e2e6e751a29f327cad Mon Sep 17 00:00:00 2001 From: Ihor Barenblat Date: Fri, 12 Mar 2021 11:20:19 +0200 Subject: [PATCH 15/17] reducant clone --- core/bin/zksync_api/src/api_server/tx_sender.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/bin/zksync_api/src/api_server/tx_sender.rs b/core/bin/zksync_api/src/api_server/tx_sender.rs index bbf01c9089..eef6485cc6 100644 --- a/core/bin/zksync_api/src/api_server/tx_sender.rs +++ b/core/bin/zksync_api/src/api_server/tx_sender.rs @@ -148,7 +148,7 @@ impl TxSender { enforce_pubkey_change_fee: config.api.common.enforce_pubkey_change_fee, forced_exit_minimum_account_age, - fee_free_accounts: HashSet::from_iter(config.api.common.fee_free_accounts.clone()), + fee_free_accounts: HashSet::from_iter(config.api.common.fee_free_accounts), max_number_of_transactions_per_batch, max_number_of_authors_per_batch, } From f98a58e49ba2eb1a6c5af9f2b6380af408206c3f Mon Sep 17 00:00:00 2001 From: Ihor Barenblat <57619852+Barichek@users.noreply.github.com> Date: Fri, 12 Mar 2021 11:21:55 +0200 Subject: [PATCH 16/17] Update core/bin/zksync_api/src/api_server/tx_sender.rs Co-authored-by: Igor Aleksanov --- core/bin/zksync_api/src/api_server/tx_sender.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/bin/zksync_api/src/api_server/tx_sender.rs b/core/bin/zksync_api/src/api_server/tx_sender.rs index eef6485cc6..6cca86b0b7 100644 --- a/core/bin/zksync_api/src/api_server/tx_sender.rs +++ b/core/bin/zksync_api/src/api_server/tx_sender.rs @@ -45,7 +45,7 @@ pub struct TxSender { pub tokens: TokenDBCache, /// Mimimum age of the account for `ForcedExit` operations to be allowed. pub forced_exit_minimum_account_age: chrono::Duration, - /// List of account ids that are not paying fees on operations. + /// List of account IDs that do not have to pay fees for operations. pub fee_free_accounts: HashSet, pub enforce_pubkey_change_fee: bool, // Limit the number of both transactions and Ethereum signatures per batch. From 8a41a00c6a46cb3bc6e92ce61d9146c6d75f4591 Mon Sep 17 00:00:00 2001 From: Ihor Barenblat Date: Fri, 12 Mar 2021 19:47:56 +0200 Subject: [PATCH 17/17] clone for config.api.common.fee_free_accounts --- core/bin/zksync_api/src/api_server/tx_sender.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/bin/zksync_api/src/api_server/tx_sender.rs b/core/bin/zksync_api/src/api_server/tx_sender.rs index 6cca86b0b7..e9b9c70dd6 100644 --- a/core/bin/zksync_api/src/api_server/tx_sender.rs +++ b/core/bin/zksync_api/src/api_server/tx_sender.rs @@ -148,7 +148,7 @@ impl TxSender { enforce_pubkey_change_fee: config.api.common.enforce_pubkey_change_fee, forced_exit_minimum_account_age, - fee_free_accounts: HashSet::from_iter(config.api.common.fee_free_accounts), + fee_free_accounts: HashSet::from_iter(config.api.common.fee_free_accounts.clone()), max_number_of_transactions_per_batch, max_number_of_authors_per_batch, }