-
Notifications
You must be signed in to change notification settings - Fork 86
feat(l1): add From for Transaction -> GenericTransaction #3227
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(l1): add From for Transaction -> GenericTransaction #3227
Conversation
Signed-off-by: Sacha Froment <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces conversion implementations to create a GenericTransaction
from various Transaction
types and adds initial unit tests for two of those conversions.
- Implemented
From<LegacyTransaction>
andFrom<EIP2930Transaction>
forGenericTransaction
. - Added a blanket
From<Transaction>
implementation covering all enum variants. - Added unit tests for
LegacyTransaction
andEIP2930Transaction
conversions.
Comments suppressed due to low confidence (1)
crates/common/types/transaction.rs:2971
- Only
LegacyTransaction
andEIP2930Transaction
conversions are tested. Add tests forEIP1559Transaction
,EIP4844Transaction
,EIP7702Transaction
, andPrivilegedL2Transaction
conversions to ensure full coverage.
fn test_legacy_transaction_into_generic() {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…#3227) **Motivation** Adding an easy way to get a GenericTransaction from any Transaction <!-- Why does this pull request exist? What are its goals? --> **Description** Adding the 2 missing From and one for the enum This will allow people who use the ethClient to make estimate_gas and eth_call request, more easily and maybe other request in the future might benefit from it <!-- A clear and concise general description of the changes this PR introduces --> <!-- Link to issues: Resolves lambdaclass#111, Resolves lambdaclass#222 --> BTW I don't know which scope I shall use Signed-off-by: Sacha Froment <[email protected]> Co-authored-by: Martin Paulucci <[email protected]>
Motivation
Adding an easy way to get a GenericTransaction from any Transaction
Description
Adding the 2 missing From and one for the enum
This will allow people who use the ethClient to make estimate_gas and eth_call request, more easily and maybe other request in the future might benefit from it
BTW I don't know which scope I shall use