From 9862c3e48074508aad656e0459fb7b95179bab42 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Mon, 31 Jul 2023 17:22:48 +0300 Subject: [PATCH 1/3] Fix sector expiration --- .../subspace-farmer/src/single_disk_plot/plotting.rs | 10 ++++++++-- crates/subspace-verification/src/lib.rs | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/subspace-farmer/src/single_disk_plot/plotting.rs b/crates/subspace-farmer/src/single_disk_plot/plotting.rs index f42f45e64d..936dcd0ca3 100644 --- a/crates/subspace-farmer/src/single_disk_plot/plotting.rs +++ b/crates/subspace-farmer/src/single_disk_plot/plotting.rs @@ -460,7 +460,11 @@ where .collect_into(&mut sectors_to_check); for (sector_index, history_size) in sectors_to_check.drain(..) { if let Some(sector_expire_at) = sectors_expire_at.get(§or_index) { - if *sector_expire_at >= archived_segment_header.segment_index() { + // +1 means we will start replotting a bit before it actually expires to avoid + // storing expired sectors + if *sector_expire_at + <= (archived_segment_header.segment_index() + SegmentIndex::ONE) + { // Time to replot sector_indices_to_replot.push(sector_index); } @@ -508,8 +512,10 @@ where metadata; qed", ); + // +1 means we will start replotting a bit before it actually expires to avoid + // storing expired sectors if expiration_history_size.segment_index() - >= archived_segment_header.segment_index() + <= (archived_segment_header.segment_index() + SegmentIndex::ONE) { // Time to replot sector_indices_to_replot.push(sector_index); diff --git a/crates/subspace-verification/src/lib.rs b/crates/subspace-verification/src/lib.rs index c7fc5e00d4..cabc2de5f8 100644 --- a/crates/subspace-verification/src/lib.rs +++ b/crates/subspace-verification/src/lib.rs @@ -283,7 +283,7 @@ where } }; - if expiration_history_size >= *current_history_size { + if expiration_history_size <= *current_history_size { return Err(Error::SectorExpired { expiration_history_size, current_history_size: *current_history_size, From 0a01c9083f9fad53466398ba9960d8cdfd831841 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Mon, 31 Jul 2023 18:27:58 +0300 Subject: [PATCH 2/3] Store archived segment header before sending notification --- crates/sc-consensus-subspace/src/archiver.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/crates/sc-consensus-subspace/src/archiver.rs b/crates/sc-consensus-subspace/src/archiver.rs index 4d5c7f9ae7..61c5f36a06 100644 --- a/crates/sc-consensus-subspace/src/archiver.rs +++ b/crates/sc-consensus-subspace/src/archiver.rs @@ -34,6 +34,7 @@ use sp_objects::ObjectsApi; use sp_runtime::generic::SignedBlock; use sp_runtime::traits::{Block as BlockT, CheckedSub, Header, NumberFor, One, Zero}; use std::future::Future; +use std::slice; use std::sync::atomic::{AtomicU16, Ordering}; use std::sync::Arc; use subspace_archiving::archiver::{Archiver, NewArchivedSegment}; @@ -687,6 +688,16 @@ where for archived_segment in archiver.add_block(encoded_block, block_object_mappings) { let segment_header = archived_segment.segment_header; + if let Err(error) = + segment_headers_store.add_segment_headers(slice::from_ref(&segment_header)) + { + error!( + target: "subspace", + "Failed to store segment headers: {error}" + ); + return; + } + send_archived_segment_notification( &archived_segment_notification_sender, archived_segment, @@ -697,14 +708,6 @@ where } if !new_segment_headers.is_empty() { - if let Err(error) = segment_headers_store.add_segment_headers(&new_segment_headers) - { - error!( - target: "subspace", - "Failed to store segment headers: {error}" - ); - return; - } let maybe_block_number_to_finalize = { let mut segment_headers = segment_headers.lock(); segment_headers.put(block_number + One::one(), new_segment_headers); From edb8b4dc1d55d0277f3af7e6513484c7531edb1c Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Mon, 31 Jul 2023 18:57:46 +0300 Subject: [PATCH 3/3] Better logging around sector expiration --- crates/sc-consensus-subspace/src/lib.rs | 10 ++++- .../src/single_disk_plot/plotting.rs | 40 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/crates/sc-consensus-subspace/src/lib.rs b/crates/sc-consensus-subspace/src/lib.rs index ffcbcfdb21..d064b116dc 100644 --- a/crates/sc-consensus-subspace/src/lib.rs +++ b/crates/sc-consensus-subspace/src/lib.rs @@ -228,7 +228,7 @@ pub enum Error { /// Stored segment header extrinsic was not found #[error("Stored segment header extrinsic was not found: {0:?}")] SegmentHeadersExtrinsicNotFound(Vec), - /// Duplicated segment commitment + /// Different segment commitment found #[error( "Different segment commitment for segment index {0} was found in storage, likely fork \ below archiving point" @@ -1161,6 +1161,14 @@ where .segment_commitment(); if &found_segment_commitment != segment_commitment { + warn!( + target: "subspace", + "Different segment commitment for segment index {} was found in storage, \ + likely fork below archiving point. expected {:?}, found {:?}", + segment_index, + segment_commitment, + found_segment_commitment + ); return Err(ConsensusError::ClientImport( Error::::DifferentSegmentCommitment(segment_index).to_string(), )); diff --git a/crates/subspace-farmer/src/single_disk_plot/plotting.rs b/crates/subspace-farmer/src/single_disk_plot/plotting.rs index 936dcd0ca3..e4faf79546 100644 --- a/crates/subspace-farmer/src/single_disk_plot/plotting.rs +++ b/crates/subspace-farmer/src/single_disk_plot/plotting.rs @@ -449,6 +449,10 @@ where while let Some(()) = archived_segments_receiver.next().await { let archived_segment_header = last_archived_segment.load(Ordering::SeqCst); + trace!( + segment_index = %archived_segment_header.segment_index(), + "New archived segment received", + ); // It is fine to take a synchronous read lock here because the only time // write lock is taken is during plotting, which we know doesn't happen @@ -460,11 +464,23 @@ where .collect_into(&mut sectors_to_check); for (sector_index, history_size) in sectors_to_check.drain(..) { if let Some(sector_expire_at) = sectors_expire_at.get(§or_index) { + trace!( + %sector_index, + %history_size, + %sector_expire_at, + "Checking sector for expiration" + ); // +1 means we will start replotting a bit before it actually expires to avoid // storing expired sectors if *sector_expire_at <= (archived_segment_header.segment_index() + SegmentIndex::ONE) { + debug!( + %sector_index, + %history_size, + %sector_expire_at, + "Sector expires soon #1, scheduling replotting" + ); // Time to replot sector_indices_to_replot.push(sector_index); } @@ -475,6 +491,12 @@ where .sector_expiration_check(min_sector_lifetime) .map(|expiration_check_history_size| expiration_check_history_size.segment_index()) { + trace!( + %sector_index, + %history_size, + %expiration_check_segment_index, + "Determined sector expiration check segment index" + ); let maybe_sector_expiration_check_segment_commitment = if let Some(segment_commitment) = archived_segment_commitments_cache.get(&expiration_check_segment_index) @@ -512,14 +534,32 @@ where metadata; qed", ); + trace!( + %sector_index, + %history_size, + sector_expire_at = %expiration_history_size.segment_index(), + "Determined sector expiration segment index" + ); // +1 means we will start replotting a bit before it actually expires to avoid // storing expired sectors if expiration_history_size.segment_index() <= (archived_segment_header.segment_index() + SegmentIndex::ONE) { + debug!( + %sector_index, + %history_size, + sector_expire_at = %expiration_history_size.segment_index(), + "Sector expires soon #2, scheduling replotting" + ); // Time to replot sector_indices_to_replot.push(sector_index); } else { + trace!( + %sector_index, + %history_size, + sector_expire_at = %expiration_history_size.segment_index(), + "Sector expires later, remembering sector expiration" + ); // Store expiration so we don't have to recalculate it later sectors_expire_at .insert(sector_index, expiration_history_size.segment_index());