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
Fix: Don't crash on bad ASNs during indexing #2586
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #2586 +/- ##
==========================================
+ Coverage 92.67% 92.72% +0.04%
==========================================
Files 139 139
Lines 5996 6003 +7
==========================================
+ Hits 5557 5566 +9
+ Misses 439 437 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Ah, now that I can see the source files, the coverage change in |
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 agree the crash on startup is a bad thing so Im good with this.
It is potentially modifying someones data in a kind of silent way (yes it logs the error of course but our users don't typically see these without looking), which obviously isn't ideal but I still don't really see the use case of an ASN > 9 million or whatever so on balance I think this is the better compromise. Again, my opinion
The range is about 4.2 billion with the validators as they are now. I do really think that should be enough. The spawning issue had an ASN of a few quadrillion, which seems to have been accidentally added anyway. |
Ha yes, 4.2 billion = enough |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion or issue for related concerns. |
Proposed change
I don't really know how the ASN value in the linked issue was allowed past the migration, but I guess it was. Perhaps the choice of DB backend affects it?
So, during indexing, if an ASN value would value the indexing to fail, instead log an error and reset the ASN to a an allowed value. I do think this is better than a straight up crash, but if there's differing opinions, let me know.
Fixes #2583
Type of change
Checklist:
pre-commit
hooks, see documentation.