Skip to content

Commit

Permalink
fix(api): correct default fee data in eth call (matter-labs#2072)
Browse files Browse the repository at this point in the history
## What ❔

Previously, if no `max_fee_per_gas` was provided, it would mean that we
would use 0 as the base fee. This would also require the gas per pubdata
to be 0.

With this PR we will now use the current gas per pubdata. This PR also
provides a large eth call gas limit to ensure that it will work fine
even under very high L1 gas price

## Why ❔

<!-- Why are these changes done? What goal do they contribute to? What
are the principles behind them? -->
<!-- Example: PR templates ensure PR reviewers, observers, and future
iterators are in context about the evolution of repos. -->

## 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 `zk spellcheck`.
  • Loading branch information
StanislavBreadless authored Jun 2, 2024
1 parent 7a50a9f commit e71f6f9
Show file tree
Hide file tree
Showing 14 changed files with 228 additions and 54 deletions.
31 changes: 29 additions & 2 deletions core/lib/multivm/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,8 +441,35 @@ pub fn get_max_batch_gas_limit(version: VmVersion) -> u64 {
}
VmVersion::Vm1_4_1 => crate::vm_1_4_1::constants::BLOCK_GAS_LIMIT as u64,
VmVersion::Vm1_4_2 => crate::vm_1_4_2::constants::BLOCK_GAS_LIMIT as u64,
VmVersion::Vm1_5_0SmallBootloaderMemory => crate::vm_latest::constants::BATCH_GAS_LIMIT,
VmVersion::Vm1_5_0IncreasedBootloaderMemory => crate::vm_latest::constants::BATCH_GAS_LIMIT,
VmVersion::Vm1_5_0SmallBootloaderMemory | VmVersion::Vm1_5_0IncreasedBootloaderMemory => {
crate::vm_latest::constants::BATCH_GAS_LIMIT
}
}
}

pub fn get_eth_call_gas_limit(version: VmVersion) -> u64 {
match version {
VmVersion::M5WithRefunds | VmVersion::M5WithoutRefunds => {
crate::vm_m5::utils::ETH_CALL_GAS_LIMIT as u64
}
VmVersion::M6Initial | VmVersion::M6BugWithCompressionFixed => {
crate::vm_m6::utils::ETH_CALL_GAS_LIMIT as u64
}
VmVersion::Vm1_3_2 => crate::vm_1_3_2::utils::ETH_CALL_GAS_LIMIT as u64,
VmVersion::VmVirtualBlocks => {
crate::vm_virtual_blocks::constants::ETH_CALL_GAS_LIMIT as u64
}
VmVersion::VmVirtualBlocksRefundsEnhancement => {
crate::vm_refunds_enhancement::constants::ETH_CALL_GAS_LIMIT as u64
}
VmVersion::VmBoojumIntegration => {
crate::vm_boojum_integration::constants::ETH_CALL_GAS_LIMIT as u64
}
VmVersion::Vm1_4_1 => crate::vm_1_4_1::constants::ETH_CALL_GAS_LIMIT as u64,
VmVersion::Vm1_4_2 => crate::vm_1_4_2::constants::ETH_CALL_GAS_LIMIT as u64,
VmVersion::Vm1_5_0SmallBootloaderMemory | VmVersion::Vm1_5_0IncreasedBootloaderMemory => {
crate::vm_latest::constants::ETH_CALL_GAS_LIMIT
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions core/lib/multivm/src/versions/vm_latest/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use zk_evm_1_5_0::aux_structures::MemoryPage;
pub use zk_evm_1_5_0::zkevm_opcode_defs::system_params::{
ERGS_PER_CIRCUIT, INITIAL_STORAGE_WRITE_PUBDATA_BYTES,
};
use zksync_system_constants::{MAX_L2_TX_GAS_LIMIT, MAX_NEW_FACTORY_DEPS};
use zksync_system_constants::MAX_NEW_FACTORY_DEPS;

use super::vm::MultiVMSubversion;
use crate::vm_latest::old_vm::utils::heap_page_from_base;
Expand Down Expand Up @@ -160,7 +160,7 @@ pub const BATCH_COMPUTATIONAL_GAS_LIMIT: u32 =
pub const BATCH_GAS_LIMIT: u64 = 1 << 50;

/// How many gas is allowed to spend on a single transaction in eth_call method
pub const ETH_CALL_GAS_LIMIT: u32 = MAX_L2_TX_GAS_LIMIT as u32;
pub const ETH_CALL_GAS_LIMIT: u64 = BATCH_GAS_LIMIT;

/// ID of the transaction from L1
pub const L1_TX_TYPE: u8 = 255;
Expand Down
57 changes: 36 additions & 21 deletions core/lib/types/src/transaction_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,32 @@ pub struct CallRequest {
pub eip712_meta: Option<Eip712Meta>,
}

/// While some default parameters are usually provided for the `eth_call` methods,
/// sometimes users may want to override those.
pub struct CallOverrides {
pub enforced_base_fee: Option<u64>,
}

impl CallRequest {
/// Function to return a builder for a Call Request
pub fn builder() -> CallRequestBuilder {
CallRequestBuilder::default()
}

pub fn get_call_overrides(&self) -> Result<CallOverrides, SerializationTransactionError> {
let provided_gas_price = self.max_fee_per_gas.or(self.gas_price);
let enforced_base_fee = if let Some(provided_gas_price) = provided_gas_price {
Some(
provided_gas_price
.try_into()
.map_err(|_| SerializationTransactionError::MaxFeePerGasNotU64)?,
)
} else {
None
};

Ok(CallOverrides { enforced_base_fee })
}
}

/// Call Request Builder
Expand Down Expand Up @@ -183,10 +204,16 @@ pub enum SerializationTransactionError {
AccessListsNotSupported,
#[error("nonce has max value")]
TooBigNonce,
/// Sanity check error to avoid extremely big numbers specified

/// Sanity checks to avoid extremely big numbers specified
/// to gas and pubdata price.
#[error("{0}")]
TooHighGas(String),
#[error("max fee per gas higher than 2^64-1")]
MaxFeePerGasNotU64,
#[error("max fee per pubdata byte higher than 2^64-1")]
MaxFeePerPubdataByteNotU64,
#[error("max priority fee per gas higher than 2^64-1")]
MaxPriorityFeePerGasNotU64,

/// OversizedData is returned if the raw tx size is greater
/// than some meaningful limit a user might use. This is not a consensus error
/// making the transaction invalid, rather a DOS protection.
Expand Down Expand Up @@ -736,16 +763,12 @@ impl TransactionRequest {

fn get_fee_data_checked(&self) -> Result<Fee, SerializationTransactionError> {
if self.gas_price > u64::MAX.into() {
return Err(SerializationTransactionError::TooHighGas(
"max fee per gas higher than 2^64-1".to_string(),
));
return Err(SerializationTransactionError::MaxFeePerGasNotU64);
}

let gas_per_pubdata_limit = if let Some(meta) = &self.eip712_meta {
if meta.gas_per_pubdata > u64::MAX.into() {
return Err(SerializationTransactionError::TooHighGas(
"max fee per pubdata byte higher than 2^64-1".to_string(),
));
return Err(SerializationTransactionError::MaxFeePerPubdataByteNotU64);
} else if meta.gas_per_pubdata == U256::zero() {
return Err(SerializationTransactionError::GasPerPubDataLimitZero);
}
Expand All @@ -757,9 +780,7 @@ impl TransactionRequest {

let max_priority_fee_per_gas = self.max_priority_fee_per_gas.unwrap_or(self.gas_price);
if max_priority_fee_per_gas > u64::MAX.into() {
return Err(SerializationTransactionError::TooHighGas(
"max priority fee per gas higher than 2^64-1".to_string(),
));
return Err(SerializationTransactionError::MaxPriorityFeePerGasNotU64);
}

Ok(Fee {
Expand Down Expand Up @@ -1316,9 +1337,7 @@ mod tests {
L2Tx::from_request(tx1, usize::MAX);
assert_eq!(
execute_tx1.unwrap_err(),
SerializationTransactionError::TooHighGas(
"max fee per gas higher than 2^64-1".to_string()
)
SerializationTransactionError::MaxFeePerGasNotU64
);

let tx2 = TransactionRequest {
Expand All @@ -1332,9 +1351,7 @@ mod tests {
L2Tx::from_request(tx2, usize::MAX);
assert_eq!(
execute_tx2.unwrap_err(),
SerializationTransactionError::TooHighGas(
"max priority fee per gas higher than 2^64-1".to_string()
)
SerializationTransactionError::MaxPriorityFeePerGasNotU64
);

let tx3 = TransactionRequest {
Expand All @@ -1352,9 +1369,7 @@ mod tests {
L2Tx::from_request(tx3, usize::MAX);
assert_eq!(
execute_tx3.unwrap_err(),
SerializationTransactionError::TooHighGas(
"max fee per pubdata byte higher than 2^64-1".to_string()
)
SerializationTransactionError::MaxFeePerPubdataByteNotU64
);
}

Expand Down
6 changes: 3 additions & 3 deletions core/node/api_server/src/execution_sandbox/apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,12 +403,12 @@ impl StoredL2BlockInfo {
}

#[derive(Debug)]
struct ResolvedBlockInfo {
pub(crate) struct ResolvedBlockInfo {
state_l2_block_number: L2BlockNumber,
state_l2_block_hash: H256,
vm_l1_batch_number: L1BatchNumber,
l1_batch_timestamp: u64,
protocol_version: ProtocolVersionId,
pub(crate) protocol_version: ProtocolVersionId,
historical_fee_input: Option<BatchFeeInput>,
}

Expand All @@ -429,7 +429,7 @@ impl BlockArgs {
)
}

async fn resolve_block_info(
pub(crate) async fn resolve_block_info(
&self,
connection: &mut Connection<'_, Core>,
) -> anyhow::Result<ResolvedBlockInfo> {
Expand Down
21 changes: 9 additions & 12 deletions core/node/api_server/src/execution_sandbox/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,13 @@ use anyhow::Context as _;
use multivm::{
interface::{TxExecutionMode, VmExecutionResultAndLogs, VmInterface},
tracers::StorageInvocations,
vm_latest::constants::ETH_CALL_GAS_LIMIT,
MultiVMTracer,
};
use tracing::{span, Level};
use zksync_dal::{ConnectionPool, Core};
use zksync_types::{
fee::TransactionExecutionMetrics, l2::L2Tx, ExecuteTransactionCommon, Nonce,
PackedEthSignature, Transaction, U256,
fee::TransactionExecutionMetrics, l2::L2Tx, transaction_request::CallOverrides,
ExecuteTransactionCommon, Nonce, PackedEthSignature, Transaction, U256,
};

use super::{
Expand Down Expand Up @@ -40,15 +39,15 @@ impl TxExecutionArgs {
}

fn for_eth_call(
enforced_base_fee: u64,
enforced_base_fee: Option<u64>,
vm_execution_cache_misses_limit: Option<usize>,
) -> Self {
let missed_storage_invocation_limit = vm_execution_cache_misses_limit.unwrap_or(usize::MAX);
Self {
execution_mode: TxExecutionMode::EthCall,
enforced_nonce: None,
added_balance: U256::zero(),
enforced_base_fee: Some(enforced_base_fee),
enforced_base_fee,
missed_storage_invocation_limit,
}
}
Expand Down Expand Up @@ -170,23 +169,21 @@ impl TransactionExecutor {
vm_permit: VmPermit,
shared_args: TxSharedArgs,
connection_pool: ConnectionPool<Core>,
call_overrides: CallOverrides,
mut tx: L2Tx,
block_args: BlockArgs,
vm_execution_cache_misses_limit: Option<usize>,
custom_tracers: Vec<ApiTracer>,
) -> anyhow::Result<VmExecutionResultAndLogs> {
let enforced_base_fee = tx.common_data.fee.max_fee_per_gas.as_u64();
let execution_args =
TxExecutionArgs::for_eth_call(enforced_base_fee, vm_execution_cache_misses_limit);
let execution_args = TxExecutionArgs::for_eth_call(
call_overrides.enforced_base_fee,
vm_execution_cache_misses_limit,
);

if tx.common_data.signature.is_empty() {
tx.common_data.signature = PackedEthSignature::default().serialize_packed().into();
}

// Protection against infinite-loop eth_calls and alike:
// limiting the amount of gas the call can use.
// We can't use `BLOCK_ERGS_LIMIT` here since the VM itself has some overhead.
tx.common_data.fee.gas_limit = ETH_CALL_GAS_LIMIT.into();
let output = self
.execute_tx_in_sandbox(
vm_permit,
Expand Down
20 changes: 19 additions & 1 deletion core/node/api_server/src/tx_sender/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use multivm::{
interface::VmExecutionResultAndLogs,
utils::{
adjust_pubdata_price_for_tx, derive_base_fee_and_gas_per_pubdata, derive_overhead,
get_max_batch_gas_limit,
get_eth_call_gas_limit, get_max_batch_gas_limit,
},
vm_latest::constants::BATCH_COMPUTATIONAL_GAS_LIMIT,
};
Expand All @@ -28,6 +28,7 @@ use zksync_types::{
fee_model::BatchFeeInput,
get_code_key, get_intrinsic_constants,
l2::{error::TxCheckError::TxDuplication, L2Tx},
transaction_request::CallOverrides,
utils::storage_key_for_eth_balance,
AccountTreeId, Address, ExecuteTransactionCommon, L2ChainId, Nonce, PackedEthSignature,
ProtocolVersionId, Transaction, VmVersion, H160, H256, MAX_L2_TX_GAS_LIMIT,
Expand Down Expand Up @@ -965,6 +966,7 @@ impl TxSender {
pub(super) async fn eth_call(
&self,
block_args: BlockArgs,
call_overrides: CallOverrides,
tx: L2Tx,
) -> Result<Vec<u8>, SubmitTxError> {
let vm_permit = self.0.vm_concurrency_limiter.acquire().await;
Expand All @@ -977,6 +979,7 @@ impl TxSender {
vm_permit,
self.shared_args().await?,
self.0.replica_connection_pool.clone(),
call_overrides,
tx,
block_args,
vm_execution_cache_misses_limit,
Expand Down Expand Up @@ -1036,4 +1039,19 @@ impl TxSender {
}
Ok(())
}

pub(crate) async fn get_default_eth_call_gas(
&self,
block_args: BlockArgs,
) -> anyhow::Result<u64> {
let mut connection = self.acquire_replica_connection().await?;

let protocol_version = block_args
.resolve_block_info(&mut connection)
.await
.context("failed to resolve block info")?
.protocol_version;

Ok(get_eth_call_gas_limit(protocol_version.into()))
}
}
16 changes: 15 additions & 1 deletion core/node/api_server/src/web3/namespaces/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ impl DebugNamespace {

pub async fn debug_trace_call_impl(
&self,
request: CallRequest,
mut request: CallRequest,
block_id: Option<BlockId>,
options: Option<TracerConfig>,
) -> Result<DebugCall, Web3Error> {
Expand All @@ -148,6 +148,19 @@ impl DebugNamespace {
.last_sealed_l2_block
.diff_with_block_args(&block_args),
);

if request.gas.is_none() {
request.gas = Some(
self.state
.tx_sender
.get_default_eth_call_gas(block_args)
.await
.map_err(Web3Error::InternalError)?
.into(),
)
}

let call_overrides = request.get_call_overrides()?;
let tx = L2Tx::from_request(request.into(), MAX_ENCODED_TX_SIZE)?;

let shared_args = self.shared_args().await;
Expand All @@ -173,6 +186,7 @@ impl DebugNamespace {
vm_permit,
shared_args,
self.state.connection_pool.clone(),
call_overrides,
tx.clone(),
block_args,
self.sender_config().vm_execution_cache_misses_limit,
Expand Down
21 changes: 19 additions & 2 deletions core/node/api_server/src/web3/namespaces/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl EthNamespace {

pub async fn call_impl(
&self,
request: CallRequest,
mut request: CallRequest,
block_id: Option<BlockId>,
) -> Result<Bytes, Web3Error> {
let block_id = block_id.unwrap_or(BlockId::Number(BlockNumber::Pending));
Expand All @@ -70,8 +70,25 @@ impl EthNamespace {
);
drop(connection);

if request.gas.is_none() {
request.gas = Some(
self.state
.tx_sender
.get_default_eth_call_gas(block_args)
.await
.map_err(Web3Error::InternalError)?
.into(),
)
}
let call_overrides = request.get_call_overrides()?;
let tx = L2Tx::from_request(request.into(), self.state.api_config.max_tx_size)?;
let call_result = self.state.tx_sender.eth_call(block_args, tx).await?;

// It is assumed that the previous checks has already enforced that the `max_fee_per_gas` is at most u64.
let call_result: Vec<u8> = self
.state
.tx_sender
.eth_call(block_args, call_overrides, tx)
.await?;
Ok(call_result.into())
}

Expand Down
Loading

0 comments on commit e71f6f9

Please sign in to comment.