Skip to content
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

Hash enc digest hex #27

Merged
merged 8 commits into from
Feb 27, 2024
Merged

Hash enc digest hex #27

merged 8 commits into from
Feb 27, 2024

Conversation

tonesnotes
Copy link
Contributor

The significant change is to the hash classes / functions in regard to the enc (encoding) input argument.

This started from an incorrect comment on PublicKey.verify that the default encoding was 'hex' when it is in fact 'utf8'.

Following this into the hash classes it turned out that the same enc input parameter was being used to determine both the encoding of the input message (msg) AND the desired output type (number[] vs hex string).

As this needed resolving in any case, and it looked like very few tests were passing 'hex' encoding. This PR proposes to resolve the issue by:

  1. using the input enc parameter only for msg encoding
  2. add an explicit 'utf8' option where missing on the type of the enc parameter
  3. add an explicit digestHex() method to mirror digest() when a hex string output is desired.

Two tests were updated. All tests pass.
This is a breaking change to fix a broken API situation.

ty-everett
ty-everett previously approved these changes Feb 27, 2024
src/primitives/__tests/ECDSA.test.ts Outdated Show resolved Hide resolved
src/primitives/__tests/ECDSA.test.ts Outdated Show resolved Hide resolved
src/primitives/__tests/ECDSA.test.ts Outdated Show resolved Hide resolved
@ty-everett ty-everett self-requested a review February 27, 2024 17:20
@ty-everett ty-everett merged commit 7bcbb2d into master Feb 27, 2024
8 checks passed
@ty-everett ty-everett deleted the hash-enc-digestHex branch February 27, 2024 17:21
@ty-everett ty-everett mentioned this pull request Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants