-
Notifications
You must be signed in to change notification settings - Fork 210
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: calculate the batches per sessions correctly to achieve fault tolerance #4188
Conversation
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.
Probably @elsirion should review before merging, but might as well tick for the second reviewer.
you can validate it using https://github.com/Maan2003/fedimint/commits/consensus-test-old ./scripts/tests/consensus-fix.sh /path/to/old-fedimintd |
How are we going to test this change works as intended? I would like to see a test where a 3/4 federation with 1 guardian offline produces a session. #4198 may help with this. |
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.
Looks equivalent to 051cb73, which made sense to me.
// indefinitely. Hence we increase the delay between rounds exponentially | ||
// such that MAX_ROUND would only be reached after roughly 350 years. | ||
// indefinitely. Hence, we increase the delay between rounds exponentially | ||
// such that max_round would only be reached after a minimum of 100 years. |
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.
That is enforced by AlephBFT on startup, right?
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.
Yes
fix: calculate the batches per sessions correctly to achieve fault tolerance
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4188 +/- ##
==========================================
- Coverage 58.17% 57.90% -0.28%
==========================================
Files 192 197 +5
Lines 43225 43416 +191
==========================================
- Hits 25145 25138 -7
- Misses 18080 18278 +198 ☔ View full report in Codecov by Sentry. |
We really need to fix CI 😭 |
Successfully created backport PR for |
No description provided.