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

Dispute distribution guide #3158

Merged
merged 12 commits into from
Jun 22, 2021
Merged

Dispute distribution guide #3158

merged 12 commits into from
Jun 22, 2021

Conversation

eskimor
Copy link
Member

@eskimor eskimor commented Jun 2, 2021

Note: Guide PR needs update. The following things have changed:

There is no separate message for reporting unavailable candidates anymore, instead ImportStatements got a more informative back channel:

		ImportStatements {
		/// The hash of the candidate.
		candidate_hash: CandidateHash,
		/// The candidate receipt itself.
		candidate_receipt: CandidateReceipt,
		/// The session the candidate appears in.
		session: SessionIndex,
		/// Statements, with signatures checked, by validators participating in disputes.
		///
		/// The validator index passed alongside each statement should correspond to the index
		/// of the validator in the set.
		statements: Vec<(SignedDisputeStatement, ValidatorIndex)>,
		/// Inform the requester once we finished importing.
		///
		/// This is, we either discarded the votes, just record them because we
		/// casted our vote already or recovered availability for the candidate
		/// successfully.
		pending_confirmation: oneshot::Sender<ImportStatementsResult>
	},

with ImportStatementsResult looking like this:

pub enum ImportStatementsResult {
	/// Import was invalid (candidate was not available)  and the sending peer should get banned.
	InvalidImport,
	/// Import was valid and can be confirmed to peer.
	ValidImport
}

Details in implementation PR.

@eskimor eskimor added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Jun 2, 2021
@eskimor eskimor requested a review from rphmeier June 2, 2021 16:23
@eskimor eskimor changed the title Dispute distribution initial design. Dispute distribution guide Jun 12, 2021
statements so they can include them in blocks.

We keep track of connected parachain validators and authorities and will issue
warnings in the logs if connected nodes are less than two thirds of the
Copy link
Contributor

Choose a reason for hiding this comment

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

What are validator operators meant to do in this situation?

Copy link
Member Author

Choose a reason for hiding this comment

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

On a live network - check their internet connection, on a test network: find the bug. Like we had the case already a couple of times that we were not connected properly and we did only realize because of other issues that have been caused by this. Dispute distribution already tries its best - it will try to send requests, even if the receiver is not connected to us and it will keep trying.

I was just thinking about disputes and because they are so critical, I wanted to do whatever I can to ensure our messages gets out, but general connection warnings should go to gossip support already, I guess. And warnings when a dispute is already happening are a bit late - still useful though as a additional safety guard though. If validators become aware of a dispute that did not work out for some weird reasons/bugs/whatever we still have governance - better than nobody noticing.

Copy link
Contributor

@rphmeier rphmeier left a comment

Choose a reason for hiding this comment

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

Looks good modulo nits. I think the spam-resilience strategy makes sense and should protect against any practical attacks.

@rphmeier rphmeier merged commit d2e21f3 into master Jun 22, 2021
@rphmeier rphmeier deleted the rk-dispute-distribution branch June 22, 2021 23:23
@rphmeier
Copy link
Contributor

Follow-up changes should go in another PR. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants