Skip to content

Commit

Permalink
Don't consider profit too low and consistent db err as errors.
Browse files Browse the repository at this point in the history
  • Loading branch information
dvush committed Oct 1, 2024
1 parent c886cf1 commit 1710cda
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 22 deletions.
28 changes: 20 additions & 8 deletions crates/rbuilder/src/building/builders/block_building_helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ use crate::{
building::{
estimate_payout_gas_limit, tracers::GasUsedSimulationTracer, BlockBuildingContext,
BlockState, BuiltBlockTrace, BuiltBlockTraceError, CriticalCommitOrderError,
EstimatePayoutGasErr, ExecutionError, ExecutionResult, FinalizeResult, PartialBlock,
Sorting,
EstimatePayoutGasErr, ExecutionError, ExecutionResult, FinalizeError, FinalizeResult,
PartialBlock, Sorting,
},
primitives::SimulatedOrder,
roothash::RootHashConfig,
Expand Down Expand Up @@ -115,12 +115,27 @@ pub enum BlockBuildingHelperError {
InsertPayoutTxErr(#[from] crate::building::InsertPayoutTxErr),
#[error("Bundle consistency check failed: {0}")]
BundleConsistencyCheckFailed(#[from] BuiltBlockTraceError),
#[error("Error finalizing block (eg:root hash): {0}")]
FinalizeError(#[from] eyre::Report),
#[error("Error finalizing block: {0}")]
FinalizeError(#[from] FinalizeError),
#[error("Payout tx not allowed for block")]
PayoutTxNotAllowed,
}

impl BlockBuildingHelperError {
/// Non critial error can happen during normal operations of the builder
pub fn is_critical(&self) -> bool {
match self {
BlockBuildingHelperError::FinalizeError(finalize) => {
!finalize.is_consistent_db_view_err()
}
BlockBuildingHelperError::InsertPayoutTxErr(
crate::building::InsertPayoutTxErr::ProfitTooLow,
) => false,
_ => true,
}
}
}

pub struct FinalizeBlockResult {
pub block: Block,
/// Since finalize_block eats the object we need the cached_reads in case we create a new
Expand Down Expand Up @@ -333,10 +348,7 @@ impl<DB: Database + Clone + 'static> BlockBuildingHelper for BlockBuildingHelper
) {
Ok(finalized_block) => finalized_block,
Err(err) => {
if err
.to_string()
.contains("failed to initialize consistent view")
{
if err.is_consistent_db_view_err() {
let last_block_number = self
.provider_factory
.last_block_number()
Expand Down
40 changes: 32 additions & 8 deletions crates/rbuilder/src/building/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use reth_primitives::proofs::calculate_requests_root;

use crate::{
primitives::{Order, OrderId, SimValue, SimulatedOrder, TransactionSignedEcRecoveredWithBlobs},
roothash::{calculate_state_root, RootHashConfig},
roothash::{calculate_state_root, RootHashConfig, RootHashError},
utils::{a2r_withdrawal, calc_gas_limit, timestamp_as_u64, Signer},
};
use ahash::HashSet;
Expand Down Expand Up @@ -409,6 +409,24 @@ pub struct FinalizeResult {
pub txs_blob_sidecars: Vec<Arc<BlobTransactionSidecar>>,
}

#[derive(Debug, thiserror::Error)]
pub enum FinalizeError {
#[error("Root hash error: {0:?}")]
RootHash(#[from] RootHashError),
#[error("Other error: {0:?}")]
Other(#[from] eyre::Report),
}

impl FinalizeError {
/// see `RootHashError::is_consistent_db_view_err`
pub fn is_consistent_db_view_err(&self) -> bool {
match self {
FinalizeError::RootHash(root_hash) => root_hash.is_consistent_db_view_err(),
FinalizeError::Other(_) => false,
}
}
}

impl<Tracer: SimulationTracer> PartialBlock<Tracer> {
pub fn with_tracer<NewTracer: SimulationTracer>(
self,
Expand Down Expand Up @@ -562,7 +580,7 @@ impl<Tracer: SimulationTracer> PartialBlock<Tracer> {
provider_factory: ProviderFactory<DB>,
root_hash_config: RootHashConfig,
root_hash_task_pool: BlockingTaskPool,
) -> eyre::Result<FinalizeResult> {
) -> Result<FinalizeResult, FinalizeError> {
let (withdrawals_root, withdrawals) = {
let mut db = state.new_db_ref();
let WithdrawalsOutcome {
Expand All @@ -573,7 +591,8 @@ impl<Tracer: SimulationTracer> PartialBlock<Tracer> {
&ctx.chain_spec,
ctx.attributes.timestamp,
ctx.attributes.withdrawals.clone(),
)?;
)
.map_err(|err| FinalizeError::Other(err.into()))?;
db.as_mut().merge_transitions(PlainState);
(withdrawals_root, withdrawals)
};
Expand All @@ -586,19 +605,22 @@ impl<Tracer: SimulationTracer> PartialBlock<Tracer> {
let mut db = state.new_db_ref();

let deposit_requests =
parse_deposits_from_receipts(&ctx.chain_spec, self.receipts.iter())?;
parse_deposits_from_receipts(&ctx.chain_spec, self.receipts.iter())
.map_err(|err| FinalizeError::Other(err.into()))?;
let withdrawal_requests = post_block_withdrawal_requests_contract_call(
&evm_config,
db.as_mut(),
&ctx.initialized_cfg,
&ctx.block_env,
)?;
)
.map_err(|err| FinalizeError::Other(err.into()))?;
let consolidation_requests = post_block_consolidation_requests_contract_call(
&evm_config,
db.as_mut(),
&ctx.initialized_cfg,
&ctx.block_env,
)?;
)
.map_err(|err| FinalizeError::Other(err.into()))?;

let requests = [
deposit_requests,
Expand Down Expand Up @@ -647,11 +669,13 @@ impl<Tracer: SimulationTracer> PartialBlock<Tracer> {
// double check blocked txs
for tx_with_blob in &self.executed_tx {
if ctx.blocklist.contains(&tx_with_blob.signer()) {
return Err(eyre::eyre!("To from blocked address."));
return Err(FinalizeError::Other(eyre::eyre!(
"To from blocked address."
)));
}
if let Some(to) = tx_with_blob.to() {
if ctx.blocklist.contains(&to) {
return Err(eyre::eyre!("Tx to blocked address"));
return Err(FinalizeError::Other(eyre::eyre!("Tx to blocked address")));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,15 @@ impl SequentialSealerBidMakerProcess {
match tokio::task::spawn_blocking(move || block.finalize_block(payout_tx_val)).await {
Ok(finalize_res) => match finalize_res {
Ok(res) => self.sink.new_block(res.block),
Err(error) => error!(
block_number,
?error,
"Error on finalize_block on SequentialSealerBidMaker"
),
Err(error) => {
if error.is_critical() {
error!(
block_number,
?error,
"Error on finalize_block on SequentialSealerBidMaker"
)
}
}
},
Err(error) => error!(
block_number,
Expand Down
20 changes: 19 additions & 1 deletion crates/rbuilder/src/roothash/mod.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
use alloy_primitives::B256;
use eth_sparse_mpt::reth_sparse_trie::{
calculate_root_hash_with_sparse_trie, SparseTrieError, SparseTrieSharedCache,
calculate_root_hash_with_sparse_trie, trie_fetcher::FetchNodeError, SparseTrieError,
SparseTrieSharedCache,
};
use reth::{
providers::{providers::ConsistentDbView, ExecutionOutcome, ProviderFactory},
tasks::pool::BlockingTaskPool,
};
use reth_db::database::Database;
use reth_errors::ProviderError;
use reth_trie_parallel::async_root::{AsyncStateRoot, AsyncStateRootError};
use tracing::trace;

Expand All @@ -33,6 +35,22 @@ pub enum RootHashError {
Verification,
}

impl RootHashError {
/// Error of this type means that db does not have trie for the required block
/// This often happens when building for block after it was proposed.
pub fn is_consistent_db_view_err(&self) -> bool {
let provider_error = match self {
RootHashError::AsyncStateRoot(AsyncStateRootError::Provider(p)) => p,
RootHashError::SparseStateRoot(SparseTrieError::FetchNode(
FetchNodeError::Provider(p),
)) => p,
_ => return false,
};

matches!(provider_error, ProviderError::ConsistentView(_))
}
}

#[derive(Debug, Clone)]
pub struct RootHashConfig {
pub mode: RootHashMode,
Expand Down

0 comments on commit 1710cda

Please sign in to comment.