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.

Sign up for GitHub

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

Jump to bottom

[Feature] Impl RlpEcdsaTx for OpTypedTransaction #316

Closed
emhane opened this issue Nov 29, 2024 · 6 comments · Fixed by #466
Closed

[Feature] Impl RlpEcdsaTx for OpTypedTransaction #316

emhane opened this issue Nov 29, 2024 · 6 comments · Fixed by #466
Assignees
@htiennv
Labels
A-consensus Area: consensus crate good-first-issue Issues that are easy to pick up for new contributors

Comments

@emhane
Copy link
Collaborator

emhane commented Nov 29, 2024

Component

consensus

Describe the feature you would like

Impl alloy_consensus::RlpEcdsaTx for OpTypedTransaction
https://github.com/alloy-rs/alloy/blob/a33937d9f4a1c67d5ea06e8eb9ad443ffde900f7/crates/consensus/src/transaction/rlp.rs#L7-L217

Additional context

No response
@emhane emhane added A-consensus Area: consensus crate good-first-issue Issues that are easy to pick up for new contributors labels Nov 29, 2024
@emhane
Copy link
Collaborator Author

emhane commented Nov 29, 2024

another one I think is easy for you if you like @htiennv

@htiennv
Copy link
Contributor

htiennv commented Nov 29, 2024

thanks @emhane, i love it

@htiennv
Copy link
Contributor

htiennv commented Nov 29, 2024

Hi @emhane, i see alloy_consensus::RlpEcdsaTx is not public

https://github.com/alloy-rs/alloy/blob/20d13faff78325dda42de8952e0addad31d11db7/crates/consensus/src/lib.rs#L31

@mantosh-mariner
Copy link

Hi, I would like to work on this issue. Could you assign it to me?

@htiennv htiennv mentioned this issue Nov 30, 2024
Closed
@refcell refcell moved this to Todo in Project Tracking Jan 7, 2025
@refcell refcell moved this from Backlog to Out of Scope in Project Tracking Feb 11, 2025
@varun-doshi
Copy link
Contributor

Is this still open? @emhane

@emhane
Copy link
Collaborator Author

emhane commented Feb 11, 2025

a bit divided opinion here since deposit tx isn't signed. so imo, perhaps we need a new op tx enum OpSignableTypedTransaction with same variants as OpTypedTransaction except Deposit, and new method OpTypedTransaction::as_signable which returns Option<&OpSignableTypedTransaction> for ease of use. then we can impl RlpEcdsaTx on OpSignableTransaction cc @mattsse @refcell

@yash-atreya yash-atreya mentioned this issue Mar 7, 2025
Merged
3 tasks
github-merge-queue bot pushed a commit that referenced this issue Mar 7, 2025
@yash-atreya
chore(deps): alloy 0.12 (#466)
6535307 
<!--
Thank you for your Pull Request. Please provide a description above and
review
the requirements below.

Bug fixes and new features should include tests.

Contributors guide:
https://github.com/alloy-rs/core/blob/main/CONTRIBUTING.md

The contributors guide includes instructions for running rustfmt and
building the
documentation.
-->

<!-- ** Please select "Allow edits from maintainers" in the PR Options
** -->

## Motivation

Bump alloy to `0.12`
Closes #316 

<!--
Explain the context and why you're making that change. What is the
problem
you're trying to solve? In some cases there is not a problem and this
can be
thought of as being the motivation for your change.
-->

## Solution

- Bump alloy deps (using patch until 0.12 release)
- Bump alloy-core
- Impl RlpEcdsaEncodableTx for OpTypedTransaction

<!--
Summarize the solution and provide any necessary context needed to
understand
the code change.
-->

## PR Checklist

- [ ] Added Tests
- [ ] Added Documentation
- [ ] Breaking changes
@github-project-automation github-project-automation bot moved this from Out of Scope to Done in Project Tracking Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@htiennv htiennv

Labels
A-consensus Area: consensus crate good-first-issue Issues that are easy to pick up for new contributors
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore(deps): alloy 0.12 alloy-rs/op-alloy
4 participants
@emhane @varun-doshi @htiennv @mantosh-mariner