tweakUtils when the tweak is 0 throws

[landabaso]

Hi @paulmillr ,

I am coming from bitcoinjs/ecpair#13.

I am using noble-secp256k1 with ecpair.

Since I still need tweakUtils' privatedAdd, ... I decided to copy the code on your tests to my wrapper as you suggest in Release Notes of 1.7.0.

Starting from version 2.1.0, ecpair runs additional tests on the embedded library.

There is this one new test that does not pass:

      ecc.privateAdd(
        h('0000000000000000000000000000000000000000000000000000000000000001'),
        h('0000000000000000000000000000000000000000000000000000000000000000'),
      ),

The reason is normalization of the tweak const t = normalizePrivateKey(tweak); throws because the tweak is zero.

I checked your tests for tweakUtils:

    it('privateAdd()', () => {
      for (const vector of privates.valid.add) {
        const { a, b, expected } = vector;
        expect(secp.utils.bytesToHex(tweakUtils.privateAdd(a, b))).toBe(expected);
      }

This passes.

Then I saw you have other test vectors for privateAdd in test/vectors/privates.json that include the tweak = 0 case:

    "privateAdd": [
      {
        "description": "1 + 0 == 1",
        "d": "0000000000000000000000000000000000000000000000000000000000000001",
        "tweak": "0000000000000000000000000000000000000000000000000000000000000000",
        "expected": "0000000000000000000000000000000000000000000000000000000000000001"
      }

But, for some reason, these vectors are not tested in your tests.

If I add them (see below), they do not pass (because the tweak = 0).

      for (const vector of privates.valid.privateAdd) {
        const { d, tweak, expected } = vector;
        expect(secp.utils.bytesToHex(tweakUtils.privateAdd(d, tweak))).toBe(expected);
      }
    });

(the test above fails)

I know tweakUtils are not part of released noble-secp256k1 anymore but decided to let you know anyway in case you want to take a look to your tests.

[paulmillr]

Thanks. This should be adjusted then

[paulmillr]

On the other hand, the current behavior was made on purpose. Why would you add an all-zero private key? It's certainly invalid in ECDH context, and in some others.

[landabaso]

On the other hand, the current behavior was made on purpose. Why would you add an all-zero private key? It's certainly invalid in ECDH context, and in some others.

I understand your point. It's more of an interoperability issue with ecpair. Not really a bug.
This issue concerns tweakUtils which aren't exposed anyway.

I copied and adjusted tweakUtils in my project with this little change. I just wanted to share this with you so you are aware.
BTW, in order to make noble-secp256k1 and ecpair work together, I also had to adjust tweakUtils functions (in fact, in the wrapper around them) so that they throw when the computed private key is zero.

Feel free to close the issue as this is more of a comment than a bug.

[coolaj86]

@landabaso, I'm also trying to use this with HD wallet paths (to make a fully browser-compatible implementation).

What is "tweakUtils"? Whether I do a search for "tweakutils" on npm or "tweakutils secp256k1" on the web, I don't seem to get back any relevant results.

What privateAdd() function are you referring to? I don't see it in the docs or code here or the ecpair repo.

Could you post a link to your implementation?

[paulmillr]

For tweak utils, look into secp tests: they have the utils. It was part of the main pkg, then got removed because I think it has bad UX and it's just legacy of other libraries while here we have better primitives (Point).

For hdkey, take a look at scure/bip32.

[landabaso]

Could you post a link to your implementation?

I hope it helps @coolaj86: https://github.com/bitcoinerlab/secp256k1

[coolaj86]

@landabaso I was able to get this to work with our APIs.

For anyone looking on, this is the code that's missing (i.e. for HD Wallets):

• privateAdd is known in other libraries as privateKeyTweakAdd
• pointAddScalar is known in other libraries as publicKeyTweakAdd

  let tweakUtils = {
    /**
     * @param {Uint8Array} privateKey
     * @param {Uint8Array} tweak
     * @returns {Uint8Array} - a new (derivative) privateKey
     */
    privateAdd: function (privateKey, tweak) {
      const p = Secp256k1.utils._normalizePrivateKey(privateKey);
      const t = Secp256k1.utils._normalizePrivateKey(tweak);
      return Secp256k1.utils._bigintTo32Bytes(
        Secp256k1.utils.mod(p + t, Secp256k1.CURVE.n),
      );
    },

    /**
     * @param {Uint8Array} p
     * @param {Uint8Array} tweak
     * @param {Boolean} [isCompressed]
     * @returns {Uint8Array} - a new (derivative) publicKey
     */
    pointAddScalar: function (p, tweak, isCompressed) {
      const P = Secp256k1.Point.fromHex(p);
      const t = Secp256k1.utils._normalizePrivateKey(tweak);
      const Q = Secp256k1.Point.BASE.multiplyAndAddUnsafe(P, t, 1n);
      if (!Q) {
        throw new Error("Tweaked point at infinity");
      }
      return Q.toRawBytes(isCompressed);
    },
  };

Adapted from ./test/index.ts:

noble-secp256k1/test/index.ts

Lines 466 to 492 in e125abd

	const tweakUtils = {
	privateAdd: (privateKey: PrivKey, tweak: Hex): Uint8Array => {
	const p = normal(privateKey);
	const t = normal(tweak);
	return secp.utils._bigintTo32Bytes(secp.utils.mod(p + t, secp.CURVE.n));
	},
	privateNegate: (privateKey: PrivKey): Uint8Array => {
	const p = normal(privateKey);
	return secp.utils._bigintTo32Bytes(secp.CURVE.n - p);
	},
	pointAddScalar: (p: Hex, tweak: Hex, isCompressed?: boolean): Uint8Array => {
	const P = secp.Point.fromHex(p);
	const t = normal(tweak);
	const Q = secp.Point.BASE.multiplyAndAddUnsafe(P, t, 1n);
	if (!Q) throw new Error('Tweaked point at infinity');
	return Q.toRawBytes(isCompressed);
	},
	pointMultiply: (p: Hex, tweak: Hex, isCompressed?: boolean): Uint8Array => {
	const P = secp.Point.fromHex(p);
	const h = typeof tweak === 'string' ? tweak : secp.utils.bytesToHex(tweak);
	const t = BigInt(`0x${h}`);
	return P.multiply(t).toRawBytes(isCompressed);
	},
	};

This may need to be updated again for the upcoming major refactor.

[landabaso]

Yeah, you can get these functions from there.
BTW, there's a full implementation in the link I copied above as an npm package ready to use and heavily tested:

https://github.com/bitcoinerlab/secp256k1

Let me know if I can help you somehow.

[coolaj86]

Thanks.

We're optimizing for the fewest dependencies and using only WebCrypto for both Web and node v18+.
(dropping node-crypto and <= node v16 entirely)

We have our own fork for a number of repos, as well as from-scratch rewrites, and the goal is that they should all work with CommonJS, ESM, Browsers, Node, and Bundlers without transpiling (adds about 5 lines of wrapper boilerplate per module).

We're preparing our "launch" of these tools for not this Monday, but the following Monday:

https://github.com/dashhive/

It's mostly just for our own community and our own tools - moving towards a fully vertically integrated model for a more optimized client experience and better developer experience - rather than so many half-baked, decade-old modules across so many ecosystems (hence @noble for secp256k1).