Skip to content

Commit

Permalink
feat: Remove TPS limiter from TX Sender (#793)
Browse files Browse the repository at this point in the history
## What ❔

Removes TPS Limiter from TX Sender component.

## Why ❔

- It didn't work properly: limit is applied per API server, and API
servers are horizontally scalable.
- It doesn't seem to be configured in any of our envs.
- It doesn't really make sense in context of ENs.
- We have other mempool protection measures anyways (tx dry-run, nonce
limits, etc).

## Checklist

<!-- Check your PR fulfills the following items. -->
<!-- For draft PRs check the boxes as you complete them. -->

- [ ] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [ ] Tests for the changes have been added / updated.
- [ ] Documentation comments have been added / updated.
- [ ] Code has been formatted via `zk fmt` and `zk lint`.
- [ ] Spellcheck has been run via `cargo spellcheck
--cfg=./spellcheck/era.cfg --code 1`.
  • Loading branch information
popzxc authored Jan 2, 2024
1 parent b309820 commit d0e9296
Show file tree
Hide file tree
Showing 6 changed files with 6 additions and 50 deletions.
2 changes: 1 addition & 1 deletion core/bin/external_node/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ pub struct OptionalENConfig {
/// Max number of cache misses during one VM execution. If the number of cache misses exceeds this value, the API server panics.
/// This is a temporary solution to mitigate API request resulting in thousands of DB queries.
pub vm_execution_cache_misses_limit: Option<usize>,
/// Inbound transaction limit used for throttling.
/// Note: Deprecated option, no longer in use. Left to display a warning in case someone used them.
pub transactions_per_sec_limit: Option<u32>,
/// Limit for fee history block range.
#[serde(default = "OptionalENConfig::default_fee_history_limit")]
Expand Down
7 changes: 3 additions & 4 deletions core/bin/external_node/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,14 +233,13 @@ async fn init_tasks(
let gas_adjuster_handle = tokio::spawn(gas_adjuster.clone().run(stop_receiver.clone()));

let (tx_sender, vm_barrier, cache_update_handle) = {
let mut tx_sender_builder =
let tx_sender_builder =
TxSenderBuilder::new(config.clone().into(), connection_pool.clone())
.with_main_connection_pool(connection_pool.clone())
.with_tx_proxy(&main_node_url);

// Add rate limiter if enabled.
if let Some(tps_limit) = config.optional.transactions_per_sec_limit {
tx_sender_builder = tx_sender_builder.with_rate_limiter(tps_limit);
if config.optional.transactions_per_sec_limit.is_some() {
tracing::warn!("`transactions_per_sec_limit` option is deprecated and ignored");
};

let max_concurrency = config.optional.vm_concurrency_limit;
Expand Down
3 changes: 0 additions & 3 deletions core/lib/config/src/configs/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ pub struct Web3JsonRpcConfig {
/// The multiplier to use when suggesting gas price. Should be higher than one,
/// otherwise if the L1 prices soar, the suggested gas price won't be sufficient to be included in block
pub gas_price_scale_factor: f64,
/// Inbound transaction limit used for throttling
pub transactions_per_sec_limit: Option<u32>,
/// Timeout for requests (in s)
pub request_timeout: Option<u64>,
/// Private keys for accounts managed by node
Expand Down Expand Up @@ -108,7 +106,6 @@ impl Web3JsonRpcConfig {
threads_per_server: 1,
max_nonce_ahead: 50,
gas_price_scale_factor: 1.2,
transactions_per_sec_limit: Default::default(),
request_timeout: Default::default(),
account_pks: Default::default(),
estimate_gas_scale_factor: 1.2,
Expand Down
2 changes: 0 additions & 2 deletions core/lib/env_config/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ mod tests {
pubsub_polling_interval: Some(200),
threads_per_server: 128,
max_nonce_ahead: 5,
transactions_per_sec_limit: Some(1000),
request_timeout: Some(10),
account_pks: Some(vec![
hash("0x0000000000000000000000000000000000000000000000000000000000000001"),
Expand Down Expand Up @@ -121,7 +120,6 @@ mod tests {
API_WEB3_JSON_RPC_THREADS_PER_SERVER=128
API_WEB3_JSON_RPC_MAX_NONCE_AHEAD=5
API_WEB3_JSON_RPC_GAS_PRICE_SCALE_FACTOR=1.2
API_WEB3_JSON_RPC_TRANSACTIONS_PER_SEC_LIMIT=1000
API_WEB3_JSON_RPC_REQUEST_TIMEOUT=10
API_WEB3_JSON_RPC_ACCOUNT_PKS="0x0000000000000000000000000000000000000000000000000000000000000001,0x0000000000000000000000000000000000000000000000000000000000000002"
API_WEB3_JSON_RPC_ESTIMATE_GAS_SCALE_FACTOR=1.0
Expand Down
35 changes: 1 addition & 34 deletions core/lib/zksync_core/src/api_server/tx_sender/mod.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,7 @@
//! Helper module to submit transactions into the zkSync Network.
use std::{cmp, num::NonZeroU32, sync::Arc, time::Instant};
use std::{cmp, sync::Arc, time::Instant};

use governor::{
clock::MonotonicClock,
middleware::NoOpMiddleware,
state::{InMemoryState, NotKeyed},
Quota, RateLimiter,
};
use multivm::{
interface::VmExecutionResultAndLogs,
vm_latest::{
Expand Down Expand Up @@ -51,10 +45,6 @@ use crate::{
mod proxy;
mod result;

/// Type alias for the rate limiter implementation.
type TxSenderRateLimiter =
RateLimiter<NotKeyed, InMemoryState, MonotonicClock, NoOpMiddleware<Instant>>;

#[derive(Debug, Clone)]
pub struct MultiVMBaseSystemContracts {
/// Contracts to be used for pre-virtual-blocks protocol versions.
Expand Down Expand Up @@ -140,8 +130,6 @@ pub struct TxSenderBuilder {
replica_connection_pool: ConnectionPool,
/// Connection pool for write requests. If not set, `proxy` must be set.
master_connection_pool: Option<ConnectionPool>,
/// Rate limiter for tx submissions.
rate_limiter: Option<TxSenderRateLimiter>,
/// Proxy to submit transactions to the network. If not set, `master_connection_pool` must be set.
proxy: Option<TxProxy>,
/// Actual state keeper configuration, required for tx verification.
Expand All @@ -155,23 +143,11 @@ impl TxSenderBuilder {
config,
replica_connection_pool,
master_connection_pool: None,
rate_limiter: None,
proxy: None,
state_keeper_config: None,
}
}

pub fn with_rate_limiter(self, transactions_per_sec: u32) -> Self {
let rate_limiter = RateLimiter::direct_with_clock(
Quota::per_second(NonZeroU32::new(transactions_per_sec).unwrap()),
&MonotonicClock,
);
Self {
rate_limiter: Some(rate_limiter),
..self
}
}

pub fn with_tx_proxy(mut self, main_node_url: &str) -> Self {
self.proxy = Some(TxProxy::new(main_node_url));
self
Expand Down Expand Up @@ -205,7 +181,6 @@ impl TxSenderBuilder {
replica_connection_pool: self.replica_connection_pool,
l1_gas_price_source,
api_contracts,
rate_limiter: self.rate_limiter,
proxy: self.proxy,
state_keeper_config: self.state_keeper_config,
vm_concurrency_limiter,
Expand Down Expand Up @@ -257,8 +232,6 @@ pub struct TxSenderInner {
// Used to keep track of gas prices for the fee ticker.
pub l1_gas_price_source: Arc<dyn L1GasPriceProvider>,
pub(super) api_contracts: ApiContracts,
/// Optional rate limiter that will limit the amount of transactions per second sent from a single entity.
rate_limiter: Option<TxSenderRateLimiter>,
/// Optional transaction proxy to be used for transaction submission.
pub(super) proxy: Option<TxProxy>,
/// An up-to-date version of the state keeper config.
Expand Down Expand Up @@ -291,12 +264,6 @@ impl TxSender {

#[tracing::instrument(skip(self, tx))]
pub async fn submit_tx(&self, tx: L2Tx) -> Result<L2TxSubmissionResult, SubmitTxError> {
if let Some(rate_limiter) = &self.0.rate_limiter {
if rate_limiter.check().is_err() {
return Err(SubmitTxError::RateLimitExceeded);
}
}

let stage_latency = SANDBOX_METRICS.submit_tx[&SubmitTxStage::Validate].start();
self.validate_tx(&tx).await?;
stage_latency.observe();
Expand Down
7 changes: 1 addition & 6 deletions core/lib/zksync_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1033,15 +1033,10 @@ async fn build_tx_sender(
l1_gas_price_provider: Arc<dyn L1GasPriceProvider>,
storage_caches: PostgresStorageCaches,
) -> (TxSender, VmConcurrencyBarrier) {
let mut tx_sender_builder = TxSenderBuilder::new(tx_sender_config.clone(), replica_pool)
let tx_sender_builder = TxSenderBuilder::new(tx_sender_config.clone(), replica_pool)
.with_main_connection_pool(master_pool)
.with_state_keeper_config(state_keeper_config.clone());

// Add rate limiter if enabled.
if let Some(transactions_per_sec_limit) = web3_json_config.transactions_per_sec_limit {
tx_sender_builder = tx_sender_builder.with_rate_limiter(transactions_per_sec_limit);
};

let max_concurrency = web3_json_config.vm_concurrency_limit();
let (vm_concurrency_limiter, vm_barrier) = VmConcurrencyLimiter::new(max_concurrency);

Expand Down

0 comments on commit d0e9296

Please sign in to comment.