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

Optimism RPC Types #2

Merged
merged 25 commits into from
Apr 27, 2024
Merged

Optimism RPC Types #2

merged 25 commits into from
Apr 27, 2024

Conversation

EmperorOrokuSaki
Copy link
Contributor

@EmperorOrokuSaki EmperorOrokuSaki commented Apr 10, 2024

Motivation

Updating the repository with Optimism RPC types.

Solution

Implement the needed OP specific RPC types and re-export whenever feasible.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking 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.

good direction, left a few more suggestions

crates/op-rpc-types/src/op/transaction/request.rs Outdated Show resolved Hide resolved
crates/op-rpc-types/src/op/transaction/receipt.rs Outdated Show resolved Hide resolved
@EmperorOrokuSaki EmperorOrokuSaki marked this pull request as ready for review April 12, 2024 09:22
Copy link
Member

@gakonst gakonst left a comment

Choose a reason for hiding this comment

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

the entire PR needs to re-export RPC types where needed, and extend them everywhere else, the requirement is "there must not be any duplicate types between the 2 crates"

i would start from the lower level crate whether it's consensus or rpc types from your POV and do that first, and you can do the other in separate PR

but def don't copy paste big structs around, find the extension fields and add them there

overall good start!

crates/op-consensus/src/receipt/envelope.rs Outdated Show resolved Hide resolved
crates/op-consensus/src/receipt/envelope.rs Outdated Show resolved Hide resolved
crates/op-rpc-types/src/op/block.rs Outdated Show resolved Hide resolved
crates/op-rpc-types/src/op/filters.rs Outdated Show resolved Hide resolved
crates/op-rpc-types/src/op/pubsub.rs Outdated Show resolved Hide resolved
crates/op-rpc-types/src/op/transaction/request.rs Outdated Show resolved Hide resolved
crates/op-rpc-types/src/op/transaction/mod.rs Show resolved Hide resolved
crates/op-rpc-types/src/op/pubsub.rs Outdated Show resolved Hide resolved
crates/op-rpc-types/src/op/filters.rs Outdated Show resolved Hide resolved
crates/op-rpc-types/src/op/call.rs Outdated Show resolved Hide resolved
@EmperorOrokuSaki
Copy link
Contributor Author

PR for making structs in the main alloy crate take generics: alloy-rs/alloy#573

@EmperorOrokuSaki EmperorOrokuSaki changed the title [WIP] Optimism RPC Types Optimism RPC Types Apr 20, 2024
@EmperorOrokuSaki
Copy link
Contributor Author

Relevant issue: #5

Cargo.toml Outdated Show resolved Hide resolved
@mattsse mattsse mentioned this pull request Apr 27, 2024
@mattsse
Copy link
Member

mattsse commented Apr 27, 2024

keeping this open because we still need receipt types, but to make incremental progress I've extracted the things we definitely need to #6

can't push here because main branch

@mattsse mattsse merged commit cf248d4 into alloy-rs:main Apr 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants