Enable download of future forks #10739
Conversation
arkpar
added
A0-please_review
| // Download the fork only if it is behind or not too far ahead our tip of the chain | ||
| // Otherwise it should be downloaded in full sync mode. | ||
| if r.number <= best_num || | ||
| (r.number - best_num).saturated_into::<u32>() < MAX_BLOCKS_TO_REQUEST as u32 |
There was a problem hiding this comment.
@eskimor let's hope disputes travel & resolve quickly! Haha
Seriously, though, this seems brittle so kind of OK for a dirty patch but we should have a think about doing this 'right' which will involve changes outside of sync about fork-choice.
|
I'm waiting on giving an approval as a result of testing on Versi to make sure this alleviates the stalls we've been seeing. |
| net.peer(0).num_peers() == 1 && | ||
| net.peer(2).num_peers() == 1 |
There was a problem hiding this comment.
This should be the consequence of net.peer(1).num_peers() == 2?
There was a problem hiding this comment.
This change is unrelated to the fix. Just noticed that a test is racing. Peers register connections asynchronously. So if peers0 considers peer1 to be connected, peer1 might not yet register peer0 as connected and will miss the block announcement that follows. Resulting in occasional test failure.
|
Is this still being tested? |
AFAIK it was successful, let us merge it. |
|
bot merge |
|
Bot will approve on the behalf of @bkchr, since they are a team lead, in an attempt to reach the minimum approval count |
![paritytech-processbot[bot]](https://avatars.githubusercontent.com/in/144942?s=60&v=4){kind=small}
* Enable download of future forks * Fixed external tests
* Enable download of future forks * Fixed external tests
* Enable download of future forks * Fixed external tests
Currently sync won't download forks ahead of the best chain announced with
is_best=falseThis PR removes this limitation as long as the fork is not too long.