Skip to content

Commit

Permalink
Merge pull request #2787 from subspace/invalid-bundle-absent-tx-fp
Browse files Browse the repository at this point in the history 
Proper handling of bad invalid bundle ER that point to non-exist extrinsic
  • Loading branch information
@NingLin-P
NingLin-P authored May 28, 2024
2 parents 2dea8bc + 288ea3e commit 47ccb63
Show file tree
Hide file tree
Showing 4 changed files with 278 additions and 123 deletions.
41 changes: 30 additions & 11 deletions crates/sp-domains-fraud-proof/src/verification.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -535,20 +535,42 @@ where
} = invalid_bundles_fraud_proof;
let (bundle_index, is_true_invalid_fraud_proof) = (*bundle_index, *is_true_invalid_fraud_proof);

let invalid_bundle_entry = check_expected_bundle_entry::<CBlock, DomainHeader, Balance>(
let targeted_invalid_bundle_entry = check_expected_bundle_entry::<CBlock, DomainHeader, Balance>(
&bad_receipt,
bundle_index,
invalid_bundle_type.clone(),
is_true_invalid_fraud_proof,
)?;
let bundle_extrinsic_root = targeted_invalid_bundle_entry.extrinsics_root;

// Verify the bundle proof so in following we can use the bundle dirctly
match proof_data {
InvalidBundlesProofData::Bundle(bundle_with_proof)
| InvalidBundlesProofData::BundleAndExecution {
bundle_with_proof, ..
} => {
if bundle_with_proof.bundle_index != bundle_index {
return Err(VerificationError::UnexpectedInvalidBundleProofData);
}
bundle_with_proof.verify::<CBlock, SKP>(domain_id, &state_root)?;
}
InvalidBundlesProofData::Extrinsic(_) => {}
}

// Fast path to check if the fraud proof is targetting a bad receipt that claim a non-exist extrinsic
// is invalid
if let Some(invalid_extrinsic_index) = targeted_invalid_bundle_entry.invalid_extrinsic_index() {
if let InvalidBundlesProofData::Bundle(bundle_with_proof) = proof_data {
if bundle_with_proof.bundle.extrinsics.len() as u32 <= invalid_extrinsic_index {
return Ok(());
}
}
}

match &invalid_bundle_type {
InvalidBundleType::OutOfRangeTx(extrinsic_index) => {
let bundle = match proof_data {
InvalidBundlesProofData::Bundle(bundle_with_proof)
if bundle_with_proof.bundle_index == bundle_index =>
{
bundle_with_proof.verify::<CBlock, SKP>(domain_id, &state_root)?;
InvalidBundlesProofData::Bundle(bundle_with_proof) => {
bundle_with_proof.bundle.clone()
}
_ => return Err(VerificationError::UnexpectedInvalidBundleProofData),
Expand Down Expand Up @@ -590,7 +612,7 @@ where
};
get_extrinsic_from_proof::<DomainHeader>(
*extrinsic_index,
invalid_bundle_entry.extrinsics_root,
bundle_extrinsic_root,
extrinsic_storage_proof,
)?
};
Expand All @@ -614,10 +636,7 @@ where
InvalidBundlesProofData::BundleAndExecution {
bundle_with_proof,
execution_proof,
} if bundle_with_proof.bundle_index == bundle_index => {
bundle_with_proof.verify::<CBlock, SKP>(domain_id, &state_root)?;
(bundle_with_proof.bundle.clone(), execution_proof.clone())
}
} => (bundle_with_proof.bundle.clone(), execution_proof.clone()),
_ => return Err(VerificationError::UnexpectedInvalidBundleProofData),
};

Expand Down Expand Up @@ -660,7 +679,7 @@ where
};
get_extrinsic_from_proof::<DomainHeader>(
*extrinsic_index,
invalid_bundle_entry.extrinsics_root,
bundle_extrinsic_root,
extrinsic_storage_proof,
)?
};
Expand Down
9 changes: 9 additions & 0 deletions crates/sp-domains/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,15 @@ impl<Hash> InboxedBundle<Hash> {
matches!(self.bundle, BundleValidity::Invalid(_))
}

pub fn invalid_extrinsic_index(&self) -> Option<u32> {
match &self.bundle {
BundleValidity::Invalid(invalid_bundle_type) => {
Some(invalid_bundle_type.extrinsic_index())
}
BundleValidity::Valid(_) => None,
}
}

#[cfg(any(feature = "std", feature = "runtime-benchmarks"))]
pub fn dummy(extrinsics_root: Hash) -> Self
where
Expand Down
235 changes: 125 additions & 110 deletions domains/client/domain-operator/src/fraud_proof.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -553,125 +553,140 @@ where
};

let extrinsic_index = invalid_type.extrinsic_index();
let encoded_extrinsics: Vec<_> = bundle.extrinsics.iter().map(Encode::encode).collect();
let proof_data = match invalid_type {
InvalidBundleType::IllegalTx(expected_extrinsic_index) => {
if expected_extrinsic_index as usize >= bundle.extrinsics.len() {
return Err(FraudProofError::OutOfBoundsExtrinsicIndex {
index: expected_extrinsic_index as usize,
max: bundle.extrinsics.len(),
});
}
let proof_data = if bundle.extrinsics.len() as u32 <= extrinsic_index {
// The bad receipt claims a non-exist extrinsic is invalid, in this case, generate a
// `bundle_with_proof` as proof data is enough
let bundle_with_proof = OpaqueBundleWithProof::generate(
&self.storage_key_provider,
self.consensus_client.as_ref(),
domain_id,
consensus_block_hash,
bundle,
bundle_index,
)?;
InvalidBundlesProofData::Bundle(bundle_with_proof)
} else {
match invalid_type {
InvalidBundleType::IllegalTx(expected_extrinsic_index) => {
if expected_extrinsic_index as usize >= bundle.extrinsics.len() {
return Err(FraudProofError::OutOfBoundsExtrinsicIndex {
index: expected_extrinsic_index as usize,
max: bundle.extrinsics.len(),
});
}

let domain_block_parent_hash = *self
.client
.header(local_receipt.domain_block_hash)?
.ok_or_else(|| {
FraudProofError::Blockchain(sp_blockchain::Error::MissingHeader(format!(
"{:?}",
local_receipt.domain_block_hash
)))
})?
.parent_hash();

let domain_block_parent_number = self
.client
.block_number_from_id(&BlockId::Hash(domain_block_parent_hash))?
.ok_or_else(|| {
FraudProofError::Blockchain(sp_blockchain::Error::Backend(format!(
"unable to get block number for domain block:{:?}",
domain_block_parent_hash
)))
})?;
let domain_block_parent_hash = *self
.client
.header(local_receipt.domain_block_hash)?
.ok_or_else(|| {
FraudProofError::Blockchain(sp_blockchain::Error::MissingHeader(
format!("{:?}", local_receipt.domain_block_hash),
))
})?
.parent_hash();

let domain_block_parent_number = self
.client
.block_number_from_id(&BlockId::Hash(domain_block_parent_hash))?
.ok_or_else(|| {
FraudProofError::Blockchain(sp_blockchain::Error::Backend(format!(
"unable to get block number for domain block:{:?}",
domain_block_parent_hash
)))
})?;

let mut runtime_api_instance = self.client.runtime_api();
runtime_api_instance.record_proof();
let proof_recorder = runtime_api_instance
.proof_recorder()
.expect("we enabled proof recording just above; qed");

let mut block_extrinsics = vec![];
for (extrinsic_index, extrinsic) in bundle
.extrinsics
.iter()
.enumerate()
.take((expected_extrinsic_index + 1) as usize)
{
let encoded_extrinsic = extrinsic.encode();
block_extrinsics.push(
<Block as BlockT>::Extrinsic::decode(&mut encoded_extrinsic.as_slice())
.map_err(|decoding_error| {
FraudProofError::UnableToDecodeOpaqueBundleExtrinsic {
extrinsic_index,
decoding_error,
}
})?,
);
}

let mut runtime_api_instance = self.client.runtime_api();
runtime_api_instance.record_proof();
let proof_recorder = runtime_api_instance
.proof_recorder()
.expect("we enabled proof recording just above; qed");

let mut block_extrinsics = vec![];
for (extrinsic_index, extrinsic) in bundle
.extrinsics
.iter()
.enumerate()
.take((expected_extrinsic_index + 1) as usize)
{
let encoded_extrinsic = extrinsic.encode();
block_extrinsics.push(
<Block as BlockT>::Extrinsic::decode(&mut encoded_extrinsic.as_slice())
.map_err(|decoding_error| {
FraudProofError::UnableToDecodeOpaqueBundleExtrinsic {
extrinsic_index,
decoding_error,
}
})?,
);
}
let validation_response = runtime_api_instance
.check_extrinsics_and_do_pre_dispatch(
domain_block_parent_hash,
block_extrinsics,
domain_block_parent_number,
domain_block_parent_hash,
)?;

// If the proof is true invalid then validation response should not be Ok.
// If the proof is false invalid then validation response should not be Err.
// OR
// If it is true invalid and expected extrinsic index does not match
if (is_true_invalid == validation_response.is_ok())
|| (is_true_invalid
&& validation_response
.as_ref()
.is_err_and(|e| e.extrinsic_index != expected_extrinsic_index))
{
return Err(FraudProofError::InvalidIllegalTxFraudProofExtrinsicIndex {
index: expected_extrinsic_index as usize,
is_true_invalid,
extrinsics_validity_response: validation_response,
});
}

let validation_response = runtime_api_instance
.check_extrinsics_and_do_pre_dispatch(
domain_block_parent_hash,
block_extrinsics,
domain_block_parent_number,
domain_block_parent_hash,
let execution_proof = proof_recorder.drain_storage_proof();

let bundle_with_proof = OpaqueBundleWithProof::generate(
&self.storage_key_provider,
self.consensus_client.as_ref(),
domain_id,
consensus_block_hash,
bundle,
bundle_index,
)?;

// If the proof is true invalid then validation response should not be Ok.
// If the proof is false invalid then validation response should not be Err.
// OR
// If it is true invalid and expected extrinsic index does not match
if (is_true_invalid == validation_response.is_ok())
|| (is_true_invalid
&& validation_response
.as_ref()
.is_err_and(|e| e.extrinsic_index != expected_extrinsic_index))
{
return Err(FraudProofError::InvalidIllegalTxFraudProofExtrinsicIndex {
index: expected_extrinsic_index as usize,
is_true_invalid,
extrinsics_validity_response: validation_response,
});
InvalidBundlesProofData::BundleAndExecution {
bundle_with_proof,
execution_proof,
}
}
InvalidBundleType::OutOfRangeTx(_) => {
let bundle_with_proof = OpaqueBundleWithProof::generate(
&self.storage_key_provider,
self.consensus_client.as_ref(),
domain_id,
consensus_block_hash,
bundle,
bundle_index,
)?;

let execution_proof = proof_recorder.drain_storage_proof();

let bundle_with_proof = OpaqueBundleWithProof::generate(
&self.storage_key_provider,
self.consensus_client.as_ref(),
domain_id,
consensus_block_hash,
bundle,
bundle_index,
)?;

InvalidBundlesProofData::BundleAndExecution {
bundle_with_proof,
execution_proof,
InvalidBundlesProofData::Bundle(bundle_with_proof)
}
}
InvalidBundleType::OutOfRangeTx(_) => {
let bundle_with_proof = OpaqueBundleWithProof::generate(
&self.storage_key_provider,
self.consensus_client.as_ref(),
domain_id,
consensus_block_hash,
bundle,
bundle_index,
)?;

InvalidBundlesProofData::Bundle(bundle_with_proof)
}
InvalidBundleType::UndecodableTx(_) | InvalidBundleType::InherentExtrinsic(_) => {
let extrinsic_proof = StorageProofProvider::<
LayoutV1<HeaderHashingFor<Block::Header>>,
>::generate_enumerated_proof_of_inclusion(
encoded_extrinsics.as_slice(),
extrinsic_index,
)
.ok_or(FraudProofError::FailToGenerateProofOfInclusion)?;
InvalidBundleType::UndecodableTx(_) | InvalidBundleType::InherentExtrinsic(_) => {
let encoded_extrinsics: Vec<_> =
bundle.extrinsics.iter().map(Encode::encode).collect();

let extrinsic_proof = StorageProofProvider::<
LayoutV1<HeaderHashingFor<Block::Header>>,
>::generate_enumerated_proof_of_inclusion(
encoded_extrinsics.as_slice(),
extrinsic_index,
)
.ok_or(FraudProofError::FailToGenerateProofOfInclusion)?;

InvalidBundlesProofData::Extrinsic(extrinsic_proof)
InvalidBundlesProofData::Extrinsic(extrinsic_proof)
}
}
};

Expand Down
Loading

0 comments on commit 47ccb63

Please sign in to comment.