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

[Feature] Implement Transaction for either #2051

Open
mattsse opened this issue Feb 13, 2025 · 5 comments
Open

[Feature] Implement Transaction for either #2051

mattsse opened this issue Feb 13, 2025 · 5 comments
Assignees
Labels
c-consensus Pertaining to the consensus crate enhancement New feature or request good first issue Good for newcomers

Comments

@mattsse
Copy link
Member

mattsse commented Feb 13, 2025

Component

consensus, eips, genesis

Describe the feature you would like

there are usecase where we need to handle two different tx types:

https://github.com/paradigmxyz/reth/pull/14472/files#diff-068a87546e97f457bc31013fabf9a116441c1050fb2f84cd6c3c49eaae5cce13R63-R68

we can solve this by implementing it for Either from the either crate:

either = "1.13.0"

TODO

  • impl Transaction for Either
  • lift either dep to workspace dep

Additional context

No response

@mattsse mattsse added enhancement New feature or request good first issue Good for newcomers labels Feb 13, 2025
@neithanmo
Copy link
Contributor

I can implement this @mattsse

@mattsse
Copy link
Member Author

mattsse commented Feb 13, 2025

@neithanmo it could be that it's more useful to roll our own Either type for this

so we can impl more logic natively

@neithanmo
Copy link
Contributor

neithanmo commented Feb 16, 2025

@mattsse
Where would this type live? maybe alloy-primitives?

There is an either module, could it be the right place for this new type?

@mattsse
Copy link
Member Author

mattsse commented Feb 16, 2025

i think we should put this in consensus/src/transaction/either.rs

There is an either module

hmm, this should really be called any

@yash-atreya yash-atreya added the c-consensus Pertaining to the consensus crate label Feb 20, 2025
@neithanmo
Copy link
Contributor

Ok, I will complete it, this weekend

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-consensus Pertaining to the consensus crate enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants