fix: remove dependency on tweetnacl #73
Conversation
|
alpha is now available at https://www.npmjs.com/package/discord-interactions/v/4.0.0-alpha.0 |
| ): Uint8Array { | ||
| if (value == null) { | ||
| return new Uint8Array(); | ||
| } |
There was a problem hiding this comment.
This is illegal in the types; are we doing this to protect against JS users?
There was a problem hiding this comment.
I would assume as such, yes. This function was copypasta'd from index.ts, in it's exact same form. I didn't change of it.
| const matches = value.match(/.{1,2}/g); | ||
| if (matches == null) { | ||
| throw new Error('Value is not a valid hex string'); | ||
| } | ||
| const hexVal = matches.map((byte: string) => Number.parseInt(byte, 16)); |
There was a problem hiding this comment.
This is kind of insane. Is this really the best approach 😆
There was a problem hiding this comment.
dude ... if you can find a better approach that works I'm all ears. This is a copy/paste from the original code, just shifted over. I tried doing some other clever things with btoa and TextEncoder, but the whole "hex string" thing threw me for a loop.
There was a problem hiding this comment.
I actually ran into this recently on a separate project and ended up resorting to buffers since the native web APIs didn't support this very well. I suppose we're stuck with this for now.
| return new Uint8Array(value); | ||
| } | ||
| } catch (ex) { | ||
| // Runtime doesn't have Buffer |
There was a problem hiding this comment.
Should we do a typeof Buffer !== 'undefined' check here in case we're swallowing exceptions?
There was a problem hiding this comment.
I can't imagine what other kind of exception could happen in this very narrow use case
This change drops the dependency on tweetnacl for key verification, instead relying on the node.js built-in Ed25519 implementation. This seems to work locally with node 18+, but needs to be tested in other environments.
I'm not sure this is a good idea. The ed225519 support in node 22 is still experimental, and this will drop a console warning on startup:
https://nodejs.org/api/webcrypto.html#ed25519ed448x25519x448-key-pairs
verifyKeyasyncFixes #30 and generally resolves #54