update comments, optimisations, and code reverts

Author: vedhavyas

crates/pallet-domains/src/block_tree.rs

@@ -372,16 +372,15 @@ pub(crate) fn process_execution_receipt<T: Config>(
                 // Collect the paid bundle storage fees and the invalid bundle author
                 let mut paid_bundle_storage_fees = BTreeMap::new();
                 let mut invalid_bundle_authors = Vec::new();
-                let bundle_digests = ExecutionInbox::<T>::get((
-                    domain_id,
-                    to_prune,
-                    execution_receipt.consensus_block_number(),
-                ));
+                let consensus_block_number = *execution_receipt.consensus_block_number();
+                let bundle_digests =
+                    ExecutionInbox::<T>::get((domain_id, to_prune, consensus_block_number));
+                let inboxed_bundles = execution_receipt.inboxed_bundles();
                 for (index, bd) in bundle_digests.into_iter().enumerate() {
                     if let Some(bundle_author) = InboxedBundleAuthor::<T>::take(bd.header_hash) {
                         // It is okay to index `ER::bundles` here since `verify_execution_receipt` have checked
                         // the `ER::bundles` have the same length of `ExecutionInbox`
-                        if execution_receipt.inboxed_bundles()[index].is_invalid() {
+                        if inboxed_bundles[index].is_invalid() {
                             invalid_bundle_authors.push(bundle_author);
                         } else {
                             paid_bundle_storage_fees
@@ -401,10 +400,7 @@ pub(crate) fn process_execution_receipt<T: Config>(
                     execution_receipt.clone(),
                 );
 
-                ConsensusBlockHash::<T>::remove(
-                    domain_id,
-                    execution_receipt.consensus_block_number(),
-                );
+                ConsensusBlockHash::<T>::remove(domain_id, consensus_block_number);
 
                 let block_fees = execution_receipt
                     .block_fees()
@@ -430,10 +426,7 @@ pub(crate) fn process_execution_receipt<T: Config>(
                     .map_err(|_| Error::DomainTransfersTracking)?;
                 }
 
-                update_domain_runtime_upgrade_records::<T>(
-                    domain_id,
-                    *execution_receipt.consensus_block_number(),
-                )?;
+                update_domain_runtime_upgrade_records::<T>(domain_id, consensus_block_number)?;
 
                 // handle chain rewards from the domain
                 execution_receipt
@@ -445,7 +438,7 @@ pub(crate) fn process_execution_receipt<T: Config>(
                     });
 
                 return Ok(Some(ConfirmedDomainBlockInfo {
-                    consensus_block_number: *execution_receipt.consensus_block_number(),
+                    consensus_block_number,
                     domain_block_number: to_prune,
                     operator_ids,
                     rewards: execution_receipt.block_fees().domain_execution_fee,

crates/pallet-domains/src/lib.rs

@@ -2403,6 +2403,8 @@ impl<T: Config> Pallet<T> {
         Ok(())
     }
 
+    /// Verifies if the submitted ER version matches with the version
+    /// defined at the block number ER is derived from.
     fn check_execution_receipt_version(
         er_derived_consensus_number: BlockNumberFor<T>,
         receipt_version: ExecutionReceiptVersion,

crates/pallet-domains/src/tests.rs

@@ -1316,6 +1316,10 @@ fn get_mock_upgrades() -> Vec<(u32, MockBundleAndExecutionReceiptVersion, bool)>
     ]
 }
 
+/// Returns mock version queries.
+/// (block_number, current_version)
+/// block_number: Consensus block at which ER is derived
+/// current_version: Version defined at the consensus block number.
 fn get_mock_version_queries() -> Vec<(u32, MockBundleAndExecutionReceiptVersion)> {
     vec![
         // version from 0..100

crates/sp-domains-fraud-proof/src/fraud_proof.rs

@@ -315,18 +315,6 @@ impl<DomainHash> From<storage_proof::VerificationError> for VerificationError<Do
     }
 }
 
-/// Represents a valid bundle index and all the extrinsics within that bundle.
-#[derive(Clone, Debug, Decode, Encode, Eq, PartialEq, TypeInfo)]
-pub struct ValidBundleDigest {
-    /// Index of this bundle in the original list of bundles in the consensus block.
-    pub bundle_index: u32,
-    /// `Vec<(tx_signer, tx_hash)>` of all extrinsics
-    pub bundle_digest: Vec<(
-        Option<domain_runtime_primitives::opaque::AccountId>,
-        ExtrinsicDigest,
-    )>,
-}
-
 /// Fraud proof for domains.
 #[derive(Decode, Encode, TypeInfo, PartialEq, Eq, Clone)]
 pub struct FraudProof<Number, Hash, DomainHeader: HeaderT, MmrHash> {
@@ -467,6 +455,18 @@ impl<Number, Hash, MmrHash, DomainHeader: HeaderT> fmt::Debug
     }
 }
 
+/// Represents a valid bundle index and all the extrinsics within that bundle.
+#[derive(Clone, Debug, Decode, Encode, Eq, PartialEq, TypeInfo)]
+pub struct ValidBundleDigest {
+    /// Index of this bundle in the original list of bundles in the consensus block.
+    pub bundle_index: u32,
+    /// `Vec<(tx_signer, tx_hash)>` of all extrinsics
+    pub bundle_digest: Vec<(
+        Option<domain_runtime_primitives::opaque::AccountId>,
+        ExtrinsicDigest,
+    )>,
+}
+
 // Domain runtime code at a specific block
 #[derive(Debug, Decode, Encode, TypeInfo, PartialEq, Eq, Clone)]
 pub struct DomainRuntimeCodeAt<Number, Hash, MmrHash> {

crates/sp-domains-fraud-proof/src/verification.rs

@@ -384,8 +384,8 @@ where
     Ok(())
 }
 
-/// Verifies invalid transfers fraud proof.
-pub fn verify_invalid_transfers_fraud_proof<
+/// Verifies invalid block fees fraud proof.
+pub fn verify_invalid_block_fees_fraud_proof<
     CBlock,
     DomainNumber,
     DomainHash,
@@ -405,37 +405,38 @@ pub fn verify_invalid_transfers_fraud_proof<
 where
     CBlock: BlockT,
     Balance: PartialEq + Decode + Encode + Zero + Default,
+    DomainHashing: Hasher<Out = DomainHash>,
     DomainNumber: Encode + Zero,
     DomainHash: Clone + Encode + Default + Copy,
-    DomainHashing: Hasher<Out = DomainHash>,
 {
     let storage_key = fraud_proof_runtime_interface::domain_storage_key(
         domain_runtime_code,
-        DomainStorageKeyRequest::Transfers,
+        DomainStorageKeyRequest::BlockFees,
     )
     .ok_or(VerificationError::FailedToGetDomainStorageKey)?;
 
-    let transfers = StorageProofVerifier::<DomainHashing>::get_decoded_value::<Transfers<Balance>>(
-        bad_receipt.final_state_root(),
-        storage_proof.clone(),
-        StorageKey(storage_key),
-    )
-    .map_err(|err| {
-        VerificationError::StorageProof(storage_proof::VerificationError::TransfersStorageProof(
-            err,
-        ))
-    })?;
+    let block_fees =
+        StorageProofVerifier::<DomainHashing>::get_decoded_value::<BlockFees<Balance>>(
+            bad_receipt.final_state_root(),
+            storage_proof.clone(),
+            StorageKey(storage_key),
+        )
+        .map_err(|err| {
+            VerificationError::StorageProof(
+                storage_proof::VerificationError::BlockFeesStorageProof(err),
+            )
+        })?;
 
     // if the rewards matches, then this is an invalid fraud proof since rewards must be different.
-    if bad_receipt.transfers() == &transfers {
+    if bad_receipt.block_fees() == &block_fees {
         return Err(VerificationError::InvalidProof);
     }
 
     Ok(())
 }
 
-/// Verifies invalid block fees fraud proof.
-pub fn verify_invalid_block_fees_fraud_proof<
+/// Verifies invalid transfers fraud proof.
+pub fn verify_invalid_transfers_fraud_proof<
     CBlock,
     DomainNumber,
     DomainHash,
@@ -455,30 +456,29 @@ pub fn verify_invalid_block_fees_fraud_proof<
 where
     CBlock: BlockT,
     Balance: PartialEq + Decode + Encode + Zero + Default,
-    DomainHashing: Hasher<Out = DomainHash>,
     DomainNumber: Encode + Zero,
     DomainHash: Clone + Encode + Default + Copy,
+    DomainHashing: Hasher<Out = DomainHash>,
 {
     let storage_key = fraud_proof_runtime_interface::domain_storage_key(
         domain_runtime_code,
-        DomainStorageKeyRequest::BlockFees,
+        DomainStorageKeyRequest::Transfers,
     )
     .ok_or(VerificationError::FailedToGetDomainStorageKey)?;
 
-    let block_fees =
-        StorageProofVerifier::<DomainHashing>::get_decoded_value::<BlockFees<Balance>>(
-            bad_receipt.final_state_root(),
-            storage_proof.clone(),
-            StorageKey(storage_key),
-        )
-        .map_err(|err| {
-            VerificationError::StorageProof(
-                storage_proof::VerificationError::BlockFeesStorageProof(err),
-            )
-        })?;
+    let transfers = StorageProofVerifier::<DomainHashing>::get_decoded_value::<Transfers<Balance>>(
+        bad_receipt.final_state_root(),
+        storage_proof.clone(),
+        StorageKey(storage_key),
+    )
+    .map_err(|err| {
+        VerificationError::StorageProof(storage_proof::VerificationError::TransfersStorageProof(
+            err,
+        ))
+    })?;
 
     // if the rewards matches, then this is an invalid fraud proof since rewards must be different.
-    if bad_receipt.block_fees() == &block_fees {
+    if bad_receipt.transfers() == &transfers {
         return Err(VerificationError::InvalidProof);
     }
 

crates/sp-domains/src/bundle/bundle_v0.rs

@@ -63,7 +63,7 @@ impl<Number: Encode, Hash: Encode, DomainHeader: HeaderT, Balance: Encode>
     }
 }
 
-/// Domain bundle v1.
+/// Domain bundle v0.
 #[derive(Debug, Decode, Encode, TypeInfo, PartialEq, Eq, Clone)]
 pub struct BundleV0<Extrinsic, Number, Hash, DomainHeader: HeaderT, Balance> {
     /// Sealed bundle header.