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

chore(rpc): make TransactionCompat::fill stateful #11732

Merged
merged 12 commits into from
Oct 24, 2024
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/optimism/rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ serde_json.workspace = true
# misc
thiserror.workspace = true
tracing.workspace = true
derive_more.workspace = true
derive_more = { workspace = true, features = ["constructor", "deref"] }

[dev-dependencies]
reth-optimism-chainspec.workspace = true
Expand Down
19 changes: 16 additions & 3 deletions crates/optimism/rpc/src/eth/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use reth_tasks::{
};
use reth_transaction_pool::TransactionPool;

use crate::{OpEthApiError, OpTxBuilder, SequencerClient};
use crate::{OpEthApiError, SequencerClient};

/// Adapter for [`EthApiInner`], which holds all the data required to serve core `eth_` API.
pub type EthApiNodeBackend<N> = EthApiInner<
Expand All @@ -59,7 +59,7 @@ pub type EthApiNodeBackend<N> = EthApiInner<
///
/// This type implements the [`FullEthApi`](reth_rpc_eth_api::helpers::FullEthApi) by implemented
/// all the `Eth` helper traits and prerequisite traits.
#[derive(Clone, Deref)]
#[derive(Deref)]
pub struct OpEthApi<N: FullNodeComponents> {
/// Gateway to node's core components.
#[deref]
Expand Down Expand Up @@ -102,7 +102,11 @@ where
{
type Error = OpEthApiError;
type NetworkTypes = Optimism;
type TransactionCompat = OpTxBuilder;
type TransactionCompat = Self;

fn tx_resp_builder(&self) -> &Self::TransactionCompat {
self
}
}

impl<N> EthApiSpec for OpEthApi<N>
Expand Down Expand Up @@ -249,3 +253,12 @@ impl<N: FullNodeComponents> fmt::Debug for OpEthApi<N> {
f.debug_struct("OpEthApi").finish_non_exhaustive()
}
}

impl<N> Clone for OpEthApi<N>
where
N: FullNodeComponents,
{
fn clone(&self) -> Self {
Self { inner: self.inner.clone(), sequencer_client: self.sequencer_client.clone() }
}
}
30 changes: 21 additions & 9 deletions crates/optimism/rpc/src/eth/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use alloy_rpc_types::TransactionInfo;
use op_alloy_rpc_types::Transaction;
use reth_node_api::FullNodeComponents;
use reth_primitives::TransactionSignedEcRecovered;
use reth_provider::{BlockReaderIdExt, TransactionsProvider};
use reth_provider::{BlockReaderIdExt, ReceiptProvider, TransactionsProvider};
use reth_rpc::eth::EthTxBuilder;
use reth_rpc_eth_api::{
helpers::{EthSigner, EthTransactions, LoadTransaction, SpawnBlocking},
Expand Down Expand Up @@ -88,30 +88,42 @@ where
}
}

/// Builds OP transaction response type.
#[derive(Clone, Debug, Copy)]
pub struct OpTxBuilder;

impl TransactionCompat for OpTxBuilder {
impl<N> TransactionCompat for OpEthApi<N>
where
N: FullNodeComponents,
{
type Transaction = Transaction;

fn fill(tx: TransactionSignedEcRecovered, tx_info: TransactionInfo) -> Self::Transaction {
fn fill(
&self,
tx: TransactionSignedEcRecovered,
tx_info: TransactionInfo,
) -> Self::Transaction {
let signed_tx = tx.clone().into_signed();
let hash = tx.hash;

let mut inner = EthTxBuilder::fill(tx, tx_info).inner;
let mut inner = EthTxBuilder.fill(tx, tx_info).inner;

if signed_tx.is_deposit() {
inner.gas_price = Some(signed_tx.max_fee_per_gas())
}

let deposit_receipt_version = self
.inner
.provider()
.receipt_by_hash(hash)
.ok() // todo: change sig to return result
.flatten()
.and_then(|receipt| receipt.deposit_receipt_version);
Comment on lines +111 to +117
Copy link
Member

Choose a reason for hiding this comment

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

let's do the todo here, maybe the error can be an associated type?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's quite a big change, effects signature of more rpc compat signatures, suggest it's done in a separate pr

Copy link
Member

Choose a reason for hiding this comment

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

alright, let's make an issue to track

Copy link
Member Author

Choose a reason for hiding this comment

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


Transaction {
inner,
source_hash: signed_tx.source_hash(),
mint: signed_tx.mint(),
// only include is_system_tx if true: <https://github.com/ethereum-optimism/op-geth/blob/641e996a2dcf1f81bac9416cb6124f86a69f1de7/internal/ethapi/api.go#L1518-L1518>
is_system_tx: (signed_tx.is_deposit() && signed_tx.is_system_transaction())
.then_some(true),
deposit_receipt_version: None, // todo: how to fill this field?
deposit_receipt_version,
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/optimism/rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ pub mod eth;
pub mod sequencer;

pub use error::{OpEthApiError, OptimismInvalidTransactionError, SequencerClientError};
pub use eth::{transaction::OpTxBuilder, OpEthApi, OpReceiptBuilder};
pub use eth::{OpEthApi, OpReceiptBuilder};
pub use sequencer::SequencerClient;
4 changes: 3 additions & 1 deletion crates/rpc/rpc-builder/src/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub struct EthHandlers<Provider, Pool, Network, Events, EthApi: EthApiTypes> {
/// Polling based filter handler available on all transports
pub filter: EthFilter<Provider, Pool, EthApi>,
/// Handler for subscriptions only available for transports that support it (ws, ipc)
pub pubsub: EthPubSub<Provider, Pool, Events, Network, EthApi>,
pub pubsub: EthPubSub<Provider, Pool, Events, Network, EthApi::TransactionCompat>,
}

impl<Provider, Pool, Network, Events, EthApi> EthHandlers<Provider, Pool, Network, Events, EthApi>
Expand Down Expand Up @@ -94,6 +94,7 @@ where
ctx.cache.clone(),
ctx.config.filter_config(),
Box::new(ctx.executor.clone()),
api.tx_resp_builder().clone(),
);

let pubsub = EthPubSub::with_spawner(
Expand All @@ -102,6 +103,7 @@ where
ctx.events.clone(),
ctx.network.clone(),
Box::new(ctx.executor.clone()),
api.tx_resp_builder().clone(),
);

Self { api, cache: ctx.cache, filter, pubsub }
Expand Down
9 changes: 6 additions & 3 deletions crates/rpc/rpc-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1199,9 +1199,12 @@ where
.into_rpc()
.into(),
RethRpcModule::Web3 => Web3Api::new(self.network.clone()).into_rpc().into(),
RethRpcModule::Txpool => {
TxPoolApi::<_, EthApi>::new(self.pool.clone()).into_rpc().into()
}
RethRpcModule::Txpool => TxPoolApi::new(
self.pool.clone(),
self.eth.api.tx_resp_builder().clone(),
)
.into_rpc()
.into(),
RethRpcModule::Rpc => RPCApi::new(
namespaces
.iter()
Expand Down
2 changes: 1 addition & 1 deletion crates/rpc/rpc-eth-api/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ where
trace!(target: "rpc::eth", ?hash, "Serving eth_getTransactionByHash");
Ok(EthTransactions::transaction_by_hash(self, hash)
.await?
.map(|tx| tx.into_transaction::<T::TransactionCompat>()))
.map(|tx| tx.into_transaction(self.tx_resp_builder())))
}

/// Handler for: `eth_getRawTransactionByBlockHashAndIndex`
Expand Down
3 changes: 2 additions & 1 deletion crates/rpc/rpc-eth-api/src/helpers/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,12 @@ pub trait EthBlocks: LoadBlock {
.map_err(Self::Error::from_eth_err)?;
}

let block = from_block::<Self::TransactionCompat>(
let block = from_block(
(*block).clone().unseal(),
total_difficulty.unwrap_or_default(),
full.into(),
Some(block_hash),
self.tx_resp_builder(),
)
.map_err(Self::Error::from_eth_err)?;
Ok(Some(block))
Expand Down
3 changes: 2 additions & 1 deletion crates/rpc/rpc-eth-api/src/helpers/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,14 +191,15 @@ pub trait EthCall: Call + LoadPendingBlock {
results.push((env.tx.caller, res.result));
}

let block = simulate::build_block::<Self::TransactionCompat>(
let block = simulate::build_block(
results,
transactions,
&block_env,
parent_hash,
total_difficulty,
return_full_transactions,
&db,
this.tx_resp_builder(),
)?;

parent_hash = block.inner.header.hash;
Expand Down
8 changes: 5 additions & 3 deletions crates/rpc/rpc-eth-api/src/helpers/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,10 @@ pub trait EthTransactions: LoadTransaction {
index: Some(index as u64),
};

return Ok(Some(from_recovered_with_block_context::<Self::TransactionCompat>(
return Ok(Some(from_recovered_with_block_context(
tx.clone().with_signer(*signer),
tx_info,
self.tx_resp_builder(),
)))
}
}
Expand All @@ -237,7 +238,7 @@ pub trait EthTransactions: LoadTransaction {
LoadState::pool(self).get_transaction_by_sender_and_nonce(sender, nonce)
{
let transaction = tx.transaction.clone().into_consensus();
return Ok(Some(from_recovered::<Self::TransactionCompat>(transaction.into())));
return Ok(Some(from_recovered(transaction.into(), self.tx_resp_builder())));
}
}

Expand Down Expand Up @@ -288,9 +289,10 @@ pub trait EthTransactions: LoadTransaction {
base_fee: base_fee_per_gas.map(u128::from),
index: Some(index as u64),
};
from_recovered_with_block_context::<Self::TransactionCompat>(
from_recovered_with_block_context(
tx.clone().with_signer(*signer),
tx_info,
self.tx_resp_builder(),
)
})
})
Expand Down
7 changes: 7 additions & 0 deletions crates/rpc/rpc-eth-api/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,19 @@ pub trait EthApiTypes: Send + Sync + Clone {
type NetworkTypes: Network<HeaderResponse = alloy_rpc_types::Header>;
/// Conversion methods for transaction RPC type.
type TransactionCompat: Send + Sync + Clone + fmt::Debug;

/// Returns reference to transaction response builder.
fn tx_resp_builder(&self) -> &Self::TransactionCompat;
}

impl EthApiTypes for () {
type Error = EthApiError;
type NetworkTypes = AnyNetwork;
type TransactionCompat = ();

fn tx_resp_builder(&self) -> &Self::TransactionCompat {
self
}
}

/// Adapter for network specific transaction type.
Expand Down
4 changes: 3 additions & 1 deletion crates/rpc/rpc-eth-types/src/simulate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ where
}

/// Handles outputs of the calls execution and builds a [`SimulatedBlock`].
#[expect(clippy::too_many_arguments)]
pub fn build_block<T: TransactionCompat>(
results: Vec<(Address, ExecutionResult)>,
transactions: Vec<TransactionSigned>,
Expand All @@ -180,6 +181,7 @@ pub fn build_block<T: TransactionCompat>(
total_difficulty: U256,
full_transactions: bool,
db: &CacheDB<StateProviderDatabase<StateProviderTraitObjWrapper<'_>>>,
tx_resp_builder: &T,
) -> Result<SimulatedBlock<Block<T::Transaction>>, EthApiError> {
let mut calls: Vec<SimCallResult> = Vec::with_capacity(results.len());
let mut senders = Vec::with_capacity(results.len());
Expand Down Expand Up @@ -304,6 +306,6 @@ pub fn build_block<T: TransactionCompat>(
let txs_kind =
if full_transactions { BlockTransactionsKind::Full } else { BlockTransactionsKind::Hashes };

let block = from_block::<T>(block, total_difficulty, txs_kind, None)?;
let block = from_block(block, total_difficulty, txs_kind, None, tx_resp_builder)?;
Ok(SimulatedBlock { inner: block, calls })
}
6 changes: 3 additions & 3 deletions crates/rpc/rpc-eth-types/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ impl TransactionSource {
}

/// Conversion into network specific transaction type.
pub fn into_transaction<T: TransactionCompat>(self) -> T::Transaction {
pub fn into_transaction<T: TransactionCompat>(self, resp_builder: &T) -> T::Transaction {
match self {
Self::Pool(tx) => from_recovered::<T>(tx),
Self::Pool(tx) => from_recovered(tx, resp_builder),
Self::Block { transaction, index, block_hash, block_number, base_fee } => {
let tx_info = TransactionInfo {
hash: Some(transaction.hash()),
Expand All @@ -53,7 +53,7 @@ impl TransactionSource {
base_fee: base_fee.map(u128::from),
};

from_recovered_with_block_context::<T>(transaction, tx_info)
from_recovered_with_block_context(transaction, tx_info, resp_builder)
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions crates/rpc/rpc-types-compat/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,8 @@ alloy-serde.workspace = true
alloy-rpc-types-engine.workspace = true
alloy-consensus.workspace = true

# io
serde.workspace = true

[dev-dependencies]
serde_json.workspace = true
8 changes: 6 additions & 2 deletions crates/rpc/rpc-types-compat/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,15 @@ pub fn from_block<T: TransactionCompat>(
total_difficulty: U256,
kind: BlockTransactionsKind,
block_hash: Option<B256>,
tx_resp_builder: &T,
) -> Result<Block<T::Transaction>, BlockError> {
match kind {
BlockTransactionsKind::Hashes => {
Ok(from_block_with_tx_hashes::<T::Transaction>(block, total_difficulty, block_hash))
}
BlockTransactionsKind::Full => from_block_full::<T>(block, total_difficulty, block_hash),
BlockTransactionsKind::Full => {
from_block_full::<T>(block, total_difficulty, block_hash, tx_resp_builder)
}
}
}

Expand Down Expand Up @@ -60,6 +63,7 @@ pub fn from_block_full<T: TransactionCompat>(
mut block: BlockWithSenders,
total_difficulty: U256,
block_hash: Option<B256>,
tx_resp_builder: &T,
) -> Result<Block<T::Transaction>, BlockError> {
let block_hash = block_hash.unwrap_or_else(|| block.block.header.hash_slow());
let block_number = block.block.number;
Expand All @@ -83,7 +87,7 @@ pub fn from_block_full<T: TransactionCompat>(
index: Some(idx as u64),
};

from_recovered_with_block_context::<T>(signed_tx_ec_recovered, tx_info)
from_recovered_with_block_context::<T>(signed_tx_ec_recovered, tx_info, tx_resp_builder)
})
.collect::<Vec<_>>();

Expand Down
Loading
Loading