314e8755df Clarify `global-context` feature (Martin Habovstiak)
d52ab85dd5 Added missing features to docs.rs config (Martin Habovstiak)
Pull request description:
Sadly, I missed two details in #353: features missing in docs.rs configuration and `global-context` being a bit confusing.
This PR fixes those, see commit messages for details.
ACKs for top commit:
apoelstra:
ACK 314e8755df
Tree-SHA512: 01bed8ae2f30adcbdd436b514f08a084492d7f4e1a739ca62e6d8b8547e379c01faeda3522733c27ab615acbb4c6cff60e13906cc88a0d2b90e439e7da517466
656f19407b Remove capital letter in middle of docs sentence (Tobin Harding)
Pull request description:
(Candidate for most trivial patch of all time.)
Seems to be a typo, change the 'L' to an 'l'.
ACKs for top commit:
real-or-random:
ACK 656f19407b
Tree-SHA512: 06a4712868c3195a8465b9cf7bd39e55a30e37574086ca27cb032e0109a8fe053411426a15bcb354642bf78e6420b6fa2789ca487c6cc499f741a11220d5dc22
69f44d9301 Manually implement Debug for SerializedSignature (Tobin Harding)
26921a31b8 Add lints to catch missing traits (Tobin Harding)
35556e22f2 Remove useless call to format (Tobin Harding)
0ad414a982 Remove unneeded return statements (Tobin Harding)
Pull request description:
We can use the linters to help us catch type definitions that are missing 'standard' derives. 'standard' is project defined to be
- Copy
- Clone
- Debug
- PartialEq and Eq
- PartialOrd and Ord
- Hash
(I've assumed this to be true based on the code and an open [PR](https://github.com/rust-bitcoin/rust-bitcoin/pull/587) in rust-bitcoin.)
While neither Rustc nor Clippy can find all of these, Rustc can warn for missing `Copy` and `Debug` implementations and these warnings can assist us find types that may need additional derives.
First two patches are trivial Clippy fixes in preparation for using the linter to improve type definitions crate wide.
Patch 3 adds
```
#![warn(missing_copy_implementations)]
#![warn(missing_debug_implementations)]
```
and fixes newly emitted warnings.
ACKs for top commit:
thomaseizinger:
ACK 69f44d9301
apoelstra:
ACK 69f44d9301
Tree-SHA512: 18f2c52d207f962ef7d6749a57a35e48eb18a18fac82d4df4ff3dce549b69661cb27f66c4cae516ae5477f5b919d9197f70a5c924955605c73f8545f430c3b42
Previously only `global-context-less-secure` was shown in the doc even
though `global-context` may also work. This was strictly correct because
`global-context` implies `global-context-less-secure` which is also
documented but people could miss it or forget about it and then worry
about security or worse, enable less secure feature.
Calling out both fetures seems useful, even important and thankfully
doesn't seem to cause too much noise in the docs.
Currently we have an implementation of `Debug` (also used by `Display`)
for `Signature` that first converts the sig to a `SerializedSignature`
then prints it as hex.
We would like to have an implementation of `Debug` for
`SerializedSignature`, this cannot be derived because of the `data: [u8;
field]`. We can manually implement `Debug` for `SerializedSignature`
exactly as it is currently done for `Signature` and call this new
implementation from `Signature::fmt()`.
This code path is already tested in `lib.rs` in the test function
`signature_display`.
Rustc can warn us when we forget to add `Copy` and `Deubg` trait
implementations to types.
Add lint directives to enable warnings for missing `Copy` and `Debug`
implementations. Use the newly emitted warnings to find types that do
not implement our 'standard' traits. These 'standard' traits are defined
as the set of attributes that it has been found beneficial to
opportunistically add to all types, these are
- Copy
- Clone
- Debug
- PartialEq and Eq
- PartialOrd and Ord
- Hash
1671dfc2ed Release 0.21.2 (sanket1729)
837be22e09 Basic derives for Parity (sanket1729)
7059192de9 Wildcard export from key module (sanket1729)
Pull request description:
Sorry for getting another point release. This time I have tested against this branch for rust-bitcoin https://github.com/rust-bitcoin/rust-bitcoin/pull/755. Hopefully, this is the last release.
Next release, we should have a Release Candidate for a couple of days before publishing a release.
ACKs for top commit:
apoelstra:
ACK 1671dfc2ed
Tree-SHA512: 263ad027da3da764bd76f719200382c47ba21a976caefc23ebef45d1c4be35ddfc80ce619b57326310aaab22bbf75ca7f1db80b45e95ec076584805efb791f3f
e595b39510 Re-export Parity struct (sanket1729)
Pull request description:
pub struct Parity is under a private module key and not re-exported in lib.rs . It is therefore not
possible to use it downstream.
ACKs for top commit:
elichai:
ACK e595b39510
apoelstra:
ACK e595b39510
Tree-SHA512: 2573689f9a08505c8dfe8f79cd921d5a2742a2a2f4f92cf4066fe6557c765c756531d13560fa4fe6461f094b0c11a52aca30b44542eb77eda7dd1ebd24d3b155
18f74d5242 Clarify what does "less security" mean (Martin Habovstiak)
94c55b4d09 Fixed typos/grammar mistakes (Martin Habovštiak)
1bf05523f0 Documented features (Martin Habovstiak)
Pull request description:
This documents the Cargo features making sure docs.rs shows warning for
feature-gated items. They are also explicitly spelled out in the crate
documentation.
The PR is similar in spirit to https://github.com/rust-bitcoin/rust-bitcoin/pull/633
ACKs for top commit:
apoelstra:
ACK 18f74d5242
Tree-SHA512: 8aac3fc5fd8ee887d6b13606d66b3d11ce44662afb92228c4f8da6169e3f70ac6a005b328f427a91d307f8d36d091dcf24bfe4d17dfc034d02b578258719a90a
c50411f798 release secp256k1-sys 0.4.2; make new `ZERO` type publically accessible (Andrew Poelstra)
Pull request description:
Exposes the new const object provided by #345
ACKs for top commit:
elichai:
ACK c50411f798
Tree-SHA512: 42fce191b68a88811c339ff267dafbb616e765108f5b2e70514b5153f64ef5152f5704982ddc0b20ece5ad15da23927e18f9c78af2763ef971c0e3b9bbf490a5
This documents the Cargo features making sure docs.rs shows warning for
feature-gated items. They are also explicitly spelled out in the crate
documentation.
fafc141782 Removed useless Makefile (Martin Habovstiak)
Pull request description:
This Makefile did nothing interesting and could confuse people.
ACKs for top commit:
apoelstra:
ACK fafc141782
Tree-SHA512: 00337677787f98c4c4f1014f5cb4205b5e4057eaa2a1d512f44280d9e7952219b8ef3804e64dca35cc19856bbe780069ce3ab072a082023c72143542aaaaacaa
ede114fb1a Improve docs on tweak_add_check method (Tobin Harding)
fbc64c7725 Add opaque parity type (Tobin Harding)
1b768b2749 Make tweak_add_assign return statements uniform (Tobin Harding)
edafb88f8c Move key unit tests to key module (Tobin Harding)
e3d21a3d87 Clean up test imports with key module (Tobin Harding)
Pull request description:
Two functions in the FFI secp code return and accept a parity integer.
Currently we are manually converting this to a bool. Doing so forces readers of the code to think what the bool means even though understanding this value is not needed since in is just passed back down to the FFI code.
We initially tried to solve this issue by adding an enum, discussion below refers to that version. Instead of an enum we can solve this issue by adding an opaque type that holds the parity value returned by the FFI function call and then just pass it back down to FFI code without devs needing to know what the value should be. This fully abstracts the value away and removes the boolean conversion code which must otherwise be read by each dev.
- Patch 1 and 2 improve unit tests that test the code path modified by this PR
- Patch 3 trivially changes code to be uniform between two similar methods (`tweak_add_assign`)
- Patch 4 is the meat and potatoes (main part of PR :)
- Patch 5 is docs improvements to code in the area of this PR
ACKs for top commit:
apoelstra:
ACK ede114fb1a
Tree-SHA512: 37843e066d9006c5daa30dece9f7eb7a802864b85606e43ed2651c6d55938c4f884cc4abab81eccb69685f6eda918a9b9ba57bf1a4efec41e89239b99ae2b726
It is not immediately apparent what 'err == 1' means, one must determine
that the FFI function call returns 1 for success. We can help readers of
the code by adding a 'Return' section to the method documentation.
Add trailing full stop to method docs initial line also.
Two functions in the FFI secp code return and accept a parity int.
Currently we are manually converting this to a bool. Doing so forces
readers of the code to think what the bool means even though
understanding this bool is not needed since in is just passed back down
to the FFI code. We can abstract this away by using an opaque type to
hold the original int and not converting it to a boolean value.
Add 'Return' and 'Error' sections to `tweak_add_assign` while fixing the
docs to describe the new opaque parity type.
We have two `tweak_add_assign` methods (one for keypair and one for
x-only pubkey). Both check the return value from a FFI function call.
We can make both sites uniform to _slightly_ reduce cognitive load when
reading the code.
Use C style code to make it obvious to readers that this is basically C
code.
There are currently two unit tests in the `schnorr` module that are
testing keys from the `key` module. This is possible because the tests
are only testing the public interface, none the less they are better
placed in the `key` module.
The import statements can be simplified by using an import
wildcard (`super::*`). While we are at it put them in std, external
crate, this crate order.
5e6d0f1363 Switch to associated constant (Jonathan Underwood)
9cf552e240 Add a static immutable zero aligned type (junderw)
Pull request description:
The `zeroed` fn can not be used in static assignments.
In environments where it is no_std and no allocator are present, the only way to get a slice of AlignedTypes is dynamically, so `preallocated_gen_new` can't be used.
By offering this as a static, it can be used in static assignments as such:
```rust
#[cfg(target_pointer_width = "32")]
static mut CONTEXT_BUFFER: [AlignedType; 69645] = [ZERO_ALIGNED; 69645];
#[cfg(target_pointer_width = "64")]
static mut CONTEXT_BUFFER: [AlignedType; 69646] = [ZERO_ALIGNED; 69646];
static mut SECP256K1: Option<Secp256k1<AllPreallocated>> = None;
pub fn get_context(seed: Option<&[u8; 32]>) -> &'static Secp256k1<AllPreallocated<'static>> {
unsafe {
if SECP256K1.is_none() {
SECP256K1 = Some(
Secp256k1::preallocated_gen_new(&mut CONTEXT_BUFFER)
.expect("CONTEXT_BUFFER size is wrong"),
);
}
if let Some(seed) = seed {
SECP256K1.as_mut().unwrap().seeded_randomize(seed);
}
SECP256K1.as_ref().unwrap()
}
}
```
ACKs for top commit:
apoelstra:
ACK 5e6d0f1363
Tree-SHA512: fc800f8c5c637fc7f81312da17f0a96d17cd087a2e6876f4dedbefffbe92b3625deb93636265f334f9fbd7ac38baa529d4ec72857dae662e26d753f32f91d394
21aa914ad2 Change context objects for schnorr sig methods (sanket1729)
Pull request description:
- The current schnorrsig verify methods should operate on verify context
as is done throughout the bitcoin core
- Finally, and importantly the XonlyPublicKey::from_keypair now operates
without any context parameter.
ACKs for top commit:
apoelstra:
ACK 21aa914ad2
Tree-SHA512: 035338f19839805a080eb262ae7b93ab187dabb63086c8b7f6015f3a6006986604dc2c6f329a99a20ddfa78c1ee518f44cd5eee2f73810fbdc83ff8df7d12506
- The current schnorrsig verify methods should operate on verify context
as is done throughout the bitcoin core
- Scondly, and importantly the XonlyPublicKey::from_keypair now operates
without any context objects.
de77518d3a Serde serialization for KeyPair (Dr Maxim Orlovsky)
Pull request description:
Serde implementation for `KeyPair` type (which hadn't it before).
Based on #312 (includes all commits from that PR, will be rebased upon merge)
ACKs for top commit:
apoelstra:
ACK de77518d3a
Tree-SHA512: 1e75c4fc772dcba5ce7edb30235a58550342cf986c6a77b4affd81defeba456c9655e28b081e0040c1f8440da3f7ad2224485d35222c1921099567b4d1533794
d244b4d747 Fix typo in docs (Thomas Eizinger)
c5c95513f2 Move helper function below usage (Thomas Eizinger)
ce4427747d Move ECDSA functionality into ECDSA module (Thomas Eizinger)
e0c3bb28c4 Rename schnorr functions on `Secp256k1` to match naming of ecdsa (Thomas Eizinger)
760559c70e Rename `schnorrsig` module to `schnorr` (Thomas Eizinger)
d4fb819d80 Move `XOnlyPublicKey` to `key` module (Thomas Eizinger)
87d936a765 Rename `schnorr::PublicKey` to `schnorr::XOnlyPublicKey` (Thomas Eizinger)
2e0e731664 Move `KeyPair` to `key` module (Thomas Eizinger)
c47ead9967 Move `Signature` and `SerializedSignature` to new `ecdsa` module (Thomas Eizinger)
49c7e21486 Prefer `use super::*` import over manually picking items (Thomas Eizinger)
52d0554423 Fully qualify Error to simplify imports (Thomas Eizinger)
8e96abae39 Make `key` module private (Thomas Eizinger)
Pull request description:
This patch-set tries to re-structure the library a bit. What we currently have seems to have been mostly driven by historical growth. For example, with the addition of Schnorr signatures, just exposing `secp256k1::Signature` is ambiguous.
This PR only contains renames and moving around of code. I've tried to structure the patches in such a way that makes this reasonably easy to review. Feedback welcome!
ACKs for top commit:
sanket1729:
ACK d244b4d747
apoelstra:
ACK d244b4d747
Tree-SHA512: d40af5c56ffa500305e40eb5dbe72f2f6d6193b3a190910018d3bacdec2820ab6a59f15d47d11e0fee7ef4de6efd46d316636cd502aad5db4f314dedfff726f9
Tobin Harding
Martin Habovstiak
Andrew Poelstra
sanket1729
Jonathan Underwood
Dr Maxim Orlovsky
Thomas Eizinger