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

feat: move core rpc server types to standalone crate #8515

Merged
merged 13 commits into from
Jun 4, 2024

Conversation

leruaa
Copy link
Contributor

@leruaa leruaa commented May 30, 2024

ref #8460

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.

great start

with a standalone, lightweight crate we can also pull this in in reth-rpc and move the constants to reth-rpc-server-types

Cargo.toml Outdated Show resolved Hide resolved
crates/node-core/Cargo.toml Outdated Show resolved Hide resolved
crates/rpc/rpc-builder/src/lib.rs Outdated Show resolved Hide resolved
crates/rpc/rpc-server-types/Cargo.toml Outdated Show resolved Hide resolved
@mattsse mattsse added the A-rpc Related to the RPC implementation label May 30, 2024
@mattsse mattsse marked this pull request as ready for review May 30, 2024 17:57
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.

solid,

next we need the remaining types that are used in node-core

crates/node-core/Cargo.toml Show resolved Hide resolved
crates/rpc/rpc-server-types/src/lib.rs Outdated Show resolved Hide resolved
crates/rpc/rpc-server-types/Cargo.toml Outdated Show resolved Hide resolved
@leruaa
Copy link
Contributor Author

leruaa commented May 31, 2024

@mattsse Can you please have a look at my last commit and tell me if I'm heading in the right direction? Feels like I'm moving the entire rpc-builder crate...

@mattsse
Copy link
Collaborator

mattsse commented Jun 2, 2024

I see, the main problem is this config trait:

pub trait RethRpcConfig {

hmm, this is a bit tricky, I need to think about this a bit,

okay, for this pr, let's only do constants first, get the pr over the line and then continue with more incremental changes. it's fine if node-core then also depends on server-types.
but it will be easier to move stuff around once we have the crate

@leruaa
Copy link
Contributor Author

leruaa commented Jun 3, 2024

@mattsse I think this is ready to be reviewed.

P.S.: Don't hesitate if you can think of some good first issues for the next steps :)

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.

lgtm

next we can move these constants and types for cache

use reth_rpc::eth::cache::{
DEFAULT_BLOCK_CACHE_MAX_LEN, DEFAULT_CONCURRENT_DB_REQUESTS, DEFAULT_ENV_CACHE_MAX_LEN,
DEFAULT_RECEIPT_CACHE_MAX_LEN,
};

then I think we move the config traits

@mattsse mattsse added this pull request to the merge queue Jun 4, 2024
Merged via the queue into paradigmxyz:main with commit 60f6657 Jun 4, 2024
31 checks passed
mw2000 pushed a commit to mw2000/reth that referenced this pull request Jun 5, 2024
@leruaa leruaa deleted the rpc_server_types branch July 12, 2024 18:55
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Related to the RPC implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants