Skip to content

Commit ebb341b

Browse files
authored
refactor(l2): remove async from Blockchain::new_evm (#5062)
**Motivation** Block execution is a blocking process, except for some parts which _could be_ async, like DB access. We're currently working on removing async-ness from block execution, for performance reasons, and a recently added `await` for L2's fee config is bringing us some problems. **Description** This PR changes the `tokio::sync::RwLock` to a `std::sync::RwLock`, to avoid `async` use during block execution. This shouldn't impact the tokio runtime, due to the lock being updated every L1 block, and the change being a single assign operation.
1 parent 7c89835 commit ebb341b

File tree

10 files changed

+43
-32
lines changed

10 files changed

+43
-32
lines changed

cmd/ethrex/l2/initializers.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ pub async fn init_l2(
180180
// We wrap fee_config in an Arc<RwLock> to let the watcher
181181
// update the L1 fee periodically.
182182
let l2_config = L2Config {
183-
fee_config: Arc::new(tokio::sync::RwLock::new(fee_config)),
183+
fee_config: Arc::new(std::sync::RwLock::new(fee_config)),
184184
};
185185

186186
let blockchain_opts = ethrex_blockchain::BlockchainOptions {

crates/blockchain/blockchain.rs

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,9 @@ use mempool::Mempool;
3535
use payload::PayloadOrTask;
3636
use std::collections::{BTreeMap, HashMap};
3737
use std::sync::atomic::{AtomicBool, Ordering};
38-
use std::sync::{Arc, Mutex};
38+
use std::sync::{Arc, Mutex, RwLock};
3939
use std::time::Instant;
40-
use tokio::sync::{Mutex as TokioMutex, RwLock};
40+
use tokio::sync::Mutex as TokioMutex;
4141
use tokio_util::sync::CancellationToken;
4242

4343
use vm::StoreVmDatabase;
@@ -156,7 +156,7 @@ impl Blockchain {
156156
validate_block(block, &parent_header, &chain_config, ELASTICITY_MULTIPLIER)?;
157157

158158
let vm_db = StoreVmDatabase::new(self.storage.clone(), block.header.parent_hash);
159-
let mut vm = self.new_evm(vm_db).await?;
159+
let mut vm = self.new_evm(vm_db)?;
160160

161161
let execution_result = vm.execute_block(block)?;
162162
let account_updates = vm.get_state_transitions()?;
@@ -641,7 +641,7 @@ impl Blockchain {
641641
first_block_header.parent_hash,
642642
block_hash_cache,
643643
);
644-
let mut vm = self.new_evm(vm_db).await.map_err(|e| (e.into(), None))?;
644+
let mut vm = self.new_evm(vm_db).map_err(|e| (e.into(), None))?;
645645

646646
let blocks_len = blocks.len();
647647
let mut all_receipts: Vec<(BlockHash, Vec<Receipt>)> = Vec::with_capacity(blocks_len);
@@ -1018,14 +1018,8 @@ impl Blockchain {
10181018
Ok(result)
10191019
}
10201020

1021-
pub async fn new_evm(&self, vm_db: StoreVmDatabase) -> Result<Evm, EvmError> {
1022-
let evm = match &self.options.r#type {
1023-
BlockchainType::L1 => Evm::new_for_l1(vm_db),
1024-
BlockchainType::L2(l2_config) => {
1025-
Evm::new_for_l2(vm_db, *l2_config.fee_config.read().await)?
1026-
}
1027-
};
1028-
Ok(evm)
1021+
pub fn new_evm(&self, vm_db: StoreVmDatabase) -> Result<Evm, EvmError> {
1022+
new_evm(&self.options.r#type, vm_db)
10291023
}
10301024

10311025
/// Get the current fork of the chain, based on the latest block's timestamp
@@ -1040,6 +1034,20 @@ impl Blockchain {
10401034
}
10411035
}
10421036

1037+
pub fn new_evm(blockchain_type: &BlockchainType, vm_db: StoreVmDatabase) -> Result<Evm, EvmError> {
1038+
let evm = match blockchain_type {
1039+
BlockchainType::L1 => Evm::new_for_l1(vm_db),
1040+
BlockchainType::L2(l2_config) => {
1041+
let fee_config = *l2_config
1042+
.fee_config
1043+
.read()
1044+
.map_err(|_| EvmError::Custom("Fee config lock was poisoned".to_string()))?;
1045+
Evm::new_for_l2(vm_db, fee_config)?
1046+
}
1047+
};
1048+
Ok(evm)
1049+
}
1050+
10431051
pub fn validate_requests_hash(
10441052
header: &BlockHeader,
10451053
chain_config: &ChainConfig,

crates/blockchain/payload.rs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ use crate::{
3838
constants::{GAS_LIMIT_BOUND_DIVISOR, MIN_GAS_LIMIT, TX_GAS_COST},
3939
error::{ChainError, InvalidBlockError},
4040
mempool::PendingTxFilter,
41+
new_evm,
4142
vm::StoreVmDatabase,
4243
};
4344

@@ -219,10 +220,10 @@ pub struct PayloadBuildContext {
219220
}
220221

221222
impl PayloadBuildContext {
222-
pub async fn new(
223+
pub fn new(
223224
payload: Block,
224225
storage: &Store,
225-
blockchain_type: BlockchainType,
226+
blockchain_type: &BlockchainType,
226227
) -> Result<Self, EvmError> {
227228
let config = storage
228229
.get_chain_config()
@@ -236,12 +237,7 @@ impl PayloadBuildContext {
236237
);
237238

238239
let vm_db = StoreVmDatabase::new(storage.clone(), payload.header.parent_hash);
239-
let vm = match blockchain_type {
240-
BlockchainType::L1 => Evm::new_for_l1(vm_db),
241-
BlockchainType::L2(l2_config) => {
242-
Evm::new_for_l2(vm_db, *l2_config.fee_config.read().await)?
243-
}
244-
};
240+
let vm = new_evm(blockchain_type, vm_db)?;
245241

246242
Ok(PayloadBuildContext {
247243
remaining_gas: payload.header.gas_limit,
@@ -393,8 +389,7 @@ impl Blockchain {
393389

394390
debug!("Building payload");
395391
let base_fee = payload.header.base_fee_per_gas.unwrap_or_default();
396-
let mut context =
397-
PayloadBuildContext::new(payload, &self.storage, self.options.r#type.clone()).await?;
392+
let mut context = PayloadBuildContext::new(payload, &self.storage, &self.options.r#type)?;
398393

399394
if let BlockchainType::L1 = self.options.r#type {
400395
self.apply_system_operations(&mut context)?;

crates/blockchain/tracing.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ impl Blockchain {
107107
parent_hash,
108108
block_hash_cache,
109109
);
110-
let mut vm = self.new_evm(vm_db).await?;
110+
let mut vm = self.new_evm(vm_db)?;
111111
// Run parents to rebuild pre-state
112112
for block in blocks_to_re_execute.iter().rev() {
113113
vm.rerun_block(block, None)?;

crates/l2/based/block_fetcher.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ impl BlockFetcher {
358358
let mut acc_account_updates: HashMap<H160, AccountUpdate> = HashMap::new();
359359
for block in batch {
360360
let vm_db = StoreVmDatabase::new(self.store.clone(), block.header.parent_hash);
361-
let mut vm = self.blockchain.new_evm(vm_db).await?;
361+
let mut vm = self.blockchain.new_evm(vm_db)?;
362362
vm.execute_block(block)
363363
.map_err(BlockFetcherError::EvmError)?;
364364
let account_updates = vm

crates/l2/sequencer/block_producer.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,10 @@ impl BlockProducer {
246246
return Err(BlockProducerError::Custom("Invalid blockchain type".into()));
247247
};
248248

249-
let fee_config = *l2_config.fee_config.read().await;
249+
let fee_config = *l2_config
250+
.fee_config
251+
.read()
252+
.map_err(|_| BlockProducerError::Custom("Fee config lock was poisoned".to_string()))?;
250253

251254
self.rollup_store
252255
.store_fee_config_by_block(block_number, fee_config)

crates/l2/sequencer/block_producer/payload_builder.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ pub async fn build_payload(
4848
let gas_limit = payload.header.gas_limit;
4949

5050
debug!("Building payload");
51-
let mut context =
52-
PayloadBuildContext::new(payload, store, blockchain.options.r#type.clone()).await?;
51+
let mut context = PayloadBuildContext::new(payload, store, &blockchain.options.r#type)?;
5352

5453
fill_transactions(
5554
blockchain.clone(),

crates/l2/sequencer/l1_watcher.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,10 @@ impl GenServer for L1Watcher {
359359
return CastResponse::NoReply;
360360
};
361361

362-
let mut fee_config_guard = l2_config.fee_config.write().await;
362+
let Ok(mut fee_config_guard) = l2_config.fee_config.write() else {
363+
error!("Fee config lock was poisoned when updating L1 blob base fee");
364+
return CastResponse::NoReply;
365+
};
363366

364367
let Some(l1_fee_config) = fee_config_guard.l1_fee_config.as_mut() else {
365368
warn!("L1 fee config is not set. Skipping L1 blob base fee update.");

crates/networking/rpc/eth/gas_price.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,10 @@ impl RpcHandler for GasPrice {
4040

4141
// Add the operator fee to the gas price if configured
4242
if let BlockchainType::L2(l2_config) = &context.blockchain.options.r#type {
43-
let fee_config = *l2_config.fee_config.read().await;
43+
let fee_config = *l2_config
44+
.fee_config
45+
.read()
46+
.map_err(|_| RpcErr::Internal("Fee config lock was poisoned".to_string()))?;
4447
if let Some(operator_fee_config) = &fee_config.operator_fee_config {
4548
gas_price += operator_fee_config.operator_fee_per_gas;
4649
}

crates/networking/rpc/eth/transaction.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ impl RpcHandler for CreateAccessListRequest {
348348
};
349349

350350
let vm_db = StoreVmDatabase::new(context.storage.clone(), header.hash());
351-
let mut vm = context.blockchain.new_evm(vm_db).await?;
351+
let mut vm = context.blockchain.new_evm(vm_db)?;
352352

353353
// Run transaction and obtain access list
354354
let (gas_used, access_list, error) = vm.create_access_list(&self.transaction, &header)?;
@@ -572,7 +572,7 @@ async fn simulate_tx(
572572
blockchain: Arc<Blockchain>,
573573
) -> Result<ExecutionResult, RpcErr> {
574574
let vm_db = StoreVmDatabase::new(storage.clone(), block_header.hash());
575-
let mut vm = blockchain.new_evm(vm_db).await?;
575+
let mut vm = blockchain.new_evm(vm_db)?;
576576

577577
match vm.simulate_tx_from_generic(transaction, block_header)? {
578578
ExecutionResult::Revert {

0 commit comments

Comments
 (0)