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: use reth-ethereum-primitives #13830

Merged
merged 13 commits into from
Jan 17, 2025
Merged

feat: use reth-ethereum-primitives #13830

merged 13 commits into from
Jan 17, 2025

Conversation

klkvr
Copy link
Member

@klkvr klkvr commented Jan 16, 2025

This PR removes primitive types declared in in reth-primitives in favor of the same types from reth-ethereum-primitives which are just re-exported for now. optimism feature is removed from reth-primitives as well

With this PR, the only optimism-gated logic in primitive types crates is this one directly related to revm

#[cfg(feature = "optimism")]
impl reth_primitives_traits::FillTxEnv for OpTransactionSigned {

Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

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

awesome, I think we're OOMing the runner unfortunately

@@ -356,7 +356,7 @@ mod test {
#[test]
fn eth68_announcement_unrecognized_tx_type() {
let types = vec![
TxType::MAX_RESERVED_EIP as u8 + 1, // the first type isn't valid
TxType::Eip7702 as u8 + 1, // the first type isn't valid
Copy link
Member

Choose a reason for hiding this comment

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

makes sense

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.

hell yeah,
op-reth error unrelated

assert_eq!(tx_type_result, expected)
}
}
pub use alloy_consensus::TxType;
Copy link
Collaborator

Choose a reason for hiding this comment

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

hell yeah

@mattsse
Copy link
Collaborator

mattsse commented Jan 17, 2025

merging this because this pr is a conflict magnet

@mattsse mattsse merged commit 8efe441 into main Jan 17, 2025
42 of 43 checks passed
@mattsse mattsse deleted the klkvr/use-eth-primitives branch January 17, 2025 00:22
@emhane
Copy link
Member

emhane commented Jan 31, 2025

second hell yeah

@clabby
Copy link
Collaborator

clabby commented Jan 31, 2025

third hell yeah, nice work!

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.

5 participants