From ecfcc9be481637caad6d0bdfc95d4a3b2e2e8089 Mon Sep 17 00:00:00 2001 From: linning Date: Sat, 29 Jul 2023 05:41:00 +0800 Subject: [PATCH 1/4] Recorrect the instantiation time of the genesis domain This value will be used in later commit Signed-off-by: linning --- crates/pallet-domains/src/lib.rs | 27 +++++++++++----------- crates/pallet-domains/src/staking_epoch.rs | 8 ++----- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index 1e36b14395..a1d326e767 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -658,11 +658,15 @@ mod pallet { ) .map_err(Error::::from)?; - do_finalize_domain_current_epoch::( - domain_id, - pruned_block_info.domain_block_number, - ) - .map_err(Error::::from)?; + if pruned_block_info.domain_block_number % T::StakeEpochDuration::get() + == Zero::zero() + { + do_finalize_domain_current_epoch::( + domain_id, + pruned_block_info.domain_block_number, + ) + .map_err(Error::::from)?; + } } } } @@ -923,19 +927,16 @@ mod pallet { genesis_domain.runtime_type.clone(), genesis_domain.runtime_version.clone(), genesis_domain.code.clone(), - Zero::zero(), + One::one(), ) .expect("Genesis runtime registration must always succeed"); // Instantiate the genesis domain let domain_config = DomainConfig::from_genesis::(genesis_domain, runtime_id); let domain_owner = genesis_domain.owner_account_id.clone(); - let domain_id = do_instantiate_domain::( - domain_config, - domain_owner.clone(), - Zero::zero(), - ) - .expect("Genesis domain instantiation must always succeed"); + let domain_id = + do_instantiate_domain::(domain_config, domain_owner.clone(), One::one()) + .expect("Genesis domain instantiation must always succeed"); // Register domain_owner as the genesis operator. let operator_config = OperatorConfig { @@ -954,7 +955,7 @@ mod pallet { ) .expect("Genesis operator registration must succeed"); - do_finalize_domain_current_epoch::(domain_id, Zero::zero()) + do_finalize_domain_current_epoch::(domain_id, One::one()) .expect("Genesis epoch must succeed"); } } diff --git a/crates/pallet-domains/src/staking_epoch.rs b/crates/pallet-domains/src/staking_epoch.rs index 043385e7ee..4ae3aedb32 100644 --- a/crates/pallet-domains/src/staking_epoch.rs +++ b/crates/pallet-domains/src/staking_epoch.rs @@ -38,11 +38,7 @@ pub enum Error { pub(crate) fn do_finalize_domain_current_epoch( domain_id: DomainId, domain_block_number: T::DomainNumber, -) -> Result { - if domain_block_number % T::StakeEpochDuration::get() != Zero::zero() { - return Ok(false); - } - +) -> Result<(), Error> { // slash the operators do_finalize_slashed_operators::(domain_id).map_err(Error::SlashOperator)?; @@ -57,7 +53,7 @@ pub(crate) fn do_finalize_domain_current_epoch( // finalize any withdrawals and then deposits do_finalize_domain_pending_transfers::(domain_id, domain_block_number)?; - Ok(true) + Ok(()) } /// Unlocks any operators who are de-registering or nominators who are withdrawing staked funds. From 1e403e3daeed46db7863941997ab65343ab94ac3 Mon Sep 17 00:00:00 2001 From: linning Date: Sat, 29 Jul 2023 05:44:49 +0800 Subject: [PATCH 2/4] Add the domain_created_at field to the domain_block_processor This value indicate the consensus block number that the domain instance first instantiated Signed-off-by: linning --- crates/pallet-domains/src/lib.rs | 24 +++++----- crates/sp-domains/src/lib.rs | 2 +- .../src/domain/domain_instance_starter.rs | 2 + crates/subspace-runtime/src/lib.rs | 2 +- .../domain-operator/src/bootstrapper.rs | 8 +++- .../src/domain_block_processor.rs | 2 + domains/client/domain-operator/src/lib.rs | 1 + .../client/domain-operator/src/operator.rs | 1 + domains/service/src/domain.rs | 3 ++ domains/test/service/src/domain.rs | 44 +++++++++---------- test/subspace-test-runtime/src/lib.rs | 2 +- 11 files changed, 54 insertions(+), 37 deletions(-) diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index a1d326e767..10667c4e7a 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -1056,16 +1056,20 @@ impl Pallet { .map(|domain_object| domain_object.domain_config.runtime_id) } - pub fn domain_instance_data(domain_id: DomainId) -> Option { - let runtime_id = DomainRegistry::::get(domain_id)? - .domain_config - .runtime_id; - let (runtime_type, runtime_code) = RuntimeRegistry::::get(runtime_id) - .map(|runtime_object| (runtime_object.runtime_type, runtime_object.code))?; - Some(DomainInstanceData { - runtime_type, - runtime_code, - }) + pub fn domain_instance_data( + domain_id: DomainId, + ) -> Option<(DomainInstanceData, T::BlockNumber)> { + let domain_obj = DomainRegistry::::get(domain_id)?; + let (runtime_type, runtime_code) = + RuntimeRegistry::::get(domain_obj.domain_config.runtime_id) + .map(|runtime_object| (runtime_object.runtime_type, runtime_object.code))?; + Some(( + DomainInstanceData { + runtime_type, + runtime_code, + }, + domain_obj.created_at, + )) } pub fn genesis_state_root(domain_id: DomainId) -> Option { diff --git a/crates/sp-domains/src/lib.rs b/crates/sp-domains/src/lib.rs index 2075ee8631..62cc7d428f 100644 --- a/crates/sp-domains/src/lib.rs +++ b/crates/sp-domains/src/lib.rs @@ -602,7 +602,7 @@ sp_api::decl_runtime_apis! { fn runtime_id(domain_id: DomainId) -> Option; /// Returns the domain instance data for given `domain_id`. - fn domain_instance_data(domain_id: DomainId) -> Option; + fn domain_instance_data(domain_id: DomainId) -> Option<(DomainInstanceData, NumberFor)>; /// Returns the current timestamp at given height. fn timestamp() -> Moment; diff --git a/crates/subspace-node/src/domain/domain_instance_starter.rs b/crates/subspace-node/src/domain/domain_instance_starter.rs index 60bb603e9d..95dfad5c60 100644 --- a/crates/subspace-node/src/domain/domain_instance_starter.rs +++ b/crates/subspace-node/src/domain/domain_instance_starter.rs @@ -43,6 +43,7 @@ impl DomainInstanceStarter { ) -> std::result::Result<(), Box> { let BootstrapResult { domain_instance_data, + domain_created_at, imported_block_notification_stream, } = bootstrap_result; @@ -142,6 +143,7 @@ impl DomainInstanceStarter { let domain_params = domain_service::DomainParams { domain_id, domain_config, + domain_created_at, consensus_client, consensus_network_sync_oracle: consensus_sync_service.clone(), select_chain, diff --git a/crates/subspace-runtime/src/lib.rs b/crates/subspace-runtime/src/lib.rs index 5f07cce284..b0a6ee5bb5 100644 --- a/crates/subspace-runtime/src/lib.rs +++ b/crates/subspace-runtime/src/lib.rs @@ -854,7 +854,7 @@ impl_runtime_apis! { Domains::runtime_id(domain_id) } - fn domain_instance_data(domain_id: DomainId) -> Option { + fn domain_instance_data(domain_id: DomainId) -> Option<(DomainInstanceData, NumberFor)> { Domains::domain_instance_data(domain_id) } diff --git a/domains/client/domain-operator/src/bootstrapper.rs b/domains/client/domain-operator/src/bootstrapper.rs index b94695bf0c..bb948007fe 100644 --- a/domains/client/domain-operator/src/bootstrapper.rs +++ b/domains/client/domain-operator/src/bootstrapper.rs @@ -11,6 +11,8 @@ pub struct BootstrapResult { // The `DomainInstanceData` used by the domain instance starter to // construct `RuntimeGenesisConfig` of the domain instance pub domain_instance_data: DomainInstanceData, + /// The consensus chain block number when the domain first instantiated. + pub domain_created_at: NumberFor, // The `imported_block_notification_stream` used by the bootstrapper // // NOTE: the domain instance starter must reuse this stream instead of @@ -53,19 +55,20 @@ where // Check if the domain instance data already exist in the consensus chain's genesis state let best_hash = self.consensus_client.info().best_hash; - if let Some(domain_instance_data) = self + if let Some((domain_instance_data, domain_created_at)) = self .consensus_client .runtime_api() .domain_instance_data(best_hash, self_domain_id)? { return Ok(BootstrapResult { domain_instance_data, + domain_created_at, imported_block_notification_stream, }); } // Check each imported consensus block to get the domain instance data - let domain_instance_data = 'outer: loop { + let (domain_instance_data, domain_created_at) = 'outer: loop { if let Some(block_imported) = imported_block_notification_stream.next().await { let header = block_imported.header; for item in header.digest().logs.iter() { @@ -97,6 +100,7 @@ where Ok(BootstrapResult { domain_instance_data, + domain_created_at, imported_block_notification_stream, }) } diff --git a/domains/client/domain-operator/src/domain_block_processor.rs b/domains/client/domain-operator/src/domain_block_processor.rs index c4a7e33597..7429a549ec 100644 --- a/domains/client/domain-operator/src/domain_block_processor.rs +++ b/domains/client/domain-operator/src/domain_block_processor.rs @@ -37,6 +37,7 @@ where CBlock: BlockT, { pub(crate) domain_id: DomainId, + pub(crate) domain_created_at: NumberFor, pub(crate) client: Arc, pub(crate) consensus_client: Arc, pub(crate) backend: Arc, @@ -54,6 +55,7 @@ where fn clone(&self) -> Self { Self { domain_id: self.domain_id, + domain_created_at: self.domain_created_at, client: self.client.clone(), consensus_client: self.consensus_client.clone(), backend: self.backend.clone(), diff --git a/domains/client/domain-operator/src/lib.rs b/domains/client/domain-operator/src/lib.rs index e957b97f9b..6c4730b439 100644 --- a/domains/client/domain-operator/src/lib.rs +++ b/domains/client/domain-operator/src/lib.rs @@ -164,6 +164,7 @@ pub struct OperatorParams< NSNS: Stream + Send + 'static, { pub domain_id: DomainId, + pub domain_created_at: NumberFor, pub consensus_client: Arc, pub consensus_network_sync_oracle: Arc, pub client: Arc, diff --git a/domains/client/domain-operator/src/operator.rs b/domains/client/domain-operator/src/operator.rs index 66962c8fdf..6e0e4f8155 100644 --- a/domains/client/domain-operator/src/operator.rs +++ b/domains/client/domain-operator/src/operator.rs @@ -153,6 +153,7 @@ where let domain_block_processor = DomainBlockProcessor { domain_id: params.domain_id, + domain_created_at: params.domain_created_at, client: params.client.clone(), consensus_client: params.consensus_client.clone(), backend: params.backend.clone(), diff --git a/domains/service/src/domain.rs b/domains/service/src/domain.rs index 77f6aca9c5..243d08b9f7 100644 --- a/domains/service/src/domain.rs +++ b/domains/service/src/domain.rs @@ -244,6 +244,7 @@ where { pub domain_id: DomainId, pub domain_config: DomainConfiguration, + pub domain_created_at: NumberFor, pub consensus_client: Arc, pub consensus_network_sync_oracle: Arc, pub select_chain: SC, @@ -341,6 +342,7 @@ where let DomainParams { domain_id, domain_config, + domain_created_at, consensus_client, consensus_network_sync_oracle, select_chain, @@ -440,6 +442,7 @@ where &select_chain, OperatorParams { domain_id, + domain_created_at, consensus_client: consensus_client.clone(), consensus_network_sync_oracle, client: client.clone(), diff --git a/domains/test/service/src/domain.rs b/domains/test/service/src/domain.rs index 9683d1a031..012777f0e1 100644 --- a/domains/test/service/src/domain.rs +++ b/domains/test/service/src/domain.rs @@ -3,7 +3,7 @@ use crate::chain_spec::{domain_instance_genesis_config, load_chain_spec_with}; use crate::{construct_extrinsic_generic, node_config, EcdsaKeyring, UncheckedExtrinsicFor}; -use domain_client_operator::{Bootstrapper, OperatorStreams}; +use domain_client_operator::{BootstrapResult, Bootstrapper, OperatorStreams}; use domain_runtime_primitives::opaque::Block; use domain_runtime_primitives::{Balance, DomainCoreApi, InherentExtrinsicApi}; use domain_service::providers::DefaultProvider; @@ -14,12 +14,12 @@ use evm_domain_test_runtime::AccountId as AccountId20; use fp_rpc::EthereumRuntimeRPCApi; use frame_support::dispatch::{DispatchInfo, PostDispatchInfo}; use pallet_transaction_payment_rpc::TransactionPaymentRuntimeApi; -use sc_client_api::{BlockchainEvents, HeaderBackend, StateBackendFor}; +use sc_client_api::{HeaderBackend, StateBackendFor}; use sc_executor::NativeExecutionDispatch; use sc_network::{NetworkService, NetworkStateInfo}; use sc_network_sync::SyncingService; use sc_service::config::MultiaddrWithPeerId; -use sc_service::{BasePath, ChainSpec, Role, RpcHandlers, TFullBackend, TaskManager}; +use sc_service::{BasePath, Role, RpcHandlers, TFullBackend, TaskManager}; use sc_utils::mpsc::TracingUnboundedSender; use serde::de::DeserializeOwned; use sp_api::{ApiExt, ConstructRuntimeApi, Metadata, NumberFor, ProvideRuntimeApi}; @@ -169,10 +169,25 @@ where domain_nodes: Vec, domain_nodes_exclusive: bool, run_relayer: bool, - chain_spec: Box, role: Role, mock_consensus_node: &mut MockConsensusNode, ) -> Self { + let BootstrapResult { + domain_instance_data, + domain_created_at, + imported_block_notification_stream, + } = { + let bootstrapper = Bootstrapper::::new(mock_consensus_node.client.clone()); + bootstrapper + .fetch_domain_bootstrap_info(domain_id) + .await + .expect("Failed to get domain instance data") + }; + let chain_spec = { + let genesis_config = + domain_instance_genesis_config(domain_id, domain_instance_data.runtime_code); + load_chain_spec_with(genesis_config) + }; let service_config = node_config( domain_id, tokio_handle.clone(), @@ -208,9 +223,7 @@ where consensus_block_import_throttling_buffer_size: 0, block_importing_notification_stream: mock_consensus_node .block_importing_notification_stream(), - imported_block_notification_stream: mock_consensus_node - .client - .every_import_notification_stream(), + imported_block_notification_stream, new_slot_notification_stream: mock_consensus_node.new_slot_notification_stream(), _phantom: Default::default(), }; @@ -221,6 +234,7 @@ where let domain_params = domain_service::DomainParams { domain_id, domain_config, + domain_created_at, consensus_client: mock_consensus_node.client.clone(), consensus_network_sync_oracle: mock_consensus_node.sync_service.clone(), select_chain: mock_consensus_node.select_chain.clone(), @@ -408,28 +422,14 @@ impl DomainNodeBuilder { domain_id: DomainId, mock_consensus_node: &mut MockConsensusNode, ) -> EvmDomainNode { - let domain_instance_data = { - let bootstrapper = Bootstrapper::::new(mock_consensus_node.client.clone()); - bootstrapper - .fetch_domain_bootstrap_info(domain_id) - .await - .expect("Failed to get domain instance data") - .domain_instance_data - }; - let chain_spec = { - let genesis_config = - domain_instance_genesis_config(domain_id, domain_instance_data.runtime_code); - load_chain_spec_with(genesis_config) - }; DomainNode::build( - DomainId::new(0u32), + domain_id, self.tokio_handle, self.key, self.base_path, self.domain_nodes, self.domain_nodes_exclusive, self.run_relayer, - chain_spec, role, mock_consensus_node, ) diff --git a/test/subspace-test-runtime/src/lib.rs b/test/subspace-test-runtime/src/lib.rs index a3aba8c90d..c86bf56561 100644 --- a/test/subspace-test-runtime/src/lib.rs +++ b/test/subspace-test-runtime/src/lib.rs @@ -1210,7 +1210,7 @@ impl_runtime_apis! { Domains::runtime_id(domain_id) } - fn domain_instance_data(domain_id: DomainId) -> Option { + fn domain_instance_data(domain_id: DomainId) -> Option<(DomainInstanceData, NumberFor)> { Domains::domain_instance_data(domain_id) } From 00e27e67b5a277cec2e5a31e3a262b907df0e317 Mon Sep 17 00:00:00 2001 From: linning Date: Sat, 29 Jul 2023 05:52:59 +0800 Subject: [PATCH 3/4] Make the domain client instantiation-aware Before this commit, the domain client assume that the domain chain start at the consensus chain's genesis block, but in domain v2, the domain chain only start after it is instantiated at the consensus chain, namely started at domain_created_at, this commit recorrect this assumption and handling various edge cases properly Signed-off-by: linning --- .../src/domain_block_processor.rs | 59 +++++++++++++++---- 1 file changed, 48 insertions(+), 11 deletions(-) diff --git a/domains/client/domain-operator/src/domain_block_processor.rs b/domains/client/domain-operator/src/domain_block_processor.rs index 7429a549ec..68a70f5ce9 100644 --- a/domains/client/domain-operator/src/domain_block_processor.rs +++ b/domains/client/domain-operator/src/domain_block_processor.rs @@ -18,6 +18,7 @@ use sp_domains::merkle_tree::MerkleTree; use sp_domains::{DomainId, DomainsApi, ExecutionReceipt, ExtrinsicsRoot}; use sp_runtime::traits::{Block as BlockT, CheckedSub, HashFor, Header as HeaderT, One, Zero}; use sp_runtime::Digest; +use std::cmp::Ordering; use std::sync::Arc; pub(crate) struct DomainBlockResult @@ -118,26 +119,46 @@ where consensus_block_hash: CBlock::Hash, consensus_block_number: NumberFor, ) -> sp_blockchain::Result>> { - if consensus_block_number == One::one() { - return Ok(Some(PendingConsensusBlocks { - initial_parent: (self.client.info().genesis_hash, Zero::zero()), - consensus_imports: vec![HashAndNumber { - hash: consensus_block_hash, - number: consensus_block_number, - }], - })); + match consensus_block_number.cmp(&(self.domain_created_at + One::one())) { + // Consensus block at `domain_created_at + 1` is the first block that possibly contains + // bundle, thus we can safely skip any consensus block that is smaller than this block. + Ordering::Less => return Ok(None), + // For consensus block at `domain_created_at + 1` use the genesis domain block as the + // initial parent block directly to avoid further processing. + Ordering::Equal => { + return Ok(Some(PendingConsensusBlocks { + initial_parent: (self.client.info().genesis_hash, Zero::zero()), + consensus_imports: vec![HashAndNumber { + hash: consensus_block_hash, + number: consensus_block_number, + }], + })); + } + // For consensus block > `domain_created_at + 1`, there is potential existing fork + // thus we need to handle it carefully as following. + Ordering::Greater => {} } let best_hash = self.client.info().best_hash; let best_number = self.client.info().best_number; - let consensus_block_hash_for_best_domain_hash = + let consensus_block_hash_for_best_domain_hash = if best_number.is_zero() { + self.consensus_client + .hash(self.domain_created_at)? + .ok_or_else(|| { + sp_blockchain::Error::Backend(format!( + "Consensus hash for block #{} not found", + self.domain_created_at + )) + })? + } else { crate::aux_schema::latest_consensus_block_hash_for(&*self.backend, &best_hash)? .ok_or_else(|| { sp_blockchain::Error::Backend(format!( "Consensus hash for domain hash #{best_number},{best_hash} not found" )) - })?; + })? + }; let consensus_from = consensus_block_hash_for_best_domain_hash; let consensus_to = consensus_block_hash; @@ -183,6 +204,22 @@ where let (common_block_number, common_block_hash) = (route.common_block().number, route.common_block().hash); + // The `common_block` is smaller than the consensus block that the domain was created at, thus + // we can safely skip any consensus block that is smaller than `domain_created_at` and start at + // the consensus block `domain_created_at + 1`, which the first block that possibly contains bundle, + // and use the genesis domain block as the initial parent block. + if common_block_number <= self.domain_created_at { + let consensus_imports = enacted + .iter() + .skip_while(|block| block.number <= self.domain_created_at) + .cloned() + .collect(); + return Ok(Some(PendingConsensusBlocks { + initial_parent: (self.client.info().genesis_hash, Zero::zero()), + consensus_imports, + })); + } + // Get the domain block that is derived from the common consensus block and use it as // the initial domain parent block let domain_block_hash: Block::Hash = crate::aux_schema::best_domain_hash_for( @@ -451,7 +488,7 @@ where "Header for consensus block {consensus_block_hash:?} not found" )) })?; - if !consensus_header.number().is_one() { + if *consensus_header.number() > self.domain_created_at + One::one() { let consensus_parent_hash = consensus_header.parent_hash(); crate::aux_schema::best_domain_hash_for(&*self.client, consensus_parent_hash)? .ok_or_else(|| { From 672c149077d37d02a12b86eaee79cde76161382e Mon Sep 17 00:00:00 2001 From: linning Date: Sat, 29 Jul 2023 06:23:15 +0800 Subject: [PATCH 4/4] Add test case to ensure the domain block processor can correctly handle the consensus block even it is out of order Signed-off-by: linning --- domains/client/domain-operator/src/tests.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/domains/client/domain-operator/src/tests.rs b/domains/client/domain-operator/src/tests.rs index 7325223b30..9e13d8f6bc 100644 --- a/domains/client/domain-operator/src/tests.rs +++ b/domains/client/domain-operator/src/tests.rs @@ -528,8 +528,12 @@ async fn test_executor_full_node_catching_up() { Ferdie, BasePath::new(directory.path().join("ferdie")), ); - // Produce 1 consensus block to initialize genesis domain - ferdie.produce_block_with_slot(1.into()).await.unwrap(); + // Produce 5 consensus block to initialize genesis domain + // + // This also make the first consensus block being processed by the alice's domain + // block processor be block #5, to ensure it can correctly handle the consensus + // block even it is out of order. + ferdie.produce_blocks(5).await.unwrap(); // Run Alice (a evm domain authority node) let alice = domain_test_service::DomainNodeBuilder::new(