Merge pull request #2431 from subspace/oldest-er-number

Correctly derive the oldest receipt number

Author: NingLin-P

Cargo.lock

@@ -86,7 +86,10 @@ mod benchmarks {
             Domains::<T>::head_receipt_number(domain_id),
             (block_tree_pruning_depth + 1).into()
         );
-        assert_eq!(Domains::<T>::oldest_receipt_number(domain_id), 1u32.into());
+        assert_eq!(
+            Domains::<T>::oldest_unconfirmed_receipt_number(domain_id),
+            Some(1u32.into())
+        );
     }
 
     /// Benchmark pending staking operation with the worst possible conditions:

crates/pallet-domains/src/benchmarking.rs

@@ -4,7 +4,7 @@ use crate::pallet::StateRoots;
 use crate::{
     BalanceOf, BlockTree, BlockTreeNodes, Config, ConsensusBlockHash, DomainBlockNumberFor,
     DomainHashingFor, ExecutionInbox, ExecutionReceiptOf, HeadReceiptExtended, HeadReceiptNumber,
-    InboxedBundleAuthor, ReceiptHashFor,
+    InboxedBundleAuthor, LatestConfirmedDomainBlockNumber, ReceiptHashFor,
 };
 use codec::{Decode, Encode};
 use frame_support::{ensure, PalletError};
@@ -107,15 +107,17 @@ pub(crate) fn execution_receipt_type<T: Config>(
     let receipt_number = execution_receipt.domain_block_number;
     let head_receipt_number = HeadReceiptNumber::<T>::get(domain_id);
     let next_receipt_number = head_receipt_number.saturating_add(One::one());
+    let latest_confirmed_domain_block_number =
+        LatestConfirmedDomainBlockNumber::<T>::get(domain_id);
 
     match receipt_number.cmp(&next_receipt_number) {
         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 {
+            // Reject receipt that already confirmed
+            if !latest_confirmed_domain_block_number.is_zero()
+                && receipt_number <= latest_confirmed_domain_block_number
+            {
                 return ReceiptType::Rejected(RejectedReceiptType::Pruned);
             }
 

crates/pallet-domains/src/block_tree.rs

@@ -59,7 +59,7 @@ use sp_domains_fraud_proof::verification::{
     verify_invalid_domain_extrinsics_root_fraud_proof, verify_invalid_state_transition_fraud_proof,
     verify_valid_bundle_fraud_proof,
 };
-use sp_runtime::traits::{CheckedSub, Hash, Header, One, Zero};
+use sp_runtime::traits::{Hash, Header, One, Zero};
 use sp_runtime::{RuntimeAppPublic, SaturatedConversion, Saturating};
 use sp_std::boxed::Box;
 use sp_std::collections::btree_map::BTreeMap;
@@ -1860,16 +1860,23 @@ impl<T: Config> Pallet<T> {
         HeadReceiptNumber::<T>::get(domain_id)
     }
 
-    /// Returns the block number of oldest execution receipt.
-    // FIXME: the `oldest_receipt_number` may not be correct if fraud proof is submitted
-    // and bad ER were pruned, see https://github.com/subspace/subspace/issues/2354
-    pub fn oldest_receipt_number(domain_id: DomainId) -> DomainBlockNumberFor<T> {
-        Self::head_receipt_number(domain_id).saturating_sub(Self::block_tree_pruning_depth())
-    }
-
-    /// Returns the block tree pruning depth.
-    pub fn block_tree_pruning_depth() -> DomainBlockNumberFor<T> {
-        T::BlockTreePruningDepth::get()
+    /// Returns the block number of the oldest existing unconfirmed execution receipt, return `None`
+    /// means there is no unconfirmed ER exist or submitted yet.
+    pub fn oldest_unconfirmed_receipt_number(
+        domain_id: DomainId,
+    ) -> Option<DomainBlockNumberFor<T>> {
+        let oldest_nonconfirmed_er_number =
+            LatestConfirmedDomainBlockNumber::<T>::get(domain_id).saturating_add(One::one());
+
+        if BlockTree::<T>::get(domain_id, oldest_nonconfirmed_er_number).is_some() {
+            Some(oldest_nonconfirmed_er_number)
+        } else {
+            // The `oldest_nonconfirmed_er_number` ER may not exist if
+            // - The domain just started and no ER submitted yet
+            // - The oldest ER just pruned by fraud proof and no new ER submitted yet
+            // - When using consensus block to derive the challenge period forward (unimplemented yet)
+            None
+        }
     }
 
     /// Returns the domain block limit of the given domain.
@@ -1887,10 +1894,10 @@ impl<T: Config> Pallet<T> {
             return true;
         }
 
+        // Start from the oldest non-confirmed ER to the head domain number
+        let mut to_check =
+            LatestConfirmedDomainBlockNumber::<T>::get(domain_id).saturating_add(One::one());
         let head_number = HeadDomainNumber::<T>::get(domain_id);
-        let mut to_check = head_number
-            .checked_sub(&T::BlockTreePruningDepth::get())
-            .unwrap_or(Zero::zero());
 
         while to_check <= head_number {
             if !ExecutionInbox::<T>::iter_prefix_values((domain_id, to_check)).all(|digests| {

crates/pallet-domains/src/lib.rs

@@ -51,6 +51,7 @@ libsecp256k1 = { version = "0.7.1", features = ["static-context", "hmac"] }
 pallet-balances = { version = "4.0.0-dev", git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8" }
 pallet-ethereum = { version = "4.0.0-dev", git = "https://github.com/subspace/frontier", rev = "7627e61d80275a4cf24d06f27491f6c31eadb7b7", features = ['default'] }
 pallet-evm = { version = "6.0.0-dev", git = "https://github.com/subspace/frontier", rev = "7627e61d80275a4cf24d06f27491f6c31eadb7b7", default-features = false }
+parking_lot = "0.12.1"
 rand = { version = "0.8.5", features = ["min_const_gen"] }
 rlp = "0.5.2"
 sp-core = { version = "21.0.0", default-features = false, git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8" }
@@ -59,7 +60,6 @@ sc-service = { version = "0.10.0-dev", git = "https://github.com/subspace/polkad
 subspace-test-client = { version = "0.1.0", path = "../../test/subspace-test-client" }
 subspace-test-service = { version = "0.1.0", path = "../../test/subspace-test-service" }
 subspace-runtime-primitives = { version = "0.1.0", path = "../../crates/subspace-runtime-primitives" }
-substrate-test-runtime-client = { version = "2.0.0", git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8" }
 tempfile = "3.9.0"
 tokio = "1.35.1"
 

crates/sp-domains-fraud-proof/Cargo.toml

@@ -156,11 +156,14 @@ where
 mod test {
     use super::{check_equivocation, MAX_SLOT_CAPACITY, PRUNING_BOUND};
     use domain_runtime_primitives::opaque::Header as DomainHeader;
+    use parking_lot::Mutex;
+    use sc_client_api::backend::AuxStore;
     use sp_core::crypto::UncheckedFrom;
     use sp_domains::{
         BundleHeader, DomainId, ExecutionReceipt, OperatorId, OperatorSignature, ProofOfElection,
         SealedBundleHeader,
     };
+    use std::collections::HashMap;
     use std::sync::Arc;
     use subspace_runtime_primitives::opaque::Block;
     use subspace_runtime_primitives::{Balance, BlockNumber, Hash};
@@ -197,9 +200,39 @@ mod test {
         }
     }
 
+    #[derive(Default)]
+    struct TestClient(Mutex<HashMap<Vec<u8>, Vec<u8>>>);
+
+    impl AuxStore for TestClient {
+        fn insert_aux<
+            'a,
+            'b: 'a,
+            'c: 'a,
+            I: IntoIterator<Item = &'a (&'c [u8], &'c [u8])>,
+            D: IntoIterator<Item = &'a &'b [u8]>,
+        >(
+            &self,
+            insert: I,
+            delete: D,
+        ) -> sp_blockchain::Result<()> {
+            let mut map = self.0.lock();
+            for d in delete {
+                map.remove(&d.to_vec());
+            }
+            for (k, v) in insert {
+                map.insert(k.to_vec(), v.to_vec());
+            }
+            Ok(())
+        }
+
+        fn get_aux(&self, key: &[u8]) -> sp_blockchain::Result<Option<Vec<u8>>> {
+            Ok(self.0.lock().get(key).cloned())
+        }
+    }
+
     #[test]
     fn test_check_equivocation() {
-        let client = Arc::new(substrate_test_runtime_client::new());
+        let client = Arc::new(TestClient::default());
         let domain_id = DomainId::new(0);
         let operator_id = 1;
 

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

@@ -994,11 +994,8 @@ sp_api::decl_runtime_apis! {
         /// Returns the best execution chain number.
         fn head_receipt_number(domain_id: DomainId) -> HeaderNumberFor<DomainHeader>;
 
-        /// Returns the block number of oldest execution receipt.
-        fn oldest_receipt_number(domain_id: DomainId) -> HeaderNumberFor<DomainHeader>;
-
-        /// Returns the block tree pruning depth.
-        fn block_tree_pruning_depth() -> HeaderNumberFor<DomainHeader>;
+        /// Returns the block number of oldest unconfirmed execution receipt.
+        fn oldest_unconfirmed_receipt_number(domain_id: DomainId) -> Option<HeaderNumberFor<DomainHeader>>;
 
         /// Returns the domain block limit of the given domain.
         fn domain_block_limit(domain_id: DomainId) -> Option<DomainBlockLimit>;

crates/sp-domains/src/lib.rs

@@ -1012,12 +1012,8 @@ impl_runtime_apis! {
             Domains::head_receipt_number(domain_id)
         }
 
-        fn oldest_receipt_number(domain_id: DomainId) -> DomainNumber {
-            Domains::oldest_receipt_number(domain_id)
-        }
-
-        fn block_tree_pruning_depth() -> DomainNumber {
-            Domains::block_tree_pruning_depth()
+        fn oldest_unconfirmed_receipt_number(domain_id: DomainId) -> Option<DomainNumber> {
+            Domains::oldest_unconfirmed_receipt_number(domain_id)
         }
 
         fn domain_block_limit(domain_id: DomainId) -> Option<sp_domains::DomainBlockLimit> {

crates/subspace-runtime/src/lib.rs

@@ -55,5 +55,4 @@ sp-state-machine = { version = "0.28.0", git = "https://github.com/subspace/polk
 subspace-core-primitives = { version = "0.1.0", default-features = false, path = "../../../crates/subspace-core-primitives" }
 subspace-test-runtime = { version = "0.1.0", path = "../../../test/subspace-test-runtime" }
 subspace-test-service = { version = "0.1.0", path = "../../../test/subspace-test-service" }
-substrate-test-runtime-client = { version = "2.0.0", git = "https://github.com/subspace/polkadot-sdk", rev = "d6b500960579d73c43fc4ef550b703acfa61c4c8" }
 tempfile = "3.9.0"