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

upgrade dependencies, move to type script #38

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

motorina0
Copy link
Member

@motorina0 motorina0 commented Dec 20, 2021

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

@motorina0
Copy link
Member Author

Regarding this comment:
bitcoinjs/tiny-secp256k1#68 (comment)

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 :)

@junderw
Copy link
Member

junderw commented Dec 21, 2021

Perhaps we should also move from travis to github actions.

@motorina0
Copy link
Member Author

Updates:

  • change from Travis to GitHub Actions
  • update README and CHANGELOG

@motorina0
Copy link
Member Author

  • replace "secp256k1": "^4.0.2 with tiny-secp256k1": "^2.2.0
    Commit: e4840bb

@lifeiscontent
Copy link

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

@motorina0
Copy link
Member Author

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.

@junderw
Copy link
Member

junderw commented Aug 9, 2022

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.

@landabaso
Copy link

Oh no! I totally missed this one 🤦‍♂️
I'm closing PR #47

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

Successfully merging this pull request may close these issues.

None yet

4 participants