Skip to content

Commit

Permalink
Apply review suggestion
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 18, 2023
1 parent a3e256d commit bd1398b
Show file tree
Hide file tree
Showing 14 changed files with 119 additions and 81 deletions.
2 changes: 1 addition & 1 deletion crates/pallet-domains/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ mod benchmarks {
let mut receipt =
BlockTree::<T>::get::<_, DomainBlockNumberFor<T>>(domain_id, Zero::zero())
.first()
.and_then(DomainBlocks::<T>::get)
.and_then(BlockTreeNodes::<T>::get)
.expect("genesis receipt must exist")
.execution_receipt;
for i in 1..=(block_tree_pruning_depth + 1) {
Expand Down
37 changes: 19 additions & 18 deletions crates/pallet-domains/src/block_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
use crate::pallet::StateRoots;
use crate::{
BalanceOf, BlockTree, Config, ConsensusBlockHash, DomainBlockDescendants, DomainBlockNumberFor,
DomainBlocks, ExecutionInbox, ExecutionReceiptOf, HeadReceiptNumber, InboxedBundleAuthor,
BalanceOf, BlockTree, BlockTreeNodes, Config, ConsensusBlockHash, DomainBlockDescendants,
DomainBlockNumberFor, ExecutionInbox, ExecutionReceiptOf, HeadReceiptNumber,
InboxedBundleAuthor,
};
use codec::{Decode, Encode};
use frame_support::{ensure, PalletError};
Expand Down Expand Up @@ -280,7 +281,7 @@ pub(crate) fn process_execution_receipt<T: Config>(
let BlockTreeNode {
execution_receipt,
operator_ids,
} = DomainBlocks::<T>::take(receipt_hash).ok_or(Error::MissingDomainBlock)?;
} = BlockTreeNodes::<T>::take(receipt_hash).ok_or(Error::MissingDomainBlock)?;

_ = StateRoots::<T>::take((
domain_id,
Expand Down Expand Up @@ -326,11 +327,11 @@ pub(crate) fn process_execution_receipt<T: Config>(
AcceptedReceiptType::CurrentHead => {
// Add confirmation to the current head receipt
let er_hash = execution_receipt.hash();
DomainBlocks::<T>::mutate(er_hash, |maybe_domain_block| {
let domain_block = maybe_domain_block.as_mut().expect(
BlockTreeNodes::<T>::mutate(er_hash, |maybe_node| {
let node = maybe_node.as_mut().expect(
"The domain block of `CurrentHead` receipt is checked to be exist in `execution_receipt_type`; qed"
);
domain_block.operator_ids.push(submitter);
node.operator_ids.push(submitter);
});
}
}
Expand Down Expand Up @@ -363,11 +364,11 @@ fn add_new_receipt_to_block_tree<T: Config>(
er_hashes.insert(er_hash);
},
);
let domain_block = BlockTreeNode {
let block_tree_node = BlockTreeNode {
execution_receipt,
operator_ids: sp_std::vec![submitter],
};
DomainBlocks::<T>::insert(er_hash, domain_block);
BlockTreeNodes::<T>::insert(er_hash, block_tree_node);
}

/// Import the genesis receipt to the block tree
Expand All @@ -377,7 +378,7 @@ pub(crate) fn import_genesis_receipt<T: Config>(
) {
let er_hash = genesis_receipt.hash();
let domain_block_number = genesis_receipt.domain_block_number;
let domain_block = BlockTreeNode {
let block_tree_node = BlockTreeNode {
execution_receipt: genesis_receipt,
operator_ids: sp_std::vec![],
};
Expand All @@ -389,11 +390,11 @@ pub(crate) fn import_genesis_receipt<T: Config>(
(
domain_id,
domain_block_number,
domain_block.execution_receipt.domain_block_hash,
block_tree_node.execution_receipt.domain_block_hash,
),
domain_block.execution_receipt.final_state_root,
block_tree_node.execution_receipt.final_state_root,
);
DomainBlocks::<T>::insert(er_hash, domain_block);
BlockTreeNodes::<T>::insert(er_hash, block_tree_node);
}

#[cfg(test)]
Expand Down Expand Up @@ -421,7 +422,7 @@ mod tests {
assert_eq!(block_tree_node_at_0.len(), 1);

let genesis_node =
DomainBlocks::<Test>::get(block_tree_node_at_0.first().unwrap()).unwrap();
BlockTreeNodes::<Test>::get(block_tree_node_at_0.first().unwrap()).unwrap();
assert!(genesis_node.operator_ids.is_empty());
assert_eq!(HeadReceiptNumber::<Test>::get(domain_id), 0);

Expand Down Expand Up @@ -516,7 +517,7 @@ mod tests {

// The submitter should be added to `operator_ids`
let parent_domain_block_receipt = parent_block_tree_nodes.first().unwrap();
let parent_node = DomainBlocks::<Test>::get(parent_domain_block_receipt).unwrap();
let parent_node = BlockTreeNodes::<Test>::get(parent_domain_block_receipt).unwrap();
assert_eq!(parent_node.operator_ids.len(), 1);
assert_eq!(parent_node.operator_ids[0], operator_id);

Expand Down Expand Up @@ -641,7 +642,7 @@ mod tests {
));

assert_eq!(
DomainBlocks::<Test>::get(stale_receipt_hash)
BlockTreeNodes::<Test>::get(stale_receipt_hash)
.unwrap()
.operator_ids,
vec![operator_id1]
Expand Down Expand Up @@ -702,11 +703,11 @@ mod tests {
let nodes = BlockTree::<Test>::get(domain_id, head_receipt_number);
assert_eq!(nodes.len(), 2);
for n in nodes.iter() {
let block = DomainBlocks::<Test>::get(n).unwrap();
let node = BlockTreeNodes::<Test>::get(n).unwrap();
if *n == new_branch_receipt_hash {
assert_eq!(block.operator_ids, vec![operator_id2]);
assert_eq!(node.operator_ids, vec![operator_id2]);
} else {
assert_eq!(block.operator_ids, vec![operator_id1]);
assert_eq!(node.operator_ids, vec![operator_id1]);
}
}
});
Expand Down
20 changes: 10 additions & 10 deletions crates/pallet-domains/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,8 +460,8 @@ mod pallet {
OptionQuery,
>;

/// The domain block tree, map (`domain_id`, `domain_block_number`) to the hash of a domain blocks,
/// which can be used get the domain block in `DomainBlocks`
/// The domain block tree, map (`domain_id`, `domain_block_number`) to the hash of ER,
/// which can be used get the block tree node in `BlockTreeNodes`
#[pallet::storage]
pub(super) type BlockTree<T: Config> = StorageDoubleMap<
_,
Expand All @@ -473,9 +473,9 @@ mod pallet {
ValueQuery,
>;

/// Mapping of domain block hash to domain block
/// Mapping of block tree node hash to the node, each node represent a domain block
#[pallet::storage]
pub(super) type DomainBlocks<T: Config> = StorageMap<
pub(super) type BlockTreeNodes<T: Config> = StorageMap<
_,
Identity,
ReceiptHash,
Expand Down Expand Up @@ -904,7 +904,7 @@ mod pallet {
let BlockTreeNode {
execution_receipt,
operator_ids,
} = DomainBlocks::<T>::take(receipt_hash)
} = BlockTreeNodes::<T>::take(receipt_hash)
.ok_or::<Error<T>>(FraudProofError::BadReceiptNotFound.into())?;

BlockTree::<T>::mutate_exists(
Expand Down Expand Up @@ -1379,7 +1379,7 @@ impl<T: Config> Pallet<T> {
pub fn genesis_state_root(domain_id: DomainId) -> Option<H256> {
BlockTree::<T>::get(domain_id, DomainBlockNumberFor::<T>::zero())
.first()
.and_then(DomainBlocks::<T>::get)
.and_then(BlockTreeNodes::<T>::get)
.map(|block| block.execution_receipt.final_state_root.into())
}

Expand Down Expand Up @@ -1532,7 +1532,7 @@ impl<T: Config> Pallet<T> {
fn validate_fraud_proof(
fraud_proof: &FraudProof<BlockNumberFor<T>, T::Hash>,
) -> Result<(), FraudProofError> {
let bad_receipt = DomainBlocks::<T>::get(fraud_proof.bad_receipt_hash())
let bad_receipt = BlockTreeNodes::<T>::get(fraud_proof.bad_receipt_hash())
.ok_or(FraudProofError::BadReceiptNotFound)?
.execution_receipt;

Expand All @@ -1557,7 +1557,7 @@ impl<T: Config> Pallet<T> {
..
}) => {
let parent_receipt =
DomainBlocks::<T>::get(bad_receipt.parent_domain_block_receipt_hash)
BlockTreeNodes::<T>::get(bad_receipt.parent_domain_block_receipt_hash)
.ok_or(FraudProofError::ParentReceiptNotFound)?
.execution_receipt;
verify_invalid_domain_block_hash_fraud_proof::<
Expand Down Expand Up @@ -1614,7 +1614,7 @@ impl<T: Config> Pallet<T> {
}
FraudProof::InvalidStateTransition(proof) => {
let bad_receipt_parent =
DomainBlocks::<T>::get(bad_receipt.parent_domain_block_receipt_hash)
BlockTreeNodes::<T>::get(bad_receipt.parent_domain_block_receipt_hash)
.ok_or(FraudProofError::BadReceiptParentNotFound)?
.execution_receipt;

Expand Down Expand Up @@ -1803,7 +1803,7 @@ impl<T: Config> Pallet<T> {
}

pub fn execution_receipt(receipt_hash: ReceiptHash) -> Option<ExecutionReceiptOf<T>> {
DomainBlocks::<T>::get(receipt_hash).map(|db| db.execution_receipt)
BlockTreeNodes::<T>::get(receipt_hash).map(|db| db.execution_receipt)
}
}

Expand Down
16 changes: 8 additions & 8 deletions crates/pallet-domains/src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::block_tree::BlockTreeNode;
use crate::domain_registry::{DomainConfig, DomainObject};
use crate::{
self as pallet_domains, BalanceOf, BlockTree, BundleError, Config, ConsensusBlockHash,
DomainBlockNumberFor, DomainBlocks, DomainRegistry, ExecutionInbox, ExecutionReceiptOf,
self as pallet_domains, BalanceOf, BlockTree, BlockTreeNodes, BundleError, Config,
ConsensusBlockHash, DomainBlockNumberFor, DomainRegistry, ExecutionInbox, ExecutionReceiptOf,
FraudProofError, FungibleHoldId, HeadReceiptNumber, NextDomainId, Operator, OperatorStatus,
Operators,
};
Expand Down Expand Up @@ -512,7 +512,7 @@ pub(crate) fn get_block_tree_node_at<T: Config>(
> {
BlockTree::<T>::get(domain_id, block_number)
.first()
.and_then(DomainBlocks::<T>::get)
.and_then(BlockTreeNodes::<T>::get)
}

// TODO: Unblock once bundle producer election v2 is finished.
Expand Down Expand Up @@ -836,7 +836,7 @@ fn test_invalid_total_rewards_fraud_proof() {
domain_block.execution_receipt.total_rewards + 1,
);
domain_block.execution_receipt.final_state_root = root;
DomainBlocks::<Test>::insert(bad_receipt_hash, domain_block);
BlockTreeNodes::<Test>::insert(bad_receipt_hash, domain_block);
assert_ok!(Domains::validate_fraud_proof(&fraud_proof),);
});
}
Expand Down Expand Up @@ -915,7 +915,7 @@ fn test_invalid_domain_extrinsic_root_proof() {
bad_receipt.consensus_block_hash,
);
ConsensusBlockHash::<Test>::insert(domain_id, consensus_block_number, consensus_block_hash);
DomainBlocks::<Test>::insert(bad_receipt_hash, domain_block);
BlockTreeNodes::<Test>::insert(bad_receipt_hash, domain_block);
fraud_proof
});

Expand Down Expand Up @@ -967,7 +967,7 @@ fn test_invalid_domain_block_hash_fraud_proof() {
domain_block.execution_receipt.final_state_root = root;
domain_block.execution_receipt.domain_block_hash = H256::random();
let bad_receipt_hash = domain_block.execution_receipt.hash();
DomainBlocks::<Test>::insert(bad_receipt_hash, domain_block);
BlockTreeNodes::<Test>::insert(bad_receipt_hash, domain_block);
let fraud_proof = FraudProof::InvalidDomainBlockHash(InvalidDomainBlockHashProof {
domain_id,
bad_receipt_hash,
Expand Down Expand Up @@ -1091,7 +1091,7 @@ fn test_fraud_proof_prune_fraudulent_branch_of_receipt() {
bad_receipts.push(bad_receipt_hash);

assert_eq!(BlockTree::<Test>::get(domain_id, bad_receipt_at).len(), 2);
assert!(DomainBlocks::<Test>::get(bad_receipt_hash).is_some());
assert!(BlockTreeNodes::<Test>::get(bad_receipt_hash).is_some());
}

// Construct and submit a fraud proof for the fraudulent branch of ER
Expand All @@ -1110,7 +1110,7 @@ fn test_fraud_proof_prune_fraudulent_branch_of_receipt() {
for i in 0..3 {
let bad_receipt_at = bad_receipt_start_at + i;
assert_eq!(BlockTree::<Test>::get(domain_id, bad_receipt_at).len(), 1);
assert!(DomainBlocks::<Test>::get(bad_receipts[i as usize]).is_none());
assert!(BlockTreeNodes::<Test>::get(bad_receipts[i as usize]).is_none());
}
});
}
7 changes: 4 additions & 3 deletions crates/sp-domains-fraud-proof/src/host_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use sp_blockchain::HeaderBackend;
use sp_core::traits::{CodeExecutor, FetchRuntimeCode, RuntimeCode};
use sp_core::H256;
use sp_domains::{DomainId, DomainsApi};
use sp_runtime::traits::{BlakeTwo256, Header as HeaderT, NumberFor};
use sp_runtime::traits::{Header as HeaderT, NumberFor};
use sp_trie::StorageProof;
use std::borrow::Cow;
use std::marker::PhantomData;
Expand Down Expand Up @@ -144,6 +144,7 @@ where
Block: BlockT,
Block::Hash: From<H256>,
DomainBlock: BlockT,
DomainBlock::Hash: From<H256>,
Client: HeaderBackend<Block> + ProvideRuntimeApi<Block>,
Client::Api: DomainsApi<Block, NumberFor<DomainBlock>, DomainBlock::Hash>,
Executor: CodeExecutor + RuntimeVersionOf,
Expand Down Expand Up @@ -192,8 +193,8 @@ where
heap_pages: None,
};

sp_state_machine::execution_proof_check::<BlakeTwo256, _>(
pre_state_root,
sp_state_machine::execution_proof_check::<<DomainBlock::Header as HeaderT>::Hashing, _>(
pre_state_root.into(),
proof,
&mut Default::default(),
self.executor.as_ref(),
Expand Down
Loading

0 comments on commit bd1398b

Please sign in to comment.