-
Notifications
You must be signed in to change notification settings - Fork 143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fail-fast when block sync #1350
Conversation
026ec04
to
a5f2174
Compare
Codecov Report
@@ Coverage Diff @@
## main #1350 +/- ##
==========================================
- Coverage 86.03% 76.92% -9.12%
==========================================
Files 361 253 -108
Lines 32457 16982 -15475
==========================================
- Hits 27925 13063 -14862
- Misses 2747 3397 +650
+ Partials 1785 522 -1263
|
) < 0 | ||
&& (canonComparer.Compare( | ||
blockChain.PerceiveBlock(blockChain.Tip), | ||
blockChain.PerceiveBlock(syncedB.Tip)) < 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it okay without PerceivedTime
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's okay because BlockChain<T>.PerceiveBlock()
uses perceivedTime
only when the given excerpt is not found on its own storage. cc @dahlia
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🆗
/rebase |
a5f2174
to
a32320f
Compare
a32320f
to
99dd04d
Compare
99dd04d
to
b1c65b8
Compare
/rebase |
b1c65b8
to
61f989b
Compare
This PR removes null check before chain swapping when block synchronization. it's to fail quickly instead of malfunctioning when a problem situation such as #1349