-
Notifications
You must be signed in to change notification settings - Fork 253
PoT parameters change and sync #1929
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
Merged
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
b4004d6
Small refactoring with to reduce code duplication and prepare for PoT…
nazar-pc 95fc104
Support parameters change in `PotSource` and block import/verificatio…
nazar-pc 6cbde16
Make `PotSource` follow canonical fork of the blockchain and rebase P…
nazar-pc bd5051a
Refactor `update_next_slot_input` to take updated parameters change v…
nazar-pc 96291ce
Small tweaks in `PotSource` to remove unnecessary `.await`
nazar-pc 521878e
Move `update_next_slot_input` into new struct `PotState` and its meth…
nazar-pc 76744ee
Introduce `PotState::try_extend` to generalize more of state management
nazar-pc efcc84c
Move `gossip` module under `source`
nazar-pc 5010a4f
Modify `PotState` to become sharable data structure with updates usin…
nazar-pc fd89282
Move `run_timekeeper` function into `timekeeper` submodule
nazar-pc 4932a4b
Make timekeeper rebase to latest proof of time available if necessary
nazar-pc cddb769
Fix verification
nazar-pc fcf7e4d
Tiny formatting change
nazar-pc e49a68a
Remove timekeeper reorg of PoT chain, only blockchain should do reorg…
nazar-pc 105ac8e
Fix initial slot in proof of time chain verification and improve veri…
nazar-pc 42d4b03
Clean up old checkpoints in slot worker in case PoT chain reorg happened
nazar-pc d63dd4d
Implementation of block import handling without runtime API in `PotSo…
nazar-pc 1b9ed48
Check proof of time in slot worker to ensure bad proofs of time are n…
nazar-pc 7ed2bd1
Relax reorg check in PoT state
nazar-pc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,8 @@ use futures::channel::oneshot; | |
| use lru::LruCache; | ||
| use parking_lot::Mutex; | ||
| use sp_consensus_slots::Slot; | ||
| #[cfg(feature = "pot")] | ||
| use sp_consensus_subspace::PotParametersChange; | ||
| use std::num::{NonZeroU32, NonZeroUsize}; | ||
| use std::sync::Arc; | ||
| use subspace_core_primitives::{PotCheckpoints, PotProof, PotSeed}; | ||
|
|
@@ -64,14 +66,20 @@ impl PotVerifier { | |
|
|
||
| /// Verify a single proof of time that is `slots` slots away from `seed`. | ||
| /// | ||
| /// In case `maybe_parameters_change` is present, it will not affect provided `seed` and | ||
| /// `slot_iterations`, meaning if parameters change occurred at `slot`, provided `seed` and | ||
| /// `slot_iterations` must already account for that. | ||
| /// | ||
| /// NOTE: Potentially much slower than checkpoints, prefer [`Self::verify_checkpoints()`] | ||
| /// whenever possible. | ||
| pub async fn is_proof_valid( | ||
| &self, | ||
| #[cfg(feature = "pot")] mut slot: Slot, | ||
| mut seed: PotSeed, | ||
| slot_iterations: NonZeroU32, | ||
| #[cfg_attr(not(feature = "pot"), allow(unused_mut))] mut slot_iterations: NonZeroU32, | ||
| slots: Slot, | ||
| proof: PotProof, | ||
| #[cfg(feature = "pot")] mut maybe_parameters_change: Option<PotParametersChange>, | ||
| ) -> bool { | ||
| let mut slots = u64::from(slots); | ||
|
|
||
|
|
@@ -90,32 +98,51 @@ impl PotVerifier { | |
| async move { | ||
| // Result doesn't matter here | ||
| let _ = result_sender | ||
| .send(verifier.derive_next_seed(seed, slot_iterations).await); | ||
| .send(verifier.calculate_proof(seed, slot_iterations).await); | ||
| } | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| seed = match result_receiver.await { | ||
| Ok(Some(seed)) => seed, | ||
| let proof = match result_receiver.await { | ||
| Ok(Some(proof)) => proof, | ||
| _ => { | ||
| return false; | ||
| } | ||
| }; | ||
nazar-pc marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| slots -= 1; | ||
| #[cfg(feature = "pot")] | ||
| { | ||
| slot = slot + Slot::from(1); | ||
| } | ||
|
|
||
| #[cfg(feature = "pot")] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could be probably joined with the previous cfg-block.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then another level of indentation would have to be added and I wanted to avoid that. |
||
| if let Some(parameters_change) = maybe_parameters_change | ||
| && parameters_change.slot == slot | ||
| { | ||
| slot_iterations = parameters_change.slot_iterations; | ||
| seed = proof.seed_with_entropy(¶meters_change.entropy); | ||
| maybe_parameters_change.take(); | ||
| } else { | ||
| seed = proof.seed(); | ||
| } | ||
| #[cfg(not(feature = "pot"))] | ||
| { | ||
| seed = proof.seed(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Derive next seed, proving might be used if necessary | ||
dariolina marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // TODO: False-positive, lock is not actually held over await point, remove suppression once | ||
| // fixed upstream | ||
| #[allow(clippy::await_holding_lock)] | ||
| async fn derive_next_seed( | ||
| async fn calculate_proof( | ||
| &self, | ||
| seed: PotSeed, | ||
| slot_iterations: NonZeroU32, | ||
| ) -> Option<PotSeed> { | ||
| ) -> Option<PotProof> { | ||
| let cache_key = CacheKey { | ||
| seed, | ||
| slot_iterations, | ||
|
|
@@ -128,7 +155,7 @@ impl PotVerifier { | |
| drop(cache); | ||
| let correct_checkpoints = cache_value.checkpoints.lock().await; | ||
| if let Some(correct_checkpoints) = correct_checkpoints.as_ref() { | ||
| return Some(correct_checkpoints.output().seed()); | ||
| return Some(correct_checkpoints.output()); | ||
| } | ||
|
|
||
| // There was another verification for these inputs and it wasn't successful, | ||
|
|
@@ -178,9 +205,9 @@ impl PotVerifier { | |
| return None; | ||
| }; | ||
|
|
||
| let seed = generated_checkpoints.output().seed(); | ||
| let proof = generated_checkpoints.output(); | ||
| checkpoints.replace(generated_checkpoints); | ||
| return Some(seed); | ||
| return Some(proof); | ||
| } | ||
| } | ||
|
|
||
|
|
||
Oops, something went wrong.
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.
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.
Can we put these three variables and two following if-statements under the same cfg-block?
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 did that in some cases, here doing
let (slot_iterations, pot_seed, next_slot) = {}would be hardly more readable.