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: add storage_proof_only argument to RollupArgs #13009

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

Conversation

joshieDo
Copy link
Collaborator

@joshieDo joshieDo commented Nov 29, 2024

    /// List of addresses that _ONLY_ return storage proofs _WITHOUT_ an account proof when called
    /// with `eth_getProof`.
    #[arg(long = "rpc.storage-proof-addresses", value_delimiter = ',', num_args(1..))]
    pub storage_proof_only: Vec<Address>,

Basically moved from odyssey, but without the need to replace the http/auth modules

Also, adds OpAddonsBuilder and OpEthApiBuilder

@joshieDo joshieDo added A-rpc Related to the RPC implementation A-op-reth Related to Optimism and op-reth labels Nov 29, 2024
@joshieDo joshieDo changed the title feat: add storage_proofs_only to RollupArgs feat: add storage_proof_only argument to RollupArgs Nov 29, 2024
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

some suggestions, directionally lgtm

Comment on lines 93 to 97
pub fn new(
ctx: &EthApiBuilderCtx<N>,
sequencer_http: Option<String>,
storage_proof_only: Vec<Address>,
) -> Self {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a feeling we might need more settings in the future so we should change the way we configure this with some builder types for OpEthApi and OpAddons

like

let api =  OpEthApi::builder(ctx).with_sequencer().proof_only().build()

same for OpAddons

@@ -67,6 +74,9 @@ pub struct OpEthApi<N: RpcNodeCore> {
/// Sequencer client, configured to forward submitted transactions to sequencer of given OP
/// network.
sequencer_client: Option<SequencerClient>,
/// List of addresses that _ONLY_ return storage proofs _WITHOUT_ an account proof when called
/// with `eth_getProof`.
storage_proof_only: Vec<Address>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should wrap all of those fields in a new Arced OpEthApiInner type because this is cloned per request

@joshieDo joshieDo marked this pull request as ready for review November 29, 2024 15:43
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
None yet
Development

Successfully merging this pull request may close these issues.

2 participants