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(primitives): re-export alloy Recovered #13670

Merged

Conversation

bendanzhentan
Copy link
Contributor

@bendanzhentan bendanzhentan commented Jan 6, 2025

The second part of #13651

  1. 9659c68 removes reth RecoveredTx
  2. 808cb97 re-exports alloy::consensus::Recovered as RecoveredTx, in order to replace the reth RecoveredTx
  3. 4b944ae fix the Rust complication error cannot define inherent impl for foreign type
  4. 77f476a fix clippy warnings

Note that I didn't impl the step "remove extension trait and integrate these functions in SignedTransaction directly", because I didn't see the reason to do that.

@bendanzhentan bendanzhentan force-pushed the feat-reexport-alloy-Recovered branch from 9fffc2c to 77f476a Compare January 6, 2025 15:11
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.

awesome,

this looks great the ci failure

thread 'tokio-runtime-worker' has overflowed its stack

indicates that there is some recursion happening, likely when doing some conversion

@mattsse
Copy link
Collaborator

mattsse commented Jan 6, 2025

Note that I didn't impl the step "remove extension trait and integrate these functions in SignedTransaction directly", because I didn't see the reason to do that.

makes sense, we can do that separately

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.

great! amazing

one step closer

@mattsse mattsse added this pull request to the merge queue Jan 6, 2025
@mattsse mattsse added M-changelog This change should be included in the changelog A-sdk Related to reth's use as a library labels Jan 6, 2025
Merged via the queue into paradigmxyz:main with commit 20e003f Jan 6, 2025
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sdk Related to reth's use as a library M-changelog This change should be included in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants