Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
1 change: 0 additions & 1 deletion cmd/ethrex/l2/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,6 @@ impl Command {

let account_updates_list = store
.apply_account_updates_from_trie_batch(trie, account_updates.values())
.await
.map_err(|e| format!("Error applying account updates: {e}"))
.unwrap();

Expand Down
4 changes: 1 addition & 3 deletions crates/blockchain/blockchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,8 +429,7 @@ impl Blockchain {
// Apply the account updates over the last block's state and compute the new state root
let account_updates_list = self
.storage
.apply_account_updates_batch(block.header.parent_hash, &updates)
.await?
.apply_account_updates_batch(block.header.parent_hash, &updates)?
.ok_or(ChainError::ParentStateNotFound)?;

let (gas_used, gas_limit, block_number, transactions_count) = (
Expand Down Expand Up @@ -606,7 +605,6 @@ impl Blockchain {
let account_updates_list = self
.storage
.apply_account_updates_batch(first_block_header.parent_hash, &account_updates)
.await
.map_err(|e| (e.into(), None))?
.ok_or((ChainError::ParentStateNotFound, None))?;

Expand Down
33 changes: 18 additions & 15 deletions crates/blockchain/payload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use crate::{
};

use thiserror::Error;
use tracing::{debug, error};
use tracing::{debug, error, warn};

#[derive(Debug)]
pub struct PayloadBuildTask {
Expand Down Expand Up @@ -370,22 +370,29 @@ impl Blockchain {
let self_clone = self.clone();
const SECONDS_PER_SLOT: Duration = Duration::from_secs(12);
// Attempt to rebuild the payload as many times within the given timeframe to maximize fee revenue
let mut res = self_clone.build_payload(payload.clone()).await?;
// TODO: start with an empty block
let mut res = self_clone.build_payload(payload.clone())?;
while start.elapsed() < SECONDS_PER_SLOT && !cancel_token.is_cancelled() {
let payload = payload.clone();
let self_clone = self.clone();
let building_task =
tokio::task::spawn_blocking(move || self_clone.build_payload(payload));
// Cancel the current build process and return the previous payload if it is requested earlier
if let Some(current_res) = cancel_token
.run_until_cancelled(self_clone.build_payload(payload))
.await
{
res = current_res?;
match cancel_token.run_until_cancelled(building_task).await {
Some(Ok(current_res)) => {
res = current_res?;
}
Some(Err(err)) => {
warn!(%err, "Payload-building task panicked");
}
None => {}
}
}
Ok(res)
}

/// Completes the payload building process, return the block value
pub async fn build_payload(&self, payload: Block) -> Result<PayloadBuildResult, ChainError> {
pub fn build_payload(&self, payload: Block) -> Result<PayloadBuildResult, ChainError> {
let since = Instant::now();
let gas_limit = payload.header.gas_limit;

Expand All @@ -400,7 +407,7 @@ impl Blockchain {
self.apply_withdrawals(&mut context)?;
self.fill_transactions(&mut context)?;
self.extract_requests(&mut context)?;
self.finalize_payload(&mut context).await?;
self.finalize_payload(&mut context)?;

let interval = Instant::now().duration_since(since).as_millis();

Expand Down Expand Up @@ -628,16 +635,12 @@ impl Blockchain {
Ok(())
}

pub async fn finalize_payload(
&self,
context: &mut PayloadBuildContext,
) -> Result<(), ChainError> {
pub fn finalize_payload(&self, context: &mut PayloadBuildContext) -> Result<(), ChainError> {
let account_updates = context.vm.get_state_transitions()?;

let ret_acount_updates_list = self
.storage
.apply_account_updates_batch(context.parent_hash(), &account_updates)
.await?
.apply_account_updates_batch(context.parent_hash(), &account_updates)?
.ok_or(ChainError::ParentStateNotFound)?;

let state_root = ret_acount_updates_list.state_trie_hash;
Expand Down
2 changes: 1 addition & 1 deletion crates/blockchain/smoke_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ mod blockchain_integration_test {
let blockchain = Blockchain::default_with_store(store.clone());

let block = create_payload(&args, store, Bytes::new()).unwrap();
let result = blockchain.build_payload(block).await.unwrap();
let result = blockchain.build_payload(block).unwrap();
result.payload
}

Expand Down
3 changes: 1 addition & 2 deletions crates/l2/sequencer/block_producer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,7 @@ impl BlockProducer {

let account_updates_list = self
.store
.apply_account_updates_batch(block.header.parent_hash, &account_updates)
.await?
.apply_account_updates_batch(block.header.parent_hash, &account_updates)?
.ok_or(ChainError::ParentStateNotFound)?;

let transactions_count = block.body.transactions.len();
Expand Down
2 changes: 1 addition & 1 deletion crates/l2/sequencer/block_producer/payload_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub async fn build_payload(
block_gas_limit,
)
.await?;
blockchain.finalize_payload(&mut context).await?;
blockchain.finalize_payload(&mut context)?;

let interval = Instant::now().duration_since(since).as_millis();
// TODO: expose as a proper metric
Expand Down
14 changes: 7 additions & 7 deletions crates/storage/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ impl Store {
/// Applies account updates based on the block's latest storage state
/// and returns the new state root after the updates have been applied.
#[instrument(level = "trace", name = "Trie update", skip_all)]
pub async fn apply_account_updates_batch(
Copy link
Collaborator

@mpaulucci mpaulucci Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im quite confused about this change (probably due to lack of knowledge). Intuitively I would expect ALL store functions to be async. And also not have to put spawn_blocking in places, which are code smell imo

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for the record, I also think that Store functions should be async. However, I prefer functions to be marked as async only when they are actually asynchronous. This one didn't call any async functions, so the keyword only misleads. What's more, I suspect the compiler actually removes the keyword internally, and I'm upset Clippy doesn't mark cases like this 😠

I agree that a spawn_blocking is a smell when intermingled with async code, but in this case, it's justified: building a block is mostly a synchronous task. To avoid using spawn_blocking, we'd need to rework the EVM itself to be async, or keep a dedicated threadpool for EVMs (essentially making our own spawn_blocking).

pub fn apply_account_updates_batch(
&self,
block_hash: BlockHash,
account_updates: &[AccountUpdate],
Expand All @@ -370,16 +370,16 @@ impl Store {
return Ok(None);
};

Ok(Some(
self.apply_account_updates_from_trie_batch(state_trie, account_updates)
.await?,
))
Ok(Some(self.apply_account_updates_from_trie_batch(
state_trie,
account_updates,
)?))
}

pub async fn apply_account_updates_from_trie_batch(
pub fn apply_account_updates_from_trie_batch<'a>(
&self,
mut state_trie: Trie,
account_updates: impl IntoIterator<Item = &AccountUpdate>,
account_updates: impl IntoIterator<Item = &'a AccountUpdate>,
) -> Result<AccountUpdatesList, StoreError> {
let mut ret_storage_updates = Vec::new();
let mut code_updates = Vec::new();
Expand Down
5 changes: 2 additions & 3 deletions tooling/ef_tests/state_v2/src/modules/result_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ pub async fn check_root(
) -> Result<(), RunnerError> {
let account_updates = backends::levm::LEVM::get_state_transitions(&mut vm.db.clone())
.map_err(|e| RunnerError::FailedToGetAccountsUpdates(e.to_string()))?;
let post_state_root = post_state_root(&account_updates, initial_block_hash, store).await;
let post_state_root = post_state_root(&account_updates, initial_block_hash, store);
if post_state_root != test_case.post.hash {
check_result.passed = false;
check_result.root_diff = Some((test_case.post.hash, post_state_root));
Expand All @@ -116,14 +116,13 @@ pub async fn check_root(

/// Calculates the post state root applying the changes (the account updates) that are a
/// result of running the transaction to the storage.
pub async fn post_state_root(
pub fn post_state_root(
account_updates: &[AccountUpdate],
initial_block_hash: H256,
store: Store,
) -> H256 {
let ret_account_updates_batch = store
.apply_account_updates_batch(initial_block_hash, account_updates)
.await
.unwrap()
.unwrap();
ret_account_updates_batch.state_trie_hash
Expand Down