Skip to content

Commit

Permalink
fix(api): Fix batch fee input for debug namespace (#2948)
Browse files Browse the repository at this point in the history
## What ❔

Changes the `debug_traceCall` handler to use the fee provider for recent
blocks (pending / latest / committed).

## Why ❔

Previously, the `debug` namespace used a static batch fee input
retrieved at the node initialization. This looks incorrect.

## Checklist

- [x] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [x] Tests for the changes have been added / updated.
- [x] Documentation comments have been added / updated.
- [x] Code has been formatted via `zk fmt` and `zk lint`.
  • Loading branch information
slowli authored Sep 30, 2024
1 parent f4631e4 commit 79b6fcf
Show file tree
Hide file tree
Showing 8 changed files with 168 additions and 114 deletions.
26 changes: 9 additions & 17 deletions core/lib/types/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -754,21 +754,17 @@ pub enum CallTracerBlockResult {
}

impl CallTracerBlockResult {
pub fn unwrap_flatten(self) -> Vec<DebugCallFlat> {
pub fn unwrap_flat(self) -> Vec<DebugCallFlat> {
match self {
Self::CallTrace(_) => {
panic!("Result is a FlatCallTrace")
}
Self::FlatCallTrace(a) => a,
Self::CallTrace(_) => panic!("Result is a FlatCallTrace"),
Self::FlatCallTrace(trace) => trace,
}
}

pub fn unwrap_default(self) -> Vec<ResultDebugCall> {
match self {
Self::CallTrace(a) => a,
Self::FlatCallTrace(_) => {
panic!("Result is a CallTrace")
}
Self::CallTrace(trace) => trace,
Self::FlatCallTrace(_) => panic!("Result is a CallTrace"),
}
}
}
Expand All @@ -783,19 +779,15 @@ pub enum CallTracerResult {
impl CallTracerResult {
pub fn unwrap_flat(self) -> Vec<DebugCallFlat> {
match self {
Self::CallTrace(_) => {
panic!("Result is a FlatCallTrace")
}
Self::FlatCallTrace(a) => a,
Self::CallTrace(_) => panic!("Result is a FlatCallTrace"),
Self::FlatCallTrace(trace) => trace,
}
}

pub fn unwrap_default(self) -> DebugCall {
match self {
Self::CallTrace(a) => a,
Self::FlatCallTrace(_) => {
panic!("Result is a CallTrace")
}
Self::CallTrace(trace) => trace,
Self::FlatCallTrace(_) => panic!("Result is a CallTrace"),
}
}
}
Expand Down
34 changes: 14 additions & 20 deletions core/node/api_server/src/web3/namespaces/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use zksync_types::{
ResultDebugCall, SupportedTracers, TracerConfig,
},
debug_flat_call::{Action, CallResult, DebugCallFlat},
fee_model::BatchFeeInput,
l2::L2Tx,
transaction_request::CallRequest,
web3, H256, U256,
Expand All @@ -22,27 +21,12 @@ use crate::{

#[derive(Debug, Clone)]
pub(crate) struct DebugNamespace {
batch_fee_input: BatchFeeInput,
state: RpcState,
}

impl DebugNamespace {
pub async fn new(state: RpcState) -> anyhow::Result<Self> {
let fee_input_provider = &state.tx_sender.0.batch_fee_input_provider;
// FIXME (PLA-1033): use the fee input provider instead of a constant value
let batch_fee_input = fee_input_provider
.get_batch_fee_input_scaled(
state.api_config.estimate_gas_scale_factor,
state.api_config.estimate_gas_scale_factor,
)
.await
.context("cannot get batch fee input")?;

Ok(Self {
// For now, the same scaling is used for both the L1 gas price and the pubdata price
batch_fee_input,
state,
})
Ok(Self { state })
}

pub(crate) fn map_call(
Expand Down Expand Up @@ -257,12 +241,22 @@ impl DebugNamespace {
if request.gas.is_none() {
request.gas = Some(block_args.default_eth_call_gas(&mut connection).await?);
}

let fee_input = if block_args.resolves_to_latest_sealed_l2_block() {
self.batch_fee_input
// It is important to drop a DB connection before calling the provider, since it acquires a connection internally
// on the main node.
drop(connection);
let scale_factor = self.state.api_config.estimate_gas_scale_factor;
let fee_input_provider = &self.state.tx_sender.0.batch_fee_input_provider;
// For now, the same scaling is used for both the L1 gas price and the pubdata price
fee_input_provider
.get_batch_fee_input_scaled(scale_factor, scale_factor)
.await?
} else {
block_args.historical_fee_input(&mut connection).await?
let fee_input = block_args.historical_fee_input(&mut connection).await?;
drop(connection);
fee_input
};
drop(connection);

let call_overrides = request.get_call_overrides()?;
let call = L2Tx::from_request(request.into(), MAX_ENCODED_TX_SIZE)?;
Expand Down
92 changes: 61 additions & 31 deletions core/node/api_server/src/web3/testonly.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,42 +97,72 @@ impl ApiServerHandles {
}
}

pub async fn spawn_http_server(
api_config: InternalApiConfig,
/// Builder for test server instances.
#[derive(Debug)]
pub struct TestServerBuilder {
pool: ConnectionPool<Core>,
api_config: InternalApiConfig,
tx_executor: MockOneshotExecutor,
method_tracer: Arc<MethodTracer>,
stop_receiver: watch::Receiver<bool>,
) -> ApiServerHandles {
spawn_server(
ApiTransportLabel::Http,
api_config,
pool,
None,
tx_executor,
method_tracer,
stop_receiver,
)
.await
.0
}

pub async fn spawn_ws_server(
api_config: InternalApiConfig,
pool: ConnectionPool<Core>,
stop_receiver: watch::Receiver<bool>,
websocket_requests_per_minute_limit: Option<NonZeroU32>,
) -> (ApiServerHandles, mpsc::UnboundedReceiver<PubSubEvent>) {
spawn_server(
ApiTransportLabel::Ws,
api_config,
pool,
websocket_requests_per_minute_limit,
MockOneshotExecutor::default(),
Arc::default(),
stop_receiver,
)
.await
impl TestServerBuilder {
/// Creates a new builder.
pub fn new(pool: ConnectionPool<Core>, api_config: InternalApiConfig) -> Self {
Self {
api_config,
pool,
tx_executor: MockOneshotExecutor::default(),
method_tracer: Arc::default(),
}
}

/// Sets a transaction / call executor for this builder.
#[must_use]
pub fn with_tx_executor(mut self, tx_executor: MockOneshotExecutor) -> Self {
self.tx_executor = tx_executor;
self
}

/// Sets an RPC method tracer for this builder.
#[must_use]
pub fn with_method_tracer(mut self, tracer: Arc<MethodTracer>) -> Self {
self.method_tracer = tracer;
self
}

/// Builds an HTTP server.
pub async fn build_http(self, stop_receiver: watch::Receiver<bool>) -> ApiServerHandles {
spawn_server(
ApiTransportLabel::Http,
self.api_config,
self.pool,
None,
self.tx_executor,
self.method_tracer,
stop_receiver,
)
.await
.0
}

/// Builds a WS server.
pub async fn build_ws(
self,
websocket_requests_per_minute_limit: Option<NonZeroU32>,
stop_receiver: watch::Receiver<bool>,
) -> (ApiServerHandles, mpsc::UnboundedReceiver<PubSubEvent>) {
spawn_server(
ApiTransportLabel::Ws,
self.api_config,
self.pool,
websocket_requests_per_minute_limit,
self.tx_executor,
self.method_tracer,
stop_receiver,
)
.await
}
}

async fn spawn_server(
Expand Down
2 changes: 1 addition & 1 deletion core/node/api_server/src/web3/tests/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ impl HttpTest for TraceBlockFlatTest {
}),
)
.await?
.unwrap_flatten();
.unwrap_flat();

// A transaction with 2 nested calls will convert into 3 Flattened calls.
// Also in this test, all tx have the same # of nested calls
Expand Down
33 changes: 20 additions & 13 deletions core/node/api_server/src/web3/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ use zksync_web3_decl::{
};

use super::*;
use crate::web3::testonly::{spawn_http_server, spawn_ws_server};
use crate::web3::testonly::TestServerBuilder;

mod debug;
mod filters;
Expand Down Expand Up @@ -245,14 +245,11 @@ async fn test_http_server(test: impl HttpTest) {
let genesis = GenesisConfig::for_tests();
let mut api_config = InternalApiConfig::new(&web3_config, &contracts_config, &genesis);
api_config.filters_disabled = test.filters_disabled();
let mut server_handles = spawn_http_server(
api_config,
pool.clone(),
test.transaction_executor(),
test.method_tracer(),
stop_receiver,
)
.await;
let mut server_handles = TestServerBuilder::new(pool.clone(), api_config)
.with_tx_executor(test.transaction_executor())
.with_method_tracer(test.method_tracer())
.build_http(stop_receiver)
.await;

let local_addr = server_handles.wait_until_ready().await;
let client = Client::http(format!("http://{local_addr}/").parse().unwrap())
Expand Down Expand Up @@ -306,6 +303,17 @@ async fn store_l2_block(
number: L2BlockNumber,
transaction_results: &[TransactionExecutionResult],
) -> anyhow::Result<L2BlockHeader> {
let header = create_l2_block(number.0);
store_custom_l2_block(storage, &header, transaction_results).await?;
Ok(header)
}

async fn store_custom_l2_block(
storage: &mut Connection<'_, Core>,
header: &L2BlockHeader,
transaction_results: &[TransactionExecutionResult],
) -> anyhow::Result<()> {
let number = header.number;
for result in transaction_results {
let l2_tx = result.transaction.clone().try_into().unwrap();
let tx_submission_result = storage
Expand All @@ -328,19 +336,18 @@ async fn store_l2_block(
.append_storage_logs(number, &[l2_block_log])
.await?;

let new_l2_block = create_l2_block(number.0);
storage.blocks_dal().insert_l2_block(&new_l2_block).await?;
storage.blocks_dal().insert_l2_block(header).await?;
storage
.transactions_dal()
.mark_txs_as_executed_in_l2_block(
new_l2_block.number,
number,
transaction_results,
1.into(),
ProtocolVersionId::latest(),
false,
)
.await?;
Ok(new_l2_block)
Ok(())
}

async fn seal_l1_batch(
Expand Down
Loading

0 comments on commit 79b6fcf

Please sign in to comment.