Commit Graph

1013 Commits

Author SHA1 Message Date
Tobin Harding 12d4583638 Implement negate that consumes self
The method `negate_assign` (on pub/sec key) is cumbersome to use because
a local variable that uses these methods changes meaning but keeps the
same identifier. It would be more useful if we had methods that consumed
`self` and returned a new key.

Add method `negate` that consumes self and returns the negated key.
Deprecated the `negate_assign` methods.
2022-06-15 08:13:45 +10:00
Tobin Harding 5eb2d745b7 Rename tweak_add_assign -> add_tweak
We now have a method `add_tweak` on the `SecretKey` and `PublicKey`. We
can add similar methods that consumes self and return the tweaked key
for the `KeyPair` and `XOnlyPublicKey` types.

The justification for doing so is that a local variable that calls
`tweak_add_assign` changes in meaning but the identifier remains the
same, this leads to cumbersome renaming of the local variable.

The tweaking done to the `KeyPair` is actually done via the xonly public
key not the public key. To reflect this call the method
`add_xonly_tweak`, this is similar to how it is named in secp
`secp256k1_keypair_xonly_tweak_add`.
2022-06-15 08:13:42 +10:00
Dr Maxim Orlovsky 0faf404f0e Benchmark for key ordering 2022-06-14 11:46:47 +10:00
Dr Maxim Orlovsky 999d165c68 FFI for pubkey comparison ops 2022-06-14 11:18:11 +10:00
Tobin Harding b9d08db8eb Replace _assign with _tweak
The key methods `add_assign`, `add_expr_assign`, and `mul_assign` are
cumbersome to use because a local variable that uses these methods
changes meaning but keeps the same identifier. It would be more useful
if we had methods that consumed `self` and returned a new key.

Observe also that these to methods are for adding/multiplying a key by a
tweak, rename the methods appropriately.

Add methods `add_tweak`, `add_expr_tweak`, and `mul_tweak` to the
`SecretKey` and `PublicKey` type. Deprecate `add_assign`,
`add_expr_assign`, and `mul_assign`.
2022-06-14 09:46:17 +10:00
Tobin C. Harding 946cd83106 Improve Error display
The current display code for `Error` is a little unusual. We typically
just implement `Display` and if a `str` is needed use `format!`.

Improve the `Error` type by doing

- Remove the `as_str` function and implement `Display` directly.
- Remove the 'secp: ' prefix of all the error messages.
- Use a newly defined macro `write_err` that writes the error if `std`
  feature is not enabled so that no-std builds do not loose error info.

Note: The `write_err` macro is currently being introduced in
`rust-bitcoin` also. Elect to just duplicate it here and not share it
between the crates.
2022-06-14 09:33:48 +10:00
Martin Habovstiak df081bede0 Changed impl `Error::cause()` to `Error::source()`
This implements `source()` method instead of `cause()` allowing
downcasting.
2022-06-10 13:36:15 +02:00
Martin Habovstiak cabb8f9e6f Implemented `TryFrom<{u8, i32}>` for `Parity` 2022-06-10 13:35:42 +02:00
Andrew Poelstra aab77b16c2
Merge rust-bitcoin/rust-secp256k1#445: Add `Scalar` newtype and use it in tweaking APIs
5a0332463d Add `Scalar` newtype and use it in tweaking APIs (Martin Habovstiak)

Pull request description:

  This adds `Scalar` newtype to better represent values accepted by
  tweaking functions. This type is always 32-bytes and guarantees being
  within curve order.

  This is an initial iteration. Things I'm not 100% sure of, would appreciate help:

  * Is it actually required for scalar to be within curve order?
  * Are non-constant-time operations actually an issue?

  Alterntive tweaking API designs:

  * Accept `Into<Scalar>` generic argument, so people can pass `SecretKey`, may slightly worsen performance
  * `unsafe`-ly implement `AsRef<Scalar> for SecretKey` (casting raw pointers) and accept that

  Possible extension: API to convert from slices so that the user doesn't need two steps (convert to array first).

ACKs for top commit:
  apoelstra:
    ACK 5a0332463d

Tree-SHA512: cf639c91f0c6f6e0178d8a6d4125fd041708a59661fbd2f1924881001011e4cfe2b998148ae652995cdc9a3791c6644fcd5f004e059d76d9182a5e409b791446
2022-06-09 13:25:56 +00:00
Martin Habovstiak 5a0332463d Add `Scalar` newtype and use it in tweaking APIs
This adds `Scalar` newtype to better represent values accepted by
tweaking functions. This type is always 32-bytes and guarantees being
within curve order.
2022-06-09 15:08:19 +02:00
Tobin Harding 3ca7f499e0 Add fixed-width-serde integration tests
Add a `tests` directory. Add `serde` tests for the recently added fixed
width binary serialization code.

Please note, serialization is only fixed width when serialized with
the `bincode` crate.
2022-06-09 16:17:11 +10:00
Tobin Harding bf9f556225 Add rustdocs describing fixed width serde
We recently added fixed width serialization for some types however
serialization is only fixed width when data is serialized with the
`bincode` crate.

Add rustdocs describing fixed width serde to `SecretKey`, `PublicKey`,
and `XOnlyPublicKey` (`KeyPair` is already done).
2022-06-09 16:17:10 +10:00
Tobin Harding c28808c5a4 Improve rustdocs for KeyPair
Currently the rustdocs for `KeyPair` are stale in regards to serde, we
_do_ implement `Serialize` and `Deserialize` for `KeyPair`.

Improve the rustdocs for `KeyPair` by removing stale docs and adding
docs on fixed width binary serialization.
2022-06-09 16:17:05 +10:00
Tobin Harding 6842383161 Use fixed width serde impls for keys
Currently we serialize keys using the `BytesVisitor`, this causes the
serialized data to contain additional metadata encoding the length (an
extra 8 bytes) when serialized with the `bincode` crate. This extra data
is unnecessary since we know in advance the length of these types.

It would be useful for users of the lib to be able to get a fixed width
binary serialization, this can be done but it depends on the crate used
to do the serialization. We elect to optimise for `bincode` and add docs
noting that other binary serialization crates may differ (rustdocs added
in separate patches).

Implement a tuple based visitor that encodes the keys as fixed width
data.

Do fixed width serde implementations for:

- `SecretKey`
- `PublicKey`
- `KeyPair`
- `XOnlyPublicKey`
2022-06-09 16:09:31 +10:00
Andrew Poelstra 4f7f138797
Merge rust-bitcoin/rust-secp256k1#331: Update the code to edition 2018, and update dependencies
5d2f1ceb64 Fix WASM build (Elichai Turkel)
39aaac6834 Use new trait TryFrom and do small refactoring (Elichai Turkel)
7d3a149ca5 Move more things from the std feature to the alloc feature (Elichai Turkel)
bc8c713631 Replace c_void with core::ffi::c_void (Elichai Turkel)
26a52bc8c8 Update secp256k1-sys to edition 2018 and fix imports (Elichai Turkel)
ebe46a4d4e Update rand to 0.8 and replace CounterRng with mock::StepRng (Elichai Turkel)
626835f540 Update secp256k1 to edition 2018 and fix imports (Elichai Turkel)
67c0922a46 Update MSRV in CI and Readme from 1.29 to 1.41 (Elichai Turkel)

Pull request description:

  As proposed in https://github.com/rust-bitcoin/rust-bitcoin/issues/510#issuecomment-881686342 this PR raises the MSRV to 1.41.1 it also changes the code to be Edition 2018.

  The PR contains a few things:
  * Moving to edition 2018 and fixing the imports
  * Sorting and combining imports to make them more concise
  * Replacing our c_void with `core::ffi::c_void`
  * Bumping the `rand` version to latest and modifying our `RngCore` implementations accordingly
  * Doing some small refactoring and using the new `TryInto` trait where it makes the code nicer

  If people prefer I can split this PR into multiple and/or drop some commits

ACKs for top commit:
  tcharding:
    ACK 5d2f1ceb64
  apoelstra:
    ACK 5d2f1ceb64

Tree-SHA512: 5bf84e7ebb6286d59f8cada0bb712c46336f0dd6c35b67e6f4ba323b5484ad925b99b73e778ae4608f123938e7ee8705a0aec576cd9c065072c4ecf1248e3470
2022-06-08 20:53:41 +00:00
Elichai Turkel 5d2f1ceb64
Fix WASM build 2022-06-07 23:59:44 +03:00
Elichai Turkel 39aaac6834
Use new trait TryFrom and do small refactoring 2022-06-07 23:59:43 +03:00
Elichai Turkel 7d3a149ca5
Move more things from the std feature to the alloc feature 2022-06-07 23:59:42 +03:00
Elichai Turkel bc8c713631
Replace c_void with core::ffi::c_void 2022-06-07 23:59:41 +03:00
Elichai Turkel 26a52bc8c8
Update secp256k1-sys to edition 2018 and fix imports 2022-06-07 23:59:40 +03:00
Elichai Turkel ebe46a4d4e
Update rand to 0.8 and replace CounterRng with mock::StepRng 2022-06-07 23:59:40 +03:00
Elichai Turkel 626835f540
Update secp256k1 to edition 2018 and fix imports 2022-06-07 23:59:25 +03:00
Andrew Poelstra 33f76e4c5c
Merge rust-bitcoin/rust-secp256k1#440: Add secp256k1_schnorrsig_sign_custom to sys crate
0b27bde60b Bump secp256k1-sys minor version (Tibo-lg)
4beebd168e Add secp256k1_schnorrsig_sign_custom to sys crate (Tibo-lg)

Pull request description:

  Trying to update to the latest version I noticed that I lost the ability to pass a nonce to the schnorr signing function. This PR restore this ability by adding `secp256k1_schnorrsig_sign_custom` to the sys crate.

  I hid the struct members of `SchnorrSigExtraParams` and added a `new` method to make sure that the `magic` field is properly initialized, I think that make the most sense but happy to hear other opinions.

ACKs for top commit:
  apoelstra:
    ACK 0b27bde60b

Tree-SHA512: 7181eddb5815ca1a5ae1044f2a6fd8a214f8df9c45352e5f2ab6607f7e0d819cb8856fc2d6596b9d740b859df91d559595e7912332e292079c9ac1d27ec5c00b
2022-05-10 18:11:01 +00:00
Andrew Poelstra 6599e24010
Merge rust-bitcoin/rust-secp256k1#441: Derive Hash for Signature
ef7f1972a7 Derive Hash for Signature (Tobin C. Harding)

Pull request description:

  In preparation for deriving `Hash` in miniscript, derive `Hash` on the `ecdsa::Signature`.

  ref: https://github.com/rust-bitcoin/rust-miniscript/issues/226

ACKs for top commit:
  apoelstra:
    ACK ef7f1972a7
  elichai:
    ACK ef7f1972a7

Tree-SHA512: 7313f59971444ae18611adbafe86a09478eddd7357f2b7f3ad3bb1761609b6358b156975086f6c318eb2777018b7b2f44386321108939acbcf2d0a522e7e208e
2022-05-09 23:44:17 +00:00
Tibo-lg 0b27bde60b Bump secp256k1-sys minor version 2022-05-07 20:32:08 +09:00
Andrew Poelstra 712a378a7c
Merge rust-bitcoin/rust-secp256k1#442: Fix depreciation warning typos
997b4b35a9 Fix depreciation warning typos (Tibo-lg)

Pull request description:

  It got me confused for a second, might save a few minutes to the next person :).

ACKs for top commit:
  apoelstra:
    ACK 997b4b35a9

Tree-SHA512: 313409bec0841c1dae6dd54511fad5ecf3ffbc48ea8df18d0916a299f60f38b7fd204f7d8720a89539ad8fc329bbea5f6eabf573278f773a0b982f72af31acf2
2022-05-06 14:16:26 +00:00
Elichai Turkel 67c0922a46
Update MSRV in CI and Readme from 1.29 to 1.41 2022-05-06 12:19:17 +03:00
Tibo-lg 997b4b35a9 Fix depreciation warning typos 2022-05-06 16:12:10 +09:00
Tobin C. Harding ef7f1972a7 Derive Hash for Signature
In preparation for deriving `Hash` in miniscript, derive `Hash` on the
`ecdsa::Signature`.
2022-05-06 13:35:23 +10:00
Tibo-lg 4beebd168e Add secp256k1_schnorrsig_sign_custom to sys crate 2022-05-06 12:16:53 +09:00
Andrew Poelstra a0064c268b
Merge rust-bitcoin/rust-secp256k1#439: release minor version of secp-sys with WASM fix
3ed7fb044c release minor version of secp-sys with WASM fix (Andrew Poelstra)

Pull request description:

  Would like to get this quickly merged and published before we merge the breaking edition stuff.

ACKs for top commit:
  tcharding:
    ACK 3ed7fb044c
  elichai:
    ACK 3ed7fb044c

Tree-SHA512: 02bade244b3e59ab438432ae9a01e75a41052c6d714d6485caeb88cfa3d0e3b0f3f970d339ceb24c2c4eb8f2d8ae4be2339272ab72a3115148ad0943c853efea
2022-05-04 20:22:49 +00:00
Andrew Poelstra a30e9bb9ff
Merge rust-bitcoin/rust-secp256k1#430: Add convenience methods for keys
f08276adfc Add convenience methods for keys (Tobin Harding)
b4c7fa0d4e Let the compiler work out int size (Tobin Harding)
c612130864 Borrow secret key (Tobin Harding)

Pull request description:

  We have a bunch of `from_<key>` methods for converting between key types. To make the API more ergonomic to use we can add methods that do the same but called on a variable e.g., once applied the following are equivalent:

  - `let pk = PublicKey::from_keypair(kp)`
  - `let pk = kp.public_key()`

  Do this for `SecretKey`, `PublicKey`, `KeyPair`, and `XOnlyKeyPair`.

  Fixes: #428

  ### Note to reviewers

  - `XOnlyPublicKey` -> `PublicKey` logic is made up by me, I could not work out how to get `libsecp256k1` to do this.
  - Please review the tests carefully, they include assumptions based on my current understanding of the cryptography :)

ACKs for top commit:
  sanket1729:
    ACK f08276adfc. Thanks for going through all the iterations.
  apoelstra:
    ACK f08276adfc

Tree-SHA512: 1503a6e570a3958110c6f24cd6d075fe5694b3b32b91a7a9d332c63aa0806198ff10bdd95e7f9de0cf73cbf4e3655c6826bd04e5044d1b019f551471b187c8ea
2022-04-30 16:21:46 +00:00
Andrew Poelstra 3ed7fb044c release minor version of secp-sys with WASM fix 2022-04-30 16:14:52 +00:00
Andrew Poelstra f02ff65102
Merge rust-bitcoin/rust-secp256k1#437: Safety docs
dc1e377d4e Improve docs on rustsecp256k1_v0_4_1_context_create (Tobin C. Harding)
ad153d82f7 Add safety rustdoc headings (Tobin C. Harding)

Pull request description:

  Do some minor docs improvements.

  Done in an effort to clear _all_ clippy warnings from the codebase.

ACKs for top commit:
  sanket1729:
    ACK dc1e377d4e
  apoelstra:
    ACK dc1e377d4e

Tree-SHA512: 9b7e3ff05a6ea05bde5df49bbfcc6eb2c7097b4d9b600dedeb8fef4e3cbdd56a357fd0846ca51cb32f0957bca1335ff3b4b0d771046481ab95447af99d6a2d9c
2022-04-30 16:11:52 +00:00
Tobin C. Harding dc1e377d4e Improve docs on rustsecp256k1_v0_4_1_context_create
In preparation for [someone] adding a `# Safety` section to this
function, clean up the docs.
2022-04-27 10:23:42 +10:00
Tobin C. Harding ad153d82f7 Add safety rustdoc headings
Clippy warns about unsafe code without a `# Safety` section. A bunch of
these warnings are for functions that do actually have safety docs.

Follow rustdoc convention and add a `# Safety` section for the already
existing explanations.
2022-04-27 10:23:42 +10:00
Andrew Poelstra 37f4f005d1
Merge rust-bitcoin/rust-secp256k1#429: Misc doc fixes
676a9800df Remove unnecessary panic message (sanket1729)
aa50cc6ced Remove Schnorr word from keypairs (sanket1729)

Pull request description:

  Keypairs are pair of EC points that don't have anything to do with the
  signature algorithm

ACKs for top commit:
  apoelstra:
    ACK 676a9800df
  tcharding:
    ACK 676a9800df

Tree-SHA512: ed3e6f5e821d18641234b308b130271dcd2ec0dd6519a0e9d91564ab8e902b82180d7df377f2bcf08cd3ca1df7ce775422e4a3c386637eaff348e58b033de3ea
2022-04-22 16:55:33 +00:00
Tobin Harding f08276adfc Add convenience methods for keys
We have a bunch of `from_<key>` methods for converting between key types.
To improve the API and make it more ergonomic to use we can add methods
that do the same but can be called on the initial key instead of on the
resulting key's type. E.g. once applied the following are equivalent:

- `let pk = PublicKey::from_keypair(kp)`
- `let pk = kp.public_key()`

Do this for `SecretKey`, `PublicKey`, `KeyPair`, and `XOnlyKeyPair`.
2022-04-04 12:58:46 +10:00
Tobin Harding b4c7fa0d4e Let the compiler work out int size
We have two places in the code where we pass a mutable parity integer
to ffi code. At one callsite we tell the compiler explicitly what type
it is (`::secp256k1_sys::types::c_int`) and at the other call site we
let the compiler figure out the type.

Is one way better than the other? I don't know. But letting the compiler
figure it out seems to make the code easier to read.
2022-04-04 12:50:52 +10:00
Tobin Harding c612130864 Borrow secret key
`SecretKey` implements `Copy` and it is fine to take owneship of it; we
have multiple methods called `from_secret_key` and they all borrow the
secret key parameter. Favour consistency over perfection.

Borrow secret key parameter as is done in other `from_secret_key`
methods.
2022-04-04 12:50:52 +10:00
Andrew Poelstra e4fb575590
Merge rust-bitcoin/rust-secp256k1#422: Fix test script silent failure
97dc0ea9ac Run correct clang --version (Tobin Harding)
a3582ff77d test.sh: Use set -e to exit on failure (Tobin Harding)
7bec31c3a6 test.sh: explicitly return 0 (Tobin Harding)

Pull request description:

  Change the test script to exit with non-zero status code if any command fails.

  The `test.sh` script is silently failing, that means changes causing failures are slipping through our CI pipeline and being merged.

  Resolves: #419

  ## Note

  Just the last 3 patches, the first 6 are from #420. re-base just shows it works on top of 420, it is going to have to be rebased again when 420 merges.

ACKs for top commit:
  apoelstra:
    ACK 97dc0ea9ac

Tree-SHA512: b86a6876d8c45a2b90b7b3c8adbc08ad6f49b430b1cfaec31cd2de8441cb96af39c63da02b98d6ed71dfab045d466d71d3757297886b5e44ebb6cbaeb4ed32dd
2022-04-01 17:12:49 +00:00
Andrew Poelstra 8caf41d55c
Merge rust-bitcoin/rust-secp256k1#420: Improve CI script
58db1b6753 Run WASM for multiple toolchains (Tobin Harding)
946ac3b51e Do docs build in Nightly job (Tobin Harding)
f7bc7d3728 Install clang to run adress sanitizer (Tobin Harding)
96685c571d Remove unnecessary matrix (Tobin Harding)
a8a679ed7d Re-name nightly CI job to Nightly (Tobin Harding)
9c9d622b0e Remove trailing whitespace (Tobin Harding)

Pull request description:

  Improve the CI pipeline. Done while investigating #419. This PR now only includes the GitHub actions changes, I'm moving the `test.sh` changes to a https://github.com/rust-bitcoin/rust-secp256k1/pull/422 because they cannot be merged yet.

  (Please note, this PR has been heavily re-worked so discussion below may be confusing to reviewers new to the PR.)

ACKs for top commit:
  apoelstra:
    ACK 58db1b6753
  thomaseizinger:
    ACK 58db1b6753

Tree-SHA512: 5520cf7a7ea0ba701aeaf6b97150416192c0629df8b65545a20d8937a4d76bd323a0d7a875deccb7ce9adc4f3a423e6cd27b300682f206f79407f5ab4eaa5ddb
2022-04-01 17:07:08 +00:00
Tobin Harding 97dc0ea9ac Run correct clang --version
For the test that uses `clang-9` do the sanity call using `clang-9`
instead of `clang`.

For the test that uses `clang` add a sanity call to `clang --version`.
2022-03-31 15:51:59 +11:00
Tobin Harding a3582ff77d test.sh: Use set -e to exit on failure
Currently the `test.sh` script is silently failing because we do not
exit if a command fails. We can achieve this by using the Bash builtin
`set -e`.

For some reason I cannot explain a chain of commands that fails does not
fail the script. Instead of working out _why_ just remove the chain and
run each command on its own. This is functionally the same and, I hazard
a guess, is what the original author hoped to achieve with the chaining.
2022-03-31 15:51:59 +11:00
Tobin Harding 7bec31c3a6 test.sh: explicitly return 0
As per UNIX convention a Bash script should exit with status code 0 if
it completes successfully.
2022-03-31 15:51:59 +11:00
Andrew Poelstra f7cae46fc7
Merge rust-bitcoin/rust-secp256k1#421: Fix wasm build
bfd88dbd6c Move WASM const definitions to a source file (Tobin Harding)

Pull request description:

  Total re-write ... again :)

  Currently we are defining the WASM integer size and alignments in the `stdio.h` header file, this is wrong because this file is included in the build by way of `build.rs` as well as by upstream `libsecp256k1`.

  Move WASM integer definitions to a `C` source file and build the file  into the binary if target is WASM.

  Fixes the first part of #419 (#422 does the second part).

  ### Note to reviewers

  I'm not exactly sure why the directory `was-sysroot` is named as it is or if the name is significant to `cargo` , please review carefully the directory tree changes.

  ```
  cd secp256k1-sys
  tree wasm
  wasm
  ├── wasm.c
  └── wasm-sysroot
             ├── stdio.h
             ├── stdlib.h
             └── string.h
  ```

ACKs for top commit:
  thomaseizinger:
    ACK bfd88dbd6c
  apoelstra:
    ACK bfd88dbd6c

Tree-SHA512: ba822b764fb5f74dfd22cc797f7e3f70440dbaabfe34e0475c796e0e5d88f2086bedb00a1ec765cce91bde6bb45130b9abe5d9289317d6c20f692c6ed711969e
2022-03-30 16:37:55 +00:00
Andrew Poelstra 2ce67d9597
Merge rust-bitcoin/rust-secp256k1#432: Move panic test to top of script
d2e1f8cc95 Move panic test to top of script (Tobin Harding)

Pull request description:

  In `test.sh` we have a test that checks for a panic by greping the output of `cargo test --exact 'tests::test_panic_raw_ctx_should_terminate_abnormally'`. If we put this test at the top of the script right after we run `cargo test` we are guaranteed to not trigger a re-build.

  ### Note to reviewers

  I just noticed this patch somehow snuck into #420, all other changes in that PR are to `.github/workflows/rust.yml` so this change does not fit in there. Hence raising it as a separate PR.

ACKs for top commit:
  apoelstra:
    ACK d2e1f8cc95

Tree-SHA512: 90ad7a8762a6fd977345f347f0aa8b0979a7576585000b6d80624c0672b7de457dec471dc63b2e7fa4c3f52143d0f6fd1f4031a70f85c9fab4b7c22a787c438b
2022-03-30 16:18:45 +00:00
Tobin Harding bfd88dbd6c Move WASM const definitions to a source file
Currently we are defining the WASM integer size and alignments in the
`stdio.h` header file, this is wrong because this file is included in
the build by way of `build.rs` as well as by upstream `libsecp256k1`.

Move WASM integer definitions to a `C` source file and build the file
into the binary if target is WASM.
2022-03-30 10:22:37 +11:00
Tobin Harding 58db1b6753 Run WASM for multiple toolchains
WASM is supported by Rust 1.30. We can therefore run the WASM tests on
any all the toolchains except MSRV (1.29.0). This has benefit of
catching nightly/beta issues before they get to stable.

Done as a separate CI job since it is conceptually different to the
`Tests` job.

Run WASM for nightly, beta, and stable toolchains.
2022-03-30 08:48:46 +11:00
Tobin Harding 946ac3b51e Do docs build in Nightly job
We have a separate CI job for things that require a nightly toolchain.
Building the docs requires a nightly toolchain (because of `--cfg
docsrc` flag). It makes more sense to run the docs build in the
`Nightly` job instead of hidden in the `Tests` job.

Do the docs build in the `Nightly` job instead of in the `Tests` job.
2022-03-30 08:45:08 +11:00