From fa94317a21131f15d0dcdf242d2098a83ffdbf54 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 13 Nov 2020 16:17:44 -0500 Subject: [PATCH 01/17] skeleton flow control --- .../src/node/approval/approval-networking.md | 58 ++++++++++++++++++- 1 file changed, 56 insertions(+), 2 deletions(-) diff --git a/roadmap/implementers-guide/src/node/approval/approval-networking.md b/roadmap/implementers-guide/src/node/approval/approval-networking.md index 558d4447c954..a0c55a4fbe11 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-networking.md +++ b/roadmap/implementers-guide/src/node/approval/approval-networking.md @@ -1,7 +1,61 @@ # Approval Networking -> TODO: +The [Approval Voting](approval-voting.md) subsystem is responsible for active participation in a protocol designed to select a sufficient number of validators to check each and every candidate which appears in the relay chain. Statements of participation in this checking process are divided into two kinds: + - **Assignments** indicate that validators have been selected to do checking + - **Approvals** indicate that validators have checked and found the candidate satisfactory. + +The [Approval Voting](approval-voting.md) subsystem handles all the issuing and tallying of this protocol, but this subsystem is responsible for the disbursal of statements among the validator-set. + +We implement this protocol as a gossip protocol, and like other parachain-related gossip protocols our primary concerns are about ensuring fast message propagation while maintaining an upper bound on the number of messages any given node must store at any time. + +Approval messages should always follow assignments, so we need to be able to discern two pieces of information based on our [View](../../types/network.md#universal-types): + 1. Is a particular assignment relevant under a given `View`? + 2. Is a particular approval relevant to any assignment in a set? + +These two queries need not be perfect, but they must never yield false positives. For our own local view, they should not yield false negatives. When applied to our peers' views, it is acceptable for them to yield false negatives. The reason for that is that our peers' views may be beyond ours, and we are not capable of fully evaluating them. Once we have caught up, we can check again for false negatives to continue distributing. + +For assignments, what we need to be checking is whether we are aware of the (block, candidate) pair that the assignment references. For approvals, we need to be aware of an assignment by the same validator which references the candidate being approved. + +However, awareness on its own of a (block, candidate) pair would imply that even ancient candidates all the way back to the genesis are relevant. We are actually not interested in anything before finality. did + ## Protocol -## Functionality \ No newline at end of file +## Functionality + +### Network updates + +#### `NetworkBridgeEvent::PeerConnected` + +TODO: add a peer entry to state + +#### `NetworkBridgeEvent::PeerDisconnected` + +TODO: remove peer entry from state + +#### `NetworkBridgeEvent::PeerViewChange` + +TODO: find all blocks in common with the peer. + +#### `NetworkBridgeEvent::OurViewChange` + +TODO: find all blocks in common with all peers. + +#### `NetworkBridgeEvent::PeerMessage` + +TODO: provide step-by-step for each of these checks. + +If the message is an assignment, + * Check if the peer is already aware of the assignment. If so, ignore & report. + * Check if we are already aware of this assignment. If so, ignore, but note that the peer is aware of the assignment also. Give a small reputation bump. Note that if we don't accept the assignment, we will not be aware of it in our state and will proceed to the next step. + * Check if we accept this assignment. If not, ignore & report. + * Issue an `ApprovalVotingMessage::CheckAndImportAssignment`. If the result is `VoteCheckResult::Bad`, ignore & report. If the result is `VoteCheckResult::Ignore`, just ignore. If the result is `VoteCheckResult::Accepted`, store the assignment and note that the peer is aware of the assignment. + * Distribute the assignment to all peers who will accept it. + * Distribute any relevant approvals to all peers who will accept them. We could not send peers approval messages before they're aware of an assignment. + +If the message is an approval, + * Check if the peer is already aware of the approval. If so, ignore & report. + * Check if we are already aware of this approval. If so, ignore, but note that the peer is aware of the approval also. Give a small reputation bump. Note that if we don't accept the approval, we will not be aware of it in our state and will proceed to the next step. + * Check if we accept this approval. If not, ignore & report. + * Issue an `ApprovalVotingMessage::CheckAndImportApproval`. If the result is `VoteCheckResult::Bad`, ignore & report. If the result is `VoteCheckResult::Ignore`, just ignore. If the result is `VoteCheckResult::Accepted`, store the approval and note that the peer is aware of the approval. + * Distribute the approval to all peers who will accept it. \ No newline at end of file From 5fd754aa56ea3ffa52ad82349e9c3bfa13f4eeda Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 13 Nov 2020 16:20:41 -0500 Subject: [PATCH 02/17] tweaks & rename to approvals distribution --- roadmap/implementers-guide/src/SUMMARY.md | 2 +- ...l-networking.md => approval-distribution.md} | 4 +++- .../src/node/approval/approval-voting.md | 8 ++++---- .../src/runtime/approvals_inherent.md | 1 + .../implementers-guide/src/types/approval.md | 6 ++---- roadmap/implementers-guide/src/types/network.md | 9 +++++++++ .../src/types/overseer-protocol.md | 17 ++++++++++------- 7 files changed, 30 insertions(+), 17 deletions(-) rename roadmap/implementers-guide/src/node/approval/{approval-networking.md => approval-distribution.md} (96%) create mode 100644 roadmap/implementers-guide/src/runtime/approvals_inherent.md diff --git a/roadmap/implementers-guide/src/SUMMARY.md b/roadmap/implementers-guide/src/SUMMARY.md index f37fc08f964a..03928cfff86e 100644 --- a/roadmap/implementers-guide/src/SUMMARY.md +++ b/roadmap/implementers-guide/src/SUMMARY.md @@ -46,7 +46,7 @@ - [Bitfield Signing](node/availability/bitfield-signing.md) - [Approval Subsystems](node/approval/README.md) - [Approval Voting](node/approval/approval-voting.md) - - [Approval Networking](node/approval/approval-networking.md) + - [Approval Distribution](node/approval/approval-distribution.md) - [Dispute Participation](node/approval/dispute-participation.md) - [Utility Subsystems](node/utility/README.md) - [Availability Store](node/utility/availability-store.md) diff --git a/roadmap/implementers-guide/src/node/approval/approval-networking.md b/roadmap/implementers-guide/src/node/approval/approval-distribution.md similarity index 96% rename from roadmap/implementers-guide/src/node/approval/approval-networking.md rename to roadmap/implementers-guide/src/node/approval/approval-distribution.md index a0c55a4fbe11..e8fbbfdd552b 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-networking.md +++ b/roadmap/implementers-guide/src/node/approval/approval-distribution.md @@ -1,4 +1,6 @@ -# Approval Networking +# Approval Distribution + +A subsystem for the distribution of assignments and approvals for approval checks on candidates over the network. The [Approval Voting](approval-voting.md) subsystem is responsible for active participation in a protocol designed to select a sufficient number of validators to check each and every candidate which appears in the relay chain. Statements of participation in this checking process are divided into two kinds: - **Assignments** indicate that validators have been selected to do checking diff --git a/roadmap/implementers-guide/src/node/approval/approval-voting.md b/roadmap/implementers-guide/src/node/approval/approval-voting.md index 72bb9d075c0b..441a53937ae1 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-voting.md +++ b/roadmap/implementers-guide/src/node/approval/approval-voting.md @@ -10,8 +10,8 @@ Input: - `ApprovalVotingMessage::ApprovedAncestor` Output: - - `ApprovalNetworkingMessage::DistributeAssignment` - - `ApprovalNetworkingMessage::DistributeApproval` + - `ApprovalDistributionMessage::DistributeAssignment` + - `ApprovalDistributionMessage::DistributeApproval` - `RuntimeApiMessage::Request` - `ChainApiMessage` - `AvailabilityRecoveryMessage::Recover` @@ -221,7 +221,7 @@ On receiving an `ApprovedAncestor(Hash, BlockNumber, response_channel)`: * Set `n_tranches = tranches_to_approve(approval_entry)` * If `OurAssignment` has tranche `<= n_tranches`, the tranche is live according to our local clock (based against block slot), and we have not triggered the assignment already * Import to `ApprovalEntry` - * Broadcast on network with an `ApprovalNetworkingMessage::DistributeAssignment`. + * Broadcast on network with an `ApprovalDistributionMessage::DistributeAssignment`. * Kick off approval work with `launch_approval` * Schedule another wakeup based on `next_wakeup` @@ -244,4 +244,4 @@ On receiving an `ApprovedAncestor(Hash, BlockNumber, response_channel)`: * Construct a `SignedApprovalVote` with the validator index for the session. * Transform into an `IndirectSignedApprovalVote` using the `block_hash` and `candidate_index` from the request. * `import_checked_approval(block_entry, candidate_entry, validator_index)` - * Dispatch an `ApprovalNetworkingMessage::DistributeApproval` message. + * Dispatch an `ApprovalDistributionMessage::DistributeApproval` message. diff --git a/roadmap/implementers-guide/src/runtime/approvals_inherent.md b/roadmap/implementers-guide/src/runtime/approvals_inherent.md new file mode 100644 index 000000000000..db9588c2794d --- /dev/null +++ b/roadmap/implementers-guide/src/runtime/approvals_inherent.md @@ -0,0 +1 @@ +# ApprovalsInherent Module diff --git a/roadmap/implementers-guide/src/types/approval.md b/roadmap/implementers-guide/src/types/approval.md index 1f44dc43ffd7..f6a6f898802d 100644 --- a/roadmap/implementers-guide/src/types/approval.md +++ b/roadmap/implementers-guide/src/types/approval.md @@ -17,11 +17,9 @@ These certificates can be checked in the context of a specific block, candidate, ```rust enum AssignmentCertKind { RelayVRFModulo { - relay_vrf: (VRFInOut, VRFProof), sample: u32, }, RelayVRFDelay { - relay_vrf: (VRFInOut, VRFProof), core_index: CoreIndex, } } @@ -30,7 +28,7 @@ struct AssignmentCert { // The criterion which is claimed to be met by this cert. kind: AssignmentCertKind, // The VRF showing the criterion is met. - vrf: VRFInOut, + vrf: (VRFPreOut, VRFProof), } ``` @@ -58,7 +56,7 @@ struct SignedApprovalVote { A signed approval vote which references the candidate indirectly via the block. If there exists a look-up to the candidate hash from the block hash and candidate index, then this can be transformed into a `SignedApprovalVote`. -Although this vote references the candidate by a specific block hash and candidate index, the vote actually applies to +Although this vote references the candidate by a specific block hash and candidate index, the signature is computed on the actual `SignedApprovalVote` payload. ```rust struct IndirectSignedApprovalVote { diff --git a/roadmap/implementers-guide/src/types/network.md b/roadmap/implementers-guide/src/types/network.md index 75f251613f25..861d70dbf19d 100644 --- a/roadmap/implementers-guide/src/types/network.md +++ b/roadmap/implementers-guide/src/types/network.md @@ -58,6 +58,14 @@ enum StatementDistributionV1Message { } ``` +### Approval Distribution V1 + +```rust +enum ApprovalDistributionV1Message { + // TODO [now] +} +``` + ### Collator Protocol V1 ```rust @@ -82,6 +90,7 @@ These are the messages for the protocol on the validation peer-set. ```rust enum ValidationProtocolV1 { + ApprovalDistribution(ApprovalDistributionV1Message), AvailabilityDistribution(AvailabilityDistributionV1Message), BitfieldDistribution(BitfieldDistributionV1Message), PoVDistribution(PoVDistributionV1Message), diff --git a/roadmap/implementers-guide/src/types/overseer-protocol.md b/roadmap/implementers-guide/src/types/overseer-protocol.md index 484d53b70f4a..b78c64b20acf 100644 --- a/roadmap/implementers-guide/src/types/overseer-protocol.md +++ b/roadmap/implementers-guide/src/types/overseer-protocol.md @@ -40,9 +40,9 @@ struct ActiveLeavesUpdate { Messages received by the approval voting subsystem. ```rust -enum VoteCheckResult { +enum VoteCheckResult { // The vote was accepted and should be propagated onwards. - Accepted, + Accepted(T), // The vote was bad and should be ignored, reporting the peer who propagated it. Bad, // We do not have enough information to evaluate the vote. Ignore but don't report. @@ -53,11 +53,14 @@ enum VoteCheckResult { enum ApprovalVotingMessage { /// Check if the assignment is valid and can be accepted by our view of the protocol. /// Should not be sent unless the block hash is known. + /// + /// If accepted, the payload of the `VoteCheckResult` is the block hash + /// and candidate index of the assignment. CheckAndImportAssignment( Hash, AssignmentCert, ValidatorIndex, - ResponseChannel, + ResponseChannel>, ), /// Check if the approval vote is valid and can be accepted by our view of the /// protocol. @@ -65,7 +68,7 @@ enum ApprovalVotingMessage { /// Should not be sent unless the block hash within the indirect vote is known. CheckAndImportApproval( IndirectSignedApprovalVote, - ResponseChannel, + ResponseChannel>, ), /// Returns the highest possible ancestor hash of the provided block hash which is /// acceptable to vote on finality for. @@ -78,12 +81,12 @@ enum ApprovalVotingMessage { } ``` -## Approval Networking +## Approval Distribution -Messages received by the approval networking subsystem. +Messages received by the approval Distribution subsystem. ```rust -enum ApprovalNetworkingMessage { +enum ApprovalDistributionMessage { /// Distribute an assignment cert from the local validator. The cert is assumed /// to be valid for the given relay-parent and validator index. DistributeAssignment(Hash, AssignmentCert, ValidatorIndex), From 9f60c322e81ad134ef55252dcfffe8d5264c9df2 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 16 Nov 2020 10:21:19 -0500 Subject: [PATCH 03/17] Update roadmap/implementers-guide/src/node/approval/approval-distribution.md Co-authored-by: Peter Goodspeed-Niklaus --- .../src/node/approval/approval-distribution.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/roadmap/implementers-guide/src/node/approval/approval-distribution.md b/roadmap/implementers-guide/src/node/approval/approval-distribution.md index e8fbbfdd552b..368573e7243d 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-distribution.md +++ b/roadmap/implementers-guide/src/node/approval/approval-distribution.md @@ -18,7 +18,7 @@ These two queries need not be perfect, but they must never yield false positives For assignments, what we need to be checking is whether we are aware of the (block, candidate) pair that the assignment references. For approvals, we need to be aware of an assignment by the same validator which references the candidate being approved. -However, awareness on its own of a (block, candidate) pair would imply that even ancient candidates all the way back to the genesis are relevant. We are actually not interested in anything before finality. did +However, awareness on its own of a (block, candidate) pair would imply that even ancient candidates all the way back to the genesis are relevant. We are actually not interested in anything before finality. ## Protocol @@ -60,4 +60,4 @@ If the message is an approval, * Check if we are already aware of this approval. If so, ignore, but note that the peer is aware of the approval also. Give a small reputation bump. Note that if we don't accept the approval, we will not be aware of it in our state and will proceed to the next step. * Check if we accept this approval. If not, ignore & report. * Issue an `ApprovalVotingMessage::CheckAndImportApproval`. If the result is `VoteCheckResult::Bad`, ignore & report. If the result is `VoteCheckResult::Ignore`, just ignore. If the result is `VoteCheckResult::Accepted`, store the approval and note that the peer is aware of the approval. - * Distribute the approval to all peers who will accept it. \ No newline at end of file + * Distribute the approval to all peers who will accept it. From 755ea5f781829fdc432b52082ec852239e18fc07 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 16 Nov 2020 10:21:25 -0500 Subject: [PATCH 04/17] Update roadmap/implementers-guide/src/node/approval/approval-distribution.md Co-authored-by: Peter Goodspeed-Niklaus --- .../src/node/approval/approval-distribution.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roadmap/implementers-guide/src/node/approval/approval-distribution.md b/roadmap/implementers-guide/src/node/approval/approval-distribution.md index 368573e7243d..0106cbfa5f1b 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-distribution.md +++ b/roadmap/implementers-guide/src/node/approval/approval-distribution.md @@ -14,7 +14,7 @@ Approval messages should always follow assignments, so we need to be able to dis 1. Is a particular assignment relevant under a given `View`? 2. Is a particular approval relevant to any assignment in a set? -These two queries need not be perfect, but they must never yield false positives. For our own local view, they should not yield false negatives. When applied to our peers' views, it is acceptable for them to yield false negatives. The reason for that is that our peers' views may be beyond ours, and we are not capable of fully evaluating them. Once we have caught up, we can check again for false negatives to continue distributing. +These two queries need not be perfect, but they must never yield false positives. For our own local view, they must not yield false negatives. When applied to our peers' views, it is acceptable for them to yield false negatives. The reason for that is that our peers' views may be beyond ours, and we are not capable of fully evaluating them. Once we have caught up, we can check again for false negatives to continue distributing. For assignments, what we need to be checking is whether we are aware of the (block, candidate) pair that the assignment references. For approvals, we need to be aware of an assignment by the same validator which references the candidate being approved. From ae542b94f5cb25732cbe3ce7d6214f3f6a1a3576 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 16 Nov 2020 20:08:45 -0500 Subject: [PATCH 05/17] add a `NewBlocks` message and dispatch --- .../src/node/approval/approval-voting.md | 5 +++-- .../implementers-guide/src/types/overseer-protocol.md | 10 ++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/roadmap/implementers-guide/src/node/approval/approval-voting.md b/roadmap/implementers-guide/src/node/approval/approval-voting.md index 441a53937ae1..5fd90a555d96 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-voting.md +++ b/roadmap/implementers-guide/src/node/approval/approval-voting.md @@ -72,7 +72,6 @@ struct BlockEntry { block_hash: Hash, session: SessionIndex, slot: SlotNumber, - received_late_by: Duration, // random bytes derived from the VRF submitted within the block by the block // author as a credential and used as input to approval assignment criteria. relay_vrf_story: [u8; 32], @@ -156,6 +155,7 @@ On receiving an `OverseerSignal::ActiveLeavesUpdate(update)`: * Only if not a member of the backing group. * Run `RelayVRFModulo` and `RelayVRFDelay` according to the [the approvals protocol section](../../protocol-approval.md#assignment-criteria) * invoke `process_wakeup(relay_block, candidate)` for each new candidate in each new block - this will automatically broadcast a 0-tranche assignment, kick off approval work, and schedule the next delay. + * Dispatch an `ApprovalDistributionMessage::NewBlocks` with the meta information filled out for each new block. #### `ApprovalVotingMessage::CheckAndImportAssignment` @@ -166,6 +166,7 @@ On receiving a `ApprovalVotingMessage::CheckAndImportAssignment` message, we che * Check the assignment cert * If the cert kind is `RelayVRFModulo`, then the certificate is valid as long as `sample < session_info.relay_vrf_samples` and the VRF is valid for the validator's key with the input `block_entry.relay_vrf_story ++ sample.encode()` as described with [the approvals protocol section](../../protocol-approval.md#assignment-criteria). We set `core_index = vrf.make_bytes().to_u32() % session_info.n_cores`. If the `BlockEntry` causes inclusion of a candidate at `core_index`, then this is a valid assignment for the candidate at `core_index` and has delay tranche 0. Otherwise, it can be ignored. * If the cert kind is `RelayVRFDelay`, then we check if the VRF is valid for the validator's key with the input `block_entry.relay_vrf_story ++ cert.core_index.encode()` as described in [the approvals protocol section](../../protocol-approval.md#assignment-criteria). The cert can be ignored if the block did not cause inclusion of a candidate on that core index. Otherwise, this is a valid assignment for the included candidate. The delay tranche for the assignment is determined by reducing `(vrf.make_bytes().to_u64() % (session_info.n_delay_tranches + session_info.zeroth_delay_tranche_width)).saturating_sub(session_info.zeroth_delay_tranche_width)`. + * If the delay tranche is too far in the future, return `VoteCheckResult::Ignore`. * `import_checked_assignment` * return the appropriate `VoteCheckResult` on the response channel. @@ -222,7 +223,7 @@ On receiving an `ApprovedAncestor(Hash, BlockNumber, response_channel)`: * If `OurAssignment` has tranche `<= n_tranches`, the tranche is live according to our local clock (based against block slot), and we have not triggered the assignment already * Import to `ApprovalEntry` * Broadcast on network with an `ApprovalDistributionMessage::DistributeAssignment`. - * Kick off approval work with `launch_approval` + * Kick off approval work with `launch_approval`. Note that if the candidate appears in multiple current blocks, we will launch approval for each block it appears in. It may make sense to shortcut around this with caching either at this level or on the level of the other subsystems invoked by that function. * Schedule another wakeup based on `next_wakeup` #### `next_wakeup(approval_entry, candidate_entry)`: diff --git a/roadmap/implementers-guide/src/types/overseer-protocol.md b/roadmap/implementers-guide/src/types/overseer-protocol.md index b78c64b20acf..6816733eeec8 100644 --- a/roadmap/implementers-guide/src/types/overseer-protocol.md +++ b/roadmap/implementers-guide/src/types/overseer-protocol.md @@ -86,7 +86,17 @@ enum ApprovalVotingMessage { Messages received by the approval Distribution subsystem. ```rust +struct BlockApprovalMeta { + hash: Hash, + number: BlockNumber, + candidates: Vec, + slot_number: SlotNumber, +} + enum ApprovalDistributionMessage { + /// Notify the `ApprovalDistribution` subsystem about new blocks and the candidates contained within + /// them. + NewBlocks(Vec), /// Distribute an assignment cert from the local validator. The cert is assumed /// to be valid for the given relay-parent and validator index. DistributeAssignment(Hash, AssignmentCert, ValidatorIndex), From 328e8346adf59dd771e409e2c6abc67156887ce6 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 16 Nov 2020 20:46:43 -0500 Subject: [PATCH 06/17] new data format for approval distribution --- .../node/approval/approval-distribution.md | 68 +++++++++++++++++-- 1 file changed, 61 insertions(+), 7 deletions(-) diff --git a/roadmap/implementers-guide/src/node/approval/approval-distribution.md b/roadmap/implementers-guide/src/node/approval/approval-distribution.md index e8fbbfdd552b..9bb8c0fb0c19 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-distribution.md +++ b/roadmap/implementers-guide/src/node/approval/approval-distribution.md @@ -8,6 +8,8 @@ The [Approval Voting](approval-voting.md) subsystem is responsible for active pa The [Approval Voting](approval-voting.md) subsystem handles all the issuing and tallying of this protocol, but this subsystem is responsible for the disbursal of statements among the validator-set. +The inclusion pipeline of candidates concludes after availability, and only after inclusion do candidates actually get pushed into the approval checking pipeline. As such, this protocol deals with the candidates _made available by_ particular blocks, as opposed to the candidates which actually appear within those blocks, which are the candidates _backed by_ those blocks. Unless stated otherwise, whenever we reference a candidate partially by block hash, we are referring to the set of candidates _made available by_ those blocks. + We implement this protocol as a gossip protocol, and like other parachain-related gossip protocols our primary concerns are about ensuring fast message propagation while maintaining an upper bound on the number of messages any given node must store at any time. Approval messages should always follow assignments, so we need to be able to discern two pieces of information based on our [View](../../types/network.md#universal-types): @@ -25,31 +27,75 @@ However, awareness on its own of a (block, candidate) pair would imply that even ## Functionality +```rust +type BlockScopedCandidate = (Hash, CandidateHash); + +/// The `State` struct is responsible for tracking the overall state of the subsystem. +/// +/// It tracks metadata about our view of the chain, which assignments and approvals we have seen, and our peers' views. +struct State { + block_view: HashMap>, + blocks: HashMap, + peer_views: HashMap, + finalized_number: BlockNumber, +} + +/// Information about blocks in our current view as well as whether peers know of them. +struct BlockEntry { + // Peers who we know are aware of this block and thus, the candidates within it. + known_by: HashSet, + // The parent hash of the block. + parent_hash: Hash, + // A votes entry for each candidate. + candidates: IndexMap, +} + +enum ApprovalState { + Assigned(AssignmentCert), + Approved(AssignmentCert, ApprovalSignature), +} + +/// Information about candidates in the context of a particular block they are included in. In other words, +/// multiple `CandidateEntry`s may exist for the same candidate, if it is included by multiple blocks - this is likely the case /// when there are forks. +struct CandidateEntry { + approvals: HashMap, +} +``` + ### Network updates #### `NetworkBridgeEvent::PeerConnected` -TODO: add a peer entry to state +Add a blank view to the `peer_views` state. #### `NetworkBridgeEvent::PeerDisconnected` -TODO: remove peer entry from state +Remove the view under the associated `PeerId` from `State::peer_views`. + +TODO: pruning? hard to see how to do it without just iterating over each `BlockEntry` but that might be rel. fast. #### `NetworkBridgeEvent::PeerViewChange` -TODO: find all blocks in common with the peer. +For each block in the view: + 1. Initialize `fresh_blocks = {}` + 2. Load the `BlockEntry` for the block. If unknown, go to step 6. + 3. Inspect the `known_by` set. If the peer is already present, go to step 6. + 4. Add the peer to `known_by` and add the hash of the block to `fresh_blocks`. + 5. Repeat steps 2-4 with the ancestor of the block. + 6. For each block in `fresh_blocks`, send all assignments and approvals for all candidates in those blocks to the peer. #### `NetworkBridgeEvent::OurViewChange` -TODO: find all blocks in common with all peers. +Do nothing. + +TODO: Technically the information we get from `OurViewChange` can conflict with `OurViewChange`. We could add a flag to `BlockEntry` to indicate whether we've broadcast them in our view and not accept stuff that we haven't broadcast maybe. But that seems a little egregious. We could base `BlockEntry` pruning on the finality information in `OurViewChange` though. #### `NetworkBridgeEvent::PeerMessage` TODO: provide step-by-step for each of these checks. If the message is an assignment, - * Check if the peer is already aware of the assignment. If so, ignore & report. - * Check if we are already aware of this assignment. If so, ignore, but note that the peer is aware of the assignment also. Give a small reputation bump. Note that if we don't accept the assignment, we will not be aware of it in our state and will proceed to the next step. + * Check if we are already aware of this assignment. If so, ignore, but note that the peer is aware of the assignment also. If the peer was already aware of the assignment, give a minor reputation punishment. Otherwise, give a small reputation bump. Note that if we don't accept the assignment, we will not be aware of it in our state and will proceed to the next step. * Check if we accept this assignment. If not, ignore & report. * Issue an `ApprovalVotingMessage::CheckAndImportAssignment`. If the result is `VoteCheckResult::Bad`, ignore & report. If the result is `VoteCheckResult::Ignore`, just ignore. If the result is `VoteCheckResult::Accepted`, store the assignment and note that the peer is aware of the assignment. * Distribute the assignment to all peers who will accept it. @@ -60,4 +106,12 @@ If the message is an approval, * Check if we are already aware of this approval. If so, ignore, but note that the peer is aware of the approval also. Give a small reputation bump. Note that if we don't accept the approval, we will not be aware of it in our state and will proceed to the next step. * Check if we accept this approval. If not, ignore & report. * Issue an `ApprovalVotingMessage::CheckAndImportApproval`. If the result is `VoteCheckResult::Bad`, ignore & report. If the result is `VoteCheckResult::Ignore`, just ignore. If the result is `VoteCheckResult::Accepted`, store the approval and note that the peer is aware of the approval. - * Distribute the approval to all peers who will accept it. \ No newline at end of file + * Distribute the approval to all peers who will accept it. + + +### Subsystem Updates + +#### `AvailabilityDistributionMessage::NewBlocks` + +TODO: create `BlockEntry` and `CandidateEntries` for all blocks. +TODO: check for new commonality with peers. Broadcast new information to peers we now understand better. From febd83a917c7d2d38ccf6a655ceb5d8fb16c1bc5 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 16 Nov 2020 23:29:29 -0500 Subject: [PATCH 07/17] guide: update view to include finalized block number --- .../implementers-guide/src/node/utility/network-bridge.md | 6 ++++++ roadmap/implementers-guide/src/types/network.md | 7 ++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/roadmap/implementers-guide/src/node/utility/network-bridge.md b/roadmap/implementers-guide/src/node/utility/network-bridge.md index 5ace56b2a9c8..d89ec7e8fe42 100644 --- a/roadmap/implementers-guide/src/node/utility/network-bridge.md +++ b/roadmap/implementers-guide/src/node/utility/network-bridge.md @@ -61,6 +61,12 @@ The `activated` and `deactivated` lists determine the evolution of our local vie If we are connected to the same peer on both peer-sets, we will send the peer two view updates as a result. +### Overseer Signal: BlockFinalized + +We obtain the number of the block hash in the event by issuing a `ChainApiMessage::BlockNumber` request and then issue a `ProtocolMessage::ViewUpdate` to each connected peer on each peer-set. We also issue a `NetworkBridgeEvent::OurViewChange` to each event handler for each protocol. + +If we are connected to the same peer on both peer-sets, we will send the peer two view updates as a result. + ### Network Event: Peer Connected Issue a `NetworkBridgeEvent::PeerConnected` for each [Event Handler](#event-handlers) of the peer-set and negotiated protocol version of the peer. diff --git a/roadmap/implementers-guide/src/types/network.md b/roadmap/implementers-guide/src/types/network.md index 861d70dbf19d..bc0bfecbe583 100644 --- a/roadmap/implementers-guide/src/types/network.md +++ b/roadmap/implementers-guide/src/types/network.md @@ -8,7 +8,12 @@ These types are those that are actually sent over the network to subsystems. type RequestId = u64; type ProtocolVersion = u32; struct PeerId(...); // opaque, unique identifier of a peer. -struct View(Vec); // Up to `N` (5?) chain heads. +struct View { + // Up to `N` (5?) chain heads. + heads: Vec, + // The number of the finalized block. + finalized_number: BlockNumber, +} enum ObservedRole { Full, From 33922dc1a1d26f8b6c7df708a435e9e295284028 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 16 Nov 2020 23:48:51 -0500 Subject: [PATCH 08/17] approvals: document view updating --- .../node/approval/approval-distribution.md | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/roadmap/implementers-guide/src/node/approval/approval-distribution.md b/roadmap/implementers-guide/src/node/approval/approval-distribution.md index b52c60f6c190..6671ce308320 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-distribution.md +++ b/roadmap/implementers-guide/src/node/approval/approval-distribution.md @@ -34,7 +34,7 @@ type BlockScopedCandidate = (Hash, CandidateHash); /// /// It tracks metadata about our view of the chain, which assignments and approvals we have seen, and our peers' views. struct State { - block_view: HashMap>, + blocks_by_number: BTreeMap>, blocks: HashMap, peer_views: HashMap, finalized_number: BlockNumber, @@ -44,6 +44,8 @@ struct State { struct BlockEntry { // Peers who we know are aware of this block and thus, the candidates within it. known_by: HashSet, + // The number of the block. + number: BlockNumber, // The parent hash of the block. parent_hash: Hash, // A votes entry for each candidate. @@ -78,17 +80,23 @@ TODO: pruning? hard to see how to do it without just iterating over each `BlockE For each block in the view: 1. Initialize `fresh_blocks = {}` - 2. Load the `BlockEntry` for the block. If unknown, go to step 6. + 2. Load the `BlockEntry` for the block. If the block is unknown, or the number is less than the view's finalized number, go to step 6. 3. Inspect the `known_by` set. If the peer is already present, go to step 6. 4. Add the peer to `known_by` and add the hash of the block to `fresh_blocks`. - 5. Repeat steps 2-4 with the ancestor of the block. + 5. Return to step 2 with the ancestor of the block. 6. For each block in `fresh_blocks`, send all assignments and approvals for all candidates in those blocks to the peer. -#### `NetworkBridgeEvent::OurViewChange` +We also need to use the `view.finalized_number` to remove the `PeerId` from any blocks that it won't be wanting information about anymore. Note that we have to be on guard for peers doing crazy stuff like jumping their 'finalized_number` forward 10 trillion blocks to try and get us stuck in a loop for ages. + +One of the safeguards we can implement is to reject view updates from peers where the new `finalized_number` is less than the previous. + +We augment that by defining `constrain(x)` to output the x bounded by the first and last numbers in `state.blocks_by_number`. -Do nothing. +From there, we can loop backwards from `constrain(view.finalized_number)` until `constrain(last_view.finalized_number)` is reached, removing the `PeerId` from all `BlockEntry`s referenced at that height. We can break the loop early if we ever exit the bound supplied by the first block in `state.blocks_by_number`. + +#### `NetworkBridgeEvent::OurViewChange` -TODO: Technically the information we get from `OurViewChange` can conflict with `OurViewChange`. We could add a flag to `BlockEntry` to indicate whether we've broadcast them in our view and not accept stuff that we haven't broadcast maybe. But that seems a little egregious. We could base `BlockEntry` pruning on the finality information in `OurViewChange` though. +Prune all lists from `blocks_by_number` with number less than or equal to `view.finalized_number`. Prune all the `BlockEntry`s referenced by those lists. #### `NetworkBridgeEvent::PeerMessage` From 3492743002fcbf063869a0952278ec1e86c1c56e Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 16 Nov 2020 23:49:18 -0500 Subject: [PATCH 09/17] pruning when peers disconnect --- .../src/node/approval/approval-distribution.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roadmap/implementers-guide/src/node/approval/approval-distribution.md b/roadmap/implementers-guide/src/node/approval/approval-distribution.md index 6671ce308320..7b22e0ec52e2 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-distribution.md +++ b/roadmap/implementers-guide/src/node/approval/approval-distribution.md @@ -74,7 +74,7 @@ Add a blank view to the `peer_views` state. Remove the view under the associated `PeerId` from `State::peer_views`. -TODO: pruning? hard to see how to do it without just iterating over each `BlockEntry` but that might be rel. fast. +Iterate over every `BlockEntry` and remove `PeerId` from it. #### `NetworkBridgeEvent::PeerViewChange` From 3f1b3dbbaff7c8d40b41e9846395f26ec882b8eb Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 16 Nov 2020 23:52:38 -0500 Subject: [PATCH 10/17] add remaining message types --- .../src/node/approval/approval-distribution.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/roadmap/implementers-guide/src/node/approval/approval-distribution.md b/roadmap/implementers-guide/src/node/approval/approval-distribution.md index 7b22e0ec52e2..9feaf39045b2 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-distribution.md +++ b/roadmap/implementers-guide/src/node/approval/approval-distribution.md @@ -118,7 +118,15 @@ If the message is an approval, ### Subsystem Updates -#### `AvailabilityDistributionMessage::NewBlocks` +#### `ApprovalDistributionMessage::NewBlocks` TODO: create `BlockEntry` and `CandidateEntries` for all blocks. TODO: check for new commonality with peers. Broadcast new information to peers we now understand better. + +#### `ApprovalDistributionMessage::DistributeAsignment` + +Load the corresponding `BlockEntry`. Distribute to all peers in `known_by`. Add to the corresponding `CandidateEntry`. + +#### `ApprovalDistributionMessage::DistributeApproval` + +Load the corresponding `BlockEntry`. Distribute to all peers in `known_by`. Add to the corresponding `CandidateEntry`. \ No newline at end of file From eba1d50865da3523f4a1ebacd5e65e3f16bba823 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 17 Nov 2020 00:01:12 -0500 Subject: [PATCH 11/17] fix link --- roadmap/implementers-guide/src/node/approval/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roadmap/implementers-guide/src/node/approval/README.md b/roadmap/implementers-guide/src/node/approval/README.md index 41ba527f1b64..2d0815376728 100644 --- a/roadmap/implementers-guide/src/node/approval/README.md +++ b/roadmap/implementers-guide/src/node/approval/README.md @@ -2,6 +2,6 @@ The approval subsystems implement the node-side of the [Approval Protocol](../../protocol-approval.md). -We make a divide between the [assignment/voting logic](approval-voting.md) and the [networking](approval-networking.md) that distributes assignment certifications and approval votes. The logic in the assignment and voting also informs the GRANDPA voting rule on how to vote. +We make a divide between the [assignment/voting logic](approval-voting.md) and the [distribution logic](approval-distribution.md) that distributes assignment certifications and approval votes. The logic in the assignment and voting also informs the GRANDPA voting rule on how to vote. This category of subsystems also contains a module for [participating in live disputes](dispute-participation.md) and tracks all observed votes (backing or approval) by all validators on all candidates. \ No newline at end of file From 4db5ec9b637440af31dae6239c78c261bc7077c8 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 17 Nov 2020 00:01:20 -0500 Subject: [PATCH 12/17] network message type --- roadmap/implementers-guide/src/types/network.md | 11 +++++++++++ .../implementers-guide/src/types/overseer-protocol.md | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/roadmap/implementers-guide/src/types/network.md b/roadmap/implementers-guide/src/types/network.md index bc0bfecbe583..2d762d9b09a7 100644 --- a/roadmap/implementers-guide/src/types/network.md +++ b/roadmap/implementers-guide/src/types/network.md @@ -23,6 +23,17 @@ enum ObservedRole { ## V1 Network Subsystem Message Types +### Approval Distribution V1 + +```rust +enum ApprovalDistributionV1Message { + /// An assignment for a candidate in some recent, unfinalized block. + Assignment(Hash, u32, AssignmentCert, ValidatorIndex), + /// An approval for a candidate in some recent, unfinalized block. + Approval(Hash, u32, ApprovalSignature), +} +``` + ### Availability Distribution V1 ```rust diff --git a/roadmap/implementers-guide/src/types/overseer-protocol.md b/roadmap/implementers-guide/src/types/overseer-protocol.md index 6816733eeec8..18048898cc84 100644 --- a/roadmap/implementers-guide/src/types/overseer-protocol.md +++ b/roadmap/implementers-guide/src/types/overseer-protocol.md @@ -99,7 +99,7 @@ enum ApprovalDistributionMessage { NewBlocks(Vec), /// Distribute an assignment cert from the local validator. The cert is assumed /// to be valid for the given relay-parent and validator index. - DistributeAssignment(Hash, AssignmentCert, ValidatorIndex), + DistributeAssignment(Hash, u32, AssignmentCert, ValidatorIndex), /// Distribute an approval vote for the local validator. DistributeApproval(IndirectApprovalVote), } From acf164fd22c94d76ac1fa4217b43f43742de248b Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 17 Nov 2020 20:05:11 -0500 Subject: [PATCH 13/17] handle incoming assignments --- .../node/approval/approval-distribution.md | 70 ++++++++++++++----- .../src/node/approval/approval-voting.md | 5 +- .../implementers-guide/src/types/approval.md | 13 ++++ .../implementers-guide/src/types/network.md | 7 +- .../src/types/overseer-protocol.md | 25 ++++--- 5 files changed, 89 insertions(+), 31 deletions(-) diff --git a/roadmap/implementers-guide/src/node/approval/approval-distribution.md b/roadmap/implementers-guide/src/node/approval/approval-distribution.md index 9feaf39045b2..27e136b16f8f 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-distribution.md +++ b/roadmap/implementers-guide/src/node/approval/approval-distribution.md @@ -40,14 +40,25 @@ struct State { finalized_number: BlockNumber, } +enum MessageFingerprint { + Assigment(Hash, u32, ValidatorIndex), + Approval(Hash, u32, ValidatorIndex), +} + +struct Knowledge { + known_messages: HashSet, +} + /// Information about blocks in our current view as well as whether peers know of them. struct BlockEntry { - // Peers who we know are aware of this block and thus, the candidates within it. - known_by: HashSet, + // Peers who we know are aware of this block and thus, the candidates within it. This maps to their knowledge of messages. + known_by: HashMap, // The number of the block. number: BlockNumber, // The parent hash of the block. parent_hash: Hash, + // Our knowledge of messages. + knowledge: Knowledge, // A votes entry for each candidate. candidates: IndexMap, } @@ -100,21 +111,9 @@ Prune all lists from `blocks_by_number` with number less than or equal to `view. #### `NetworkBridgeEvent::PeerMessage` -TODO: provide step-by-step for each of these checks. - -If the message is an assignment, - * Check if we are already aware of this assignment. If so, ignore, but note that the peer is aware of the assignment also. If the peer was already aware of the assignment, give a minor reputation punishment. Otherwise, give a small reputation bump. Note that if we don't accept the assignment, we will not be aware of it in our state and will proceed to the next step. - * Check if we accept this assignment. If not, ignore & report. - * Issue an `ApprovalVotingMessage::CheckAndImportAssignment`. If the result is `VoteCheckResult::Bad`, ignore & report. If the result is `VoteCheckResult::Ignore`, just ignore. If the result is `VoteCheckResult::Accepted`, store the assignment and note that the peer is aware of the assignment. - * Distribute the assignment to all peers who will accept it. - * Distribute any relevant approvals to all peers who will accept them. We could not send peers approval messages before they're aware of an assignment. +If the message is of type `ApprovalDistributionV1Message::Assignment(assignment_cert, claimed_index)`, then call `import_and_circulate_assignment(MessageSource::Peer(sender), assignment_cert, claimed_index)` -If the message is an approval, - * Check if the peer is already aware of the approval. If so, ignore & report. - * Check if we are already aware of this approval. If so, ignore, but note that the peer is aware of the approval also. Give a small reputation bump. Note that if we don't accept the approval, we will not be aware of it in our state and will proceed to the next step. - * Check if we accept this approval. If not, ignore & report. - * Issue an `ApprovalVotingMessage::CheckAndImportApproval`. If the result is `VoteCheckResult::Bad`, ignore & report. If the result is `VoteCheckResult::Ignore`, just ignore. If the result is `VoteCheckResult::Accepted`, store the approval and note that the peer is aware of the approval. - * Distribute the approval to all peers who will accept it. +If the message is of type `ApprovalDistributionV1Message::Approval(approval_vote)`, then call `import_and_circulate_approval(MessageSource::Peer(sender), approval_vote)` ### Subsystem Updates @@ -129,4 +128,41 @@ Load the corresponding `BlockEntry`. Distribute to all peers in `known_by`. Add #### `ApprovalDistributionMessage::DistributeApproval` -Load the corresponding `BlockEntry`. Distribute to all peers in `known_by`. Add to the corresponding `CandidateEntry`. \ No newline at end of file +Load the corresponding `BlockEntry`. Distribute to all peers in `known_by`. Add to the corresponding `CandidateEntry`. + +### Utility + +```rust +enum MessageSource { + Peer(PeerId), + Local, +} +``` + +#### `import_and_circulate_assignment(source: MessageSource, assignment: IndirectAssignmentCert, claimed_candidate_index: u32)` + +Imports an assignment cert referenced by block hash and candidate index. As a postcondition, if the cert is valid, it will have distributed the cert to all peers who have the block in their view, with the exclusion of the peer referenced by the `MessageSource`. + + * Load the BlockEntry using `assignment.block_hash`. + * Compute a fingerprint for the `assignment` using `claimed_candidate_index`. + * If the source is `MessageSource::Peer(sender)`: + * check if `peer` appears under `known_by` and whether the fingerprint is in the `known_messages` of the peer. If the peer does not know the block, report for providing data out-of-view and proceed. If the peer does know the block and the knowledge contains the fingerprint, report for providing replicate data and return. + * If the message fingerprint appears under the `BlockEntry`'s `Knowledge`, give the peer a small positive reputation boost and return. Note that we must do this after checking for out-of-view to avoid being spammed. + * Dispatch `ApprovalVotingMessage::CheckAndImportAssignment(assignment.block_hash, assignment.cert, assignment.validator)` + * If the result is `VoteCheckResult::Accepted(candidate_index)` or `VoteCheckResult::AcceptedDuplicate(candidate_index)` + * If `candidate_index` does not match `claimed_candidate_index`, punish the peer's reputation, recompute the fingerprint, and re-do our knowledge checks. The goal here is to accept messages which peers send us that are labeled wrongly, but punish them for it as they've made us do extra work. + * Otherwise, if the vote was accepted but not duplicate, give the peer a positive reputation boost. + * add the fingerprint to both our and the peer's knowledge in the `BlockEntry`. Note that we only doing this after making sure we have the right fingerprint. + * If the result is `VoteCheckResult::TooFarInFuture`, mildly punish the peer and return. + * If the result is `VoteCheckResult::Ignore`, return. + * If the result is `VoteCheckResult::Bad`, punish the peer and return. + * If the source is `MessageSource::Local(CandidateIndex)` + * check if the fingerprint appears under the `BlockEntry's` knowledge. If not, add it. + * Load the candidate entry for the given candidate index. It should exist unless there is a logic error in the approval voting subsystem. + * Set the approval state for the validator index to `ApprovalState::Assigned` unless the approval state is set already. This should not happen as long as the approval voting subsystem instructs us to ignore duplicate assignments. + * Dispatch a `ApprovalDistributionV1Message::Assignment(assignment)` to all peers in the `BlockEntry`'s `known_by` set, excluding the peer in the `source`, if `source` has kind `MessageSource::Peer`. Add the fingerprint of the assignment to the knowledge of each peer. + + +#### `import_and_circulate_approval(source: MessageSource, approval: IndirectSignedApprovalVote)` + +Imports an approval signature referenced by block hash and candidate index. \ No newline at end of file diff --git a/roadmap/implementers-guide/src/node/approval/approval-voting.md b/roadmap/implementers-guide/src/node/approval/approval-voting.md index 5fd90a555d96..bc101cb24859 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-voting.md +++ b/roadmap/implementers-guide/src/node/approval/approval-voting.md @@ -193,6 +193,7 @@ On receiving an `ApprovedAncestor(Hash, BlockNumber, response_channel)`: #### `import_checked_assignment` * Load the candidate in question and access the `approval_entry` for the block hash the cert references. + * Ignore if we already observe the validator as having been assigned. * Ensure the validator index is not part of the backing group for the candidate. * Ensure the validator index is not present in the approval entry already. * Create a tranche entry for the delay tranche in the approval entry and note the assignment within it. @@ -243,6 +244,6 @@ On receiving an `ApprovedAncestor(Hash, BlockNumber, response_channel)`: #### `issue_approval(request)`: * Fetch the block entry and candidate entry. Ignore if `None` - we've probably just lost a race with finality. * Construct a `SignedApprovalVote` with the validator index for the session. - * Transform into an `IndirectSignedApprovalVote` using the `block_hash` and `candidate_index` from the request. * `import_checked_approval(block_entry, candidate_entry, validator_index)` - * Dispatch an `ApprovalDistributionMessage::DistributeApproval` message. + * Construct a `IndirectSignedApprovalVote` using the informatio about the vote. + * Dispatch `ApprovalDistributionMessage::DistributeApproval`. diff --git a/roadmap/implementers-guide/src/types/approval.md b/roadmap/implementers-guide/src/types/approval.md index f6a6f898802d..9f1e421e6d79 100644 --- a/roadmap/implementers-guide/src/types/approval.md +++ b/roadmap/implementers-guide/src/types/approval.md @@ -34,6 +34,19 @@ struct AssignmentCert { > TODO: RelayEquivocation cert. Probably can only be broadcast to chains that have handled an equivocation report. +## IndirectAssignmentCert + +An assignment cert which refers to the candidate under which the assignment is relevant by block hash. + +```rust +struct IndirectAssignmentCert { + // A block hash where the candidate appears. + block_hash: Hash, + validator: ValidatorIndex, + cert: AssignmentCert, +} +``` + ## ApprovalVote A vote of approval on a candidate. diff --git a/roadmap/implementers-guide/src/types/network.md b/roadmap/implementers-guide/src/types/network.md index 2d762d9b09a7..3770d914005e 100644 --- a/roadmap/implementers-guide/src/types/network.md +++ b/roadmap/implementers-guide/src/types/network.md @@ -28,9 +28,12 @@ enum ObservedRole { ```rust enum ApprovalDistributionV1Message { /// An assignment for a candidate in some recent, unfinalized block. - Assignment(Hash, u32, AssignmentCert, ValidatorIndex), + /// + /// The u32 is the claimed index of the candidate this assignment corresponds to. Actually checking the assignment + /// may yield a different result. + Assignment(IndirectAssignmentCert, u32), /// An approval for a candidate in some recent, unfinalized block. - Approval(Hash, u32, ApprovalSignature), + Approval(IndirectSignedApprovalVote), } ``` diff --git a/roadmap/implementers-guide/src/types/overseer-protocol.md b/roadmap/implementers-guide/src/types/overseer-protocol.md index 18048898cc84..f4c35373a48c 100644 --- a/roadmap/implementers-guide/src/types/overseer-protocol.md +++ b/roadmap/implementers-guide/src/types/overseer-protocol.md @@ -43,6 +43,10 @@ Messages received by the approval voting subsystem. enum VoteCheckResult { // The vote was accepted and should be propagated onwards. Accepted(T), + // The vote was valid but duplicate and should not be propagated onwards. + AcceptedDuplicate(T), + // The vote was valid but too far in the future to accept right now. + TooFarInFuture, // The vote was bad and should be ignored, reporting the peer who propagated it. Bad, // We do not have enough information to evaluate the vote. Ignore but don't report. @@ -54,13 +58,10 @@ enum ApprovalVotingMessage { /// Check if the assignment is valid and can be accepted by our view of the protocol. /// Should not be sent unless the block hash is known. /// - /// If accepted, the payload of the `VoteCheckResult` is the block hash - /// and candidate index of the assignment. + /// If accepted, the payload of the `VoteCheckResult` is the candidate index of the assignment. CheckAndImportAssignment( - Hash, - AssignmentCert, - ValidatorIndex, - ResponseChannel>, + IndirectAssignmentCert, + ResponseChannel>, ), /// Check if the approval vote is valid and can be accepted by our view of the /// protocol. @@ -98,10 +99,14 @@ enum ApprovalDistributionMessage { /// them. NewBlocks(Vec), /// Distribute an assignment cert from the local validator. The cert is assumed - /// to be valid for the given relay-parent and validator index. - DistributeAssignment(Hash, u32, AssignmentCert, ValidatorIndex), - /// Distribute an approval vote for the local validator. - DistributeApproval(IndirectApprovalVote), + /// to be valid, relevant, and for the given relay-parent and validator index. + /// + /// The `u32` param is the candidate index in the fully-included list. + DistributeAssignment(IndirectAssignmentCert, u32), + /// Distribute an approval vote for the local validator. The approval vote is assumed to be + /// valid, relevant, and the corresponding approval already issued. If not, the subsystem is free to drop + /// the message. + DistributeApproval(IndirectSignedApprovalVote), } ``` From ee28e9048bdd34f275981c4756fbb4602463e3d4 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 17 Nov 2020 20:27:57 -0500 Subject: [PATCH 14/17] import_and_circulate_approval --- .../node/approval/approval-distribution.md | 37 ++++++++++++++----- .../src/node/approval/approval-voting.md | 10 ++--- .../src/types/overseer-protocol.md | 22 ++++++----- 3 files changed, 44 insertions(+), 25 deletions(-) diff --git a/roadmap/implementers-guide/src/node/approval/approval-distribution.md b/roadmap/implementers-guide/src/node/approval/approval-distribution.md index 27e136b16f8f..d3180f8e6d9a 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-distribution.md +++ b/roadmap/implementers-guide/src/node/approval/approval-distribution.md @@ -143,26 +143,43 @@ enum MessageSource { Imports an assignment cert referenced by block hash and candidate index. As a postcondition, if the cert is valid, it will have distributed the cert to all peers who have the block in their view, with the exclusion of the peer referenced by the `MessageSource`. - * Load the BlockEntry using `assignment.block_hash`. + * Load the BlockEntry using `assignment.block_hash`. If it does not exist, report the source if it is `MessageSource::Peer` and return. * Compute a fingerprint for the `assignment` using `claimed_candidate_index`. * If the source is `MessageSource::Peer(sender)`: * check if `peer` appears under `known_by` and whether the fingerprint is in the `known_messages` of the peer. If the peer does not know the block, report for providing data out-of-view and proceed. If the peer does know the block and the knowledge contains the fingerprint, report for providing replicate data and return. - * If the message fingerprint appears under the `BlockEntry`'s `Knowledge`, give the peer a small positive reputation boost and return. Note that we must do this after checking for out-of-view to avoid being spammed. - * Dispatch `ApprovalVotingMessage::CheckAndImportAssignment(assignment.block_hash, assignment.cert, assignment.validator)` - * If the result is `VoteCheckResult::Accepted(candidate_index)` or `VoteCheckResult::AcceptedDuplicate(candidate_index)` + * If the message fingerprint appears under the `BlockEntry`'s `Knowledge`, give the peer a small positive reputation boost and return. Note that we must do this after checking for out-of-view to avoid being spammed. If we did this check earlier, a peer could provide data out-of-view repeatedly and be rewarded for it. + * Dispatch `ApprovalVotingMessage::CheckAndImportAssignment(assignment)` and wait for the response. + * If the result is `AssignmentCheckResult::Accepted(candidate_index)` or `AssignmentCheckResult::AcceptedDuplicate(candidate_index)` * If `candidate_index` does not match `claimed_candidate_index`, punish the peer's reputation, recompute the fingerprint, and re-do our knowledge checks. The goal here is to accept messages which peers send us that are labeled wrongly, but punish them for it as they've made us do extra work. - * Otherwise, if the vote was accepted but not duplicate, give the peer a positive reputation boost. + * Otherwise, if the vote was accepted but not duplicate, give the peer a positive reputation boost * add the fingerprint to both our and the peer's knowledge in the `BlockEntry`. Note that we only doing this after making sure we have the right fingerprint. - * If the result is `VoteCheckResult::TooFarInFuture`, mildly punish the peer and return. - * If the result is `VoteCheckResult::Ignore`, return. - * If the result is `VoteCheckResult::Bad`, punish the peer and return. + * If the result is `AssignmentCheckResult::TooFarInFuture`, mildly punish the peer and return. + * If the result is `AssignmentCheckResult::Bad`, punish the peer and return. * If the source is `MessageSource::Local(CandidateIndex)` * check if the fingerprint appears under the `BlockEntry's` knowledge. If not, add it. * Load the candidate entry for the given candidate index. It should exist unless there is a logic error in the approval voting subsystem. * Set the approval state for the validator index to `ApprovalState::Assigned` unless the approval state is set already. This should not happen as long as the approval voting subsystem instructs us to ignore duplicate assignments. - * Dispatch a `ApprovalDistributionV1Message::Assignment(assignment)` to all peers in the `BlockEntry`'s `known_by` set, excluding the peer in the `source`, if `source` has kind `MessageSource::Peer`. Add the fingerprint of the assignment to the knowledge of each peer. + * Dispatch a `ApprovalDistributionV1Message::Assignment(assignment, candidate_index)` to all peers in the `BlockEntry`'s `known_by` set, excluding the peer in the `source`, if `source` has kind `MessageSource::Peer`. Add the fingerprint of the assignment to the knowledge of each peer. #### `import_and_circulate_approval(source: MessageSource, approval: IndirectSignedApprovalVote)` -Imports an approval signature referenced by block hash and candidate index. \ No newline at end of file +Imports an approval signature referenced by block hash and candidate index. + + * Load the BlockEntry using `approval.block_hash` and the candidate entry using `approval.candidate_entry`. If either does not exist, report the source if it is `MessageSource::Peer` and return. + * Compute a fingerprint for the approval. + * Compute a fingerprint for the corresponding assignment. If the `BlockEntry`'s knowledge does not contain that fingerprint, then report the source if it is `MessageSource::Peer` and return. All references to a fingerprint after this refer to the approval's, not the assignment's. + * If the source is `MessageSource::Peer(sender)`: + * check if `peer` appears under `known_by` and whether the fingerprint is in the `known_messages` of the peer. If the peer does not know the block, report for providing data out-of-view and proceed. If the peer does know the block and the knowledge contains the fingerprint, report for providing replicate data and return. + * If the message fingerprint appears under the `BlockEntry`'s `Knowledge`, give the peer a small positive reputation boost and return. Note that we must do this after checking for out-of-view to avoid being spammed. If we did this check earlier, a peer could provide data out-of-view repeatedly and be rewarded for it. + * Dispatch `ApprovalVotingMessage::CheckAndImportApproval(approval)` and wait for the response. + * If the result is `VoteCheckResult::Accepted(())`: + * Give the peer a positive reputation boost and add the fingerprint to both our and the peer's knowledge. + * If the result is `VoteCheckResult::Bad`: + * Report the peer and return. + * Load the candidate entry for the given candidate index. It should exist unless there is a logic error in the approval voting subsystem. + * Set the approval state for the validator index to `ApprovalState::Approved`. It should already be in the `Assigned` state as our `BlockEntry` knowledge contains a fingerprint for the assignment. + * Dispatch a `ApprovalDistributionV1Message::Approval(approval)` to all peers in the `BlockEntry`'s `known_by` set, excluding the peer in the `source`, if `source` has kind `MessageSource::Peer`. Add the fingerprint of the assignment to the knowledge of each peer. Note that this obeys the politeness conditions: + * We guarantee elsewhere that all peers within `known_by` are aware of all assignments relative to the block. + * We've checked that this specific approval has a corresponding assignment within the `BlockEntry`. + * Thus, all peers are aware of the assignment or have a message to them in-flight which will make them so. \ No newline at end of file diff --git a/roadmap/implementers-guide/src/node/approval/approval-voting.md b/roadmap/implementers-guide/src/node/approval/approval-voting.md index bc101cb24859..7ff4981d1e4f 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-voting.md +++ b/roadmap/implementers-guide/src/node/approval/approval-voting.md @@ -173,10 +173,10 @@ On receiving a `ApprovalVotingMessage::CheckAndImportAssignment` message, we che #### `ApprovalVotingMessage::CheckAndImportApproval` On receiving a `CheckAndImportApproval(indirect_approval_vote, response_channel)` message: - * Fetch the `BlockEntry` from the indirect approval vote's `block_hash`. If none, return `VoteCheckResult::Bad`. - * Fetch the `CandidateEntry` from the indirect approval vote's `candidate_index`. If the block did not trigger inclusion of enough candidates, return `VoteCheckResult::Bad`. - * Construct a `SignedApprovalVote` using the candidate hash and check against the validator's approval key, based on the session info of the block. If invalid or no such validator, return `VoteCheckResult::Bad`. - * Send `VoteCheckResult::Accepted`, + * Fetch the `BlockEntry` from the indirect approval vote's `block_hash`. If none, return `ApprovalCheckResult::Bad`. + * Fetch the `CandidateEntry` from the indirect approval vote's `candidate_index`. If the block did not trigger inclusion of enough candidates, return `ApprovalCheckResult::Bad`. + * Construct a `SignedApprovalVote` using the candidate hash and check against the validator's approval key, based on the session info of the block. If invalid or no such validator, return `ApprovalCheckResult::Bad`. + * Send `ApprovalCheckResult::Accepted` * `import_checked_approval(BlockEntry, CandidateEntry, ValidatorIndex)` #### `ApprovalVotingMessage::ApprovedAncestor` @@ -200,7 +200,7 @@ On receiving an `ApprovedAncestor(Hash, BlockNumber, response_channel)`: * Note the candidate index within the approval entry. #### `import_checked_approval(BlockEntry, CandidateEntry, ValidatorIndex)` - * Set the corresponding bit of the `approvals` bitfield in the `CandidateEntry` to `1`. + * Set the corresponding bit of the `approvals` bitfield in the `CandidateEntry` to `1`. If already `1`, return. * For each `ApprovalEntry` in the `CandidateEntry` (typically only 1), check whether the validator is assigned as a checker. * If so, set `n_tranches = tranches_to_approve(approval_entry)`. * If `check_approval(block_entry, approval_entry, n_tranches)` is true, set the corresponding bit in the `block_entry.approved_bitfield`. diff --git a/roadmap/implementers-guide/src/types/overseer-protocol.md b/roadmap/implementers-guide/src/types/overseer-protocol.md index f4c35373a48c..ba0e6dc15e21 100644 --- a/roadmap/implementers-guide/src/types/overseer-protocol.md +++ b/roadmap/implementers-guide/src/types/overseer-protocol.md @@ -40,28 +40,30 @@ struct ActiveLeavesUpdate { Messages received by the approval voting subsystem. ```rust -enum VoteCheckResult { +enum AssignmentCheckResult { // The vote was accepted and should be propagated onwards. - Accepted(T), + Accepted(u32), // The vote was valid but duplicate and should not be propagated onwards. - AcceptedDuplicate(T), + AcceptedDuplicate(u32), // The vote was valid but too far in the future to accept right now. TooFarInFuture, // The vote was bad and should be ignored, reporting the peer who propagated it. Bad, - // We do not have enough information to evaluate the vote. Ignore but don't report. - // This should occur primarily on startup. - Ignore, +} + +enum ApprovalCheckResult { + // The vote was accepted and should be propagated onwards. + Accepted, + // The vote was bad and should be ignored, reporting the peer who propagated it. + Bad, } enum ApprovalVotingMessage { /// Check if the assignment is valid and can be accepted by our view of the protocol. /// Should not be sent unless the block hash is known. - /// - /// If accepted, the payload of the `VoteCheckResult` is the candidate index of the assignment. CheckAndImportAssignment( IndirectAssignmentCert, - ResponseChannel>, + ResponseChannel, ), /// Check if the approval vote is valid and can be accepted by our view of the /// protocol. @@ -69,7 +71,7 @@ enum ApprovalVotingMessage { /// Should not be sent unless the block hash within the indirect vote is known. CheckAndImportApproval( IndirectSignedApprovalVote, - ResponseChannel>, + ResponseChannel, ), /// Returns the highest possible ancestor hash of the provided block hash which is /// acceptable to vote on finality for. From 739bc98066a4b89130b5a4e5693177aa212a6c5d Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 17 Nov 2020 21:08:40 -0500 Subject: [PATCH 15/17] handle new blocks --- .../node/approval/approval-distribution.md | 28 ++++++++++++------- .../implementers-guide/src/types/network.md | 8 +++--- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/roadmap/implementers-guide/src/node/approval/approval-distribution.md b/roadmap/implementers-guide/src/node/approval/approval-distribution.md index d3180f8e6d9a..8ca1f16e4a22 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-distribution.md +++ b/roadmap/implementers-guide/src/node/approval/approval-distribution.md @@ -89,13 +89,7 @@ Iterate over every `BlockEntry` and remove `PeerId` from it. #### `NetworkBridgeEvent::PeerViewChange` -For each block in the view: - 1. Initialize `fresh_blocks = {}` - 2. Load the `BlockEntry` for the block. If the block is unknown, or the number is less than the view's finalized number, go to step 6. - 3. Inspect the `known_by` set. If the peer is already present, go to step 6. - 4. Add the peer to `known_by` and add the hash of the block to `fresh_blocks`. - 5. Return to step 2 with the ancestor of the block. - 6. For each block in `fresh_blocks`, send all assignments and approvals for all candidates in those blocks to the peer. +Invoke `unify_with_peer(peer, view)` to catch them up to messages we have. We also need to use the `view.finalized_number` to remove the `PeerId` from any blocks that it won't be wanting information about anymore. Note that we have to be on guard for peers doing crazy stuff like jumping their 'finalized_number` forward 10 trillion blocks to try and get us stuck in a loop for ages. @@ -119,8 +113,11 @@ If the message is of type `ApprovalDistributionV1Message::Approval(approval_vote #### `ApprovalDistributionMessage::NewBlocks` -TODO: create `BlockEntry` and `CandidateEntries` for all blocks. -TODO: check for new commonality with peers. Broadcast new information to peers we now understand better. +Create `BlockEntry` and `CandidateEntries` for all blocks. + +For all peers: + * Compute `view_intersection` as the intersection of the peer's view blocks with the hashes of the new blocks. + * Invoke `unify_with_peer(peer, view_intersection)`. #### `ApprovalDistributionMessage::DistributeAsignment` @@ -182,4 +179,15 @@ Imports an approval signature referenced by block hash and candidate index. * Dispatch a `ApprovalDistributionV1Message::Approval(approval)` to all peers in the `BlockEntry`'s `known_by` set, excluding the peer in the `source`, if `source` has kind `MessageSource::Peer`. Add the fingerprint of the assignment to the knowledge of each peer. Note that this obeys the politeness conditions: * We guarantee elsewhere that all peers within `known_by` are aware of all assignments relative to the block. * We've checked that this specific approval has a corresponding assignment within the `BlockEntry`. - * Thus, all peers are aware of the assignment or have a message to them in-flight which will make them so. \ No newline at end of file + * Thus, all peers are aware of the assignment or have a message to them in-flight which will make them so. + + +#### `unify_with_peer(peer: PeerId, view)`: + +For each block in the view: + 1. Initialize `fresh_blocks = {}` + 2. Load the `BlockEntry` for the block. If the block is unknown, or the number is less than the view's finalized number, go to step 6. + 3. Inspect the `known_by` set of the `BlockEntry`. If the peer is already present, go to step 6. + 4. Add the peer to `known_by` with a cloned version of `block_entry.knowledge`. and add the hash of the block to `fresh_blocks`. + 5. Return to step 2 with the ancestor of the block. + 6. For each block in `fresh_blocks`, send all assignments and approvals for all candidates in those blocks to the peer. \ No newline at end of file diff --git a/roadmap/implementers-guide/src/types/network.md b/roadmap/implementers-guide/src/types/network.md index 3770d914005e..f11fae85f9d3 100644 --- a/roadmap/implementers-guide/src/types/network.md +++ b/roadmap/implementers-guide/src/types/network.md @@ -27,13 +27,13 @@ enum ObservedRole { ```rust enum ApprovalDistributionV1Message { - /// An assignment for a candidate in some recent, unfinalized block. + /// Assignments for candidates in recent, unfinalized blocks. /// /// The u32 is the claimed index of the candidate this assignment corresponds to. Actually checking the assignment /// may yield a different result. - Assignment(IndirectAssignmentCert, u32), - /// An approval for a candidate in some recent, unfinalized block. - Approval(IndirectSignedApprovalVote), + Assignments(Vec<(IndirectAssignmentCert, u32)>), + /// Approvals for candidates in some recent, unfinalized block. + Approvals(Vec), } ``` From ceb50aeaea6d9be3947bfa336df7c3f8ad23c27e Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 18 Nov 2020 16:11:24 -0500 Subject: [PATCH 16/17] address review comments --- roadmap/implementers-guide/src/SUMMARY.md | 1 - .../src/node/approval/approval-distribution.md | 12 ++++++++---- .../src/runtime/approvals_inherent.md | 1 - roadmap/implementers-guide/src/types/network.md | 8 -------- .../src/types/overseer-protocol.md | 8 +++++++- 5 files changed, 15 insertions(+), 15 deletions(-) delete mode 100644 roadmap/implementers-guide/src/runtime/approvals_inherent.md diff --git a/roadmap/implementers-guide/src/SUMMARY.md b/roadmap/implementers-guide/src/SUMMARY.md index 03928cfff86e..3c899b6d3888 100644 --- a/roadmap/implementers-guide/src/SUMMARY.md +++ b/roadmap/implementers-guide/src/SUMMARY.md @@ -8,7 +8,6 @@ - [Architecture Overview](architecture.md) - [Messaging Overview](messaging.md) - [Runtime Architecture](runtime/README.md) - - [ApprovalsInherent Module](runtime/approvals_inherent.md) - [Initializer Module](runtime/initializer.md) - [Configuration Module](runtime/configuration.md) - [Disputes Module](runtime/disputes.md) diff --git a/roadmap/implementers-guide/src/node/approval/approval-distribution.md b/roadmap/implementers-guide/src/node/approval/approval-distribution.md index 8ca1f16e4a22..b70981616ace 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-distribution.md +++ b/roadmap/implementers-guide/src/node/approval/approval-distribution.md @@ -32,12 +32,15 @@ type BlockScopedCandidate = (Hash, CandidateHash); /// The `State` struct is responsible for tracking the overall state of the subsystem. /// -/// It tracks metadata about our view of the chain, which assignments and approvals we have seen, and our peers' views. +/// It tracks metadata about our view of the unfinalized chain, which assignments and approvals we have seen, and our peers' views. struct State { + // These three fields are used in conjunction to construct a view over the unfinalized chain. blocks_by_number: BTreeMap>, blocks: HashMap, - peer_views: HashMap, finalized_number: BlockNumber, + + // Peer view data is partially stored here, and partially inline within the `BlockEntry`s + peer_views: HashMap, } enum MessageFingerprint { @@ -69,7 +72,8 @@ enum ApprovalState { } /// Information about candidates in the context of a particular block they are included in. In other words, -/// multiple `CandidateEntry`s may exist for the same candidate, if it is included by multiple blocks - this is likely the case /// when there are forks. +/// multiple `CandidateEntry`s may exist for the same candidate, if it is included by multiple blocks - this is likely the case +/// when there are forks. struct CandidateEntry { approvals: HashMap, } @@ -185,7 +189,7 @@ Imports an approval signature referenced by block hash and candidate index. #### `unify_with_peer(peer: PeerId, view)`: For each block in the view: - 1. Initialize `fresh_blocks = {}` + 1. Initialize a set `fresh_blocks = {}` 2. Load the `BlockEntry` for the block. If the block is unknown, or the number is less than the view's finalized number, go to step 6. 3. Inspect the `known_by` set of the `BlockEntry`. If the peer is already present, go to step 6. 4. Add the peer to `known_by` with a cloned version of `block_entry.knowledge`. and add the hash of the block to `fresh_blocks`. diff --git a/roadmap/implementers-guide/src/runtime/approvals_inherent.md b/roadmap/implementers-guide/src/runtime/approvals_inherent.md deleted file mode 100644 index db9588c2794d..000000000000 --- a/roadmap/implementers-guide/src/runtime/approvals_inherent.md +++ /dev/null @@ -1 +0,0 @@ -# ApprovalsInherent Module diff --git a/roadmap/implementers-guide/src/types/network.md b/roadmap/implementers-guide/src/types/network.md index f11fae85f9d3..7fcbf8783166 100644 --- a/roadmap/implementers-guide/src/types/network.md +++ b/roadmap/implementers-guide/src/types/network.md @@ -77,14 +77,6 @@ enum StatementDistributionV1Message { } ``` -### Approval Distribution V1 - -```rust -enum ApprovalDistributionV1Message { - // TODO [now] -} -``` - ### Collator Protocol V1 ```rust diff --git a/roadmap/implementers-guide/src/types/overseer-protocol.md b/roadmap/implementers-guide/src/types/overseer-protocol.md index ba0e6dc15e21..785b6e1bd9f7 100644 --- a/roadmap/implementers-guide/src/types/overseer-protocol.md +++ b/roadmap/implementers-guide/src/types/overseer-protocol.md @@ -71,7 +71,7 @@ enum ApprovalVotingMessage { /// Should not be sent unless the block hash within the indirect vote is known. CheckAndImportApproval( IndirectSignedApprovalVote, - ResponseChannel, + ResponseChannel, ), /// Returns the highest possible ancestor hash of the provided block hash which is /// acceptable to vote on finality for. @@ -89,10 +89,16 @@ enum ApprovalVotingMessage { Messages received by the approval Distribution subsystem. ```rust +/// Metadata about a block which is now live in the approval protocol. struct BlockApprovalMeta { + /// The hash of the block. hash: Hash, + /// The number of the block. number: BlockNumber, + /// The candidates included by the block. Note that these are not the same as the candidates that appear within the + /// block body. candidates: Vec, + /// The consensus slot number of the block. slot_number: SlotNumber, } From 4fcd2fed2f8891c08f98beb6e02be6548a7d8a6c Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 24 Nov 2020 20:51:25 -0500 Subject: [PATCH 17/17] address review comments and use nifty VRFProof --- .../src/node/approval/approval-distribution.md | 7 +++---- .../src/node/approval/approval-voting.md | 3 ++- roadmap/implementers-guide/src/types/overseer-protocol.md | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/roadmap/implementers-guide/src/node/approval/approval-distribution.md b/roadmap/implementers-guide/src/node/approval/approval-distribution.md index b70981616ace..6ded8b6db9ad 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-distribution.md +++ b/roadmap/implementers-guide/src/node/approval/approval-distribution.md @@ -16,7 +16,7 @@ Approval messages should always follow assignments, so we need to be able to dis 1. Is a particular assignment relevant under a given `View`? 2. Is a particular approval relevant to any assignment in a set? -These two queries need not be perfect, but they must never yield false positives. For our own local view, they must not yield false negatives. When applied to our peers' views, it is acceptable for them to yield false negatives. The reason for that is that our peers' views may be beyond ours, and we are not capable of fully evaluating them. Once we have caught up, we can check again for false negatives to continue distributing. +It is acceptable for these two queries to yield false negatives with respect to our peers' views. For our own local view, they must not yield false negatives. When applied to our peers' views, it is acceptable for them to yield false negatives. The reason for that is that our peers' views may be beyond ours, and we are not capable of fully evaluating them. Once we have caught up, we can check again for false negatives to continue distributing. For assignments, what we need to be checking is whether we are aware of the (block, candidate) pair that the assignment references. For approvals, we need to be aware of an assignment by the same validator which references the candidate being approved. @@ -150,9 +150,8 @@ Imports an assignment cert referenced by block hash and candidate index. As a po * check if `peer` appears under `known_by` and whether the fingerprint is in the `known_messages` of the peer. If the peer does not know the block, report for providing data out-of-view and proceed. If the peer does know the block and the knowledge contains the fingerprint, report for providing replicate data and return. * If the message fingerprint appears under the `BlockEntry`'s `Knowledge`, give the peer a small positive reputation boost and return. Note that we must do this after checking for out-of-view to avoid being spammed. If we did this check earlier, a peer could provide data out-of-view repeatedly and be rewarded for it. * Dispatch `ApprovalVotingMessage::CheckAndImportAssignment(assignment)` and wait for the response. - * If the result is `AssignmentCheckResult::Accepted(candidate_index)` or `AssignmentCheckResult::AcceptedDuplicate(candidate_index)` - * If `candidate_index` does not match `claimed_candidate_index`, punish the peer's reputation, recompute the fingerprint, and re-do our knowledge checks. The goal here is to accept messages which peers send us that are labeled wrongly, but punish them for it as they've made us do extra work. - * Otherwise, if the vote was accepted but not duplicate, give the peer a positive reputation boost + * If the result is `AssignmentCheckResult::Accepted` or `AssignmentCheckResult::AcceptedDuplicate` + * If the vote was accepted but not duplicate, give the peer a positive reputation boost * add the fingerprint to both our and the peer's knowledge in the `BlockEntry`. Note that we only doing this after making sure we have the right fingerprint. * If the result is `AssignmentCheckResult::TooFarInFuture`, mildly punish the peer and return. * If the result is `AssignmentCheckResult::Bad`, punish the peer and return. diff --git a/roadmap/implementers-guide/src/node/approval/approval-voting.md b/roadmap/implementers-guide/src/node/approval/approval-voting.md index 7ff4981d1e4f..481407338da9 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-voting.md +++ b/roadmap/implementers-guide/src/node/approval/approval-voting.md @@ -153,7 +153,7 @@ On receiving an `OverseerSignal::ActiveLeavesUpdate(update)`: * Ensure that the `CandidateEntry` contains a `block_assignments` entry for the block, with the correct backing group set. * If a validator in this session, compute and assign `our_assignment` for the `block_assignments` * Only if not a member of the backing group. - * Run `RelayVRFModulo` and `RelayVRFDelay` according to the [the approvals protocol section](../../protocol-approval.md#assignment-criteria) + * Run `RelayVRFModulo` and `RelayVRFDelay` according to the [the approvals protocol section](../../protocol-approval.md#assignment-criteria). Ensure that the assigned core derived from the output is covered by the auxiliary signature aggregated in the `VRFPRoof`. * invoke `process_wakeup(relay_block, candidate)` for each new candidate in each new block - this will automatically broadcast a 0-tranche assignment, kick off approval work, and schedule the next delay. * Dispatch an `ApprovalDistributionMessage::NewBlocks` with the meta information filled out for each new block. @@ -166,6 +166,7 @@ On receiving a `ApprovalVotingMessage::CheckAndImportAssignment` message, we che * Check the assignment cert * If the cert kind is `RelayVRFModulo`, then the certificate is valid as long as `sample < session_info.relay_vrf_samples` and the VRF is valid for the validator's key with the input `block_entry.relay_vrf_story ++ sample.encode()` as described with [the approvals protocol section](../../protocol-approval.md#assignment-criteria). We set `core_index = vrf.make_bytes().to_u32() % session_info.n_cores`. If the `BlockEntry` causes inclusion of a candidate at `core_index`, then this is a valid assignment for the candidate at `core_index` and has delay tranche 0. Otherwise, it can be ignored. * If the cert kind is `RelayVRFDelay`, then we check if the VRF is valid for the validator's key with the input `block_entry.relay_vrf_story ++ cert.core_index.encode()` as described in [the approvals protocol section](../../protocol-approval.md#assignment-criteria). The cert can be ignored if the block did not cause inclusion of a candidate on that core index. Otherwise, this is a valid assignment for the included candidate. The delay tranche for the assignment is determined by reducing `(vrf.make_bytes().to_u64() % (session_info.n_delay_tranches + session_info.zeroth_delay_tranche_width)).saturating_sub(session_info.zeroth_delay_tranche_width)`. + * We also check that the core index derived by the output is covered by the `VRFProof` by means of an auxiliary signature. * If the delay tranche is too far in the future, return `VoteCheckResult::Ignore`. * `import_checked_assignment` * return the appropriate `VoteCheckResult` on the response channel. diff --git a/roadmap/implementers-guide/src/types/overseer-protocol.md b/roadmap/implementers-guide/src/types/overseer-protocol.md index 785b6e1bd9f7..cfcfdf444a96 100644 --- a/roadmap/implementers-guide/src/types/overseer-protocol.md +++ b/roadmap/implementers-guide/src/types/overseer-protocol.md @@ -42,9 +42,9 @@ Messages received by the approval voting subsystem. ```rust enum AssignmentCheckResult { // The vote was accepted and should be propagated onwards. - Accepted(u32), + Accepted, // The vote was valid but duplicate and should not be propagated onwards. - AcceptedDuplicate(u32), + AcceptedDuplicate, // The vote was valid but too far in the future to accept right now. TooFarInFuture, // The vote was bad and should be ignored, reporting the peer who propagated it.