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.

Sign up for GitHub

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

eth_sendRawTransactionConditional L2 RPC endpoint #13762

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

eth_sendRawTransactionConditional L2 RPC endpoint #13762

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f98ea5a
add blanket implementation of L2EthApiExtServer for T
Jan 10, 2025
0c967ab
update trace log message to eth_sendRawTransactionConditional
Jan 10, 2025
5bded0d
ignore unused condition variable
Jan 10, 2025
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
20 changes: 20 additions & 0 deletions crates/rpc/rpc-eth-api/src/ext.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
use alloy_primitives::{Bytes, B256};
use alloy_rpc_types_eth::erc4337::ConditionalOptions;
use jsonrpsee::{core::RpcResult, proc_macros::rpc};
use tracing::trace;

use crate::helpers::{EthTransactions, FullEthApi};

/// Extension trait for `eth_` namespace for L2s.
#[cfg_attr(not(feature = "client"), rpc(server, namespace = "eth"))]
Expand All @@ -16,3 +19,20 @@ pub trait L2EthApiExt {
condition: ConditionalOptions,
) -> RpcResult<B256>;
}

#[async_trait::async_trait]
impl<T> L2EthApiExtServer for T
where
T: FullEthApi,
jsonrpsee_types::error::ErrorObject<'static>: From<T::Error>,
{
Comment on lines +23 to +28
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can't implement this default like this because we need to actually handle the conditionals.

can we start with a new helper type in

reth/crates/optimism/rpc/Cargo.toml

Line 2 in 5db0129

name = "reth-optimism-rpc"

instead, that wraps P: Pool?

like struct L2TransactionConditional<P>{ pool: P}

Copy link

@hamdiallam hamdiallam Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Based on the impl in op-geth, we don't check the conditional in the pool since that means repeated state checks as transactions are reinserted in the pool.

Instead we check the conditional at the rpc layer preemptively to the pool and at the block building process. The transaction has a mutable rejected field, set by the that is used to drive eviction in the mempool (also being used in the interop feature)

still wrapping my head around the different modules integrate with each other in this code. If a similar mutable approach is possible with OpTransactionSigned or if there is a nicer idiomatic way for the payload builder to better surface a list of non-included tx that should be evicted the mempool

Copy link
Collaborator

Choose a reason for hiding this comment

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

let's start with my suggestion and I'll leave more instructions once we have that placeholder

Copy link

Choose a reason for hiding this comment

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

oh nice I didn't see #13806, I was making the same exact change to OpPooledTransaction to manage some external state. Let me pull that in.

I'll create a separate PR (hopefully tomorrow) so we can discuss from there separately from there. Can you briefly elaborate on why we'd need L2TransactionConditional<P> { pool: P }? I'm not fully following a change to the api. In the draft I have I simply have an Option<TransactionConditional> field on OpTransactionSigned so afaik pool api should have to be wrapped at all since the pool transactions are already typed.

Unless you were thinking OpSignedTransaction is untouched and the pool has a separate submission api for these txs.

I think a draft a small diff in the optimism crate will help clarify. Will get this out super soon!

Copy link

Choose a reason for hiding this comment

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

hey @mattsse, I opened this brief draft PR #13926 with a few comments/questions.

Would appreciate your thoughts and I can dig into an implementation from there

/// Handler for: `eth_sendRawTransactionConditional`
async fn send_raw_transaction_conditional(
&self,
bytes: Bytes,
_condition: ConditionalOptions,
) -> RpcResult<B256> {
trace!(target: "rpc::eth", ?bytes, "Serving eth_sendRawTransactionConditional");
Ok(EthTransactions::send_raw_transaction(self, bytes).await?)
}
}