This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Initial draft code for approvals assignments#1558
Closed
burdges wants to merge 3 commits intoparitytech:masterfrom
Closed
Initial draft code for approvals assignments#1558burdges wants to merge 3 commits intoparitytech:masterfrom
burdges wants to merge 3 commits intoparitytech:masterfrom
Conversation
Jeff's original commits still exist at https://github.com/w3f/polkadot-1/tree/jeff-approval-assignments-draft
Parity's style. Apple's sed appears oddly buggy.
|
@burdges, Your signature has been received. |
Contributor
Author
|
@rphmeier We could largely divorce this code from the chain like grandpa, and remove most |
Contributor
Author
|
We should split tracker.rs into an independent implementation of the counting algorithm, and a separate implementation of its database. We can then adapt the stories.rs, criteria.rs, and the counting algorithm for actual use, and someone can reimplement all the data storage logic twice, once for on-chain and once for off-chain. |
f00512e to
a170db5
Compare
a170db5 to
73c0480
Compare
Contributor
Author
|
Superseded by #2112 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Initial draft implementation of the approval assignments subsystem described in #1518
In polkadot, we have first backing checks in which some slashable validator commits to a parachain candidate being valid, second availability aka inclusion in which a 2/3rd majority claims the parachain candidate is available, and third approval checks in which random validators check the parachain candidate.
In this PR, we actually assign those individual validators who perform approval checks of available aka included parachain candidates. It's vaguely like some OS task scheduler, well if your CPU cores were adversarial and needed to randomly check one anthers' work.
As explained in #1518 assignments are computed with VRFs using the assignee's secret key, not just some secret system randomness, which gives assignments some meaningful prioritization, called a delay tranche.
We've several priorities here, which include:
There is a lot missing here, but some initial steps:
As a part of Initial guide text for approvals and especially approvals assignments #1518 Rob should decide if approvals and disputes live in the same crate. I think separation looks viable, but disputes must listen to both backing and approval validity votes.
There are numerous
unimplemented!()calls, mostly instories.rsand especially onApprovalContext, but elsewhere too, that import data about relay chain blocks, like their VRFs, included paraids and candidate information, and equivocations. All these should be replaced with appropriate plumbing by someone who actually knows the substrate and polakdot code base (not me).There are a few
unimplemented!()calls for accessing the local key store, which someone should fill in. We've zero need for remote signer support here, as assignment keys would not be slashable, not anytime soon anyways, so this should be straightforward for anyone who knows the keystore interface.There are
unimplemented!()calls for serialization incriteria.rsI think, which anyone here except me could figure out quickly probably. I've actually outright omitted most serialization work here, which we can discuss in the channel. We should avoid exposing too many implementation details from assignments, which becomes easier to achieve once serialization work starts. Also, we've a wider discussion about gossip, politeness, etc. but that's mostly for follow ups to Initial guide text for approvals and especially approvals assignments #1518 not here.I've omitted any tests here because the 1-3 must happen first.
I kinda (ab)used
dynhere for simplicity in this first draft, butenum SomeCriterialooks helpful not just for serialization, but maybe forannoucer.rstoo.I've omitted entirely the announcement postponement decision system, but I designed the current data structures with announcement postponement in mind. It's important that postponement happens before announcement, but I've no opinion about whether this happens via separate routines or parameters to the announcement routine.
I've avoided doing the on-chain approval checker. We could just store all the gossip messages on-chain and process them in one batch in some future relay chain block that claims its ancestor is now approved. We could add separate on-chain types that
impl Criteriabut check and strip the VRFs signatures when inserted into the state, and thus reduce state usage.We rescan some data structures here in ways that seems (a) inefficient if done on-chain, but maybe it'll only ever be done once on chain, not sure yet, and (b) risky against denial-of-services, but our usage of VRFs prevents too much spam. It's still maybe worth optimizing, provided we avoid risks from code complexity.
In the vein of 8, I've opted for a more complex assignee counting and approval determination system, which keeps our assignment types down to just three, and keeps the stories being only about the relay chain VRF and equivocations. We could however simplify this system by adding a third
NoShowstory type about another announcement, and adding fourth criteria that assigns and announces this. I suppose this somewhat simplifies the data structures in 7 too, but the total complexity sounds higher.We could merge our announcements in one tranche, which improves VRF signature verification time and slightly improves our gossip, but complicates the code somewhat. I abandoned this optimization because 40ms * num_validators sounds small-ish compared with the cost of even one validator checking a whole block, but..
I still need to go through this and retune all the miriad parameters, and chat with Alistair, Handan, etc. about them. Yet, they also require explanation, so feel free to discuss them here too.
"We go to 11" -- This Is Spinal Tap ;)
In short, can folks try to fill in the
unimplemented!()calls in this?