-
Notifications
You must be signed in to change notification settings - Fork 215
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
[Merged by Bors] - Simplify switch to different poet services #6116
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #6116 +/- ##
=======================================
Coverage 82.0% 82.1%
=======================================
Files 309 309
Lines 34033 34120 +87
=======================================
+ Hits 27919 28014 +95
+ Misses 4329 4324 -5
+ Partials 1785 1782 -3 ☔ View full report in Codecov by Sentry. |
- fixed review issues
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 the logic below needs to be updated. The idea was that if all registrations failed and it's already too late to submit, then the challenge expires. I think we could simplify the check now. If we have 0 registrations at the end of this function and the registration deadline passed, we expire the challenge.
allInvalid := true
for err := range errChan {
if err == nil {
allInvalid = false
continue
}
nb.logger.Warn("failed to submit challenge to poet", zap.Error(err), log.ZShortStringer("smesherID", nodeID))
if !errors.Is(err, ErrInvalidRequest) {
allInvalid = false
}
}
if allInvalid {
nb.logger.Warn("all poet submits were too late. ATX challenge expires", log.ZShortStringer("smesherID", nodeID))
# Conflicts: # activation/activation_errors.go # activation/nipost_test.go
For me there are now only 2 points left open to merge this PR (see my last 2 comments). @poszu could you also take another look? |
bors merge |
## Motivation Fix of issue #5563 Co-authored-by: Matthias <5011972+fasmat@users.noreply.github.com>
Build failed: |
bors merge |
## Motivation Fix of issue #5563 Co-authored-by: Matthias <5011972+fasmat@users.noreply.github.com>
Pull request successfully merged into develop. Build succeeded: |
Motivation
Fix of issue #5563
Description
Implemented:
Test Plan
Tested by unit tests
TODO