Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(api): Fix batch fee input for debug namespace #2948

Merged
merged 6 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -5,7 +5,6 @@ use zksync_system_constants::MAX_ENCODED_TX_SIZE;
use zksync_types::{
api::{BlockId, BlockNumber, DebugCall, DebugCallType, ResultDebugCall, TracerConfig},
debug_flat_call::{flatten_debug_calls, DebugCallFlat},
fee_model::BatchFeeInput,
l2::L2Tx,
transaction_request::CallRequest,
web3, H256, U256,
Expand All @@ -19,27 +18,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(call: Call, only_top_call: bool) -> DebugCall {
Expand Down Expand Up @@ -162,12 +146,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
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
61 changes: 53 additions & 8 deletions core/node/api_server/src/web3/tests/vm.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
//! Tests for the VM-instantiating methods (e.g., `eth_call`).

use std::sync::{
atomic::{AtomicU32, Ordering},
Mutex,
use std::{
str,
sync::{
atomic::{AtomicU32, Ordering},
Mutex,
},
};

use api::state_override::{OverrideAccount, StateOverride};
Expand Down Expand Up @@ -51,6 +54,11 @@ impl ExpectedFeeInput {
self.expect_for_block(api::BlockNumber::Pending, scale);
}

#[allow(dead_code)]
fn expect_custom(&self, expected: BatchFeeInput) {
*self.0.lock().unwrap() = expected;
}

fn assert_eq(&self, actual: BatchFeeInput) {
let expected = *self.0.lock().unwrap();
// We do relaxed comparisons to deal with the fact that the fee input provider may convert inputs to pubdata independent form.
Expand Down Expand Up @@ -87,11 +95,18 @@ impl CallTest {
expected_fee_input.assert_eq(env.l1_batch.fee_input);

let expected_block_number = match tx.execute.calldata() {
b"pending" => latest_block + 1,
b"latest" => latest_block,
b"pending" => latest_block.0 + 1,
b"latest" => latest_block.0,
block if block.starts_with(b"block=") => str::from_utf8(block)
.expect("non-UTF8 calldata")
.strip_prefix("block=")
.unwrap()
.trim()
.parse()
.expect("invalid block number"),
data => panic!("Unexpected calldata: {data:?}"),
};
assert_eq!(env.l1_batch.first_l2_block.number, expected_block_number.0);
assert_eq!(env.l1_batch.first_l2_block.number, expected_block_number);

ExecutionResult::Success {
output: b"output".to_vec(),
Expand All @@ -115,7 +130,6 @@ impl HttpTest for CallTest {
// Store an additional L2 block because L2 block #0 has some special processing making it work incorrectly.
let mut connection = pool.connection().await?;
store_l2_block(&mut connection, L2BlockNumber(1), &[]).await?;
drop(connection);

let call_result = client
.call(Self::call_request(b"pending"), None, None)
Expand Down Expand Up @@ -148,6 +162,22 @@ impl HttpTest for CallTest {
panic!("Unexpected error: {error:?}");
}

// Check that the method handler fetches fee inputs for recent blocks. To do that, we create a new block
// with a large fee input; it should be loaded by `ApiFeeInputProvider` and override the input provided by the wrapped mock provider.
let mut block_header = create_l2_block(2);
block_header.batch_fee_input =
<dyn BatchFeeModelInputProvider>::default_batch_fee_input_scaled(
FeeParams::sensible_v1_default(),
2.5,
2.5,
);
store_custom_l2_block(&mut connection, &block_header, &[]).await?;
// Fee input is not scaled further as per `ApiFeeInputProvider` implementation
self.fee_input.expect_custom(block_header.batch_fee_input);
let call_request = CallTest::call_request(b"block=3");
let call_result = client.call(call_request.clone(), None, None).await?;
assert_eq!(call_result.0, b"output");

Ok(())
}
}
Expand Down Expand Up @@ -484,7 +514,6 @@ impl HttpTest for TraceCallTest {
// Store an additional L2 block because L2 block #0 has some special processing making it work incorrectly.
let mut connection = pool.connection().await?;
store_l2_block(&mut connection, L2BlockNumber(1), &[]).await?;
drop(connection);

self.fee_input.expect_default(Self::FEE_SCALE);
let call_request = CallTest::call_request(b"pending");
Expand Down Expand Up @@ -525,6 +554,22 @@ impl HttpTest for TraceCallTest {
panic!("Unexpected error: {error:?}");
}

// Check that the method handler fetches fee inputs for recent blocks. To do that, we create a new block
// with a large fee input; it should be loaded by `ApiFeeInputProvider` and override the input provided by the wrapped mock provider.
let mut block_header = create_l2_block(2);
block_header.batch_fee_input =
<dyn BatchFeeModelInputProvider>::default_batch_fee_input_scaled(
FeeParams::sensible_v1_default(),
3.0,
3.0,
);
store_custom_l2_block(&mut connection, &block_header, &[]).await?;
// Fee input is not scaled further as per `ApiFeeInputProvider` implementation
self.fee_input.expect_custom(block_header.batch_fee_input);
let call_request = CallTest::call_request(b"block=3");
let call_result = client.trace_call(call_request.clone(), None, None).await?;
Self::assert_debug_call(&call_request, &call_result);

Ok(())
}
}
Expand Down
10 changes: 3 additions & 7 deletions core/node/api_server/src/web3/tests/ws.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,9 @@ async fn test_ws_server(test: impl WsTest) {
drop(storage);

let (stop_sender, stop_receiver) = watch::channel(false);
let (mut server_handles, pub_sub_events) = spawn_ws_server(
api_config,
pool.clone(),
stop_receiver,
test.websocket_requests_per_minute_limit(),
)
.await;
let (mut server_handles, pub_sub_events) = TestServerBuilder::new(pool.clone(), api_config)
.build_ws(test.websocket_requests_per_minute_limit(), stop_receiver)
.await;

let local_addr = server_handles.wait_until_ready().await;
let client = Client::ws(format!("ws://{local_addr}").parse().unwrap())
Expand Down
Loading
Loading