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 duplicate DB connection acquired in eth_call #2763

Merged
merged 2 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 2 additions & 1 deletion core/node/api_server/src/tx_sender/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1012,14 +1012,15 @@ impl TxSender {
) -> Result<Vec<u8>, SubmitTxError> {
let vm_permit = self.0.vm_concurrency_limiter.acquire().await;
let vm_permit = vm_permit.ok_or(SubmitTxError::ServerShuttingDown)?;
let setup_args = self.call_args(&tx, Some(&call_overrides)).await?;

let connection = self.acquire_replica_connection().await?;
let result = self
.0
.executor
.execute_tx_in_sandbox(
vm_permit,
self.call_args(&tx, Some(&call_overrides)).await?,
setup_args,
TxExecutionArgs::for_eth_call(tx),
connection,
block_args,
Expand Down
51 changes: 49 additions & 2 deletions core/node/api_server/src/tx_sender/tests.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
//! Tests for the transaction sender.

use std::time::Duration;

use assert_matches::assert_matches;
use zksync_multivm::interface::ExecutionResult;
use zksync_node_fee_model::MockBatchFeeParamsProvider;
use zksync_node_genesis::{insert_genesis_batch, GenesisParams};
use zksync_node_test_utils::{create_l2_block, create_l2_transaction, prepare_recovery_snapshot};
use zksync_types::{get_nonce_key, L1BatchNumber, L2BlockNumber, StorageLog};
use zksync_types::{api, get_nonce_key, L1BatchNumber, L2BlockNumber, StorageLog};
use zksync_utils::u256_to_h256;

use super::*;
use crate::{
execution_sandbox::testonly::MockOneshotExecutor, web3::testonly::create_test_tx_sender,
execution_sandbox::{testonly::MockOneshotExecutor, BlockStartInfo},
web3::testonly::create_test_tx_sender,
};

#[tokio::test]
Expand Down Expand Up @@ -155,3 +158,47 @@ async fn submitting_tx_requires_one_connection() {
.unwrap()
.expect("transaction is not persisted");
}

#[tokio::test]
async fn eth_call_requires_single_connection() {
let pool = ConnectionPool::<Core>::constrained_test_pool(1).await;
let mut storage = pool.connection().await.unwrap();
let genesis_params = GenesisParams::mock();
insert_genesis_batch(&mut storage, &genesis_params)
.await
.unwrap();
let start_info = BlockStartInfo::new(&mut storage, Duration::MAX)
.await
.unwrap();
let block_id = api::BlockId::Number(api::BlockNumber::Latest);
let block_args = BlockArgs::new(&mut storage, block_id, &start_info)
.await
.unwrap();
drop(storage);

let tx = create_l2_transaction(10, 100);
let tx_hash = tx.hash();

let mut tx_executor = MockOneshotExecutor::default();
tx_executor.set_call_responses(move |received_tx, _| {
assert_eq!(received_tx.hash(), tx_hash);
ExecutionResult::Success {
output: b"success!".to_vec(),
}
});
let tx_executor = tx_executor.into();
let (tx_sender, _) = create_test_tx_sender(
pool.clone(),
genesis_params.config().l2_chain_id,
tx_executor,
)
.await;
let call_overrides = CallOverrides {
enforced_base_fee: None,
};
let output = tx_sender
.eth_call(block_args, call_overrides, tx, None)
.await
.unwrap();
assert_eq!(output, b"success!");
}
39 changes: 36 additions & 3 deletions core/node/api_server/src/web3/testonly.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,18 @@

use std::{pin::Pin, time::Instant};

use async_trait::async_trait;
use tokio::sync::watch;
use zksync_config::configs::{api::Web3JsonRpcConfig, chain::StateKeeperConfig, wallets::Wallets};
use zksync_dal::ConnectionPool;
use zksync_health_check::CheckHealth;
use zksync_node_fee_model::MockBatchFeeParamsProvider;
use zksync_node_fee_model::{BatchFeeModelInputProvider, MockBatchFeeParamsProvider};
use zksync_state::PostgresStorageCaches;
use zksync_state_keeper::seal_criteria::NoopSealer;
use zksync_types::L2ChainId;
use zksync_types::{
fee_model::{BatchFeeInput, FeeParams},
L2ChainId,
};

use super::{metrics::ApiTransportLabel, *};
use crate::{
Expand All @@ -20,6 +24,32 @@ use crate::{
const TEST_TIMEOUT: Duration = Duration::from_secs(90);
const POLL_INTERVAL: Duration = Duration::from_millis(50);

/// Same as [`MockBatchFeeParamsProvider`], but also artificially acquires a Postgres connection on each call
/// (same as the real node implementation).
#[derive(Debug)]
struct MockApiBatchFeeParamsProvider {
inner: MockBatchFeeParamsProvider,
pool: ConnectionPool<Core>,
}

#[async_trait]
impl BatchFeeModelInputProvider for MockApiBatchFeeParamsProvider {
async fn get_batch_fee_input_scaled(
&self,
l1_gas_price_scale_factor: f64,
l1_pubdata_price_scale_factor: f64,
) -> anyhow::Result<BatchFeeInput> {
let _connection = self.pool.connection().await?;
self.inner
.get_batch_fee_input_scaled(l1_gas_price_scale_factor, l1_pubdata_price_scale_factor)
.await
}

fn get_fee_model_params(&self) -> FeeParams {
self.inner.get_fee_model_params()
}
}

pub(crate) async fn create_test_tx_sender(
pool: ConnectionPool<Core>,
l2_chain_id: L2ChainId,
Expand All @@ -36,7 +66,10 @@ pub(crate) async fn create_test_tx_sender(
);

let storage_caches = PostgresStorageCaches::new(1, 1);
let batch_fee_model_input_provider = Arc::new(MockBatchFeeParamsProvider::default());
let batch_fee_model_input_provider = Arc::new(MockApiBatchFeeParamsProvider {
inner: MockBatchFeeParamsProvider::default(),
pool: pool.clone(),
});
let (mut tx_sender, vm_barrier) = crate::tx_sender::build_tx_sender(
&tx_sender_config,
&web3_config,
Expand Down
Loading