bitcoinjs / tiny-secp256k1

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

expose recover() function #68

Closed motorina0 closed 2 years ago

motorina0 commented 2 years ago

Related discussion: https://github.com/bitcoinjs/ecpair/issues/3#issuecomment-994129267

 function verify (message,...
    ...
    const publicKey = secp256k1.recover(
      hash,
      parsed.signature,
      parsed.recovery,
      parsed.compressed
    )
junderw commented 2 years ago

I remember back when tiny-secp256k1 first started, bitcoin-message was brought up.

The reality is: the message signing from Bitcoin Core early versions was never standardized. Many wallets have incompatible methods that are slightly similar, especially in dealing with the various address types. (back then, there was only one address type and the only variance was compressed or not.)

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)

Adding recovery just to fill a need for bitcoin-message is not a good reason enough imo.

junderw commented 2 years ago

It could be useful for BOLT11 and various Lightning usecases though.

motorina0 commented 2 years ago

I am trying to have a look at this, but I get a compilation error when running make. It's my first time using Rust.

For more information about this error, try rustc --explain E0432.



I assume the `recovery` module must be `enabled` or something of the sort.
I did add `--enable-module-recovery`  to `util/gen-fixtures/Makefile` but no luck.
junderw commented 2 years ago

you need to enable the recovery feature in the Cargo.toml file entry for secp256k1-sys

junderw commented 2 years ago
secp256k1-sys = { version = "0.4.1", default-features = false, features = ["recovery"], git = "https://github.com/rust-bitcoin/rust-secp256k1", rev = "455ee57ba4051bb2cfea5f5f675378170fb42c7f" }

should give you a new recovery module to work with.

motorina0 commented 2 years ago

should give you a new recovery module to work with.

No luck, exactly the same error. I can't see what I'm missing. rust-secp256k1/Cargo.toml has

[features]
recovery = ["secp256k1-sys/recovery"]

rust-secp256k1/secp256k1-sys/Cargo.tom has

[features]
recovery = []

secp256k1-sys/build.rs has:

    #[cfg(feature = "recovery")]
    base_config.define("ENABLE_MODULE_RECOVERY", Some("1"));

According to dependency-features doc:

Features of dependencies can also be enabled in the [features] table. The syntax is "package-name/feature-name"

junderw commented 2 years ago

you need to import from the recovery submodule, not the root crate.

junderw commented 2 years ago

I am talking about tiny-secp256k1's Cargo.toml btw

motorina0 commented 2 years ago

you need to import from the recovery submodule, not the root crate.

Thanks, that was the issue: use secp256k1_sys::recovery::{ secp256k1_ecdsa_recover };

Noob mistake.

motorina0 commented 2 years ago

PR merged. https://github.com/bitcoinjs/tiny-secp256k1/pull/69