Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add an example for tiny-secp256k1 alternative (implement TinySecp256k1Interface) #1781

Open
motorina0 opened this issue Mar 10, 2022 · 23 comments
Assignees

Comments

@motorina0
Copy link
Member

The existing tests and sample use tiny-secp256k1, however this lib might not be well suited for every one.

Request:

  • add an ECC example that implements the TinySecp256k1Interface but does not use tiny-secp256k1
  • eg: @noble/secp256k1
@motorina0 motorina0 self-assigned this Mar 10, 2022
@junderw
Copy link
Member

junderw commented Mar 10, 2022

Related paulmillr/noble-secp256k1#50

@landabaso
Copy link

landabaso commented Jul 8, 2022

I just hit this :)
I'm trying to use bitcoinjs-lib in an app built with react-native. I just found out wasm support is not there yet (unless using wkwebview - but I don't want to spend time digging how to use it).
I'll take a look to noble-secp256k1. Fingers crossed. Do you guys know of other alternatives to compare speed? I need to sign thousands of txs and would be great having something as fast as possible.

EDIT (Jan/11/23): I published this npm package that implements tiny-secp256k1 API but using noble-secp256k1: https://github.com/bitcoinerlab/secp256k1

It allows replacing tiny-secp256k1 If you can support BigInt but not WASM.

//const ecc = require('tiny-secp256k1');
const ecc = require('@bitcoinerlab/secp256k1');
const { BIP32Factory } = require('bip32');
const { ECPairFactory } = require('ecpair');
const BIP32 = BIP32Factory(ecc);
const ECPair = ECPairFactory(ecc);

I made it sure it passes all the tests used in tiny-secp256k1. But, please add your own tests when using it.

@motorina0
Copy link
Member Author

motorina0 commented Jul 8, 2022

The tiny-secp256k1 lib has a benchmark framework. See https://github.com/bitcoinjs/tiny-secp256k1/tree/master/benches

You could try to replace the tiny-secp256k1 implementation with the noble-secp256k1 one and check the difference in performance.

@landabaso
Copy link

landabaso commented Jul 8, 2022

Thanks @motorina0 . Before benchmarking first I wanted to see if it would work. Unfortunately it's not plug-n-play.

Some methods are missing. F.ex. I have this in some part of my code:

import ECPairFactory from 'ecpair';
let fromPublicKey;

//import('tiny-secp256k1').then(ecc => {
//  fromPublicKey = ECPairFactory(ecc).fromPublicKey;
//});

import * as ecc from 'noble-secp256k1';
fromPublicKey = ECPairFactory(ecc).fromPublicKey;

But ECPair factory complains about not having isPoint.
In fact I checked the the benchmark you mentioned and saw many methods were not tested because of this reason.

EDIT:

I made some progress.
I found this file that implements the rest of the interface in javascript. I'm almost there. Apparently it's being used in Bitgo somehow.

I put it together but it fails the sign test, also the verify test (also for sign and verify for schnorr). F.ex., this fails:

  assert(
    Buffer.from(
      ecc.sign(
        h('5e9f0a0d593efdcf78ac923bc3313e4e7d408d574354ee2b3288c0da9fbba6ed'),
        h('fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364140'),
      ),
    ).equals(
      h(
        '54c4a33c6423d689378f160a7ff8b61330444abb58fb470f96ea16d99d4a2fed07082304410efa6b2943111b6a4e0aaa7b7db55a07e9861d1fb3cb1f421044a5',
      ),
    ),
  );

This is the file that implements the failing tests.

Any idea what may be happening @brandonblack @paulmillr? I'm using npm package 'noble-secp256k1'. Should I be using some specific version? Thanks

@brandonblack
Copy link
Contributor

You do have to do some coding to fill in the rest of the interface. An example that may eventually be used at BitGo is here: https://github.com/BitGo/BitGoJS/blob/bitcoinjs_lib_6_sync/modules/utxo-lib/src/noble_ecc.ts

@paulmillr
Copy link
Contributor

We have a compatibility layer with tiny-secp, it's located here: https://github.com/ethereum/js-ethereum-cryptography/blob/master/src/secp256k1-compat.ts

@landabaso
Copy link

landabaso commented Jul 8, 2022

-I have edited the original message (see below). I managed to make it work-

Thanks @brandonblack @paulmillr I am in fact already using Bitgo's compatibility layer.

EDIT:

Now I see the problem. I was installing this:

npm install noble-secp256k1 (it's an older 1.2 version)
(I took it from https://github.com/bitcoinjs/tiny-secp256k1/blob/master/benches/package.json)

I guess it would be a good idea to update that file and also remove old noble-secp256k1 npm package to avoid confusions.

while it should be:

npm install @noble/secp256k1 (1.6)

Now it works together bitcoinjs-lib/bip32 & ecpair just fine! It's a bit slower (2x slower or so than tiny-secp256k1 signing thousands of txs) but works and passes my tests!

@paulmillr
Copy link
Contributor

paulmillr commented Jul 25, 2022

@junderw What's the decision regarding usage of @noble cryptography in bitcoinjs? Would you be open to integration? Metamask is planning to update crypto libs to noble, i'm walking through all MM deps which are using alternative projects.

@junderw
Copy link
Member

junderw commented Jul 26, 2022

No decision is needed.

bitcoinjs is now modular, so as long as you can fill the TinySecp256k1Interface interface it doesn't matter what library you use.

In fact, I believe BitGo is already using @noble with bitcoinjs-lib@v6

If you'd like to provide a separate package that wraps noble into that interface and add something to our examples/README that shows your alternative and how to plug it in to bitcoinjs, I am more than happy to review/merge it.

@junderw
Copy link
Member

junderw commented Jul 26, 2022

@brandonblack Do you have a link to the wrapper you created? (I remember you sent it to me a long time ago.)

@junderw
Copy link
Member

junderw commented Jul 26, 2022

As a side note, I have personally reviewed @noble myself. Great work.

@paulmillr
Copy link
Contributor

paulmillr commented Jul 26, 2022

Thanks, Jonathan!

What about create-hash? Thoughts on replacing it with /hashes? It was audited and supports tree shaking/typescript.

@junderw
Copy link
Member

junderw commented Jul 26, 2022

I was actually considering making hashes also modular, but hashing it too intertwined with almost every data type in this library to split it out, so I decided against it.

I would be open to considering replacing create-hash, though.

@landabaso
Copy link

landabaso commented Jul 26, 2022

@junderw In case it helps, in FarVault I'm currently using bitcoinjs with 3 different ecc libraries (including noble with BitGo's wrapper -links below-).

This is how it works. The user imports an ecc module which automatically selects tiny-secp256k1 (WASM), noble or elliptic-js (old v1 tiny-secp256k1) depending on the runtime environment:

import * as ecc from './secp256k1';

Then ecc can be passed to bitcoinjs as usual: const bip32 = BIP32Factory(ecc); or const ECPair = ECPairFactory(ecc);

The implementation is trivial. See here https://github.com/farvault/farvault-lib/blob/main/src/secp256k1.js.

As you said noble needs a tiny compatibility layer.

I am using this https://github.com/farvault/farvault-lib/blob/main/src/noble_ecc.js compatibility layer for noble.
I took it from BitGo @brandonblack and slightly adapted it for my purposes. All credits go to @brandonblack/BitGo.

@meketiuk
Copy link

meketiuk commented Nov 3, 2022

After many hours of searching and coding, I found a solution that works for me.

Packages vesrsion:

"@noble/secp256k1": "1.7.0",
"bip32": "^3.1.0",
"bitcoinjs-lib": "^6.0.2",
"create-hash": "^1.2.0",
"create-hmac": "^1.1.7",
"ecpair": "^2.1.0",

TypeScript file

noble-ecc-wrapper.ts
import * as necc from '@noble/secp256k1';
import { BIP32API, BIP32Factory, BIP32Interface } from 'bip32';
import createHash from 'create-hash';
import createHmac from 'create-hmac';
import { ECPairAPI, ECPairFactory, ECPairInterface } from 'ecpair';

necc.utils.sha256Sync = (...messages: Uint8Array[]): Uint8Array => {
  const sha256 = createHash('sha256');
  for (const message of messages) sha256.update(message);
  return sha256.digest();
};

necc.utils.hmacSha256Sync = (
  key: Uint8Array,
  ...messages: Uint8Array[]
): Uint8Array => {
  const hash = createHmac('sha256', Buffer.from(key));
  messages.forEach((m) => hash.update(m));
  return Uint8Array.from(hash.digest());
};

const normalizePrivateKey = necc.utils._normalizePrivateKey;

const defaultTrue = (param?: boolean): boolean => param !== false;

function throwToNull<Type>(fn: () => Type): Type | null {
  try {
    return fn();
  } catch (e) {
    return null;
  }
}

function isPoint(p: Uint8Array, xOnly: boolean): boolean {
  if ((p.length === 32) !== xOnly) return false;
  try {
    return !!necc.Point.fromHex(p);
  } catch (e) {
    return false;
  }
}

function hexToNumber(hex) {
  if (typeof hex !== 'string') {
    throw new TypeError('hexToNumber: expected string, got ' + typeof hex);
  }
  return BigInt(`0x${hex}`);
}

function bytesToNumber(bytes) {
  return hexToNumber(necc.utils.bytesToHex(bytes));
}

function normalizeScalar(scalar) {
  let num;
  if (typeof scalar === 'bigint') {
    num = scalar;
  } else if (
    typeof scalar === 'number' &&
    Number.isSafeInteger(scalar) &&
    scalar >= 0
  ) {
    num = BigInt(scalar);
  } else if (typeof scalar === 'string') {
    if (scalar.length !== 64)
      throw new Error('Expected 32 bytes of private scalar');
    num = hexToNumber(scalar);
  } else if (scalar instanceof Uint8Array) {
    if (scalar.length !== 32)
      throw new Error('Expected 32 bytes of private scalar');
    num = bytesToNumber(scalar);
  } else {
    throw new TypeError('Expected valid private scalar');
  }
  if (num < 0) throw new Error('Expected private scalar >= 0');
  return num;
}

function pointAddScalar(p, tweak, isCompressed) {
  const P = necc.Point.fromHex(p);
  const t = normalizeScalar(tweak);
  const Q = necc.Point.BASE.multiplyAndAddUnsafe(P, t, BigInt(1));
  if (!Q) throw new Error('Tweaked point at infinity');
  return Q.toRawBytes(isCompressed);
}

function pointMultiply(p, tweak, isCompressed) {
  const P = necc.Point.fromHex(p);
  const h = typeof tweak === 'string' ? tweak : necc.utils.bytesToHex(tweak);
  const t = BigInt(`0x${h}`);
  return P.multiply(t).toRawBytes(isCompressed);
}

function privateAdd(privateKey, tweak) {
  const p = normalizePrivateKey(privateKey);
  const t = normalizeScalar(tweak);
  const add = necc.utils._bigintTo32Bytes(necc.utils.mod(p + t, necc.CURVE.n));
  if (necc.utils.isValidPrivateKey(add)) return add;
  else return null;
}

function privateNegate(privateKey) {
  const p = normalizePrivateKey(privateKey);
  const not = necc.utils._bigintTo32Bytes(necc.CURVE.n - p);
  if (necc.utils.isValidPrivateKey(not)) return not;
  else return null;
}

const ecc = {
  isPoint: (p: Uint8Array): boolean => isPoint(p, false),
  isPrivate: (d: Uint8Array): boolean => necc.utils.isValidPrivateKey(d),
  isXOnlyPoint: (p: Uint8Array): boolean => isPoint(p, true),

  xOnlyPointAddTweak: (
    p: Uint8Array,
    tweak: Uint8Array
  ): { parity: 0 | 1; xOnlyPubkey: Uint8Array } | null =>
    throwToNull(() => {
      const P = pointAddScalar(p, tweak, true);
      const parity = P[0] % 2 === 1 ? 1 : 0;
      return { parity, xOnlyPubkey: P.slice(1) };
    }),

  pointFromScalar: (sk: Uint8Array, compressed?: boolean): Uint8Array | null =>
    throwToNull(() => necc.getPublicKey(sk, defaultTrue(compressed))),

  pointCompress: (p: Uint8Array, compressed?: boolean): Uint8Array => {
    return necc.Point.fromHex(p).toRawBytes(defaultTrue(compressed));
  },

  pointMultiply: (a: Uint8Array, tweak: Uint8Array, compressed?: boolean) =>
    throwToNull(() => pointMultiply(a, tweak, defaultTrue(compressed))),

  pointAdd: (
    a: Uint8Array,
    b: Uint8Array,
    compressed?: boolean
  ): Uint8Array | null =>
    throwToNull(() => {
      const A = necc.Point.fromHex(a);
      const B = necc.Point.fromHex(b);
      return A.add(B).toRawBytes(defaultTrue(compressed));
    }),

  pointAddScalar: (p: Uint8Array, tweak: Uint8Array, compressed?: boolean) =>
    throwToNull(() => pointAddScalar(p, tweak, defaultTrue(compressed))),

  privateAdd: (d: Uint8Array, tweak: Uint8Array): Uint8Array | null =>
    throwToNull(() => {
      const res = privateAdd(d, tweak);
      // tiny-secp256k1 returns null rather than allowing a 0 private key to be returned
      // ECPair.testEcc() requires that behavior.
      if (res?.every((i) => i === 0)) return null;
      return res;
    }),

  privateNegate: (d: Uint8Array): Uint8Array => privateNegate(d),

  sign: (h: Uint8Array, d: Uint8Array, e?: Uint8Array): Uint8Array => {
    return necc.signSync(h, d, { der: false, extraEntropy: e });
  },

  signSchnorr: (
    h: Uint8Array,
    d: Uint8Array,
    e: Uint8Array = Buffer.alloc(32, 0x00)
  ): Uint8Array => {
    return necc.schnorr.signSync(h, d, e);
  },

  verify: (
    h: Uint8Array,
    Q: Uint8Array,
    signature: Uint8Array,
    strict?: boolean
  ): boolean => {
    return necc.verify(signature, h, Q, { strict });
  },

  verifySchnorr: (
    h: Uint8Array,
    Q: Uint8Array,
    signature: Uint8Array
  ): boolean => {
    return necc.schnorr.verifySync(signature, h, Q);
  },
};

const ECPair: ECPairAPI = ECPairFactory(ecc);
const bip32: BIP32API = BIP32Factory(ecc);

export {
  ecc,
  ECPair,
  ECPairAPI,
  ECPairInterface,
  bip32,
  BIP32API,
  BIP32Interface,
};

and now you can use bip32 or ECPair directly from this file

Example

import { ECPair, bip32 } from '../utils/noble-ecc-wrapper';

const privateKey = '...';
const node = ECPair.fromWIF(privateKey);

@landabaso
Copy link

landabaso commented Jan 11, 2023

Hi there,

I've noticed many wrappers that use noble have been released into the wild. Some of these have been integrated into real products or are planned for inclusion.

I've noticed that some of these wrappers use code that has been shared in this thread (including earlier versions of my code), which had subtle bugs/misbehavior. The code does not strictly follow the API of tiny-secp256k1.

I suggest that you add the tiny-secp256k1 suite of tests to your wrappers/compatibility layers and fix any issues. You can find these tests here: https://github.com/bitcoinjs/tiny-secp256k1/tree/master/tests. Note that these tests are way more extensive than the runtime tests from ECPair.

When I ran these tests on my wrapper, I detected hundreds of failing tests that were not detected in the runtime tests of ECPair. These will also fail with some of the shared code in this thread. Some examples include:

  • The default values for pointCompress, pointFromScalar, pointMultiply (or any method that uses the compress parameter) use the defaultTrue method, which is incorrect. This is not handled correctly (see There are 2 test vectors for pointCompress which are not correct tiny-secp256k1#103)
  • privateAdd, pointAddScalar, xOnlyPointAddTweak, pointFromScalar, pointMultiply, pointAdd, and others, use a techinque that converts all exceptions in noble into null. This is incorrect. The tiny-secp256k1 API is implemented differently. For example, if a tweak, hash, extra data (in sign), private key or point is invalid, it should throw an exception, not return null.
  • The newest version of noble (1.7.1) does not throw for cases like "1 + -1 == 0/Infinity". As a result, you must handle this case now in your side.
  • There may be more subtle bugs and misbehavior that I cannot recall at the moment. I went through many of them.

These issues may or may not impact you, but it's worth taking a look and at least adding some test fixtures to your wrappers.

In any case, I have decided to publish an npm package with all of these corrections, in case it helps others. You can find the package here: https://github.com/bitcoinerlab/secp256k1. Contributions are more than welcome.

@paulmillr
Copy link
Contributor

@landabaso we have a compat layer here: https://github.com/ethereum/js-ethereum-cryptography/blob/master/src/secp256k1-compat.ts

It has a lot of tests to ensure the behavior matches the other package. Maybe also worth to use the tests.

@landabaso
Copy link

@landabaso we have a compat layer here: https://github.com/ethereum/js-ethereum-cryptography/blob/master/src/secp256k1-compat.ts

It has a lot of tests to ensure the behavior matches the other package. Maybe also worth to use the tests.

Thanks @paulmillr !

From a cursory glance, it appears that the compatibility layer that you reference has a different interface than the one in https://github.com/bitcoinjs/tiny-secp256k1#documentation, which is the one I followed.

I will investigate the necessary adjustments to the tests to incorporate them, though.

@mahnunchik
Copy link

Any news about implementing TinySecp256k1Interface for @noble library?

@junderw
Copy link
Member

junderw commented Mar 29, 2023

I have been experimenting with using wasm2js to create an asmjs version of the library.

I just need to figure out how to integrate it into the build process for releases, and trying to support both in the same package will probably cause issues since every bundler under the sun seems to treat package.json differently when bundling.

Perhaps publishing it as a separate package might be better. tiny-secp256k1-asmjs or something... not sure what name we should use.

bitcoinjs/tiny-secp256k1@8fb3498

This clobbers the existing WASM. Ideally I should make a build step that will modify the package name, modify the imports to use asm.js.

Also, a few tweaks to wasm2js command make performance much better (disabling GC, disabling input checks (we check them in the TS)) so I wonder if there are any other optimizations we could make to get asm.js performance a little better.

It's a given, but asm.js is somewhere around 10x to 20x slower than WASM depending on the operation, but most use cases that are bundling for React-Native etc. are not performance oriented (otherwise they wouldn't use React 🤣)

I also need to merge my refactor (which the asmjs branch is based on).

@junderw
Copy link
Member

junderw commented Jul 22, 2023

https://www.npmjs.com/package/@bitcoin-js/tiny-secp256k1-asmjs

Released asmjs version of tiny-secp256k1.

CI will automatically generate the packages for us, so releasing both packages simultaneously is simple.

@junderw
Copy link
Member

junderw commented Jul 22, 2023

Also @landabaso released @bitcoinerlab/secp256k1

I'm trying to update all the issues related to this, but there's a lot and they all tend to be different keywords.

@cpuchainorg
Copy link

For anyone looking for the code for UMD this simply works

const bitcoin = require('bitcoinjs-lib');
const { Buffer } = require('buffer');
const { ECPairFactory } = require('ecpair');
const ecc = require('@bitcoinerlab/secp256k1');

const ECPair = ECPairFactory(ecc);

bitcoin.initEccLib(ecc);

module.exports = {
  ...bitcoin,
  Buffer,
  ECPair
}

This should cover for the most cases using taproot including address creation, key tweaks, and PSBT signing

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

No branches or pull requests

8 participants