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

chore(sdk): define OpTransactionSigned #11433

Merged
merged 53 commits into from
Dec 3, 2024
Merged

chore(sdk): define OpTransactionSigned #11433

merged 53 commits into from
Dec 3, 2024

Conversation

emhane
Copy link
Member

@emhane emhane commented Oct 2, 2024

Closes #11575

  • Defines new type OpTransactionSigned, that wraps reth_optimism_primitives::OpTransaction
  • Extracts optimism feature gated logic from TransactionSigned impl, into impl SignedTransaction for OpTransactionSigned

@emhane emhane added C-debt A clean up/refactor of existing code A-op-reth Related to Optimism and op-reth A-sdk Related to reth's use as a library labels Oct 2, 2024
@emhane emhane changed the base branch from main to emhane/signed-tx-trait October 2, 2024 14:14
@mattsse
Copy link
Collaborator

mattsse commented Oct 5, 2024

if you stack prs, please reference them, otherwise this makes it harder to navigate

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.

before we do anything her let's please try to make progress on some abstractions first (#11432)

and validate this against our current primitives types before we introduce new ones.
this is more efficient than working on separate network specific types already.

@emhane
Copy link
Member Author

emhane commented Oct 5, 2024

before we do anything her let's please try to make progress on some abstractions first (#11432)

and validate this against our current primitives types before we introduce new ones. this is more efficient than working on separate network specific types already.

gottcha. this is code is checked out from #11348 (referenced in commit message). it moves the logic in TransactionSigned impl, and removes the optimism feature gate from it.

@emhane emhane marked this pull request as draft October 5, 2024 18:06
@emhane emhane force-pushed the emhane/signed-tx-trait branch from cf72724 to 00f899f Compare October 5, 2024 18:33
Base automatically changed from emhane/signed-tx-trait to main October 18, 2024 09:57
@emhane emhane changed the base branch from main to emhane/sig-trait October 18, 2024 10:07
@emhane emhane changed the base branch from emhane/sig-trait to emhane/sig-op October 18, 2024 10:08
@emhane emhane changed the base branch from emhane/sig-op to main October 30, 2024 09:25
@emhane emhane marked this pull request as ready for review November 4, 2024 14:06
@emhane emhane force-pushed the emhane/op-signed-tx branch from 2d15914 to b4121c2 Compare November 4, 2024 14:49
@emhane
Copy link
Member Author

emhane commented Nov 5, 2024

any idea about the failing test @Rjected @joshieDo @mattsse ?

I'm suspecting it has to do with the OpTypedTransaction not having tx type eip4844, and how reth_codecs::add_arbitrary_tests is implemented.

@emhane emhane added this pull request to the merge queue Nov 29, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 29, 2024
@emhane emhane added this pull request to the merge queue Nov 29, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 29, 2024
@emhane emhane enabled auto-merge November 29, 2024 16:11
@emhane emhane force-pushed the emhane/op-signed-tx branch from c81bca9 to 8aa1aef Compare December 3, 2024 05:50
@emhane emhane added this pull request to the merge queue Dec 3, 2024
Merged via the queue into main with commit ea82cbd Dec 3, 2024
42 checks passed
@emhane emhane deleted the emhane/op-signed-tx branch December 3, 2024 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-op-reth Related to Optimism and op-reth A-sdk Related to reth's use as a library C-debt A clean up/refactor of existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Define OpTransactionSigned
2 participants