We no longer need to manually configure the docsrs build to highlight
feature guards since we use the `doc_auto_cfg` feature. Somehow when we
added usage of that feature we forgot to remove the other attributes.
896e6c7f2d Introduce SPDX license identifiers (Tobin C. Harding)
Pull request description:
Licenses are boring as hell, so is are all the comments at the top of each file. This patch makes no comment on the merit of license comments in each file, rather this patch reduces the license comment to the minimum possible with no loss of meaning - an SPDX license identifier.
Note also please that we remove the "written by" comments as well for the following reasons (discussed recently on rust-bitcoin repo):
- they are not descriptive because many devs contributed
- they have a tendency to include the wrong date because of cut'n'pasta
- all this info is in the git history
ref: https://spdx.dev/ids/#how
cc elichai because this PR removes your name but you were not explicitly part of the conversation on `rust-bitcoin` about this topic. Here is the issue: https://github.com/rust-bitcoin/rust-bitcoin/issues/1816 also for more on SPDX see https://github.com/rust-bitcoin/rust-bitcoin/pull/1076
ACKs for top commit:
Kixunil:
ACK 896e6c7f2d
apoelstra:
ACK 896e6c7f2d
Tree-SHA512: 6f0ff7ec2632aed510df362e2fb9cf25fe02cae347bdd4a481804a3ea2b9e060c4ec2c85de3e9d1d40920e4b9c4eecfab127e61f3d076886fe8f2fb4bff9f5a7
Licenses are boring as hell, so is are all the comments at the top of
each file. This patch makes no comment on the merit of license comments
in each file, rather this patch reduces the license comment to the
minimum possible with no loss of meaning - an SPDX license identifier.
Note also please that we remove the "written by" comments as well for
the following reasons (discussed recently on rust-bitcoin repo):
- they are not descriptive because many devs contributed
- they have a tendency to include the wrong date because of cut'n'pasta
- all this info is in the git history
ref: https://spdx.dev/ids/#how
Note: Only effects code when fuzzing is enabled, as such does not
include a mention in the changelog.
Now that we have Rust 1.48 as the MSRV we no longer need the custom
implementations of `PartialEq`, `Eq`, `PartialOrd`, `Ord`, and `Hash`.
We can just let users of the `impl_array_newtype` macro derive these
traits if they want them.
Remove the custom implementations and add derives to our two users of
the macro.
We are ready to release a new minor version of `secp256k1-sys`, in order
to do so we must make change the symbol names to reflect the new version
as well as the usual changelog and version bump.
In preparation for releasing `secp256k1-sys` v0.8.1 do:
- Rename symbols to from `0_8_0` -> `0_8_1`, done mechanically (search
and replace)
- Add changes log notes (includes changelog entry for 0.8.0)
- Bump `secp256k1-sys` crate version 0.8.0 -> 0.8.1, justified because
we have added a new public function.
This PR implements a `non_secure_erase()` method on SecretKey,
KeyPair, SharedSecret, Scalar, and DisplaySecret. The purpose
of this method is to (attempt to) overwrite secret data with
valid default values. This method can be used by libraries
to implement Zeroize on structs containing secret values.
`non_secure_erase()` attempts to avoid being optimized away or
reordered using the same mechanism as the zeroize crate: first,
using `std::ptr::write_volatile` (which will not be optimized
away) to overwrite the memory, then using a memory fence to
prevent subtle issues due to load or store reordering.
Note, however, that this method is *very unlikely* to do anything
useful on its own. Effective use involves carefully placing these
values inside non-Copy structs and pinning those structs in place.
See the [`zeroize`](https://docs.rs/zeroize) documentation for tips
and tricks, and for further discussion.
[this commit includes a squashed-in commit from tcharding to fix docs
and helpful suggestions from apoelstra and Kixunil]
`libsecp256k1` v0.2.0 was just released.
Update the vendored code using
`./vendor-libsecp.sh depend 0_8_0 21ffe4b`
```
git show 21ffe4b
commit 21ffe4b22a9683cf24ae0763359e401d1284cc7a (tag: v0.2.0)
Merge: 8c949f5 e025ccd
Author: Pieter Wuille <pieter@wuille.net>
Date: Mon Dec 12 17:00:52 2022 -0500
Merge bitcoin-core/secp256k1#1055: Prepare initial release
e025ccdf7473702a76bb13d763dc096548ffefba release: prepare for initial release 0.2.0 (Jonas Nick)
6d1784a2e2c1c5a8d89ffb08a7f76fa15e84fff5 build: add missing files to EXTRA_DIST (Jonas Nick)
13bf1b6b324f2ed1c1fb4c8d17a4febd3556839e changelog: make order of change types match keepachangelog.com (Jonas Nick)
b1f992a552785395d2e60b10862626fd11f66f84 doc: improve release process (Jonas Nick)
ad39e2dc417f85c1577a6a6a9c519f5c60453def build: change package version to 0.1.0-dev (Jonas Nick)
90618e9263ebc2a0d73d487d6d94fd3af96b973c doc: move CHANGELOG from doc/ to root directory (Jonas Nick)
Pull request description:
Based on #964
ACKs for top commit:
sipa:
ACK e025ccdf7473702a76bb13d763dc096548ffefba
Tree-SHA512: b9ab71d7362537d383a32b5e321ef44069f00e3e92340375bcd662267bc5a60c2bad60222998e6602cfac24ad65efb23d772eac37c86065036b90ef090b54c49
```
Requires a new version of `secp256k1-sys`, use v0.8.0
- Update the `secp256k1-sys` manifest (including links field)
- Update symbols to use 0_8_0
- Add a changelog entry
- depend on the new version in `secp256k1`
Which in turn requires a new version of `secp256k1`, use v0.26.0
We are ready to release a new minor version of `secp256k1-sys`, in order
to do so we must make change the symbol names to reflect the new version
as well as the usual changelog and version bump.
In preparation for releasing `secp256k1-sys` v0.7.0 do:
- Rename symbols to from `0_6_1` -> `0_7_0`, done mechanically (search
and replace)
- Add changes log notes
- Bump `secp256k1-sys` crate version 0.6.1 -> 0.7.0, justified because
we have added new public methods to various types (e.g.,
`PublicKey::cmp_fast_unstable`)
Currently we expect non-null pointers when we take `*mut T` parameters,
however we do not check that the pointers are non-null because we never
set VERIFY in our C build. We can use the `NonNull` type to enforce
no-null-ness as long as we use `NonNull::new`. In a couple of instances
we manually check that a buffer is not empty and therefore that the
pointer to it is non-null so we can safely use `NonNull::new_unchecked`.
Replace mutable pointer parameters `*mut T` (e.g. `*mut c_void`) and
return types with `NonNull<T>`.
Fix#546
Functions that are `unsafe` should include a `# Safety` section. Because
we have wrapper functions to handle symbol renaming we essentially have
duplicate functions i.e., they require the same docs, instead of
duplicating the docs put the symbol renamed function below the
non-renamed function and add a docs linking to the non-renamed function.
Also add attribute to stop the linter warning about the missing safety
docs section.
Remove the clippy attribute for `missing_safety_doc`.
There is no obvious reason why not to derive `Copy` and `Clone` for
types that use the `impl_newtype_macro`. Derives are less surprising so
deriving makes the code marginally easier to read.
Currently we rely on the inner bytes with types that are passed across
the FFI boundry when implementing comparison functions (e.g. `Ord`,
`PartialEq`), this is incorrect because the bytes are opaque, meaning
the byte layout is not guaranteed across versions of `libsecp26k1`.
Implement stable comparison functionality by doing:
- Implement `core::cmp` traits by first coercing the data into a stable
form e.g., by serializing it.
- Add fast comparison methods to `secp256k1-sys` types that wrap types
from libsecp, add similar methods to types in `secp256k1` that wrap
`secp256k1-sys` types (just call through to inner type).
- In `secp256k1-sys` feature gate the new `core::cmp` impls on
`not(fuzzing)`, when fuzzing just derive the impls instead.
Any additional methods added to `secp256k1-sys` types are private,
justified by the fact the -sys is meant to be just a thin wrapper around
libsecp256k1, we don't want to commit to supporting additional API
functions.
Please note, the solution presented in this patch is already present for
`secp256k1::PublicKey`, this PR removes that code in favour of deriving
traits that then call down to the same logic in `secp256k1-sys`.
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.
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.
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.
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