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
The public key is unrelated to the signature algorithm. It will
be moved out of the module in another commit. For ease of review,
the renamed is kept separate.
With the introduction of Schnorr signatures, exporting a `Signature`
type without any further qualification is ambiguous. To minimize the
ambiguity, the `ecdsa` module is public which should encourage users
to refer to its types as `ecdsa::Signature` and `ecdsa::SerializedSignature`.
To reduce ambiguity in the APIs on `Secp256k1`, we deprecate several
fucntions and introduce new variants that explicitly mention the use of
the ECDSA signature algorithm.
Due to the move of `Signature` and `SerializedSignature` to a new module,
this patch is a breaking change. The impact is minimal though and fixing the
compile errors encourages a qualified naming of the type.
75b49efb3d Implement `Hash` for all array newtypes (elsirion)
Pull request description:
I pondered putting the impl into the array type macro together with `(Partial)Eq`, but that would have meant removing other implementations and potentially implementing it for types where it is not wanted. The drawback of the separate impl is that it is more disconnected from the `(Partial)Eq` impl and could theoretically diverge (although unlikely in case of such a simple type) which would break the trait's contract.
ACKs for top commit:
apoelstra:
ACK 75b49efb3d
Tree-SHA512: 44d1bebdd3437dfd86de8b475f12097c4a2f872905c822a9cde624089fdc20f68f59a7734fdcc6f3a17ed233f70f63258dfd204ca269d2baf8002ffc325ddc87
801c3789c4 disable illumos and netbsd (Riccardo Casatta)
a426456bfa [CI] add cache (Riccardo Casatta)
f1bdee210a add cross testing on rust tier 1 and tier 2 with host tools (Riccardo Casatta)
Pull request description:
After working on https://github.com/rust-bitcoin/rust-bitcoin/pull/627 I thought it may be simple and useful to use [cross](https://github.com/rust-embedded/cross) environment to test across different architectures and started here.
So I took all rust tier1 and tier2 with Host tools archictectures and added to the test matrix, run here https://github.com/RCasatta/rust-secp256k1/actions/runs/985791240.
Errors on darwin are due to the environment because it works fine on physical machine, however errors on illumos and netbsd maybe are useful to know?
ACKs for top commit:
apoelstra:
utACK 801c3789c4
Tree-SHA512: 43c7220e41856344d4a932426d8acb8e9f004fcf33acfbde4b26f3a6074b0ce3e766d99afaca6c18381a4438775fa693b06c70d3204d83ff5a97fcbebe126056
24d6f62603 Use explicit u8 when assigning a byte slice (junderw)
Pull request description:
Is there a way to tell the compiler to not allow `[0; 64]` and require that either the type is explicitly given to the variable, or that each member uses explicit `0u8` notation?
I noticed the usage was a mix of explicit and implicit, so I changed all to explicit.
ACKs for top commit:
apoelstra:
ACK 24d6f62603
Tree-SHA512: f7796dcc3ae240983257bef0f25bd0df741943f75d86e9bca7c45076af179d96ce213bd9c339a01f721f7dc9b96a0a4a56ef2cf44339f4c91d208103b7659d9f
6810c2b547 Dedicated display_secret fn for secret-containing types (Dr Maxim Orlovsky)
635a6ae441 Add to_hex converter and add tests for hex conversion (Elichai Turkel)
Pull request description:
Extract of concept ACK part of #311 related to changing the way secret keys are displayed/printed out
ACKs for top commit:
apoelstra:
ACK 6810c2b547
thomaseizinger:
ACK 6810c2b547
Tree-SHA512: 22ad7b22f47b177e299ec133129d607f8c3ced1970c4c9bea6e81e49506534c7e15b4fb1d745ba1d3f85f27715f7793c6fef0b93f258037665b7f740b967afe5
bc42529a16 Rename `secp256k1::bitcoin_hashes` module to `secp256k1::hashes` (Thomas Eizinger)
ae1f8f4609 Bump bitcoin_hashes to version 0.10 (Thomas Eizinger)
Pull request description:
Requires for interoperability of the `ThirtyTwoByteHash` trait with
rust-bitcoin.
ACKs for top commit:
apoelstra:
ACK bc42529a16
Tree-SHA512: 85fcb284ff82b543a0c3ea2b568351b3af938a26ac42c6a975480ae97def84e4f0795105bd4572f930a7bf82654eba416cf0c5e25f62809e2ea331443ffb5807
Andrew Poelstra
Martin Habovstiak
Tobin Harding
sanket1729
Jonathan Underwood
Dr Maxim Orlovsky
Thomas Eizinger
elsirion
Riccardo Casatta
Elichai Turkel
Tibo-lg