Skip to content
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

Test if the block downloader and verifier error checking is correct #2909

Closed
conradoplg opened this issue Oct 19, 2021 · 2 comments
Closed
Labels
C-enhancement Category: This is an improvement

Comments

@conradoplg
Copy link
Collaborator

Motivation

In #2890 a downcast was added in the block downloader and verifier, after verifying the block, to differentiate between a verification error and a generic service error (since the buffered verification service returns a BoxError). Depending on the downcast result the underlying error is put into one of the BlockDownloadVerifyError variants.

The error returned is eventually used in should_restart_sync by the syncer to ignore specific errors when deciding whether to restart the sync or not.

However, if the downcast is wrong (or becomes wrong after some refactoring, for example) then this filter will fail because all errors will be considered "service errors" which will make the syncer restart everytime one of these "ignorable" errors happen, which slows the sync down.

#2890 added a best-effort check which logs an error if it detects that happening. However, it would be best to have a proper test for it, however it is tricky to write and was postponed.

In other words, what needs to be tested is: when these errors happen, are they correctly detected by should_restart_sync?

Specifications

Designs

Related Work

@conradoplg conradoplg added C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage labels Oct 19, 2021
@mpguerra
Copy link
Contributor

@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Dec 10, 2021
@teor2345
Copy link
Contributor

teor2345 commented Jun 2, 2022

We should fix the actual bug risk in #2908 instead.

@teor2345 teor2345 closed this as not planned Won't fix, can't repro, duplicate, stale Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: This is an improvement
Projects
None yet
Development

No branches or pull requests

3 participants