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

Reduce network bandwidth, improve parablock times: optimize approval-distribution #5164

Merged
merged 80 commits into from
Apr 19, 2022

Conversation

rphmeier
Copy link
Contributor

@rphmeier rphmeier commented Mar 21, 2022

Closes #5162

Pre-reqs: Gossip-Support changes

This comes with a number of changes to gossip-support to make this possible.

The grid topology is identified by the session, and peers' ValidatorIndex values are given with their AuthorityDiscoveryId and PeerId.

Connection requests are now made against past/present/future AuthorityDiscoveryIds, but the grid is only based on validator IDs. Previously, the grid was based on past/present/future AuthorityDiscoveryIds, but this behavior was undesirable for the obvious reason that if validators rotate their keys often, the grid might be 5/6 populated by dead or duplicate entries. These issues in the topology could lead to failures to propagate or excessive propagation, respectively.

The reason we connect to past/present/future AuthorityDiscoveryIds even though we discount them from the session's grid topology is that we still need to communicate with those old/future authorities and being generally connected guarantees good req/res behavior. And gossip subsystems like approval-distribution will want to gossip with different peers based on the session.

For Statement Distribution and Bitfield Distribution, adjusting the grid topology doesn't matter much because we only care about the most recent session, and failures here only impact parachain liveness. For approval distribution, this runs some risk that assignments/approvals don't propagate between validator sets of different sessions. However, assignments and approvals only need to reach the validators of the session of the block, as those are the GRANDPA voters assigned to finalize those blocks. Actually, because sessions are delayed by one block in the parachains protocol, the last block of any session S is actually finalized by the validators of S+1, but since validator sets shouldn't change much and we incorporate random gossip, this isn't likely to cause issues. But we have to be more careful in this subsystem because it's important for parachain safety and relay chain liveness.

Actual Subsystem Changes

This PR alters the approval-distribution subsystem to use the deterministic grid topology of validators more effectively, while implementing fallbacks in the case that the grid topology is compromised.

Every session, validators are organized into a (close to square) grid, where each validator has row-neighbors and column-neighbors.

The basic operation of the 2D grid topology is that:

  • A validator producing a message sends it to its row-neighbors and its column-neighbors
  • A validator receiving a message originating from one of its row-neighbors sends it to its column-neighbors
  • A validator receiving a message originating from one of its column-neighbors sends it to its row-neighbors

This grid approach defines 2 unique paths for every validator to reach every other validator in at most 2 hops.

However, we also supplement this with some degree of random propagation: every validator, upon seeing a message for the first time, propagates it to 8 random peers. This includes the originator of the message. This inserts some redundancy in case the grid topology isn't working or is being attacked - an adversary doesn't know which peers a validator will send to. This is combined with the property that the adversary doesn't know which validators will elect to check a block.

But, in case these mechanisms don't work on their own, we need to trade bandwidth for protocol liveness by introducing aggression.

Aggression has 3 levels:

  • Aggression Level 0: The basic behaviors described above.
  • Aggression Level 1: The originator of a message sends to all peers. Other peers follow the rules above.
  • Aggression Level 2: All peers send all messages to all their row and column neighbors. This means that each validator will, on average, receive each message approximately 2*sqrt(n) times.

The aggression level of messages pertaining to a block increases when that block is unfinalized and is a child of the finalized block. This means that only one block at a time has its messages propagated with aggression > 0.

Also, we re-send messages every few blocks to all peers required by our aggression level.

Lastly, there is also redundancy in the form of the 'no-shows' feature of the core approval-voting code. If, for whatever reason, some approval messages aren't propagating through the grid or random gossip, more and more validators will elect to check the para-block, covering prior no-shows. If some assignment messages aren't getting through, it'll also cause more validators to self-select.

@rphmeier rphmeier added the A3-in_progress Pull request is in progress. No review needed at this stage. label Mar 21, 2022
@paritytech-ci paritytech-ci requested review from a team April 10, 2022 20:10
Copy link
Contributor

@drahnr drahnr left a comment

Choose a reason for hiding this comment

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

We should clarify which random source to use. Other than that, LGTM

@rphmeier
Copy link
Contributor Author

rphmeier commented Apr 11, 2022

We should clarify which random source to use.

@drahnr What do you mean by this?

I added a CryptoRng which is used throughout the subsystem but it's not exposed as part of the public API of the subsystem (StdRng is used in fn run). This gives the tests some more determinism.

@drahnr
Copy link
Contributor

drahnr commented Apr 12, 2022

Sorry for the brevity, I meant this comment in particular: https://github.com/paritytech/polkadot/pull/5164/files#r835130703 - where do we take the entropy from to initialize the cprng? After all this defines the gossip topology and should be consistent across nodes (or a sufficiently large subset) to work as anticipated iiuc. So updating later has to be done with more diligence from what I understand. (I did not do the math for this yet)

@burdges
Copy link
Contributor

burdges commented Apr 12, 2022

Availability demands all validators have network connections with all other validators anyways. We thus do not care if different messages obey have different typologies. We do not want a bad validator to be able to spam by sending the same message with multiple topology, so whatever randomness we use needs to be verified form the chain state. It depends upon the validator set anyways, so anything we do has resolution no coarser than babe two epochs ago, but it's just fine to use say babe two epochs ago hashed with the senders' public key.

@rphmeier
Copy link
Contributor Author

@burdges the comment was about whether we should draw the (same) randomness value from the BABE runtime API or through the SessionInfo interface exposed from the parachains runtime API. In both cases they're BABE from 2 epochs ago, but there is a potential OBO edge case when switching from one to the other. To be addressed in a follow-up PR.

Copy link
Member

@ordian ordian left a comment

Choose a reason for hiding this comment

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

Looks good overall.

It'd be good to have high level overview and goals of aggression levels mentioned in the guide as well.

node/network/approval-distribution/src/metrics.rs Outdated Show resolved Hide resolved
node/network/gossip-support/src/lib.rs Show resolved Hide resolved
(false, false) => RequiredRouting::None,
(true, false) => RequiredRouting::GridY, // messages from X go to Y
(false, true) => RequiredRouting::GridX, // messages from Y go to X
(true, true) => RequiredRouting::GridXY, // if the grid works as expected, this shouldn't happen.
Copy link
Member

Choose a reason for hiding this comment

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

I have a slight preference for use always_assert::never as we do in PVF subsystem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Panics are for unrecoverable errors. This might be unexplainable, but it's obviously recoverable. Panics in our use-case are playing with fire. We've written bugs before and will do it again.

Copy link
Member

Choose a reason for hiding this comment

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

This thing is like a debug_assert in a nice wrapping. So this will panic only in CI/testnet, which is better than not noticing it at all.

Copy link
Contributor Author

@rphmeier rphmeier Apr 19, 2022

Choose a reason for hiding this comment

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

I see. Well, we could do a follow-up for that. I'm not really opinionated about debug-asserts.

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Only approving the runtime lib.rs changes.

@eskimor eskimor merged commit 8bb84d2 into master Apr 19, 2022
@eskimor eskimor deleted the rh-approval-grid branch April 19, 2022 18:26
@jakoblell jakoblell added D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited. and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited. labels Jul 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. C3-medium PR touches the given topic and has a medium impact on builders. D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimize Approval-Distribution to more effectively use the grid topology
8 participants