Skip to content

Commit

Permalink
fix: validate gas_limit and max_fee_per_gas before transaction execut…
Browse files Browse the repository at this point in the history
…ion (#207)

* fix: validate gas_limit and max_fee_per_gas before transaction execution

* fix: add validation for max_fee_per_gas being to low

* fix: move validate_tx to node/in_memoty.ts

* fix: lint

* fix: move tx validation logic to run_l2_tx

* test: add tests for tx validation

* fix: remove unneeded match from eth.rs

* fix: lint

* fix: remove +nightly from vscode debug config

* fix: remove extra new line log
  • Loading branch information
Romsters authored Oct 31, 2023
1 parent fae30ab commit c5350bf
Show file tree
Hide file tree
Showing 2 changed files with 162 additions and 26 deletions.
90 changes: 90 additions & 0 deletions src/node/in_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1211,6 +1211,35 @@ impl<S: ForkSource + std::fmt::Debug + Clone> InMemoryNode<S> {
}
}

// Validates L2 transaction
fn validate_tx(&self, tx: &L2Tx) -> Result<(), String> {
let max_gas = U256::from(u32::MAX);
if tx.common_data.fee.gas_limit > max_gas
|| tx.common_data.fee.gas_per_pubdata_limit > max_gas
{
return Err("exceeds block gas limit".into());
}

if tx.common_data.fee.max_fee_per_gas < L2_GAS_PRICE.into() {
tracing::info!(
"Submitted Tx is Unexecutable {:?} because of MaxFeePerGasTooLow {}",
tx.hash(),
tx.common_data.fee.max_fee_per_gas
);
return Err("block base fee higher than max fee per gas".into());
}

if tx.common_data.fee.max_fee_per_gas < tx.common_data.fee.max_priority_fee_per_gas {
tracing::info!(
"Submitted Tx is Unexecutable {:?} because of MaxPriorityFeeGreaterThanMaxFee {}",
tx.hash(),
tx.common_data.fee.max_fee_per_gas
);
return Err("max priority fee per gas higher than max fee per gas".into());
}
Ok(())
}

/// Executes the given L2 transaction and returns all the VM logs.
pub fn run_l2_tx_inner(
&self,
Expand Down Expand Up @@ -1425,7 +1454,17 @@ impl<S: ForkSource + std::fmt::Debug + Clone> InMemoryNode<S> {
/// Runs L2 transaction and commits it to a new block.
pub fn run_l2_tx(&self, l2_tx: L2Tx, execution_mode: TxExecutionMode) -> Result<(), String> {
let tx_hash = l2_tx.hash();

tracing::info!("");
tracing::info!("Validating {}", format!("{:?}", tx_hash).bold());

match self.validate_tx(&l2_tx) {
Ok(_) => (),
Err(e) => {
return Err(e);
}
};

tracing::info!("Executing {}", format!("{:?}", tx_hash).bold());

{
Expand Down Expand Up @@ -1639,3 +1678,54 @@ impl BlockContext {
}
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::{http_fork_source::HttpForkSource, node::InMemoryNode, testing};

#[tokio::test]
async fn test_run_l2_tx_validates_tx_gas_limit_too_high() {
let node = InMemoryNode::<HttpForkSource>::default();
let tx = testing::TransactionBuilder::new()
.set_gas_limit(U256::from(u32::MAX) + 1)
.build();
node.set_rich_account(tx.common_data.initiator_address);

let result = node.run_l2_tx(tx, TxExecutionMode::VerifyExecute);

assert_eq!(result.err(), Some("exceeds block gas limit".into()));
}

#[tokio::test]
async fn test_run_l2_tx_validates_tx_max_fee_per_gas_too_low() {
let node = InMemoryNode::<HttpForkSource>::default();
let tx = testing::TransactionBuilder::new()
.set_max_fee_per_gas(U256::from(250_000_000 - 1))
.build();
node.set_rich_account(tx.common_data.initiator_address);

let result = node.run_l2_tx(tx, TxExecutionMode::VerifyExecute);

assert_eq!(
result.err(),
Some("block base fee higher than max fee per gas".into())
);
}

#[tokio::test]
async fn test_run_l2_tx_validates_tx_max_priority_fee_per_gas_higher_than_max_fee_per_gas() {
let node = InMemoryNode::<HttpForkSource>::default();
let tx = testing::TransactionBuilder::new()
.set_max_priority_fee_per_gas(U256::from(250_000_000 + 1))
.build();
node.set_rich_account(tx.common_data.initiator_address);

let result = node.run_l2_tx(tx, TxExecutionMode::VerifyExecute);

assert_eq!(
result.err(),
Some("max priority fee per gas higher than max fee per gas".into())
);
}
}
98 changes: 72 additions & 26 deletions src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,7 @@ use multivm::interface::{ExecutionResult, VmExecutionResultAndLogs};
use std::str::FromStr;
use zksync_basic_types::{H160, U64};
use zksync_types::api::{BridgeAddresses, DebugCall, DebugCallType, Log};
use zksync_types::{
fee::Fee, l2::L2Tx, Address, L2ChainId, Nonce, PackedEthSignature, ProtocolVersionId, H256,
U256,
};
use zksync_types::{fee::Fee, l2::L2Tx, Address, L2ChainId, Nonce, ProtocolVersionId, H256, U256};

/// Configuration for the [MockServer]'s initial block.
#[derive(Default, Debug, Clone)]
Expand Down Expand Up @@ -357,6 +354,75 @@ impl RawTransactionsResponseBuilder {
}
}

#[derive(Debug, Clone)]
pub struct TransactionBuilder {
tx_hash: H256,
from_account_private_key: H256,
gas_limit: U256,
max_fee_per_gas: U256,
max_priority_fee_per_gas: U256,
}

impl Default for TransactionBuilder {
fn default() -> Self {
Self {
tx_hash: H256::repeat_byte(0x01),
from_account_private_key: H256::random(),
gas_limit: U256::from(1_000_000),
max_fee_per_gas: U256::from(250_000_000),
max_priority_fee_per_gas: U256::from(250_000_000),
}
}
}

impl TransactionBuilder {
pub fn new() -> Self {
Self::default()
}

pub fn set_hash(&mut self, hash: H256) -> &mut Self {
self.tx_hash = hash;
self
}

pub fn set_gas_limit(&mut self, gas_limit: U256) -> &mut Self {
self.gas_limit = gas_limit;
self
}

pub fn set_max_fee_per_gas(&mut self, max_fee_per_gas: U256) -> &mut Self {
self.max_fee_per_gas = max_fee_per_gas;
self
}

pub fn set_max_priority_fee_per_gas(&mut self, max_priority_fee_per_gas: U256) -> &mut Self {
self.max_priority_fee_per_gas = max_priority_fee_per_gas;
self
}

pub fn build(&mut self) -> L2Tx {
let mut tx = L2Tx::new_signed(
Address::random(),
vec![],
Nonce(0),
Fee {
gas_limit: self.gas_limit,
max_fee_per_gas: self.max_fee_per_gas,
max_priority_fee_per_gas: self.max_priority_fee_per_gas,
gas_per_pubdata_limit: U256::from(20000),
},
U256::from(1),
L2ChainId::from(260),
&self.from_account_private_key,
None,
Default::default(),
)
.unwrap();
tx.set_input(vec![], self.tx_hash);
tx
}
}

/// Applies a transaction with a given hash to the node and returns the block hash.
pub fn apply_tx<T: ForkSource + std::fmt::Debug + Clone>(
node: &InMemoryNode<T>,
Expand All @@ -369,28 +435,8 @@ pub fn apply_tx<T: ForkSource + std::fmt::Debug + Clone>(
.expect("failed getting current batch number");
let produced_block_hash = compute_hash(next_miniblock, tx_hash);

let private_key = H256::random();
let from_account = PackedEthSignature::address_from_private_key(&private_key)
.expect("failed generating address");
node.set_rich_account(from_account);
let mut tx = L2Tx::new_signed(
Address::random(),
vec![],
Nonce(0),
Fee {
gas_limit: U256::from(1_000_000),
max_fee_per_gas: U256::from(250_000_000),
max_priority_fee_per_gas: U256::from(250_000_000),
gas_per_pubdata_limit: U256::from(20000),
},
U256::from(1),
L2ChainId::from(260),
&private_key,
None,
Default::default(),
)
.unwrap();
tx.set_input(vec![], tx_hash);
let tx = TransactionBuilder::new().set_hash(tx_hash).build();
node.set_rich_account(tx.common_data.initiator_address);
node.apply_txs(vec![tx]).expect("failed applying tx");

(produced_block_hash, U64::from(next_miniblock))
Expand Down

0 comments on commit c5350bf

Please sign in to comment.