Skip to content

feat: refactor FullNodeTypes #15537

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

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat: refactor FullNodeTypes #15537

wants to merge 1 commit into from

Conversation

fgimenez
Copy link
Member

@fgimenez fgimenez commented Apr 4, 2025

Closes: #15507

@fgimenez fgimenez added C-enhancement New feature or request A-sdk Related to reth's use as a library labels Apr 4, 2025
@fgimenez fgimenez self-assigned this Apr 4, 2025
mattsse
mattsse reviewed Apr 4, 2025
View reviewed changes
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.

I like this a lot already

crates/node/api/src/node.rs
/// Node's types with the database.
type Types: NodeTypes;
/// Underlying database type used by the node to store and retrieve data.
type DB: Database + DatabaseMetrics + Clone + Unpin + 'static;
/// The provider type used to interact with the node.
type Provider: FullProvider<NodeTypesWithDBAdapter<Self::Types, Self::DB>>;
type Provider: FullProvider<Self>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

so much better

crates/node/types/src/lib.rs Outdated

/// An adapter type combining [`NodeTypes`] and db into [`NodeTypesWithDB`].
#[derive(Clone, Debug, Default)]
pub struct NodeTypesWithDBAdapter<Types, DB> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

rip bozo

@fgimenez fgimenez force-pushed the fgimenez/refactor-node-types-with-db branch from f6853b7 to 9bad524 Compare April 5, 2025 13:54
@fgimenez fgimenez moved this from Todo to In Progress in Reth Tracker Apr 8, 2025
@fgimenez fgimenez changed the title feat: refactor NodeTypesWithDB feat: refactor FullNodeTypes Apr 8, 2025
@fgimenez fgimenez force-pushed the fgimenez/refactor-node-types-with-db branch from 7398197 to 9c785bd Compare April 22, 2025 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattsse mattsse mattsse left review comments

@Rjected Rjected Awaiting requested review from Rjected Rjected will be requested when the pull request is marked ready for review Rjected is a code owner

@onbjerg onbjerg Awaiting requested review from onbjerg onbjerg will be requested when the pull request is marked ready for review onbjerg is a code owner

@klkvr klkvr Awaiting requested review from klkvr klkvr will be requested when the pull request is marked ready for review klkvr is a code owner

@shekhirin shekhirin Awaiting requested review from shekhirin shekhirin will be requested when the pull request is marked ready for review shekhirin is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
A-sdk Related to reth's use as a library C-enhancement New feature or request
Projects
Reth Tracker
Status: In Progress
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Explore removing NodeTypesWithDBAdapter in FullNodeTypes
2 participants
@fgimenez @mattsse