From 451397902ef6b669098dee85dbe866b42490cb91 Mon Sep 17 00:00:00 2001 From: Volker Mische Date: Mon, 14 Sep 2020 16:22:52 +0200 Subject: [PATCH] fix: improve sector ID logging (#1280) Sector IDs are more often included in log messages/attached to errors. This PR is based on @IPFS-grandhelmsman work at #1261. Closes #1261. --- filecoin-proofs/src/api/post.rs | 95 ++++++++++++++------ filecoin-proofs/src/api/seal.rs | 16 ++-- storage-proofs/post/src/fallback/compound.rs | 2 +- storage-proofs/post/src/fallback/vanilla.rs | 23 ++--- 4 files changed, 90 insertions(+), 46 deletions(-) diff --git a/filecoin-proofs/src/api/post.rs b/filecoin-proofs/src/api/post.rs index 861d453b9..b98a5b6c0 100644 --- a/filecoin-proofs/src/api/post.rs +++ b/filecoin-proofs/src/api/post.rs @@ -127,12 +127,12 @@ impl PrivateReplicaInfo { as_safe_commitment(&self.comm_r, "comm_r") } - pub fn safe_comm_c(&self) -> Result<::Domain> { - Ok(self.aux.comm_c) + pub fn safe_comm_c(&self) -> ::Domain { + self.aux.comm_c } - pub fn safe_comm_r_last(&self) -> Result<::Domain> { - Ok(self.aux.comm_r_last) + pub fn safe_comm_r_last(&self) -> ::Domain { + self.aux.comm_r_last } /// Generate the merkle tree of this particular replica. @@ -355,20 +355,28 @@ pub fn generate_winning_post( let trees = replicas .iter() - .map(|(_, replica)| replica.merkle_tree(post_config.sector_size)) + .map(|(sector_id, replica)| { + replica + .merkle_tree(post_config.sector_size) + .with_context(|| { + format!("generate_winning_post: merkle_tree failed: {:?}", sector_id) + }) + }) .collect::>>()?; let mut pub_sectors = Vec::with_capacity(param_sector_count); let mut priv_sectors = Vec::with_capacity(param_sector_count); for _ in 0..param_sector_count { - for ((id, replica), tree) in replicas.iter().zip(trees.iter()) { - let comm_r = replica.safe_comm_r()?; - let comm_c = replica.safe_comm_c()?; - let comm_r_last = replica.safe_comm_r_last()?; + for ((sector_id, replica), tree) in replicas.iter().zip(trees.iter()) { + let comm_r = replica.safe_comm_r().with_context(|| { + format!("generate_winning_post: safe_comm_r failed: {:?}", sector_id) + })?; + let comm_c = replica.safe_comm_c(); + let comm_r_last = replica.safe_comm_r_last(); pub_sectors.push(fallback::PublicSector::<::Domain> { - id: *id, + id: *sector_id, comm_r, }); priv_sectors.push(fallback::PrivateSector { @@ -493,7 +501,7 @@ pub fn generate_fallback_sector_challenges( randomness_safe, u64::from(*sector), challenge_index, - )?; + ); challenges.push(challenged_leaf); } @@ -514,12 +522,24 @@ pub fn generate_single_vanilla_proof( replica: &PrivateReplicaInfo, challenges: &[u64], ) -> Result> { - info!("generate_single_vanilla_proof:start"); - - let tree = &replica.merkle_tree(post_config.sector_size)?; - let comm_r = replica.safe_comm_r()?; - let comm_c = replica.safe_comm_c()?; - let comm_r_last = replica.safe_comm_r_last()?; + info!("generate_single_vanilla_proof:start: {:?}", sector_id); + + let tree = &replica + .merkle_tree(post_config.sector_size) + .with_context(|| { + format!( + "generate_single_vanilla_proof: merkle_tree failed: {:?}", + sector_id + ) + })?; + let comm_r = replica.safe_comm_r().with_context(|| { + format!( + "generate_single_vanilla_poof: safe_comm_r failed: {:?}", + sector_id + ) + })?; + let comm_c = replica.safe_comm_c(); + let comm_r_last = replica.safe_comm_r_last(); // There is only enough information in the arguments to generate a // single vanilla proof, so the behaviour is unexpected if the @@ -542,9 +562,15 @@ pub fn generate_single_vanilla_proof( sectors: &priv_sectors, }; - let vanilla_proof = fallback::vanilla_proof(sector_id, &priv_inputs, challenges)?; + let vanilla_proof = + fallback::vanilla_proof(sector_id, &priv_inputs, challenges).with_context(|| { + format!( + "generate_single_vanilla_proof: vanilla_proof failed: {:?}", + sector_id + ) + })?; - info!("generate_single_vanilla_proof:finish"); + info!("generate_single_vanilla_proof:finish: {:?}", sector_id); Ok(FallbackPoStSectorProof { sector_id, @@ -594,9 +620,14 @@ pub fn verify_winning_post( let mut pub_sectors = Vec::with_capacity(param_sector_count); for _ in 0..param_sector_count { - for (id, replica) in replicas.iter() { - let comm_r = replica.safe_comm_r()?; - pub_sectors.push(fallback::PublicSector { id: *id, comm_r }); + for (sector_id, replica) in replicas.iter() { + let comm_r = replica.safe_comm_r().with_context(|| { + format!("verify_winning_post: safe_comm_r failed: {:?}", sector_id) + })?; + pub_sectors.push(fallback::PublicSector { + id: *sector_id, + comm_r, + }); } } @@ -881,16 +912,24 @@ pub fn generate_window_post( let trees: Vec<_> = replicas .iter() - .map(|(_id, replica)| replica.merkle_tree(post_config.sector_size)) + .map(|(sector_id, replica)| { + replica + .merkle_tree(post_config.sector_size) + .with_context(|| { + format!("generate_window_post: merkle_tree failed: {:?}", sector_id) + }) + }) .collect::>()?; let mut pub_sectors = Vec::with_capacity(sector_count); let mut priv_sectors = Vec::with_capacity(sector_count); for ((sector_id, replica), tree) in replicas.iter().zip(trees.iter()) { - let comm_r = replica.safe_comm_r()?; - let comm_c = replica.safe_comm_c()?; - let comm_r_last = replica.safe_comm_r_last()?; + let comm_r = replica.safe_comm_r().with_context(|| { + format!("generate_window_post: safe_comm_r failed: {:?}", sector_id) + })?; + let comm_c = replica.safe_comm_c(); + let comm_r_last = replica.safe_comm_r_last(); pub_sectors.push(fallback::PublicSector { id: *sector_id, @@ -958,7 +997,9 @@ pub fn verify_window_post( let pub_sectors: Vec<_> = replicas .iter() .map(|(sector_id, replica)| { - let comm_r = replica.safe_comm_r()?; + let comm_r = replica.safe_comm_r().with_context(|| { + format!("verify_window_post: safe_comm_r failed: {:?}", sector_id) + })?; Ok(fallback::PublicSector { id: *sector_id, comm_r, diff --git a/filecoin-proofs/src/api/seal.rs b/filecoin-proofs/src/api/seal.rs index 5a42b974b..9b17b68aa 100644 --- a/filecoin-proofs/src/api/seal.rs +++ b/filecoin-proofs/src/api/seal.rs @@ -57,7 +57,7 @@ where S: AsRef, T: AsRef, { - info!("seal_pre_commit_phase1:start"); + info!("seal_pre_commit_phase1:start: {:?}", sector_id); // Sanity check all input path types. ensure!( @@ -185,7 +185,7 @@ where comm_d, }; - info!("seal_pre_commit_phase1:finish"); + info!("seal_pre_commit_phase1:finish: {:?}", sector_id); Ok(out) } @@ -324,7 +324,7 @@ pub fn seal_commit_phase1, Tree: 'static + MerkleTreeTrait>( pre_commit: SealPreCommitOutput, piece_infos: &[PieceInfo], ) -> Result> { - info!("seal_commit_phase1:start"); + info!("seal_commit_phase1:start: {:?}", sector_id); // Sanity check all input path types. ensure!( @@ -435,7 +435,7 @@ pub fn seal_commit_phase1, Tree: 'static + MerkleTreeTrait>( ticket, }; - info!("seal_commit_phase1:finish"); + info!("seal_commit_phase1:finish: {:?}", sector_id); Ok(out) } @@ -446,7 +446,7 @@ pub fn seal_commit_phase2( prover_id: ProverId, sector_id: SectorId, ) -> Result { - info!("seal_commit_phase2:start"); + info!("seal_commit_phase2:start: {:?}", sector_id); let SealCommitPhase1Output { vanilla_proofs, @@ -529,7 +529,7 @@ pub fn seal_commit_phase2( let out = SealCommitOutput { proof: buf }; - info!("seal_commit_phase2:finish"); + info!("seal_commit_phase2:finish: {:?}", sector_id); Ok(out) } @@ -571,7 +571,7 @@ pub fn verify_seal( seed: Ticket, proof_vec: &[u8], ) -> Result { - info!("verify_seal:start"); + info!("verify_seal:start: {:?}", sector_id); ensure!(comm_d_in != [0; 32], "Invalid all zero commitment (comm_d)"); ensure!(comm_r_in != [0; 32], "Invalid all zero commitment (comm_r)"); @@ -661,7 +661,7 @@ pub fn verify_seal( ) }; - info!("verify_seal:finish"); + info!("verify_seal:finish: {:?}", sector_id); result } diff --git a/storage-proofs/post/src/fallback/compound.rs b/storage-proofs/post/src/fallback/compound.rs index f934f0962..4a404a78e 100644 --- a/storage-proofs/post/src/fallback/compound.rs +++ b/storage-proofs/post/src/fallback/compound.rs @@ -73,7 +73,7 @@ impl<'a, Tree: 'static + MerkleTreeTrait> pub_inputs.randomness, sector.id.into(), challenge_index, - )?; + ); let por_pub_inputs = por::PublicInputs { commitment: None, diff --git a/storage-proofs/post/src/fallback/vanilla.rs b/storage-proofs/post/src/fallback/vanilla.rs index 2988e9299..3810f48db 100644 --- a/storage-proofs/post/src/fallback/vanilla.rs +++ b/storage-proofs/post/src/fallback/vanilla.rs @@ -4,7 +4,7 @@ use std::marker::PhantomData; use anyhow::ensure; use byteorder::{ByteOrder, LittleEndian}; use generic_array::typenum::Unsigned; -use log::trace; +use log::{error, trace}; use paired::bls12_381::Fr; use rayon::prelude::*; use serde::{Deserialize, Serialize}; @@ -202,7 +202,7 @@ pub fn generate_leaf_challenges( randomness: T, sector_id: u64, challenge_count: usize, -) -> Result> { +) -> Vec { let mut challenges = Vec::with_capacity(challenge_count); for leaf_challenge_index in 0..challenge_count { @@ -211,11 +211,11 @@ pub fn generate_leaf_challenges( randomness, sector_id, leaf_challenge_index as u64, - )?; + ); challenges.push(challenge) } - Ok(challenges) + challenges } /// Generates challenge, such that the range fits into the sector. @@ -224,7 +224,7 @@ pub fn generate_leaf_challenge( randomness: T, sector_id: u64, leaf_challenge_index: u64, -) -> Result { +) -> u64 { let mut hasher = Sha256::new(); hasher.update(AsRef::<[u8]>::as_ref(&randomness)); hasher.update(§or_id.to_le_bytes()[..]); @@ -233,9 +233,7 @@ pub fn generate_leaf_challenge( let leaf_challenge = LittleEndian::read_u64(&hash[..8]); - let challenged_range_index = leaf_challenge % (pub_params.sector_size / NODE_SIZE as u64); - - Ok(challenged_range_index) + leaf_challenge % (pub_params.sector_size / NODE_SIZE as u64) } enum ProofOrFault { @@ -397,7 +395,7 @@ impl<'a, Tree: 'a + MerkleTreeTrait> ProofScheme<'a> for FallbackPoSt<'a, Tree> pub_inputs.randomness, sector_id.into(), challenge_index, - )?; + ); let proof = tree.gen_cached_proof( challenged_leaf_start as usize, @@ -423,6 +421,7 @@ impl<'a, Tree: 'a + MerkleTreeTrait> ProofScheme<'a> for FallbackPoSt<'a, Tree> inclusion_proofs.push(proof); } ProofOrFault::Fault(sector_id) => { + error!("faulty sector: {:?}", sector_id); faulty_sectors.insert(sector_id); } } @@ -507,6 +506,7 @@ impl<'a, Tree: 'a + MerkleTreeTrait> ProofScheme<'a> for FallbackPoSt<'a, Tree> &comm_r_last, )) != AsRef::<[u8]>::as_ref(comm_r) { + error!("comm_c != comm_r_last: {:?}", sector_id); return Ok(false); } @@ -525,10 +525,11 @@ impl<'a, Tree: 'a + MerkleTreeTrait> ProofScheme<'a> for FallbackPoSt<'a, Tree> pub_inputs.randomness, sector_id.into(), challenge_index, - )?; + ); // validate all comm_r_lasts match if inclusion_proof.root() != comm_r_last { + error!("inclusion proof root != comm_r_last: {:?}", sector_id); return Ok(false); } @@ -537,10 +538,12 @@ impl<'a, Tree: 'a + MerkleTreeTrait> ProofScheme<'a> for FallbackPoSt<'a, Tree> inclusion_proof.expected_len(pub_params.sector_size as usize / NODE_SIZE); if expected_path_length != inclusion_proof.path().len() { + error!("wrong path length: {:?}", sector_id); return Ok(false); } if !inclusion_proof.validate(challenged_leaf_start as usize) { + error!("invalid inclusion proof: {:?}", sector_id); return Ok(false); } }