style: replace next_sync_target Receiver loop with call to wait_for
#3618
Conversation
onbjerg
added
C-debt
Codecov Report
... and 7 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| _ => return Err(StageError::StageCheckpoint(checkpoint)), | ||
| }; | ||
|
|
||
| Ok(SyncGap { local_head, target }) | ||
| } | ||
|
|
||
| async fn next_sync_target(&mut self, head: BlockNumber) -> SyncTarget { | ||
| async fn next_sync_target(&mut self, head: BlockNumber) -> Option<SyncTarget> { |
There was a problem hiding this comment.
why does this now return an Option why is this okay?
There was a problem hiding this comment.
I originally wanted to return a Result but the signature doesn't provide enough information for creating a StageError::StageCheckpoint, so the Option is looked at to return a StageError in the event we return None. This is possible because a channel closure was previously unhandled, now it will return None and then return a StageError.
To be clear, the error only appears if the channel has been closed AND the last value has been seen previously. If the channel closes and the last value has not yet been seen wait_for will not return an error.
There was a problem hiding this comment.
This is possible because a channel closure was previously unhandled, now it will return None and then return a StageError.
okay I see, because we never handled the error case here (sender is dropped)
|
@mattsse The only difference is that in the event of a channel closure with a previously seen value will return a |
This PR replaces the loop polling in the Header Stage for acquiring the next tip target with a call to
wait_forintokio::sync::watch::Receiver. As far as I can tell, the internal implementation ofwait_foris almost identical to the loop used in the Reth code, so I think we should prefer to use the library's implementation over a custom one.I've also modified the return signature of
next_sync_targetto return an option so that we can return an error in the event of a channel closure (which previously was hidden). All geth & ef tests pass.