Move already in state check #2327
Conversation
| @@ -70,6 +70,9 @@ const MIN_LOOKAHEAD_LIMIT: usize = zebra_consensus::MAX_CHECKPOINT_HEIGHT_GAP * | |||
| /// failure loop. | |||
| const TIPS_RESPONSE_TIMEOUT: Duration = Duration::from_secs(6); | |||
|
|
|||
| /// Controls how long we wait for a state `Depth` response to return. | |||
| const DEPTH_RESPONSE_TIMEOUT: Duration = Duration::from_secs(6); | |||
There was a problem hiding this comment.
I am not totally sure if we want a timeout for this, or if we can use some already existing constant, or if we don't need a timer at all.
There was a problem hiding this comment.
This request doesn't need a timeout - there are no awaits or futures in its code:
https://github.com/ZcashFoundation/zebra/blob/main/zebra-state/src/service.rs#L601
Only block verify/commit, transaction verify/commit, and AwaitUTXO requests need timeouts.
Janito just asked a similar question, so I've updated the documentation:
https://github.com/ZcashFoundation/zebra/pull/2337/files#diff-750c624c9e2c5c7252b8cb6036b9d6ac8d90bb6c4e4afa68025feb98223ea2eb
There was a problem hiding this comment.
You can also read the Depth request docs here:
https://github.com/ZcashFoundation/zebra/blob/main/zebra-state/src/request.rs#L207
It says that if the block is missing, it returns None.
There was a problem hiding this comment.
It seems not possible to remove the timer wrapper from this service due to https://github.com/ZcashFoundation/zebra/pull/2327/files#diff-d62cdeb1ec8372f0d832318236258f39a8b89a7fe87989723199a06f5deddeacR267
If this is changed and just attempt to pass the service:
expected struct tower::timeout::Timeout, found type parameter ZS
|
This PR is still worth doing, because it fixes part of #862. But it's a low priority. |
|
I added this PR to ticket #862, we can re-open it when we do that ticket. |
Motivation
We want to move an already in the state check from the zebra-consensus into the downloaders. Will close #2307 when done.
This change also implements part of #862.
Solution
We have 2 very similar downloaders in zebrad:
The inbound one already have this check in https://github.com/ZcashFoundation/zebra/blob/main/zebrad/src/components/inbound/downloads.rs#L200-L208
For the sync we bring the state and use the same check as the inbound, we then remove the check from zebra-consensus.
I used the check from inbound to sync instead of https://github.com/ZcashFoundation/zebra/blob/main/zebra-consensus/src/block.rs#L125-L139 because it looks simpler.
Review
This is currently a draft to ask @teor2345 some questions i added to the code as comments.
Reviewer Checklist