cryptocoinjs / keccak

Keccak sponge function family
MIT License
87 stars 24 forks source link

createKeccakHash throws an error when run in bun #27

Closed mshakeg closed 1 year ago

mshakeg commented 1 year ago

I initially tried to run a script in a hardhat project using bun, however, that failed as explained in NomicFoundation/hardhat/issues/4383

@schaable pointed out that the issue was with this dependency package as createKeccakHash("keccak256") throws the following error:

 8 |     this._capacity = capacity
 9 |     this._delimitedSuffix = delimitedSuffix
10 |     this._hashBitLength = hashBitLength
11 |     this._options = options
12 | 
13 |     this._state = new KeccakState()
                      ^
TypeError: Object is not a constructor (evaluating 'new KeccakState')
      at new Keccak (/Users/user/hardhat-project/node_modules/keccak/lib/api/keccak.js:13:18)
      at /Users/user/hardhat-project/node_modules/keccak/lib/api/index.js:12:31
      at /Users/user/hardhat-project/scripts/accounts.ts:8:12
      at main (/Users/user/hardhat-project/scripts/accounts.ts:4:19)
      at /Users/user/hardhat-project/scripts/accounts.ts:27:0
      at processTicksAndRejections (:1:2602)
junderw commented 1 year ago
  1. When you installed keccak, did the native compile succeed or fail? (I wonder if you are using the native bindings or the raw JS code)
  2. If you're using JS (native bindings failed to install) then could you try re-writing the lib/keccak.js file to use class syntax instead of the old function-class syntax? If that works, then the problem is caused by bun's inability to support old JS syntax.

This sounds like an issue with bun

Keep in mind that this library only supports NodeJS and the browser. bun is responsible for making sure they can run valid JavaScript and valid NodeJS native modules.

mshakeg commented 1 year ago

@junderw thanks for the quick reply. To answer your questions:

  1. I didn't explicitly install keccak, it was installed as a dependency of hardhat
  2. I'm using TS in my hardhat project
junderw commented 1 year ago
  1. Which means you installed it. I am not asking about whether it's a direct dependency or not, I am asking which version is installed on your computer when you run the install command.
  2. When I say "You are using JS" I mean "You are using the JS version of keccak" because keccak installs a native (C++) version or a non-native (JS) version depending on whether you have the proper C++ toolchain installed or not. If not, install logs will show an error and say "falling back to JS implementation." So I am asking which keccak you are using.
mshakeg commented 1 year ago

@junderw

  1. keccak@3.0.3 was installed
  2. I don't see anything like "falling back to JS implementation" in my install logs, so I would assume that the native version was installed?

node_modules/.pnpm/keccak@3.0.3/node_modules/keccak: Running install script, done in 6.3s

junderw commented 1 year ago

It looks like require('node-gyp-build')(__dirname) is broken and returns an empty object {} whereas in NodeJS it returns a function.

https://github.com/cryptocoinjs/keccak/blob/cacd3a529d8477149f2b75284a7d77302e58aa00/bindings.js#L1

Perhaps bun doesn't support N-API native add-ons?

Let me check...

Answer: It does not support N-API, yet.

https://github.com/oven-sh/bun/issues/158

One solution is to import the JS version directly. (this is difficult because it's not your direct dependency)

import createKeccakHash from "keccak/js.js";

Another fix would be on our side. We can fall back to JS when native bindings don't load a function.

junderw commented 1 year ago

I've opened up #28 but unfortunately I don't have publish rights to NPM. I only have merge rights to GitHub.

So we'll have to wait for someone with NPM publish rights to merge and publish.

junderw commented 1 year ago

We published a workaround in 3.0.4

bun shouldn't crash anymore for keccak.

In hardhat specifically, they use ^3.0.2 which will pull in 3.0.4 automatically.