[ecash-secp256k1] Add `ecash-secp256k1-sys`
ClosedPublic
Actions

Authored by tobias_ruck on Oct 22 2024, 11:42.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

Summary

Import the secp256k1-sys crate from rust-secp256k1 repository, and apply the following modifications:

Removed the vendored secp256k1 library imported from bitcoin-core
Use our secp256k1 library of the monorepo
Add ABC's secp256k1_schnorr_sign and secp256k1_schnorr_verify, which are the Schnorr signatures used by ABC. These are distinct from the x-only pubkey BIP340 signatures from BTC, but we keep them to simplify backports, and to (in the future) allow eCash apps to also talk some BTC.
Port the GitHub CI from the repository to a test.sh. Those depend on https://github.com/rust-bitcoin/rust-bitcoin-maintainer-tools, so the relevant parts have been ported and cleaned up.
Adapt the Rust workspace structure to our workspace.
Reformat everything with our formatting rules.

Test Plan

./modules/ecash-secp256k1/contrib/test.sh

Repository

rABC Bitcoin ABC

Branch

ecash-secp256k1-sys

Lint

Lint Passed

Unit

No Test Coverage

Build Status

@bot build-ecash-secp256k1

tobias_ruck retitled this revision from [ecash-lib] Add `ecash-secp256k1-sys` to [ecash-secp256k1] Add `ecash-secp256k1-sys`.Oct 22 2024, 19:34

fuzz: fix linking & warnings, add ABC schnorr funcs

@bot build-ecash-secp256k1

tobias_ruck published this revision for review.Oct 22 2024, 20:09

Fabien requested changes to this revision.Oct 23 2024, 09:14

Fabien added inline comments.

modules/ecash-secp256k1/contrib/test.sh
8 ↗	(On Diff #50292)	Better reference from the top level which you can determine easily from git. It's also passed down by CI. See the other bash scripts, most of them do so. Rationale: it's much easier to grep if something is moved than a relative path and it works the same on all bash versions (even if here the script needs a recent enough version for other things). Also the var is missing surrounding brackets and would fail if there is a space in the path.
modules/ecash-secp256k1/ecash-secp256k1-sys/build.rs
17 ↗	(On Diff #50292)	Any chance we can reference from the repo root instead ? I'm not a fan of this ../../..
modules/ecash-secp256k1/ecash-secp256k1-sys/src/lib.rs
24 ↗	(On Diff #50292)	What is this doing ?

This revision now requires changes to proceed.Oct 23 2024, 09:14

modules/ecash-secp256k1/ecash-secp256k1-sys/build.rs
17 ↗	(On Diff #50292)	Apparently it's not available via environment variable: https://users.rust-lang.org/t/workspace-root-environment-variable/104779 We can use anything in `fn main` to figure it out, but I think the complexity will be bigger than just using ../../../ for now...
modules/ecash-secp256k1/ecash-secp256k1-sys/src/lib.rs
24 ↗	(On Diff #50292)	It's a bit puzzling to me too, but I think it's meant to inform that when `#[cfg(secp256k1_fuzz)]` is on, all crypto is replaced by dummy crypto. Perhaps if someone enables it by accident, at least they get an "unsed const" warning