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.

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

added debug in FullProvider #14955

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

added debug in FullProvider #14955

wants to merge 10 commits into from

Conversation

Soubhik-10
Copy link
Contributor

@Soubhik-10 Soubhik-10 commented Mar 11, 2025 •
edited by emhane
Loading

Closes #14953

@Soubhik-10 Soubhik-10 marked this pull request as draft March 11, 2025 13:18
mattsse
mattsse reviewed Mar 11, 2025
View reviewed changes
crates/node/builder/src/builder/mod.rs
@@ -239,7 +239,7 @@ where
/// Configures the types of the node.
pub fn with_types<T>(self) -> NodeBuilderWithTypes<RethFullAdapter<DB, T>>
where
T: NodeTypesWithEngine<ChainSpec = ChainSpec> + NodeTypesForProvider,
T: NodeTypesWithEngine<ChainSpec = ChainSpec> + NodeTypesForProvider + Debug,
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 also add thi to NodeTypesforProvider directly I think

wdyt @klkvr

Copy link
Member

@klkvr klkvr Mar 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

yeah I think it should be either on NodeTypesForProvider or just be a supertrait of NodeTypes

@Soubhik-10
Copy link
Contributor Author

Soubhik-10 commented Mar 12, 2025 •
edited
Loading

@mattsse @klkvr I have added the debug but the e2e tests are failing..should i modify those as well??

@Soubhik-10 Soubhik-10 marked this pull request as ready for review March 12, 2025 07:47
@Soubhik-10 Soubhik-10 requested review from mattsse and klkvr March 12, 2025 07:48
emhane
emhane requested changes Mar 12, 2025
View reviewed changes
Copy link
Member

@emhane emhane left a comment

Choose a reason for hiding this comment

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

makes more sense to add Debug as super trait of FullNodeTypes instead of explicitly adding the trait bound everywhere

@emhane emhane added the A-sdk Related to reth's use as a library label Mar 12, 2025
@Soubhik-10 Soubhik-10 marked this pull request as draft March 12, 2025 10:41
@Soubhik-10 Soubhik-10 mentioned this pull request Mar 12, 2025
Open
@Soubhik-10
Copy link
Contributor Author

makes more sense to add Debug as super trait of FullNodeTypes instead of explicitly adding the trait bound everywhere
@emhane can you check this? #14981 ..was facing problems ere so created a new pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emhane emhane emhane requested changes

@rakita rakita Awaiting requested review from rakita rakita is a code owner

@joshieDo joshieDo Awaiting requested review from joshieDo joshieDo is a code owner

@shekhirin shekhirin Awaiting requested review from shekhirin shekhirin is a code owner

@Rjected Rjected Awaiting requested review from Rjected Rjected is a code owner

@onbjerg onbjerg Awaiting requested review from onbjerg onbjerg is a code owner

@mattsse mattsse Awaiting requested review from mattsse mattsse is a code owner

@klkvr klkvr Awaiting requested review from klkvr klkvr is a code owner

Requested changes must be addressed to merge this pull request.

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

Successfully merging this pull request may close these issues.

Make FullProvider require Debug
4 participants
@Soubhik-10 @mattsse @emhane @klkvr