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

chore(sdk): Add NodePrimitives::BlockHeader and NodePrimitives::BlockBody #12647

Merged
merged 39 commits into from
Nov 21, 2024
Merged

chore(sdk): Add NodePrimitives::BlockHeader and NodePrimitives::BlockBody #12647

merged 39 commits into from
Nov 21, 2024

Conversation

emhane
Copy link
Member

@emhane emhane commented Nov 19, 2024 •
edited
Loading

Based on #12683

Ref #12358

@emhane emhane added C-debt Refactor of code section that is hard to understand or maintain A-sdk Related to reth's use as a library labels Nov 19, 2024
@emhane emhane changed the base branch from main to emhane/chain-generic-prims November 19, 2024 00:34
@emhane emhane mentioned this pull request Nov 19, 2024
Merged
mattsse
mattsse requested changes Nov 19, 2024
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.

hmm, why do we need this if the header and body type are already defined in the block type?

Base automatically changed from emhane/chain-generic-prims to main November 19, 2024 09:42
@emhane
Copy link
Member Author

emhane commented Nov 19, 2024 •
edited
Loading

hmm, why do we need this if the header and body type are already defined in the block type?

it makes it a lot easier to access the header type...one could argue the same about signed transaction too otherwise

@emhane emhane requested a review from mattsse November 19, 2024 12:19
@mattsse
Copy link
Collaborator

mattsse commented Nov 19, 2024

I suspect this would make things harder when enforcing bounds because then we need to restrict Header to Block::Header.

don't think we should do this

auto-merge was automatically disabled November 19, 2024 19:08

Merge commits are not allowed on this repository

@emhane emhane force-pushed the emhane/node-prims-block-parts branch from 25b4177 to abd6bdc Compare November 19, 2024 19:41
@emhane
Copy link
Member Author

emhane commented Nov 19, 2024

can't figure out what's wrong with the features here https://github.com/paradigmxyz/reth/actions/runs/11921032967/job/33224297145?pr=12647

@emhane emhane requested a review from klkvr as a code owner November 20, 2024 15:14
mattsse
mattsse approved these changes Nov 20, 2024
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.

seems okay, I guess this would make accessing the type more convenient.

pending ci

@emhane emhane force-pushed the emhane/node-prims-block-parts branch from ae26057 to e813460 Compare November 20, 2024 21:48
This was referenced Nov 20, 2024
Closed
Merged
Base automatically changed from emhane/maybe-compact to main November 21, 2024 16:26
@emhane emhane enabled auto-merge November 21, 2024 16:48
@emhane emhane added this pull request to the merge queue Nov 21, 2024
Merged via the queue into main with commit 2093d2b Nov 21, 2024
42 checks passed
@emhane emhane deleted the emhane/node-prims-block-parts branch November 21, 2024 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattsse mattsse mattsse approved these changes

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

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

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

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

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

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

@rkrasiuk rkrasiuk Awaiting requested review from rkrasiuk

@shekhirin shekhirin Awaiting requested review from shekhirin

@rakita rakita Awaiting requested review from rakita

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

Assignees
No one assigned
Labels
A-sdk Related to reth's use as a library C-debt Refactor of code section that is hard to understand or maintain
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@emhane @mattsse