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(protocol): payload module #89

Closed

Conversation

DoTheBestToGetTheBest
Copy link
Contributor

Since we dont use anyhow anymore we made new type for error handling

Comment on lines +69 to +187
/// OP Stack full block.
///
/// Withdrawals can be optionally included at the end of the RLP encoded message.
///
/// Taken from [reth-primitives](https://github.com/paradigmxyz/reth)
#[derive(Debug, Clone, PartialEq, Eq, Default, RlpEncodable, RlpDecodable)]
#[rlp(trailing)]
pub struct OpBlock {
/// Block header.
pub header: Header,
/// Transactions in this block.
pub body: Vec<OpTxEnvelope>,
/// Ommers/uncles header.
pub ommers: Vec<Header>,
/// Block withdrawals.
pub withdrawals: Option<Vec<Withdrawal>>,
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We want to use https://github.com/alloy-rs/alloy/blob/main/crates/rpc-types-eth/src/block.rs#L20 instead of duplicating the block definition here. We can then alias Block<OpTxEnvelope> as OpBlock. The issue right now is alloy-rpc-types-eth doesn't support no_std and requires serde by default. I have a pr open for this here: alloy-rs/alloy#1252

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We want to use https://github.com/alloy-rs/alloy/blob/main/crates/rpc-types-eth/src/block.rs#L20 instead of duplicating the block definition here. We can then alias Block<OpTxEnvelope> as OpBlock. The issue right now is alloy-rpc-types-eth doesn't support no_std and requires serde by default. I have a pr open for this here: alloy-rs/alloy#1252

thank you for your fast reply, i'll wait your pr get merged to use Block type then!

/// The execution payload.
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[derive(Debug, Default, Clone, PartialEq, Eq)]
pub struct L2ExecutionPayload {
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 be able to avoid duplicating this type and use an alloy execution payload type as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

I also have a pr open in upstream alloy to support no_std in alloy-rpc-types-engine: alloy-rs/alloy#1233

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for sharing this,

Should we wait for your changes to get merged upstream before we integrate this into the crate?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm preferable to integrating this the right way with alloy types. It will prevent a lot of duplicate type confusion over the long run. Let's keep this open for now if you don't mind.

@refcell
Copy link
Collaborator

refcell commented Sep 30, 2024

@refcell refcell closed this Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants