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: reth's block body fns #1775

Merged
merged 7 commits into from
Dec 11, 2024
Merged

Conversation

stevencartavia
Copy link
Contributor

Motivation

paradigmxyz/reth#12519

Solution

Brought BlockBody fns from reth to replace reth's BlockBody type with alloy's

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@stevencartavia
Copy link
Contributor Author

I just brought these fns at the moment since the rest explicitly use T: TransactionSigned

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.

cool, makes sense

one suggestion

Comment on lines 184 to 194
/// Returns true if the transaction is an EIP-4844 transaction.
#[inline]
fn is_eip4844(&self) -> bool {
self.ty() == EIP4844_TX_TYPE_ID
}

/// Returns true if the transaction is an EIP-7702 transaction.
#[inline]
fn is_eip7702(&self) -> bool {
self.ty() == EIP7702_TX_TYPE_ID
}
Copy link
Member

Choose a reason for hiding this comment

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

these are no longer necessary with #1773

@stevencartavia stevencartavia marked this pull request as ready for review December 10, 2024 16:28
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.

nits

Comment on lines 96 to 100
/// Helper for blob_transactions_iter function
#[inline]
fn is_eip4844(tx: &T) -> bool {
tx.is_eip4844()
}
Copy link
Member

Choose a reason for hiding this comment

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

this function is not needed and can be removed

Comment on lines 108 to 112
/// Returns only the blob transactions, if any, from the block body.
#[inline]
pub fn blob_transactions(&self) -> Vec<&T> {
self.blob_transactions_iter().collect()
}
Copy link
Member

Choose a reason for hiding this comment

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

this should be an impl Iterator instead because this is mostly useful when iterating over blob txs

@mattsse mattsse enabled auto-merge (squash) December 11, 2024 17:27
@mattsse mattsse merged commit da40969 into alloy-rs:main Dec 11, 2024
26 checks passed
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