Make auditing fallible in case it faces read error as read errors are not recoverable

nazar-pc · nazar-pc · commit 2af35233567f · 2024-01-29T06:25:02.000+02:00
diff --git a/crates/pallet-subspace/src/mock.rs b/crates/pallet-subspace/src/mock.rs
@@ -480,7 +480,8 @@ pub fn create_signed_vote(
             vote_solution_range,
             &plotted_sector_bytes,
             &plotted_sector.sector_metadata,
-        );
+        )
+        .unwrap();
 
         let Some(audit_result) = maybe_audit_result else {
             // Sector didn't have any solutions
diff --git a/crates/subspace-farmer-components/benches/auditing.rs b/crates/subspace-farmer-components/benches/auditing.rs
@@ -151,14 +151,17 @@ pub fn criterion_benchmark(c: &mut Criterion) {
     group.throughput(Throughput::Elements(1));
     group.bench_function("memory/sync", |b| {
         b.iter(|| async {
-            black_box(audit_plot_sync(
-                black_box(public_key),
-                black_box(global_challenge),
-                black_box(solution_range),
-                black_box(&plotted_sector_bytes),
-                black_box(slice::from_ref(&plotted_sector.sector_metadata)),
-                black_box(None),
-            ));
+            black_box(
+                audit_plot_sync(
+                    black_box(public_key),
+                    black_box(global_challenge),
+                    black_box(solution_range),
+                    black_box(&plotted_sector_bytes),
+                    black_box(slice::from_ref(&plotted_sector.sector_metadata)),
+                    black_box(None),
+                )
+                .unwrap(),
+            );
         })
     });
 
@@ -193,14 +196,17 @@ pub fn criterion_benchmark(c: &mut Criterion) {
         group.throughput(Throughput::Elements(sectors_count));
         group.bench_function("disk/sync", |b| {
             b.iter(|| {
-                black_box(audit_plot_sync(
-                    black_box(public_key),
-                    black_box(global_challenge),
-                    black_box(solution_range),
-                    black_box(&plot_file),
-                    black_box(&sectors_metadata),
-                    black_box(None),
-                ));
+                black_box(
+                    audit_plot_sync(
+                        black_box(public_key),
+                        black_box(global_challenge),
+                        black_box(solution_range),
+                        black_box(&plot_file),
+                        black_box(&sectors_metadata),
+                        black_box(None),
+                    )
+                    .unwrap(),
+                );
             });
         });
 
diff --git a/crates/subspace-farmer-components/benches/proving.rs b/crates/subspace-farmer-components/benches/proving.rs
@@ -166,7 +166,8 @@ pub fn criterion_benchmark(c: &mut Criterion) {
             &plotted_sector_bytes,
             slice::from_ref(&plotted_sector.sector_metadata),
             None,
-        );
+        )
+        .unwrap();
 
         let solution_candidates = match audit_results.into_iter().next() {
             Some(audit_result) => audit_result.solution_candidates,
@@ -249,7 +250,8 @@ pub fn criterion_benchmark(c: &mut Criterion) {
                 &plot_file,
                 &sectors_metadata,
                 None,
-            );
+            )
+            .unwrap();
             let solution_candidates = audit_results
                 .into_iter()
                 .map(|audit_result| audit_result.solution_candidates)
diff --git a/crates/subspace-farmer-components/src/auditing.rs b/crates/subspace-farmer-components/src/auditing.rs
@@ -2,12 +2,28 @@ use crate::proving::SolutionCandidates;
 use crate::sector::{sector_size, SectorContentsMap, SectorMetadataChecksummed};
 use crate::{ReadAtOffset, ReadAtSync};
 use rayon::prelude::*;
+use std::io;
 use subspace_core_primitives::crypto::Scalar;
 use subspace_core_primitives::{
     Blake3Hash, PublicKey, SBucket, SectorId, SectorIndex, SectorSlotChallenge, SolutionRange,
 };
 use subspace_verification::is_within_solution_range;
-use tracing::warn;
+use thiserror::Error;
+
+/// Errors that happen during proving
+#[derive(Debug, Error)]
+pub enum AuditingError {
+    /// Failed read s-bucket
+    #[error("Failed read s-bucket {s_bucket_audit_index} of sector {sector_index}: {error}")]
+    SBucketReading {
+        /// Sector index
+        sector_index: SectorIndex,
+        /// S-bucket audit index
+        s_bucket_audit_index: SBucket,
+        /// Low-level error
+        error: io::Error,
+    },
+}
 
 /// Result of sector audit
 #[derive(Debug, Clone)]
@@ -42,7 +58,7 @@ pub fn audit_sector_sync<'a, Sector>(
     solution_range: SolutionRange,
     sector: Sector,
     sector_metadata: &'a SectorMetadataChecksummed,
-) -> Option<AuditResult<'a, Sector>>
+) -> Result<Option<AuditResult<'a, Sector>>, AuditingError>
 where
     Sector: ReadAtSync + 'a,
 {
@@ -55,26 +71,24 @@ where
     } = collect_sector_auditing_details(public_key.hash(), global_challenge, sector_metadata);
 
     let mut s_bucket = vec![0; s_bucket_audit_size];
-    let read_s_bucket_result = sector.read_at(&mut s_bucket, s_bucket_audit_offset_in_sector);
-
-    if let Err(error) = read_s_bucket_result {
-        warn!(
-            %error,
-            sector_index = %sector_metadata.sector_index,
-            %s_bucket_audit_index,
-            "Failed read s-bucket",
-        );
-        return None;
-    }
+    sector
+        .read_at(&mut s_bucket, s_bucket_audit_offset_in_sector)
+        .map_err(|error| AuditingError::SBucketReading {
+            sector_index: sector_metadata.sector_index,
+            s_bucket_audit_index,
+            error,
+        })?;
 
-    let (winning_chunks, best_solution_distance) = map_winning_chunks(
+    let Some((winning_chunks, best_solution_distance)) = map_winning_chunks(
         &s_bucket,
         global_challenge,
         &sector_slot_challenge,
         solution_range,
-    )?;
+    ) else {
+        return Ok(None);
+    };
 
-    Some(AuditResult {
+    Ok(Some(AuditResult {
         sector_index: sector_metadata.sector_index,
         solution_candidates: SolutionCandidates::new(
             public_key,
@@ -85,7 +99,7 @@ where
             winning_chunks.into(),
         ),
         best_solution_distance,
-    })
+    }))
 }
 
 /// Audit the whole plot and generate streams of solutions
@@ -96,7 +110,7 @@ pub fn audit_plot_sync<'a, Plot>(
     plot: &'a Plot,
     sectors_metadata: &'a [SectorMetadataChecksummed],
     maybe_sector_being_modified: Option<SectorIndex>,
-) -> Vec<AuditResult<'a, ReadAtOffset<'a, Plot>>>
+) -> Result<Vec<AuditResult<'a, ReadAtOffset<'a, Plot>>>, AuditingError>
 where
     Plot: ReadAtSync + 'a,
 {
@@ -135,14 +149,11 @@ where
                 &mut s_bucket,
                 sector_auditing_info.s_bucket_audit_offset_in_sector,
             ) {
-                warn!(
-                    %error,
-                    sector_index = %sector_metadata.sector_index,
-                    s_bucket_audit_index = %sector_auditing_info.s_bucket_audit_index,
-                    "Failed read s-bucket",
-                );
-
-                return None;
+                return Some(Err(AuditingError::SBucketReading {
+                    sector_index: sector_metadata.sector_index,
+                    s_bucket_audit_index: sector_auditing_info.s_bucket_audit_index,
+                    error,
+                }));
             }
 
             let (winning_chunks, best_solution_distance) = map_winning_chunks(
@@ -152,7 +163,7 @@ where
                 solution_range,
             )?;
 
-            Some(AuditResult {
+            Some(Ok(AuditResult {
                 sector_index: sector_metadata.sector_index,
                 solution_candidates: SolutionCandidates::new(
                     public_key,
@@ -163,7 +174,7 @@ where
                     winning_chunks.into(),
                 ),
                 best_solution_distance,
-            })
+            }))
         })
         .collect()
 }
diff --git a/crates/subspace-farmer/src/bin/subspace-farmer/commands/benchmark.rs b/crates/subspace-farmer/src/bin/subspace-farmer/commands/benchmark.rs
@@ -250,7 +250,7 @@ fn prove(
                 table_generator: &table_generator,
             };
 
-            let mut audit_results = plot_audit.audit(options);
+            let mut audit_results = plot_audit.audit(options).unwrap();
 
             group.bench_function("plot/single", |b| {
                 b.iter_batched(
@@ -259,7 +259,7 @@ fn prove(
                             return result;
                         }
 
-                        audit_results = plot_audit.audit(options);
+                        audit_results = plot_audit.audit(options).unwrap();
 
                         audit_results.pop().unwrap()
                     },
@@ -293,7 +293,7 @@ fn prove(
                 maybe_sector_being_modified: None,
                 table_generator: &table_generator,
             };
-            let mut audit_results = plot_audit.audit(options);
+            let mut audit_results = plot_audit.audit(options).unwrap();
 
             group.bench_function("plot/rayon", |b| {
                 b.iter_batched(
@@ -302,7 +302,7 @@ fn prove(
                             return result;
                         }
 
-                        audit_results = plot_audit.audit(options);
+                        audit_results = plot_audit.audit(options).unwrap();
 
                         audit_results.pop().unwrap()
                     },
diff --git a/crates/subspace-farmer/src/single_disk_farm/farming.rs b/crates/subspace-farmer/src/single_disk_farm/farming.rs
@@ -15,10 +15,10 @@ use std::{fmt, io};
 use subspace_core_primitives::crypto::kzg::Kzg;
 use subspace_core_primitives::{PosSeed, PublicKey, SectorIndex, Solution, SolutionRange};
 use subspace_erasure_coding::ErasureCoding;
-use subspace_farmer_components::auditing::audit_plot_sync;
+use subspace_farmer_components::auditing::{audit_plot_sync, AuditingError};
 use subspace_farmer_components::proving::{ProvableSolutions, ProvingError};
 use subspace_farmer_components::sector::SectorMetadataChecksummed;
-use subspace_farmer_components::{proving, ReadAtSync};
+use subspace_farmer_components::ReadAtSync;
 use subspace_proof_of_space::{Table, TableGenerator};
 use subspace_rpc_primitives::{SlotInfo, SolutionResponse};
 use thiserror::Error;
@@ -88,9 +88,12 @@ pub enum FarmingError {
         /// Lower-level error
         error: node_client::Error,
     },
+    /// Low-level auditing error
+    #[error("Low-level auditing error: {0}")]
+    LowLevelAuditing(#[from] AuditingError),
     /// Low-level proving error
     #[error("Low-level proving error: {0}")]
-    LowLevelProving(#[from] proving::ProvingError),
+    LowLevelProving(#[from] ProvingError),
     /// I/O error occurred
     #[error("I/O error: {0}")]
     Io(#[from] io::Error),
@@ -181,10 +184,13 @@ where
     pub fn audit<PosTable>(
         &'a self,
         options: PlotAuditOptions<'a, PosTable>,
-    ) -> Vec<(
-        SectorIndex,
-        impl ProvableSolutions<Item = Result<Solution<PublicKey, PublicKey>, ProvingError>> + 'a,
-    )>
+    ) -> Result<
+        Vec<(
+            SectorIndex,
+            impl ProvableSolutions<Item = Result<Solution<PublicKey, PublicKey>, ProvingError>> + 'a,
+        )>,
+        AuditingError,
+    >
     where
         PosTable: Table,
     {
@@ -206,9 +212,9 @@ where
             &self.0,
             sectors_metadata,
             maybe_sector_being_modified,
-        );
+        )?;
 
-        audit_results
+        Ok(audit_results
             .into_iter()
             .filter_map(|audit_results| {
                 let sector_index = audit_results.sector_index;
@@ -239,7 +245,7 @@ where
 
                 Some((sector_index, sector_solutions))
             })
-            .collect()
+            .collect())
     }
 }
 
@@ -311,7 +317,7 @@ where
                 erasure_coding: &erasure_coding,
                 maybe_sector_being_modified,
                 table_generator: &table_generator,
-            });
+            })?;
 
             sectors_solutions
         };
diff --git a/test/subspace-test-client/src/lib.rs b/test/subspace-test-client/src/lib.rs
@@ -172,6 +172,7 @@ async fn start_farming<PosTable, Client>(
             );
 
             let solution = audit_result
+                .unwrap()
                 .unwrap()
                 .solution_candidates
                 .into_solutions(&public_key, &kzg, &erasure_coding, |seed: &PosSeed| {
