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

feat: embed consensus header into RPC #1573

Merged
merged 3 commits into from
Oct 28, 2024
Merged

feat: embed consensus header into RPC #1573

merged 3 commits into from
Oct 28, 2024

Conversation

klkvr
Copy link
Member

@klkvr klkvr commented Oct 26, 2024

Changes in this PR:

  • size is moved from block body to header. we don't have a way to distinct which fields in RPC belong to block body vs header but moving size to header leaves RPC block fields as header + flattened body which makes sense imo
  • rpc_types_eth::Header now wraps an inner consensus header and additionaly stores hash, totalDifficulty and size which are RPC-specific
  • the previous header type without RPC-specific fields is now AnyHeader in alloy-consensus crate which is the same as alloy_consensus::Header with mixHash and nonce being optional.
  • HeaderResponse trait is removed and Network::HeaderResponse is now required to implement AsRef<Network::Header> making it possible to simply use methods from consensus BlockHeader trait everywhere

Copy link
Member

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

this will unlock a ton of simplifications

Comment on lines +5 to +11
/// Block header representation with certain fields made optional to account for possible
/// differencies in network implementations.
#[cfg_attr(any(test, feature = "arbitrary"), derive(arbitrary::Arbitrary))]
#[derive(Clone, Debug, Default, PartialEq, Eq, Hash)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "serde", serde(rename_all = "camelCase"))]
pub struct AnyHeader {
Copy link
Member

Choose a reason for hiding this comment

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

ah yeah we def need this for foundry


type TransactionRequest = WithOtherFields<TransactionRequest>;

type TransactionResponse = WithOtherFields<Transaction>;

type ReceiptResponse = AnyTransactionReceipt;

type HeaderResponse = Header;
type HeaderResponse = alloy_rpc_types_eth::Header<Self::Header>;
Copy link
Member

Choose a reason for hiding this comment

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

nice

@klkvr klkvr merged commit 2fbadbf into main Oct 28, 2024
26 checks passed
@klkvr klkvr deleted the klkvr/header-conversions branch October 28, 2024 14:01
@klkvr klkvr self-assigned this Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants