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

Extend AuthRpcModules in op-reth node building #13832

Open
emhane opened this issue Jan 16, 2025 · 5 comments
Open

Extend AuthRpcModules in op-reth node building #13832

emhane opened this issue Jan 16, 2025 · 5 comments
Assignees
Labels
A-op-reth Related to Optimism and op-reth A-rpc Related to the RPC implementation

Comments

@emhane
Copy link
Member

emhane commented Jan 16, 2025

Describe the feature

Add RpcAddOns to OpAddOnsBuilder

/// A regular optimism evm and executor builder.
#[derive(Debug, Default, Clone)]
#[non_exhaustive]
pub struct OpAddOnsBuilder {
/// Sequencer client, configured to forward submitted transactions to sequencer of given OP
/// network.
sequencer_client: Option<SequencerClient>,
/// Data availability configuration for the OP builder.
da_config: Option<OpDAConfig>,
}
impl OpAddOnsBuilder {
/// With a [`SequencerClient`].
pub fn with_sequencer(mut self, sequencer_client: Option<String>) -> Self {
self.sequencer_client = sequencer_client.map(SequencerClient::new);
self
}
/// Configure the data availability configuration for the OP builder.
pub fn with_da_config(mut self, da_config: OpDAConfig) -> Self {
self.da_config = Some(da_config);
self
}
}

Additional context

No response

@emhane emhane added A-op-reth Related to Optimism and op-reth A-rpc Related to the RPC implementation labels Jan 16, 2025
@htiennv
Copy link
Contributor

htiennv commented Jan 16, 2025

Hi @emhane, Can you assign to me? Thanks

@mattsse
Copy link
Collaborator

mattsse commented Jan 18, 2025

instructions for this issue aren't super clear, what are the todos here?
extend the auth modules with what?

unclear how the referenced code is relevant here,

we can already do this

// install the miner extension in the authenticated if configured
if modules.module_config().contains_any(&RethRpcModule::Miner) {
debug!(target: "reth::cli", "Installing miner DA rpc enddpoint");
auth_modules.merge_auth_methods(miner_ext.into_rpc())?;
}

so I think we can close this issue

@emhane
Copy link
Member Author

emhane commented Jan 18, 2025

thanks for linking the code @mattsse . the engine_superchainSignalV1 method is optional for example, so at least a boolean is needed on OpAddOns to let it be enabled at sdk level. however, it's nicer if we open up to the user to define custom rpc modules they want to add as well, don't you agree?

the action items, is to impl a method on the OpAddOnsBuilder::rpc_addon() which will add RpcAddOns to a list stored in the builder.

we can also have additional method specifically for the super chain engine extension api
https://github.com/op-rs/maili/blob/624e63c2393ed2026bd5220cd8a65ce43ca63b0f/crates/rpc/src/api.rs#L128-L144

@emhane
Copy link
Member Author

emhane commented Jan 18, 2025

maybe a better idea is to have OpAddOnsBuilder::rpc_hook and adding field rpc_hooks: RpcHooks on OpAddOnsBuilder and OpAddOns actually, since that's more specifically the concern here @htiennv

@emhane
Copy link
Member Author

emhane commented Jan 18, 2025

@mattsse it also occurred to me why we shouldn't do this via the TransportModuleConfig as in the code you linked, because we don't want to add an op-reth specific module for superchain, to RethRpcModule type

#[serde(rename_all = "snake_case")]
#[strum(serialize_all = "kebab-case")]
pub enum RethRpcModule {
/// `admin_` module
Admin,
/// `debug_` module
Debug,
/// `eth_` module
Eth,
/// `net_` module
Net,
/// `trace_` module
Trace,
/// `txpool_` module
Txpool,
/// `web3_` module
Web3,
/// `rpc_` module
Rpc,
/// `reth_` module
Reth,
/// `ots_` module
Ots,
/// `flashbots_` module
Flashbots,
/// `miner_` module
Miner,
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-op-reth Related to Optimism and op-reth A-rpc Related to the RPC implementation
Projects
Status: Todo
Development

No branches or pull requests

3 participants