-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Non-overflowing parse for signatures #67
Conversation
(Have to fix some issues on #65 before we can merge this!) |
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.
Minor nitpicks. LGTM otherwise.
/// meaning you will have compatibility issues if you also use other | ||
/// SECP256K! libraries. It's not recommended to use this function. Please | ||
/// use `parse_standard` instead. | ||
pub fn parse_overflowing(p: &[u8; util::SIGNATURE_SIZE]) -> Signature { |
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.
Any reason not to use #[deprecated(…)]
here?
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.
deprecated
imply that we can eventually remove this, which we cannot do. Same reason why der-lax
is not marked deprecated.
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.
While that is true I’d probably use it anyway to get those nice compile time warnings. You could set the depreciation version very far out perhaps?
Either way, your call.
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 think we simply can't get rid of it. In Substrate we'll have to create a new function version of this, with the old one always exist (and we cannot remove). In this sense, deprecated
warning is just noise and not really useful.
We changed the function names, so when people upgrade they'll have a compile failure anyway and have to change the function name, which is more than safe enough to make sure they don't use the incorrect function.
Co-authored-by: David <dvdplm@gmail.com>
Co-authored-by: David <dvdplm@gmail.com>
Co-authored-by: David <dvdplm@gmail.com>
Co-authored-by: David <dvdplm@gmail.com>
Are there test vectors available that pass |
* Non-overflowing parse for signatures * Fix tests * Fix styling * Add docs for Signature struct functions * Update src/lib.rs Co-authored-by: David <dvdplm@gmail.com> * Update src/lib.rs Co-authored-by: David <dvdplm@gmail.com> * Update src/lib.rs Co-authored-by: David <dvdplm@gmail.com> * Update src/lib.rs Co-authored-by: David <dvdplm@gmail.com> Co-authored-by: David <dvdplm@gmail.com>
This changes the current method to
parse_overflowing
andparse_overflowing_slice
, and a new non-overflowing signature parse function underparse_standard
andparse_standard_slice
.