This repository was archived by the owner on Nov 15, 2023. It is now read-only.
pvf-precheck: Add sign in subsystem-util#4407
Merged
Conversation
Contributor
Author
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite and will continue to be automatically updated while this PR remains open. |
pepyakin
added
B0-silent
Right now, most of operations that sign stuff in polkadot protocol are handled by a very convenient tool - `Signed`. However `Signed` assumes that whatever is signed is anchored to some `parent_hash` which works for most cases, but does not work for others. One instance of such a case is pre-checking (#3211). There validators submit signed votes on-chain. A vote is valid for the entire session. If we were to use `Signed` we would have to root a vote in some block of that session and during vote verification check that this block is indeed within the session. This is especially annoying since we agreed to use unsigned extrinsics to submit votes and we need to make the unsigned extrinsic validation as slim as possible. (FWIW, the definition of a pre-checking vote can be seen in the next diff in the stack) That's the reason why we opted-out from using `Signed` for pre-checking and decided to go with the manual signing approach. Almost every piece of machinery is in place except for signing which is presented in this PR.
pepyakin
force-pushed
the
pep-sign-in-utils
branch
from
12249c4 to
f452557
Compare
This was referenced Nov 29, 2021
bkchr
approved these changes
Nov 29, 2021
pepyakin
requested review from
Lldenaurois and
drahnr
and removed request for
Lldenaurois
drahnr
reviewed
Nov 30, 2021
|
|
||
| match signature { | ||
| Some(sig) => | ||
| Ok(Some(sig.try_into().map_err(|_| KeystoreError::KeyNotSupported(ValidatorId::ID))?)), |
Contributor
There was a problem hiding this comment.
Dropping the conversion error is interesting, we are dropping information here which might be valuable to diagnose what's the error.
Contributor
Author
There was a problem hiding this comment.
I should've mentioned that I just repeat the picture I found there.
polkadot/primitives/src/v1/signed.rs
Line 238 in 8952974
Lmk, if this is something you want to change
Member
There was a problem hiding this comment.
The error is () anyway, so not really valuable.
drahnr
approved these changes
Nov 30, 2021
chevdor
pushed a commit
that referenced
this pull request
Dec 1, 2021
Right now, most of operations that sign stuff in polkadot protocol are handled by a very convenient tool - `Signed`. However `Signed` assumes that whatever is signed is anchored to some `parent_hash` which works for most cases, but does not work for others. One instance of such a case is pre-checking (#3211). There validators submit signed votes on-chain. A vote is valid for the entire session. If we were to use `Signed` we would have to root a vote in some block of that session and during vote verification check that this block is indeed within the session. This is especially annoying since we agreed to use unsigned extrinsics to submit votes and we need to make the unsigned extrinsic validation as slim as possible. (FWIW, the definition of a pre-checking vote can be seen in the next diff in the stack) That's the reason why we opted-out from using `Signed` for pre-checking and decided to go with the manual signing approach. Almost every piece of machinery is in place except for signing which is presented in this PR.
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.
3 participants
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.
Right now, most of operations that sign stuff in polkadot protocol are
handled by a very convenient tool -
Signed. HoweverSignedassumesthat whatever is signed is anchored to some
parent_hashwhich worksfor most cases, but does not work for others.
One instance of such a case is pre-checking (#3211). There validators
submit signed votes on-chain. A vote is valid for the entire session. If
we were to use
Signedwe would have to root a vote in some block ofthat session and during vote verification check that this block is
indeed within the session. This is especially annoying since we agreed
to use unsigned extrinsics to submit votes and we need to make the
unsigned extrinsic validation as slim as possible.
(FWIW, the definition of a pre-checking vote can be seen in the next
diff in the stack)
That's the reason why we opted-out from using
Signedfor pre-checkingand decided to go with the manual signing approach. Almost every piece
of machinery is in place except for signing which is presented in this
PR.