-
Notifications
You must be signed in to change notification settings - Fork 24
Refactor payload builder to accept generic builder tx #217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0f64c49
3c48c31
c860fed
07d9965
8bd5226
6c8681d
a95aae2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,32 +1,169 @@ | ||
use alloy_evm::Database; | ||
use alloy_primitives::{ | ||
Address, | ||
map::foldhash::{HashSet, HashSetExt}, | ||
}; | ||
use core::fmt::Debug; | ||
use op_revm::OpTransactionError; | ||
use reth_evm::{ConfigureEvm, Evm, eth::receipt_builder::ReceiptBuilderCtx}; | ||
use reth_node_api::PayloadBuilderError; | ||
use reth_optimism_primitives::OpTransactionSigned; | ||
use reth_primitives::Recovered; | ||
use reth_provider::{ProviderError, StateProvider}; | ||
use reth_revm::{ | ||
State, database::StateProviderDatabase, db::states::bundle_state::BundleRetention, | ||
}; | ||
use revm::{ | ||
DatabaseCommit, | ||
context::result::{EVMError, ResultAndState}, | ||
}; | ||
use tracing::warn; | ||
|
||
use crate::tx_signer::Signer; | ||
use crate::{builders::context::OpPayloadBuilderCtx, primitives::reth::ExecutionInfo}; | ||
|
||
pub trait BuilderTx { | ||
fn estimated_builder_tx_gas(&self) -> u64; | ||
fn estimated_builder_tx_da_size(&self) -> Option<u64>; | ||
fn signed_builder_tx(&self) -> Result<Recovered<OpTransactionSigned>, secp256k1::Error>; | ||
#[derive(Debug, Clone)] | ||
pub struct BuilderTransactionCtx { | ||
pub gas_used: u64, | ||
pub da_size: u64, | ||
pub signed_tx: Recovered<OpTransactionSigned>, | ||
} | ||
|
||
// Scaffolding for how to construct the end of block builder transaction | ||
// This will be the regular end of block transaction without the TEE key | ||
#[derive(Clone)] | ||
pub(super) struct StandardBuilderTx { | ||
#[allow(dead_code)] | ||
pub signer: Option<Signer>, | ||
/// Possible error variants during construction of builder txs. | ||
#[derive(Debug, thiserror::Error)] | ||
pub enum BuilderTransactionError { | ||
/// Builder account load fails to get builder nonce | ||
#[error("failed to load account {0}")] | ||
AccountLoadFailed(Address), | ||
/// Signature signing fails | ||
#[error("failed to sign transaction: {0}")] | ||
SigningError(secp256k1::Error), | ||
/// Unrecoverable error during evm execution. | ||
#[error("evm execution error {0}")] | ||
EvmExecutionError(Box<dyn core::error::Error + Send + Sync>), | ||
/// Any other builder transaction errors. | ||
#[error(transparent)] | ||
Other(Box<dyn core::error::Error + Send + Sync>), | ||
} | ||
|
||
impl BuilderTx for StandardBuilderTx { | ||
fn estimated_builder_tx_gas(&self) -> u64 { | ||
todo!() | ||
impl From<secp256k1::Error> for BuilderTransactionError { | ||
fn from(error: secp256k1::Error) -> Self { | ||
BuilderTransactionError::SigningError(error) | ||
} | ||
} | ||
|
||
impl From<EVMError<ProviderError, OpTransactionError>> for BuilderTransactionError { | ||
fn from(error: EVMError<ProviderError, OpTransactionError>) -> Self { | ||
BuilderTransactionError::EvmExecutionError(Box::new(error)) | ||
} | ||
} | ||
|
||
fn estimated_builder_tx_da_size(&self) -> Option<u64> { | ||
todo!() | ||
impl From<BuilderTransactionError> for PayloadBuilderError { | ||
fn from(error: BuilderTransactionError) -> Self { | ||
match error { | ||
BuilderTransactionError::EvmExecutionError(e) => { | ||
PayloadBuilderError::EvmExecutionError(e) | ||
} | ||
_ => PayloadBuilderError::Other(Box::new(error)), | ||
} | ||
} | ||
} | ||
|
||
pub trait BuilderTransactions<ExtraCtx: Debug + Default = ()>: Debug { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it'd be a lot cleaner to have this as an enum as opposed to a trait, so you don't need to use generic eg. pub enum BuilderTransactions {
Flashblocks(FlashblocksBuilderTx),
Standard(StandardBuilderTx),
Flashtestations(FlashtestationsBuilderTx),
}
impl BuilderTransactions {
pub fn simulate_builder_txs(&self, ...) {
match self {
Self::Flashblocks(inner) => inner.simulate_builder_txs(...),
Self::Standard(inner) => inner.simulate_builder_txs(...),
Self::Flashtestations(inner) => inner.simulate_builder_txs(...),
}
}
pub fn add_builder_txs(&self, ...) { ... }
pub fn simulate_builder_txs_state(&self, ...) {...}
} you can also get rid of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good but I would try the enum approach in a follow up PR to refactor the existing generic builder tx There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that would be great! |
||
fn simulate_builder_txs<Extra: Debug + Default>( | ||
&self, | ||
state_provider: impl StateProvider + Clone, | ||
info: &mut ExecutionInfo<Extra>, | ||
ctx: &OpPayloadBuilderCtx<ExtraCtx>, | ||
db: &mut State<impl Database>, | ||
) -> Result<Vec<BuilderTransactionCtx>, BuilderTransactionError>; | ||
|
||
fn add_builder_txs<Extra: Debug + Default>( | ||
&self, | ||
state_provider: impl StateProvider + Clone, | ||
info: &mut ExecutionInfo<Extra>, | ||
builder_ctx: &OpPayloadBuilderCtx<ExtraCtx>, | ||
db: &mut State<impl Database>, | ||
) -> Result<Vec<BuilderTransactionCtx>, BuilderTransactionError> { | ||
{ | ||
let mut evm = builder_ctx | ||
.evm_config | ||
.evm_with_env(&mut *db, builder_ctx.evm_env.clone()); | ||
|
||
let mut invalid: HashSet<Address> = HashSet::new(); | ||
|
||
let builder_txs = | ||
self.simulate_builder_txs(state_provider, info, builder_ctx, evm.db_mut())?; | ||
for builder_tx in builder_txs.iter() { | ||
if invalid.contains(&builder_tx.signed_tx.signer()) { | ||
warn!(target: "payload_builder", tx_hash = ?builder_tx.signed_tx.tx_hash(), "builder signer invalid as previous builder tx reverted"); | ||
continue; | ||
} | ||
|
||
let ResultAndState { result, state } = evm | ||
.transact(&builder_tx.signed_tx) | ||
.map_err(|err| BuilderTransactionError::EvmExecutionError(Box::new(err)))?; | ||
|
||
if !result.is_success() { | ||
warn!(target: "payload_builder", tx_hash = ?builder_tx.signed_tx.tx_hash(), "builder tx reverted"); | ||
invalid.insert(builder_tx.signed_tx.signer()); | ||
continue; | ||
} | ||
|
||
// Add gas used by the transaction to cumulative gas used, before creating the receipt | ||
let gas_used = result.gas_used(); | ||
info.cumulative_gas_used += gas_used; | ||
|
||
let ctx = ReceiptBuilderCtx { | ||
tx: builder_tx.signed_tx.inner(), | ||
evm: &evm, | ||
result, | ||
state: &state, | ||
cumulative_gas_used: info.cumulative_gas_used, | ||
}; | ||
info.receipts.push(builder_ctx.build_receipt(ctx, None)); | ||
|
||
// Commit changes | ||
evm.db_mut().commit(state); | ||
|
||
// Append sender and transaction to the respective lists | ||
info.executed_senders.push(builder_tx.signed_tx.signer()); | ||
info.executed_transactions | ||
.push(builder_tx.signed_tx.clone().into_inner()); | ||
} | ||
|
||
// Release the db reference by dropping evm | ||
drop(evm); | ||
|
||
Ok(builder_txs) | ||
} | ||
} | ||
|
||
fn simulate_builder_txs_state<Extra: Debug + Default>( | ||
avalonche marked this conversation as resolved.
Show resolved
Hide resolved
|
||
&self, | ||
state_provider: impl StateProvider + Clone, | ||
builder_txs: Vec<&BuilderTransactionCtx>, | ||
ctx: &OpPayloadBuilderCtx<ExtraCtx>, | ||
db: &mut State<impl Database>, | ||
) -> Result<State<StateProviderDatabase<impl StateProvider>>, BuilderTransactionError> { | ||
let state = StateProviderDatabase::new(state_provider.clone()); | ||
let mut simulation_state = State::builder() | ||
.with_database(state) | ||
.with_bundle_prestate(db.bundle_state.clone()) | ||
.with_bundle_update() | ||
.build(); | ||
let mut evm = ctx | ||
.evm_config | ||
.evm_with_env(&mut simulation_state, ctx.evm_env.clone()); | ||
|
||
for builder_tx in builder_txs { | ||
let ResultAndState { state, .. } = evm | ||
.transact(&builder_tx.signed_tx) | ||
.map_err(|err| BuilderTransactionError::EvmExecutionError(Box::new(err)))?; | ||
|
||
evm.db_mut().commit(state); | ||
evm.db_mut().merge_transitions(BundleRetention::Reverts); | ||
} | ||
|
||
fn signed_builder_tx(&self) -> Result<Recovered<OpTransactionSigned>, secp256k1::Error> { | ||
todo!() | ||
Ok(simulation_state) | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.