Commit Graph

517 Commits

Author SHA1 Message Date
Andrew Poelstra 191184539c
Merge rust-bitcoin/rust-secp256k1#511: Remove deprecated code
8c7c5e7394 Remove deprecated code (Tobin C. Harding)
e779e5dc05 doc: Use add_tweak in example code (Tobin C. Harding)
eedbd0b7e4 secp256k1-sys: Remove deprecated code (Tobin C. Harding)

Pull request description:

  Remove deprecated code from `secp256k1-sys` and `secp256k1`.

ACKs for top commit:
  apoelstra:
    ACK 8c7c5e7394

Tree-SHA512: 830d4459cf21fba98e75e1c099c96316c9db1c1fb87dd28343cea066544ac8568685ec9fc85969caee3d35014f64c3f42b5a5afbf4f4d16221a57a204e6a3524
2022-11-13 01:03:58 +00:00
Tobin C. Harding 8c7c5e7394 Remove deprecated code
We are currently on version 0.24.1 (i.e., next release is 0.25.0), we
can comfortably remove any code deprecated in 0.23.x or earlier.
2022-11-10 11:30:40 +11:00
Tobin C. Harding e779e5dc05 doc: Use add_tweak in example code
In preparation for removing the deprecated `tweak_add_check` use the new
version `add_tweak` instead.
2022-11-10 11:30:31 +11:00
Tobin C. Harding 5ccf0c8db7 Manually implement PartialEq, Eq, and Hash for PublicKey
`PartialEq` and `Eq` should agree with `PartialOrd` and `Ord` but we are
deriving `PartialEq`/`Eq` and doing a custom implementation of
`PartialOrd` and `Ord` (that calls down to ffi functions).

If two keys are equal their hashes should be equal so, we should add a
custom implementation of `Hash` also. In order to guarantee the digest
will be the same across library versions first serialize the key before
hashing it.

Add custom implementation of `PartialEq`, `Eq`, and `Hash` when not
fuzzing.

Please note, this is for the main `PublicKey` type, the patch does not
effect the `ffi::PublicKey`, nor do we call methods on the
`ffi::PublicKey`.
2022-11-09 06:31:23 +11:00
Andrew Poelstra 497654ea23
Merge rust-bitcoin/rust-secp256k1#504: Add array constants
603f441548 Add array constants (Tobin C. Harding)

Pull request description:

  In multiple places we use array constants for zero and one. Add two constants and use them throughout the codebase. Note the endian-ness of `ONE` in the docs.

ACKs for top commit:
  apoelstra:
    ACK 603f441548

Tree-SHA512: 70c455ee42f8a04feec37c3963b030c0f2c07b83801caf818dbb1661b7a0f65c4b92ff6a5df496a4dd6a917d13af4d60624a072c6f8a083293db9cd80d194232
2022-11-06 15:14:55 +00:00
Tobin C. Harding 603f441548 Add array constants
In multiple places we use array constants for zero and one. Add two
constants and use them throughout the codebase. Note the endian-ness of
`ONE` in the docs.
2022-11-06 05:21:16 +11:00
Andrew Poelstra dfb51e5d14
Merge rust-bitcoin/rust-secp256k1#497: Fix typo in public method
76a0804ca5 Fix typo in public method (Tobin C. Harding)

Pull request description:

  We have a method called `from_raw_signining_only`, I'm guessing this should be `from_raw_signing_only`.

ACKs for top commit:
  apoelstra:
    ACK 76a0804ca5

Tree-SHA512: ee03dbf3f69f0b348a483fa928fab9fba73bfdea383aee385a853b99998a882695c6839fff6433784ad097709ca31c67fc0a4e1a948caae7356be2eab7e332e5
2022-11-04 23:34:54 +00:00
Tobin C. Harding 76a0804ca5 Fix typo in public method
We have a method called `from_raw_signining_only`, I'm guessing this
should be `from_raw_signing_only`.
2022-11-01 15:25:50 +11:00
Tobin C. Harding 5417fad7cb Add method SecretKey::from_hashed_data
Analogous to the method on `Message`; add a constructor method on
`SecretKey` that hashes the input data.

While we are at it improve the rustdocs on `Message::from_hashed_data`
so docs on both methods are uniform.
2022-10-28 12:16:15 +11:00
elsirion 53c1354cc5
Fix broken `serde::Deserialize` and `FromStr` impl of `keyPair`
Fixes #491
2022-10-24 16:54:13 +02:00
Andrew Poelstra 0f29348b6c move some unsafe code inside an unsafe{} boundary
An internal function had a non-unsafe signature but could be called
with data that would cause it to exhibit UB. Move the unsafety inside
of the function so that the function signature now enforces soundness.

Fixes #481
2022-08-12 16:02:55 +00:00
Tobin C. Harding 6e98ec0475 Fix clippy warnings
Clippy default settings seemed to have changed introducing a few new
warnings.

warning: variable does not need to be mutable
warning: deref on an immutable reference
warning: returning the result of a `let` binding from a block

Fix them all in a single patch because CI has to pass for each patch.
2022-08-12 12:58:00 +10:00
Andrew Poelstra 71b47d1273
Merge rust-bitcoin/rust-secp256k1#473: Create configuration conditional "bench"
a431edb86a Create configuration conditional bench (Tobin C. Harding)
2a1c9ab4b8 Remove rand-std feature from unstable (Tobin C. Harding)
ddc108c117 Increase heading size (Tobin C. Harding)
596adff8ba Remove unneeded whitespace (Tobin C. Harding)

Pull request description:

  As we did in rust-bitcoin [0] create a configuration conditional `bench`
  that we can use to guard bench mark code. This has the benefit of
  making our features additive i.e., we can now test with `--all-features`
  with a stable toolchain (currently this fails because of our use of the
  `test` crate).

  Please note, this patch maintains the current behaviour of turning on
  the `recovery` and `rand-std` features when benching although I was
  unable to ascertain why this is needed.

  [0] - https://github.com/rust-bitcoin/rust-bitcoin/pull/1092

ACKs for top commit:
  sanket1729:
    ACK a431edb86a.
  apoelstra:
    ACK a431edb86a

Tree-SHA512: 913f5fbe0da08ec649081bf237c1d31cee58dacdac251d6030afabb99d455286c6d1dbdb6b2ac892b5d3c24584933254d1cfeec8e12f531cc420bd9d455a6531
2022-07-19 21:11:13 +00:00
Andrew Poelstra b01337cfb5 context: unconditionally disable auto-rerandomization on wasm
This causes panics. We can't add catch the panic, we can't change its output, we
can't detect if it'll happen, etc. Rather than dealing with confused bug reports
let's just drop this.

If users want to rerandomize their contexts they can do so manually.

There is probably a better solution to this but it is still under debate, even
upstream in the C library, what this should look like. Meanwhile we have bug
reports now.
2022-07-14 14:08:04 +00:00
Andrew Poelstra 748284633b apply `global-context-not-secure` logic to Secp256k1::new
Disable auto-rerandomization for both global and local contexts.
2022-07-14 14:06:41 +00:00
Tobin C. Harding a431edb86a Create configuration conditional bench
As we did in rust-bitcoin [0] create a configuration conditional `bench`
that we can use to guard bench mark code. This has the benefit of
making our features additive i.e., we can now test with `--all-features`
with a stable toolchain (currently this fails because of our use of the
`test` crate).

[0] - https://github.com/rust-bitcoin/rust-bitcoin/pull/1092
2022-07-14 09:35:23 +10:00
Tobin C. Harding 596adff8ba Remove unneeded whitespace
We do not customarily put two lines of whitespace before modules.

Remove unneeded whitespace from before the `benches` module.
2022-07-13 14:30:17 +10:00
Tobin C. Harding d2c97d43d8 Remove unnecessary instances of must_use
`Result` is already `must_use`, adding the compiler directive to
functions that return `Result` is unnecessary.
2022-07-11 07:56:47 +10:00
Tobin C. Harding 5f611f6f7f Conditionally compile the hex macro
We only use this macro when not fuzzing, add a cfg attribute to build it
in only when needed.
2022-06-29 11:11:39 +10:00
Andrew Poelstra 5f59820a8a
Merge rust-bitcoin/rust-secp256k1#465: Add must_use for mut self key manipulation methods
56f18430ff Add must_use for mut self key manipulation methods (Tobin C. Harding)
5b86e38aea Put compiler attributes below rustdocs (Tobin C. Harding)

Pull request description:

  We recently added a bunch of key tweaking methods that take `mut self`
  and return the tweaked/negated keys. These functions are pure and as
  such the returned result is expected to be used. To help downstream
  users use the API correctly add `must_use` attributes with a descriptive
  error string for each of the methods that takes `mut self`.

  Patch 1 is preparatory cleanup.

ACKs for top commit:
  apoelstra:
    ACK 56f18430ff

Tree-SHA512: 95ee63d5d0a34a9915551471d2f71de1963875eda04bf4217544076be0ed2836dcdee1875432dba5e02678556af86d7487e39daac6e928083807661430ddbcd6
2022-06-28 14:56:58 +00:00
Tobin C. Harding 0c15c01eb1 Use fuzzing not feature = "fuzzing"
Currently the following command fails

`RUSTFLAGS='--cfg=fuzzing' RUSTDOCFLAGS='--cfg=fuzzing' cargo test --all --all-features`

This is because `fuzzing` is not a feature, we should be using `fuzzing`
directly not `feature = "fuzzing"`.

I have no idea how this got past CI.
2022-06-28 13:30:33 +10:00
Tobin C. Harding 56f18430ff Add must_use for mut self key manipulation methods
We recently added a bunch of key tweaking methods that take `mut self`
and return the tweaked/negated keys. These functions are pure and as
such the returned result is expected to be used. To help downstream
users use the API correctly add `must_use` attributes with a descriptive
error string for each of the methods that takes `mut self`.
2022-06-28 13:18:57 +10:00
Andrew Poelstra 2b8297a468
Merge rust-bitcoin/rust-secp256k1#462: derive Hash for RecoverableSignature
e275166652 derive Hash for RecoverableSignature (NicolaLS)

Pull request description:

  It would be nice to also derive Hash for `RecoverableSignature` so data structures containing it don't have to implement it themself if they need to derive Hash

ACKs for top commit:
  apoelstra:
    ACK e275166652

Tree-SHA512: 418337e16e82a5e736c54d123450fdb164f4776db68952cf8095b36c501436446542821d554fa781dffa0f9067fc2464833a6c461897e655ff4449018da12ca2
2022-06-27 14:02:41 +00:00
NicolaLS e275166652 derive Hash for RecoverableSignature 2022-06-27 14:16:49 +02:00
Martin Habovstiak b18f5d0454 Removed `Default` from `SerializedSignature`
`Default` was pointless, so it was replaced with internal
`from_raw_parts` method which also checks the length.

This commit also documents changes to `SerializedSignature`.

Closes #454
2022-06-22 00:29:57 +02:00
Andrew Poelstra 7975be53cf
Merge rust-bitcoin/rust-secp256k1#456: `SerializedSignature` improvements
0e0fa06e41 Simplify `Display` impl of `SerializedSignature` (Martin Habovstiak)
5d51b9d94b Added `MAX_LEN` constant to `serialized_signature` (Martin Habovstiak)
e642a52e7d Add `#[inline]` to methods of `SerializedSignatre` (Martin Habovstiak)
e92540beb8 `impl IntoIterator for SerializedSignature` (Martin Habovstiak)
7f2d3d2452 Move `SerializedSignature` into its own module (Martin Habovstiak)
901d5ffeb9 `impl<'a> IntoIterator for &'a SerializedSignature` (Martin Habovstiak)
1d2a1c3fee Deduplicate `self.data[..self.len]` expressions (Martin Habovstiak)

Pull request description:

  This

  * Deduplicates slicing operations
  * Implements `IntoIterator` (owned and borrowed)
  * Reorganizes the code for better clarity
  * Adds `#[inline]`s
  * Checks length set by libsep256k1

  Closes #367
  Closes #368

  Individual commits are hopefully easier to review.

ACKs for top commit:
  apoelstra:
    ACK 0e0fa06e41

Tree-SHA512: bbc759af767c8b84bfd6720456efc1e86da501aa193641dae3c99847a3c882f7d4aa7e5cbec074fdd9c2595f1f65e5fbb4c80620539a6357927149e5c2fbc734
2022-06-21 20:46:42 +00:00
Martin Habovstiak 0e0fa06e41 Simplify `Display` impl of `SerializedSignature`
This is shorter and avoids duplication of slicing logic.
2022-06-21 21:14:14 +02:00
Martin Habovstiak 5d51b9d94b Added `MAX_LEN` constant to `serialized_signature`
This also asserts that libsecp256k1 set the correct length to help the
compiler elide bound checks.
2022-06-21 21:12:35 +02:00
Martin Habovstiak e642a52e7d Add `#[inline]` to methods of `SerializedSignatre`
These methods are trivial so great candidates for inlining.
2022-06-21 21:12:25 +02:00
Martin Habovstiak e92540beb8 `impl IntoIterator for SerializedSignature`
This adds owned iterator for `SerializedSignature` and implements
`IntoIterator`.
2022-06-21 21:12:22 +02:00
Martin Habovstiak 7f2d3d2452 Move `SerializedSignature` into its own module
This de-clutters the code and prepares for the next step of adding
`IntoIterator`. The type is still re-exported so the change is neither
breaking nor inconvenient.

This also adds more datialed explanation of `SerializedSignature` and
why it's needed.
2022-06-21 20:41:38 +02:00
Martin Habovstiak 901d5ffeb9 `impl<'a> IntoIterator for &'a SerializedSignature`
This allows using `&SerializedSignature` in `for` loops and methods like
`Iterator::zip`.
2022-06-21 19:26:43 +02:00
Martin Habovstiak 1d2a1c3fee Deduplicate `self.data[..self.len]` expressions
This removes the duplication ensuring single source of truth and making
the code simpler.
2022-06-21 19:23:01 +02:00
Martin Habovštiak e612458dc7
Remove mentions of 32-byte slice from tweak APIs
These methods accept `&Scalar`, not slice and `&Scalar` already guarantees 32-bytes, so this failure case is impossible.
2022-06-21 18:37:35 +02:00
Andrew Poelstra a1ac3fb311
Merge rust-bitcoin/rust-secp256k1#448: Add clippy to CI
65186e732a Add githooks (Tobin C. Harding)
6d76bd4a89 Add clippy to CI (Tobin C. Harding)
9f1ebb93cb Allow nonminimal_bool in unit test (Tobin C. Harding)
685444c342 Use "a".repeats() instead of manual implementation (Tobin C. Harding)
42de876e01 Allow let_and_return for feature guarded code (Tobin C. Harding)
d64132cd4b Allow missing_safety_doc (Tobin C. Harding)
2cb687fc69 Use to_le_bytes instead of mem::transmute (Tobin C. Harding)
c15b9d2699 Remove unneeded explicit reference (Tobin C. Harding)
35d59e7cc6 Remove explicit 'static lifetime (Tobin C. Harding)
1a582db160 Remove redundant import (Tobin C. Harding)

Pull request description:

  The first 8 patches clear clippy warnings. Next we add a CI job to run clippy. Finally we add a `githooks` directory that includes running clippy, also adds a section to the README on how to use the githooks. This is identical to the text in the [open PR](https://github.com/rust-bitcoin/rust-bitcoin/pull/1044) on `rust-bitcoin` that adds githooks _without_ yet adding clippy.

  **Note**: The new clippy CI job runs and is green :)

ACKs for top commit:
  Kixunil:
    ACK 65186e732a
  apoelstra:
    ACK 65186e732a

Tree-SHA512: f70a157896ce2a83af8cfc10f2fbacc8f68256ac96ef7dec4d190aa72324b568d2267418eb4fe99099aeda5486957c31070943d7c209973859b7b9290676ccd7
2022-06-17 17:12:12 +00:00
Tobin C. Harding 9f1ebb93cb Allow nonminimal_bool in unit test
We are explicitly testing various boolean statements, tell clippy to
allow less than minimal statements.
2022-06-17 10:17:21 +10:00
Tobin C. Harding 685444c342 Use "a".repeats() instead of manual implementation
Clippy emits:

  warning: manual implementation of `str::repeat` using iterators

As suggested, use `"a".repeats()`.
2022-06-17 10:17:21 +10:00
Tobin C. Harding 42de876e01 Allow let_and_return for feature guarded code
Clippy emits:

  warning: returning the result of a `let` binding from a block

This is due to feature guarded code, add 'allow' attribute. Use
`cfg_attr` to restrict the allow to when it is needed. Add the already
present `unused_mut` inside the `cfg_attr` guard also.
2022-06-17 10:17:21 +10:00
Tobin C. Harding d64132cd4b Allow missing_safety_doc
We have a whole bunch of unsafe code that calls down to the FFI layer.
It would be nice to have clippy running on CI, these safety docs
warnings are prohibiting that. Until we can add the docs add a compiler
attribute to allow the lint.
2022-06-17 10:17:21 +10:00
Tobin C. Harding 2cb687fc69 Use to_le_bytes instead of mem::transmute
Since we bumped the MSRV we have `to_le_bytes` available now, use it
instead of `mem::transmute`.

Remove code comment suggesting to do exactly this and clear clippy
warning.

While we are at it, use `cast` instead of `as`.
2022-06-17 10:16:00 +10:00
Tobin C. Harding c15b9d2699 Remove unneeded explicit reference
Clippy emits:

  warning: this expression creates a reference which is immediately
  dereferenced by the compiler

Remove the explicit reference.
2022-06-16 09:56:52 +10:00
Tobin C. Harding 35d59e7cc6 Remove explicit 'static lifetime
Clippy emits:

  warning: statics have by default a `'static` lifetime

Static strings no longer need an explicit lifetime, remove it.
2022-06-16 09:56:52 +10:00
Tobin C. Harding 1a582db160 Remove redundant import
Clippy emits:

  warning: this import is redundant

This is a remnant of edition 2015, now we have edition 2018 we no longer
need this import statement.
2022-06-16 09:54:00 +10:00
Tim Ruffing f419fe884b Fix getting parity from keypair in fuzzing
This also enables a test that was failung due to the parity bug.
2022-06-15 22:41:36 +02:00
Andrew Poelstra aba2663bc8
Merge rust-bitcoin/rust-secp256k1#449: Re-implement public key ordering using underlying FFI functions
13af51926a Make key comparison non-fuzzable (Dr Maxim Orlovsky)
739660499b Implement PublicKey ordering using FFI (Dr Maxim Orlovsky)
0faf404f0e Benchmark for key ordering (Dr Maxim Orlovsky)
999d165c68 FFI for pubkey comparison ops (Dr Maxim Orlovsky)

Pull request description:

  Re-base #309 for @dr-orlovsky on request by @Kixunil.

  To do the rebase I just had to change instances of cfg_attr to use `v0_5_0` instead of `v0_4_1` e.g.,
  ```
      #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_5_0_xonly_pubkey_cmp")]
  ```

  And drop the changes to `src/schnorrsig.rs`, all these changes are covered by the changes in `key.rs` I believe.

ACKs for top commit:
  Kixunil:
    ACK 13af51926a
  apoelstra:
    ACK 13af51926a

Tree-SHA512: 3054fcbc1707679f54466cdc91162c286394ad691e4f5c8ee18635a22b0854a4e60f1186ef3ca1532aacd8a637d0a153601ec203947e9e58dfcebf1bcb619955
2022-06-15 15:45:49 +00:00
Andrew Poelstra 4dacf55ed5
Merge rust-bitcoin/rust-secp256k1#435: Add functional style methods to various keys
12d4583638 Implement negate that consumes self (Tobin Harding)
5eb2d745b7 Rename tweak_add_assign -> add_tweak (Tobin Harding)
b9d08db8eb Replace _assign with _tweak (Tobin Harding)

Pull request description:

  The various `_assign` methods (`add_assign`, `add_expr_assign`, `mul_assign`, `tweak_add_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 the newly modified type.

  We notice also that this API is for adding/multiplying tweaks not arbitraryly adding keys.

  - Patch 1: Changes add/mul_assign -> add/mul_tweak for `PublicKey` and `SecretKey` (incl. re-working unit tests)
  - Patch 2: Changes `tweak_add_assign` -> `add_tweak` for `KeyPair` and `XOnlyPublicKey`
  - Patch 3: Changes `negate_assign` -> `negate`

  All methods changed include:
  - New method consumes self and returns the tweaked key
  - Original  method remains with a `deprecated` attribute, however I've left a TODO in there for adding the `since` field.

  Close: #415

ACKs for top commit:
  apoelstra:
    ACK 12d4583638

Tree-SHA512: 026e8722892f3a0f18956281e4d2356d2789ef535a7ab71a375758201b180663d068397cde2dca5f60858ab7158069e53d7096326bfbd5a364269b0be680940c
2022-06-15 15:39:30 +00:00
Andrew Poelstra 613d7dc1cb
Merge rust-bitcoin/rust-secp256k1#406: Use fixed width serde impls for keys
3ca7f499e0 Add fixed-width-serde integration tests (Tobin Harding)
bf9f556225 Add rustdocs describing fixed width serde (Tobin Harding)
c28808c5a4 Improve rustdocs for KeyPair (Tobin Harding)
6842383161 Use fixed width serde impls for keys (Tobin Harding)

Pull request description:

  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 [bincode.](https://docs.rs/bincode/latest/bincode/index.html). This extra data is unnecessary since we know in advance the length of these two types.

  We do not control the data output by serialization of our types because it depends on which crate is used to do the serialization. This PR improves the situation for serialization using the `bincode` crate, and this PR introduces mentions of `bincode` in the rustdocs, is this acceptable? See below for a table that describes binary serialization by other crates.

  Implement a sequence based visitor that encodes the keys as fixed width data for:

  - `SecretKey`
  - `PublicKey`
  - `KeyPair`
  - `XOnlyPublicKey`

  Fixes: #295

  **Question**: PR only does keys, do we want to do signatures as well?

ACKs for top commit:
  apoelstra:
    ACK 3ca7f499e0

Tree-SHA512: 77babce74fa9f0981bb3b869c4e77a68a4d1ec28d22d2c3be4305e27ef01d4828dac210e20b968cbbe5de8a0563cd985d7969bccf75cfe627a34a116fed1a5df
2022-06-15 15:21:28 +00:00
Andrew Poelstra 73ad30dda1
Merge rust-bitcoin/rust-secp256k1#409: After MSRV bump: Implemented `TryFrom<{u8, i32}>` for `Parity`
df081bede0 Changed impl `Error::cause()` to `Error::source()` (Martin Habovstiak)
cabb8f9e6f Implemented `TryFrom<{u8, i32}>` for `Parity` (Martin Habovstiak)

Pull request description:

ACKs for top commit:
  tcharding:
    ACK df081bede0
  apoelstra:
    ACK df081bede0

Tree-SHA512: 5587a9684a4bfc1f659548a3787ff3602c56b2d6db72e89783529b85b670b681bce7e99683c751a03697c8faa1c2aa2f314c2c9b28e16e4c53fd07b01e949af2
2022-06-15 15:09:25 +00:00
Dr Maxim Orlovsky 13af51926a Make key comparison non-fuzzable
Feature guard the custom implementations of `Ord` and `PartialOrd` on
`cfg(not(fuzzing))`. When fuzzing, auto-derive implementations.

Co-authored-by: Tobin C. Harding <me@tobin.cc>
2022-06-15 08:35:50 +10:00
Dr Maxim Orlovsky 739660499b Implement PublicKey ordering using FFI
Instead of selializing the key we can call down to the ffi layer to do
ordering.

Co-authored-by: Tobin C. Harding <me@tobin.cc>
2022-06-15 08:34:05 +10:00