diff --git a/Cargo.lock b/Cargo.lock index adcc5d94f6..3eb912fc1e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2578,7 +2578,6 @@ dependencies = [ "subspace-runtime-primitives", "subspace-test-runtime", "subspace-test-service", - "substrate-test-runtime-client", "tempfile", "thiserror", "tokio", @@ -10772,6 +10771,7 @@ dependencies = [ "pallet-ethereum", "pallet-evm", "parity-scale-codec", + "parking_lot 0.12.1", "rand", "rlp", "sc-cli", @@ -10795,7 +10795,6 @@ dependencies = [ "subspace-runtime-primitives", "subspace-test-client", "subspace-test-service", - "substrate-test-runtime-client", "tempfile", "thiserror", "tokio", diff --git a/crates/pallet-domains/src/benchmarking.rs b/crates/pallet-domains/src/benchmarking.rs index 71baace9ef..8346dbe844 100644 --- a/crates/pallet-domains/src/benchmarking.rs +++ b/crates/pallet-domains/src/benchmarking.rs @@ -86,7 +86,10 @@ mod benchmarks { Domains::::head_receipt_number(domain_id), (block_tree_pruning_depth + 1).into() ); - assert_eq!(Domains::::oldest_receipt_number(domain_id), 1u32.into()); + assert_eq!( + Domains::::oldest_unconfirmed_receipt_number(domain_id), + Some(1u32.into()) + ); } /// Benchmark pending staking operation with the worst possible conditions: diff --git a/crates/pallet-domains/src/block_tree.rs b/crates/pallet-domains/src/block_tree.rs index 03920f57a9..b262665529 100644 --- a/crates/pallet-domains/src/block_tree.rs +++ b/crates/pallet-domains/src/block_tree.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( 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()); + let latest_confirmed_domain_block_number = + LatestConfirmedDomainBlockNumber::::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); } diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index 58d5d94169..517c43313a 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.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 Pallet { HeadReceiptNumber::::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 { - 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::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> { + let oldest_nonconfirmed_er_number = + LatestConfirmedDomainBlockNumber::::get(domain_id).saturating_add(One::one()); + + if BlockTree::::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 Pallet { return true; } + // Start from the oldest non-confirmed ER to the head domain number + let mut to_check = + LatestConfirmedDomainBlockNumber::::get(domain_id).saturating_add(One::one()); let head_number = HeadDomainNumber::::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::::iter_prefix_values((domain_id, to_check)).all(|digests| { diff --git a/crates/sp-domains-fraud-proof/Cargo.toml b/crates/sp-domains-fraud-proof/Cargo.toml index 94828974e0..ff18149b98 100644 --- a/crates/sp-domains-fraud-proof/Cargo.toml +++ b/crates/sp-domains-fraud-proof/Cargo.toml @@ -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" diff --git a/crates/sp-domains-fraud-proof/src/bundle_equivocation.rs b/crates/sp-domains-fraud-proof/src/bundle_equivocation.rs index bf950b16b7..91bf913eee 100644 --- a/crates/sp-domains-fraud-proof/src/bundle_equivocation.rs +++ b/crates/sp-domains-fraud-proof/src/bundle_equivocation.rs @@ -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, Vec>>); + + impl AuxStore for TestClient { + fn insert_aux< + 'a, + 'b: 'a, + 'c: 'a, + I: IntoIterator, + D: IntoIterator, + >( + &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>> { + 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; diff --git a/crates/sp-domains/src/lib.rs b/crates/sp-domains/src/lib.rs index d5659b49f1..7e31d91e7a 100644 --- a/crates/sp-domains/src/lib.rs +++ b/crates/sp-domains/src/lib.rs @@ -994,11 +994,8 @@ sp_api::decl_runtime_apis! { /// Returns the best execution chain number. fn head_receipt_number(domain_id: DomainId) -> HeaderNumberFor; - /// Returns the block number of oldest execution receipt. - fn oldest_receipt_number(domain_id: DomainId) -> HeaderNumberFor; - - /// Returns the block tree pruning depth. - fn block_tree_pruning_depth() -> HeaderNumberFor; + /// Returns the block number of oldest unconfirmed execution receipt. + fn oldest_unconfirmed_receipt_number(domain_id: DomainId) -> Option>; /// Returns the domain block limit of the given domain. fn domain_block_limit(domain_id: DomainId) -> Option; diff --git a/crates/subspace-runtime/src/lib.rs b/crates/subspace-runtime/src/lib.rs index f4d0bd7d0f..94fc3d0030 100644 --- a/crates/subspace-runtime/src/lib.rs +++ b/crates/subspace-runtime/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 { + Domains::oldest_unconfirmed_receipt_number(domain_id) } fn domain_block_limit(domain_id: DomainId) -> Option { diff --git a/domains/client/domain-operator/Cargo.toml b/domains/client/domain-operator/Cargo.toml index 4f0f41ba9b..0666ef70a3 100644 --- a/domains/client/domain-operator/Cargo.toml +++ b/domains/client/domain-operator/Cargo.toml @@ -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" diff --git a/domains/client/domain-operator/src/aux_schema.rs b/domains/client/domain-operator/src/aux_schema.rs index 41172fb637..cc82755393 100644 --- a/domains/client/domain-operator/src/aux_schema.rs +++ b/domains/client/domain-operator/src/aux_schema.rs @@ -5,7 +5,8 @@ use codec::{Decode, Encode}; use sc_client_api::backend::AuxStore; use sp_blockchain::{Error as ClientError, Result as ClientResult}; use sp_domains::InvalidBundleType; -use sp_runtime::traits::{Block as BlockT, NumberFor, One, SaturatedConversion}; +use sp_runtime::traits::{Block as BlockT, NumberFor, One, SaturatedConversion, Zero}; +use sp_runtime::Saturating; use subspace_core_primitives::BlockNumber; const EXECUTION_RECEIPT: &[u8] = b"execution_receipt"; @@ -35,6 +36,10 @@ const LATEST_CONSENSUS_HASH: &[u8] = b"latest_consensus_hash"; const BEST_DOMAIN_HASH: &[u8] = b"best_domain_hash"; /// Prune the execution receipts when they reach this number. +/// +/// NOTE: `PRUNING_DEPTH` must larger than the consensus chain `ConfirmationDepthK` to avoid +/// accidentally delete non-confirmed ER from consensus forks that can potentially become the +/// best fork in the future const PRUNING_DEPTH: BlockNumber = 1000; fn execution_receipt_key(block_hash: impl Encode) -> Vec { @@ -59,7 +64,7 @@ fn load_decode( /// too old. pub(super) fn write_execution_receipt( backend: &Backend, - head_receipt_number: NumberFor, + oldest_unconfirmed_receipt_number: Option>, execution_receipt: &ExecutionReceiptFor, ) -> Result<(), sp_blockchain::Error> where @@ -78,15 +83,22 @@ where let first_saved_receipt = load_decode::<_, NumberFor>(backend, EXECUTION_RECEIPT_START)? - .unwrap_or(block_number); + .unwrap_or(Zero::zero()); let mut new_first_saved_receipt = first_saved_receipt; let mut keys_to_delete = vec![]; - if let Some(delete_receipts_to) = head_receipt_number - .saturated_into::() - .checked_sub(PRUNING_DEPTH) + // Delete ER that have comfirned long time ago, also see the comment of `PRUNING_DEPTH` + if let Some(delete_receipts_to) = oldest_unconfirmed_receipt_number + .map(|oldest_unconfirmed_receipt_number| { + oldest_unconfirmed_receipt_number.saturating_sub(One::one()) + }) + .and_then(|latest_confirmed_receipt_number| { + latest_confirmed_receipt_number + .saturated_into::() + .checked_sub(PRUNING_DEPTH) + }) { new_first_saved_receipt = Into::>::into(delete_receipts_to) + One::one(); for receipt_to_delete in first_saved_receipt.saturated_into()..=delete_receipts_to { @@ -205,15 +217,6 @@ where ) } -// TODO: Unlock once domain test infra is workable again. -#[allow(dead_code)] -pub(super) fn target_receipt_is_pruned( - head_receipt_number: BlockNumber, - target_block: BlockNumber, -) -> bool { - head_receipt_number.saturating_sub(target_block) >= PRUNING_DEPTH -} - #[derive(Encode, Decode, Debug, PartialEq)] pub(super) enum BundleMismatchType { // The invalid bundle is mismatch @@ -229,7 +232,9 @@ pub(super) enum BundleMismatchType { mod tests { use super::*; use domain_test_service::evm_domain_test_runtime::Block; + use parking_lot::Mutex; use sp_core::hash::H256; + use std::collections::HashMap; use subspace_runtime_primitives::{Balance, BlockNumber, Hash}; use subspace_test_runtime::Block as CBlock; @@ -252,12 +257,40 @@ mod tests { } } - // TODO: Remove `substrate_test_runtime_client` dependency for faster build time - // TODO: Un-ignore once test client is fixed and working again on Windows + #[derive(Default)] + struct TestClient(Mutex, Vec>>); + + impl AuxStore for TestClient { + fn insert_aux< + 'a, + 'b: 'a, + 'c: 'a, + I: IntoIterator, + D: IntoIterator, + >( + &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>> { + Ok(self.0.lock().get(key).cloned()) + } + } + #[test] - #[ignore] fn normal_prune_execution_receipt_works() { - let client = substrate_test_runtime_client::new(); + let block_tree_pruning_depth = 256; + let client = TestClient::default(); let receipt_start = || { load_decode::<_, BlockNumber>(&client, EXECUTION_RECEIPT_START.to_vec().as_slice()) @@ -272,14 +305,17 @@ mod tests { .unwrap() }; + let target_receipt_is_pruned = |number: BlockNumber| hashes_at(number).is_none(); + let receipt_at = |consensus_block_hash: Hash| { load_execution_receipt::<_, Block, CBlock>(&client, consensus_block_hash).unwrap() }; - let write_receipt_at = |number: BlockNumber, receipt: &ExecutionReceipt| { + let write_receipt_at = |oldest_unconfirmed_receipt_number: Option, + receipt: &ExecutionReceipt| { write_execution_receipt::<_, Block, CBlock>( &client, - number - 1, // Ideally, the receipt of previous block has been included when writing the receipt of current block. + oldest_unconfirmed_receipt_number, receipt, ) .unwrap() @@ -287,66 +323,74 @@ mod tests { assert_eq!(receipt_start(), None); - // Create PRUNING_DEPTH receipts. - let block_hash_list = (1..=PRUNING_DEPTH) + // Create as many ER as before any ER being pruned yet + let receipt_count = PRUNING_DEPTH + block_tree_pruning_depth - 1; + let block_hash_list = (1..=receipt_count) .map(|block_number| { let receipt = create_execution_receipt(block_number); let consensus_block_hash = receipt.consensus_block_hash; - write_receipt_at(block_number, &receipt); + let oldest_unconfirmed_receipt_number = block_number + .checked_sub(block_tree_pruning_depth) + .map(|n| n + 1); + write_receipt_at(oldest_unconfirmed_receipt_number, &receipt); assert_eq!(receipt_at(consensus_block_hash), Some(receipt)); assert_eq!(hashes_at(block_number), Some(vec![consensus_block_hash])); - assert_eq!(receipt_start(), Some(1)); + // No ER have been pruned yet + assert_eq!(receipt_start(), Some(0)); consensus_block_hash }) .collect::>(); - assert!(!target_receipt_is_pruned(PRUNING_DEPTH, 1)); + assert_eq!(receipt_start(), Some(0)); + assert!(!target_receipt_is_pruned(1)); - // Create PRUNING_DEPTH + 1 receipt, head_receipt_number is PRUNING_DEPTH. - let receipt = create_execution_receipt(PRUNING_DEPTH + 1); + // Create `receipt_count + 1` receipt, `oldest_unconfirmed_receipt_number` is `PRUNING_DEPTH + 1`. + let receipt = create_execution_receipt(receipt_count + 1); assert!(receipt_at(receipt.consensus_block_hash).is_none()); - write_receipt_at(PRUNING_DEPTH + 1, &receipt); + write_receipt_at(Some(PRUNING_DEPTH + 1), &receipt); assert!(receipt_at(receipt.consensus_block_hash).is_some()); + assert_eq!(receipt_start(), Some(1)); - // Create PRUNING_DEPTH + 2 receipt, head_receipt_number is PRUNING_DEPTH + 1. - let receipt = create_execution_receipt(PRUNING_DEPTH + 2); - write_receipt_at(PRUNING_DEPTH + 2, &receipt); + // Create `receipt_count + 2` receipt, `oldest_unconfirmed_receipt_number` is `PRUNING_DEPTH + 2`. + let receipt = create_execution_receipt(receipt_count + 2); + write_receipt_at(Some(PRUNING_DEPTH + 2), &receipt); assert!(receipt_at(receipt.consensus_block_hash).is_some()); - // ER of block #1 should be pruned. + // ER of block #1 should be pruned, its block number mapping should be pruned as well. assert!(receipt_at(block_hash_list[0]).is_none()); - // block number mapping should be pruned as well. assert!(hashes_at(1).is_none()); - assert!(target_receipt_is_pruned(PRUNING_DEPTH + 1, 1)); + assert!(target_receipt_is_pruned(1)); assert_eq!(receipt_start(), Some(2)); - // Create PRUNING_DEPTH + 3 receipt, head_receipt_number is PRUNING_DEPTH + 2. - let receipt = create_execution_receipt(PRUNING_DEPTH + 3); + // Create `receipt_count + 3` receipt, `oldest_unconfirmed_receipt_number` is `PRUNING_DEPTH + 3`. + let receipt = create_execution_receipt(receipt_count + 3); let consensus_block_hash1 = receipt.consensus_block_hash; - write_receipt_at(PRUNING_DEPTH + 3, &receipt); + write_receipt_at(Some(PRUNING_DEPTH + 3), &receipt); assert!(receipt_at(consensus_block_hash1).is_some()); // ER of block #2 should be pruned. assert!(receipt_at(block_hash_list[1]).is_none()); - assert!(target_receipt_is_pruned(PRUNING_DEPTH + 2, 2)); - assert!(!target_receipt_is_pruned(PRUNING_DEPTH + 2, 3)); + assert!(target_receipt_is_pruned(2)); + assert!(!target_receipt_is_pruned(3)); assert_eq!(receipt_start(), Some(3)); - // Multiple hashes attached to the block #(PRUNING_DEPTH + 3) - let receipt = create_execution_receipt(PRUNING_DEPTH + 3); + // Multiple hashes attached to the block #`receipt_count + 3` + let receipt = create_execution_receipt(receipt_count + 3); let consensus_block_hash2 = receipt.consensus_block_hash; - write_receipt_at(PRUNING_DEPTH + 3, &receipt); + write_receipt_at(Some(PRUNING_DEPTH + 3), &receipt); assert!(receipt_at(consensus_block_hash2).is_some()); assert_eq!( - hashes_at(PRUNING_DEPTH + 3), + hashes_at(receipt_count + 3), Some(vec![consensus_block_hash1, consensus_block_hash2]) ); + // No ER pruned since the `oldest_unconfirmed_receipt_number` is the same + assert!(!target_receipt_is_pruned(3)); + assert_eq!(receipt_start(), Some(3)); } - // TODO: Un-ignore once test client is fixed and working again on Windows #[test] - #[ignore] - fn execution_receipts_should_be_kept_against_head_receipt_number() { - let client = substrate_test_runtime_client::new(); + fn execution_receipts_should_be_kept_against_oldest_unconfirmed_receipt_number() { + let block_tree_pruning_depth = 256; + let client = TestClient::default(); let receipt_start = || { load_decode::<_, BlockNumber>(&client, EXECUTION_RECEIPT_START.to_vec().as_slice()) @@ -365,60 +409,71 @@ mod tests { load_execution_receipt::<_, Block, CBlock>(&client, consensus_block_hash).unwrap() }; - let write_receipt_at = |head_receipt_number: BlockNumber, receipt: &ExecutionReceipt| { - write_execution_receipt::<_, Block, CBlock>(&client, head_receipt_number, receipt) - .unwrap() + let write_receipt_at = |oldest_unconfirmed_receipt_number: Option, + receipt: &ExecutionReceipt| { + write_execution_receipt::<_, Block, CBlock>( + &client, + oldest_unconfirmed_receipt_number, + receipt, + ) + .unwrap() }; + let target_receipt_is_pruned = |number: BlockNumber| hashes_at(number).is_none(); + assert_eq!(receipt_start(), None); - // Create PRUNING_DEPTH receipts, head_receipt_number is 0, i.e., no receipt - // has ever been included on consensus chain. - let block_hash_list = (1..=PRUNING_DEPTH) + // Create as many ER as before any ER being pruned yet, `oldest_unconfirmed_receipt_number` is `Some(1)`, + // i.e., no receipt has ever been confirmed/pruned on consensus chain. + let receipt_count = PRUNING_DEPTH + block_tree_pruning_depth - 1; + + let block_hash_list = (1..=receipt_count) .map(|block_number| { let receipt = create_execution_receipt(block_number); let consensus_block_hash = receipt.consensus_block_hash; - write_receipt_at(0, &receipt); + write_receipt_at(Some(One::one()), &receipt); assert_eq!(receipt_at(consensus_block_hash), Some(receipt)); assert_eq!(hashes_at(block_number), Some(vec![consensus_block_hash])); - assert_eq!(receipt_start(), Some(1)); + // No ER have been pruned yet + assert_eq!(receipt_start(), Some(0)); consensus_block_hash }) .collect::>(); - assert!(!target_receipt_is_pruned(PRUNING_DEPTH, 1)); + assert_eq!(receipt_start(), Some(0)); + assert!(!target_receipt_is_pruned(1)); - // Create PRUNING_DEPTH + 1 receipt, head_receipt_number is 0. - let receipt = create_execution_receipt(PRUNING_DEPTH + 1); + // Create `receipt_count + 1` receipt, `oldest_unconfirmed_receipt_number` is `Some(1)`. + let receipt = create_execution_receipt(receipt_count + 1); assert!(receipt_at(receipt.consensus_block_hash).is_none()); - write_receipt_at(0, &receipt); + write_receipt_at(Some(One::one()), &receipt); - // Create PRUNING_DEPTH + 2 receipt, head_receipt_number is 0. - let receipt = create_execution_receipt(PRUNING_DEPTH + 2); - write_receipt_at(0, &receipt); + // Create `receipt_count + 2` receipt, `oldest_unconfirmed_receipt_number` is `Some(1)`. + let receipt = create_execution_receipt(receipt_count + 2); + write_receipt_at(Some(One::one()), &receipt); - // ER of block #1 should not be pruned even the size of stored receipts exceeds the pruning depth. + // ER of block #1 and its block number mapping should not be pruned even the size of stored + // receipts exceeds the pruning depth. assert!(receipt_at(block_hash_list[0]).is_some()); - // block number mapping for #1 should not be pruned neither. assert!(hashes_at(1).is_some()); - assert!(!target_receipt_is_pruned(0, 1)); - assert_eq!(receipt_start(), Some(1)); + assert!(!target_receipt_is_pruned(1)); + assert_eq!(receipt_start(), Some(0)); - // Create PRUNING_DEPTH + 3 receipt, head_receipt_number is 0. - let receipt = create_execution_receipt(PRUNING_DEPTH + 3); - write_receipt_at(0, &receipt); + // Create `receipt_count + 3` receipt, `oldest_unconfirmed_receipt_number` is `Some(1)`. + let receipt = create_execution_receipt(receipt_count + 3); + write_receipt_at(Some(One::one()), &receipt); - // Create PRUNING_DEPTH + 4 receipt, head_receipt_number is PRUNING_DEPTH + 3. - let receipt = create_execution_receipt(PRUNING_DEPTH + 4); + // Create `receipt_count + 4` receipt, `oldest_unconfirmed_receipt_number` is `Some(PRUNING_DEPTH + 4)`. + let receipt = create_execution_receipt(receipt_count + 4); write_receipt_at( - PRUNING_DEPTH + 3, // Now assuming all the missing receipts are included. + Some(PRUNING_DEPTH + 4), // Now assuming all the missing receipts are confirmed. &receipt, ); assert!(receipt_at(block_hash_list[0]).is_none()); // receipt and block number mapping for [1, 2, 3] should be pruned. (1..=3).for_each(|pruned| { assert!(hashes_at(pruned).is_none()); - assert!(target_receipt_is_pruned(PRUNING_DEPTH + 3, pruned)); + assert!(target_receipt_is_pruned(pruned)); }); assert_eq!(receipt_start(), Some(4)); } diff --git a/domains/client/domain-operator/src/bundle_processor.rs b/domains/client/domain-operator/src/bundle_processor.rs index 989b4ee347..1f754df120 100644 --- a/domains/client/domain-operator/src/bundle_processor.rs +++ b/domains/client/domain-operator/src/bundle_processor.rs @@ -267,11 +267,6 @@ where on top of parent block #{parent_number},{parent_hash}" ); - let head_receipt_number = self - .consensus_client - .runtime_api() - .head_receipt_number(consensus_block_hash, self.domain_id)?; - let maybe_preprocess_result = self .domain_block_preprocessor .preprocess_consensus_block(consensus_block_hash, parent_hash)?; @@ -289,11 +284,8 @@ where "Skip building new domain block, no bundles and runtime upgrade for this domain \ in consensus block #{consensus_block_number:?},{consensus_block_hash}" ); - self.domain_block_processor.on_consensus_block_processed( - consensus_block_hash, - None, - head_receipt_number, - )?; + self.domain_block_processor + .on_consensus_block_processed(consensus_block_hash, None)?; return Ok(None); }; @@ -311,6 +303,10 @@ where ) .await?; + let head_receipt_number = self + .consensus_client + .runtime_api() + .head_receipt_number(consensus_block_hash, self.domain_id)?; assert!( domain_block_result.header_number > head_receipt_number, "Domain chain number must larger than the head number of the receipt chain \ @@ -341,11 +337,8 @@ where ); } - self.domain_block_processor.on_consensus_block_processed( - consensus_block_hash, - Some(domain_block_result), - head_receipt_number, - )?; + self.domain_block_processor + .on_consensus_block_processed(consensus_block_hash, Some(domain_block_result))?; let post_block_execution_took = start .elapsed() diff --git a/domains/client/domain-operator/src/domain_block_processor.rs b/domains/client/domain-operator/src/domain_block_processor.rs index 31e2075780..0f7b1e6e33 100644 --- a/domains/client/domain-operator/src/domain_block_processor.rs +++ b/domains/client/domain-operator/src/domain_block_processor.rs @@ -510,7 +510,6 @@ where &self, consensus_block_hash: CBlock::Hash, domain_block_result: Option>, - head_receipt_number: NumberFor, ) -> sp_blockchain::Result<()> { let domain_hash = match domain_block_result { Some(DomainBlockResult { @@ -518,9 +517,13 @@ where header_number: _, execution_receipt, }) => { + let oldest_unconfirmed_receipt_number = self + .consensus_client + .runtime_api() + .oldest_unconfirmed_receipt_number(consensus_block_hash, self.domain_id)?; crate::aux_schema::write_execution_receipt::<_, Block, CBlock>( &*self.client, - head_receipt_number, + oldest_unconfirmed_receipt_number, &execution_receipt, )?; @@ -759,12 +762,17 @@ where .consensus_client .runtime_api() .head_receipt_number(consensus_block_hash, self.domain_id)?; - let oldest_receipt_number = self + let oldest_unconfirmed_receipt_number = match self .consensus_client .runtime_api() - .oldest_receipt_number(consensus_block_hash, self.domain_id)?; + .oldest_unconfirmed_receipt_number(consensus_block_hash, self.domain_id)? + { + Some(er_number) => er_number, + // Early return if there is no non-confirmed ER exist + None => return Ok(None), + }; - while !to_check.is_zero() && oldest_receipt_number < to_check { + while !to_check.is_zero() && oldest_unconfirmed_receipt_number <= to_check { let onchain_receipt_hash = self .consensus_client .runtime_api() diff --git a/domains/client/relayer/src/worker.rs b/domains/client/relayer/src/worker.rs index 8deb8ce852..16c9a8a7a9 100644 --- a/domains/client/relayer/src/worker.rs +++ b/domains/client/relayer/src/worker.rs @@ -117,9 +117,11 @@ pub async fn relay_domain_messages( let api = consensus_chain_client.runtime_api(); let at = consensus_chain_client.info().best_hash; - let oldest_tracked_number = api.oldest_receipt_number(at, domain_id)?; - // ensure block number is at least the oldest tracked number - Ok(block_number >= oldest_tracked_number) + Ok(api + .oldest_unconfirmed_receipt_number(at, domain_id)? + // ensure block number is at least the oldest tracked number + .map(|oldest_tracked_number| block_number >= oldest_tracked_number) + .unwrap_or(false)) }, ) .await; diff --git a/test/subspace-test-runtime/src/lib.rs b/test/subspace-test-runtime/src/lib.rs index dcc4a1c876..838e98ea50 100644 --- a/test/subspace-test-runtime/src/lib.rs +++ b/test/subspace-test-runtime/src/lib.rs @@ -1206,12 +1206,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 { + Domains::oldest_unconfirmed_receipt_number(domain_id) } fn domain_block_limit(domain_id: DomainId) -> Option {