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

Remove multi provider trait functions in rpc helper traits #11422

Open
8 of 10 tasks
mattsse opened this issue Oct 2, 2024 · 3 comments
Open
8 of 10 tasks

Remove multi provider trait functions in rpc helper traits #11422

mattsse opened this issue Oct 2, 2024 · 3 comments
Labels
A-rpc Related to the RPC implementation C-enhancement New feature or request S-needs-design This issue requires design work to think about how it would best be accomplished

Comments

@mattsse
Copy link
Collaborator

mattsse commented Oct 2, 2024

Describe the feature

we compose various rpc traits with default impls

like:

pub trait Trace: LoadState {

these are effective stacked traits.
currently we have some functions multiple times:

fn evm_config(&self) -> &impl ConfigureEvm<Header = Header>;

fn evm_config(&self) -> &impl ConfigureEvm<Header = Header>;

or

fn provider(&self) -> impl BlockReaderIdExt;

fn provider(
&self,
) -> impl BlockReaderIdExt
+ EvmEnvProvider
+ ChainSpecProvider<ChainSpec: EthChainSpec + EthereumHardforks>
+ StateProviderFactory;

resulting in some inconvenient usage and restrictions.

like:

LoadBlock::provider(self).ommers_by_id(block_id).map_err(Self::Error::from_eth_err)

ideally we only have one evm_config and one provider function, perhaps in a single trait that sits at the bottom of the stack, these should like by an associated type and we can easily implement this because we have this generic:

impl<Provider, Pool, Network, EvmConfig> EthTransactions
for EthApi<Provider, Pool, Network, EvmConfig>
where
Self: LoadTransaction,
Pool: TransactionPool + 'static,
Provider: BlockReaderIdExt,
{
#[inline]
fn provider(&self) -> impl BlockReaderIdExt {
self.inner.provider()
}

and it is expected that the final provider is then the full provider that satisfies all traits.

the stacked traits can then add the required bounds

trait EthCore // naming tbd {
  type Provider;
  type Evm;

  fn provider(&self) -> &Self::Provider;
  fn evm_config...
}

then we can force the required trait bounds on Trace for example

trait Trace: EthCore<Provider: ....> {

}

Additional context

No response

Tasks

  1. A-rpc A-sdk C-debt
  2. A-rpc A-sdk C-debt
  3. A-rpc A-sdk C-debt
  4. A-rpc A-sdk C-debt
  5. A-rpc A-sdk C-debt
  6. A-rpc A-sdk C-debt
  7. A-rpc A-sdk C-debt
  8. A-rpc A-sdk C-debt
  9. A-rpc A-sdk C-debt
  10. A-rpc A-sdk C-debt
@mattsse mattsse added C-enhancement New feature or request S-needs-triage This issue needs to be labelled S-needs-design This issue requires design work to think about how it would best be accomplished A-rpc Related to the RPC implementation and removed S-needs-triage This issue needs to be labelled labels Oct 2, 2024
@emhane
Copy link
Member

emhane commented Oct 2, 2024

something like this would be helpful: https://github.com/paradigmxyz/reth/pull/10616/files

then moving all the getters in FullNodeComponents to this trait.

removing this redundancy at the price of making the the trait bounds less precise, isn't helpful. Some rollups won't use all the FullProvider, only certain providers. it's nice sdk if they don't have to do lots of noop impl of provider traits they aren't using on their provider type.

besides it makes it harder to build unit tests for this code, which we should aim to do despite hive test coverage.

@mattsse
Copy link
Collaborator Author

mattsse commented Oct 3, 2024

right, #10616 is more or less the unlock here, or similar at least

Copy link
Contributor

This issue is stale because it has been open for 21 days with no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Related to the RPC implementation C-enhancement New feature or request S-needs-design This issue requires design work to think about how it would best be accomplished
Projects
Status: Todo
Development

No branches or pull requests

2 participants