From 22256c21551a61c550ad5c47e1a42c43ec4a35a1 Mon Sep 17 00:00:00 2001 From: vedhavyas Date: Wed, 16 Aug 2023 20:49:07 +0200 Subject: [PATCH] ensure bundles are produced until genesis er is confirmed and improve readabillity --- crates/pallet-domains/src/lib.rs | 7 ++++++- crates/sp-domains/src/lib.rs | 2 +- crates/subspace-runtime/src/lib.rs | 4 ++-- .../domain-operator/src/domain_bundle_producer.rs | 2 +- domains/client/domain-operator/src/parent_chain.rs | 10 +++------- domains/client/domain-operator/src/tests.rs | 8 ++++++-- domains/test/service/src/domain.rs | 13 +++++++------ test/subspace-test-runtime/src/lib.rs | 4 ++-- test/subspace-test-service/src/lib.rs | 4 ++-- 9 files changed, 30 insertions(+), 24 deletions(-) diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index 18512027e5..8b283a9644 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -1468,7 +1468,12 @@ impl Pallet { } /// Returns if there are any ERs in the challenge period that have non empty extrinsics. - pub fn non_empty_bundle_exists(domain_id: DomainId) -> bool { + /// Note that Genesis ER is also considered special and hence non empty + pub fn non_empty_er_exists(domain_id: DomainId) -> bool { + if BlockTree::::contains_key(domain_id, T::DomainNumber::zero()) { + return true; + } + let head_number = HeadDomainNumber::::get(domain_id); let mut to_check = head_number .checked_sub(&T::BlockTreePruningDepth::get()) diff --git a/crates/sp-domains/src/lib.rs b/crates/sp-domains/src/lib.rs index c2ce5b7d2c..6a672b79f4 100644 --- a/crates/sp-domains/src/lib.rs +++ b/crates/sp-domains/src/lib.rs @@ -766,7 +766,7 @@ sp_api::decl_runtime_apis! { fn domain_block_limit(domain_id: DomainId) -> Option; /// Returns true if there are any ERs in the challenge period with non empty extrinsics. - fn non_empty_bundle_exists(domain_id: DomainId) -> bool; + fn non_empty_er_exists(domain_id: DomainId) -> bool; } pub trait BundleProducerElectionApi { diff --git a/crates/subspace-runtime/src/lib.rs b/crates/subspace-runtime/src/lib.rs index 4231abe226..b1c9c6dffc 100644 --- a/crates/subspace-runtime/src/lib.rs +++ b/crates/subspace-runtime/src/lib.rs @@ -913,8 +913,8 @@ impl_runtime_apis! { Domains::domain_block_limit(domain_id) } - fn non_empty_bundle_exists(domain_id: DomainId) -> bool { - Domains::non_empty_bundle_exists(domain_id) + fn non_empty_er_exists(domain_id: DomainId) -> bool { + Domains::non_empty_er_exists(domain_id) } } diff --git a/domains/client/domain-operator/src/domain_bundle_producer.rs b/domains/client/domain-operator/src/domain_bundle_producer.rs index 828772c49a..02f212dc11 100644 --- a/domains/client/domain-operator/src/domain_bundle_producer.rs +++ b/domains/client/domain-operator/src/domain_bundle_producer.rs @@ -218,7 +218,7 @@ where if extrinsics.is_empty() && !self .parent_chain - .non_empty_bundle_exists(parent_chain_best_hash, self.domain_id)? + .non_empty_er_exists(parent_chain_best_hash, self.domain_id)? { tracing::warn!( ?domain_best_number, diff --git a/domains/client/domain-operator/src/parent_chain.rs b/domains/client/domain-operator/src/parent_chain.rs index 30930ce16b..74baa9d3fc 100644 --- a/domains/client/domain-operator/src/parent_chain.rs +++ b/domains/client/domain-operator/src/parent_chain.rs @@ -58,7 +58,7 @@ pub trait ParentChainInterface { fraud_proof: FraudProof, ParentChainBlock::Hash>, ) -> Result<(), sp_api::ApiError>; - fn non_empty_bundle_exists( + fn non_empty_er_exists( &self, at: ParentChainBlock::Hash, domain_id: DomainId, @@ -192,13 +192,9 @@ where Ok(()) } - fn non_empty_bundle_exists( - &self, - at: CBlock::Hash, - domain_id: DomainId, - ) -> Result { + fn non_empty_er_exists(&self, at: CBlock::Hash, domain_id: DomainId) -> Result { self.consensus_client .runtime_api() - .non_empty_bundle_exists(at, domain_id) + .non_empty_er_exists(at, domain_id) } } diff --git a/domains/client/domain-operator/src/tests.rs b/domains/client/domain-operator/src/tests.rs index 99a820045c..81ea27cbfb 100644 --- a/domains/client/domain-operator/src/tests.rs +++ b/domains/client/domain-operator/src/tests.rs @@ -118,7 +118,7 @@ async fn test_domain_block_production() { for i in 0..50 { let (tx, slot) = if i % 2 == 0 { // Produce bundle and include it in the primary block hence produce a domain block - alice.send_remark_extrinsic().await.unwrap(); + alice.send_system_remark().await; let (slot, _) = ferdie.produce_slot_and_wait_for_bundle_submission().await; // `None` means collect tx from the tx pool (None, slot) @@ -179,6 +179,7 @@ async fn test_domain_block_production() { assert_eq!(alice.client.info().best_hash, domain_block_hash); // Simply producing more block on fork C + alice.send_system_remark().await; for _ in 0..10 { let (slot, bundle) = ferdie.produce_slot_and_wait_for_bundle_submission().await; let tx = subspace_test_runtime::UncheckedExtrinsic::new_unsigned( @@ -632,7 +633,10 @@ async fn test_executor_full_node_catching_up() { // 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(); + for _ in 0..5 { + let slot = ferdie.produce_slot(); + ferdie.produce_block_with_slot(slot).await.unwrap(); + } // Run Alice (a evm domain authority node) let mut alice = domain_test_service::DomainNodeBuilder::new( diff --git a/domains/test/service/src/domain.rs b/domains/test/service/src/domain.rs index 46321bf3d0..7ea7ef535e 100644 --- a/domains/test/service/src/domain.rs +++ b/domains/test/service/src/domain.rs @@ -318,13 +318,14 @@ where } /// Sends an system.remark extrinsic to the pool. - pub async fn send_remark_extrinsic(&mut self) -> Result<(), RpcTransactionError> { + pub async fn send_system_remark(&mut self) { let nonce = self.account_nonce(); - self.construct_and_send_extrinsic(frame_system::Call::remark { - remark: nonce.encode(), - }) - .await - .map(|_| ()) + let _ = self + .construct_and_send_extrinsic(frame_system::Call::remark { + remark: nonce.encode(), + }) + .await + .map(|_| ()); } /// Construct an extrinsic with the current nonce of the node account and send it to this node. diff --git a/test/subspace-test-runtime/src/lib.rs b/test/subspace-test-runtime/src/lib.rs index 1afdfcc949..d7a81b6f14 100644 --- a/test/subspace-test-runtime/src/lib.rs +++ b/test/subspace-test-runtime/src/lib.rs @@ -1259,8 +1259,8 @@ impl_runtime_apis! { Domains::domain_block_limit(domain_id) } - fn non_empty_bundle_exists(domain_id: DomainId) -> bool { - Domains::non_empty_bundle_exists(domain_id) + fn non_empty_er_exists(domain_id: DomainId) -> bool { + Domains::non_empty_er_exists(domain_id) } } diff --git a/test/subspace-test-service/src/lib.rs b/test/subspace-test-service/src/lib.rs index 7a251a3af7..cb59e71c36 100644 --- a/test/subspace-test-service/src/lib.rs +++ b/test/subspace-test-service/src/lib.rs @@ -908,7 +908,7 @@ where macro_rules! produce_blocks { ($primary_node:ident, $operator_node:ident, $count: literal $(, $domain_node:ident)*) => { { - let _ = $operator_node.send_remark_extrinsic().await; + $operator_node.send_system_remark().await; async { let domain_fut = { let mut futs: Vec>>> = Vec::new(); @@ -930,7 +930,7 @@ macro_rules! produce_blocks { macro_rules! produce_block_with { ($primary_node_produce_block:expr, $operator_node:ident $(, $domain_node:ident)*) => { { - let _ = $operator_node.send_remark_extrinsic().await; + $operator_node.send_system_remark().await; async { let domain_fut = { let mut futs: Vec>>> = Vec::new();