-
Notifications
You must be signed in to change notification settings - Fork 2.7k
babe: Disable unused median calculation #3251
Conversation
|
I have also removed annoying forbid/deny clauses. |
64c21f3 to
84f9f50
Compare
marcio-diaz
left a comment
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.
lgtm
5082ded to
0947a5b
Compare
|
Reverted removal of forbid/deny clauses to avoid conflicting with #3213. |
| //! | ||
| //! BABE (Blind Assignment for Blockchain Extension) consensus in Substrate. | ||
| #![forbid(unsafe_code, missing_docs, unused_must_use, unused_imports, unused_variables)] |
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.
| #![forbid(unsafe_code, missing_docs, unused_must_use, unused_imports, unused_variables)] | |
| #![forbid(unsafe_code, missing_docs)] |
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.
Originally I removed all forbid/deny but since Gav did the same in #3213 I reverted to avoid conflicts. The only thing I removed was forbid(dead_code) which was necessary to make this PR compile.
Demi-Marie
left a comment
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.
This looks good, although I would like to know what was wrong with the median algorithm.
|
@demimarie-parity a180365 (see #3248 as well). |
The median calculation that is done to figure out at what time the current slot should start (i.e. offset) is currently not properly implemented. Since it is currently unused I have removed the call from verify to it to avoid any issues. I have also fixed an issue that caused it to panic.
Fixes #3248.