Skip to content

expose recover() function #68

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

Closed
motorina0 opened this issue Dec 17, 2021 · 10 comments
Closed

expose recover() function #68

motorina0 opened this issue Dec 17, 2021 · 10 comments

Comments

@motorina0
Copy link
Member

  • bitcoin-message lib is using secp256k1.recover()
  • this function is not exposed by tiny-secp256k1

Related discussion: bitcoinjs/ecpair#3 (comment)

 function verify (message,...
    ...
    const publicKey = secp256k1.recover(
      hash,
      parsed.signature,
      parsed.recovery,
      parsed.compressed
    )
@junderw
Copy link
Member

junderw commented Dec 17, 2021

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 junderw closed this as completed Dec 17, 2021
@junderw junderw reopened this Dec 17, 2021
@junderw
Copy link
Member

junderw commented Dec 17, 2021

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

@motorina0
Copy link
Member Author

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.

  • in lib.rs I've added secp256k1_ecdsa_recover to use secp256k1_sys::{...
  • run make (using the Docker image):
for f in ./lib/cjs/*.cjs; do sed -i -e 's/\(require(".*\)\.js");/\1.cjs");/g' -- "$f"; done
RUSTFLAGS="-C link-args=-zstack-size=655360" cargo build --target wasm32-unknown-unknown --release
    Updating git repository `https://github.com/rust-bitcoin/rust-secp256k1`
    Updating crates.io index
  Downloaded cc v1.0.71
  Downloaded 1 crate (57.5 KB) in 1.29s
   Compiling secp256k1-wasm v0.0.0 (/tiny-secp256k1)
error[E0432]: unresolved import `secp256k1_sys::secp256k1_ecdsa_recover`
  --> src/lib.rs:19:36
   |
19 |     secp256k1_ec_seckey_tweak_add, secp256k1_ecdsa_recover, secp256k1_ecdsa_sign,
   |                                    ^^^^^^^^^^^^^^^^^^^^^^^
   |                                    |
   |                                    no `secp256k1_ecdsa_recover` in the root
   |                                    help: a similar name exists in the module: `secp256k1_ecdsa_verify`

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
Copy link
Member

junderw commented Dec 21, 2021

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

@junderw
Copy link
Member

junderw commented Dec 22, 2021

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
Copy link
Member Author

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
Copy link
Member

junderw commented Dec 22, 2021

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

@junderw
Copy link
Member

junderw commented Dec 22, 2021

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

@motorina0
Copy link
Member Author

motorina0 commented Dec 22, 2021

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
Copy link
Member Author

PR merged. #69

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

No branches or pull requests

2 participants