-
Notifications
You must be signed in to change notification settings - Fork 425
feat(provider
): watch_full_blocks
#2194
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
Conversation
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.
not immediately obvious why this needs an additional task.
I'm a bit confused by how the PollerBuilder stuff works, or why this appears to be so complex,
isn't this jus feature just mapped stream effectively?
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.
a few more questions
|
||
/// Consumes the stream of block hashes from the inner [`PollerBuilder`] and maps it to a stream | ||
/// of [`BlockResponse`]. | ||
pub fn into_stream(self) -> impl Stream<Item = TransportResult<BlockResp>> + Unpin { |
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.
if we return anonymous types then this should also enforce Send because this can't be inferred on the callsite
but this is likely problematic for wasm compat
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.
we don't do this anywhere for streams
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.
this however means we can't send this stream, e.g. for background monitoring.
maybe this limitation is fine but doesn't sound ideal, we could roll our own stream type that this generic over the future so that Send is auto impl.
but this can also be a followup
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.
you can call into_stream
after sending, but yes ideally these are all named types...
|
||
/// Consumes the stream of block hashes from the inner [`PollerBuilder`] and maps it to a stream | ||
/// of [`BlockResponse`]. | ||
pub fn into_stream(self) -> impl Stream<Item = TransportResult<BlockResp>> + Unpin { |
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.
we don't do this anywhere for streams
|
||
/// Consumes the stream of block hashes from the inner [`PollerBuilder`] and maps it to a stream | ||
/// of [`BlockResponse`]. | ||
pub fn into_stream(self) -> impl Stream<Item = TransportResult<BlockResp>> + Unpin { |
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.
this however means we can't send this stream, e.g. for background monitoring.
maybe this limitation is fine but doesn't sound ideal, we could roll our own stream type that this generic over the future so that Send is auto impl.
but this can also be a followup
@@ -496,6 +496,36 @@ pub trait Provider<N: Network = Ethereum>: Send + Sync { | |||
Ok(PollerBuilder::new(self.weak_client(), "eth_getFilterChanges", (id,))) | |||
} | |||
|
|||
/// Watch for new blocks by polling the provider with |
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.
shouldn't this be just watch_blocks().full()
instead of a separate method?
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'd like to keep it as a different method because if we want to get a block with full txns we'll have to do watch_blocks().await?.full().full_txns()
vs watch_full_blocks().await?.full()
Motivation
Partially addresses #325
Solution
WatchBlocks
WatchBlocks
consumes the stream of block hashes fromPollerBuilder
and maps it toStream<Item = TransportResult<BlockResponse>
PR Checklist