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

Move RethModuleRegistry functionality into RpcRegistry #8985

Closed
Tracked by #8988
emhane opened this issue Jun 20, 2024 · 2 comments
Closed
Tracked by #8988

Move RethModuleRegistry functionality into RpcRegistry #8985

emhane opened this issue Jun 20, 2024 · 2 comments
Labels
A-rpc Related to the RPC implementation C-debt Refactor of code section that is hard to understand or maintain S-blocked This cannot more forward until something else changes S-needs-design This issue requires design work to think about how it would best be accomplished S-wontfix This issue is the result of a deliberate design decision, and will not be fixed

Comments

@emhane
Copy link
Member

emhane commented Jun 20, 2024

Describe the feature

Move RethModuleRegistry functionality into RpcRegistry, which is rn a wrapper around RethModuleRegistry.

/// Helper wrapper type to encapsulate the [`RethModuleRegistry`] over components trait.
#[derive(Debug)]
pub struct RpcRegistry<Node: FullNodeComponents> {
pub(crate) registry: RethModuleRegistry<
Node::Provider,
Node::Pool,
NetworkHandle,
TaskExecutor,
Node::Provider,
Node::Evm,
>,
}

Additional context

No response

@emhane emhane added C-debt Refactor of code section that is hard to understand or maintain A-rpc Related to the RPC implementation labels Jun 20, 2024
@emhane emhane added the S-blocked This cannot more forward until something else changes label Jun 24, 2024
@tcoratger
Copy link
Contributor

@emhane Here a few questions:

  • Do you plan to keep the RethModuleRegistry structure inside the rpc builder and just migrate all the features or do you also plan to completely remove RethModuleRegistry?
  • If we want to delete RethModuleRegistry then we must also migrate the RpcModuleBuilder to the node builder and as on the node builder we have an implementation constraint which is Node: FullNodeComponents, then we will no longer be able to keep RpcModuleBuilder as generic as it is now. I am thinking in particular of the default() function which could no longer work as it currently does.

@emhane emhane added the S-needs-design This issue requires design work to think about how it would best be accomplished label Jun 24, 2024
@emhane
Copy link
Member Author

emhane commented Jun 24, 2024

@emhane Here a few questions:

  • Do you plan to keep the RethModuleRegistry structure inside the rpc builder and just migrate all the features or do you also plan to completely remove RethModuleRegistry?
  • If we want to delete RethModuleRegistry then we must also migrate the RpcModuleBuilder to the node builder and as on the node builder we have an implementation constraint which is Node: FullNodeComponents, then we will no longer be able to keep RpcModuleBuilder as generic as it is now. I am thinking in particular of the default() function which could no longer work as it currently does.

goal is to follow payload or pool builder pattern, but this issue is blocked rn, would be nice if you can pick up any other 🙏

@emhane emhane added the S-wontfix This issue is the result of a deliberate design decision, and will not be fixed label Jul 13, 2024
@emhane emhane closed this as completed Jul 13, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in Reth Tracker Jul 13, 2024
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-debt Refactor of code section that is hard to understand or maintain S-blocked This cannot more forward until something else changes S-needs-design This issue requires design work to think about how it would best be accomplished S-wontfix This issue is the result of a deliberate design decision, and will not be fixed
Projects
Archived in project
Development

No branches or pull requests

2 participants