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

chore(rpc): EthApiTypes trait for network specific error AT #9523

Merged
merged 15 commits into from
Jul 25, 2024
Merged

chore(rpc): EthApiTypes trait for network specific error AT #9523

merged 15 commits into from
Jul 25, 2024

Conversation

emhane
Copy link
Member

@emhane emhane commented Jul 15, 2024 •
edited
Loading

Ref #8780

  • Adds new trait EthApiTypes, with AT EthApiTypes::Error, for network specific errors

@emhane emhane changed the base branch from main to emhane/node-addons July 15, 2024 15:46
@emhane emhane marked this pull request as draft July 15, 2024 16:22
Base automatically changed from emhane/node-addons to main July 16, 2024 14:06
@emhane emhane mentioned this pull request Jul 16, 2024
Closed
@emhane emhane requested a review from mattsse July 17, 2024 12:16
@emhane emhane changed the base branch from main to emhane/move-ethapi July 17, 2024 12:18
@emhane emhane changed the base branch from emhane/move-ethapi to main July 17, 2024 12:18
@emhane emhane changed the base branch from main to emhane/move-ethapi July 17, 2024 13:19
@emhane emhane changed the base branch from emhane/move-ethapi to main July 17, 2024 13:19
@emhane emhane closed this Jul 17, 2024
@emhane emhane force-pushed the emhane/rpc-error branch from 53adc7f to 0630621 Compare July 17, 2024 13:21
@emhane emhane reopened this Jul 17, 2024
@emhane emhane added C-debt A clean up/refactor of existing code A-rpc Related to the RPC implementation A-op-reth Related to Optimism and op-reth labels Jul 17, 2024
@emhane emhane marked this pull request as ready for review July 24, 2024 14:08
@emhane emhane requested a review from rkrasiuk as a code owner July 24, 2024 14:08
@emhane
Copy link
Member Author

emhane commented Jul 24, 2024

re-request review @mattsse

@emhane emhane force-pushed the emhane/rpc-error branch from 2641951 to f1d02a7 Compare July 24, 2024 16:25
mattsse
mattsse requested changes Jul 24, 2024
View reviewed changes
Copy link
Collaborator

@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.

I like this a lot, this is the logical next step towards rpc types abstraction.

I have a few requests and suggestions and overall the scope of this pr is appropriate

bin/reth/Cargo.toml Show resolved Hide resolved
crates/optimism/rpc/src/error.rs Outdated Show resolved Hide resolved
crates/optimism/rpc/src/eth/call.rs Outdated Show resolved Hide resolved
crates/rpc/rpc-eth-api/src/core.rs Outdated Show resolved Hide resolved
crates/rpc/rpc-eth-api/src/core.rs Outdated Show resolved Hide resolved
crates/rpc/rpc-eth-types/src/error.rs Outdated Show resolved Hide resolved
crates/rpc/rpc-eth-api/src/core.rs Outdated Show resolved Hide resolved
@emhane emhane requested a review from mattsse July 25, 2024 13:31
mattsse
mattsse requested changes Jul 25, 2024
View reviewed changes
Copy link
Collaborator

@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.

last pedantic naming nits

crates/rpc/rpc-eth-api/src/core.rs Show resolved Hide resolved
crates/rpc/rpc-eth-api/src/helpers/error.rs Outdated Show resolved Hide resolved
crates/rpc/rpc-eth-api/src/helpers/error.rs Outdated Show resolved Hide resolved
emhane and others added 2 commits July 25, 2024 17:59
mattsse
mattsse approved these changes Jul 25, 2024
View reviewed changes
Copy link
Collaborator

@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.

gg, onwards

@emhane emhane enabled auto-merge July 25, 2024 17:19
@emhane emhane added this pull request to the merge queue Jul 25, 2024
Merged via the queue into main with commit 0be2c17 Jul 25, 2024
33 checks passed
@emhane emhane deleted the emhane/rpc-error branch July 25, 2024 17:43
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

@Rjected Rjected Awaiting requested review from Rjected Rjected is a code owner

@fgimenez fgimenez Awaiting requested review from fgimenez fgimenez is a code owner

@onbjerg onbjerg Awaiting requested review from onbjerg onbjerg is a code owner

@shekhirin shekhirin Awaiting requested review from shekhirin shekhirin is a code owner

@gakonst gakonst Awaiting requested review from gakonst gakonst is a code owner

@rkrasiuk rkrasiuk Awaiting requested review from rkrasiuk rkrasiuk is a code owner

Assignees
No one assigned
Labels
A-op-reth Related to Optimism and op-reth A-rpc Related to the RPC implementation C-debt A clean up/refactor of existing code
Projects
None yet
Milestone
No milestone
Development

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