This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
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 guide text for approvals and especially approvals assignments #1518
Initial guide text for approvals and especially approvals assignments #1518
Changes from 2 commits
c86c667
2b93eeb
f52f440
d798afd
4b7c36b
9bac5af
99dbb35
a613466
19888c9
2e5a84c
5848faf
e49c707
f656d83
db2a8ee
7b4527c
94f65ab
1d4b69b
dd1d459
82babb6
3bec017
1e133b4
d3413e3
bf0b6fc
2b2e4f9
ae67543
6bd4758
ecfce2b
be4ab74
328556e
296025d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This references an assignments subsystem which hasn't been defined.
What are "outgoing assignment notices"? Are these notifications from some other piece of code that we need to be checking some particular candidate?
Great that this references reconstruction & candidate validity : ) - that's exactly how this will be implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, so this implies yet another session key?
not sure if we have yet migrated a Substrate chain to add more session keys, but it should be doable. However, there is a bootstrapping concern here. When migrating the chain nobody will have yet registered the extra key, but we can't just throw out the validator set.
So I think the process would be that we'd have to add the session key, make an announcement that everyone should rotate session keys, and then enable parachains.
Or is there some way that both of these keys can be the same? It would make the practicalities of upgrading the relay-chain much simpler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could make the approval vote key be the grandpa key. We could always separate them if some distant future super-sentry node iteration would support consensus running on a separate machine from candidate worker nodes or whatever.
We need either the assignments key to be some new key immune to large slashes, or else some validators would ask that assignments by run in a remote signer, which sounds absolutely nightmarish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, that makes a lot of sense. Expounding on the "risk" these keys have as the reason for the separation would make sense in this section. I'm fine with reusing the GRANDPA key for that, although we will be upgrading that to BLS at some point. Maybe makes most sense to just upgrade and add the extra key type, put out an announcement that every validator should rotate their session keys, and then enable parachains a few weeks later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We actually cannot wholly replace Schnorr with BLS because BLS verification seems just too slow. We'll likely add a BLS12-381 G1 point with a Schnorr proof-of-possession as either the only or as a second GRANDPA public key, but we've then two choices:
We could sign GRANDPA messages first with this BLS public key and then sign that signed message with the Ed25519 public key. We could even replace Ed25519 with Rabin-Williams here, which gets shockingly fast. We'll need slashing condition for when something messy happens.
We sign GRANDPA messages with a Schnorr VRF using this BLS public key, so verification runs much slower than Ed25519, but still vastly faster than BLS signatures. At this point the VRF pre-output actually is a BLS signature however, so we can transition smoothly to BLS verification whenever we gain enough signatures for aggregation to help, but doing this avoids any slashing conditions.
I'd wager 1 sounds easiest, so maybe Ed25519 would stick around for quite a while.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just say to use ed25519 for approval vote keys now, which along with previous tweaks seemingly finishes this one. Anything else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/paritytech/polkadot/issues/1269#issuecomment-667650052
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, that's cool. cc @infinity0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When do they reveal their assignments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm afraid this first pass focused primarily upon the assignment module's interface, so I forgot that part presumably, but..
We should clarify that assignment noticed go out only when their delay tranche gets reached. It's more complex actually since they could be delayed further, either by waiting for no shows, or else by legitimately waiting to reduce their workload. I've spent today coding that portion, which became obnoxiously subtle. I'll add some text once I've cleaned up this new code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/w3f/research-internal/issues/519
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3bec017
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another point on delay-tranches. It seems that there is no consensus on which delay-tranches should be used.
For reconstruction and gossip, this seems important. If I receive a reconstruction request, I want it to be legitimized by an assignment proof.
And as I gossip assignments, I will only want to gossip assignments from tranches that I believe should be active. However, how are my peers supposed to know what I accept and what I drop?
The common thread here is to make sure that there is no way for a single validator to create an unbounded amount of assignment proofs that other nodes are forced to circulate or respond to for some reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's one tranche every k seconds after the relay chain block's slot. I've two numbers in the code: delay tranches start from zero with the relay chain block's slot, while AnV slots are
12 * relay_chain_slot + delay_tranche
give an absolute close. I'll let someone else figure out which should be more or less exposed in the interface, etc.Yes and no, we could let validators reconstruct anything, but prioritize approval assignments.
You need not drop anything:
We still need politeness for relay chain block knowledge of course.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does replacement mean in this context? This happens off-chain, right. What is the significance of any single validator adding an extra tranche of validators? Does that make its way on-chain somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd move any more details discussion about the asymptotics to https://github.com/w3f/research-internal/issues/515
We need the same logic on-chain as off-chain to know if a candidate is approved on-chain, but..
I'd expect no shows would usually resolve themselves fast, so the on-chain logic could abandon the replacements too. We'll discuss incentives for replacements later, but incentives impact this..