-
Notifications
You must be signed in to change notification settings - Fork 75
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
Refactor transport
Module to ethrpc
#699
Conversation
@@ -43,7 +43,7 @@ impl Default for Configuration { | |||
/// Buffered `Transport` implementation that implements automatic batching of | |||
/// JSONRPC requests. | |||
#[derive(Clone, Debug)] | |||
pub struct Buffered<Inner> { | |||
pub struct BufferedTransport<Inner> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although I do think BatchingTransport
might be a slightly better name. BatchTransport
is a name already used by the web3
crate to mean something different (a transport allowing batches), but I don't think that's a problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I called it Buffered
instead of Batched
just because it does primarily request buffering (and, as a side-effect, automatically groups buffered requests into batches).
For example:
Configuration {
max_batch_size: 1,
max_concurrent_requests: 5,
..Default::default()
}
Will just buffer requests and make sure only 5 at a time are sent to the node, but never make any batched RPC requests.
Don't know if this makes sense. Also fine with changing it, I'm not too attached to the name (and "buffered" is quite generic lacking specificity).
3a6961c
to
a2c46a5
Compare
7bbc123
to
e07dcf7
Compare
In refactoring for configurable max batch size, I wanted to move all
Web3
related code into a parent module.transport
felt like the right place, but with the wrong name.This PR just renames the
transport
module toethrpc
and moves theWeb3*
types to this new module. I felt like this name was slightly more appropriate because:transport
is very generic andweb3
-crate-specific terminologyethrpc
more accurately represents what the module is for (i.e. all Ethereum RPC related code: theWeb3
"connection" instance, underlyingTransport
implementations, Ethereum RPC extensions, etc.)Test Plan
CI. No logic changes, just moved code.