Skip to content

Conversation

dynst
Copy link
Contributor

@dynst dynst commented Oct 14, 2025

Half of #1721

@scure/bip39 2.0 is an ESM package with dependencies, and Yarn 4's import loader hooks are compatible with node 22+, not node 20, so there's an error in the node 20 CI. Same trouble as at #1804

node:internal/modules/esm/resolve:873
  throw new ERR_MODULE_NOT_FOUND(packageName, fileURLToPath(base), null);
        ^

Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@noble/hashes' imported from /home/runner/work/cosmjs/cosmjs/.yarn/cache/@scure-bip39-npm-2.0.1-715af0e367-ed8a0788bc.zip/node_modules/@scure/bip39/index.js
    at packageResolve (node:internal/modules/esm/resolve:873:9)
    at moduleResolve (node:internal/modules/esm/resolve:946:18)
    at defaultResolve (node:internal/modules/esm/resolve:1188:11)
    at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:708:12)
    at #cachedDefaultResolve (node:internal/modules/esm/loader:657:25)
    at ModuleLoader.getModuleJobForRequire (node:internal/modules/esm/loader:413:53)
    at new ModuleJobSync (node:internal/modules/esm/module_job:375:34)
    at ModuleLoader.importSyncForRequire (node:internal/modules/esm/loader:386:11)
    at loadESMFromCJS (node:internal/modules/cjs/loader:1363:24)
    at Module._compile (node:internal/modules/cjs/loader:1503:5) {
  code: 'ERR_MODULE_NOT_FOUND'
}

Node.js v20.19.5

@dynst dynst force-pushed the scure-bip39 branch 6 times, most recently from d9d68ef to 6187f4b Compare October 14, 2025 16:09
Copy link
Member

@webmaster128 webmaster128 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

"abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon art",
),
).toThrowError(/invalid word count(.*)got: 25/i);
).toThrowError(/invalid/i);
Copy link
Member

Choose a reason for hiding this comment

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

Can we make the error test tests more specific here? "invalid" could be anything

Copy link
Contributor Author

@dynst dynst Oct 14, 2025

Choose a reason for hiding this comment

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

In testing, the full error message here wasn't actually any more specific, so in a literal sense we can't. It's just two words, 'Invalid mnemonic'. Which could mean anything.

https://github.com/paulmillr/scure-bip39/blob/1.6.0/src/index.ts#L52

Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

LGTM. Is there anything left before merging this?

@webmaster128 webmaster128 added this to the 0.37 milestone Oct 15, 2025
@dynst
Copy link
Contributor Author

dynst commented Oct 15, 2025

Good to merge!

@webmaster128 webmaster128 merged commit 242673f into cosmos:main Oct 15, 2025
13 checks passed
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