-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[Core] rename hasher #3651
[Core] rename hasher #3651
Conversation
|
||
namespace Neo.Cryptography | ||
{ | ||
public enum HashAlgorithmType : byte |
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.
HashAlgorithm
is better?
|
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.
Merge master into this?
|
||
namespace Neo.Cryptography | ||
{ | ||
public enum HashAlgorithm : byte |
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.
Maybe add an implicit operator
here(it converts Hasher
to HashAlgorithm
)?
/// <returns>The ECDSA signature for the specified message.</returns> | ||
public static byte[] Sign(byte[] message, byte[] priKey, ECC.ECCurve ecCurve = null, Hasher hasher = Hasher.SHA256) | ||
public static byte[] Sign(byte[] message, byte[] priKey, ECC.ECCurve ecCurve = null, HashAlgorithm hashAlgorithm = HashAlgorithm.SHA256) |
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.
Only change the argument name for obsolete?
namespace Neo.Cryptography | ||
{ | ||
/// <summary> | ||
/// Represents hash function identifiers supported by ECDSA message signature and verification. | ||
/// </summary> | ||
[Obsolete("Use HashAlgorithm instead")] |
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 prefer rename the enum instead of duplicate all the methods
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.
You have to, those methods are public, previously our principle is duplicate a new one and obsolete old one before remove it, at least previouly we deal with public methods that way.
Description
As is pointed out by @Hecate2 and @cschuchardt88 #3633 (comment), the name Hasher is ambiguous and improper, thus having this pr to rename it to
HashAlgorithmType
.Fixes # #3633 (comment)
Type of change
Checklist: