Skip to content

Commit

Permalink
Revamp bad_receipts_to_delete
Browse files Browse the repository at this point in the history
Signed-off-by: linning <linningde25@gmail.com>
  • Loading branch information
NingLin-P committed Oct 9, 2023
1 parent 71677a3 commit c500f7d
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 35 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 12 additions & 8 deletions domains/client/domain-operator/src/aux_schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,11 +334,15 @@ where
)
}

pub(super) fn delete_bad_receipt<Backend: AuxStore>(
pub(super) fn delete_bad_receipt<Backend, CBlock>(
backend: &Backend,
block_number: BlockNumber,
block_number: NumberFor<CBlock>,
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<H256> =
load_decode(backend, bad_receipt_hashes_key.as_slice())?.unwrap_or_default();
Expand All @@ -359,7 +363,7 @@ pub(super) fn delete_bad_receipt<Backend: AuxStore>(
let to_insert = if hashes_at_block_number.is_empty() {
keys_to_delete.push(bad_receipt_hashes_key);

let mut bad_receipt_numbers: Vec<BlockNumber> =
let mut bad_receipt_numbers: Vec<NumberFor<CBlock>> =
load_decode(backend, BAD_RECEIPT_NUMBERS.encode().as_slice())?.ok_or_else(|| {
ClientError::Backend("Stored bad receipt numbers must exist".into())
})?;
Expand Down Expand Up @@ -836,20 +840,20 @@ 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!(
bad_receipts_at(10).unwrap(),
[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());
Expand All @@ -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) = (
Expand Down
52 changes: 26 additions & 26 deletions domains/client/domain-operator/src/domain_block_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Expand Down Expand Up @@ -736,6 +736,7 @@ where

fn check_receipts(
&self,
parent_chain_block_hash: ParentChainBlock::Hash,
receipts: Vec<ExecutionReceiptFor<Block, ParentChainBlock>>,
fraud_proofs: Vec<FraudProof<NumberFor<ParentChainBlock>, ParentChainBlock::Hash>>,
) -> Result<(), sp_blockchain::Error> {
Expand Down Expand Up @@ -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::<Vec<_>>();
}
}

for (bad_receipt_number, bad_receipt_hash, mismatch_info) in bad_receipts_to_write {
crate::aux_schema::write_bad_receipt::<_, ParentChainBlock>(
Expand All @@ -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,
Expand Down
14 changes: 13 additions & 1 deletion domains/client/domain-operator/src/parent_chain.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -16,6 +16,11 @@ type FraudProofFor<ParentChainBlock> =
pub trait ParentChainInterface<Block: BlockT, ParentChainBlock: BlockT> {
fn best_hash(&self) -> ParentChainBlock::Hash;

fn parent_hash(
&self,
hash: ParentChainBlock::Hash,
) -> sp_blockchain::Result<ParentChainBlock::Hash>;

fn block_body(
&self,
at: ParentChainBlock::Hash,
Expand Down Expand Up @@ -111,6 +116,13 @@ where
self.consensus_client.info().best_hash
}

fn parent_hash(&self, at: CBlock::Hash) -> sp_blockchain::Result<CBlock::Hash> {
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<Vec<CBlock::Extrinsic>> {
self.consensus_client.block_body(at)?.ok_or_else(|| {
sp_blockchain::Error::Backend(format!("Consensus block body for {at} not found"))
Expand Down

0 comments on commit c500f7d

Please sign in to comment.