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

Move more optimism receipt checks to optimism crate #10713

Closed
Tracked by #11109 ...
mattsse opened this issue Sep 5, 2024 · 5 comments · Fixed by #11230
Closed
Tracked by #11109 ...

Move more optimism receipt checks to optimism crate #10713

mattsse opened this issue Sep 5, 2024 · 5 comments · Fixed by #11230
Assignees
Labels
A-op-reth Related to Optimism and op-reth C-debt Refactor of code section that is hard to understand or maintain D-complex Quite challenging from either a design or technical perspective Ask for help!

Comments

@mattsse
Copy link
Collaborator

mattsse commented Sep 5, 2024

Describe the feature

with #10520 we moved some optimism receipt root functions to optimism consensus crate

we still have some in primitives that should be moved:

#[cfg(feature = "optimism")]
pub fn calculate_receipt_root_no_memo_optimism(

this has some side-effects on other code, mainly:

/// Retrieves the receipt root for all recorded receipts from index.
#[cfg(feature = "optimism")]
pub fn optimism_root_slow(

#[cfg(feature = "optimism")]
pub fn optimism_receipts_root_slow(

those should be moved as well, and perhaps converted to a trait

Additional context

No response

@mattsse mattsse added C-enhancement New feature or request S-needs-triage This issue needs to be labelled 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 Sep 5, 2024
@garwahl
Copy link
Contributor

garwahl commented Sep 5, 2024

I can take this @mattsse

@garwahl
Copy link
Contributor

garwahl commented Sep 11, 2024

Hey @mattsse where should Receipts::optimism_root_slow() be moved to? Do you have a preference?

@garwahl
Copy link
Contributor

garwahl commented Sep 16, 2024

ping @mattsse on the above WDYT

@emhane emhane added C-debt Refactor of code section that is hard to understand or maintain D-complex Quite challenging from either a design or technical perspective Ask for help! A-op-reth Related to Optimism and op-reth and removed C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started labels Sep 18, 2024
@garwahl
Copy link
Contributor

garwahl commented Sep 19, 2024

Appears blocked by #7649 according to #10932 (comment) @emhane

@klkvr
Copy link
Collaborator

klkvr commented Sep 26, 2024

didn't notice deeper discussion here before opening #11230, happy to keep this open, PR pretty much just moves things around

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 C-debt Refactor of code section that is hard to understand or maintain D-complex Quite challenging from either a design or technical perspective Ask for help!
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants