From 2f92af446a45fe277e57c85e397aa840398e3ec2 Mon Sep 17 00:00:00 2001 From: linning Date: Sat, 7 Oct 2023 03:25:09 +0800 Subject: [PATCH] Revamp bad_receipts_to_delete Signed-off-by: linning --- Cargo.lock | 1 + .../client/domain-operator/src/aux_schema.rs | 20 ++++--- .../src/domain_block_processor.rs | 52 +++++++++---------- .../domain-operator/src/parent_chain.rs | 14 ++++- 4 files changed, 52 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fe8d5338f82..d52876791b5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10849,6 +10849,7 @@ dependencies = [ "frame-support", "hash-db 0.16.0", "hexlit", + "memory-db", "num-traits", "parity-scale-codec", "rand 0.8.5", diff --git a/domains/client/domain-operator/src/aux_schema.rs b/domains/client/domain-operator/src/aux_schema.rs index e972c9fc778..9f40b73b615 100644 --- a/domains/client/domain-operator/src/aux_schema.rs +++ b/domains/client/domain-operator/src/aux_schema.rs @@ -334,11 +334,15 @@ where ) } -pub(super) fn delete_bad_receipt( +pub(super) fn delete_bad_receipt( backend: &Backend, - block_number: BlockNumber, + block_number: NumberFor, bad_receipt_hash: H256, -) -> Result<(), ClientError> { +) -> Result<(), ClientError> +where + Backend: AuxStore, + CBlock: BlockT, +{ let bad_receipt_hashes_key = (BAD_RECEIPT_HASHES, block_number).encode(); let mut hashes_at_block_number: Vec = load_decode(backend, bad_receipt_hashes_key.as_slice())?.unwrap_or_default(); @@ -359,7 +363,7 @@ pub(super) fn delete_bad_receipt( let to_insert = if hashes_at_block_number.is_empty() { keys_to_delete.push(bad_receipt_hashes_key); - let mut bad_receipt_numbers: Vec = + let mut bad_receipt_numbers: Vec> = load_decode(backend, BAD_RECEIPT_NUMBERS.encode().as_slice())?.ok_or_else(|| { ClientError::Backend("Stored bad receipt numbers must exist".into()) })?; @@ -836,7 +840,7 @@ mod tests { ); assert_eq!(bad_receipts_at(20).unwrap(), [bad_receipt_hash4].into()); - assert!(delete_bad_receipt(&client, 10, bad_receipt_hash1).is_ok()); + assert!(delete_bad_receipt::<_, CBlock>(&client, 10, bad_receipt_hash1).is_ok()); assert_eq!(bad_receipt_numbers(), Some(vec![10, 20])); assert!(trace_mismatch_info_for(bad_receipt_hash1).is_none()); assert_eq!( @@ -844,12 +848,12 @@ mod tests { [bad_receipt_hash2, bad_receipt_hash3].into() ); - assert!(delete_bad_receipt(&client, 10, bad_receipt_hash2).is_ok()); + assert!(delete_bad_receipt::<_, CBlock>(&client, 10, bad_receipt_hash2).is_ok()); assert_eq!(bad_receipt_numbers(), Some(vec![10, 20])); assert!(trace_mismatch_info_for(bad_receipt_hash2).is_none()); assert_eq!(bad_receipts_at(10).unwrap(), [bad_receipt_hash3].into()); - assert!(delete_bad_receipt(&client, 10, bad_receipt_hash3).is_ok()); + assert!(delete_bad_receipt::<_, CBlock>(&client, 10, bad_receipt_hash3).is_ok()); assert_eq!(bad_receipt_numbers(), Some(vec![20])); assert!(trace_mismatch_info_for(bad_receipt_hash3).is_none()); assert!(bad_receipts_at(10).is_none()); @@ -858,7 +862,7 @@ mod tests { Some((bad_receipt_hash4, (1, block_hash4).into())) ); - assert!(delete_bad_receipt(&client, 20, bad_receipt_hash4).is_ok()); + assert!(delete_bad_receipt::<_, CBlock>(&client, 20, bad_receipt_hash4).is_ok()); assert_eq!(first_unconfirmed_bad_receipt_info(20), None); let (bad_receipt_hash5, block_hash5) = ( diff --git a/domains/client/domain-operator/src/domain_block_processor.rs b/domains/client/domain-operator/src/domain_block_processor.rs index e5bf2f4ebdc..17a9c5076bc 100644 --- a/domains/client/domain-operator/src/domain_block_processor.rs +++ b/domains/client/domain-operator/src/domain_block_processor.rs @@ -705,7 +705,7 @@ where .parent_chain .extract_fraud_proofs(parent_chain_block_hash, extrinsics)?; - self.check_receipts(receipts, fraud_proofs)?; + self.check_receipts(parent_chain_block_hash, receipts, fraud_proofs)?; Ok(()) } @@ -736,6 +736,7 @@ where fn check_receipts( &self, + parent_chain_block_hash: ParentChainBlock::Hash, receipts: Vec>, fraud_proofs: Vec, ParentChainBlock::Hash>>, ) -> Result<(), sp_blockchain::Error> { @@ -810,31 +811,30 @@ where } } - let bad_receipts_to_delete = fraud_proofs - .into_iter() - .filter_map(|fraud_proof| { - match fraud_proof { - FraudProof::InvalidStateTransition(fraud_proof) => { - let bad_receipt_number = todo!(); - let bad_receipt_hash = fraud_proof.bad_receipt_hash; - - // In order to not delete a receipt which was just inserted, accumulate the write&delete operations - // in case the bad receipt and corresponding fraud proof are included in the same block. - if let Some(index) = bad_receipts_to_write - .iter() - .map(|(_, receipt_hash, _)| receipt_hash) - .position(|v| *v == bad_receipt_hash) - { - bad_receipts_to_write.swap_remove(index); - None - } else { - Some((bad_receipt_number, bad_receipt_hash)) - } - } - _ => None, + // Use the `parent_chain_parent_hash` to get the `bad_receipt` because fraud proof already pruned the bad + // receipt in `parent_chain_block_hash` + let parent_chain_parent_hash = self.parent_chain.parent_hash(parent_chain_block_hash)?; + let mut bad_receipts_to_delete = vec![]; + for fraud_proof in fraud_proofs { + let bad_receipt_hash = fraud_proof.bad_receipt_hash(); + if let Some(bad_receipt) = self + .parent_chain + .execution_receipt(parent_chain_parent_hash, bad_receipt_hash)? + { + // In order to not delete a receipt which was just inserted, accumulate the write&delete operations + // in case the bad receipt and corresponding fraud proof are included in the same block. + if let Some(index) = bad_receipts_to_write + .iter() + .map(|(_, receipt_hash, _)| receipt_hash) + .position(|v| *v == bad_receipt_hash) + { + bad_receipts_to_write.swap_remove(index); + } else { + bad_receipts_to_delete + .push((bad_receipt.consensus_block_number, bad_receipt_hash)); } - }) - .collect::>(); + } + } for (bad_receipt_number, bad_receipt_hash, mismatch_info) in bad_receipts_to_write { crate::aux_schema::write_bad_receipt::<_, ParentChainBlock>( @@ -846,7 +846,7 @@ where } for (bad_receipt_number, bad_receipt_hash) in bad_receipts_to_delete { - if let Err(e) = crate::aux_schema::delete_bad_receipt( + if let Err(e) = crate::aux_schema::delete_bad_receipt::<_, ParentChainBlock>( &*self.client, bad_receipt_number, bad_receipt_hash, diff --git a/domains/client/domain-operator/src/parent_chain.rs b/domains/client/domain-operator/src/parent_chain.rs index f16d73c6371..351574538c9 100644 --- a/domains/client/domain-operator/src/parent_chain.rs +++ b/domains/client/domain-operator/src/parent_chain.rs @@ -1,6 +1,6 @@ use crate::ExecutionReceiptFor; use sc_client_api::BlockBackend; -use sp_api::{ApiError, NumberFor, ProvideRuntimeApi}; +use sp_api::{ApiError, HeaderT, NumberFor, ProvideRuntimeApi}; use sp_blockchain::HeaderBackend; use sp_domains::fraud_proof::FraudProof; use sp_domains::{DomainBlockLimit, DomainId, DomainsApi, ReceiptHash}; @@ -16,6 +16,11 @@ type FraudProofFor = pub trait ParentChainInterface { fn best_hash(&self) -> ParentChainBlock::Hash; + fn parent_hash( + &self, + hash: ParentChainBlock::Hash, + ) -> sp_blockchain::Result; + fn block_body( &self, at: ParentChainBlock::Hash, @@ -111,6 +116,13 @@ where self.consensus_client.info().best_hash } + fn parent_hash(&self, at: CBlock::Hash) -> sp_blockchain::Result { + let header = self.consensus_client.header(at)?.ok_or_else(|| { + sp_blockchain::Error::Backend(format!("Consensus block body for {at} not found")) + })?; + Ok(*header.parent_hash()) + } + fn block_body(&self, at: CBlock::Hash) -> sp_blockchain::Result> { self.consensus_client.block_body(at)?.ok_or_else(|| { sp_blockchain::Error::Backend(format!("Consensus block body for {at} not found"))