From 73e2d28dc0405a2ba92eaeab99bdf3c9613a2027 Mon Sep 17 00:00:00 2001 From: wacban Date: Tue, 5 Sep 2023 07:46:59 +0000 Subject: [PATCH] code review --- chain/chain/src/store.rs | 78 ++++++++++++--------- tools/state-viewer/src/apply_chain_range.rs | 1 - 2 files changed, 44 insertions(+), 35 deletions(-) diff --git a/chain/chain/src/store.rs b/chain/chain/src/store.rs index f5a60fe9728..586c08aee83 100644 --- a/chain/chain/src/store.rs +++ b/chain/chain/src/store.rs @@ -222,15 +222,16 @@ pub trait ChainStoreAccess { last_chunk_height_included: BlockHeight, ) -> Result, Error> { let _span = - tracing::debug_span!(target: "store", "get_incoming_receipts_for_shard").entered(); + tracing::debug_span!(target: "chain", "get_incoming_receipts_for_shard", ?shard_id, ?block_hash, last_chunk_height_included).entered(); let mut ret = vec![]; - let mut header = self.get_block_header(&block_hash)?; let target_shard_id = shard_id; let target_shard_layout = epoch_manager.get_shard_layout_from_prev_block(&block_hash)?; loop { + let header = self.get_block_header(&block_hash)?; + if header.height() < last_chunk_height_included { panic!("get_incoming_receipts_for_shard failed"); } @@ -246,7 +247,7 @@ pub trait ChainStoreAccess { if shard_layout != prev_shard_layout { let parent_shard_id = shard_layout.get_parent_shard_id(shard_id)?; tracing::debug!( - target: "store", + target: "chain", version = shard_layout.version(), prev_version = prev_shard_layout.version(), shard_id, @@ -260,44 +261,24 @@ pub trait ChainStoreAccess { match receipts_proofs { Ok(receipt_proofs) => { tracing::debug!( - target: "store", - ?shard_id, - ?last_chunk_height_included, - ?block_hash, + target: "chain", "found receipts from block with missing chunks", ); - // If the shard layout changed we need to filter to make - // sure we only include receipts where receiver belongs to - // the target shard id in the target shard layout. - let mut filtered_receipt_proofs = vec![]; - for receipt_proof in receipt_proofs.iter() { - let mut filtered_receipts = vec![]; - let ReceiptProof(receipts, shard_proof) = receipt_proof.clone(); - for receipt in receipts { - let receiver_shard_id = - account_id_to_shard_id(&receipt.receiver_id, &target_shard_layout); - if receiver_shard_id == target_shard_id { - tracing::trace!(target:"store", receipt_id=?receipt.receipt_id, "including receipt"); - filtered_receipts.push(receipt); - } else { - tracing::trace!(target:"store", receipt_id=?receipt.receipt_id, "excluding receipt"); - } - } - // TODO(resharding) adjust the shard proof accordingly - // currently this only matters for state sync - let receipt_proof = ReceiptProof(filtered_receipts, shard_proof); - filtered_receipt_proofs.push(receipt_proof); - } + // If the shard layout changed we need to filter receipts to + // make sure we only include receipts where receiver belongs + // to the target shard id in the target shard layout. + let filtered_receipt_proofs = filter_incoming_receipts_for_shard( + &target_shard_layout, + target_shard_id, + receipt_proofs, + ); ret.push(ReceiptProofResponse(block_hash, filtered_receipt_proofs.into())); } Err(err) => { tracing::debug!( - target: "store", - ?shard_id, - ?last_chunk_height_included, - ?block_hash, + target: "chain", ?err, "could not find receipts from block with missing chunks" ); @@ -312,7 +293,6 @@ pub trait ChainStoreAccess { } block_hash = *prev_hash; - header = self.get_block_header(&block_hash)?; } Ok(ret) @@ -391,6 +371,36 @@ pub trait ChainStoreAccess { } } +/// Given a vector of receipts return only the receipts that should be assigned +/// to the target shard id in the target shard layout. Used when collecting the +/// incoming receipts and the shard layout changed. +fn filter_incoming_receipts_for_shard( + target_shard_layout: &ShardLayout, + target_shard_id: u64, + receipt_proofs: Arc>, +) -> Vec { + let mut filtered_receipt_proofs = vec![]; + for receipt_proof in receipt_proofs.iter() { + let mut filtered_receipts = vec![]; + let ReceiptProof(receipts, shard_proof) = receipt_proof.clone(); + for receipt in receipts { + let receiver_shard_id = + account_id_to_shard_id(&receipt.receiver_id, target_shard_layout); + if receiver_shard_id == target_shard_id { + tracing::trace!(target: "chain", receipt_id=?receipt.receipt_id, "including receipt"); + filtered_receipts.push(receipt); + } else { + tracing::trace!(target: "chain", receipt_id=?receipt.receipt_id, "excluding receipt"); + } + } + // TODO(resharding) adjust the shard proof accordingly + // currently this only matters for state sync + let receipt_proof = ReceiptProof(filtered_receipts, shard_proof); + filtered_receipt_proofs.push(receipt_proof); + } + filtered_receipt_proofs +} + /// All chain-related database operations. pub struct ChainStore { store: Store, diff --git a/tools/state-viewer/src/apply_chain_range.rs b/tools/state-viewer/src/apply_chain_range.rs index fa6444bffdb..0b217af1ca5 100644 --- a/tools/state-viewer/src/apply_chain_range.rs +++ b/tools/state-viewer/src/apply_chain_range.rs @@ -192,7 +192,6 @@ fn apply_block_from_range( } }; - // TODO(wacban) OK, has epoch manager handy let chain_store_update = ChainStoreUpdate::new(&mut chain_store); let receipt_proof_response = chain_store_update .get_incoming_receipts_for_shard(