Details

Reviewers

Group Reviewers

Restricted Project

Commits

rABCbeb30ceabc8b: [ecash-secp256k1] Add `ecash-secp256k1`, use in `ecash-lib-wasm`

Summary

This is a follow-up to D16953, where we port the remaining code from rust-secp256k1.

The code again is largely kept unchanged, however we remove all the "ElligatorSwift" related code, as our secp256k1 library doesn't support this at all (and I also have no clue what that even is).

Other than that, is it mostly only reformatting everything with our formatting rules.

Also, we add support for our own Schnorr signatures (via "schnorrabc"), and add custom tests, partially inspired by the remainder of the library.

The tests are the same as for ecash-secp256k1-sys, we just introduce a lot more feature flags that are tested in a matrix. The result is that despite the library compiling in just a few seconds, the full CI now takes multiple minutes. This is fine though, because in return we get a similar level of stability as the original repository.

In the future, we can port more of the CI, e.g. building it on nightly, running the benchmarks, as well as the no_std tests.

Depends on D16953 and D16958.

Test Plan

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

Diff Detail

Repository

rABC Bitcoin ABC

Branch

ecash-secp256k1

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 30789
Build 61092: Build Diff	ecash-lib-integration-tests · ecash-agora-integration-tests · ecash-agora-tests · ecash-lib-tests · build-ecash-secp256k1
Build 61091: arc lint + arc unit

Event Timeline

tobias_ruck created this revision.Oct 22 2024, 16:46

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 22 2024, 16:46

Harbormaster completed remote builds in B30737: Diff 50283.Tue, Oct 22, 17:11

@bot build-ecash-secp256k1

teamcity awarded a token.Tue, Oct 22, 17:25

Build Bitcoin ABC Diffs / Diff Testing (build-ecash-secp256k1) failed.

Tail of the build log:

test key::test::create_pubkey_combine ... ok
test key::test::roundtrip_public_key_via_x_only_public_key ... ok
test key::test::roundtrip_x_only_public_key_via_public_key ... ok
test key::test::test_pubkey_from_bad_slice ... ok
test key::test::test_display_output ... ok
test ecdsa::recovery::tests::capabilities ... ok
test key::test::test_seckey_from_bad_slice ... ok
test key::test::convert_secret_key_to_public_key ... ok
test key::test::convert_secret_key_to_x_only_public_key ... ok
test key::test::roundtrip_keypair_via_secret_key ... ok
test key::test::test_out_of_range ... ok
test key::test::test_pubkey_serialize ... ok
test key::test::test_negation ... ok
test key::test::test_keypair_from_str ... ok
test key::test::test_serde_x_only_pubkey ... ok
test key::test::test_serde ... ok
test key::test::test_keypair_deserialize_serde ... ok
test key::test::roundtrip_secret_key_via_keypair ... ok
test key::test::tweak_add_zero ... ok
test key::test::test_serde_keypair ... ok
test key::test::tweak_mul_zero ... ok
test key::test::tweak_add_arbitrary_data ... ok
test schnorr::tests::test_pubkey_from_bad_slice ... ok
test key::test::tweak_mul_arbitrary_data ... ok
test schnorr::tests::test_pubkey_from_slice ... ok
test schnorr::tests::schnorr_sign ... ok
test schnorr::tests::test_pubkey_display_output ... ok
test schnorr::tests::test_pubkey_serialize ... ok
test schnorr::tests::test_pubkey_serialize_roundtrip ... ok
test schnorr::tests::test_serialize ... ok
test schnorr::tests::schnorr_verify ... ok
test schnorr::tests::test_serde ... ok
test schnorr::tests::test_xonly_key_extraction ... ok
test schnorrabc::tests::test_schnorrabc_verify ... ok
test schnorrabc::tests::test_schnorrabc_sign ... ok
test key::test::test_tweak_add_then_tweak_add_check ... ok
test schnorr::tests::bip340_test_vectors ... ok
test tests::capabilities ... ok
test tests::sign_and_verify_fail ... ok
test tests::signature_display ... ok
test tests::signature_lax_der ... ok
test tests::sign_and_verify_extreme ... ok
test tests::test_bad_slice ... ok
test tests::test_global_context ... ok
test schnorr::tests::schnor_sign_with_rng_verify ... ok
test schnorr::tests::schnorr_sign_no_aux_rand_verify ... ok
test schnorr::tests::schnorr_sign_verify ... ok
test schnorr::tests::schnorr_sign_with_aux_rand_verify ... ok
test tests::test_low_s ... ok
test tests::test_panic_raw_ctx_should_terminate_abnormally ... ignored
test tests::test_low_r ... ok
test tests::test_raw_ctx ... ignored
test tests::test_noncedata ... ok
test tests::test_serde ... ok
malloc(): corrupted top size
error: test failed, to rerun pass `--lib`

Caused by:
  process didn't exit successfully: `/work/target/debug/deps/ecash_secp256k1-80ae2dc932f112f0` (signal: 6, SIGABRT: process abort signal)
Build build-ecash-secp256k1 failed with exit code 101

disable test_preallocation

@bot build-ecash-secp256k1

teamcity updated a token.Tue, Oct 22, 17:40

Harbormaster completed remote builds in B30740: Diff 50286.Tue, Oct 22, 18:07

tobias_ruck retitled this revision from [ecash-lib] Add `ecash-secp256k1`, use in `ecash-lib-wasm` to [ecash-secp256k1] Add `ecash-secp256k1`, use in `ecash-lib-wasm`.Tue, Oct 22, 19:44

tobias_ruck edited the summary of this revision. (Show Details)

tobias_ruck edited the test plan for this revision. (Show Details)

rebase, clean somethings up

@bot build-ecash-secp256k1

teamcity updated a token.Tue, Oct 22, 19:52

rebase on D16953 again

tobias_ruck edited the summary of this revision. (Show Details)Tue, Oct 22, 20:09

tobias_ruck added a parent revision: D16953: [ecash-secp256k1] Add `ecash-secp256k1-sys`.

tobias_ruck published this revision for review.Tue, Oct 22, 20:13

tobias_ruck edited the summary of this revision. (Show Details)Tue, Oct 22, 20:16

tobias_ruck added a parent revision: D16958: [secp256k1] Backport "Schnorrsig API overhaul".

Harbormaster completed remote builds in B30744: Diff 50291.Tue, Oct 22, 20:19

Harbormaster completed remote builds in B30746: Diff 50293.Tue, Oct 22, 20:23

@bot build-ecash-secp256k1

teamcity updated a token.Tue, Oct 22, 20:25

Fabien requested changes to this revision.Wed, Oct 23, 09:55

Fabien added a subscriber: Fabien.

Fabien added inline comments.

modules/ecash-secp256k1/Cargo.toml
11 ↗	(On Diff #50293)	should homepage and repository be updated ?
modules/ecash-secp256k1/README.md
9 ↗	(On Diff #50293)	You can add in this file that it's a fork of https://github.com/rust-bitcoin/rust-secp256k1
modules/ecash-secp256k1/src/constants.rs
33 ↗	(On Diff #50293)	remove FYI I think it relates to bip324: https://bips.dev/324/
modules/ecash-secp256k1/src/ecdsa/mod.rs
155 ↗	(On Diff #50293)	Can we remove these deprecated functions already ?
270 ↗	(On Diff #50293)	What needs the unsafe here ? The nonce function ?
modules/ecash-secp256k1/src/key.rs
460 ↗	(On Diff #50293)	dito
modules/ecash-secp256k1/src/schnorr.rs
520 ↗	(On Diff #50293)	uncomment or remove

This revision now requires changes to proceed.Wed, Oct 23, 09:55

update URLs in Cargo.toml
update README
remove unused elligator const
remove unused deprecated functions, and undeprecate functions that clearly are useful
uncomment test

tobias_ruck added inline comments.Wed, Oct 23, 23:12

modules/ecash-secp256k1/src/ecdsa/mod.rs
270 ↗	(On Diff #50293)	ffi functions in Rust are generally unsafe Only when we're using cxx we get a safe interface, but otherwise it's unsafe

Harbormaster completed remote builds in B30788: Diff 50351.Wed, Oct 23, 23:27

Build Bitcoin ABC Diffs / Diff Testing (build-ecash-secp256k1) failed.

Tail of the build log:

    = help: maybe it is overwritten before being read?
    = note: `#[warn(unused_assignments)]` on by default

warning: `ecash-secp256k1` (lib) generated 1 warning
       Dirty serde_test v1.0.177: the rustflags changed
   Compiling serde_test v1.0.177
       Dirty bincode v1.3.3: the rustflags changed
   Compiling bincode v1.3.3
       Dirty serde_cbor v0.10.2: the rustflags changed
   Compiling serde_cbor v0.10.2
     Running `/root/.rustup/toolchains/1.76.0-x86_64-unknown-linux-gnu/bin/rustc --crate-name bincode /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bincode-1.3.3/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C debuginfo=2 -C metadata=1597d2471c042c26 -C extra-filename=-1597d2471c042c26 --out-dir /work/target/debug/deps -L dependency=/work/target/debug/deps --extern serde=/work/target/debug/deps/libserde-aba37e576cff5b45.rmeta --cap-lints allow --cfg=secp256k1_fuzz`
     Running `/root/.rustup/toolchains/1.76.0-x86_64-unknown-linux-gnu/bin/rustc --crate-name serde_test --edition=2021 /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde_test-1.0.177/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C debuginfo=2 -C metadata=43e91a8559d01af8 -C extra-filename=-43e91a8559d01af8 --out-dir /work/target/debug/deps -L dependency=/work/target/debug/deps --extern serde=/work/target/debug/deps/libserde-aba37e576cff5b45.rmeta --cap-lints allow --cfg=secp256k1_fuzz`
     Running `/root/.rustup/toolchains/1.76.0-x86_64-unknown-linux-gnu/bin/rustc --crate-name serde_cbor --edition=2018 /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde_cbor-0.10.2/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C debuginfo=2 --cfg 'feature="default"' --cfg 'feature="std"' -C metadata=4cd9ffa51045af72 -C extra-filename=-4cd9ffa51045af72 --out-dir /work/target/debug/deps -L dependency=/work/target/debug/deps --extern byteorder=/work/target/debug/deps/libbyteorder-c1ab3960e8485fad.rmeta --extern half=/work/target/debug/deps/libhalf-409eaafc980c278f.rmeta --extern serde=/work/target/debug/deps/libserde-aba37e576cff5b45.rmeta --cap-lints allow --cfg=secp256k1_fuzz`
     Running `/root/.rustup/toolchains/1.76.0-x86_64-unknown-linux-gnu/bin/rustc --crate-name ecash_secp256k1 --edition=2021 modules/ecash-secp256k1/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 --deny=unexpected_cfgs --test --cfg 'feature="alloc"' --cfg 'feature="default"' --cfg 'feature="std"' -C metadata=59fc2688d9592407 -C extra-filename=-59fc2688d9592407 --out-dir /work/target/debug/deps -C incremental=/work/target/debug/incremental -L dependency=/work/target/debug/deps --extern bincode=/work/target/debug/deps/libbincode-1597d2471c042c26.rlib --extern secp256k1_sys=/work/target/debug/deps/libecash_secp256k1_sys-fc32e15e0f8b34e1.rlib --extern hex_lit=/work/target/debug/deps/libhex_lit-007583bf5a3f32ae.rlib --extern rand_core=/work/target/debug/deps/librand_core-ca4df19ab44e3828.rlib --extern serde_cbor=/work/target/debug/deps/libserde_cbor-4cd9ffa51045af72.rlib --extern serde_test=/work/target/debug/deps/libserde_test-43e91a8559d01af8.rlib --cfg=secp256k1_fuzz -L native=/work/target/debug/build/ecash-secp256k1-sys-9dd08800e9c5ff8c/out`
     Running `/root/.rustup/toolchains/1.76.0-x86_64-unknown-linux-gnu/bin/rustc --crate-name serde --edition=2021 modules/ecash-secp256k1/tests/serde.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 --deny=unexpected_cfgs --test --cfg 'feature="alloc"' --cfg 'feature="default"' --cfg 'feature="std"' -C metadata=d0c6250847131c93 -C extra-filename=-d0c6250847131c93 --out-dir /work/target/debug/deps -C incremental=/work/target/debug/incremental -L dependency=/work/target/debug/deps --extern bincode=/work/target/debug/deps/libbincode-1597d2471c042c26.rlib --extern ecash_secp256k1=/work/target/debug/deps/libecash_secp256k1-18439e34a4f2c64a.rlib --extern secp256k1_sys=/work/target/debug/deps/libecash_secp256k1_sys-fc32e15e0f8b34e1.rlib --extern hex_lit=/work/target/debug/deps/libhex_lit-007583bf5a3f32ae.rlib --extern rand_core=/work/target/debug/deps/librand_core-ca4df19ab44e3828.rlib --extern serde_cbor=/work/target/debug/deps/libserde_cbor-4cd9ffa51045af72.rlib --extern serde_test=/work/target/debug/deps/libserde_test-43e91a8559d01af8.rlib --cfg=secp256k1_fuzz -L native=/work/target/debug/build/ecash-secp256k1-sys-9dd08800e9c5ff8c/out`
error[E0599]: no function or associated item named `from_byte_array` found for struct `key::PublicKey` in the current scope
    --> modules/ecash-secp256k1/src/key.rs:2004:29
     |
163  | pub struct PublicKey(ffi::PublicKey);
     | -------------------- function or associated item `from_byte_array` not found for this struct
...
2004 |         let pk = PublicKey::from_byte_array(&[
     |                             ^^^^^^^^^^^^^^^
     |                             |
     |                             function or associated item not found in `PublicKey`
     |                             help: there is an associated function with a similar name: `from_byte_array_compressed`
     |
note: if you're trying to build a new `key::PublicKey` consider using one of the following associated functions:
      key::PublicKey::from_secret_key
      key::PublicKey::from_slice
      key::PublicKey::from_byte_array_compressed
      key::PublicKey::from_byte_array_uncompressed
      and 3 others
    --> modules/ecash-secp256k1/src/key.rs:457:5
     |
457  | /     pub fn from_secret_key<C: Signing>(
458  | |         secp: &Secp256k1<C>,
459  | |         sk: &SecretKey,
460  | |     ) -> PublicKey {
     | |__________________^
...
486  |       pub fn from_slice(data: &[u8]) -> Result<PublicKey, Error> {
     |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
508  | /     pub fn from_byte_array_compressed(
509  | |         data: &[u8; constants::PUBLIC_KEY_SIZE],
510  | |     ) -> Result<PublicKey, Error> {
     | |_________________________________^
...
529  | /     pub fn from_byte_array_uncompressed(
530  | |         data: &[u8; constants::UNCOMPRESSED_PUBLIC_KEY_SIZE],
531  | |     ) -> Result<PublicKey, Error> {
     | |_________________________________^

For more information about this error, try `rustc --explain E0599`.
error: could not compile `ecash-secp256k1` (lib test) due to 1 previous error

Caused by:
  process didn't exit successfully: `/root/.rustup/toolchains/1.76.0-x86_64-unknown-linux-gnu/bin/rustc --crate-name ecash_secp256k1 --edition=2021 modules/ecash-secp256k1/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 --deny=unexpected_cfgs --test --cfg 'feature="alloc"' --cfg 'feature="default"' --cfg 'feature="std"' -C metadata=59fc2688d9592407 -C extra-filename=-59fc2688d9592407 --out-dir /work/target/debug/deps -C incremental=/work/target/debug/incremental -L dependency=/work/target/debug/deps --extern bincode=/work/target/debug/deps/libbincode-1597d2471c042c26.rlib --extern secp256k1_sys=/work/target/debug/deps/libecash_secp256k1_sys-fc32e15e0f8b34e1.rlib --extern hex_lit=/work/target/debug/deps/libhex_lit-007583bf5a3f32ae.rlib --extern rand_core=/work/target/debug/deps/librand_core-ca4df19ab44e3828.rlib --extern serde_cbor=/work/target/debug/deps/libserde_cbor-4cd9ffa51045af72.rlib --extern serde_test=/work/target/debug/deps/libserde_test-43e91a8559d01af8.rlib --cfg=secp256k1_fuzz -L native=/work/target/debug/build/ecash-secp256k1-sys-9dd08800e9c5ff8c/out` (exit status: 1)
Build build-ecash-secp256k1 failed with exit code 101

fix one usage of PublicKey

Harbormaster completed remote builds in B30789: Diff 50352.Wed, Oct 23, 23:53

Fabien requested changes to this revision.Thu, Oct 24, 07:18

Fabien added inline comments.

modules/ecash-secp256k1/contrib/test.sh
17	oups ?
modules/ecash-secp256k1/src/ecdsa/serialized_signature.rs
219	This doesn't look useful

This revision now requires changes to proceed.Thu, Oct 24, 07:18

uncomment secp256k1_sys test

fix is_empty

Fabien accepted this revision.Thu, Oct 24, 07:40

This revision is now accepted and ready to land.Thu, Oct 24, 07:40

Harbormaster completed remote builds in B30793: Diff 50359.Thu, Oct 24, 07:40

Harbormaster completed remote builds in B30794: Diff 50360.

Closed by commit rABCbeb30ceabc8b: [ecash-secp256k1] Add `ecash-secp256k1`, use in `ecash-lib-wasm` (authored by tobias_ruck). · Explain WhyThu, Oct 24, 08:23

This revision was automatically updated to reflect the committed changes.

tobias_ruck added a commit: rABCbeb30ceabc8b: [ecash-secp256k1] Add `ecash-secp256k1`, use in `ecash-lib-wasm`.

[ecash-secp256k1] Add `ecash-secp256k1`, use in `ecash-lib-wasm`
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 50352

Cargo.lock

Cargo.toml

modules/ecash-lib-wasm/Cargo.toml

modules/ecash-lib-wasm/src/ecc.rs

modules/ecash-secp256k1/.gitignore

modules/ecash-secp256k1/Cargo.toml

modules/ecash-secp256k1/LICENSE

modules/ecash-secp256k1/README.md

modules/ecash-secp256k1/contrib/test.sh

modules/ecash-secp256k1/src/constants.rs

modules/ecash-secp256k1/src/context.rs

modules/ecash-secp256k1/src/ecdh.rs

modules/ecash-secp256k1/src/ecdsa/mod.rs

modules/ecash-secp256k1/src/ecdsa/recovery.rs

modules/ecash-secp256k1/src/ecdsa/serialized_signature.rs

modules/ecash-secp256k1/src/key.rs

modules/ecash-secp256k1/src/lib.rs

modules/ecash-secp256k1/src/macros.rs

modules/ecash-secp256k1/src/scalar.rs

modules/ecash-secp256k1/src/schnorr.rs

modules/ecash-secp256k1/src/schnorrabc.rs

modules/ecash-secp256k1/src/secret.rs

modules/ecash-secp256k1/src/serde_util.rs

modules/ecash-secp256k1/tests/serde.rs

[ecash-secp256k1] Add `ecash-secp256k1`, use in `ecash-lib-wasm`ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 50352

Cargo.lock

Cargo.toml

modules/ecash-lib-wasm/Cargo.toml

modules/ecash-lib-wasm/src/ecc.rs

modules/ecash-secp256k1/.gitignore

modules/ecash-secp256k1/Cargo.toml

modules/ecash-secp256k1/LICENSE

modules/ecash-secp256k1/README.md

modules/ecash-secp256k1/contrib/test.sh

modules/ecash-secp256k1/src/constants.rs

modules/ecash-secp256k1/src/context.rs

modules/ecash-secp256k1/src/ecdh.rs

modules/ecash-secp256k1/src/ecdsa/mod.rs

modules/ecash-secp256k1/src/ecdsa/recovery.rs

modules/ecash-secp256k1/src/ecdsa/serialized_signature.rs

modules/ecash-secp256k1/src/key.rs

modules/ecash-secp256k1/src/lib.rs

modules/ecash-secp256k1/src/macros.rs

modules/ecash-secp256k1/src/scalar.rs

modules/ecash-secp256k1/src/schnorr.rs

modules/ecash-secp256k1/src/schnorrabc.rs

modules/ecash-secp256k1/src/secret.rs

modules/ecash-secp256k1/src/serde_util.rs

modules/ecash-secp256k1/tests/serde.rs

[ecash-secp256k1] Add `ecash-secp256k1`, use in `ecash-lib-wasm`
ClosedPublic
Actions

Revision Contents
Changeset List