Skip to content

Conversation

@prestwich
Copy link
Member

@prestwich prestwich commented May 29, 2025

-_-

this PR removes reth dependencies from all crates, and aggregates the test utils into a single crate (with reth deps)
This involves some type duplication (in signet_types::primitives). Those types are partially unfinished, and should be refined in future work

to enable removing the dep,

  • the extractor is now generic over trait Extractable
  • the driver is now generic over trait Extractable
  • reth::primtive types have been swapped for signet_types::primitive types
  • several high-level functionality tests have been moved to integration tests in the signet-test-utils crate

Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@prestwich prestwich marked this pull request as ready for review May 29, 2025 18:30
@prestwich prestwich requested a review from a team as a code owner May 29, 2025 18:30
/// A trait for types that contain transactions.
pub trait HasTxns {
/// Get the transactions in the block.
fn transactions(&self) -> &[TransactionSigned];
Copy link
Member

Choose a reason for hiding this comment

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

maybe add transaction_count and into_transactions too?

Copy link
Member Author

Choose a reason for hiding this comment

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

i don't think it's worth it, as this trait is basically a shim

@prestwich prestwich force-pushed the prestwich/this-was-not-a-good-idea branch from ecef4d5 to 8b02178 Compare May 30, 2025 01:06
@Evalir Evalir self-requested a review June 3, 2025 12:58
Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

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

lgtm. i want to see how this will feel being used in the node

@prestwich prestwich merged commit de39435 into main Jun 3, 2025
7 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.

3 participants