feat(utils): BlockWatcher
#108
Conversation
src/utils/block_watcher.rs
Outdated
| let sub = match self.host_provider.subscribe_blocks().await { | ||
| Ok(sub) => sub, | ||
| Err(err) => { | ||
| error!(error = ?err, "failed to subscribe to host chain blocks"); |
There was a problem hiding this comment.
use display, not debug
additional nits:
- could bind
Err(error)to avoid theerror = %errlater failed to subscribe to host chain blockswill appear in the event twice. once in theerrorfield and once in the event message.
Is the error type useful since there only 1 possible failure mode?
There was a problem hiding this comment.
so, there's indeed only one failure mode (pubsub not being available on the provider), but, the error message itself is quite clear, so I don't mind just using it. we can remove the added string
There was a problem hiding this comment.
is the error type useful? what does it provide that TransportError does not? Will our code be less useful or safe if we delete the type and use TransportError instead?
| } | ||
|
|
||
| async fn task_future(self) { | ||
| let mut sub = match self.host_provider.subscribe_blocks().await { |
There was a problem hiding this comment.
reading again, this actually doesn't produce a BlockWatcherError. so the error is only produced by with_current_block, and in that context the error description ("failed to subscribe") is incorrect
at this point, i think the error type should be deleted
| self.block_number.send_replace(block_number); | ||
| trace!(block_number, "updated host block number"); | ||
| } | ||
| Err(RecvError::Lagged(missed)) => { |
There was a problem hiding this comment.
what is the downside of this? why is it a library warn?
| warn!(%missed, "block subscription lagged"); | ||
| } | ||
| Err(RecvError::Closed) => { | ||
| error!("block subscription closed"); |
There was a problem hiding this comment.
this is not an error, it happens intentionally during normal program operation
| /// can be read or awaited for changes. This allows multiple tasks to observe | ||
| /// block number updates. | ||
| #[derive(Debug, Clone)] | ||
| pub struct SharedBlockNumber(watch::Receiver<u64>); |
There was a problem hiding this comment.
what does this type provide that watch::Receiver<u64> does not?
| } | ||
|
|
||
| /// Spawns the block watcher task. | ||
| pub fn spawn(self) -> JoinHandle<()> { |
There was a problem hiding this comment.
it should not be possible to spawn the task without having a subscription to it. i.e. the spawn fn should either take a Sender as an arg, or return a Receiver as an arg
Closes ENG-1677