From cb922442d2400c687851003d7fccdd42a29fde41 Mon Sep 17 00:00:00 2001 From: linning Date: Wed, 1 Nov 2023 03:02:12 +0800 Subject: [PATCH 1/4] Reject Stale and NewBranch type ER The stale type ER is previously used to measure bundle production rate for the dynamic tx range, which is now deprecated so we should reject stale ER. The new branch type ER is rejected, so the honest operator must submit a fraud proof to prune the bad ER at the same height before submitting ER at that height Signed-off-by: linning --- crates/pallet-domains/src/block_tree.rs | 76 +++++++++++-------------- crates/pallet-domains/src/lib.rs | 5 -- crates/pallet-domains/src/tests.rs | 64 --------------------- 3 files changed, 34 insertions(+), 111 deletions(-) diff --git a/crates/pallet-domains/src/block_tree.rs b/crates/pallet-domains/src/block_tree.rs index ea566e5a6d..5b55ee8ae4 100644 --- a/crates/pallet-domains/src/block_tree.rs +++ b/crates/pallet-domains/src/block_tree.rs @@ -24,6 +24,8 @@ pub enum Error { BuiltOnUnknownConsensusBlock, InFutureReceipt, PrunedReceipt, + StaleReceipt, + NewBranchReceipt, BadGenesisReceipt, UnexpectedReceiptType, MaxHeadDomainNumber, @@ -49,9 +51,7 @@ pub struct BlockTreeNode { pub(crate) enum AcceptedReceiptType { // New head receipt that extend the longest branch NewHead, - // Receipt that creates a new branch of the block tree - NewBranch, - // Receipt that comfirms the current head receipt + // Receipt that confirms the current head receipt CurrentHead, } @@ -61,6 +61,13 @@ pub(crate) enum RejectedReceiptType { InFuture, // Receipt that already been pruned Pruned, + // Receipt that confirm a non-head receipt + Stale, + // Receipt that tries to create a new branch of the block tree + // + // The honests operatpr must submit fraud proof to prune the bad receipt at the + // same height before submitting the valid receipt. + NewBranch, } impl From for Error { @@ -68,6 +75,8 @@ impl From for Error { match rejected_receipt { RejectedReceiptType::InFuture => Error::InFutureReceipt, RejectedReceiptType::Pruned => Error::PrunedReceipt, + RejectedReceiptType::Stale => Error::StaleReceipt, + RejectedReceiptType::NewBranch => Error::NewBranchReceipt, } } } @@ -77,8 +86,6 @@ impl From for Error { pub(crate) enum ReceiptType { Accepted(AcceptedReceiptType), Rejected(RejectedReceiptType), - // Receipt that comfirm a non-head receipt - Stale, } /// Get the receipt type of the given receipt based on the current block tree state @@ -103,13 +110,13 @@ pub(crate) fn execution_receipt_type( ReceiptType::Rejected(RejectedReceiptType::Pruned) } else if !already_exist { // Create new branch - ReceiptType::Accepted(AcceptedReceiptType::NewBranch) + ReceiptType::Rejected(RejectedReceiptType::NewBranch) } else if receipt_number == head_receipt_number { - // Add comfirm to the current head receipt + // Add confirm to the current head receipt ReceiptType::Accepted(AcceptedReceiptType::CurrentHead) } else { - // Add comfirm to a non-head receipt - ReceiptType::Stale + // Add confirm to a non-head receipt + ReceiptType::Rejected(RejectedReceiptType::Stale) } } } @@ -215,7 +222,8 @@ pub(crate) fn verify_execution_receipt( ); Err(Error::PrunedReceipt) } - ReceiptType::Accepted(_) | ReceiptType::Stale => Ok(()), + ReceiptType::Rejected(rejected_receipt_type) => Err(rejected_receipt_type.into()), + ReceiptType::Accepted(_) => Ok(()), } } @@ -240,9 +248,6 @@ pub(crate) fn process_execution_receipt( receipt_type: AcceptedReceiptType, ) -> ProcessExecutionReceiptResult { match receipt_type { - AcceptedReceiptType::NewBranch => { - add_new_receipt_to_block_tree::(domain_id, submitter, execution_receipt); - } AcceptedReceiptType::NewHead => { let domain_block_number = execution_receipt.domain_block_number; @@ -619,7 +624,7 @@ mod tests { let domain_id = register_genesis_domain(creator, vec![operator_id1, operator_id2]); extend_block_tree(domain_id, operator_id1, 3); - // Receipt that comfirm a non-head receipt is stale receipt + // Receipt that confirm a non-head receipt is stale receipt let head_receipt_number = HeadReceiptNumber::::get(domain_id); let stale_receipt = get_block_tree_node_at::(domain_id, head_receipt_number - 1) .unwrap() @@ -629,21 +634,21 @@ mod tests { // Stale receipt can pass the verification assert_eq!( execution_receipt_type::(domain_id, &stale_receipt), - ReceiptType::Stale + ReceiptType::Rejected(RejectedReceiptType::Stale) + ); + assert_err!( + verify_execution_receipt::(domain_id, &stale_receipt), + Error::StaleReceipt ); - assert_ok!(verify_execution_receipt::(domain_id, &stale_receipt)); - // Stale receipt can be submitted but won't be added to the block tree + // Stale receipt will be rejected and won't be added to the block tree let bundle = create_dummy_bundle_with_receipts( domain_id, operator_id2, H256::random(), stale_receipt, ); - assert_ok!(crate::Pallet::::submit_bundle( - RawOrigin::None.into(), - bundle, - )); + assert!(crate::Pallet::::submit_bundle(RawOrigin::None.into(), bundle,).is_err()); assert_eq!( BlockTreeNodes::::get(stale_receipt_hash) @@ -685,35 +690,22 @@ mod tests { // New branch receipt can pass the verification assert_eq!( execution_receipt_type::(domain_id, &new_branch_receipt), - ReceiptType::Accepted(AcceptedReceiptType::NewBranch) + ReceiptType::Rejected(RejectedReceiptType::NewBranch) + ); + assert_err!( + verify_execution_receipt::(domain_id, &new_branch_receipt), + Error::NewBranchReceipt ); - assert_ok!(verify_execution_receipt::( - domain_id, - &new_branch_receipt - )); - // Submit the new branch receipt will create fork in the block tree + // Submit the new branch receipt will will be rejected let bundle = create_dummy_bundle_with_receipts( domain_id, operator_id2, H256::random(), new_branch_receipt, ); - assert_ok!(crate::Pallet::::submit_bundle( - RawOrigin::None.into(), - bundle, - )); - - let nodes = BlockTree::::get(domain_id, head_receipt_number); - assert_eq!(nodes.len(), 2); - for n in nodes.iter() { - let node = BlockTreeNodes::::get(n).unwrap(); - if *n == new_branch_receipt_hash { - assert_eq!(node.operator_ids, vec![operator_id2]); - } else { - assert_eq!(node.operator_ids, vec![operator_id1]); - } - } + assert!(crate::Pallet::::submit_bundle(RawOrigin::None.into(), bundle).is_err()); + assert!(BlockTreeNodes::::get(new_branch_receipt_hash).is_none()); }); } diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index f8083beba3..907ee9e4d1 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -779,11 +779,6 @@ mod pallet { let receipt = opaque_bundle.into_receipt(); match execution_receipt_type::(domain_id, &receipt) { - // The stale receipt should not be further processed, but we still track them for purposes - // of measuring the bundle production rate. - ReceiptType::Stale => { - return Ok(()); - } ReceiptType::Rejected(rejected_receipt_type) => { return Err(Error::::BlockTree(rejected_receipt_type.into()).into()); } diff --git a/crates/pallet-domains/src/tests.rs b/crates/pallet-domains/src/tests.rs index 27fc71a819..f180337377 100644 --- a/crates/pallet-domains/src/tests.rs +++ b/crates/pallet-domains/src/tests.rs @@ -1250,67 +1250,3 @@ fn test_basic_fraud_proof_processing() { ); }); } - -#[test] -fn test_fraud_proof_prune_fraudulent_branch_of_receipt() { - let creator = 0u64; - let operator_id = 1u64; - let head_domain_number = BlockTreePruningDepth::get() * 2; - let bad_receipt_start_at = head_domain_number - BlockTreePruningDepth::get() / 2; - let mut ext = new_test_ext_with_extensions(); - ext.execute_with(|| { - let domain_id = register_genesis_domain(creator, vec![operator_id]); - extend_block_tree(domain_id, operator_id, head_domain_number + 1); - assert_eq!( - HeadReceiptNumber::::get(domain_id), - head_domain_number - ); - - // Construct a fraudulent branch of ER in the block tree start at `head_domain_number - BlockTreePruningDepth::get() / 2` - let mut bad_receipts = vec![]; - for i in 0..3 { - let bad_receipt_at = bad_receipt_start_at + i; - let bad_receipt = { - let mut er = get_block_tree_node_at::(domain_id, bad_receipt_at) - .unwrap() - .execution_receipt; - er.final_state_root = H256::random(); - if let Some(h) = bad_receipts.last() { - er.parent_domain_block_receipt_hash = *h; - } - er - }; - let bad_receipt_hash = bad_receipt.hash::>(); - let bundle = create_dummy_bundle_with_receipts( - domain_id, - operator_id, - H256::random(), - bad_receipt, - ); - assert_ok!(Domains::submit_bundle(RawOrigin::None.into(), bundle)); - bad_receipts.push(bad_receipt_hash); - - assert_eq!(BlockTree::::get(domain_id, bad_receipt_at).len(), 2); - assert!(BlockTreeNodes::::get(bad_receipt_hash).is_some()); - } - - // Construct and submit a fraud proof for the fraudulent branch of ER - let fraud_proof = FraudProof::dummy_fraud_proof(domain_id, bad_receipts[0]); - assert_ok!(Domains::submit_fraud_proof( - RawOrigin::None.into(), - Box::new(fraud_proof) - )); - - // The head receipt number should be unchanged since the best branch of ER is valid - // and only the fraudulent branch of ER should be removed from the block tree - assert_eq!( - HeadReceiptNumber::::get(domain_id), - head_domain_number - ); - for i in 0..3 { - let bad_receipt_at = bad_receipt_start_at + i; - assert_eq!(BlockTree::::get(domain_id, bad_receipt_at).len(), 1); - assert!(BlockTreeNodes::::get(bad_receipts[i as usize]).is_none()); - } - }); -} From 1ea0ba920d0583ecba6985a6cab416a463172fcb Mon Sep 17 00:00:00 2001 From: linning Date: Thu, 2 Nov 2023 07:26:55 +0800 Subject: [PATCH 2/4] Storing only one ER at a specific height in the block tree This commit change the block tree to use Option to store ER instead of BTreeSet, thus only one ER at a specific height. This change also bring a overhaul to the fraud proof processing and remove some TODO and FIXME Signed-off-by: linning --- crates/pallet-domains/src/benchmarking.rs | 8 +- crates/pallet-domains/src/block_tree.rs | 109 +++++++++------------ crates/pallet-domains/src/lib.rs | 87 ++++++----------- crates/pallet-domains/src/tests.rs | 114 ++++++++++++---------- 4 files changed, 136 insertions(+), 182 deletions(-) diff --git a/crates/pallet-domains/src/benchmarking.rs b/crates/pallet-domains/src/benchmarking.rs index 69b91d11b9..ad0556922b 100644 --- a/crates/pallet-domains/src/benchmarking.rs +++ b/crates/pallet-domains/src/benchmarking.rs @@ -38,7 +38,6 @@ mod benchmarks { let mut receipt = BlockTree::::get::<_, DomainBlockNumberFor>(domain_id, Zero::zero()) - .first() .and_then(BlockTreeNodes::::get) .expect("genesis receipt must exist") .execution_receipt; @@ -59,8 +58,6 @@ mod benchmarks { // Create ER for the above bundle let head_receipt_number = HeadReceiptNumber::::get(domain_id); let parent_domain_block_receipt = BlockTree::::get(domain_id, head_receipt_number) - .first() - .cloned() .expect("parent receipt must exist"); receipt = ExecutionReceipt::dummy::>( consensus_block_number, @@ -243,9 +240,8 @@ mod benchmarks { assert_eq!(domain_obj.domain_config, domain_config); assert_eq!(domain_obj.owner_account_id, creator); assert!(DomainStakingSummary::::get(domain_id).is_some()); - assert_eq!( - BlockTree::::get::<_, DomainBlockNumberFor>(domain_id, Zero::zero()).len(), - 1 + assert!( + BlockTree::::get::<_, DomainBlockNumberFor>(domain_id, Zero::zero()).is_some(), ); assert_eq!(NextDomainId::::get(), domain_id + 1.into()); } diff --git a/crates/pallet-domains/src/block_tree.rs b/crates/pallet-domains/src/block_tree.rs index 5b55ee8ae4..b5d7280bee 100644 --- a/crates/pallet-domains/src/block_tree.rs +++ b/crates/pallet-domains/src/block_tree.rs @@ -2,9 +2,9 @@ use crate::pallet::StateRoots; use crate::{ - BalanceOf, BlockTree, BlockTreeNodes, Config, ConsensusBlockHash, DomainBlockDescendants, - DomainBlockNumberFor, DomainHashingFor, ExecutionInbox, ExecutionReceiptOf, HeadReceiptNumber, - InboxedBundleAuthor, + BalanceOf, BlockTree, BlockTreeNodes, Config, ConsensusBlockHash, DomainBlockNumberFor, + DomainHashingFor, ExecutionInbox, ExecutionReceiptOf, HeadReceiptNumber, InboxedBundleAuthor, + ReceiptHashFor, }; use codec::{Decode, Encode}; use frame_support::{ensure, PalletError}; @@ -29,7 +29,6 @@ pub enum Error { BadGenesisReceipt, UnexpectedReceiptType, MaxHeadDomainNumber, - MultipleERsAfterChallengePeriod, MissingDomainBlock, InvalidTraceRoot, UnavailableConsensusBlockHash, @@ -88,6 +87,16 @@ pub(crate) enum ReceiptType { Rejected(RejectedReceiptType), } +pub(crate) fn is_receipt_exist( + domain_id: DomainId, + domain_number: DomainBlockNumberFor, + receipt_hash: ReceiptHashFor, +) -> bool { + BlockTree::::get(domain_id, domain_number) + .map(|h| h == receipt_hash) + .unwrap_or(false) +} + /// Get the receipt type of the given receipt based on the current block tree state pub(crate) fn execution_receipt_type( domain_id: DomainId, @@ -102,8 +111,11 @@ pub(crate) fn execution_receipt_type( Ordering::Less => { let oldest_receipt_number = head_receipt_number.saturating_sub(T::BlockTreePruningDepth::get()); - let already_exist = BlockTree::::get(domain_id, receipt_number) - .contains(&execution_receipt.hash::>()); + let already_exist = is_receipt_exist::( + domain_id, + receipt_number, + execution_receipt.hash::>(), + ); if receipt_number < oldest_receipt_number { // Receipt already pruned @@ -142,8 +154,11 @@ pub(crate) fn verify_execution_receipt( // The genesis receipt is generated and added to the block tree by the runtime upon domain // instantiation, thus it is unchallengeable and must always be the same. ensure!( - BlockTree::::get(domain_id, domain_block_number) - .contains(&execution_receipt.hash::>()), + is_receipt_exist::( + domain_id, + *domain_block_number, + execution_receipt.hash::>(), + ), Error::BadGenesisReceipt ); } else { @@ -200,8 +215,11 @@ pub(crate) fn verify_execution_receipt( } if let Some(parent_block_number) = domain_block_number.checked_sub(&One::one()) { - let parent_block_exist = BlockTree::::get(domain_id, parent_block_number) - .contains(parent_domain_block_receipt_hash); + let parent_block_exist = is_receipt_exist::( + domain_id, + parent_block_number, + *parent_domain_block_receipt_hash, + ); ensure!(parent_block_exist, Error::UnknownParentBlockReceipt); } @@ -260,29 +278,12 @@ pub(crate) fn process_execution_receipt( if let Some(to_prune) = domain_block_number.checked_sub(&T::BlockTreePruningDepth::get()) { - let receipts_at_number = BlockTree::::take(domain_id, to_prune); - - // The receipt at `to_prune` may already been pruned if there is fraud proof being - // processed and the `HeadReceiptNumber` is reverted. - if receipts_at_number.is_empty() { - return Ok(None); - } - - // FIXME: the following check is wrong because the ER at `to_prune` may not necessarily go through - // the whole challenge period since the malicious operator can submit `NewBranch` ER any time, thus - // the ER may just submitted a few blocks ago and `receipts_at_number` may contains more than one block. - // - // In current implementatiomn, the correct finalized ER should be the one that has `BlockTreePruningDepth` - // length of descendants which is non-trival to find, but once https://github.com/subspace/subspace/issues/1731 - // is implemented this issue should be fixed automatically. - if receipts_at_number.len() != 1 { - return Err(Error::MultipleERsAfterChallengePeriod); - } - - let receipt_hash = receipts_at_number - .first() - .cloned() - .expect("should always have a value due to check above"); + let receipt_hash = match BlockTree::::take(domain_id, to_prune) { + Some(h) => h, + // The receipt at `to_prune` may already been pruned if there is fraud proof being + // processed previously and the `HeadReceiptNumber` is reverted. + None => return Ok(None), + }; let BlockTreeNode { execution_receipt, @@ -294,7 +295,6 @@ pub(crate) fn process_execution_receipt( to_prune, execution_receipt.domain_block_hash, )); - _ = DomainBlockDescendants::::take(receipt_hash); // Collect the invalid bundle author let mut invalid_bundle_authors = Vec::new(); @@ -361,15 +361,7 @@ fn add_new_receipt_to_block_tree( execution_receipt.final_state_root, ); - BlockTree::::mutate(domain_id, domain_block_number, |er_hashes| { - er_hashes.insert(er_hash); - }); - DomainBlockDescendants::::mutate( - execution_receipt.parent_domain_block_receipt_hash, - |er_hashes| { - er_hashes.insert(er_hash); - }, - ); + BlockTree::::insert(domain_id, domain_block_number, er_hash); let block_tree_node = BlockTreeNode { execution_receipt, operator_ids: sp_std::vec![submitter], @@ -388,10 +380,8 @@ pub(crate) fn import_genesis_receipt( execution_receipt: genesis_receipt, operator_ids: sp_std::vec![], }; - // NOTE: no need to update the head receipt number as we are using `ValueQuery` - BlockTree::::mutate(domain_id, domain_block_number, |er_hashes| { - er_hashes.insert(er_hash); - }); + // NOTE: no need to update the head receipt number as `HeadReceiptNumber` is using `ValueQuery` + BlockTree::::insert(domain_id, domain_block_number, er_hash); StateRoots::::insert( ( domain_id, @@ -424,11 +414,9 @@ mod tests { let domain_id = register_genesis_domain(0u64, vec![0u64]); // The genesis receipt should be added to the block tree - let block_tree_node_at_0 = BlockTree::::get(domain_id, 0); - assert_eq!(block_tree_node_at_0.len(), 1); + let block_tree_node_at_0 = BlockTree::::get(domain_id, 0).unwrap(); - let genesis_node = - BlockTreeNodes::::get(block_tree_node_at_0.first().unwrap()).unwrap(); + let genesis_node = BlockTreeNodes::::get(block_tree_node_at_0).unwrap(); assert!(genesis_node.operator_ids.is_empty()); assert_eq!(HeadReceiptNumber::::get(domain_id), 0); @@ -517,12 +505,10 @@ mod tests { assert_eq!(head_receipt_number, block_number as u32 - 1); // As we only extending the block tree there should be no fork - let parent_block_tree_nodes = - BlockTree::::get(domain_id, head_receipt_number); - assert_eq!(parent_block_tree_nodes.len(), 1); + let parent_domain_block_receipt = + BlockTree::::get(domain_id, head_receipt_number).unwrap(); // The submitter should be added to `operator_ids` - let parent_domain_block_receipt = parent_block_tree_nodes.first().unwrap(); let parent_node = BlockTreeNodes::::get(parent_domain_block_receipt).unwrap(); assert_eq!(parent_node.operator_ids.len(), 1); assert_eq!(parent_node.operator_ids[0], operator_id); @@ -532,7 +518,7 @@ mod tests { receipt = create_dummy_receipt( block_number, H256::random(), - *parent_domain_block_receipt, + parent_domain_block_receipt, vec![bundle_extrinsics_root], ); @@ -547,7 +533,7 @@ mod tests { // `InvalidExtrinsicsRoots` error as `ExecutionInbox` of block 1 is pruned let pruned_receipt = receipt_of_block_1.unwrap(); let pruned_bundle = bundle_header_hash_of_block_1.unwrap(); - assert!(BlockTree::::get(domain_id, 1).is_empty()); + assert!(BlockTree::::get(domain_id, 1).is_none()); assert!(ExecutionInbox::::get((domain_id, 1, 1)).is_empty()); assert!(!InboxedBundleAuthor::::contains_key(pruned_bundle)); assert_eq!( @@ -563,10 +549,6 @@ mod tests { pruned_receipt.consensus_block_number, ) .is_none()); - assert!(DomainBlockDescendants::::get( - pruned_receipt.hash::>() - ) - .is_empty()); }); } @@ -670,10 +652,7 @@ mod tests { extend_block_tree(domain_id, operator_id1, 3); let head_receipt_number = HeadReceiptNumber::::get(domain_id); - assert_eq!( - BlockTree::::get(domain_id, head_receipt_number).len(), - 1 - ); + assert!(BlockTree::::get(domain_id, head_receipt_number).is_some()); // Construct new branch receipt that fork away from an existing node of // the block tree diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index 907ee9e4d1..63851da646 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -161,7 +161,7 @@ mod pallet { AtLeast32BitUnsigned, BlockNumberProvider, CheckEqual, CheckedAdd, Header as HeaderT, MaybeDisplay, One, SimpleBitOps, Zero, }; - use sp_runtime::SaturatedConversion; + use sp_runtime::{SaturatedConversion, Saturating}; use sp_std::boxed::Box; use sp_std::collections::btree_map::BTreeMap; use sp_std::collections::btree_set::BTreeSet; @@ -467,8 +467,8 @@ mod pallet { DomainId, Identity, DomainBlockNumberFor, - BTreeSet>, - ValueQuery, + ReceiptHashFor, + OptionQuery, >; /// Mapping of block tree node hash to the node, each node represent a domain block @@ -487,13 +487,6 @@ mod pallet { OptionQuery, >; - // Mapping of the parent ER to all its immediate descendants ER - // TODO: remove this mapping once https://github.com/subspace/subspace/issues/1731 is implemented - // by then every parent ER should only have one immediate descendants ER - #[pallet::storage] - pub(super) type DomainBlockDescendants = - StorageMap<_, Identity, ReceiptHashFor, BTreeSet>, ValueQuery>; - /// The head receipt number of each domain #[pallet::storage] pub(super) type HeadReceiptNumber = @@ -728,7 +721,7 @@ mod pallet { }, FraudProofProcessed { domain_id: DomainId, - new_head_receipt_number: Option>, + new_head_receipt_number: DomainBlockNumberFor, }, DomainOperatorAllowListUpdated { domain_id: DomainId, @@ -882,77 +875,56 @@ mod pallet { log::trace!(target: "runtime::domains", "Processing fraud proof: {fraud_proof:?}"); let domain_id = fraud_proof.domain_id(); - let mut receipt_to_remove = vec![fraud_proof.bad_receipt_hash()]; + let head_receipt_number = HeadReceiptNumber::::get(domain_id); + let bad_receipt_number = BlockTreeNodes::::get(fraud_proof.bad_receipt_hash()) + .ok_or::>(FraudProofError::BadReceiptNotFound.into())? + .execution_receipt + .domain_block_number; + // The `head_receipt_number` must greater than or equal to any existing receipt, including + // the bad receipt, otherwise the fraud proof should be rejected due to `BadReceiptNotFound`, + // double check here to make it more robust. + ensure!( + head_receipt_number >= bad_receipt_number, + Error::::from(FraudProofError::BadReceiptNotFound), + ); + + // Starting from the head receipt, prune all ER between [bad_receipt_number..head_receipt_number] + let mut to_prune = head_receipt_number; let mut operator_to_slash = BTreeSet::new(); - let mut next_head_receipt_number = None; + while to_prune >= bad_receipt_number { + let receipt_hash = BlockTree::::take(domain_id, to_prune) + .ok_or::>(FraudProofError::BadReceiptNotFound.into())?; - // Prune the bad ER and all of its descendants from the block tree. ER are pruning - // with BFS order from lower height to higher height. - while let Some(receipt_hash) = receipt_to_remove.pop() { let BlockTreeNode { execution_receipt, operator_ids, } = BlockTreeNodes::::take(receipt_hash) .ok_or::>(FraudProofError::BadReceiptNotFound.into())?; - BlockTree::::mutate_exists( - domain_id, - execution_receipt.domain_block_number, - |maybe_er_hashes| { - if let Some(er_hashes) = maybe_er_hashes { - // Remove ER hash from the set, remove the whole set if it is empty. - er_hashes.remove(&receipt_hash); - if er_hashes.is_empty() { - maybe_er_hashes.take(); - } - // If all the ER at `domain_block_number` are pruned then any ER that derive from domain - // block with height > `domain_block_number` must also be pruned since their parent ER - // are pruned thus we can reset the new head receipt number to `domain_block_number - 1`. - if maybe_er_hashes.is_none() && next_head_receipt_number.is_none() { - next_head_receipt_number - .replace(execution_receipt.domain_block_number - One::one()); - } else if maybe_er_hashes.is_some() - && next_head_receipt_number.is_some() - { - // `next_head_receipt_number` is `Some` means all the ER at prior height are pruned - // thus the descendants must also be pruned - return Err::<(), Error>( - FraudProofError::DescendantsOfFraudulentERNotPruned.into(), - ); - } - } - Ok(()) - }, - )?; - - _ = StateRoots::::take(( + let _ = StateRoots::::take(( domain_id, execution_receipt.domain_block_number, execution_receipt.domain_block_hash, )); - // Add all the immediate descendants of the pruned ER to the `receipt_to_remove` list - DomainBlockDescendants::::take(receipt_hash) - .into_iter() - .for_each(|descendant| receipt_to_remove.push(descendant)); - // NOTE: the operator id will be deduplicated since we are using `BTreeSet` operator_ids.into_iter().for_each(|id| { operator_to_slash.insert(id); }); - } - // Update the head receipt number - if let Some(next_head_receipt_number) = next_head_receipt_number { - HeadReceiptNumber::::insert(domain_id, next_head_receipt_number); + to_prune -= One::one(); } + // Update the head receipt number to `bad_receipt_number - 1` + let new_head_receipt_number = bad_receipt_number.saturating_sub(One::one()); + HeadReceiptNumber::::insert(domain_id, new_head_receipt_number); + // Slash operator who have submitted the pruned fraudulent ER do_slash_operators::(operator_to_slash.into_iter()).map_err(Error::::from)?; Self::deposit_event(Event::FraudProofProcessed { domain_id, - new_head_receipt_number: next_head_receipt_number, + new_head_receipt_number, }); Ok(()) @@ -1366,7 +1338,6 @@ impl Pallet { pub fn genesis_state_root(domain_id: DomainId) -> Option { BlockTree::::get(domain_id, DomainBlockNumberFor::::zero()) - .first() .and_then(BlockTreeNodes::::get) .map(|block| block.execution_receipt.final_state_root.into()) } diff --git a/crates/pallet-domains/src/tests.rs b/crates/pallet-domains/src/tests.rs index f180337377..4ce5ccbba4 100644 --- a/crates/pallet-domains/src/tests.rs +++ b/crates/pallet-domains/src/tests.rs @@ -554,9 +554,7 @@ pub(crate) fn get_block_tree_node_at( ) -> Option< BlockTreeNode, T::Hash, DomainBlockNumberFor, T::DomainHash, BalanceOf>, > { - BlockTree::::get(domain_id, block_number) - .first() - .and_then(BlockTreeNodes::::get) + BlockTree::::get(domain_id, block_number).and_then(BlockTreeNodes::::get) } // TODO: Unblock once bundle producer election v2 is finished. @@ -1196,57 +1194,67 @@ fn generate_invalid_domain_block_hash_fraud_proof( fn test_basic_fraud_proof_processing() { let creator = 0u64; let operator_id = 1u64; - let head_domain_number = BlockTreePruningDepth::get() * 2; - let bad_receipt_at = head_domain_number - BlockTreePruningDepth::get() / 2; - let mut ext = new_test_ext_with_extensions(); - ext.execute_with(|| { - let domain_id = register_genesis_domain(creator, vec![operator_id]); - extend_block_tree(domain_id, operator_id, head_domain_number + 1); - assert_eq!( - HeadReceiptNumber::::get(domain_id), - head_domain_number - ); - - // Construct and submit fraud proof that target ER at `head_domain_number - BlockTreePruningDepth::get() / 2` - let bad_receipt = get_block_tree_node_at::(domain_id, bad_receipt_at) - .unwrap() - .execution_receipt; - let bad_receipt_hash = bad_receipt.hash::>(); - let fraud_proof = FraudProof::dummy_fraud_proof(domain_id, bad_receipt_hash); - assert_ok!(Domains::submit_fraud_proof( - RawOrigin::None.into(), - Box::new(fraud_proof) - )); - - // The head receipt number should be reverted to `bad_receipt_at - 1` - let head_receipt_number_after_fraud_proof = HeadReceiptNumber::::get(domain_id); - assert_eq!(head_receipt_number_after_fraud_proof, bad_receipt_at - 1); + let head_domain_number = BlockTreePruningDepth::get() - 1; + let test_cases = vec![ + 1, + 2, + head_domain_number - BlockTreePruningDepth::get() / 2, + head_domain_number - 1, + head_domain_number, + ]; + for bad_receipt_at in test_cases { + let mut ext = new_test_ext_with_extensions(); + ext.execute_with(|| { + let domain_id = register_genesis_domain(creator, vec![operator_id]); + extend_block_tree(domain_id, operator_id, head_domain_number + 1); + assert_eq!( + HeadReceiptNumber::::get(domain_id), + head_domain_number + ); - for block_number in bad_receipt_at..=head_domain_number { - // The targetted ER and all its descendants should be removed from the block tree - assert!(BlockTree::::get(domain_id, block_number).is_empty()); + // Construct and submit fraud proof that target ER at `head_domain_number - BlockTreePruningDepth::get() / 2` + let bad_receipt = get_block_tree_node_at::(domain_id, bad_receipt_at) + .unwrap() + .execution_receipt; + let bad_receipt_hash = bad_receipt.hash::>(); + let fraud_proof = FraudProof::dummy_fraud_proof(domain_id, bad_receipt_hash); + assert_ok!(Domains::submit_fraud_proof( + RawOrigin::None.into(), + Box::new(fraud_proof) + )); + + // The head receipt number should be reverted to `bad_receipt_at - 1` + let head_receipt_number_after_fraud_proof = HeadReceiptNumber::::get(domain_id); + assert_eq!(head_receipt_number_after_fraud_proof, bad_receipt_at - 1); + + for block_number in bad_receipt_at..=head_domain_number { + // The targetted ER and all its descendants should be removed from the block tree + assert!(BlockTree::::get(domain_id, block_number).is_none()); + + // The other data that used to verify ER should not be removed, such that the honest + // operator can re-submit the valid ER + assert!(!ExecutionInbox::::get(( + domain_id, + block_number, + block_number as u64 + )) + .is_empty()); + assert!(ConsensusBlockHash::::get(domain_id, block_number as u64).is_some()); + } - // The other data that used to verify ER should not be removed, such that the honest - // operator can re-submit the valid ER - assert!( - !ExecutionInbox::::get((domain_id, block_number, block_number as u64)) - .is_empty() + // Re-submit the valid ER + let resubmit_receipt = bad_receipt; + let bundle = create_dummy_bundle_with_receipts( + domain_id, + operator_id, + H256::random(), + resubmit_receipt, ); - assert!(ConsensusBlockHash::::get(domain_id, block_number as u64).is_some()); - } - - // Re-submit the valid ER - let resubmit_receipt = bad_receipt; - let bundle = create_dummy_bundle_with_receipts( - domain_id, - operator_id, - H256::random(), - resubmit_receipt, - ); - assert_ok!(Domains::submit_bundle(RawOrigin::None.into(), bundle,)); - assert_eq!( - HeadReceiptNumber::::get(domain_id), - head_receipt_number_after_fraud_proof + 1 - ); - }); + assert_ok!(Domains::submit_bundle(RawOrigin::None.into(), bundle,)); + assert_eq!( + HeadReceiptNumber::::get(domain_id), + head_receipt_number_after_fraud_proof + 1 + ); + }); + } } From 2cb70a36516ca5bcae9f773d29cd1d80b440f4aa Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 3 Nov 2023 08:52:15 +0800 Subject: [PATCH 3/4] Consider the head receipt from the previous consensus block as Stale instead of CurrentHead In this way we can ensure only ER that derived from the latest domain block will be accepted Signed-off-by: linning --- crates/pallet-domains/src/block_tree.rs | 172 ++++++++++++++++-------- crates/pallet-domains/src/lib.rs | 8 ++ crates/pallet-domains/src/tests.rs | 60 ++++++--- 3 files changed, 163 insertions(+), 77 deletions(-) diff --git a/crates/pallet-domains/src/block_tree.rs b/crates/pallet-domains/src/block_tree.rs index b5d7280bee..894590cc6b 100644 --- a/crates/pallet-domains/src/block_tree.rs +++ b/crates/pallet-domains/src/block_tree.rs @@ -3,8 +3,8 @@ use crate::pallet::StateRoots; use crate::{ BalanceOf, BlockTree, BlockTreeNodes, Config, ConsensusBlockHash, DomainBlockNumberFor, - DomainHashingFor, ExecutionInbox, ExecutionReceiptOf, HeadReceiptNumber, InboxedBundleAuthor, - ReceiptHashFor, + DomainHashingFor, ExecutionInbox, ExecutionReceiptOf, HeadReceiptExtended, HeadReceiptNumber, + InboxedBundleAuthor, ReceiptHashFor, }; use codec::{Decode, Encode}; use frame_support::{ensure, PalletError}; @@ -50,7 +50,7 @@ pub struct BlockTreeNode { pub(crate) enum AcceptedReceiptType { // New head receipt that extend the longest branch NewHead, - // Receipt that confirms the current head receipt + // Receipt that confirms the head receipt that added in the current block CurrentHead, } @@ -60,7 +60,7 @@ pub(crate) enum RejectedReceiptType { InFuture, // Receipt that already been pruned Pruned, - // Receipt that confirm a non-head receipt + // Receipt that confirm a non-head receipt or head receipt of the previous block Stale, // Receipt that tries to create a new branch of the block tree // @@ -109,27 +109,34 @@ pub(crate) fn execution_receipt_type( Ordering::Greater => ReceiptType::Rejected(RejectedReceiptType::InFuture), Ordering::Equal => ReceiptType::Accepted(AcceptedReceiptType::NewHead), Ordering::Less => { + // Reject receipt that already pruned/confirmed let oldest_receipt_number = head_receipt_number.saturating_sub(T::BlockTreePruningDepth::get()); + if receipt_number < oldest_receipt_number { + return ReceiptType::Rejected(RejectedReceiptType::Pruned); + } + + // Reject receipt that try to create new branch in the block tree let already_exist = is_receipt_exist::( domain_id, receipt_number, execution_receipt.hash::>(), ); + if !already_exist { + return ReceiptType::Rejected(RejectedReceiptType::NewBranch); + } - if receipt_number < oldest_receipt_number { - // Receipt already pruned - ReceiptType::Rejected(RejectedReceiptType::Pruned) - } else if !already_exist { - // Create new branch - ReceiptType::Rejected(RejectedReceiptType::NewBranch) - } else if receipt_number == head_receipt_number { - // Add confirm to the current head receipt - ReceiptType::Accepted(AcceptedReceiptType::CurrentHead) - } else { - // Add confirm to a non-head receipt - ReceiptType::Rejected(RejectedReceiptType::Stale) + // Add confirm to the head receipt that added in the current block or it is + // the genesis receipt + let head_receipt_extended = HeadReceiptExtended::::get(domain_id); + if receipt_number == head_receipt_number + && (head_receipt_extended || receipt_number.is_zero()) + { + return ReceiptType::Accepted(AcceptedReceiptType::CurrentHead); } + + // Add confirm to a non-head receipt or head receipt of the previous block + ReceiptType::Rejected(RejectedReceiptType::Stale) } } } @@ -273,6 +280,7 @@ pub(crate) fn process_execution_receipt( // Update the head receipt number HeadReceiptNumber::::insert(domain_id, domain_block_number); + HeadReceiptExtended::::insert(domain_id, true); // Prune expired domain block if let Some(to_prune) = @@ -398,8 +406,8 @@ mod tests { use super::*; use crate::tests::{ create_dummy_bundle_with_receipts, create_dummy_receipt, extend_block_tree, - get_block_tree_node_at, new_test_ext_with_extensions, register_genesis_domain, - run_to_block, BlockTreePruningDepth, Test, + extend_block_tree_from_zero, get_block_tree_node_at, new_test_ext_with_extensions, + register_genesis_domain, run_to_block, BlockTreePruningDepth, Test, }; use frame_support::dispatch::RawOrigin; use frame_support::{assert_err, assert_ok}; @@ -553,24 +561,45 @@ mod tests { } #[test] - fn test_confirm_head_receipt() { + fn test_confirm_current_head_receipt() { let creator = 0u64; let operator_id1 = 1u64; let operator_id2 = 2u64; let mut ext = new_test_ext_with_extensions(); ext.execute_with(|| { let domain_id = register_genesis_domain(creator, vec![operator_id1, operator_id2]); - extend_block_tree(domain_id, operator_id1, 3); + let next_head_receipt = extend_block_tree_from_zero(domain_id, operator_id1, 3); - let head_receipt_number = HeadReceiptNumber::::get(domain_id); + // Submit the new head receipt + assert_eq!( + execution_receipt_type::(domain_id, &next_head_receipt), + ReceiptType::Accepted(AcceptedReceiptType::NewHead) + ); + assert_ok!(verify_execution_receipt::( + domain_id, + &next_head_receipt + )); + let bundle = create_dummy_bundle_with_receipts( + domain_id, + operator_id1, + H256::random(), + next_head_receipt.clone(), + ); + assert_ok!(crate::Pallet::::submit_bundle( + RawOrigin::None.into(), + bundle, + )); - // Get the head receipt + let head_receipt_number = HeadReceiptNumber::::get(domain_id); let current_head_receipt = get_block_tree_node_at::(domain_id, head_receipt_number) .unwrap() .execution_receipt; - // Receipt should be valid + // Now `next_head_receipt` become the head receipt + assert_eq!(next_head_receipt, current_head_receipt); + + // Head receipt added in the current block is consider valid assert_eq!( execution_receipt_type::(domain_id, ¤t_head_receipt), ReceiptType::Accepted(AcceptedReceiptType::CurrentHead) @@ -580,7 +609,7 @@ mod tests { ¤t_head_receipt )); - // Re-submit the receipt will add confirm to the head receipt + // Re-submit the head receipt by a different operator is okay let bundle = create_dummy_bundle_with_receipts( domain_id, operator_id2, @@ -591,20 +620,21 @@ mod tests { RawOrigin::None.into(), bundle, )); + let head_node = get_block_tree_node_at::(domain_id, head_receipt_number).unwrap(); assert_eq!(head_node.operator_ids, vec![operator_id1, operator_id2]); }); } #[test] - fn test_stale_receipt() { + fn test_non_head_receipt() { let creator = 0u64; let operator_id1 = 1u64; let operator_id2 = 2u64; let mut ext = new_test_ext_with_extensions(); ext.execute_with(|| { let domain_id = register_genesis_domain(creator, vec![operator_id1, operator_id2]); - extend_block_tree(domain_id, operator_id1, 3); + extend_block_tree_from_zero(domain_id, operator_id1, 3); // Receipt that confirm a non-head receipt is stale receipt let head_receipt_number = HeadReceiptNumber::::get(domain_id); @@ -641,6 +671,47 @@ mod tests { }); } + #[test] + fn test_previous_head_receipt() { + let creator = 0u64; + let operator_id1 = 1u64; + let operator_id2 = 2u64; + let mut ext = new_test_ext_with_extensions(); + ext.execute_with(|| { + let domain_id = register_genesis_domain(creator, vec![operator_id1, operator_id2]); + extend_block_tree_from_zero(domain_id, operator_id1, 3); + + // No new receipt submitted in current block + assert!(!HeadReceiptExtended::::get(domain_id)); + + // Receipt that confirm a head receipt of the previous block is stale receipt + let head_receipt_number = HeadReceiptNumber::::get(domain_id); + let previous_head_receipt = + get_block_tree_node_at::(domain_id, head_receipt_number) + .unwrap() + .execution_receipt; + + // Stale receipt can not pass the verification + assert_eq!( + execution_receipt_type::(domain_id, &previous_head_receipt), + ReceiptType::Rejected(RejectedReceiptType::Stale) + ); + assert_err!( + verify_execution_receipt::(domain_id, &previous_head_receipt), + Error::StaleReceipt + ); + + // Stale receipt will be rejected and won't be added to the block tree + let bundle = create_dummy_bundle_with_receipts( + domain_id, + operator_id2, + H256::random(), + previous_head_receipt, + ); + assert!(crate::Pallet::::submit_bundle(RawOrigin::None.into(), bundle,).is_err()); + }); + } + #[test] fn test_new_branch_receipt() { let creator = 0u64; @@ -649,7 +720,7 @@ mod tests { let mut ext = new_test_ext_with_extensions(); ext.execute_with(|| { let domain_id = register_genesis_domain(creator, vec![operator_id1, operator_id2]); - extend_block_tree(domain_id, operator_id1, 3); + extend_block_tree_from_zero(domain_id, operator_id1, 3); let head_receipt_number = HeadReceiptNumber::::get(domain_id); assert!(BlockTree::::get(domain_id, head_receipt_number).is_some()); @@ -695,7 +766,7 @@ mod tests { let mut ext = new_test_ext_with_extensions(); ext.execute_with(|| { let domain_id = register_genesis_domain(creator, vec![operator_id]); - extend_block_tree(domain_id, operator_id, 3); + extend_block_tree_from_zero(domain_id, operator_id, 3); let head_receipt_number = HeadReceiptNumber::::get(domain_id); let current_head_receipt = @@ -780,7 +851,7 @@ mod tests { let mut ext = new_test_ext_with_extensions(); ext.execute_with(|| { let domain_id = register_genesis_domain(creator, vec![operator_id1, operator_id2]); - extend_block_tree(domain_id, operator_id1, 3); + extend_block_tree_from_zero(domain_id, operator_id1, 3); let head_receipt_number = HeadReceiptNumber::::get(domain_id); @@ -832,26 +903,22 @@ mod tests { let mut ext = new_test_ext_with_extensions(); ext.execute_with(|| { let domain_id = register_genesis_domain(creator, operator_set.clone()); - let mut target_receipt = extend_block_tree(domain_id, operator_set[0], 3); + let next_receipt = extend_block_tree_from_zero(domain_id, operator_set[0], 3); - // Submit bundle for every operator except operator 1 since it already submit one - let head_receipt_number = HeadReceiptNumber::::get(domain_id); - let current_head_receipt = - get_block_tree_node_at::(domain_id, head_receipt_number) - .unwrap() - .execution_receipt; - for operator_id in operator_set.iter().skip(1) { + // Submit bundle for every operator + for operator_id in operator_set.iter() { let bundle = create_dummy_bundle_with_receipts( domain_id, *operator_id, H256::random(), - current_head_receipt.clone(), + next_receipt.clone(), ); assert_ok!(crate::Pallet::::submit_bundle( RawOrigin::None.into(), bundle, )); } + let head_receipt_number = HeadReceiptNumber::::get(domain_id); let head_node = get_block_tree_node_at::(domain_id, head_receipt_number).unwrap(); assert_eq!(head_node.operator_ids, operator_set); @@ -886,31 +953,20 @@ mod tests { bundles.push(InboxedBundle::valid(H256::random(), extrinsics_root)); } } - target_receipt.inboxed_bundles = bundles; - - // Run into next block - run_to_block::( - current_block_number + 1, - target_receipt.consensus_block_hash, + let mut target_receipt = create_dummy_receipt( + current_block_number, + H256::random(), + next_receipt.hash::>(), + vec![], ); - let current_block_number = current_block_number + 1; - - // Submit `target_receipt` - assert_ok!(crate::Pallet::::submit_bundle( - RawOrigin::None.into(), - create_dummy_bundle_with_receipts( - domain_id, - operator_set[0], - H256::random(), - target_receipt, - ), - )); + target_receipt.inboxed_bundles = bundles; - // Extend the block tree by `challenge_period - 1` blocks + // Extend the block tree by `challenge_period + 1` blocks let next_receipt = extend_block_tree( domain_id, operator_set[0], - (current_block_number + challenge_period) as u32 - 1, + (current_block_number + challenge_period) as u32 + 1, + target_receipt, ); // Confirm `target_receipt` let confirmed_domain_block = process_execution_receipt::( diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index 63851da646..7b06de7195 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -492,6 +492,13 @@ mod pallet { pub(super) type HeadReceiptNumber = StorageMap<_, Identity, DomainId, DomainBlockNumberFor, ValueQuery>; + /// Whether the head receipt have extended in the current consensus block + /// + /// Temporary storage only exist during block execution + #[pallet::storage] + pub(super) type HeadReceiptExtended = + StorageMap<_, Identity, DomainId, bool, ValueQuery>; + /// State root mapped again each domain (block, hash) /// This acts as an index for other protocols like XDM to fetch state roots faster. #[pallet::storage] @@ -1223,6 +1230,7 @@ mod pallet { fn on_finalize(_: BlockNumberFor) { let _ = LastEpochStakingDistribution::::clear(u32::MAX, None); + let _ = HeadReceiptExtended::::clear(u32::MAX, None); } } diff --git a/crates/pallet-domains/src/tests.rs b/crates/pallet-domains/src/tests.rs index 4ce5ccbba4..d6d9cda0a2 100644 --- a/crates/pallet-domains/src/tests.rs +++ b/crates/pallet-domains/src/tests.rs @@ -34,8 +34,10 @@ use sp_domains_fraud_proof::{ FraudProofExtension, FraudProofHostFunctions, FraudProofVerificationInfoRequest, FraudProofVerificationInfoResponse, SetCodeExtrinsic, }; -use sp_runtime::traits::{AccountIdConversion, BlakeTwo256, Hash as HashT, IdentityLookup, Zero}; -use sp_runtime::{BuildStorage, Digest, OpaqueExtrinsic}; +use sp_runtime::traits::{ + AccountIdConversion, BlakeTwo256, BlockNumberProvider, Hash as HashT, IdentityLookup, One, Zero, +}; +use sp_runtime::{BuildStorage, Digest, OpaqueExtrinsic, Saturating}; use sp_state_machine::backend::AsTrieBackend; use sp_state_machine::{prove_read, Backend, TrieBackendBuilder}; use sp_std::sync::Arc; @@ -446,10 +448,16 @@ impl sp_core::traits::ReadRuntimeVersion for ReadRuntimeVersion { } pub(crate) fn run_to_block(block_number: BlockNumberFor, parent_hash: T::Hash) { + // Finalize previous block + as Hooks>>::on_finalize( + block_number.saturating_sub(One::one()), + ); + frame_system::Pallet::::finalize(); + + // Initialize current block frame_system::Pallet::::set_block_number(block_number); frame_system::Pallet::::initialize(&block_number, &parent_hash, &Default::default()); as Hooks>>::on_initialize(block_number); - frame_system::Pallet::::finalize(); } pub(crate) fn register_genesis_domain(creator: u64, operator_ids: Vec) -> DomainId { @@ -502,20 +510,31 @@ pub(crate) fn register_genesis_domain(creator: u64, operator_ids: Vec, +) -> ExecutionReceiptOf { + let genesis_receipt = get_block_tree_node_at::(domain_id, 0) + .unwrap() + .execution_receipt; + extend_block_tree(domain_id, operator_id, to, genesis_receipt) +} + // Submit new head receipt to extend the block tree pub(crate) fn extend_block_tree( domain_id: DomainId, operator_id: u64, to: DomainBlockNumberFor, + mut latest_receipt: ExecutionReceiptOf, ) -> ExecutionReceiptOf { - let head_receipt_number = HeadReceiptNumber::::get(domain_id); - assert!(head_receipt_number < to); + let current_block_number = frame_system::Pallet::::current_block_number(); + assert!(current_block_number < to as u64); - let head_node = get_block_tree_node_at::(domain_id, head_receipt_number).unwrap(); - let mut receipt = head_node.execution_receipt; - for block_number in (head_receipt_number as u64 + 1)..=to as u64 { + for block_number in (current_block_number + 1)..to as u64 { // Finilize parent block and initialize block at `block_number` - run_to_block::(block_number, receipt.consensus_block_hash); + run_to_block::(block_number, latest_receipt.consensus_block_hash); // Submit a bundle with the receipt of the last block let bundle_extrinsics_root = H256::random(); @@ -523,7 +542,7 @@ pub(crate) fn extend_block_tree( domain_id, operator_id, bundle_extrinsics_root, - receipt, + latest_receipt, ); assert_ok!(crate::Pallet::::submit_bundle( RawOrigin::None.into(), @@ -534,7 +553,7 @@ pub(crate) fn extend_block_tree( let head_receipt_number = HeadReceiptNumber::::get(domain_id); let parent_block_tree_node = get_block_tree_node_at::(domain_id, head_receipt_number).unwrap(); - receipt = create_dummy_receipt( + latest_receipt = create_dummy_receipt( block_number, H256::random(), parent_block_tree_node @@ -544,7 +563,10 @@ pub(crate) fn extend_block_tree( ); } - receipt + // Finilize parent block and initialize block at `to` + run_to_block::(to as u64, latest_receipt.consensus_block_hash); + + latest_receipt } #[allow(clippy::type_complexity)] @@ -825,7 +847,7 @@ fn test_invalid_fraud_proof() { let mut ext = new_test_ext_with_extensions(); ext.execute_with(|| { let domain_id = register_genesis_domain(creator, vec![operator_id]); - extend_block_tree(domain_id, operator_id, head_domain_number + 1); + extend_block_tree_from_zero(domain_id, operator_id, head_domain_number + 2); assert_eq!( HeadReceiptNumber::::get(domain_id), head_domain_number @@ -861,7 +883,7 @@ fn test_invalid_total_rewards_fraud_proof() { let mut ext = new_test_ext_with_extensions(); ext.execute_with(|| { let domain_id = register_genesis_domain(creator, vec![operator_id]); - extend_block_tree(domain_id, operator_id, head_domain_number + 1); + extend_block_tree_from_zero(domain_id, operator_id, head_domain_number + 2); assert_eq!( HeadReceiptNumber::::get(domain_id), head_domain_number @@ -931,7 +953,7 @@ fn test_invalid_domain_extrinsic_root_proof() { let mut ext = new_test_ext_with_extensions(); let fraud_proof = ext.execute_with(|| { let domain_id = register_genesis_domain(creator, vec![operator_id]); - extend_block_tree(domain_id, operator_id, head_domain_number + 1); + extend_block_tree_from_zero(domain_id, operator_id, head_domain_number + 2); assert_eq!( HeadReceiptNumber::::get(domain_id), head_domain_number @@ -1004,7 +1026,7 @@ fn test_true_invalid_bundles_inherent_extrinsic_proof() { let mut ext = new_test_ext_with_extensions(); let fraud_proof = ext.execute_with(|| { let domain_id = register_genesis_domain(creator, vec![operator_id]); - extend_block_tree(domain_id, operator_id, head_domain_number + 1); + extend_block_tree_from_zero(domain_id, operator_id, head_domain_number + 2); assert_eq!( HeadReceiptNumber::::get(domain_id), head_domain_number @@ -1064,7 +1086,7 @@ fn test_false_invalid_bundles_inherent_extrinsic_proof() { let mut ext = new_test_ext_with_extensions(); let fraud_proof = ext.execute_with(|| { let domain_id = register_genesis_domain(creator, vec![operator_id]); - extend_block_tree(domain_id, operator_id, head_domain_number + 1); + extend_block_tree_from_zero(domain_id, operator_id, head_domain_number + 2); assert_eq!( HeadReceiptNumber::::get(domain_id), head_domain_number @@ -1150,7 +1172,7 @@ fn test_invalid_domain_block_hash_fraud_proof() { let mut ext = new_test_ext_with_extensions(); ext.execute_with(|| { let domain_id = register_genesis_domain(creator, vec![operator_id]); - extend_block_tree(domain_id, operator_id, head_domain_number + 1); + extend_block_tree_from_zero(domain_id, operator_id, head_domain_number + 2); assert_eq!( HeadReceiptNumber::::get(domain_id), head_domain_number @@ -1206,7 +1228,7 @@ fn test_basic_fraud_proof_processing() { let mut ext = new_test_ext_with_extensions(); ext.execute_with(|| { let domain_id = register_genesis_domain(creator, vec![operator_id]); - extend_block_tree(domain_id, operator_id, head_domain_number + 1); + extend_block_tree_from_zero(domain_id, operator_id, head_domain_number + 2); assert_eq!( HeadReceiptNumber::::get(domain_id), head_domain_number From 67683236ca7a358d8a1fce6459069ddb65995856 Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 3 Nov 2023 18:19:32 +0800 Subject: [PATCH 4/4] Apply review suggestion Signed-off-by: linning --- crates/pallet-domains/src/block_tree.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/pallet-domains/src/block_tree.rs b/crates/pallet-domains/src/block_tree.rs index 894590cc6b..60507e188c 100644 --- a/crates/pallet-domains/src/block_tree.rs +++ b/crates/pallet-domains/src/block_tree.rs @@ -87,7 +87,7 @@ pub(crate) enum ReceiptType { Rejected(RejectedReceiptType), } -pub(crate) fn is_receipt_exist( +pub(crate) fn does_receipt_exists( domain_id: DomainId, domain_number: DomainBlockNumberFor, receipt_hash: ReceiptHashFor, @@ -104,8 +104,9 @@ pub(crate) fn execution_receipt_type( ) -> ReceiptType { let receipt_number = execution_receipt.domain_block_number; let head_receipt_number = HeadReceiptNumber::::get(domain_id); + let next_receipt_number = head_receipt_number.saturating_add(One::one()); - match receipt_number.cmp(&head_receipt_number.saturating_add(One::one())) { + match receipt_number.cmp(&next_receipt_number) { Ordering::Greater => ReceiptType::Rejected(RejectedReceiptType::InFuture), Ordering::Equal => ReceiptType::Accepted(AcceptedReceiptType::NewHead), Ordering::Less => { @@ -117,7 +118,7 @@ pub(crate) fn execution_receipt_type( } // Reject receipt that try to create new branch in the block tree - let already_exist = is_receipt_exist::( + let already_exist = does_receipt_exists::( domain_id, receipt_number, execution_receipt.hash::>(), @@ -161,7 +162,7 @@ pub(crate) fn verify_execution_receipt( // The genesis receipt is generated and added to the block tree by the runtime upon domain // instantiation, thus it is unchallengeable and must always be the same. ensure!( - is_receipt_exist::( + does_receipt_exists::( domain_id, *domain_block_number, execution_receipt.hash::>(), @@ -222,7 +223,7 @@ pub(crate) fn verify_execution_receipt( } if let Some(parent_block_number) = domain_block_number.checked_sub(&One::one()) { - let parent_block_exist = is_receipt_exist::( + let parent_block_exist = does_receipt_exists::( domain_id, parent_block_number, *parent_domain_block_receipt_hash,