Skip to content

Commit

Permalink
Merge pull request #1831 from subspace/update_tests_to_handle_slot_miss
Browse files Browse the repository at this point in the history
Domains: Ensure unconfirmed genesis ER triggers bundles and adjust tests
  • Loading branch information
vedhavyas authored Aug 18, 2023
2 parents d731c89 + 36951d7 commit d6fb246
Show file tree
Hide file tree
Showing 9 changed files with 30 additions and 24 deletions.
7 changes: 6 additions & 1 deletion crates/pallet-domains/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1468,7 +1468,12 @@ impl<T: Config> Pallet<T> {
}

/// 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::<T>::contains_key(domain_id, T::DomainNumber::zero()) {
return true;
}

let head_number = HeadDomainNumber::<T>::get(domain_id);
let mut to_check = head_number
.checked_sub(&T::BlockTreePruningDepth::get())
Expand Down
2 changes: 1 addition & 1 deletion crates/sp-domains/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@ sp_api::decl_runtime_apis! {
fn domain_block_limit(domain_id: DomainId) -> Option<DomainBlockLimit>;

/// 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<Balance: Encode + Decode> {
Expand Down
4 changes: 2 additions & 2 deletions crates/subspace-runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 3 additions & 7 deletions domains/client/domain-operator/src/parent_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub trait ParentChainInterface<Block: BlockT, ParentChainBlock: BlockT> {
fraud_proof: FraudProof<NumberFor<ParentChainBlock>, ParentChainBlock::Hash>,
) -> Result<(), sp_api::ApiError>;

fn non_empty_bundle_exists(
fn non_empty_er_exists(
&self,
at: ParentChainBlock::Hash,
domain_id: DomainId,
Expand Down Expand Up @@ -192,13 +192,9 @@ where
Ok(())
}

fn non_empty_bundle_exists(
&self,
at: CBlock::Hash,
domain_id: DomainId,
) -> Result<bool, ApiError> {
fn non_empty_er_exists(&self, at: CBlock::Hash, domain_id: DomainId) -> Result<bool, ApiError> {
self.consensus_client
.runtime_api()
.non_empty_bundle_exists(at, domain_id)
.non_empty_er_exists(at, domain_id)
}
}
8 changes: 6 additions & 2 deletions domains/client/domain-operator/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
13 changes: 7 additions & 6 deletions domains/test/service/src/domain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions test/subspace-test-runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
4 changes: 2 additions & 2 deletions test/subspace-test-service/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::pin::Pin<Box<dyn futures::Future<Output = ()>>>> = Vec::new();
Expand All @@ -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<std::pin::Pin<Box<dyn futures::Future<Output = ()>>>> = Vec::new();
Expand Down

0 comments on commit d6fb246

Please sign in to comment.