Implement BlocksClient for working with blocks
#671
Conversation
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
lexnv
changed the title
BlocksClient for working with blocks
Co-authored-by: James Wilson <james@jsdw.me>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
|
Looks awesome, good job! Just a couple of minor nits :) |
| pub fn subscribe_headers( | ||
| &self, | ||
| ) -> impl Future<Output = Result<impl Stream<Item = Result<T::Header, Error>>, Error>> | ||
| + Send | ||
| + 'static { | ||
| let client = self.client.clone(); | ||
| async move { client.rpc().subscribe_blocks().await } | ||
| } |
There was a problem hiding this comment.
Just thinking about that other PR; does this return all new blocks or all new "best" blocks?
Might be worth clarifying this in the docs either way?
I wonder whether if we only subscribe to all new best blocks, we might miss some blocks?
But I also guess that the block number might be the same for more than one block we get back via this call, if the "best" chain changes (or if we are asking for all blocks anyway).
So maybe w can't fill things in, but we can document precisely what is returned either way :)
There was a problem hiding this comment.
Just thinking about that other PR; does this return all new blocks or all new "best" blocks?
This does subscribe to all new "best" blocks indeed.
Might be worth clarifying this in the docs either way?
It's a good idea to document this properly.
But I also guess that the block number might be the same for more than one block we get back via this call, if the "best" chain changes (or if we are asking for all blocks anyway).
Yep, I think we can end up in a situation where we have two blocks with the same block number. In this case, we can't guarantee that we can fill in missing blocks.
There was a problem hiding this comment.
Yep, I think we can end up in a situation where we have two blocks with the same block number. In this case, we can't guarantee that we can fill in missing blocks.
Yup, for that reason I don't think we should even try; as long as we document what will be returned it's all good.
I do wonder whether we should be subscribing to all new blocks too. Or whether just to add both "subscribe_best_headers" and "subscribe_all_headers" to go with "subscribe_finalized_headers". But as you said inthe other PR, let's just do what the new RPC API is going to support for now (I can't remember!)
There was a problem hiding this comment.
The new API will support all events: blocks, best blocks, and finalized blocks.
We don't support the simple block events (non best and non finalized) in subxt yet.
We can easily extend this to add all types of events, tho I think we can wait for a bit and implement it on top of the substrate's new API once it's released.
There was a problem hiding this comment.
As long as the API and naming here means that we can migrate easily enough and add whatever else we need, I'm happy with it! And the current choice of function is what we use in events, so it makes sense to me how it is!
Let's just make sure to clearly document it :)
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
subxt/src/blocks/blocks_client.rs
Outdated
| E: Into<Error> + Send + 'static, | ||
| { | ||
| sub.flat_map(move |s| { | ||
| let rpc = client.clone(); |
There was a problem hiding this comment.
I don't understand why this is clone is needed, but I guess it the stream::iter below, can't you remove the "move stuff" there to avoid this clone?
There was a problem hiding this comment.
I don't recall off the top of my head why there is a clone here. My guess is removing the move stuff would introduce a lifetime on client which wouldn't work out since we want to return that thing. Perhaps we can avoid a little cloning somewhere though?
There was a problem hiding this comment.
Couldn't remove the clone from here, but removed one clone from above client.rpc().clone() when calling this function. The clone was circumvented by passing in a Client trait directly and then using the .rpc() inside the fn.
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
The filling in of missing finalized blocks can be applied to other use cases than that of finding events.
Move the core functionality of filling to the RPC client to make it more generic.
Blocked by #673.
Closes #630.