38106866c1 Revert "Automated update to Github CI to rustc nightly-2024-09-10" (Andrew Poelstra)
d3d9a050a7 fix docs for new clippy lint. (Andrew Poelstra)
Pull request description:
There are a bunch of doccomments whose first lines are (much) too long. Most of these are also difficult to understand and/or out-of-date. Just rewrite them all.
ACKs for top commit:
Kixunil:
ACK 38106866c1
Tree-SHA512: 291bd2c30c8d46c54d99eba17b6cc5f018912b906f4395fa753218551c1ba50724bdd55699f12bf9de254debf9612541c47e1fcd9c2eb04784f71c21e94b5ea5
There are a bunch of doccomments whose first lines are (much) too long.
Most of these are also difficult to understand and/or out-of-date. Just
rewrite them all.
78d93b7ca5 Automated update to Github CI to rustc nightly-2024-09-10 (Update Nightly Rustc Bot)
Pull request description:
Automated update to Github CI workflow `rust.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
ACKs for top commit:
tcharding:
ACK 78d93b7ca5
Tree-SHA512: fa48f5318e37dd8bf5e28892c4d086f3609bd9c5f11adbaea4898ee54a45302cae39c9b86cfac7a929fb1b4e7eee23be23fb5be3c9669a003d32ac9e4fc7a524
f0100e1c55 Stop indexing hash types (Tobin C. Harding)
Pull request description:
In preparation for removing `SliceIndex` from hash type impls (in `bitcoin_hashes`) lets stop indexing hash types here.
Internal change only.
ACKs for top commit:
jlest01:
ACK f0100e1c55
Kixunil:
ACK f0100e1c55
apoelstra:
ACK f0100e1c55 successfully ran local tests
Tree-SHA512: 196d83326f8d62bb095c39a67083d3f8daca65b458abffc92ebec469a6213d03003786d046f795f8b082d8e94fc1a0f7f4d5f8c66021785fb79cbbdeed8042f7
ae0a304c2d ci: add shellcheck job (Jose Storopoli)
9f9bd34365 shellcheck: fix warnings (Jose Storopoli)
Pull request description:
Following https://github.com/rust-bitcoin/rust-bitcoin/pull/2762,
adding CI shellcheck cheks here as well.
I also did all fixes that I could find with
```bash
shellcheck **/*.sh
```
If I've missed any please let me know.
ACKs for top commit:
Kixunil:
ACK ae0a304c2d
tcharding:
ACK ae0a304c2d
apoelstra:
ACK ae0a304c2d successfully ran local tests
Tree-SHA512: 0293027439cc922ad9b1ccd5184b481d33fe80b4cd270f425805e7c31108b0bfd7ebe8a76b6aca55300584846f4ca9fbd810faa9ab1d5402e5f1be2bf12fed1a
aee0cfcbf9 CI: Re-write using maintainer tools (Tobin C. Harding)
89facea27f Rewrite cross to use dtolnay runner (Tobin C. Harding)
0668943330 CI: Remove cross job (Tobin C. Harding)
Pull request description:
Patch 1 is now on its own in #728
Re-write CI using the new maintainer tools script. A few things to note:
- Currently we have `Cross` job in `rust.yaml` as well as `cross.yaml`, remove the one in `rust.yaml`.
- Put sanitizer and wasm jobs in their own scripts
- Utilize `extra_tests.sh` for additional feature combos
- We are exceeding the 20 job limit, see the README
Unless I'm made a mistake this shouldn't reduce the test coverage in any way (except sanitizer mentioned below).
I commented out the MSAN stuff same as we did in `hashes`. I'm not sure what is the status of that but it seems to be failing still - did not look into it.
Please note, I do not know _why_ the xargo stuff is run from in the ASAN job currently, but this PR keep it that way - adding it to the `sanitizer.sh` script.
ACKs for top commit:
storopoli:
ACK aee0cfcbf9
apoelstra:
ACK aee0cfcbf9 successfully ran local tests
Tree-SHA512: ab828c19c9189bb3af7e517deafca1adf606d3e9db81a890b12125bb9923b6f9f3b8e2ab7afc538aa58aa62958e38f07e6418ccfa985c06595a1b6dbeca247b6
Re-write CI using the new maintainer tools script. A few things to note:
- Put sanitizer and wasm jobs in their own scripts
- Utilize `extra_tests.sh` for additional feature combos
- We are exceeding the 20 job limit, see the README
Unless I'm made a mistake this shouldn't reduce the test coverage in any
way.
d600a6cf00 Feature gate the Keypair::FromStr impl (Tobin C. Harding)
Pull request description:
Currently we are panicing if neither `global-context` or `alloc` features are enabled. We do not need to do so, we can just disable the whole impl of `FromStr`.
This was pulled out of #699.
ACKs for top commit:
apoelstra:
ACK d600a6cf00 successfully ran local tests
Kixunil:
ACK d600a6cf00
Tree-SHA512: 940bec95ce732b4bc482e23da114cb03b767780f93777621c9d0985d1288e36756bdf6f050172eac00f89b6f39aa0efdb30cc77425b6f87505659c8c012981ca
As we do in other places stop using the `actions` runner and use the
`dtolnay` one to checkout toolchain.
While we are at it, use double quotes for `name` fields (this is a small
stylistic thing I have been introducing in an effort to make the yaml
files a bit easier to read).
Currently we are panicing if neither `global-context` or `alloc`
features are enabled. We do not need to do so, we can just disable the
whole impl of `FromStr`.
b8ac971745 keypair: use public key for Debug output (Andrew Poelstra)
a16e5ecd49 secret keys: debug output only when `hashes` is enabled (Andrew Poelstra)
Pull request description:
In addition to changing `SecretKey` and `SharedSecret` to use `hashes`, we also unconditionally use the public half of `KeyPair` as a fingerprint, since that's always available and does not need extra deps.
This patches the existing unit tests but doesn't add more. Maybe they should be removed; it's a bit weird to have unit tests for `Debug` output. But in this case we're doing some nontrivial logic and I guess we wanted to double-check that it was taking effect.
I'd also like to change the manual tagged-hash implementation to use `bitcoin_hashes` methods but those are under construction https://github.com/rust-bitcoin/rust-bitcoin/pull/3184 and the existing stuff is neither faster nor less code than what's currently done. So we'll live with it.
Fixes#725
ACKs for top commit:
Kixunil:
ACK b8ac971745
Tree-SHA512: d0a65e0a0069bcbc663c1d3e7f98b75868355c4db48e9a9c905cdcd2af1606ac86090cdf0aae5caa23337c5d565e6420d7c956dd0a65a1877004840075bc08e9
3d1ce0d261 Truncate secret hash using precision (Tobin C. Harding)
4b143d6f9c Remove bitcoin_hashes range dependency (Tobin C. Harding)
Pull request description:
The `core:#️⃣:Hasher` and `bitcoin_hashes` hash types implement formatting traits slightly differently
- We default to displaying in hex but `core` defaults to using base 10
- We truncate with precision not width parameter but core truncates with both
Anywho, this PR fixes the secret display truncation.
ACKs for top commit:
Kixunil:
ACK 3d1ce0d261
apoelstra:
ACK 3d1ce0d261 successfully ran local tests
Tree-SHA512: f4f15c084f33bf270eab7b578891b50aa743caac12eb0cc3f7ced8fce2df2af93fcca859a2bc0a50396434514fad63368cd81753b8634a41dc3da996d1b1996c
Currently we are attempting to truncate the hash created using
`bitcoin_hashes` by using the "width" formatting parameter instead of
the "precision" parameter. `hex-conservative` truncates with the
"precision" parameter as is expected since a hash is not an integral
type.
Use the formatting string `"{:.16}"` which is the "precision"
formatting parameter.
dc3fc0919d rustfmt: Use show_parse_errors (Tobin C. Harding)
Pull request description:
Clear deprecation warning by using the new option.
ACKs for top commit:
apoelstra:
ACK dc3fc0919d successfully ran local tests
Tree-SHA512: 3277d832d71a8d9ca773c1a0dd54ca79435c703a80994b3512c5a781b1a4e56ecf21639d9e0bc12b60b984b4ccf2201c532b4c7df52c012e1f507f8d0bc607b8
1b0c79ce90 Remove check-for-api infrastructure (Tobin C. Harding)
Pull request description:
This was a short lived, and unfortunately unsuccessful experiment on how to catch API breaking changes. As we did elsewhere in the org, remove the check-for-api infrastructure.
ACKs for top commit:
apoelstra:
ACK 1b0c79ce90 successfully ran local tests
Tree-SHA512: 0fddf186d37af3863428af80ff5c5a941b0d1b40fd4d72b6c1bcf8dec1cb4127446d4d61e19cc12e2ff35a5cd3f98dce0980f913cc38a947dc4d12605b8bebe8
This was a short lived, and unfortunately unsuccessful experiment on how
to catch API breaking changes. As we did elsewhere in the org, remove
the check-for-api infrastructure.
72e09c1a7c Improve the comment on `Message::from_digest` (Martin Habovstiak)
Pull request description:
Minor improvement on top of #712
ACKs for top commit:
apoelstra:
ACK 72e09c1a7c
Tree-SHA512: 06e8e706bb9732ea46ef3488ed33f7c7c84ea5afa5b1b2bca03cd2641524ff61156133436c1dd62df62769c8544644e1a4453fbacf4413fece73282ae154a387
The example claimed it'd be unsafe, which is a specific Rust term and
thus confusing. It'd just be cryptographically broken. Also the example
passes in a constant which looks ridiculously unrealistic.
Fix these by
* changing the comment to say cryptographically broken
* making the example pass the input through invisible fake hash function
939bf9ed5e Deprecate `Message::from_digest_slice` (Martin Habovstiak)
13c5366238 Use `hex_lit::hex` in tests (Martin Habovstiak)
Pull request description:
Closes#710
On top of #709
ACKs for top commit:
apoelstra:
ACK 939bf9ed5e
Tree-SHA512: 34afc2c040e84745881d8d6d250e6fbe5a42b0fcd4dc3ea01579c52948b6ab89d31eb0ffd449d322ec1fe2c7076c05e5b6343a3e1821eae62fd0c13db926f228
All sensible hash engines return arrays, not slices or other things,
therefore `Message::from_digest_slice` is most likely entirely unneeded
since the array version does a better job and in those rare cases where
it is, the users can just call `.try_into()` themselves.
This commit deprecates `from_digest_slice` and changes all tests to use
`from_digest` except the test that tests `from_digest_slice`. It also
simplifies its code to use `try_into` rather than convert manually and
inefficiently.
The tests defined custom `hex!` macros (yes, two actually) that
evaluated to `Vec<u8>`. While the performance didn't matter it made it
harder to use with interfaces that require arrays and all current uses
were passing it as slices anyway.
So, in preparation for upcoming changes, this commit introduces
`hex_lit` dev-dependency which evaluates to array allowing better
interaction with type checker.
55c2efc320 Bump MSRV to 1.63 (Martin Habovstiak)
Pull request description:
The version 1.63 satisfies our requirements for MSRV and provides significant benefits so this commit bumps it. This commit also starts using weak dependencies.
ACKs for top commit:
tcharding:
ACK 55c2efc320
Tree-SHA512: 565fd46768384e7c026c3aa8873e321a20425a6526bcd379ba442cf2504517a435c6c14e21186b36c99185d0a8439f4de2d3ba097b91119483d1a83ab05010ba
The version 1.63 satisfies our requirements for MSRV and provides
significant benefits so this commit bumps it. This commit also starts
using weak dependencies.
df98b160d8 Make schnorr sign/verify accept a message slice (Elichai Turkel)
Pull request description:
As discussed on https://github.com/rust-bitcoin/rust-secp256k1/issues/702 and on IRC,
BIP340 has evolved from supporting only "pre-hashed" 32 byte messages, to supporting messages of "any length" and as such we should allow the users to pass a message of any length.
Note that passing exactly 32 bytes will make the API behave exactly as before (ie it will produce the same signatures).
I added all the test vectors from: https://github.com/bitcoin/bips/blob/master/bip-0340/test-vectors.csv To make sure the API is correct even for empty messages and shorter/longer ones :)
ACKs for top commit:
Kixunil:
ACK df98b160d8
apoelstra:
ACK df98b160d8 thanks for all the new test vectors\!
Tree-SHA512: bd99ea8e17fcc6fd71ad39a87c7c21761f325006998a61b33b6f2abc9f892f90a4236bd25615cb34dc83214a70dcdd34ce3e7cece7d6f971c3843505356c97c5
The `no_std` test disables `std`, so unwinding is unsupported, so we use
`panic = "abort"` but the `core` library is compiled with unwind by
default which breaks the build. Xargo can handle this by recompiling
`core` with `panic = "abort"` so we use it.
Previously we had dependency problems that were resolved by resolver v2.
We want to activate it just in case it happens again but even better,
bump the edition. This was probably forgotten when other crates were
migrated.