e63c8cbcfc Add tests for ecdsa::Signature::from_compact (Shing Him Ng)
Pull request description:
Used `Signature::from_compact` on another change and saw there were no unit tests for it, so I added some. Not sure how useful they are since it's just a wrapper though but thought I'd add some real quick. Feel free to close if it's not necessary
ACKs for top commit:
apoelstra:
ACK e63c8cbcfc successfully ran local tests; nice!
Tree-SHA512: f99df1b3025737f5de4c892d8e649c0c30fa4126d04e2536da17d6caf9b4ab8ae8b0489bf6e7ddfefe0867277c7a254d8ce27bcf1dbffd23851ed31e5919cd11
ca32f7b403 improved PublicKey debug impl (Liam Aharon)
Pull request description:
Closes#729
Adjusts `Debug` impls to write serialized hex representation of the keys rather than the inner u8 bytes.
ACKs for top commit:
apoelstra:
ACK ca32f7b403 successfully ran local tests
tcharding:
ACK ca32f7b403
Tree-SHA512: e73c10733fe3b493492c16f6b2e68149339395b882c70d2d80b9b5e0d75ec671155b09a101ca019e44260413ecb82e93919f2caf0d2508b5baa641e839689909
fa0c086431 refactor: recoveryid into enum (Liam Aharon)
Pull request description:
Closes#727
- Refactors `RecoveryId` into an enum.
- Replaces custom type methods `from_i32` and `to_i32` with `TryFrom<i32>` and `Into<i32>` (via `From<RecoveryId> for i32`) implementations.
- Removes derive `Ord` `PartialOrd` and `Hash`, they don't appear to be used. I can implement on the enum if we want to keep them.
ACKs for top commit:
apoelstra:
ACK fa0c086431 successfully ran local tests
tcharding:
ACK fa0c086431
Tree-SHA512: 2b4f448c69d51ca8bf66110a46aa3a846cc47dc137b67f04643ae01a181f7208508c6af27429e26b3ee5d625c37923adc7fd20ccca701b5f5433b5a62d41a802
624fd47574 CI: Run the nightly update job at 5 past midnight (Tobin C. Harding)
Pull request description:
Currently we run the job at midnight here and in `bitcoin`, this led recently to one using the nightly toolchain from the 10th of Sep and the other using the toolchain from 11th of Sep.
Update to run at 5 past so this doesn't happen again.
ACKs for top commit:
apoelstra:
ACK 624fd47574 successfully ran local tests
Tree-SHA512: 6fe9a4e2d6c3c04c962f99611f3536a99787393214402a0de2632eb867271560744b842c633c5318281f6e40ad848d42a88bfe6a7f393d19d8601d8e67590620
Currently we run the job at midnight here and in `bitcoin`, this led
recently to one using the nightly toolchain from the 10th of Sep and the
other using the toolchain from 11th of Sep.
Update to run at 5 past so this doesn't happen again.
537b85b793 Deprecate and replace `from_slice` (Jamil Lambert, PhD)
1661f57d84 Add `from_byte_array` functions (Jamil Lambert, PhD)
Pull request description:
As brought up in issue https://github.com/rust-bitcoin/rust-bitcoin/issues/3102 support for Rust arrays is now much better so slice-accepting methods that require a fixed length can be replaced with a method that accepts an array.
`from_slice()` methods have been deprecated and calls to it from within the crate have been changed to use the equivalent array method.
ACKs for top commit:
apoelstra:
ACK 537b85b793 successfully ran local tests
tcharding:
ACK 537b85b793
Kixunil:
ACK 537b85b793
Tree-SHA512: 8f8f807af0032130b64a93ff86cae0df1ccee83de052603968be9e30751c27dfc763a6128472e6d0f3a5c2016e7da8e4d06eefc86d8310bdaacc6be0f8fe3822
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
The PrivateKey and XOnlyPublicKey from_slice functions have been
deprecated and calls to them from within the crate have been changed to
use the equivalent array function.
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.
merge-script
Shing Him Ng
Liam Aharon
Tobin C. Harding
Update Nightly Rustc Bot
Jamil Lambert, PhD
Ethan Tuttle
Yuki Kishimoto
Jose Storopoli
Martin Habovstiak