Skip to content

Commit

Permalink
feat: Remove generic bounds on L1GasPriceProvider (#792)
Browse files Browse the repository at this point in the history
## What ❔

- Replaces uses of `G: L1GasPriceProvider` with `dyn
L1GasPriceProvider`.
- ...as a result, `StateKeeper` and `TxSender` (and thus all the API
structures) are now generic-free.

## Why ❔

- Prerequisite for modular system (generic types are hard for
dynamically-configured setup).
- Invoking `L1GasPriceProvider` is not related to any hot loops where
dynamic dispatch would be noticeable, so having it as a generic is kinda
meaningless anyways.

## 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 committed Jan 2, 2024
1 parent 88f1b35 commit edf071d
Show file tree
Hide file tree
Showing 22 changed files with 103 additions and 191 deletions.
5 changes: 3 additions & 2 deletions core/lib/eth_client/src/clients/mock.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::{
collections::{BTreeMap, HashMap},
fmt,
sync::{
atomic::{AtomicU64, Ordering},
RwLock,
Expand Down Expand Up @@ -415,7 +416,7 @@ impl BoundEthInterface for MockEthereum {
}

#[async_trait]
impl<T: AsRef<MockEthereum> + Send + Sync> EthInterface for T {
impl<T: 'static + AsRef<MockEthereum> + Send + Sync + fmt::Debug> EthInterface for T {
async fn nonce_at_for_account(
&self,
account: Address,
Expand Down Expand Up @@ -535,7 +536,7 @@ impl<T: AsRef<MockEthereum> + Send + Sync> EthInterface for T {
}

#[async_trait::async_trait]
impl<T: AsRef<MockEthereum> + Send + Sync> BoundEthInterface for T {
impl<T: 'static + AsRef<MockEthereum> + Send + Sync + fmt::Debug> BoundEthInterface for T {
fn contract(&self) -> &ethabi::Contract {
self.as_ref().contract()
}
Expand Down
4 changes: 3 additions & 1 deletion core/lib/eth_client/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#![allow(clippy::upper_case_acronyms, clippy::derive_partial_eq_without_eq)]

use std::fmt;

use async_trait::async_trait;
use zksync_types::{
web3::{
Expand Down Expand Up @@ -38,7 +40,7 @@ pub mod types;
/// unnecessary high amount of Web3 calls. Implementations are advice to count invocations
/// per component and expose them to Prometheus.
#[async_trait]
pub trait EthInterface: Sync + Send {
pub trait EthInterface: 'static + Sync + Send + fmt::Debug {
/// Returns the nonce of the provided account at the specified block.
async fn nonce_at_for_account(
&self,
Expand Down
2 changes: 1 addition & 1 deletion core/lib/eth_signer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub mod pk_signer;
pub mod raw_ethereum_tx;

#[async_trait]
pub trait EthereumSigner: Send + Sync + Clone {
pub trait EthereumSigner: 'static + Send + Sync + Clone {
async fn sign_message(&self, message: &[u8]) -> Result<PackedEthSignature, SignerError>;
async fn sign_typed_data<S: EIP712TypedStructure + Sync>(
&self,
Expand Down
26 changes: 9 additions & 17 deletions core/lib/zksync_core/src/api_server/tx_sender/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,13 +187,13 @@ impl TxSenderBuilder {
self
}

pub async fn build<G: L1GasPriceProvider>(
pub async fn build(
self,
l1_gas_price_source: Arc<G>,
l1_gas_price_source: Arc<dyn L1GasPriceProvider>,
vm_concurrency_limiter: Arc<VmConcurrencyLimiter>,
api_contracts: ApiContracts,
storage_caches: PostgresStorageCaches,
) -> TxSender<G> {
) -> TxSender {
assert!(
self.master_connection_pool.is_some() || self.proxy.is_some(),
"Either master connection pool or proxy must be set"
Expand Down Expand Up @@ -250,12 +250,12 @@ impl TxSenderConfig {
}
}

pub struct TxSenderInner<G> {
pub struct TxSenderInner {
pub(super) sender_config: TxSenderConfig,
pub master_connection_pool: Option<ConnectionPool>,
pub replica_connection_pool: ConnectionPool,
// Used to keep track of gas prices for the fee ticker.
pub l1_gas_price_source: Arc<G>,
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>,
Expand All @@ -271,24 +271,16 @@ pub struct TxSenderInner<G> {
storage_caches: PostgresStorageCaches,
}

pub struct TxSender<G>(pub(super) Arc<TxSenderInner<G>>);
#[derive(Clone)]
pub struct TxSender(pub(super) Arc<TxSenderInner>);

// Custom implementation is required due to generic param:
// Even though it's under `Arc`, compiler doesn't generate the `Clone` implementation unless
// an unnecessary bound is added.
impl<G> Clone for TxSender<G> {
fn clone(&self) -> Self {
Self(self.0.clone())
}
}

impl<G> std::fmt::Debug for TxSender<G> {
impl std::fmt::Debug for TxSender {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("TxSender").finish()
}
}

impl<G: L1GasPriceProvider> TxSender<G> {
impl TxSender {
pub(crate) fn vm_concurrency_limiter(&self) -> Arc<VmConcurrencyLimiter> {
Arc::clone(&self.0.vm_concurrency_limiter)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,10 @@ use zksync_web3_decl::{
namespaces::en::EnNamespaceServer,
};

use crate::{
api_server::web3::{backend_jsonrpsee::into_jsrpc_error, namespaces::EnNamespace},
l1_gas_price::L1GasPriceProvider,
};
use crate::api_server::web3::{backend_jsonrpsee::into_jsrpc_error, namespaces::EnNamespace};

#[async_trait]
impl<G: L1GasPriceProvider + Send + Sync + 'static> EnNamespaceServer for EnNamespace<G> {
impl EnNamespaceServer for EnNamespace {
async fn sync_l2_block(
&self,
block_number: MiniblockNumber,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,10 @@ use zksync_web3_decl::{
types::{Filter, FilterChanges},
};

use crate::{
api_server::web3::{backend_jsonrpsee::into_jsrpc_error, EthNamespace},
l1_gas_price::L1GasPriceProvider,
};
use crate::api_server::web3::{backend_jsonrpsee::into_jsrpc_error, EthNamespace};

#[async_trait]
impl<G: L1GasPriceProvider + Send + Sync + 'static> EthNamespaceServer for EthNamespace<G> {
impl EthNamespaceServer for EthNamespace {
async fn get_block_number(&self) -> RpcResult<U64> {
self.get_block_number_impl().await.map_err(into_jsrpc_error)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,12 @@ use zksync_types::{
};
use zksync_web3_decl::{jsonrpsee::core::RpcResult, namespaces::SnapshotsNamespaceServer};

use crate::{
api_server::web3::{backend_jsonrpsee::into_jsrpc_error, namespaces::SnapshotsNamespace},
l1_gas_price::L1GasPriceProvider,
use crate::api_server::web3::{
backend_jsonrpsee::into_jsrpc_error, namespaces::SnapshotsNamespace,
};

#[async_trait]
impl<G: L1GasPriceProvider + Send + Sync + 'static> SnapshotsNamespaceServer
for SnapshotsNamespace<G>
{
impl SnapshotsNamespaceServer for SnapshotsNamespace {
async fn get_all_snapshots(&self) -> RpcResult<AllSnapshots> {
self.get_all_snapshots_impl()
.await
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,10 @@ use zksync_web3_decl::{
types::Token,
};

use crate::{
api_server::web3::{backend_jsonrpsee::into_jsrpc_error, ZksNamespace},
l1_gas_price::L1GasPriceProvider,
};
use crate::api_server::web3::{backend_jsonrpsee::into_jsrpc_error, ZksNamespace};

#[async_trait]
impl<G: L1GasPriceProvider + Send + Sync + 'static> ZksNamespaceServer for ZksNamespace<G> {
impl ZksNamespaceServer for ZksNamespace {
async fn estimate_fee(&self, req: CallRequest) -> RpcResult<Fee> {
self.estimate_fee_impl(req).await.map_err(into_jsrpc_error)
}
Expand Down
25 changes: 10 additions & 15 deletions core/lib/zksync_core/src/api_server/web3/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ use crate::{
execution_sandbox::VmConcurrencyBarrier, tree::TreeApiHttpClient, tx_sender::TxSender,
web3::backend_jsonrpsee::batch_limiter_middleware::LimitMiddleware,
},
l1_gas_price::L1GasPriceProvider,
sync_layer::SyncState,
};

Expand Down Expand Up @@ -119,12 +118,12 @@ struct OptionalApiParams {

/// Full API server parameters.
#[derive(Debug)]
struct FullApiParams<G> {
struct FullApiParams {
pool: ConnectionPool,
last_miniblock_pool: ConnectionPool,
config: InternalApiConfig,
transport: ApiTransport,
tx_sender: TxSender<G>,
tx_sender: TxSender,
vm_barrier: VmConcurrencyBarrier,
threads: usize,
polling_interval: Duration,
Expand All @@ -133,14 +132,14 @@ struct FullApiParams<G> {
}

#[derive(Debug)]
pub struct ApiBuilder<G> {
pub struct ApiBuilder {
pool: ConnectionPool,
last_miniblock_pool: ConnectionPool,
config: InternalApiConfig,
polling_interval: Duration,
// Mandatory params that must be set using builder methods.
transport: Option<ApiTransport>,
tx_sender: Option<TxSender<G>>,
tx_sender: Option<TxSender>,
vm_barrier: Option<VmConcurrencyBarrier>,
threads: Option<usize>,
// Optional params that may or may not be set using builder methods. We treat `namespaces`
Expand All @@ -149,7 +148,7 @@ pub struct ApiBuilder<G> {
optional: OptionalApiParams,
}

impl<G> ApiBuilder<G> {
impl ApiBuilder {
const DEFAULT_POLLING_INTERVAL: Duration = Duration::from_millis(200);

pub fn jsonrpsee_backend(config: InternalApiConfig, pool: ConnectionPool) -> Self {
Expand Down Expand Up @@ -185,11 +184,7 @@ impl<G> ApiBuilder<G> {
self
}

pub fn with_tx_sender(
mut self,
tx_sender: TxSender<G>,
vm_barrier: VmConcurrencyBarrier,
) -> Self {
pub fn with_tx_sender(mut self, tx_sender: TxSender, vm_barrier: VmConcurrencyBarrier) -> Self {
self.tx_sender = Some(tx_sender);
self.vm_barrier = Some(vm_barrier);
self
Expand Down Expand Up @@ -255,7 +250,7 @@ impl<G> ApiBuilder<G> {
self
}

fn into_full_params(self) -> anyhow::Result<FullApiParams<G>> {
fn into_full_params(self) -> anyhow::Result<FullApiParams> {
Ok(FullApiParams {
pool: self.pool,
last_miniblock_pool: self.last_miniblock_pool,
Expand All @@ -276,7 +271,7 @@ impl<G> ApiBuilder<G> {
}
}

impl<G: 'static + Send + Sync + L1GasPriceProvider> ApiBuilder<G> {
impl ApiBuilder {
pub async fn build(
self,
stop_receiver: watch::Receiver<bool>,
Expand All @@ -285,8 +280,8 @@ impl<G: 'static + Send + Sync + L1GasPriceProvider> ApiBuilder<G> {
}
}

impl<G: 'static + Send + Sync + L1GasPriceProvider> FullApiParams<G> {
fn build_rpc_state(self) -> RpcState<G> {
impl FullApiParams {
fn build_rpc_state(self) -> RpcState {
// Chosen to be significantly smaller than the interval between miniblocks, but larger than
// the latency of getting the latest sealed miniblock number from Postgres. If the API server
// processes enough requests, information about the latest sealed miniblock will be updated
Expand Down
25 changes: 11 additions & 14 deletions core/lib/zksync_core/src/api_server/web3/namespaces/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,17 @@ use zksync_types::{
};
use zksync_web3_decl::error::Web3Error;

use crate::{
api_server::{
execution_sandbox::{
execute_tx_eth_call, ApiTracer, BlockArgs, TxSharedArgs, VmConcurrencyLimiter,
},
tx_sender::ApiContracts,
web3::{
backend_jsonrpsee::internal_error,
metrics::API_METRICS,
resolve_block,
state::{RpcState, SealedMiniblockNumber},
},
use crate::api_server::{
execution_sandbox::{
execute_tx_eth_call, ApiTracer, BlockArgs, TxSharedArgs, VmConcurrencyLimiter,
},
tx_sender::ApiContracts,
web3::{
backend_jsonrpsee::internal_error,
metrics::API_METRICS,
resolve_block,
state::{RpcState, SealedMiniblockNumber},
},
l1_gas_price::L1GasPriceProvider,
};

#[derive(Debug, Clone)]
Expand All @@ -42,7 +39,7 @@ pub struct DebugNamespace {
}

impl DebugNamespace {
pub async fn new<G: L1GasPriceProvider>(state: RpcState<G>) -> Self {
pub async fn new(state: RpcState) -> Self {
let sender_config = &state.tx_sender.0.sender_config;

let api_contracts = ApiContracts::load_from_disk();
Expand Down
21 changes: 5 additions & 16 deletions core/lib/zksync_core/src/api_server/web3/namespaces/en.rs
Original file line number Diff line number Diff line change
@@ -1,28 +1,17 @@
use zksync_types::{api::en::SyncBlock, MiniblockNumber};
use zksync_web3_decl::error::Web3Error;

use crate::{
api_server::web3::{backend_jsonrpsee::internal_error, state::RpcState},
l1_gas_price::L1GasPriceProvider,
};
use crate::api_server::web3::{backend_jsonrpsee::internal_error, state::RpcState};

/// Namespace for External Node unique methods.
/// Main use case for it is the EN synchronization.
#[derive(Debug)]
pub struct EnNamespace<G> {
pub state: RpcState<G>,
pub struct EnNamespace {
pub state: RpcState,
}

impl<G> Clone for EnNamespace<G> {
fn clone(&self) -> Self {
Self {
state: self.state.clone(),
}
}
}

impl<G: L1GasPriceProvider> EnNamespace<G> {
pub fn new(state: RpcState<G>) -> Self {
impl EnNamespace {
pub fn new(state: RpcState) -> Self {
Self { state }
}

Expand Down
Loading

0 comments on commit edf071d

Please sign in to comment.