Guide updates for disputes. (#3401)

* Guide updates for disputes.

* Working availability recovery flood protection.

* More fixes.

* Formatting.

* Fix.

* Update roadmap/implementers-guide/src/node/disputes/dispute-participation.md

Co-authored-by: Sergei Shulepov <sergei@parity.io>

* Review remarks.

Co-authored-by: Sergei Shulepov <sergei@parity.io>

roadmap/implementers-guide/src/node/disputes/dispute-coordinator.md

@@ -59,6 +59,13 @@ struct State {
 }
 ```
 
+### On startup
+
+Check DB for recorded votes for non concluded disputes we have not yet
+recorded a local statement for.
+For all of those send `DisputeParticipationMessage::Participate` message to
+dispute participation subsystem.
+
 ### On `OverseerSignal::ActiveLeavesUpdate`
 
 For each leaf in the leaves update:
@@ -80,18 +87,39 @@ Do nothing.
 
 ### On `DisputeCoordinatorMessage::ImportStatement`
 
-* Deconstruct into parts `{ candidate_hash, candidate_receipt, session, statements }`.
-* If the session is earlier than `state.highest_session - DISPUTE_WINDOW`, return.
-* Load from underlying DB by querying `("candidate-votes", session, candidate_hash). If that does not exist, create fresh with the given candidate receipt.
-* If candidate votes is empty and the statements only contain dispute-specific votes, return.
-* Otherwise, if there is already an entry from the validator in the respective `valid` or `invalid` field of the `CandidateVotes`, return.
-* Add an entry to the respective `valid` or `invalid` list of the `CandidateVotes` for each statement in `statements`.
-* Write the `CandidateVotes` to the underyling DB.
-* If the both `valid` and `invalid` lists now have non-zero length where previously one or both had zero length, the candidate is now freshly disputed.
-* If freshly disputed, load `"active-disputes"` and add the candidate hash and session index. Also issue a [`DisputeParticipationMessage::Participate`][DisputeParticipationMessage].
-* If the dispute now has supermajority votes in the "valid" direction, according to the `SessionInfo` of the dispute candidate's session, remove from `"active-disputes"`.
-* If the dispute now has supermajority votes in the "invalid" direction, there is no need to do anything explicitly. The actual rollback will be handled during the active leaves update by observing digests from the runtime.
-* Write `"active-disputes"`
+1. Deconstruct into parts `{ candidate_hash, candidate_receipt, session, statements }`.
+2. If the session is earlier than `state.highest_session - DISPUTE_WINDOW`,
+   respond with `ImportStatementsResult::InvalidImport` and return.
+3. Load from underlying DB by querying `("candidate-votes", session,
+   candidate_hash)`.  If that does not exist, create fresh with the given
+   candidate receipt.
+4. If candidate votes is empty and the statements only contain dispute-specific
+   votes, respond with `ImportStatementsResult::InvalidImport` and return.
+5. Otherwise, if there is already an entry from the validator in the respective
+  `valid` or `invalid` field of the `CandidateVotes`,  respond with
+  `ImportStatementsResult::ValidImport` and return.
+6. Add an entry to the respective `valid` or `invalid` list of the
+   `CandidateVotes` for each statement in `statements`.
+7. If the both `valid` and `invalid` lists now became non-zero length where
+   previously one or both had zero length, the candidate is now freshly
+   disputed.
+8. If the candidate is not freshly disputed as determined by 7, continue with
+   10. If it is freshly disputed now, load `"active-disputes"` and add the
+   candidate hash and session index. Then, if we have local statements with
+   regards to that candidate,  also continue with 10. Otherwise proceed with 9.
+9. Issue a
+   [`DisputeParticipationMessage::Participate`][DisputeParticipationMessage].
+   Wait for response on the `report_availability` oneshot. If available, continue
+   with 10. If not send back `ImportStatementsResult::InvalidImport` and return.
+10. Write the `CandidateVotes` to the underyling DB.
+11. Send back `ImportStatementsResult::ValidImport`.
+12. If the dispute now has supermajority votes in the "valid" direction,
+    according to the `SessionInfo` of the dispute candidate's session, remove
+    from `"active-disputes"`.
+13. If the dispute now has supermajority votes in the "invalid" direction, there
+    is no need to do anything explicitly. The actual rollback will be handled
+    during the active leaves update by observing digests from the runtime.
+14. Write `"active-disputes"`
 
 ### On `DisputeCoordinatorMessage::ActiveDisputes`
 

roadmap/implementers-guide/src/node/disputes/dispute-distribution.md

 # Dispute Distribution
 
-Dispute distribution is responsible for ensuring all concerned validators will be aware of a dispute and have the relevant votes.
+Dispute distribution is responsible for ensuring all concerned validators will
+be aware of a dispute and have the relevant votes.
 
 ## Design Goals
 
@@ -35,33 +36,43 @@ Request:
 
 ```rust
 struct DisputeRequest {
-  // Either initiating invalid vote or our own (if we voted invalid).
-  invalid_vote: InvalidVote,
-  // Some invalid vote (can be from backing/approval) or our own if we voted
-  // valid.
-  valid_vote: ValidVote,
-}
+  /// The candidate being disputed.
+  pub candidate_receipt: CandidateReceipt,
+
+  /// The session the candidate appears in.
+  pub session_index: SessionIndex,
 
-struct InvalidVote {
-  subject: VoteSubject,
-  kind: InvalidDisputeStatementKind,
+  /// The invalid vote data that makes up this dispute.
+  pub invalid_vote: InvalidDisputeVote,
+
+  /// The valid vote that makes this dispute request valid.
+  pub valid_vote: ValidDisputeVote,
 }
 
-struct ValidVote {
-  subject: VoteSubject,
-  kind: ValidDisputeStatementKind,
+/// Any invalid vote (currently only explicit).
+pub struct InvalidDisputeVote {
+  /// The voting validator index.
+  pub validator_index: ValidatorIndex,
+
+  /// The validator signature, that can be verified when constructing a
+  /// `SignedDisputeStatement`.
+  pub signature: ValidatorSignature,
+
+  /// Kind of dispute statement.
+  pub kind: InvalidDisputeStatementKind,
 }
 
-struct VoteSubject {
-  /// The candidate being disputed.
-  candidate_hash: CandidateHash,
-  /// The voting validator.
-  validator_index: ValidatorIndex,
-  /// The session the candidate appears in.
-  candidate_session: SessionIndex,
+/// Any valid vote (backing, approval, explicit).
+pub struct ValidDisputeVote {
+  /// The voting validator index.
+  pub validator_index: ValidatorIndex,
+
   /// The validator signature, that can be verified when constructing a
   /// `SignedDisputeStatement`.
-  validator_signature: ValidatorSignature,
+  pub signature: ValidatorSignature,
+
+  /// Kind of dispute statement.
+  pub kind: ValidDisputeStatementKind,
 }
 ```
 
@@ -135,17 +146,11 @@ initially received `Invalid` vote.
 
 Note, that we rely on the coordinator to check availability for spam protection
 (see below).
-In case the current node is only a potential block producer and does not
-actually need to recover availability (as it is not going to participate in the
-dispute), there is a potential optimization available: The coordinator could
-first just check whether we have our piece and only if we don't, try to recover
-availability. Our node having a piece would be proof enough of the
-data to be available and thus the dispute to not be spam.
 
 ### Sending of messages
 
 Starting and participating in a dispute are pretty similar from the perspective
-of disptute distribution. Once we receive a `SendDispute` message we try to make
+of dispute distribution. Once we receive a `SendDispute` message we try to make
 sure to get the data out. We keep track of all the parachain validators that
 should see the message, which are all the parachain validators of the session
 where the dispute happened as they will want to participate in the dispute.  In
@@ -165,8 +170,8 @@ a dispute is no longer live, we will clean up the state accordingly.
 
 ### Reception & Spam Considerations
 
-Because we are not forwarding foreign statements, spam is not so much of an
-issue as in other subsystems. Rate limiting should be implemented at the
+Because we are not forwarding foreign statements, spam is less of an issue in
+comparison to gossip based systems. Rate limiting should be implemented at the
 substrate level, see
 [#7750](https://github.com/paritytech/substrate/issues/7750). Still we should
 make sure that it is not possible via spamming to prevent a dispute concluding
@@ -180,7 +185,9 @@ Considered attack vectors:
 2. An attacker can just flood us with notifications on any notification
    protocol, assuming flood protection is not effective enough, our unbounded
    buffers can fill up and we will run out of memory eventually.
-3. Attackers could spam us at a high rate with invalid disputes. Our incoming
+3. An attacker could participate in a valid dispute, but send its votes multiple
+   times.
+4. Attackers could spam us at a high rate with invalid disputes. Our incoming
    queue of requests could get monopolized by those malicious requests and we
    won't be able to import any valid disputes and we could run out of resources,
    if we tried to process them all in parallel.
@@ -194,15 +201,17 @@ For 2, we will pick up on any dispute on restart, so assuming that any realistic
 memory filling attack will take some time, we should be able to participate in a
 dispute under such attacks.
 
-For 3, full monopolization of the incoming queue should not be possible assuming
+Importing/discarding redundant votes should be pretty quick, so measures with
+regards to 4 should suffice to prevent 3, from doing any real harm.
+
+For 4, full monopolization of the incoming queue should not be possible assuming
 substrate handles incoming requests in a somewhat fair way. Still we want some
 defense mechanisms, at the very least we need to make sure to not exhaust
 resources.
 
-The dispute coordinator will notify us
-via `DisputeDistributionMessage::ReportCandidateUnavailable` about unavailable
-candidates and we can disconnect from such peers/decrease their reputation
-drastically. This alone should get us quite far with regards to queue
+The dispute coordinator will notify us on import about unavailable candidates or
+otherwise invalid imports and we can disconnect from such peers/decrease their
+reputation drastically. This alone should get us quite far with regards to queue
 monopolization, as availability recovery is expected to fail relatively quickly
 for unavailable data.
 
@@ -270,7 +279,7 @@ received a `SendDispute` message for that candidate.
 ## Backing and Approval Votes
 
 Backing and approval votes get imported when they arrive/are created via the
-distpute coordinator by corresponding subsystems.
+dispute coordinator by corresponding subsystems.
 
 We assume that under normal operation each node will be aware of backing and
 approval votes and optimize for that case. Nevertheless we want disputes to
@@ -346,6 +355,6 @@ dispute will succeed eventually, which is all that matters. And again, even if
 an attacker managed to prevent such a dispute from happening somehow, there is
 no real harm done: There was no serious attack to begin with.
 
-[DistputeDistributionMessage]: ../../types/overseer-protocol.md#dispute-distribution-message
+[DisputeDistributionMessage]: ../../types/overseer-protocol.md#dispute-distribution-message
 [RuntimeApiMessage]: ../../types/overseer-protocol.md#runtime-api-message
 [DisputeParticipationMessage]: ../../types/overseer-protocol.md#dispute-participation-message

roadmap/implementers-guide/src/node/disputes/dispute-participation.md

@@ -12,6 +12,7 @@ Output:
   - [RuntimeApiMessage][RuntimeApiMessage]
   - [CandidateValidationMessage][CandidateValidationMessage]
   - [AvailabilityRecoveryMessage][AvailabilityRecoveryMessage]
+  - [AvailabilityStoreMessage][AvailabilityStoreMessage]
   - [ChainApiMessage][ChainApiMessage]
 
 ## Functionality
@@ -40,6 +41,8 @@ Conclude.
 
 * Decompose into parts: `{ candidate_hash, candidate_receipt, session, voted_indices }`
 * Issue an [`AvailabilityRecoveryMessage::RecoverAvailableData`][AvailabilityRecoveryMessage]
+* Report back availability result to the `AvailabilityRecoveryMessage` sender
+  via the `report_availability` oneshot.
 * If the result is `Unavailable`, return.
 * If the result is `Invalid`, [cast invalid votes](#cast-votes) and return.
 * If the data is recovered, dispatch a [`RuntimeApiMessage::ValidationCodeByHash`][RuntimeApiMessage] with the parameters `(candidate_receipt.descriptor.validation_code_hash)` at `state.recent_block.hash`.

roadmap/implementers-guide/src/types/overseer-protocol.md

@@ -326,7 +326,7 @@ enum ChainApiMessage {
     BlockHeader(Hash, ResponseChannel<Result<Option<BlockHeader>, Error>>),
     /// Get the cumulative weight of the given block, by hash.
     /// If the block or weight is unknown, this returns `None`.
-    /// 
+    ///
     /// Weight is used for comparing blocks in a fork-choice rule.
     BlockWeight(Hash, ResponseChannel<Result<Option<Weight>, Error>>),
     /// Get the finalized block hash by number.
@@ -438,7 +438,7 @@ enum DisputeCoordinatorMessage {
         /// 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<()>,
+        pending_confirmation: oneshot::Sender<ImportStatementsResult>
     },
     /// Fetch a list of all active disputes that the co-ordinator is aware of.
     ActiveDisputes(ResponseChannel<Vec<(SessionIndex, CandidateHash)>>),
@@ -459,6 +459,14 @@ enum DisputeCoordinatorMessage {
         rx: ResponseSender<Option<(BlockNumber, BlockHash)>>,
     }
 }
+
+/// Result of `ImportStatements`.
+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
+}
 ```
 
 ## Dispute Participation Message
@@ -479,6 +487,9 @@ enum DisputeParticipationMessage {
         session: SessionIndex,
         /// The number of validators in the session.
         n_validators: u32,
+        /// Give immediate feedback on whether the candidate was available or
+        /// not.
+        report_availability: oneshot::Sender<bool>,
     }
 }
 ```
@@ -507,9 +518,6 @@ enum DisputeDistributionMessage {
     /// referenced session.
     from_validator: Option<ValidatorIndex>,
   }
-  /// Tell the subsystem that a candidate is not available. Dispute distribution
-  /// can punish peers distributing votes on unavailable hashes.
-  ReportCandidateUnavailable(CandidateHash),
 }
 ```