Skip to content
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

feat(4337): Enforce ConditionalOptions check during payload building #76

Merged
merged 5 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
22 changes: 19 additions & 3 deletions world-chain-builder/src/payload/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ use reth_optimism_payload_builder::error::OptimismPayloadBuilderError;
use reth_primitives::{proofs, BlockBody};
use reth_primitives::{Block, Header, Receipt, TxType};
use reth_provider::{
CanonStateSubscriptions, ChainSpecProvider, ExecutionOutcome, StateProviderFactory,
BlockReaderIdExt, CanonStateSubscriptions, ChainSpecProvider, ExecutionOutcome,
StateProviderFactory,
};
use reth_trie::HashedPostState;
use revm_primitives::calc_excess_blob_gas;
Expand All @@ -45,6 +46,7 @@ use tracing::{debug, trace, warn};

use crate::pool::noop::NoopWorldChainTransactionPool;
use crate::pool::tx::WorldChainPoolTransaction;
use crate::rpc::bundle::validate_conditional_options;

/// Priority blockspace for humans builder
#[derive(Debug, Clone)]
Expand Down Expand Up @@ -72,7 +74,7 @@ where
/// Implementation of the [`PayloadBuilder`] trait for [`WorldChainPayloadBuilder`].
impl<Pool, Client, EvmConfig> PayloadBuilder<Pool, Client> for WorldChainPayloadBuilder<EvmConfig>
where
Client: StateProviderFactory + ChainSpecProvider<ChainSpec = OpChainSpec>,
Client: StateProviderFactory + ChainSpecProvider<ChainSpec = OpChainSpec> + BlockReaderIdExt,
Pool: TransactionPool<Transaction: WorldChainPoolTransaction>,
EvmConfig: ConfigureEvm<Header = Header>,
{
Expand Down Expand Up @@ -218,7 +220,7 @@ pub(crate) fn worldchain_payload<EvmConfig, Pool, Client>(
) -> Result<BuildOutcome<OptimismBuiltPayload>, PayloadBuilderError>
where
EvmConfig: ConfigureEvm<Header = Header>,
Client: StateProviderFactory + ChainSpecProvider<ChainSpec = OpChainSpec>,
Client: StateProviderFactory + ChainSpecProvider<ChainSpec = OpChainSpec> + BlockReaderIdExt,
Pool: TransactionPool<Transaction: WorldChainPoolTransaction>,
{
let BuildArguments {
Expand Down Expand Up @@ -406,8 +408,17 @@ where
}

if !attributes.no_tx_pool {
let mut invalid_txs = vec![];
let verified_gas_limit = (verified_blockspace_capacity as u64 * block_gas_limit) / 100;
while let Some(pool_tx) = best_txs.next() {
if let Some(conditional_options) = pool_tx.transaction.conditional_options() {
0xKitsune marked this conversation as resolved.
Show resolved Hide resolved
if let Err(_) = validate_conditional_options(conditional_options, &client) {
best_txs.mark_invalid(&pool_tx);
invalid_txs.push(pool_tx.hash().clone());
continue;
}
}

// If the transaction is verified, check if it can be added within the verified gas limit
if pool_tx.transaction.pbh_payload().is_some()
&& cumulative_gas_used + pool_tx.gas_limit() > verified_gas_limit
Expand Down Expand Up @@ -503,6 +514,10 @@ where
executed_senders.push(tx.signer());
executed_txs.push(tx.into_signed());
}

if !invalid_txs.is_empty() {
pool.remove_transactions(invalid_txs);
}
}

// check if we have a better block
Expand Down Expand Up @@ -995,6 +1010,7 @@ mod tests {
WorldChainPooledTransaction {
inner: pooled_tx,
pbh_payload,
conditional_options: None,
}
})
.collect::<Vec<_>>()
Expand Down
12 changes: 11 additions & 1 deletion world-chain-builder/src/pool/tx.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use alloy_primitives::TxHash;
use alloy_rpc_types::erc4337::ConditionalOptions;
use reth::transaction_pool::{EthPoolTransaction, EthPooledTransaction, PoolTransaction};
use reth_primitives::transaction::TryFromRecoveredTransactionError;
use reth_primitives::{PooledTransactionsElementEcRecovered, TransactionSignedEcRecovered};
Expand All @@ -9,12 +10,14 @@ use crate::primitives::WorldChainPooledTransactionsElementEcRecovered;

pub trait WorldChainPoolTransaction: EthPoolTransaction {
fn pbh_payload(&self) -> Option<&PbhPayload>;
fn conditional_options(&self) -> Option<&ConditionalOptions>;
}

#[derive(Debug, Clone, PartialEq, Eq)]
#[derive(Debug, Clone)]
pub struct WorldChainPooledTransaction {
pub inner: EthPooledTransaction,
pub pbh_payload: Option<PbhPayload>,
pub conditional_options: Option<ConditionalOptions>,
}

impl EthPoolTransaction for WorldChainPooledTransaction {
Expand Down Expand Up @@ -43,6 +46,10 @@ impl WorldChainPoolTransaction for WorldChainPooledTransaction {
fn pbh_payload(&self) -> Option<&PbhPayload> {
self.pbh_payload.as_ref()
}

fn conditional_options(&self) -> Option<&ConditionalOptions> {
self.conditional_options.as_ref()
}
}

impl From<WorldChainPooledTransaction> for TransactionSignedEcRecovered {
Expand All @@ -58,6 +65,7 @@ impl TryFrom<TransactionSignedEcRecovered> for WorldChainPooledTransaction {
Ok(Self {
inner: EthPooledTransaction::try_from(tx)?,
pbh_payload: None,
conditional_options: None,
})
}
}
Expand All @@ -67,6 +75,7 @@ impl From<WorldChainPooledTransactionsElementEcRecovered> for WorldChainPooledTr
Self {
inner: EthPooledTransaction::from_pooled(tx.inner),
pbh_payload: tx.pbh_payload,
conditional_options: None,
}
}
}
Expand Down Expand Up @@ -99,6 +108,7 @@ impl PoolTransaction for WorldChainPooledTransaction {
EthPooledTransaction::try_from_consensus(tx).map(|inner| Self {
inner,
pbh_payload: None,
conditional_options: None,
})
}

Expand Down
167 changes: 89 additions & 78 deletions world-chain-builder/src/rpc/bundle.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{pool::tx::WorldChainPooledTransaction, primitives::recover_raw_transaction};
use alloy_eips::BlockId;
use alloy_primitives::map::HashMap;
use alloy_primitives::{map::HashMap, StorageKey};
use alloy_rpc_types::erc4337::{AccountStorage, ConditionalOptions};
use jsonrpsee::{
core::{async_trait, RpcResult},
Expand Down Expand Up @@ -44,9 +44,11 @@ where
tx: Bytes,
options: ConditionalOptions,
) -> RpcResult<B256> {
self.validate_options(options)?;
validate_conditional_options(&options, self.provider())?;

let (recovered, _) = recover_raw_transaction(tx.clone())?;
let pool_transaction = WorldChainPooledTransaction::from_pooled(recovered);
let mut pool_transaction = WorldChainPooledTransaction::from_pooled(recovered);
pool_transaction.conditional_options = Some(options);

// submit the transaction to the pool with a `Local` origin
let hash = self
Expand Down Expand Up @@ -75,97 +77,106 @@ where
pub fn pool(&self) -> &Pool {
&self.pool
}
}

/// Validates the conditional inclusion options provided by the client.
///
/// reference for the implementation <https://notes.ethereum.org/@yoav/SkaX2lS9j#>
/// See also <https://pkg.go.dev/github.com/aK0nshin/go-ethereum/arbitrum_types#ConditionalOptions>
pub fn validate_options(&self, options: ConditionalOptions) -> RpcResult<()> {
let latest = self
.provider()
.block_by_id(BlockId::latest())
.map_err(|e| {
ErrorObject::owned(ErrorCode::InternalError.code(), e.to_string(), Some(""))
})?
.ok_or(ErrorObjectOwned::from(ErrorCode::InternalError))?;

self.validate_known_accounts(options.known_accounts, latest.header.number.into())?;

if let Some(min_block) = options.block_number_min {
if min_block > latest.number {
return Err(ErrorCode::from(-32003).into());
}
/// Validates the conditional inclusion options provided by the client.
///
/// reference for the implementation <https://notes.ethereum.org/@yoav/SkaX2lS9j#>
/// See also <https://pkg.go.dev/github.com/aK0nshin/go-ethereum/arbitrum_types#ConditionalOptions>
pub fn validate_conditional_options<Client>(
options: &ConditionalOptions,
provider: &Client,
) -> RpcResult<()>
where
Client: BlockReaderIdExt + StateProviderFactory,
{
let latest = provider
.block_by_id(BlockId::latest())
0xKitsune marked this conversation as resolved.
Show resolved Hide resolved
.map_err(|e| ErrorObject::owned(ErrorCode::InternalError.code(), e.to_string(), Some("")))?
.ok_or(ErrorObjectOwned::from(ErrorCode::InternalError))?;

validate_known_accounts(
&options.known_accounts,
latest.header.number.into(),
provider,
)?;

if let Some(min_block) = options.block_number_min {
if min_block > latest.number {
return Err(ErrorCode::from(-32003).into());
}
}

if let Some(max_block) = options.block_number_max {
if max_block <= latest.number {
return Err(ErrorCode::from(-32003).into());
}
if let Some(max_block) = options.block_number_max {
if max_block <= latest.number {
0xKitsune marked this conversation as resolved.
Show resolved Hide resolved
return Err(ErrorCode::from(-32003).into());
}
}

if let Some(min_timestamp) = options.timestamp_min {
if min_timestamp > latest.timestamp {
return Err(ErrorCode::from(-32003).into());
}
if let Some(min_timestamp) = options.timestamp_min {
if min_timestamp > latest.timestamp {
return Err(ErrorCode::from(-32003).into());
}
}

if let Some(max_timestamp) = options.timestamp_max {
if max_timestamp <= latest.timestamp {
return Err(ErrorCode::from(-32003).into());
}
if let Some(max_timestamp) = options.timestamp_max {
if max_timestamp <= latest.timestamp {
0xKitsune marked this conversation as resolved.
Show resolved Hide resolved
return Err(ErrorCode::from(-32003).into());
}

Ok(())
}

/// Validates the account storage slots/storage root provided by the client
///
/// Matches the current state of the account storage slots/storage root.
pub fn validate_known_accounts(
&self,
known_accounts: HashMap<Address, AccountStorage, FbBuildHasher<20>>,
latest: BlockId,
) -> RpcResult<()> {
let state = self.provider().state_by_block_id(latest).map_err(|e| {
ErrorObject::owned(ErrorCode::InternalError.code(), e.to_string(), Some(""))
})?;

for (address, storage) in known_accounts.into_iter() {
match storage {
AccountStorage::Slots(slots) => {
for (slot, value) in slots.into_iter() {
let current = state.storage(address, slot.into()).map_err(|e| {
ErrorObject::owned(
ErrorCode::InternalError.code(),
e.to_string(),
Some(""),
)
})?;
if let Some(current) = current {
if FixedBytes::<32>::from_slice(&current.to_be_bytes::<32>()) != value {
return Err(ErrorCode::from(-32003).into());
}
} else {
Ok(())
}

/// Validates the account storage slots/storage root provided by the client
///
/// Matches the current state of the account storage slots/storage root.
pub fn validate_known_accounts<Client>(
known_accounts: &HashMap<Address, AccountStorage, FbBuildHasher<20>>,
latest: BlockId,
provider: &Client,
) -> RpcResult<()>
where
Client: BlockReaderIdExt + StateProviderFactory,
{
let state = provider.state_by_block_id(latest).map_err(|e| {
ErrorObject::owned(ErrorCode::InternalError.code(), e.to_string(), Some(""))
})?;

for (address, storage) in known_accounts.iter() {
match storage {
AccountStorage::Slots(slots) => {
for (slot, value) in slots.iter() {
let current =
state
.storage(*address, StorageKey::from(*slot))
.map_err(|e| {
ErrorObject::owned(
ErrorCode::InternalError.code(),
e.to_string(),
Some(""),
)
})?;
if let Some(current) = current {
if FixedBytes::<32>::from_slice(&current.to_be_bytes::<32>()) != *value {
return Err(ErrorCode::from(-32003).into());
}
}
}
AccountStorage::RootHash(expected) => {
let root = state
.storage_root(address, Default::default())
.map_err(|e| {
ErrorObject::owned(
ErrorCode::InternalError.code(),
e.to_string(),
Some(""),
)
})?;
if *expected != root {
} else {
return Err(ErrorCode::from(-32003).into());
}
}
}
AccountStorage::RootHash(expected) => {
let root = state
.storage_root(*address, Default::default())
.map_err(|e| {
ErrorObject::owned(ErrorCode::InternalError.code(), e.to_string(), Some(""))
})?;
if *expected != root {
return Err(ErrorCode::from(-32003).into());
}
}
}
Ok(())
}
Ok(())
}
2 changes: 2 additions & 0 deletions world-chain-builder/src/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pub fn get_non_pbh_transaction() -> WorldChainPooledTransaction {
WorldChainPooledTransaction {
inner: eth_tx,
pbh_payload: None,
conditional_options: None,
}
}

Expand All @@ -53,6 +54,7 @@ pub fn get_pbh_transaction(nonce: u16) -> WorldChainPooledTransaction {
WorldChainPooledTransaction {
inner: eth_tx,
pbh_payload: Some(pbh_payload),
conditional_options: None,
}
}

Expand Down
Loading