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

feat: extract optimism genesis info #17

Merged
merged 1 commit into from
Jul 3, 2024
Merged

feat: extract optimism genesis info #17

merged 1 commit into from
Jul 3, 2024

Conversation

Vid201
Copy link
Contributor

@Vid201 Vid201 commented Jun 27, 2024

Motivation

Adds functionalities to extract Optimism genesis data from genesis.

extract_from returns Option since the extraction can be unsuccessful - negative case should be handled outside.

@Vid201
Copy link
Contributor Author

Vid201 commented Jun 27, 2024

@mattsse do we want some tests here as well?

@mattsse
Copy link
Member

mattsse commented Jun 27, 2024

yeah, having the sanity test would be great

mattsse
mattsse requested changes Jun 27, 2024
View reviewed changes
crates/rpc-types/src/genesis.rs Outdated Show resolved Hide resolved
crates/rpc-types/src/genesis.rs Outdated Show resolved Hide resolved
@Vid201
Copy link
Contributor Author

Vid201 commented Jun 27, 2024

Addressed the comments

@Vid201 Vid201 requested a review from mattsse June 28, 2024 14:38
mattsse
mattsse requested changes Jun 29, 2024
View reviewed changes
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great, a few more nits.

the tryinto are more or less just for convenience but should reuse the existing functions so we don't have the code duplicated.

crates/rpc-types/src/genesis.rs Show resolved Hide resolved
crates/rpc-types/src/genesis.rs Outdated Show resolved Hide resolved
crates/rpc-types/src/genesis.rs Show resolved Hide resolved
crates/rpc-types/src/genesis.rs Outdated Show resolved Hide resolved
crates/rpc-types/src/genesis.rs Outdated Show resolved Hide resolved
crates/rpc-types/src/genesis.rs Outdated Show resolved Hide resolved
@Vid201
Copy link
Contributor Author

Vid201 commented Jul 2, 2024

great, a few more nits.

the tryinto are more or less just for convenience but should reuse the existing functions so we don't have the code duplicated.

@mattsse Addressed the comments

@Vid201 Vid201 requested a review from mattsse July 2, 2024 10:11
mattsse
mattsse approved these changes Jul 3, 2024
View reviewed changes
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

@mattsse mattsse merged commit c93be8e into alloy-rs:main Jul 3, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattsse mattsse mattsse approved these changes

@gakonst gakonst Awaiting requested review from gakonst

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Vid201 @mattsse