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

Move optimism_deposit_signature to op-alloy #11781

Closed
Tracked by #12576
mattsse opened this issue Oct 16, 2024 · 2 comments · Fixed by #12016
Closed
Tracked by #12576

Move optimism_deposit_signature to op-alloy #11781

mattsse opened this issue Oct 16, 2024 · 2 comments · Fixed by #12016
Assignees
@caglaryucekaya
Labels
C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started

Comments

@mattsse
Copy link
Collaborator

mattsse commented Oct 16, 2024

Describe the feature

we mostly need this for rpc responses or as a placeholder:

reth/crates/optimism/chainspec/src/lib.rs

Lines 181 to 185 in d4be773

/// Returns the signature for the optimism deposit transactions, which don't include a
/// signature.
pub fn optimism_deposit_tx_signature() -> Signature {
Signature::new(U256::ZERO, U256::ZERO, Parity::Parity(false))
}

this constant should be moved to op-alloy/consensus

https://github.com/alloy-rs/op-alloy

Additional context

No response
@mattsse mattsse added C-enhancement New feature or request S-needs-triage This issue needs to be labelled labels Oct 16, 2024
@mattsse mattsse added D-good-first-issue Nice and easy! A great choice to get started and removed S-needs-triage This issue needs to be labelled labels Oct 16, 2024
@caglaryucekaya
Copy link
Contributor

Hey, I'd like to work on this

@caglaryucekaya caglaryucekaya mentioned this issue Oct 16, 2024
Merged
3 tasks
@caglaryucekaya
Copy link
Contributor

caglaryucekaya commented Oct 16, 2024 •
edited
Loading

I opened a PR on op-alloy as part of this issue

github-merge-queue bot pushed a commit to alloy-rs/op-alloy that referenced this issue Oct 16, 2024
@caglaryucekaya
feat: add signature function to TxDeposit (#174)
9fd8a65 
<!--
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
`optimism_deposit_tx_signature` function in `reth` should be moved to
`op-alloy` as part of an
[issue](paradigmxyz/reth#11781).

paradigmxyz/reth#11781

<!--
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
An associated function `signature` is added to the `TxDeposit` struct.

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

## PR Checklist

- [ ] Added Tests
- [ ] Added Documentation
- [ ] Breaking changes
refcell pushed a commit to alloy-rs/op-alloy that referenced this issue Oct 22, 2024
@caglaryucekaya @refcell
feat: add signature function to TxDeposit (#174)
15acf12 
<!--
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
`optimism_deposit_tx_signature` function in `reth` should be moved to
`op-alloy` as part of an
[issue](paradigmxyz/reth#11781).

paradigmxyz/reth#11781

<!--
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
An associated function `signature` is added to the `TxDeposit` struct.

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

## PR Checklist

- [ ] Added Tests
- [ ] Added Documentation
- [ ] Breaking changes
@caglaryucekaya caglaryucekaya mentioned this issue Oct 23, 2024
Merged
@github-project-automation github-project-automation bot moved this from Todo to Done in Reth Tracker Oct 24, 2024
@emhane emhane mentioned this issue Nov 15, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@caglaryucekaya caglaryucekaya

Labels
C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started
Projects
Reth Tracker
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor: use op-alloy deposit signature caglaryucekaya/reth
2 participants
@caglaryucekaya @mattsse