Skip to content

Consider moving away from elliptic #309

Closed

Description

@bobbrodie

It looks like elliptic is not going to address the low severity vuln. Even if it doesn't affect this SDK, it would beneficial to consider options.

Something we could consider, if it doesn't break backward compatibility whatsoever with tokens could be https://github.com/paulmillr/noble-curves. There's a shim that can help as well: https://github.com/soatok/elliptic-to-noble.

Let's weigh our options, including patching elliptic.

Activity

  1. swlodarski-sumoheavy commented on Jun 24, 2026

    @swlodarski-sumoheavy
    Collaborator

    @aharoitx For this SDK, @noble/secp256k1 is the best fit in my opinion because we only need Bitcoin-style secp256k1 signing. It is smaller, purpose-built, and faster to reason about than @noble/curves, which is a broader multi-curve package and would add unnecessary code and complexity for no functional gain here.

    Also, @soatok/elliptic-to-noble is not a good long-term choice in this case. It is a migration bridge, not a final architecture, and it appears to be unmaintained for some time, with open issues that can block practical use. Keeping a bridge layer in production adds extra abstraction, maintenance overhead, and potential behavior mismatches versus using the native noble API directly. In short: direct @noble/secp256k1 usage is leaner, clearer, and safer for this Bitcoin-focused SDK.

    I also ran a quick validation with @noble/secp256k1 and got very good results: all requests worked after migration, including flows using newly created tokens and existing tokens (created with elliptic).

  2. aharoitx commented on Sep 23, 2026

    @aharoitx
    Contributor

    @swlodarski-sumoheavy Thanks, good to know it worked. Which version did you test with?

    I checked a few versions. Only 1.7.x can do DER, which we need for X-Signature:

    1.7.x: DER ✅, CommonJS
    2.x: compact only, ESM-only
    3.x (latest): throws Signature format "der" is not supported: switch to noble-curves

    So on 1.7.x we're on the old line, and moving to 3.x later means writing our own DER encoding or switching to @noble/curves. I'm fine with 1.7.x if we accept that.

    One more thing either way: KeyUtils is public and returns elliptic KeyPair objects (getPrivate(), getPublic()). Removing elliptic breaks that, so I think this should be 9.0.0 with a short migration note. Agree?

  3. swlodarski-sumoheavy commented on Sep 24, 2026

    @swlodarski-sumoheavy
    Collaborator

    @aharoitx I tested with @noble/secp256k1@3.1.0 and added a custom method that converts a 64-byte compact ECDSA signature into standard DER format.

    I agree that this is a breaking change and should go into 9.0.0.

  4. aharoitx commented on Sep 30, 2026

    @aharoitx
    Contributor

    Fixed in 8.1.0: https://github.com/bitpay/nodejs-bitpay-client/releases/tag/8.1.0

    The SDK now uses @noble/secp256k1 instead of elliptic. Existing keys keep the same public key and SIN, so no new pairing is needed. The move to @noble/secp256k1 3.x is tracked in #XXX for 9.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions