BABE refactoring: split out verification #3658
Conversation
Demi-Marie
added
the
A3-in_progress
|
Note that there is a big incoming refactor in #3583 which may conflict |
Demi-Marie
added
A0-please_review
| } | ||
|
|
||
| #[allow(deprecated)] | ||
| pub(super) fn make_transcript( |
rphmeier
left a comment
rphmeier
left a comment
There was a problem hiding this comment.
I don't understand what the conceptual boundary between slots and authorship is, given that everything within the slots module is about claiming slots, which is inherently tied to authorship.
Generally, I'm a proponent of keeping modules on the larger side and only aiming to break them up at around 1KLoC - so I suggest merging the slots and authorship modules into authorship, with potential future refactorings to reorganize once that module gets too large.
Other than that and the other nits, looks fine to me.
rphmeier
left a comment
rphmeier
left a comment
There was a problem hiding this comment.
I would also like to see BabeVerifier::verify and BabeBlockImport::import_block mostly extracted (and hopefully split into more unit-testable components).
i wouldn't mind that being a follow-on PR (although I would expect it to follow this one shortly) because it is a bit more logic-intensive.
rphmeier
added
A5-grumble
and removed
A0-please_review
rphmeier
added
A0-please_review
Move BABE verification into its own module.
This will fix #3642 once it is completed, which it isn’t.