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.
Instead of providing a mechanism for users to opt out of randomization
we can just feature gate the call site i.e., opportunistically randomize
the global context on creation if `rand-std` feature is enabled.
Seems there is a bug in cargo, the tests in `key.rs` run successfully
but AFAICT they should fail. Here is an example, running `cargo test
--features=rand` should make this test fail but it doesn't?
```
/// Secret 256-bit key used as `x` in an ECDSA signature.
///
/// # Examples
///
/// Basic usage:
///
/// ```
/// # #[cfg(all(feature = "rand", any(feature = "alloc", feature = "std")))] {
/// use secp256k1::{rand, Secp256k1, SecretKey};
///
/// let secp = Secp256k1::new();
/// let secret_key = SecretKey::new(&mut rand::thread_rng());
/// # }
/// ```
Anywho, use the correct feature gate: `rand-std`.
Various combinations of features trigger lint warnings for unused code,
all warnings are caused by incorrect feature gating.
Correct feature gating to remove Clippy warnings during testing.
Currently various features fail to build when enabled without default
features. This is because many tests need feature gating.
Feature gating the import statements quickly turns into spaghetti when
trying to cover all combinations of two features correctly, instead just
allow unused imports on `tests` modules where needed.
Add correct feature requirements to the examples so they also can be run
without default features.
Improve the CI script by doing:
- Add `std` to the feature matrix.
- Add `--no-default-features` to test runs in the CI script.
aa828f01a5 Improve documentation in the key module (Tobin Harding)
9e46d6f122 Add examples to types and methods in key module (Tobin Harding)
a7f3d9bcfd Improve key module docs (Tobin Harding)
6d23614467 Improve lib.rs rustdocs (Tobin Harding)
4c4268f1ad Improve docs on method generate_keypair (Tobin Harding)
Pull request description:
This PR is an initial attempt to more thoroughly test our public API.
Add examples to various types/methods/functions in the key module.
I'm not entirely sure when is enough, do we want an example on every single public method, function, and type or is this overkill. In this PR I tried to find a balance by doing ever method/function that took an argument that is a custom type from this lib. I think this should be extended to include return values too though ...
Thanks to @thomaseizinger for the idea!
First 2 patches are docs improvements to `lib.rs`.
ACKs for top commit:
apoelstra:
ACK aa828f01a5
Tree-SHA512: 9383ad263469f98ce7e988d47edc1482a09a0ce82f43d3991bd80aabdf621430f4a3c86be4debf33232dcb1d60d3e81f2c6d930ea7de7aa0e34b037accd7bc98
We recently patched much of the docs in the `key` module, lets attempt
to attain perfection.
Improve docs by doing:
- Use full stops
- Use 100 character column width
- Use plural third person tense
- Use plural for section headings
- Fix any grammar mistakes
- Use code ticks and links as appropriate
Done in an effort to better test our public API.
Add tests in the `Examples` section as is idiomatic in the Rust
ecosystem.
Make other minor improvements to any rusdocs we touch:
- Use full stops
- Use 100 character column width
- Use plural third person tense
- Use plural for section headings
Recently we made a wee mess with the `Parity` opaque type. Let's fix it
up by doing:
- Use an enum with variants `Even` and `Odd`.
- Add explicit conversion methods to/from u8 and i32
- Implement `BitXor`
Note: This patch is an API breaking change that does _not_ follow the
deprecation guidelines. Rust does not allow deprecating `From` impl
blocks AFAICT.
We have deprecated all other functions that use the identifier
'schnorrsig' but we missed `generate_schnorrsig_keypair`.
This function is purely a helper function and serves no real purpose
other than to reduce two lines of code to a single line. Downstream
users can write this function themselves if they need it.
Also, we recently added a new public method to `KeyPair` to get the
public key in a slightly more ergonomic fashion. Use `kp.public_key()`
when replacing usage of now deprecated `generate_schnorrsig_keypair`
function.
Currently to get the `XOnlyPublicKey` from a `KeyPair` users must do
`XOnlyPublicKey::from_keypair(&kp)`. While this does the job we can make
the lib more ergonomic by providing a method directly on `KeyPair` that
calls through to `XOnlyPublicKey::from_keypair`.
Add method `KeyPair::public_key(&self)`.
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
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
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.
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.
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.
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
This reduces the usage of real cryptography in --cfg=fuzzing,
specifically replacing the secret->public key derivation with a
simple copy and ECDH with XOR of the public and private parts
(plus a stream of 1s to make a test pass that expected non-0
output).
It leaves secret tweak addition/multiplication as-is.
It also changes the context creation to over-allocate and store
the context flags at the end of the context buffer, allowing us
to easily test context flags in each function.
While it would be nice to have something fancier (eg XOR-based),
its not immediately obvious how to accomplish this, and better to
fix the issues I have than spend too much time on it.
Fixes#271.
This partially reverts b811ec133a
In the next commit the secret->public key derivation in fuzzing cfg
is changed to be simpler, as well as the validity rules of public
keys relaxed.
This adds a new test to ensure random keys can be added, not just
the hard-coded keys test that exists today.
Currently we are misusing `map` on an iterator to loop `n` times,
additionally the assertion is pointless. Use a for loop and assert
against the length of the set.
Instead of repeating ourselves in defining one big test for the wasm
target, we can override the `test` attribute with the `wasm-bindgen-test`
one and therefore automatically run all (supported) tests in wasm.
Unfortunately, wasm doesn't support catching panics yet which means we
have to disable the `test_panic_raw_ctx` test.
The interfaces for negate should always returns 1 as mentioned secp256k1.h L574, L563.
But in the future it might return 0 if the seckey or pubkey is invalid, but our type system doesn't allow that to ever happen.
Tobin Harding
Andrew Poelstra
elsirion
sanket1729
Martin Habovstiak
Dr Maxim Orlovsky
Thomas Eizinger
junderw
Tibo-lg
Matt Corallo
Elichai Turkel
Justin Moon
Kohei Taniguchi
Rodolfo Araujo
Steven Roose
Jonas Nick
Tim Ruffing
Aleksei Sidorov
François Garillot
Pierre Krieger
Tim Ruffing