Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

dispute-coordinator: Cleanup + Bug fixes #5323

Merged
eskimor merged 13 commits into from
Apr 19, 2022
Merged

dispute-coordinator: Cleanup + Bug fixes #5323

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
1fd3f98
Make import confirmation oneshot optional.
eskimor Mar 24, 2022
1d509a5
Cleanup for further improvements.
eskimor Mar 28, 2022
7c29be2
Queue adoptions.
eskimor Apr 8, 2022
a2edaad
Fix fieldname
eskimor Apr 8, 2022
8008523
Use correct relay parent
eskimor Apr 8, 2022
6c2b2c2
Fix scraper tests.
eskimor Apr 13, 2022
219c67e
Small optimization.
eskimor Apr 13, 2022
b220963
Fix all tests.
eskimor Apr 13, 2022
84bb7b1
Fix other tests.
eskimor Apr 13, 2022
c8fce32
Merge branch 'master' into rk-better-on-chain-scraping
eskimor Apr 13, 2022
9a01681
fmt
eskimor Apr 13, 2022
3f3a60c
spelling
eskimor Apr 13, 2022
e8529ce
Fix warning.
eskimor Apr 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 1 addition & 4 deletions node/core/approval-voting/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -950,15 +950,12 @@ async fn handle_actions(
dispute_statement,
validator_index,
} => {
// TODO: Log confirmation results in an efficient way:
// https://github.com/paritytech/polkadot/issues/5156
let (pending_confirmation, _confirmation_rx) = oneshot::channel();
ctx.send_message(DisputeCoordinatorMessage::ImportStatements {
candidate_hash,
candidate_receipt,
session,
statements: vec![(dispute_statement, validator_index)],
pending_confirmation,
pending_confirmation: None,
})
.await;
},
Expand Down
4 changes: 1 addition & 3 deletions node/core/approval-voting/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ use polkadot_node_primitives::{
use polkadot_node_subsystem::{
messages::{
AllMessages, ApprovalVotingMessage, AssignmentCheckResult, AvailabilityRecoveryMessage,
ImportStatementsResult,
},
ActivatedLeaf, ActiveLeavesUpdate, LeafStatus,
};
Expand Down Expand Up @@ -605,11 +604,10 @@ async fn check_and_import_approval(
overseer_recv(overseer).await,
AllMessages::DisputeCoordinator(DisputeCoordinatorMessage::ImportStatements {
candidate_hash: c_hash,
pending_confirmation,
pending_confirmation: None,
..
}) => {
assert_eq!(c_hash, candidate_hash);
let _ = pending_confirmation.send(ImportStatementsResult::ValidImport);
}
);
}
Expand Down
5 changes: 1 addition & 4 deletions node/core/backing/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -887,16 +887,13 @@ impl CandidateBackingJob {
if let (Some(candidate_receipt), Some(dispute_statement)) =
(maybe_candidate_receipt, maybe_signed_dispute_statement)
{
// TODO: Log confirmation results in an efficient way:
// https://github.com/paritytech/polkadot/issues/5156
let (pending_confirmation, _confirmation_rx) = oneshot::channel();
sender
.send_message(DisputeCoordinatorMessage::ImportStatements {
candidate_hash,
candidate_receipt,
session: self.session_index,
statements: vec![(dispute_statement, validator_index)],
pending_confirmation,
pending_confirmation: None,
})
.await;
}
Expand Down
7 changes: 2 additions & 5 deletions node/core/backing/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ use polkadot_primitives::v2::{
ScheduledCore,
};
use polkadot_subsystem::{
messages::{
CollatorProtocolMessage, ImportStatementsResult, RuntimeApiMessage, RuntimeApiRequest,
},
messages::{CollatorProtocolMessage, RuntimeApiMessage, RuntimeApiRequest},
ActivatedLeaf, ActiveLeavesUpdate, FromOverseer, LeafStatus, OverseerSignal,
};
use sp_application_crypto::AppKey;
Expand Down Expand Up @@ -284,15 +282,14 @@ async fn test_dispute_coordinator_notifications(
candidate_receipt: c_receipt,
session: s,
statements,
pending_confirmation,
pending_confirmation: None,
}
) => {
assert_eq!(c_hash, candidate_hash);
assert_eq!(c_receipt.hash(), c_hash);
assert_eq!(s, session);
assert_eq!(statements.len(), 1);
assert_eq!(statements[0].1, validator_index);
let _ = pending_confirmation.send(ImportStatementsResult::ValidImport);
}
)
}
Expand Down
1 change: 1 addition & 0 deletions node/core/dispute-coordinator/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-keystore = { git = "https://github.com/paritytech/substrate", branch = "master" }
assert_matches = "1.4.0"
test-helpers = { package = "polkadot-primitives-test-helpers", path = "../../../primitives/test-helpers" }
futures-timer = "3.0.2"

[features]
# If not enabled, the dispute coordinator will do nothing.
Expand Down
4 changes: 2 additions & 2 deletions node/core/dispute-coordinator/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub enum Error {
#[error(transparent)]
Oneshot(#[from] oneshot::Canceled),

#[error("Dispute import confirmation send failed (receiver canceled)")]
#[error("Could not send import confirmation (receiver canceled)")]
DisputeImportOneshotSend,

#[error(transparent)]
Expand Down Expand Up @@ -118,7 +118,7 @@ impl JfyiError {
pub fn log(self) {
match self {
// don't spam the log with spurious errors
Self::Runtime(_) | Self::Oneshot(_) | Self::DisputeImportOneshotSend => {
Copy link
Member Author

@eskimor eskimor Apr 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can warn again, because senders who don't care will pass in None now for pending_confirmation

Self::Runtime(_) | Self::Oneshot(_) => {
gum::debug!(target: LOG_TARGET, error = ?self)
},
// it's worth reporting otherwise
Expand Down
Loading