From c741b2b2c421a302ddcd36fd53e0ccc57844c4be Mon Sep 17 00:00:00 2001 From: Andronik Ordian Date: Wed, 29 Sep 2021 12:30:43 +0200 Subject: [PATCH 1/6] CI: run disputes tests --- scripts/gitlab/test_linux_stable.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/gitlab/test_linux_stable.sh b/scripts/gitlab/test_linux_stable.sh index 8ba62ecbbb80..f7b36141af3a 100755 --- a/scripts/gitlab/test_linux_stable.sh +++ b/scripts/gitlab/test_linux_stable.sh @@ -4,4 +4,5 @@ set -e #shellcheck source=../common/lib.sh source "$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )/../common/lib.sh" +time cargo test --release --locked -p polkadot-node-core-dispute-coordinator --features disputes time cargo test --workspace --release --verbose --locked --features=runtime-benchmarks From 02a1ecf3cacb52d6dd467e8f226a69bae651d319 Mon Sep 17 00:00:00 2001 From: Andronik Ordian Date: Wed, 29 Sep 2021 12:37:06 +0200 Subject: [PATCH 2/6] Revert "minor chore changes (#3944)" This reverts commit 1d05f779b25e01a1d54dbf98a82662d12a8320f9. --- node/core/dispute-coordinator/src/real/mod.rs | 37 ++++++++++++++----- .../test-parachains/adder/collator/README.md | 2 - 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/node/core/dispute-coordinator/src/real/mod.rs b/node/core/dispute-coordinator/src/real/mod.rs index f0d0f5b8597a..aab0043e2d2c 100644 --- a/node/core/dispute-coordinator/src/real/mod.rs +++ b/node/core/dispute-coordinator/src/real/mod.rs @@ -522,7 +522,7 @@ async fn handle_incoming( statements, pending_confirmation, } => { - let outcome = handle_import_statements( + handle_import_statements( ctx, overlay_db, state, @@ -531,10 +531,10 @@ async fn handle_incoming( session, statements, now, + pending_confirmation, metrics, ) .await?; - pending_confirmation.send(outcome).map_err(|_| Error::OneshotSend)?; }, DisputeCoordinatorMessage::RecentDisputes(rx) => { let recent_disputes = overlay_db.load_recent_disputes()?.unwrap_or_default(); @@ -633,10 +633,14 @@ async fn handle_import_statements( now: Timestamp, pending_confirmation: oneshot::Sender, metrics: &Metrics, -) -> Result { +) -> Result<(), Error> { if state.highest_session.map_or(true, |h| session + DISPUTE_WINDOW < h) { // It is not valid to participate in an ancient dispute (spam?). - return Ok(ImportStatementsResult::InvalidImport) + pending_confirmation + .send(ImportStatementsResult::InvalidImport) + .map_err(|_| Error::OneshotSend)?; + + return Ok(()) } let validators = match state.rolling_session_window.session_info(session) { @@ -647,7 +651,11 @@ async fn handle_import_statements( "Missing info for session which has an active dispute", ); - return Ok(ImportStatementsResult::InvalidImport) + pending_confirmation + .send(ImportStatementsResult::InvalidImport) + .map_err(|_| Error::OneshotSend)?; + + return Ok(()) }, Some(info) => info.validators.clone(), }; @@ -772,12 +780,15 @@ async fn handle_import_statements( // // We expect that if the candidate is truly disputed that the higher-level network // code will retry. + pending_confirmation + .send(ImportStatementsResult::InvalidImport) + .map_err(|_| Error::OneshotSend)?; tracing::debug!( target: LOG_TARGET, "Recovering availability failed - invalid import." ); - return Ok(ImportStatementsResult::InvalidImport) + return Ok(()) } metrics.on_open(); @@ -795,7 +806,11 @@ async fn handle_import_statements( overlay_db.write_candidate_votes(session, candidate_hash, votes.into()); - Ok(ImportStatementsResult::ValidImport) + pending_confirmation + .send(ImportStatementsResult::ValidImport) + .map_err(|_| Error::OneshotSend)?; + + Ok(()) } fn find_controlled_validator_indices( @@ -902,7 +917,8 @@ async fn issue_local_statement( // Do import if !statements.is_empty() { - match handle_import_statements( + let (pending_confirmation, rx) = oneshot::channel(); + handle_import_statements( ctx, overlay_db, state, @@ -911,10 +927,11 @@ async fn issue_local_statement( session, statements, now, + pending_confirmation, metrics, ) - .await? - { + .await?; + match rx.await { Err(_) => { tracing::error!( target: LOG_TARGET, diff --git a/parachain/test-parachains/adder/collator/README.md b/parachain/test-parachains/adder/collator/README.md index 4347a9a8ced7..be5922b9f95a 100644 --- a/parachain/test-parachains/adder/collator/README.md +++ b/parachain/test-parachains/adder/collator/README.md @@ -1,14 +1,12 @@ # How to run this collator First start two validators that will run for the relay chain: - ```sh cargo run --release -- -d alice --chain rococo-local --validator --alice --port 50551 cargo run --release -- -d bob --chain rococo-local --validator --bob --port 50552 ``` Next start the collator that will collate for the adder parachain: - ```sh cargo run --release -p test-parachain-adder-collator -- --tmp --chain rococo-local --port 50553 ``` From d99a96dcf9c400c57ea7e66fa758aceb6d469362 Mon Sep 17 00:00:00 2001 From: Andronik Ordian Date: Wed, 29 Sep 2021 12:41:33 +0200 Subject: [PATCH 3/6] fix em --- node/core/dispute-coordinator/src/real/mod.rs | 2 +- .../dispute-coordinator/src/real/tests.rs | 19 ++++--------------- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/node/core/dispute-coordinator/src/real/mod.rs b/node/core/dispute-coordinator/src/real/mod.rs index aab0043e2d2c..10220251573a 100644 --- a/node/core/dispute-coordinator/src/real/mod.rs +++ b/node/core/dispute-coordinator/src/real/mod.rs @@ -38,7 +38,7 @@ use polkadot_node_primitives::{ use polkadot_node_subsystem::{ errors::{ChainApiError, RuntimeApiError}, messages::{ - BlockDescription, ChainApiMessage, DisputeCoordinatorMessage, DisputeDistributionMessage, + BlockDescription, DisputeCoordinatorMessage, DisputeDistributionMessage, DisputeParticipationMessage, ImportStatementsResult, }, overseer, FromOverseer, OverseerSignal, SpawnedSubsystem, SubsystemContext, SubsystemError, diff --git a/node/core/dispute-coordinator/src/real/tests.rs b/node/core/dispute-coordinator/src/real/tests.rs index 2f78a5c48707..6832cf3eb321 100644 --- a/node/core/dispute-coordinator/src/real/tests.rs +++ b/node/core/dispute-coordinator/src/real/tests.rs @@ -27,7 +27,7 @@ use parity_scale_codec::Encode; use polkadot_node_subsystem::{ jaeger, messages::{ - AllMessages, BlockDescription, ChainApiMessage, RuntimeApiMessage, RuntimeApiRequest, + AllMessages, BlockDescription, RuntimeApiMessage, RuntimeApiRequest, }, ActivatedLeaf, ActiveLeavesUpdate, LeafStatus, }; @@ -170,7 +170,7 @@ impl TestState { ))) .await; - self.handle_sync_queries(virtual_overseer, block_hash, block_header, session) + self.handle_sync_queries(virtual_overseer, block_hash, session) .await; } @@ -178,25 +178,15 @@ impl TestState { &self, virtual_overseer: &mut VirtualOverseer, block_hash: Hash, - block_header: Header, session: SessionIndex, ) { - assert_matches!( - virtual_overseer.recv().await, - AllMessages::ChainApi(ChainApiMessage::BlockHeader(h, tx)) => { - assert_eq!(h, block_hash); - let _ = tx.send(Ok(Some(block_header))); - } - ); - assert_matches!( virtual_overseer.recv().await, AllMessages::RuntimeApi(RuntimeApiMessage::Request( h, RuntimeApiRequest::SessionIndexForChild(tx), )) => { - let parent_hash = session_to_hash(session, b"parent"); - assert_eq!(h, parent_hash); + assert_eq!(h, block_hash); let _ = tx.send(Ok(session)); } ); @@ -236,8 +226,7 @@ impl TestState { ))) .await; - let header = self.headers.get(leaf).unwrap().clone(); - self.handle_sync_queries(virtual_overseer, *leaf, header, session).await; + self.handle_sync_queries(virtual_overseer, *leaf, session).await; } } From edf995b7445de483007401edf606b4a7cbd81b71 Mon Sep 17 00:00:00 2001 From: Andronik Ordian Date: Wed, 29 Sep 2021 12:43:41 +0200 Subject: [PATCH 4/6] Revert "Revert "minor chore changes (#3944)"" This reverts commit 02a1ecf3cacb52d6dd467e8f226a69bae651d319. --- node/core/dispute-coordinator/src/real/mod.rs | 37 +++++-------------- .../test-parachains/adder/collator/README.md | 2 + 2 files changed, 12 insertions(+), 27 deletions(-) diff --git a/node/core/dispute-coordinator/src/real/mod.rs b/node/core/dispute-coordinator/src/real/mod.rs index 10220251573a..6a6c4f02d250 100644 --- a/node/core/dispute-coordinator/src/real/mod.rs +++ b/node/core/dispute-coordinator/src/real/mod.rs @@ -522,7 +522,7 @@ async fn handle_incoming( statements, pending_confirmation, } => { - handle_import_statements( + let outcome = handle_import_statements( ctx, overlay_db, state, @@ -531,10 +531,10 @@ async fn handle_incoming( session, statements, now, - pending_confirmation, metrics, ) .await?; + pending_confirmation.send(outcome).map_err(|_| Error::OneshotSend)?; }, DisputeCoordinatorMessage::RecentDisputes(rx) => { let recent_disputes = overlay_db.load_recent_disputes()?.unwrap_or_default(); @@ -633,14 +633,10 @@ async fn handle_import_statements( now: Timestamp, pending_confirmation: oneshot::Sender, metrics: &Metrics, -) -> Result<(), Error> { +) -> Result { if state.highest_session.map_or(true, |h| session + DISPUTE_WINDOW < h) { // It is not valid to participate in an ancient dispute (spam?). - pending_confirmation - .send(ImportStatementsResult::InvalidImport) - .map_err(|_| Error::OneshotSend)?; - - return Ok(()) + return Ok(ImportStatementsResult::InvalidImport) } let validators = match state.rolling_session_window.session_info(session) { @@ -651,11 +647,7 @@ async fn handle_import_statements( "Missing info for session which has an active dispute", ); - pending_confirmation - .send(ImportStatementsResult::InvalidImport) - .map_err(|_| Error::OneshotSend)?; - - return Ok(()) + return Ok(ImportStatementsResult::InvalidImport) }, Some(info) => info.validators.clone(), }; @@ -780,15 +772,12 @@ async fn handle_import_statements( // // We expect that if the candidate is truly disputed that the higher-level network // code will retry. - pending_confirmation - .send(ImportStatementsResult::InvalidImport) - .map_err(|_| Error::OneshotSend)?; tracing::debug!( target: LOG_TARGET, "Recovering availability failed - invalid import." ); - return Ok(()) + return Ok(ImportStatementsResult::InvalidImport) } metrics.on_open(); @@ -806,11 +795,7 @@ async fn handle_import_statements( overlay_db.write_candidate_votes(session, candidate_hash, votes.into()); - pending_confirmation - .send(ImportStatementsResult::ValidImport) - .map_err(|_| Error::OneshotSend)?; - - Ok(()) + Ok(ImportStatementsResult::ValidImport) } fn find_controlled_validator_indices( @@ -917,8 +902,7 @@ async fn issue_local_statement( // Do import if !statements.is_empty() { - let (pending_confirmation, rx) = oneshot::channel(); - handle_import_statements( + match handle_import_statements( ctx, overlay_db, state, @@ -927,11 +911,10 @@ async fn issue_local_statement( session, statements, now, - pending_confirmation, metrics, ) - .await?; - match rx.await { + .await? + { Err(_) => { tracing::error!( target: LOG_TARGET, diff --git a/parachain/test-parachains/adder/collator/README.md b/parachain/test-parachains/adder/collator/README.md index be5922b9f95a..4347a9a8ced7 100644 --- a/parachain/test-parachains/adder/collator/README.md +++ b/parachain/test-parachains/adder/collator/README.md @@ -1,12 +1,14 @@ # How to run this collator First start two validators that will run for the relay chain: + ```sh cargo run --release -- -d alice --chain rococo-local --validator --alice --port 50551 cargo run --release -- -d bob --chain rococo-local --validator --bob --port 50552 ``` Next start the collator that will collate for the adder parachain: + ```sh cargo run --release -p test-parachain-adder-collator -- --tmp --chain rococo-local --port 50553 ``` From 51ef8c0fd256a60de320e8b75d055f9f5259aa3a Mon Sep 17 00:00:00 2001 From: Lldenaurois Date: Wed, 29 Sep 2021 12:42:31 +0200 Subject: [PATCH 5/6] Update handle_import_statement function --- node/core/dispute-coordinator/src/real/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/node/core/dispute-coordinator/src/real/mod.rs b/node/core/dispute-coordinator/src/real/mod.rs index 6a6c4f02d250..b4a1437d7d55 100644 --- a/node/core/dispute-coordinator/src/real/mod.rs +++ b/node/core/dispute-coordinator/src/real/mod.rs @@ -631,7 +631,6 @@ async fn handle_import_statements( session: SessionIndex, statements: Vec<(SignedDisputeStatement, ValidatorIndex)>, now: Timestamp, - pending_confirmation: oneshot::Sender, metrics: &Metrics, ) -> Result { if state.highest_session.map_or(true, |h| session + DISPUTE_WINDOW < h) { @@ -913,7 +912,7 @@ async fn issue_local_statement( now, metrics, ) - .await? + .await { Err(_) => { tracing::error!( From dd6f22b0bb4e25075acec3004070571b19bbeade Mon Sep 17 00:00:00 2001 From: Andronik Ordian Date: Wed, 29 Sep 2021 12:49:16 +0200 Subject: [PATCH 6/6] fmt --- node/core/dispute-coordinator/src/real/tests.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/node/core/dispute-coordinator/src/real/tests.rs b/node/core/dispute-coordinator/src/real/tests.rs index 6832cf3eb321..c8709da92916 100644 --- a/node/core/dispute-coordinator/src/real/tests.rs +++ b/node/core/dispute-coordinator/src/real/tests.rs @@ -26,9 +26,7 @@ use overseer::TimeoutExt; use parity_scale_codec::Encode; use polkadot_node_subsystem::{ jaeger, - messages::{ - AllMessages, BlockDescription, RuntimeApiMessage, RuntimeApiRequest, - }, + messages::{AllMessages, BlockDescription, RuntimeApiMessage, RuntimeApiRequest}, ActivatedLeaf, ActiveLeavesUpdate, LeafStatus, }; use polkadot_node_subsystem_test_helpers::{make_subsystem_context, TestSubsystemContextHandle}; @@ -170,8 +168,7 @@ impl TestState { ))) .await; - self.handle_sync_queries(virtual_overseer, block_hash, session) - .await; + self.handle_sync_queries(virtual_overseer, block_hash, session).await; } async fn handle_sync_queries(