Skip to content

fix(bitcore-lib): normalize elliptic BN in ECDSA.sign#4144

Open
wqy000wqy wants to merge 1 commit intobitpay:masterfrom
wqy000wqy:fix/bitcore-lib-ecdsa-bn-normalization
Open

fix(bitcore-lib): normalize elliptic BN in ECDSA.sign#4144
wqy000wqy wants to merge 1 commit intobitpay:masterfrom
wqy000wqy:fix/bitcore-lib-ecdsa-bn-normalization

Conversation

@wqy000wqy
Copy link
Copy Markdown

Summary

This fixes an assertion failure in bitcore-lib ECDSA.sign() that can happen in bundled environments where bitcore-lib and elliptic end up using different bn.js instances.

Root cause

ECDSA.sign() computes:

Q = G.mul(k);
r = Q.x.umod(N);

In some bundled runtimes:

  • Q.x comes from elliptic
  • N comes from bitcore's BN wrapper

Even though both are BN-like values, they are not guaranteed to be compatible for direct arithmetic across instances, which can trigger internal assertions.

Fix

Normalize the elliptic coordinate through bytes before modular arithmetic:

r = BN.fromBuffer(Buffer.from(Q.getX().toArray('be', 32))).umod(N);

Q.x is produced by elliptic, while the subsequent arithmetic uses bitcore's BN wrapper. Normalizing through bytes preserves the math while avoiding cross-instance BN operations.

Validation

  • added a regression test in test/crypto/ecdsa.js
  • verified npx mocha test/crypto/ecdsa.js
  • verified npx mocha test/message.js

@wqy000wqy
Copy link
Copy Markdown
Author

ci/circleci: bitcore-lib passed successfully.

The only failing check appears to be ci/circleci: bitcore-node, specifically an EVM integration test in test/integration/routes/address.test.ts:

  • Address Routes
  • EVM
  • should get token transactions

The failure is expected [] to deeply equal [...], which looks like the test did not receive the expected token transaction fixtures/data in CI.

Since this PR only changes packages/bitcore-lib/lib/crypto/ecdsa.js and its regression test, this failure appears unrelated to the bitcore-lib ECDSA change.

@kajoseph
Copy link
Copy Markdown
Collaborator

@wqy000wqy Thanks for the PR. You'll need to sign your commits before this can be merged - see CONTRIBUTING.md

@kajoseph kajoseph added the BCN This pull request modifies the bitcore-node package label Apr 16, 2026
@wqy000wqy wqy000wqy force-pushed the fix/bitcore-lib-ecdsa-bn-normalization branch from 57216f0 to 06fced9 Compare April 17, 2026 02:41
@wqy000wqy wqy000wqy force-pushed the fix/bitcore-lib-ecdsa-bn-normalization branch from 06fced9 to 84cae99 Compare April 17, 2026 02:45
@wqy000wqy
Copy link
Copy Markdown
Author

I checked the existing issues before opening this PR.

This appears to be related to #3883, which describes broader compatibility problems around bitcore's BN/Point abstractions and their interaction with underlying bn.js / elliptic types. However, that issue does not appear to include a fix yet for this specific ECDSA.sign() failure.

I reproduced the problem against the latest master before preparing this change.

This PR is intended as a minimal, focused fix for the concrete failure in bitcore-lib ECDSA.sign(), by normalizing the elliptic coordinate into bitcore's BN type before modular arithmetic.

I have also updated the PR with a signed commit, and the latest commit is now marked as Verified.

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

Labels

BCN This pull request modifies the bitcore-node package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants