Skip to content

fix(crypto): replace elliptic with @noble/curves for secp256k1; retry RPC on 429; CI on Node 22 - #200

Open
pyramation wants to merge 2 commits into
mainfrom
feat/replace-elliptic
Open

pyramation wants to merge 2 commits into
mainfrom
feat/replace-elliptic

Conversation

@pyramation

@pyramation pyramation commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #190. All secp256k1 signing, verification, recovery, and key derivation used elliptic, which is flagged by GHSA-848j-6mx2-7j84 on every published version (latest 6.6.1, so bumping doesn't help). This switches it to @noble/curves (already used by the Ethereum package) and removes elliptic from the install graph entirely.

  • packages/crypto/src/secp256k1.ts: makeKeypair / createSignature / verifySignature / recoverPubkey / compressPubkey / uncompressPubkey now use secp256k1 from @noble/curves/secp256k1. Same public API and output:
    sign(hash, privkey, { prehash: false, lowS: true })   // was keypair.sign(hash, { canonical: true })
    verify(r32||s32, hash, pubkey, { prehash: false, lowS: false })  // elliptic didn't enforce low-S on verify either
    Note: verify builds r(32)||s(32) explicitly because ExtendedSecp256k1Signature.toFixedLength() appends the recovery byte.
  • packages/crypto/src/slip10.ts: BIP-32 parent point serialization uses secp256k1.getPublicKey(privkey, true); bn.js stays for the scalar math.
  • packages/auth/src/config/algorithms.ts: sync makeKeypair uses getPublicKey(privkey, false).
  • Removed unused @ethersproject/* deps from ethereum and injective (no source imports them); @ethersproject/signing-key was the remaining transitive path to elliptic. rg elliptic pnpm-lock.yaml is now empty.
  • @noble/curves pinned to ^1.9.7, not v2: noble v2 (and @scure/bip32 v2) are ESM-only. They break the CJS build for consumers on Node < 20.19 and break Jest in this repo. That's why fix: dependency upgrades (bech32 v2, @noble/hashes v2, bs58 v6, @noble/curves v2, chain-registry v2) #198's CI is red. 1.9.7 ships CJS + ESM and already has the v2-style API.
  • HttpRpcClient retries HTTP 429 (packages/utils/src/clients/http-client.ts). Run Tests was red because the Solana suite's live-devnet tests got 429 Too Many Requests from GitHub runners. Public devnet answers 429 with Retry-After: 10. Every user of the shared client (cosmos/ethereum/solana) hit the same wall, so the fix goes in the client, not the tests:
    new HttpRpcClient(endpoint, { timeout, headers, maxRetries /* default 3 */ })
    // on 429: wait Retry-After (seconds or HTTP date, capped 30s), else 500ms * 2^attempt; then re-POST
    // other statuses / network errors: unchanged, no retry
    Covered by http-client.spec.ts (Retry-After path, exhausted retries, maxRetries: 0, exponential backoff, no retry on 500).
  • README: migration-guide link drops .mdx (403 → 200). Fixes Migration doc is not accessible #155.
  • CI: Node 20 → 22, actions/checkout / setup-node v4 → v5.

Verification: Run Tests and E2E are green on CI. Locally (Node 24), full pnpm run build passes, and these unit suites pass: crypto (131, including the existing pyca/cryptography vectors, deterministic signatures, recovery, compression, and SLIP-10 vectors), auth (42), utils (39), solana (181, against live devnet), amino, math, encoding, types, and pubkey. eslint crashes on the repo config both here and on main (Failed to load plugin '@typescript-eslint'), so I didn't run lint.

Link to Devin session: https://app.devin.ai/sessions/a02c84f654b94cbcb50936e98373fba5
Open in Devin Desktop: https://app.devin.ai/desktop/session/a02c84f654b94cbcb50936e98373fba5?variant=devin
Requested by: @pyramation

@devin-ai-integration

Copy link
Copy Markdown

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot changed the title fix(crypto): replace elliptic with @noble/curves for secp256k1; CI on Node 22 fix(crypto): replace elliptic with @noble/curves for secp256k1; retry RPC on 429; CI on Node 22 Oct 2, 2026
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.

private key exposure bug in elliptic Migration doc is not accessible

1 participant