search

bitcoinjs / tiny-secp256k1

A tiny secp256k1 native/JS wrapper
MIT License
86 stars 55 forks source link

There are 2 test vectors for pointCompress which are not correct #103

Closed landabaso closed 1 year ago

landabaso commented 1 year ago

The following 2 test vectors for pointCompress should have: "expected": "022f01e5e15cca351daff3843fb70f3c2f0a1bdd05e5af888a67784ef3e10a2a01"

     {
        "P": "042f01e5e15cca351daff3843fb70f3c2f0a1bdd05e5af888a67784ef3e10a2a015c4da8a741539949293d082a132d13b4c2e213d6ba5b7617b5da2cb76cbde904",
        "noarg": true,
        "expected": "042f01e5e15cca351daff3843fb70f3c2f0a1bdd05e5af888a67784ef3e10a2a015c4da8a741539949293d082a132d13b4c2e213d6ba5b7617b5da2cb76cbde904",
        "note": "Need to test if pointCompress keeps compressed status without any compressed arg"
      },
      {
        "P": "042f01e5e15cca351daff3843fb70f3c2f0a1bdd05e5af888a67784ef3e10a2a015c4da8a741539949293d082a132d13b4c2e213d6ba5b7617b5da2cb76cbde904",
        "noarg": false,
        "expected": "042f01e5e15cca351daff3843fb70f3c2f0a1bdd05e5af888a67784ef3e10a2a015c4da8a741539949293d082a132d13b4c2e213d6ba5b7617b5da2cb76cbde904",
        "note": "Need to test if pointCompress keeps compressed status when explicitly passing undefined. (This is why this fixture has no 'compressed')"
      }

You can find them here.

See how previous valid test using explicit compress = true, has the expected value above.

junderw commented 1 year ago

This is actually as designed.

As stated in the note, pointCompress requires a boolean to explicitly change the compressed state. If passing no boolean or undefined explicitly, pointCompress returns the input unchanged.

It is a bit confusing, but that is how the API is designed.

As a breaking change, this API could be modified to be a little bit more intuitive.

landabaso commented 1 year ago

You're correct. I'm working on an npm package that can serve as an alternative to tiny-secp256k1 for bitcoinjs, but without using WASM so that it can be used in React Native (using Paul Miller's noble). This package will be released as an npm package in the next few days, and I wanted to thoroughly test it using the same tests as tiny-secp256k1. These 2 fixtures failed, which prompted me to open this issue. The errors were on my package instead. My mistake.