Skip to content
This repository has been archived by the owner on Jun 17, 2021. It is now read-only.

Make hash methods type strict #247

Merged
merged 5 commits into from
Apr 29, 2020
Merged

Make hash methods type strict #247

merged 5 commits into from
Apr 29, 2020

Conversation

cgewecke
Copy link
Contributor

@cgewecke cgewecke commented Apr 28, 2020

Part of #172 Breaking API changes

  • Makes keccak, sha256 and ripemd160 buffer-only and adds fromString and fromArray variants for those inputs.

  • Adds assertIsBuffer check to input of hashPersonalMessage (excluding the possibility that someone could submit an array).

Notes:

There are more methods in signature.ts which could have their params validated but overall that file seems to have clearly specified input types.

In adding convenience methods, I omitted number because idk if that's really used. However, this has been a valid input to the hash methods.

@github-actions
Copy link

Coverage Status

Coverage increased (+0.03%) to 99.704% when pulling 0b7b413 on enforce-buffer-hash-sig into 3141eda on master.

@github-actions
Copy link

Coverage Status

Coverage increased (+0.03%) to 99.704% when pulling 0b7b413 on enforce-buffer-hash-sig into 3141eda on master.

@github-actions
Copy link

Coverage Status

Coverage increased (+0.03%) to 99.704% when pulling 0b7b413 on enforce-buffer-hash-sig into 3141eda on master.

@lgtm-com
Copy link

lgtm-com bot commented Apr 28, 2020

This pull request introduces 1 alert when merging 0b7b413 into 3141eda - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@cgewecke cgewecke requested a review from holgerd77 April 28, 2020 05:59
holgerd77
holgerd77 previously approved these changes Apr 29, 2020
Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One case to discuss/adopt, otherwise looks good, thanks.

src/hash.ts Outdated Show resolved Hide resolved
src/hash.ts Show resolved Hide resolved
test/hash.spec.ts Show resolved Hide resolved
@holgerd77 holgerd77 dismissed their stale review April 29, 2020 14:55

Mis-clicked. 😄

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One case to discuss/adopt, otherwise looks good, thanks!

@lgtm-com
Copy link

lgtm-com bot commented Apr 29, 2020

This pull request introduces 1 alert when merging c7d016d into 3141eda - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Apr 29, 2020

This pull request introduces 1 alert when merging dbda438 into 3141eda - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me know, will merge.

@holgerd77 holgerd77 merged commit f86dc07 into master Apr 29, 2020
@holgerd77 holgerd77 deleted the enforce-buffer-hash-sig branch April 29, 2020 20:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants