Sign V2#3284
Open
Antonio95 wants to merge 9 commits into
Open
Conversation
…n_v2 in some instances
vicsn
reviewed
Jun 18, 2026
| let message: Vec<_> = (0..num_fields).map(|_| Uniform::rand(rng)).collect(); | ||
| let signature = console::Signature::sign(&private_key, &message, rng).unwrap(); | ||
| assert!(signature.verify(&address, &message)); | ||
| let signature = console::Signature::sign_v2(&private_key, &message, rng).unwrap(); |
Collaborator
There was a problem hiding this comment.
Is this arithmetization equivalent / backwards compatible?
Comment on lines
+102
to
+109
| // TODO (Antonio) re-introduce | ||
| // Ok(fields) => self.verify_v2(address, &fields), | ||
| Ok(fields) => { | ||
| for f in fields.iter() { | ||
| println!(" f: {f}"); | ||
| } | ||
| self.verify_v2(address, &fields) | ||
| } |
Collaborator
There was a problem hiding this comment.
Leftover debug code?
| } | ||
|
|
||
| #[test] | ||
| fn test_sign_and_verify_bits_v2_padding() -> Result<()> { |
Collaborator
There was a problem hiding this comment.
No test would catch a change to SIGNATURE_V2_PREFIX or the length encoding so maybe worth checking against hardcoded values?
Comment on lines
+241
to
+246
| let candidate_string = serde_json::to_string(&expected_signature).unwrap(); | ||
| assert_eq!(expected_string, serde_json::Value::from_str(&candidate_string).unwrap().as_str().unwrap()); | ||
|
|
||
| // Deserialize | ||
| assert_eq!(expected_signature_v2, serde_json::from_str(&candidate_string).unwrap()); | ||
| assert_eq!(expected_signature, Signature::<CurrentNetwork>::from_str(expected_string).unwrap()); |
Collaborator
There was a problem hiding this comment.
Suggested change
| let candidate_string = serde_json::to_string(&expected_signature).unwrap(); | |
| assert_eq!(expected_string, serde_json::Value::from_str(&candidate_string).unwrap().as_str().unwrap()); | |
| // Deserialize | |
| assert_eq!(expected_signature_v2, serde_json::from_str(&candidate_string).unwrap()); | |
| assert_eq!(expected_signature, Signature::<CurrentNetwork>::from_str(expected_string).unwrap()); | |
| let candidate_string = serde_json::to_string(&expected_signature_v2).unwrap(); | |
| assert_eq!(expected_string, serde_json::Value::from_str(&candidate_string).unwrap().as_str().unwrap()); | |
| // Deserialize | |
| assert_eq!(expected_signature_v2, serde_json::from_str(&candidate_string).unwrap()); | |
| assert_eq!(expected_signature_v2, Signature::<CurrentNetwork>::from_str(expected_string).unwrap()); |
Comment on lines
164
to
+165
| } | ||
|
|
Collaborator
There was a problem hiding this comment.
Suggested change
| } | |
| } |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR
Signature::sign_v2function which adds a prefix to the message being signed.Signature::sign_bits_v2andSignature::sign_bytes_v2, which operate similarly to their original (non-v2) counterparts but 1) add the length of the message to the message itself and 2) sign usingSignature::sign_v2(i.e. with a prefix) under the hood.Signature::sign_bits_raw_v2andSignature::sign_bytes_raw_v2, analogous to the above but not including message length (i.e. incorporating 2) but not 1)).Signature::sign.Note that all
Signature::sign*/verify*functions have alsoPrivateKey::sign*/verify*wrappers.Request::sign/verifyare unaffected.The last clean build is the next-to-last commit at the time of writing: c534e88.
This PR is in draft mode until we decide:
Signature::sign.Some options to handle the resulting complications include:
signas deprecated, and simply documenting/making developers awaresign_v2is the preferred option from now on.sign_v2. Note signatures are used in some delicate (e.g. consensus-related) paths.#[allow(deprecated)](deciding on a case-by-case basis whether to switch tosign_v2instead)sign_v1) and updating ourCargo.tomls to import the crate with that flag. This makes it less likely external developers would use the deprecated function by mistake (they would have to activate the feature flag in the dependency). As a slightly negative aspect of this approach, this means future developers of our own repos would have to be aware thatsign_v2is the preferred option from now onwards (since the presence of the feature flag in the import means the deprecation notice would not be displayed).The last commit at the time of writing, 96e9269, adds the deprecation notice to
Signature::signand, for most of its current uses, it switches to sign_v2 or annotates them with#[allow(deprecated)]. Note this is only for illustrative purposes.