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

dependency: replace js-sha3 to noble hashes #35

Merged
merged 1 commit into from
Apr 7, 2024

Conversation

joshstevens19
Copy link
Owner

@joshstevens19 joshstevens19 commented Apr 7, 2024

replace js-sha3 to noble hashes as that is a more mature and actively funded repo

tests all pass

requested on issue > #34


/**
* Keccak256 hash
* @param data The data
*/
export function keccak256(data: string | ArrayLike<number>): string {
return '0x' + sha3.keccak_256(toByteArray(data));
return bytesToHex(keccak_256(toByteArray(data)));
Copy link

@paulmillr paulmillr Apr 7, 2024

Choose a reason for hiding this comment

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

'0x' + bytesToHex(

Copy link
Owner Author

Choose a reason for hiding this comment

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

Bytes to hex already does that

Copy link
Owner Author

Choose a reason for hiding this comment

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

return `0x${hex.join('').replace(/^0+/, '')}`;

Choose a reason for hiding this comment

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

ok

@paulmillr
Copy link

fix 0x and looks good!

@joshstevens19
Copy link
Owner Author

fix 0x and looks good!

Cool replied to that comment it does do it lower down, if looks good will merge and deploy tomorrow

@joshstevens19 joshstevens19 merged commit 7325a1f into master Apr 7, 2024
1 check passed
@joshstevens19 joshstevens19 deleted the dependency/js-sha3-replace-with-noble-hashes branch April 7, 2024 20:27
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