impl guide: Describe the PVF check subsystem #4685
Conversation
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
pepyakin
added
A0-please_review
|
|
||
| The subsystem tracks all the statement that it submitted within a session. If for some reason a PVF became irrelevant and then becomes relevant again, the subsystem will not submit a new statement for that PVF within the same session. | ||
|
|
||
| If the node is not in the active validator set, it will still perform all the checks. However, it will only submit the check statements when the node is in the active validator set. |
There was a problem hiding this comment.
I do not think this is something that would be attackable and the validators in the active set should be ready for this so we cannot really use the freed up performance as an advantage on the validators not in the active set (and they won't do any work either). On the other hand, if we do this, then pre-check will actually prepare the PVF and save it in the cache and as soon as the validator gets into the active set, even though it did not participate in the pre-checking vote, it will be ready to execute candidates from the get go.
In other words, we are being optimistic here, because I assume/hope 99.9% of pre-checks will end up with accepting the PVF. At the same time, if the PVF does not get accepted, then it is just a little bit of wasted effort for an inactive validator that does not do much anyway.
There was a problem hiding this comment.
That's, by the way, is a poor man's heads up signal https://github.com/paritytech/polkadot/issues/2867
There was a problem hiding this comment.
Got it - I mixed that up, with whether the node is a validator at all. You are talking about the set that is restricted by max_validators.... In that case I think the code is wrong though. You are using the validators runtime call, which ends up reading ActiveValidatorKeys, which in turn gets set to the active set, instead of all potential validators.
There was a problem hiding this comment.
When I say active set / activate validator set (both in my message and in the code) I refer to the whole set of validators returned by validator
When I say when the node is not in active set, I mean the set of all nodes that do run the overseer but are not collator and which do not have keys for the validator ids in the validators list. That definition, hopefully, should only include validators but not full nodes (they do not run overseer) and collators (they have IsCollator::Yes).
There was a problem hiding this comment.
💡 uh right - you do the validation regardless whether or not you find your key in validators, but are you sure full nodes don't run the overseer?
I believe checking for role.is_authority() in this line would make sense - I don't see any code that disables the overseer if is_authority() is false, only chain selection is different. 🤔
There was a problem hiding this comment.
I moved this discussion into a separate issue.
closes #4611
A follow-up for the recently merged PVF pre-checking subsystem (aka pvf-checker) #4610