68c73850d8 Minimise FFI in the public API (Tobin C. Harding)
Pull request description:
Normal users should never need to directly interact with the FFI layer.
Audit and reduce the use of `ffi` types in the public API of various types. Leave only the implementation of `CPtr`, and document this clearly as not required by normal users. Done for:
- PublicKey
- XOnlyPublicKey
- KeyPair
- ecdsa::Signature
- ecdsa::RecoverableSignature
ACKs for top commit:
apoelstra:
ACK 68c73850d8
Tree-SHA512: 8242527837872f9aba2aab19b02c2280ca1eb1dfd33c8ca619726d981811d72de3e5a57cbde2fbe621eb8e50e43f488804cd51d27949459da1c0ceb03fca35e3
Normal users should never need to directly interact with the FFI layer.
Audit and reduce the use of `ffi` types in the public API of various
types. Leave only the implementation of `CPtr`, and document this
clearly as not required by normal users. Done for:
- PublicKey
- XOnlyPublicKey
- KeyPair
- ecdsa::Signature
- ecdsa::RecoverableSignature
Needed to build secp-sys 0.5 and secp-sys 0.6 in the same tree. Fixes#489.
This PR can be reproduced by running
./vendor-libsecp.sh depend/ 0_6_1 a1102b12196ea27f44d6201de4d25926a2ae9640
in the secp256k1-sys directory.
Dunno why we haven't seen this elsewhere, but when trying to build
locally for an ARM embedded target `secp256k1-sys` failed to
compile as it was missing `string.h`, just like WASM.
This patch adds a trivial fallback - if we fail to compile
initially we unconditionally retry with the wasm-sysroot, giving us
a valid `string.h`.
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
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.
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>
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
Clippy warns about unsafe code without a `# Safety` section. A bunch of
these warnings are for functions that do actually have safety docs.
Follow rustdoc convention and add a `# Safety` section for the already
existing explanations.
Currently we are defining the WASM integer size and alignments in the
`stdio.h` header file, this is wrong because this file is included in
the build by way of `build.rs` as well as by upstream `libsecp256k1`.
Move WASM integer definitions to a `C` source file and build the file
into the binary if target is WASM.
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
When building with --no-default-features the compiler emits:
warning: unused import: `mem`
The call site is feature gated so we either need to feature gate the
import or use a fully qualified path. Since 'core' is quite short elect
to use the fully qualified path.
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.
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
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
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
cc-rs builds C dependencies with reduced visibility to avoid
exporting the C symbols all the way out to any rust-built shared
libraries however we override it with SECP256K1_API. We should
avoid doing this, allowing LTO/DCE to do its work.
We can now run unit tests with the fuzz feature on, and they'll pass,
which is some assurance that fuzzing with the feature on won't lead to
spurious failures due to the fuzz harness inadequately simulating message
signing.
It's super dangerous to use Cargo features for this, since they can be set
accidentally (or maliciously by any crate in a user's entire dep tree). Instead
we can just require users set `RUSTFLAGS` appropriately, which we can easily
do in our fuzzing scripts.
This feature was not useful for Cargo users, since Cargo does not give you
the kind of fine-grained control over C library linkage that you need. So
it was just unnecessarily confusing and would cause the build to break if
you enabled it accidentally, say, with --all-features.
Clippy emits a warning since we define a method that has the same name
as a standard trait. Implement the trait `AsRef` instead of using a
custom method.
This is configurable in upstream now.
Fixes#214.
So far, this just sets it to `4`. It's tempting to set it to `2` when `lowmemory` is enabled
but `lowmemory` is about RAM and not about binary size. This is better addressed by
a resolution of #193.
libsecp256k1 really only barely uses libc at all, and in practice,
things like memcpy/memcmp get optimized into something other than a
libc call. Thus, if we provide simple stub headers, things seem to
work with wasm-pack just fine.
* rust-bitcoin/master:
Fix no-std raw test, after removal of lang items
Fix broken benchmarks
Disable emscripten tests until they work again https://github.com/rust-lang/rust/issues/66916https://github.com/rustwasm/team/issues/291
Add constant of the prime of the curve field.
Simplify callback logic to returning raw coordinates
Removed no longer used dont_replace_c_symbols feature
Fix wrong feature name external-symbols
Fix missing return c_int in NonceFn
Andrew Poelstra
Tobin C. Harding
Matt Corallo
Tim Ruffing
Dr Maxim Orlovsky
Elichai Turkel
Tibo-lg
Martin Habovstiak
Jonathan Underwood
elsirion
Riccardo Casatta
Kirill Fomichev
Gregory Hill
Kohei Taniguchi
Andronik Ordian