Some refactorings to sc-consensus-subspace #99
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
liuchengxu
requested review from
nazar-pc,
ozgunozerk and
rg3l3dr
as code owners
liuchengxu
commented
Oct 31, 2021
nazar-pc
reviewed
Nov 1, 2021
| @@ -26,7 +26,7 @@ use sp_core::Public; | |||
| use sp_runtime::{traits::DigestItemFor, traits::Header, RuntimeAppPublic}; | |||
| use subspace_archiving::archiver; | |||
| use subspace_core_primitives::{crypto, Piece, Randomness, Salt, Sha256Hash}; | |||
| use subspace_solving::SubspaceCodec; | |||
| use subspace_solving::{derive_global_challenge, derive_local_challenge, SubspaceCodec}; | |||
There was a problem hiding this comment.
It was an intentional decision to not import functions in the codebase anywhere. They often have generic names and thus it makes sense to instead import a module they are contained in (think channel() vs mpsc::channel()).
There was a problem hiding this comment.
That's fair for some common topics, but not sure it's wise to apply it globally default.
|
liuchengxu
force-pushed
the
refactor-verify-solution
branch
2 times, most recently
from
7f6150e
to
b8d3c5f
Compare
Also make SubspaceSlotWorker `pub(super)`.
liuchengxu
force-pushed
the
refactor-verify-solution
branch
from
8881323
to
6427acf
Compare
nazar-pc
approved these changes
Nov 4, 2021
12 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #80.
This PR refactors
verify_solutionand separates outauthorship.rsandarchiver.rs(no code changes in these new modules) in sc-consensus-subspace, for better maintainability and future refactorings. Should be reviewed per commit.