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

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

Open
wants to merge 27 commits into
base: emhane/maybe-compact
Choose a base branch
from

Conversation

emhane
Copy link
Member

@emhane emhane commented Nov 19, 2024

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
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

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

@emhane
Copy link
Member Author

emhane 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

we have the same problem with signed tx, must be restricted to Self::Block: Block<Body: Body<Transaction: Self::SignedTx>>

@mattsse

@emhane
Copy link
Member Author

emhane commented Nov 19, 2024

I don't see why this is a problem when NetworkPrimitives does this exactly...

@emhane emhane changed the base branch from main to emhane/maybe-compact November 19, 2024 19:05
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

2 participants