chore(keys): drop unused poseidon injection on initializeEddsa - #19
Merged
Merged
Conversation
mattgle
added a commit
to railgun-reloaded/cryptography
that referenced
this pull request
May 7, 2026
- sha256 was being used by wallet-sdk/services/wallet/wallet-id.ts to generate deterministic wallet IDs. Removing it would have changed every consumer's wallet IDs after upgrade — a critical regression. Restore hash.ts and re-add the export to primitives/index. Verified by running wallet-sdk's wallet-id vector tests against local cryptography (7/7 match the canonical IDs). - Drop the _injectPoseidon parameter from initializeEddsa now that the wallet-node call site has been updated in railgun-reloaded/wallet-node#19. The transition shim is no longer needed. - Add a sha256 test file (NIST FIPS-180-4 test vectors).
mattgle
marked this pull request as ready for review
May 11, 2026 12:31
bhflm
previously approved these changes
May 12, 2026
bhflm
previously approved these changes
May 12, 2026
The argument to initializeEddsa(poseidonBuild.pure) was already a no-op: the cryptography package shipped a circomlibjs patch that would have made it effective, but patch-package was never wired up so circomlibjs's upstream buildEddsa was always the unpatched version that ignores its argument. The injection has been removed from cryptography (see railgun-reloaded/cryptography#8); drop the call-site argument and the now-unused poseidonBuild import. No behavior change: eddsa already builds its own internal poseidon regardless of what was passed in.
…t option
The lockfile was pinned to bytes 5f5711c2 (Apr 29), which predates the
{ strict: true } option added in 5b3f30a (May 4). src/notes/token-utils.ts
uses padBytesLeft(..., N, { strict: true }) in 8 places, so CI rejected
every call with 'Expected 2 arguments, but got 3'.
Refresh the lockfile to bytes/dev HEAD (4980c5c) which includes the strict
option and the prepare script for git+https consumers.
The hand-rolled postinstall.sh bootstrapped @railgun-reloaded/cryptography,
0zk-addresses, and scanner with nested npm install + build steps. All three
now ship a 'prepare: tsc --build' script, so npm's standard install
lifecycle handles them automatically — provided the lockfile pins SHAs that
include the prepare script.
- Remove postinstall.sh and its package.json entry.
- Add 'prepare: tsc --build' here so this package builds for git+https
consumers (e.g. balance-scanner) the same way.
- Refresh lockfile pins:
cryptography 5b121136 -> 8f4a819 (PR #8)
0zk-addresses c2ffb953 -> f4809f4
scanner 099522e8 -> ece364e (PRs in scanner/dev incl. #35)
These pins also drop @noble/secp256k1 (removed from cryptography) and
include scanner's .npmignore fix so built .js files actually ship.
mattgle
force-pushed
the
chore/drop-eddsa-poseidon-injection
branch
from
May 12, 2026 20:22
0e56a6a to
ba81e9b
Compare
bhflm
approved these changes
May 12, 2026
mattgle
added a commit
that referenced
this pull request
May 13, 2026
Convert all 13 brittle-using test files (the 11 brittle 'hook'-using
files plus commitment, encoding, mnemonic, wallet-info etc.) to
node:test + node:assert/strict.
- import (default-test | { hook, test }) from 'brittle' ->
{ before, test } from 'node:test' (only importing 'before' when the
file used hook), plus 'node:assert/strict' for assertions.
- hook('name', fn) -> before(fn). node:test 'before' takes only the
callback; the descriptive name argument is dropped.
- t.is -> assert.equal, t.alike -> assert.deepEqual,
t.ok -> assert.ok, t.not -> assert.notEqual, t.fail -> assert.fail,
t.absent(x) -> assert.ok(!x), t.pass(msg) -> assert.ok(true, msg),
t.unlike -> assert.notDeepEqual.
- Synchronous t.exception(() => ...) -> assert.throws(() => ...).
- Asynchronous await t.exception(async () => ...) ->
await assert.rejects(async () => ...).
- t.execution(() => ...) -> assert.doesNotThrow(() => ...).
- Test callbacks no longer take a (t) parameter; assertions use the
module-level imported assert.
- npm test invokes 'node --test --enable-source-maps' over
test/*.test.js and test/**/*.test.js. Drops brittle's --solo flag
for test:only in favour of node:test's --test-only.
- Drop brittle and @types/brittle from devDependencies.
Local end-to-end verification was blocked by a pre-existing build
error on origin/main (src/keys.ts:52 calls initializeEddsa with one
argument; the current cryptography main signature takes none). That
fix is in flight on the chore/drop-eddsa-poseidon-injection branch
(PR #19) and is intentionally out of scope here. Test files were
checked syntactically — no brittle imports or t.* assertion methods
remain.
mattgle
added a commit
that referenced
this pull request
May 13, 2026
Convert all 13 brittle-using test files (the 11 brittle 'hook'-using
files plus commitment, encoding, mnemonic, wallet-info etc.) to
node:test + node:assert/strict.
- import (default-test | { hook, test }) from 'brittle' ->
{ before, test } from 'node:test' (only importing 'before' when the
file used hook), plus 'node:assert/strict' for assertions.
- hook('name', fn) -> before(fn). node:test 'before' takes only the
callback; the descriptive name argument is dropped.
- t.is -> assert.equal, t.alike -> assert.deepEqual,
t.ok -> assert.ok, t.not -> assert.notEqual, t.fail -> assert.fail,
t.absent(x) -> assert.ok(!x), t.pass(msg) -> assert.ok(true, msg),
t.unlike -> assert.notDeepEqual.
- Synchronous t.exception(() => ...) -> assert.throws(() => ...).
- Asynchronous await t.exception(async () => ...) ->
await assert.rejects(async () => ...).
- t.execution(() => ...) -> assert.doesNotThrow(() => ...).
- Test callbacks no longer take a (t) parameter; assertions use the
module-level imported assert.
- npm test invokes 'node --test --enable-source-maps' over
test/*.test.js and test/**/*.test.js. Drops brittle's --solo flag
for test:only in favour of node:test's --test-only.
- Drop brittle and @types/brittle from devDependencies.
Local end-to-end verification was blocked by a pre-existing build
error on origin/main (src/keys.ts:52 calls initializeEddsa with one
argument; the current cryptography main signature takes none). That
fix is in flight on the chore/drop-eddsa-poseidon-injection branch
(PR #19) and is intentionally out of scope here. Test files were
checked syntactically — no brittle imports or t.* assertion methods
remain.
mattgle
added a commit
that referenced
this pull request
May 13, 2026
Convert all 13 brittle-using test files (the 11 brittle 'hook'-using
files plus commitment, encoding, mnemonic, wallet-info etc.) to
node:test + node:assert/strict.
- import (default-test | { hook, test }) from 'brittle' ->
{ before, test } from 'node:test' (only importing 'before' when the
file used hook), plus 'node:assert/strict' for assertions.
- hook('name', fn) -> before(fn). node:test 'before' takes only the
callback; the descriptive name argument is dropped.
- t.is -> assert.equal, t.alike -> assert.deepEqual,
t.ok -> assert.ok, t.not -> assert.notEqual, t.fail -> assert.fail,
t.absent(x) -> assert.ok(!x), t.pass(msg) -> assert.ok(true, msg),
t.unlike -> assert.notDeepEqual.
- Synchronous t.exception(() => ...) -> assert.throws(() => ...).
- Asynchronous await t.exception(async () => ...) ->
await assert.rejects(async () => ...).
- t.execution(() => ...) -> assert.doesNotThrow(() => ...).
- Test callbacks no longer take a (t) parameter; assertions use the
module-level imported assert.
- npm test invokes 'node --test --enable-source-maps' over
test/*.test.js and test/**/*.test.js. Drops brittle's --solo flag
for test:only in favour of node:test's --test-only.
- Drop brittle and @types/brittle from devDependencies.
Local end-to-end verification was blocked by a pre-existing build
error on origin/main (src/keys.ts:52 calls initializeEddsa with one
argument; the current cryptography main signature takes none). That
fix is in flight on the chore/drop-eddsa-poseidon-injection branch
(PR #19) and is intentionally out of scope here. Test files were
checked syntactically — no brittle imports or t.* assertion methods
remain.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
initializeEddsa(poseidonBuild.pure)was passing an argument that was already being silently ignored: the cryptography package shipped acircomlibjs+0.1.7.patchintended to add poseidon-injection support, butpatch-packagewas never wired up in cryptography's deps, socircomlibjs.buildEddsawas always the unpatched version that ignores its argument. circomlibjs builds its own internal poseidon regardless.Change
poseidonBuild.pureargument from theinitializeEddsa()call.poseidonBuildimport.Behavior
No change. Eddsa was already building its own poseidon. This is a code-cleanup follow-up to railgun-reloaded/cryptography#8, which removes the parameter from cryptography entirely once both sides have stopped passing it.
Sequencing
Independent of cryptography#8 at the type level — the cryptography PR keeps the parameter optional during the transition. This PR can land in either order, but landing it first (or in parallel) means cryptography#8 can fully drop the parameter when it merges.