bitcoinjs-message icon indicating copy to clipboard operation
bitcoinjs-message copied to clipboard

upgrade dependencies, move to type script

Open motorina0 opened this issue 4 years ago • 9 comments

This PR has no functional changes. The main goals are to:

  • upgrade dependencies to latest versions (secp256k1 v3 particularly)
    • this small lib was forcing the user to carry some heavy dependecies
  • migrate to TypeScript (both the lib and the tests) - this makes it easier to check the code and understand the public interface
  • prepare for decoupling this lib from secp256k1 and move to injectable tiny-secp256k1
    • after this is issue is implemented: https://github.com/bitcoinjs/tiny-secp256k1/issues/68

motorina0 avatar Dec 20 '21 13:12 motorina0

Regarding this comment: https://github.com/bitcoinjs/tiny-secp256k1/issues/68#issuecomment-996828894

To be honest... bitcoin-message should be archived. At this point any "updates" to it would be meaningless back-end optimizations and arguing minutiae about why we should adopt format X over format Y or how we could make it backwards compatible with format Z (where X Y Z are all different wallet apps that put their own spin on message signing)

This PR is not trying to do any functional updates. Allowing to have a custom ecc lib instead of hardcoded dependency to secp256k1 makes this lib much lighter. Then it can be archived :)

motorina0 avatar Dec 20 '21 13:12 motorina0

Perhaps we should also move from travis to github actions.

junderw avatar Dec 21 '21 01:12 junderw

Updates:

  • [x] change from Travis to GitHub Actions
  • [x] update README and CHANGELOG

motorina0 avatar Dec 21 '21 10:12 motorina0

  • [x] replace "secp256k1": "^4.0.2 with tiny-secp256k1": "^2.2.0 Commit: https://github.com/bitcoinjs/bitcoinjs-message/pull/38/commits/e4840bbbf51e40f9bcf05bc1e85a8e5c69d5eab3

motorina0 avatar Jan 22 '22 11:01 motorina0

@motorina0 @junderw any updates on this PR? ran into a build issue today related to an older version of secp256k1

lifeiscontent avatar Aug 08 '22 17:08 lifeiscontent

Am mentioned here, it is not clear if this library should be maintained or just archived. You can advocate here for keeping it alive.

Also tiny-secp256k does not work out of the box on some environments.

motorina0 avatar Aug 09 '22 05:08 motorina0

I'm willing to merge this.

  1. bump major version
  2. Add line at top of README about archiving this. Mention that if any bugs are found in the latest major version try using the last major version. Severe security vulnerabilities we might put out a patch.
  3. I'll merge, publish, and archive.

Maybe we should also mention BIP322.

I've been thinking about how to implement BIP322, but since it requires a script interpreter, I think maybe we should make it generic with a function parameter that solves the script and returns true or false... then we can offer defaults for p2pkh, p2wpkh, p2sh-p2wpkh, and encourage others to submit solvers for other script types. (but anyone can pass in their own solver function)...

That said, BIP322 adoption is near-zero right now, so I'm putting it off.

junderw avatar Aug 09 '22 05:08 junderw

Oh no! I totally missed this one 🤦‍♂️ I'm closing PR https://github.com/bitcoinjs/bitcoinjs-message/pull/47

landabaso avatar Feb 09 '24 11:02 landabaso