From 6bbfb33eb2b1823ea6059b075512e5edb78e0c7b Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Sat, 14 Oct 2023 21:42:01 +0300 Subject: [PATCH 1/2] Fix sync deadlock by avoiding async PotVerifier API with `tokio::task::spawn_blocking` entirely --- Cargo.lock | 2 - .../sc-consensus-subspace/src/slot_worker.rs | 20 +- crates/sc-consensus-subspace/src/verifier.rs | 27 +- crates/sc-proof-of-time/Cargo.toml | 2 - crates/sc-proof-of-time/src/source/gossip.rs | 35 +- crates/sc-proof-of-time/src/verifier.rs | 122 ++--- crates/sc-proof-of-time/src/verifier/tests.rs | 418 ++++++++---------- crates/subspace-service/src/lib.rs | 29 +- 8 files changed, 249 insertions(+), 406 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 902618df09..22b8b900b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9501,7 +9501,6 @@ dependencies = [ name = "sc-proof-of-time" version = "0.1.0" dependencies = [ - "async-lock", "atomic", "core_affinity", "derive_more", @@ -9523,7 +9522,6 @@ dependencies = [ "sp-runtime", "subspace-core-primitives", "subspace-proof-of-time", - "tokio", "tracing", ] diff --git a/crates/sc-consensus-subspace/src/slot_worker.rs b/crates/sc-consensus-subspace/src/slot_worker.rs index dea79fc1f2..559290d4a6 100644 --- a/crates/sc-consensus-subspace/src/slot_worker.rs +++ b/crates/sc-consensus-subspace/src/slot_worker.rs @@ -324,16 +324,12 @@ where }; // Ensure proof of time is valid according to parent block - if !self - .pot_verifier - .is_output_valid( - pot_input, - Slot::from(u64::from(slot) - u64::from(parent_slot)), - proof_of_time, - parent_pot_parameters.next_parameters_change(), - ) - .await - { + if !self.pot_verifier.is_output_valid( + pot_input, + Slot::from(u64::from(slot) - u64::from(parent_slot)), + proof_of_time, + parent_pot_parameters.next_parameters_change(), + ) { warn!( target: "subspace", "Proof of time is invalid, skipping block authoring at slot {slot:?}" @@ -363,11 +359,11 @@ where for slot in *parent_future_slot + 1..=*future_slot { let slot = Slot::from(slot); - let maybe_slot_checkpoints_fut = self.pot_verifier.get_checkpoints( + let maybe_slot_checkpoints = self.pot_verifier.get_checkpoints( checkpoints_pot_input.slot_iterations, checkpoints_pot_input.seed, ); - let Some(slot_checkpoints) = maybe_slot_checkpoints_fut.await else { + let Some(slot_checkpoints) = maybe_slot_checkpoints else { warn!("Proving failed during block authoring"); return None; }; diff --git a/crates/sc-consensus-subspace/src/verifier.rs b/crates/sc-consensus-subspace/src/verifier.rs index cfdadc48ea..8e51fffdcb 100644 --- a/crates/sc-consensus-subspace/src/verifier.rs +++ b/crates/sc-consensus-subspace/src/verifier.rs @@ -2,8 +2,6 @@ use crate::Error; use futures::lock::Mutex; -use futures::stream::FuturesUnordered; -use futures::StreamExt; use log::{debug, info, trace, warn}; use rand::prelude::*; use sc_client_api::backend::AuxStore; @@ -309,14 +307,15 @@ where // All checkpoints must be valid, at least according to the seed included in // justifications - let verification_results = FuturesUnordered::new(); for checkpoints in &checkpoints { if full_pot_verification { - verification_results.push(self.pot_verifier.verify_checkpoints( - seed, - slot_iterations, - checkpoints, - )); + // Try to find invalid checkpoints + if !self + .pot_verifier + .verify_checkpoints(seed, slot_iterations, checkpoints) + { + return Err(VerificationError::InvalidProofOfTime); + } } else { // We inject verified checkpoints in order to avoid full proving when votes // included in the block will inevitably be verified during block execution @@ -340,18 +339,6 @@ where slot_iterations = pot_input.slot_iterations; seed = pot_input.seed; } - // Try to find invalid checkpoints - if verification_results - // TODO: Ideally we'd use `find` here instead, but it does not yet exist: - // https://github.com/rust-lang/futures-rs/issues/2705 - .filter(|&success| async move { !success }) - .boxed() - .next() - .await - .is_some() - { - return Err(VerificationError::InvalidProofOfTime); - } } // Verify that block is signed properly diff --git a/crates/sc-proof-of-time/Cargo.toml b/crates/sc-proof-of-time/Cargo.toml index a32cdac7b5..8bb102df6b 100644 --- a/crates/sc-proof-of-time/Cargo.toml +++ b/crates/sc-proof-of-time/Cargo.toml @@ -11,7 +11,6 @@ include = [ ] [dependencies] -async-lock = "2.8.0" atomic = "0.5.3" core_affinity = "0.8.1" derive_more = "0.99.17" @@ -33,5 +32,4 @@ subspace-core-primitives = { version = "0.1.0", path = "../subspace-core-primiti subspace-proof-of-time = { version = "0.1.0", path = "../subspace-proof-of-time" } parking_lot = "0.12.1" rayon = "1.8.0" -tokio = { version = "1.32.0", features = ["macros", "time"] } tracing = "0.1.37" diff --git a/crates/sc-proof-of-time/src/source/gossip.rs b/crates/sc-proof-of-time/src/source/gossip.rs index 5933a53e6b..f0061f31a9 100644 --- a/crates/sc-proof-of-time/src/source/gossip.rs +++ b/crates/sc-proof-of-time/src/source/gossip.rs @@ -24,7 +24,6 @@ use std::hash::{Hash, Hasher}; use std::num::{NonZeroU32, NonZeroUsize}; use std::sync::{atomic, Arc}; use subspace_core_primitives::{PotCheckpoints, PotSeed, SlotNumber}; -use tokio::runtime::Handle; use tracing::{debug, error, trace, warn}; /// How many slots can proof be before it is too far @@ -280,11 +279,11 @@ where } } - if self - .pot_verifier - .verify_checkpoints(proof.seed, proof.slot_iterations, &proof.checkpoints) - .await - { + if self.pot_verifier.verify_checkpoints( + proof.seed, + proof.slot_iterations, + &proof.checkpoints, + ) { debug!(%sender, slot = %proof.slot, "Full verification succeeded"); self.engine @@ -385,7 +384,6 @@ where } // Avoid blocking gossip for too long - let handle = Handle::current(); rayon::spawn({ let engine = Arc::clone(&self.engine); let pot_verifier = self.pot_verifier.clone(); @@ -393,19 +391,19 @@ where let topic = self.topic; move || { - handle.block_on(Self::handle_potentially_matching_proofs( + Self::handle_potentially_matching_proofs( next_slot_input, potentially_matching_proofs, engine, &pot_verifier, from_gossip_sender, topic, - )); + ); } }); } - async fn handle_potentially_matching_proofs( + fn handle_potentially_matching_proofs( next_slot_input: PotNextSlotInput, potentially_matching_proofs: Vec<(GossipProof, Vec)>, engine: Arc>>, @@ -425,10 +423,11 @@ where // Verify all proofs for (proof, _senders) in &potentially_matching_proofs { - if pot_verifier - .verify_checkpoints(proof.seed, proof.slot_iterations, &proof.checkpoints) - .await - { + if pot_verifier.verify_checkpoints( + proof.seed, + proof.slot_iterations, + &proof.checkpoints, + ) { correct_proof.replace(*proof); break; } @@ -468,15 +467,17 @@ where } sent = true; - if let Err(error) = from_gossip_sender.send((sender, proof)).await { + engine.lock().gossip_message(topic, proof.encode(), false); + + if let Err(error) = + futures::executor::block_on(from_gossip_sender.send((sender, proof))) + { warn!( %error, slot = %proof.slot, "Failed to send future proof", ); } - - engine.lock().gossip_message(topic, proof.encode(), false); } } else { let engine = engine.lock(); diff --git a/crates/sc-proof-of-time/src/verifier.rs b/crates/sc-proof-of-time/src/verifier.rs index f43b13ccf1..a64b1af193 100644 --- a/crates/sc-proof-of-time/src/verifier.rs +++ b/crates/sc-proof-of-time/src/verifier.rs @@ -3,8 +3,6 @@ #[cfg(test)] mod tests; -use async_lock::Mutex as AsyncMutex; -use futures::channel::oneshot; use lru::LruCache; use parking_lot::Mutex; use sp_consensus_slots::Slot; @@ -12,8 +10,6 @@ use sp_consensus_subspace::{PotNextSlotInput, PotParametersChange}; use std::num::{NonZeroU32, NonZeroUsize}; use std::sync::Arc; use subspace_core_primitives::{PotCheckpoints, PotOutput, PotSeed}; -use tokio::runtime::Handle; -use tracing::trace; #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] struct CacheKey { @@ -23,7 +19,7 @@ struct CacheKey { #[derive(Debug, Clone)] struct CacheValue { - checkpoints: Arc>>, + checkpoints: Arc>>, } /// Verifier data structure that verifies and caches results of PoT verification @@ -54,7 +50,7 @@ impl PotVerifier { slot_iterations, }, CacheValue { - checkpoints: Arc::new(AsyncMutex::new(Some(checkpoints))), + checkpoints: Arc::new(Mutex::new(Some(checkpoints))), }, ); } @@ -66,28 +62,15 @@ impl PotVerifier { /// Try to get checkpoints for provided seed and slot iterations, returns `None` if proving /// fails internally. - pub async fn get_checkpoints( + pub fn get_checkpoints( &self, slot_iterations: NonZeroU32, seed: PotSeed, ) -> Option { - // TODO: This "proxy" is a workaround for https://github.com/rust-lang/rust/issues/57478 - let result_fut = tokio::task::spawn_blocking({ - let verifier = self.clone(); - - move || { - Handle::current().block_on(verifier.calculate_checkpoints( - slot_iterations, - seed, - true, - )) - } - }); - - result_fut.await.unwrap_or_default() + self.calculate_checkpoints(slot_iterations, seed, true) } - /// Try to get checkpoints quickly without waiting for potentially locked async mutex or proving + /// Try to get checkpoints quickly without waiting for potentially locked mutex or proving pub fn try_get_checkpoints( &self, slot_iterations: NonZeroU32, @@ -113,7 +96,7 @@ impl PotVerifier { /// /// NOTE: Potentially much slower than checkpoints, prefer [`Self::verify_checkpoints()`] /// whenever possible. - pub async fn is_output_valid( + pub fn is_output_valid( &self, input: PotNextSlotInput, slots: Slot, @@ -121,13 +104,12 @@ impl PotVerifier { maybe_parameters_change: Option, ) -> bool { self.is_output_valid_internal(input, slots, output, maybe_parameters_change, true) - .await } /// Does the same verification as [`Self::is_output_valid()`] except it relies on proofs being /// pre-validated before and will return `false` in case proving is necessary, this is meant to /// be a quick and cheap version of the function. - pub async fn try_is_output_valid( + pub fn try_is_output_valid( &self, input: PotNextSlotInput, slots: Slot, @@ -135,10 +117,9 @@ impl PotVerifier { maybe_parameters_change: Option, ) -> bool { self.is_output_valid_internal(input, slots, output, maybe_parameters_change, false) - .await } - async fn is_output_valid_internal( + fn is_output_valid_internal( &self, mut input: PotNextSlotInput, slots: Slot, @@ -149,19 +130,13 @@ impl PotVerifier { let mut slots = u64::from(slots); loop { - let maybe_calculated_checkpoints_fut = tokio::task::spawn_blocking({ - let verifier = self.clone(); - - move || { - Handle::current().block_on(verifier.calculate_checkpoints( - input.slot_iterations, - input.seed, - do_proving_if_necessary, - )) - } - }); + let maybe_calculated_checkpoints = self.calculate_checkpoints( + input.slot_iterations, + input.seed, + do_proving_if_necessary, + ); - let Ok(Some(calculated_checkpoints)) = maybe_calculated_checkpoints_fut.await else { + let Some(calculated_checkpoints) = maybe_calculated_checkpoints else { return false; }; let calculated_output = calculated_checkpoints.output(); @@ -182,10 +157,7 @@ impl PotVerifier { } /// Derive next seed, proving might be used if necessary - // TODO: False-positive, lock is not actually held over await point, remove suppression once - // fixed upstream - #[allow(clippy::await_holding_lock)] - async fn calculate_checkpoints( + fn calculate_checkpoints( &self, slot_iterations: NonZeroU32, seed: PotSeed, @@ -201,7 +173,7 @@ impl PotVerifier { let maybe_cache_value = cache.get(&cache_key).cloned(); if let Some(cache_value) = maybe_cache_value { drop(cache); - let correct_checkpoints = cache_value.checkpoints.lock().await; + let correct_checkpoints = cache_value.checkpoints.lock(); if let Some(correct_checkpoints) = *correct_checkpoints { return Some(correct_checkpoints); } @@ -228,17 +200,9 @@ impl PotVerifier { // Cache lock is no longer necessary, other callers should be able to access cache too drop(cache); - let (result_sender, result_receiver) = oneshot::channel(); - - rayon::spawn(move || { - let result = subspace_proof_of_time::prove(seed, slot_iterations); + let proving_result = subspace_proof_of_time::prove(seed, slot_iterations); - if let Err(_error) = result_sender.send(result) { - trace!("Verification result receiver is gone before result was sent"); - } - }); - - let Ok(Ok(generated_checkpoints)) = result_receiver.await else { + let Ok(generated_checkpoints) = proving_result else { // Avoid deadlock when taking a lock below drop(checkpoints); @@ -247,8 +211,7 @@ impl PotVerifier { if let Some(removed_cache_value) = maybe_removed_cache_value { // It is possible that we have removed a verified value that we have not // inserted, check for this and restore if that was the case - let removed_verified_value = - removed_cache_value.checkpoints.lock().await.is_some(); + let removed_verified_value = removed_cache_value.checkpoints.lock().is_some(); if removed_verified_value { self.cache.lock().push(cache_key, removed_cache_value); } @@ -262,33 +225,16 @@ impl PotVerifier { } /// Verify proof of time checkpoints - pub async fn verify_checkpoints( + pub fn verify_checkpoints( &self, seed: PotSeed, slot_iterations: NonZeroU32, checkpoints: &PotCheckpoints, ) -> bool { - // TODO: This "proxy" is a workaround for https://github.com/rust-lang/rust/issues/57478 - let result_fut = tokio::task::spawn_blocking({ - let verifier = self.clone(); - let checkpoints = *checkpoints; - - move || { - Handle::current().block_on(verifier.verify_checkpoints_internal( - seed, - slot_iterations, - &checkpoints, - )) - } - }); - - result_fut.await.unwrap_or_default() + self.verify_checkpoints_internal(seed, slot_iterations, checkpoints) } - // TODO: False-positive, lock is not actually held over await point, remove suppression once - // fixed upstream - #[allow(clippy::await_holding_lock)] - async fn verify_checkpoints_internal( + fn verify_checkpoints_internal( &self, seed: PotSeed, slot_iterations: NonZeroU32, @@ -303,7 +249,7 @@ impl PotVerifier { let mut cache = self.cache.lock(); if let Some(cache_value) = cache.get(&cache_key).cloned() { drop(cache); - let correct_checkpoints = cache_value.checkpoints.lock().await; + let correct_checkpoints = cache_value.checkpoints.lock(); if let Some(correct_checkpoints) = correct_checkpoints.as_ref() { return checkpoints == correct_checkpoints; } @@ -325,20 +271,11 @@ impl PotVerifier { // Cache lock is no longer necessary, other callers should be able to access cache too drop(cache); - let (result_sender, result_receiver) = oneshot::channel(); - - let checkpoints = *checkpoints; - rayon::spawn(move || { - let result = - subspace_proof_of_time::verify(seed, slot_iterations, checkpoints.as_slice()) - .unwrap_or_default(); - - if let Err(_error) = result_sender.send(result) { - trace!("Verification result receiver is gone before result was sent"); - } - }); + let verified_successfully = + subspace_proof_of_time::verify(seed, slot_iterations, checkpoints.as_slice()) + .unwrap_or_default(); - if !result_receiver.await.unwrap_or_default() { + if !verified_successfully { // Avoid deadlock when taking a lock below drop(correct_checkpoints); @@ -347,8 +284,7 @@ impl PotVerifier { if let Some(removed_cache_value) = maybe_removed_cache_value { // It is possible that we have removed a verified value that we have not // inserted, check for this and restore if that was the case - let removed_verified_value = - removed_cache_value.checkpoints.lock().await.is_some(); + let removed_verified_value = removed_cache_value.checkpoints.lock().is_some(); if removed_verified_value { self.cache.lock().push(cache_key, removed_cache_value); } @@ -357,7 +293,7 @@ impl PotVerifier { } // Store known good checkpoints in cache - correct_checkpoints.replace(checkpoints); + correct_checkpoints.replace(*checkpoints); return true; } diff --git a/crates/sc-proof-of-time/src/verifier/tests.rs b/crates/sc-proof-of-time/src/verifier/tests.rs index 8dbe9fa088..c1ab555472 100644 --- a/crates/sc-proof-of-time/src/verifier/tests.rs +++ b/crates/sc-proof-of-time/src/verifier/tests.rs @@ -9,8 +9,8 @@ const SEED: [u8; 16] = [ 0xd6, 0x66, 0xcc, 0xd8, 0xd5, 0x93, 0xc2, 0x3d, 0xa8, 0xdb, 0x6b, 0x5b, 0x14, 0x13, 0xb1, 0x3a, ]; -#[tokio::test] -async fn test_basic() { +#[test] +fn test_basic() { let genesis_seed = PotSeed::from(SEED); let slot_iterations = NonZeroU32::new(512).unwrap(); let checkpoints_1 = subspace_proof_of_time::prove(genesis_seed, slot_iterations).unwrap(); @@ -18,158 +18,114 @@ async fn test_basic() { let verifier = PotVerifier::new(genesis_seed, NonZeroUsize::new(1000).unwrap()); // Expected to be valid - assert!( - verifier - .is_output_valid( - PotNextSlotInput { - slot: Slot::from(1), - slot_iterations, - seed: genesis_seed, - }, - Slot::from(1), - checkpoints_1.output(), - None - ) - .await - ); - assert!( - verifier - .verify_checkpoints(genesis_seed, slot_iterations, &checkpoints_1) - .await - ); + assert!(verifier.is_output_valid( + PotNextSlotInput { + slot: Slot::from(1), + slot_iterations, + seed: genesis_seed, + }, + Slot::from(1), + checkpoints_1.output(), + None + )); + assert!(verifier.verify_checkpoints(genesis_seed, slot_iterations, &checkpoints_1)); // Invalid number of slots - assert!( - !verifier - .is_output_valid( - PotNextSlotInput { - slot: Slot::from(1), - slot_iterations, - seed: genesis_seed, - }, - Slot::from(2), - checkpoints_1.output(), - None - ) - .await - ); + assert!(!verifier.is_output_valid( + PotNextSlotInput { + slot: Slot::from(1), + slot_iterations, + seed: genesis_seed, + }, + Slot::from(2), + checkpoints_1.output(), + None + )); // Invalid seed - assert!( - !verifier - .is_output_valid( - PotNextSlotInput { - slot: Slot::from(1), - slot_iterations, - seed: checkpoints_1.output().seed(), - }, - Slot::from(1), - checkpoints_1.output(), - None - ) - .await - ); + assert!(!verifier.is_output_valid( + PotNextSlotInput { + slot: Slot::from(1), + slot_iterations, + seed: checkpoints_1.output().seed(), + }, + Slot::from(1), + checkpoints_1.output(), + None + )); // Invalid number of iterations - assert!( - !verifier - .verify_checkpoints( - genesis_seed, - slot_iterations - .checked_mul(NonZeroU32::new(2).unwrap()) - .unwrap(), - &checkpoints_1 - ) - .await - ); + assert!(!verifier.verify_checkpoints( + genesis_seed, + slot_iterations + .checked_mul(NonZeroU32::new(2).unwrap()) + .unwrap(), + &checkpoints_1 + )); let seed_1 = checkpoints_1.output().seed(); let checkpoints_2 = subspace_proof_of_time::prove(seed_1, slot_iterations).unwrap(); // Expected to be valid - assert!( - verifier - .is_output_valid( - PotNextSlotInput { - slot: Slot::from(2), - slot_iterations, - seed: seed_1, - }, - Slot::from(1), - checkpoints_2.output(), - None - ) - .await - ); - assert!( - verifier - .is_output_valid( - PotNextSlotInput { - slot: Slot::from(1), - slot_iterations, - seed: genesis_seed, - }, - Slot::from(2), - checkpoints_2.output(), - None - ) - .await - ); - assert!( - verifier - .verify_checkpoints(seed_1, slot_iterations, &checkpoints_2) - .await - ); + assert!(verifier.is_output_valid( + PotNextSlotInput { + slot: Slot::from(2), + slot_iterations, + seed: seed_1, + }, + Slot::from(1), + checkpoints_2.output(), + None + )); + assert!(verifier.is_output_valid( + PotNextSlotInput { + slot: Slot::from(1), + slot_iterations, + seed: genesis_seed, + }, + Slot::from(2), + checkpoints_2.output(), + None + )); + assert!(verifier.verify_checkpoints(seed_1, slot_iterations, &checkpoints_2)); // Invalid number of slots - assert!( - !verifier - .is_output_valid( - PotNextSlotInput { - slot: Slot::from(1), - slot_iterations, - seed: seed_1, - }, - Slot::from(2), - checkpoints_2.output(), - None - ) - .await - ); + assert!(!verifier.is_output_valid( + PotNextSlotInput { + slot: Slot::from(1), + slot_iterations, + seed: seed_1, + }, + Slot::from(2), + checkpoints_2.output(), + None + )); // Invalid seed - assert!( - !verifier - .is_output_valid( - PotNextSlotInput { - slot: Slot::from(1), - slot_iterations, - seed: seed_1, - }, - Slot::from(2), - checkpoints_2.output(), - None - ) - .await - ); + assert!(!verifier.is_output_valid( + PotNextSlotInput { + slot: Slot::from(1), + slot_iterations, + seed: seed_1, + }, + Slot::from(2), + checkpoints_2.output(), + None + )); // Invalid number of iterations - assert!( - !verifier - .is_output_valid( - PotNextSlotInput { - slot: Slot::from(1), - slot_iterations: slot_iterations - .checked_mul(NonZeroU32::new(2).unwrap()) - .unwrap(), - seed: genesis_seed, - }, - Slot::from(2), - checkpoints_2.output(), - None - ) - .await - ); + assert!(!verifier.is_output_valid( + PotNextSlotInput { + slot: Slot::from(1), + slot_iterations: slot_iterations + .checked_mul(NonZeroU32::new(2).unwrap()) + .unwrap(), + seed: genesis_seed, + }, + Slot::from(2), + checkpoints_2.output(), + None + )); } -#[tokio::test] -async fn parameters_change() { +#[test] +fn parameters_change() { let genesis_seed = PotSeed::from(SEED); let slot_iterations_1 = NonZeroU32::new(512).unwrap(); let entropy = [1; mem::size_of::()]; @@ -186,114 +142,90 @@ async fn parameters_change() { let verifier = PotVerifier::new(genesis_seed, NonZeroUsize::new(1000).unwrap()); // Changing parameters after first slot - assert!( - verifier - .is_output_valid( - PotNextSlotInput { - slot: Slot::from(1), - slot_iterations: slot_iterations_1, - seed: genesis_seed, - }, - Slot::from(1), - checkpoints_1.output(), - Some(PotParametersChange { - slot: Slot::from(2), - slot_iterations: slot_iterations_2, - entropy, - }) - ) - .await - ); + assert!(verifier.is_output_valid( + PotNextSlotInput { + slot: Slot::from(1), + slot_iterations: slot_iterations_1, + seed: genesis_seed, + }, + Slot::from(1), + checkpoints_1.output(), + Some(PotParametersChange { + slot: Slot::from(2), + slot_iterations: slot_iterations_2, + entropy, + }) + )); // Changing parameters in the middle - assert!( - verifier - .is_output_valid( - PotNextSlotInput { - slot: Slot::from(1), - slot_iterations: slot_iterations_1, - seed: genesis_seed, - }, - Slot::from(3), - checkpoints_3.output(), - Some(PotParametersChange { - slot: Slot::from(2), - slot_iterations: slot_iterations_2, - entropy, - }) - ) - .await - ); + assert!(verifier.is_output_valid( + PotNextSlotInput { + slot: Slot::from(1), + slot_iterations: slot_iterations_1, + seed: genesis_seed, + }, + Slot::from(3), + checkpoints_3.output(), + Some(PotParametersChange { + slot: Slot::from(2), + slot_iterations: slot_iterations_2, + entropy, + }) + )); // Changing parameters on last slot - assert!( - verifier - .is_output_valid( - PotNextSlotInput { - slot: Slot::from(1), - slot_iterations: slot_iterations_1, - seed: genesis_seed, - }, - Slot::from(2), - checkpoints_2.output(), - Some(PotParametersChange { - slot: Slot::from(2), - slot_iterations: slot_iterations_2, - entropy, - }) - ) - .await - ); + assert!(verifier.is_output_valid( + PotNextSlotInput { + slot: Slot::from(1), + slot_iterations: slot_iterations_1, + seed: genesis_seed, + }, + Slot::from(2), + checkpoints_2.output(), + Some(PotParametersChange { + slot: Slot::from(2), + slot_iterations: slot_iterations_2, + entropy, + }) + )); // Not changing parameters because changes apply to the very first slot that is verified - assert!( - verifier - .is_output_valid( - PotNextSlotInput { - slot: Slot::from(2), - slot_iterations: slot_iterations_2, - seed: checkpoints_1.output().seed_with_entropy(&entropy), - }, - Slot::from(2), - checkpoints_3.output(), - Some(PotParametersChange { - slot: Slot::from(2), - slot_iterations: slot_iterations_2, - entropy, - }) - ) - .await - ); + assert!(verifier.is_output_valid( + PotNextSlotInput { + slot: Slot::from(2), + slot_iterations: slot_iterations_2, + seed: checkpoints_1.output().seed_with_entropy(&entropy), + }, + Slot::from(2), + checkpoints_3.output(), + Some(PotParametersChange { + slot: Slot::from(2), + slot_iterations: slot_iterations_2, + entropy, + }) + )); // Missing parameters change - assert!( - !verifier - .is_output_valid( - PotNextSlotInput { - slot: Slot::from(1), - slot_iterations: slot_iterations_1, - seed: genesis_seed, - }, - Slot::from(3), - checkpoints_3.output(), - None - ) - .await - ); + assert!(!verifier.is_output_valid( + PotNextSlotInput { + slot: Slot::from(1), + slot_iterations: slot_iterations_1, + seed: genesis_seed, + }, + Slot::from(3), + checkpoints_3.output(), + None + )); // Invalid slot - assert!( - !verifier - .is_output_valid( - PotNextSlotInput { - slot: Slot::from(2), - slot_iterations: slot_iterations_1, - seed: genesis_seed, - }, - Slot::from(3), - checkpoints_3.output(), - Some(PotParametersChange { - slot: Slot::from(2), - slot_iterations: slot_iterations_2, - entropy, - }) - ) - .await - ); + assert!(!verifier.is_output_valid( + PotNextSlotInput { + slot: Slot::from(2), + slot_iterations: slot_iterations_1, + seed: genesis_seed, + }, + Slot::from(3), + checkpoints_3.output(), + Some(PotParametersChange { + slot: Slot::from(2), + slot_iterations: slot_iterations_2, + entropy, + }) + )); } diff --git a/crates/subspace-service/src/lib.rs b/crates/subspace-service/src/lib.rs index a9a4f93a27..d50eaf38c0 100644 --- a/crates/subspace-service/src/lib.rs +++ b/crates/subspace-service/src/lib.rs @@ -105,7 +105,6 @@ use subspace_proof_of_space::Table; use subspace_runtime_primitives::opaque::Block; use subspace_runtime_primitives::{AccountId, Balance, Hash, Nonce}; use subspace_transaction_pool::{FullPool, PreValidateTransaction}; -use tokio::runtime::Handle; use tracing::{debug, error, info, Instrument}; // There are multiple places where it is assumed that node is running on 64-bit system, refuse to @@ -345,23 +344,19 @@ where // valid if quick_verification { - tokio::task::block_in_place(|| { - Handle::current().block_on(pot_verifier.try_is_output_valid( - pot_input, - Slot::from(slot - u64::from(parent_slot)), - proof_of_time, - pot_parameters.next_parameters_change(), - )) - }) + pot_verifier.try_is_output_valid( + pot_input, + Slot::from(slot - u64::from(parent_slot)), + proof_of_time, + pot_parameters.next_parameters_change(), + ) } else { - tokio::task::block_in_place(|| { - Handle::current().block_on(pot_verifier.is_output_valid( - pot_input, - Slot::from(slot - u64::from(parent_slot)), - proof_of_time, - pot_parameters.next_parameters_change(), - )) - }) + pot_verifier.is_output_valid( + pot_input, + Slot::from(slot - u64::from(parent_slot)), + proof_of_time, + pot_parameters.next_parameters_change(), + ) } }, ) From 21a098fc20a2d8127463f090bcd6014ee695c09a Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Sat, 14 Oct 2023 22:21:33 +0300 Subject: [PATCH 2/2] Increase PoT verifier cache size temporarily --- crates/subspace-service/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/subspace-service/src/lib.rs b/crates/subspace-service/src/lib.rs index d50eaf38c0..db0b44eab6 100644 --- a/crates/subspace-service/src/lib.rs +++ b/crates/subspace-service/src/lib.rs @@ -113,7 +113,7 @@ const_assert!(std::mem::size_of::() >= std::mem::size_of::()); /// This is over 15 minutes of slots assuming there are no forks, should be both sufficient and not /// too large to handle -const POT_VERIFIER_CACHE_SIZE: NonZeroUsize = NonZeroUsize::new(10_000).expect("Not zero; qed"); +const POT_VERIFIER_CACHE_SIZE: NonZeroUsize = NonZeroUsize::new(30_000).expect("Not zero; qed"); const SYNC_TARGET_UPDATE_INTERVAL: Duration = Duration::from_secs(1); /// Error type for Subspace service.