Commit Graph

550 Commits

Author SHA1 Message Date
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
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
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
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 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 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 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 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 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
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 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
junderw f93ca81348
Add sign_ecdsa_with_noncedata and sign_ecdsa_recoverable_with_noncedata 2022-03-22 21:13:31 +09:00
sanket1729 676a9800df Remove unnecessary panic message
1) All types in rust should have the guarantee that well-formed data is
stored in SecretKey type. Therefore, IMO the panic message is
unnecessary.
2022-03-21 16:37:15 -07:00
sanket1729 aa50cc6ced Remove Schnorr word from keypairs
Keypairs are pair of EC points that don't have anything to do with the
signature algorithm
2022-03-21 16:36:58 -07:00
Tobin Harding de65fb2f1e
Implement de/serialization for SharedSecret
As we do for other keys implement serde de/serialization for the
`SharedSecret`. Includes implementation of `from_slice` method that is
the borrowed version of `from_bytes` as well as a `FromStr`
implementation that parses a hex string.
2022-03-12 07:46:21 +11:00
Dominik Spicher 9be8e74107 Allow SharedSecret to be created from byte array
This was accidentally removed in 8b2edad. See also the discussion
on https://github.com/rust-bitcoin/rust-secp256k1/pull/402
2022-03-10 22:38:25 +01:00
Andrew Poelstra f3d48a298e update "should terminate abnormally" test to trigger a different ARG_CHECK
We can no longer produce non-verification context objects, so instead produce
an invalid public key.
2022-03-08 19:45:43 +00:00
Andrew Poelstra 8294ea3f50 secp256k1-sys: update upstream library
Two API changes needed to be reflected: schnorrsig_sign and schnorrsig_verify.

Also bump both Cargo.toml files
2022-03-08 19:45:41 +00:00
Tobin Harding 7b91f9d8ef
Remove schnorrsig from test names
Recently we moved from using the identifier 'schnorrsig' to 'schnorr',
we omitted to update the tests.

While we are at it use more idiomatic Rust unit test names (i.e., do not
start test name with `test_` because it stutters when the name is read
in output of `cargo test`).
2022-03-04 14:28:29 +00:00
Tobin Harding 4b840ffe87
Remove schnorrsig from helper method
Recently we moved from using the identifier 'schnorrsig' to 'schnorr',
we omitted to update a helper function.
2022-03-04 14:28:28 +00:00
Tobin Harding 79770e17f3
Deprecate SCHNORRSIG_SIGNATURE_SIZE
Recently we moved from using the identifier 'schnorrsig' to 'schnorr',
we omitted to update the schnorr signature size constant.

Deprecate `SCHNORRSIG_SIGNATURE_SIZE` and add
`SCHONORR_SIGNATURE_SIZE`.
2022-03-04 14:28:24 +00:00
Tobin Harding 7a417fd1c5
Deprecate SCHNORRSIG_PUBLIC_KEY_SIZE
Recently we moved from using the identifier 'schnorrsig' to 'schnorr',
we omitted to update the schnorr public key size constant.

Deprecate `SCHNORRSIG_PUBLIC_KEY_SIZE` and add
`SCHONORR_PUBLIC_KEY_SIZE`.
2022-03-04 14:27:58 +00:00
Andrew Poelstra dc90a43e68
Merge rust-bitcoin/rust-secp256k1#403: `Parity` conversion and error handling cleanup
5acf6d23d3 `Parity` conversion and error handling cleanup (Martin Habovstiak)

Pull request description:

  This removes the deprecated `From` conversion, replaces it with
  `TryFrom`, and adds more convenience conversions. A new error type is
  created for the invalid parity error with conversion to catch-all
  `Error`.

  This is intended for an API-breaking version.

ACKs for top commit:
  apoelstra:
    ACK 5acf6d23d3

Tree-SHA512: 49b73fc90455c172012b46f36eafa7d256b940f4b431b4eedb577ab07d9402eae40af931e00b3c409bbe502dbcac064a742e874a5e8bedd8d0cbe92a468ae4f6
2022-03-01 13:01:15 +00:00
Martin Habovstiak 5acf6d23d3 `Parity` conversion and error handling cleanup
This removes the deprecated `From` conversion and adds a new error
type for the invalid parity error with a conversion to the catch-all
`Error`.
2022-02-28 20:59:51 +01:00
Tobin Harding cf6badf96a
Obfuscate SharedSecret when printing
Currently printing the `SharedSecret` using `Display` or `Debug` prints
the real secret, this is sub-optimal. We have a solution for other
secrets in the project where printing is obfuscated and we provide a
`display_secret` method for explicitly printing.

Mirror the logic for other secrets and obfuscate the `SharedSecret` when printing.
2022-02-28 07:22:17 +00:00
Tobin Harding e4be664d97
Improve rustdocs for displaying secrets
Improve rustdocs on `display_secret` by doing:

- Minor improvements to the rustdocs to aid readability in the editor.
- Do not guarantee (`assert_eq!`) debug output
2022-02-28 07:16:41 +00:00
Tobin Harding 5c7c76eb74
Rename serialize_secret -> secret_bytes
The `serialize_secret` method is a getter method, it does not do any
serialisation. However we use the method on secret keys and key types so
in order for the name to be uniform use the descriptive name
`secret_bytes`.

Rename `serialize_secret` to be `secret_bytes`.
2022-02-28 07:11:24 +00:00
Tobin Harding 4ded2c0478
Use byte instead of i
The identifier `i` is predominantly used for indexing an array but we
are using it as a place holder for the iterated value of an array that
is then printed. The identifier `byte` is more descriptive.

Done in preparation for adding similar code to the `ecdh` module.
2022-02-24 19:48:23 +00:00
Tobin Harding 91106f5685
Remove magic number
In array initialisation we use magic number 64, this is the secret bytes
length multiplied by 2.

Please note; we still use the magic number 32, left as such because it
is used in various ways and its not immediately clear that using a
single const would be any more descriptive.

Use `SECRET_KEY_SIZE * 2` instead of magic number 64.
2022-02-24 19:48:19 +00:00
Tobin Harding 6dca99631f
Mention bitcoin_hashes in obfuscated secret msg
Hashing the debug output for secrets can be done with `bitcoin_hashes`
not just `std`. Mention this in the obfuscated string output when
neither are available.
2022-02-24 19:48:15 +00:00
Andrew Poelstra 8b2edad041
Merge rust-bitcoin/rust-secp256k1#402: Limit SharedSecret to 32 byte buffer
5603d71ad3 Limit SharedSecret to 32 byte buffer (Tobin Harding)
d5eeb099ad Use more intuitive local var numbering (Tobin Harding)
834f63c26c Separate new_with_hash into public function (Tobin Harding)

Pull request description:

  Currently `SharedSecret` provides a way to get a shared secret using SHA256 _as well as_ a way to use a custom hash function to get the shared secret. Internally `SharedSecret` uses a 256 byte buffer, this is a tad wasteful. We would like to keep the current functionality but reduce memory usage.

  - Patch 1: Pulls the `new_with_hash` logic out into a standalone public function that just returns the 64 bytes representing the x,y co-ordinates of the computed shared secret point. Callers are then responsible for hashing this point to get the shared secret (idea by @Kixunil, thanks).
  - Patch 2: Does trivial refactor
  - Patch 3: Uses a 32 byte buffer internally for `SharedSecret`. This is basically a revert of the work @elichai did to add the custom hashing logic. @elichai please holla if you are not happy with me walking all over this code :)

  ### Note to reviewers

  Secret obfuscation is done on top of this in https://github.com/rust-bitcoin/rust-secp256k1/pull/396, they could be reviewed in order if this work is of interest to you.

ACKs for top commit:
  apoelstra:
    ACK 5603d71ad3

Tree-SHA512: 48982a4a6a700a111e4c1d5d21d62503d34f433d8cb303d11ff018d2f2be2467fa806107018db16b6d0fcc5ff1a0325dd5790c62c47831c7cd2141a1b6f9467d
2022-02-24 15:17:44 +00:00
Andrew Poelstra c7d6cdbaba
Merge rust-bitcoin/rust-secp256k1#401: Breaking: changed Parity serialization to u8
e6cb588a23 Breaking: changed `Parity` serialization to `u8` (Martin Habovstiak)

Pull request description:

  Serializing the value as `u8` is more compact but this is a breaking
  change.

  `Visitor` was renamed to avoid hungarian notation and maybe allow other
  integers in the future.

  For next major version, depends on #400

ACKs for top commit:
  tcharding:
    tACK e6cb588
  apoelstra:
    ACK e6cb588a23

Tree-SHA512: 1432a2f3c913c3a7eaec5228fd2dd4e8320d828128bec71812cbf56dd8950c969ed22c69867402eb9e820127868d29b291f3374c6e15de0a3ff2341420c4bbab
2022-02-24 15:16:21 +00:00
Tobin Harding 5603d71ad3
Limit SharedSecret to 32 byte buffer
The `SharedSecret` uses sha256 to hash the secret, this implies the
secret is 32 bytes of data.

Currently we use a buffer of 256 bytes, this is unnecessary.

Change the implementation of `SharedSecret` to use a 32 byte buffer.
2022-02-21 13:33:17 +00:00
Tobin Harding d5eeb099ad
Use more intuitive local var numbering
In test code we use multiple pub/sec keys. It is more intuitive if the
'secret 1' is generated by the owner of secret key 1.

Refactor only, no logic changes.
2022-02-21 13:11:30 +00:00
Tobin Harding 834f63c26c
Separate new_with_hash into public function
In preparation for simplifying the `SharedSecret` internals pull the
`new_with_hash` function logic out into a standalone public function
that provides similar functionality without use of the `SharedSecret`
struct. Function now returns the 64 bytes of data representing a shared
point on the curve, callers are expected to the hash these bytes to get
a shared secret.
2022-02-18 09:51:06 +00:00
Andrew Poelstra 2a25e5eae8 restore `global-context-less-secure` feature 2022-02-16 23:46:52 +00:00
Tobin Harding 4c9bab9f6e
Remove explicit mention of feature requirements
We are using `cfg_attr` to instruct the rustdocs build system to
highlight feature requirements for functions, there is no need to
explicitly mention feature requirements in the text.
2022-02-11 07:47:52 +00:00
Tobin Harding 806eaca5f1
Use feature std with rand-std
Recently we fixed a bunch of feature gates to use `rand-std` instead
of `rand` but in doing so did not notice that the same feature gates
were using `alloc` which is meaningless if `std` is enabled.

Feature gate on `std` if we are using `rand-std`.
2022-02-11 07:44:16 +00:00
Andrew Poelstra df7520e951
Merge rust-bitcoin/rust-secp256k1#340: Improve documentation
c73eb2f391 Use 'extra' instead of 'cheap' (Tobin Harding)
c79eb976ca Remove unnecessary explanation (Tobin Harding)
f95e91a6da Use isn't instead of shouldn't (Tobin Harding)
c9e6ca1680 Use rust-bitcoin module doc style (Tobin Harding)
3fa6762437 Add link to referenced commit (Tobin Harding)
f5e68f3ba7 Add ticks around code snippet (Tobin Harding)
d25431c1da Use 3rd person tense for function docs (Tobin Harding)
c3be285c1d Fix size constant docs (Tobin Harding)
5e07e7596b Add period to sentences (Tobin Harding)
269bde042f Remove unnecessary capitalisation (Tobin Harding)

Pull request description:

  In a continued effort to find my feet around here, and inspired by issue #128 I've done a codebase wide audit of the docs (primarily just rustdocs but I glanced at `//` docs as well). Each change is in a separate commit so can be removed if resistance is met. (_"resistance is futile"_).

  I've based the stylistic decisions on [work done](https://github.com/rust-bitcoin/rust-bitcoin/pull/704) in rust-bitcoin.

  I believe the only controversial change is the last (commit: da161c9 Use rust-bitcoin module doc style), please review that one carefully.

ACKs for top commit:
  apoelstra:
    ACK c73eb2f391

Tree-SHA512: 5ea215de3fd23ca2a4f25d8f8d59a85a299044fe495269c43b621291ea50c58856fa8544e36cc109b7bdb1a7a59bcab8711f30113572ddce4509d3b06ff0d3b6
2022-02-10 15:42:30 +00:00
Tobin Harding c73eb2f391
Use 'extra' instead of 'cheap'
The word 'extra' better describes the sidechannel resistance gained by
re-randomising the context.
2022-02-10 09:57:15 +00:00
Tobin Harding c79eb976ca
Remove unnecessary explanation
The nested pub inside a private module is easy to understand, we do not
need an explanation.
2022-02-10 09:55:41 +00:00
Tobin Harding f95e91a6da
Use isn't instead of shouldn't
This definitely isn't possible, change the phrase.
2022-02-10 09:54:35 +00:00
Martin Habovstiak e6cb588a23 Breaking: changed `Parity` serialization to `u8`
Serializing the value as `u8` is more compact but this is a breaking
change.

`Visitor` was renamed to avoid hungarian notation and maybe allow other
integers in the future.
2022-02-09 20:46:20 +01:00
Martin Habovstiak 662843e73b Improved error handling in `Parity` serde impl
* Fixes error message to be according to the trait documentation
* Uses `unexpected_value` to provide more information about the error
2022-02-09 20:42:12 +01:00
Andrew Poelstra 8bf29271de
Merge rust-bitcoin/rust-secp256k1#399: Clarified conversions between `Parity` and integers
705c9cfbc1 Clarified conversions between `Parity` and integers (Martin Habovstiak)

Pull request description:

  This was discussed in https://github.com/rust-bitcoin/rust-secp256k1/pull/390#issuecomment-1033018430

ACKs for top commit:
  apoelstra:
    ACK 705c9cfbc1

Tree-SHA512: 3ba2ec566099c3c6d1c6f830e4959312b818b8766d924e3d995e6b23bd196ab747cc03d46f494ef451569188b0163f53e3236cacd20bfae9118ee76bcdbc9c02
2022-02-09 18:36:19 +00:00
Andrew Poelstra f97e41ae21
Merge rust-bitcoin/rust-secp256k1#398: Implement LowerHex and Display for Message
a209836a99 Implement LowerHex and Display for Message (Tobin Harding)

Pull request description:

  Implement `fmt::LowerHex` for `Message`. Implement `Display` by calling `LowerHex`.

  Resolves: #251

ACKs for top commit:
  apoelstra:
    ACK a209836a99

Tree-SHA512: 64eeafc57ea2814108228d8427cd650076eb3cbb85ae14a7c5a6f39f5e20ca9b83b4ccc27c201668fd57a34fde0a37be4098aa5c602208a81a2018293b40b64d
2022-02-09 18:33:34 +00:00
Andrew Poelstra bc278fa351
Merge rust-bitcoin/rust-secp256k1#393: Add custom Debug impl for RecoverableSignature
4c43d5e20f Add custom Debug impl for RecoverableSignature (Tobin Harding)

Pull request description:

  Currently when debug printing the `RecoverableSignature` we do so byte by byte, this means that the output differs depending on the endianess of the machine. If instead we serialize the signature in compact form then the output is the same irrespective of the endianess.

  With this applied the following two commands now pass:

  ```
  cargo test test_debug_output --features=recovery
  ```
  ```
  cross test --target powerpc-unknown-linux-gnu test_debug_output --features=recovery
  ```

  Fixes: #375

ACKs for top commit:
  apoelstra:
    ACK 4c43d5e20f

Tree-SHA512: 073c2e0e23ce41a2b35f1b1193b07a755b726bf565d61e6bcb23b6bdaab31ba3591f31aa92230b07f7dfc018de0401eba09a6858dc261e66dacb331355f40d76
2022-02-09 18:31:33 +00:00
Andrew Poelstra b8615e2132
Merge rust-bitcoin/rust-secp256k1#397: Fix example dependency list
3c9dd2fb32 Fix example dependency list (Tobin Harding)

Pull request description:

  Example relies on `rand-std` not plain `rand` dependency.

  I do not understand why the following command passes without this patch
  applied

  ```
  cargo test --no-default-features --features=std,rand,bitcoin_hashes
  ```

  But if we put the same code in a standalone binary it fails as expected?

  Since the running of this test is _unusual_ and it is primarily meant as
  an entry point example to the library, remove the mention of "alloc"
  feature and just depend upon "std".

  Fixes: #395

ACKs for top commit:
  apoelstra:
    ACK 3c9dd2fb32

Tree-SHA512: 8e7ec7ac846e2916c29b74c7485650e5242ae1141c12c69b50d74efdfee71c11a52cd454231d2a7cdd6f8f683d3ba4369f9bf898a6b9351dc92c2a4e2bd626cd
2022-02-09 18:26:58 +00:00
Martin Habovstiak 705c9cfbc1 Clarified conversions between `Parity` and integers 2022-02-09 15:33:25 +01:00
Tobin Harding a209836a99
Implement LowerHex and Display for Message
Implement `fmt::LowerHex` for `Message`. Implement `Display` by calling
`LowerHex`.

Resolves: #251
2022-02-09 09:22:07 +00:00
Tobin Harding eb453b8227
Add global context API
Our API often involves a `Secp256k1` parameter, when users enable the
`global-context` feature they must then pass `SECP256K1` into these
functions. This is kind of clunky since the global is by definition
available everywhere.

Make the API more ergonomic for `global-context` builds by adding
various API functions/methods that use the global context implicitly.
2022-02-09 09:09:06 +00:00
Tobin Harding 3ecb5e41b3
Refactor from_secret_key definition
The `from_secret_key` method definition currently uses non-standard
indentation.

Improve uniformity by using 'standard' indentation.
2022-02-09 09:04:13 +00:00
Tobin Harding e2d47a29e2
Remove unnecessary import statement
The global context is already in scope in tests since we use a glob
import. No clue why Clippy does not warn for this.

Remove unnecessary import statement in test function.
2022-02-09 08:53:31 +00:00
Tobin Harding d79989bc95
Remove erroneous duplicate feature
When we removed the "global-context-less-secure" a duplicate feature
snuck in, remove it.
2022-02-09 08:53:31 +00:00