Currently we only get `std::io::Write` impls when the `bitcoin-io`
dependency is used. This is overly restrictive, it would be nice to have
`std::io::Write` imlps even without the `bitcoin-io` dependency.
Copy the logic out of the `bitcoin_io::impl_write` macro into `hashes`
but feature gate it differently.
Call the new macro inside `hash_type` (and in `hmac`), remove the
`impls` module, and move the tests to the integration test directory.
Remove the `io` feature from `hashes`, now if users enable `std` they
get `std::io::Write` impls and if they enable `bitcoin-io` they get
`bitcoin_io::Write` impls as well.
eda87517c0 Update documentation to indicate that the Display implementation in Amount is unstable (Shing Him Ng)
Pull request description:
From [this comment](https://github.com/rust-bitcoin/rust-bitcoin/issues/2954#issuecomment-2325679334) in #2954, update the documentation to indicate that the Display implementation in Amount is unstable
ACKs for top commit:
Kixunil:
ACK eda87517c0
apoelstra:
ACK eda87517c0 successfully ran local tests
Tree-SHA512: de761b41696e1f43b1a823b0d8a62ce82ec3247e260b7f2b0253f176224e47da64542889c86b9150a9d1a4e29409008b45882cb16c954cbe601d3c1a0093e3cf
d72f730211 hashes: Use $crate in internal macros (Tobin C. Harding)
Pull request description:
These are only called from within the crate but it is still more correct to use `$crate` and saves this from biting us later if we copy the code someplace else.
Internal change only.
ACKs for top commit:
Kixunil:
ACK d72f730211
apoelstra:
ACK d72f730211 successfully ran local tests
Tree-SHA512: d278643c3fbeb28ca377ebf59958054dd2893c46b48469e03a8c7517c5b0b33271de061ae662c400d45962724fe4d13cada41fd5b839a1ff784521ac69c9db72
c00afe8d52 Change MessageSignatureError to secp256k1::Error (Jamil Lambert, PhD)
a20d0bc4eb Deprecate `from_slice()` in sha256.rs (Jamil Lambert, PhD)
089043546f Deprecate `from_slice` methods in favor of arrays (Jamil Lambert, PhD)
Pull request description:
As brought up in issue #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 that require a fixed length have been deprecated and where needed a `from_byte_array()` method created that accepts an array.
There are still `from_slice` methods that rely on changes to external crates before they can be changed to arrays.
ACKs for top commit:
apoelstra:
ACK c00afe8d52 successfully ran local tests
tcharding:
ACK c00afe8d52
Kixunil:
ACK c00afe8d52
Tree-SHA512: c505b78d5ca57e7e1004df6761ac6760d5d9b63c93edc6ac1f9adf047bc67011883575f835b06f6d35d7f6c2b6a4c6c7f0a82a3f0e293bfb4ef58123b75d3809
333c8ab297 Add additional docs to Witness (Tobin C. Harding)
Pull request description:
The `Witness` struct is non-trivial, in particular it is not immediately obvious where and when the compact size encode value for each witness element is stored.
Make an effort to improve the docs on `Witness` in relation to the compact size encoded length of each witness element.
ACKs for top commit:
apoelstra:
ACK 333c8ab297 successfully ran local tests
Kixunil:
ACK 333c8ab297
Tree-SHA512: 1c61a9ad071c035d5ad2e54446120d29ebf8cc4a779c96f04eda825890687dcbd53accc17522f57ef4ffb226eb1d85c6a3a115f27bebcfc7ad3c677033a8a414
`from_byte_array` cannot error due to InvalidLength so the returned
MessageSignatureError has been changed to return a secp256k1::Error,
which is the only error type returned by the function.
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()` has been deprecated. A `from_byte_array()` function
already exists to be used instead.
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()` has been deprecated and replaced with `from_byte_array()`
e9ecc0989c Automated update to Github CI to rustc stable-1.81.0 (Update Stable Rustc Bot)
Pull request description:
Automated update to Github CI workflow `semver-checks.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
ACKs for top commit:
tcharding:
ACK e9ecc0989c
Tree-SHA512: 1643f3f04650df4f4521ac16d1941961a7e32219f63cdb3556487ed29dcebdfb69a90c4b2ee10e8ea7207af0d43ad8774d23dab4b5cd04edba8fb9129c74a659
These are only called from within the crate but it is still more correct
to use `$crate` and saves this from biting us later if we copy the code
someplace else.
Internal change only.
b60156321c Automated update to Github CI to rustc nightly-2024-09-04 (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 b60156321c
Tree-SHA512: 0d7227d52f58d09ef44882d5b1727c09c2949b5f0f6b7149bf14cf009cf55f995d9453c415280db33cb2b52e92396b59c97d0704671a70de7e6ba045ecf2bf35
The `Witness` struct is non-trivial, in particular it is not immediately
obvious where and when the compact size encode value for each witness
element is stored.
Make an effort to improve the docs on `Witness` in relation to the
compact size encoded length of each witness element.
8f5bde9f17 units: Fix typo in measured (Tobin C. Harding)
Pull request description:
Its "time is measured" not "time is measure".
ACKs for top commit:
Kixunil:
ACK 8f5bde9f17
apoelstra:
ACK 8f5bde9f17 successfully ran local tests
Tree-SHA512: 5dfb7c1def9fb9cb8b51083e2c6d22ae1fab30002802a71d93818ddbe457bcb20998d0fc7235d6953f212a801e66f7f87b01f130d58b76098716ccdd05736c9e
c48d9d6523 Move transaction::Version to primitives (Tobin C. Harding)
f490222068 Introduce the VersionExt trait (Tobin C. Harding)
fb89974b82 Run the formatter (Tobin C. Harding)
bb3a3ecbaa Introduce temporary module for Version (Tobin C. Harding)
1fde868f51 Separate Version impl blocks (Tobin C. Harding)
Pull request description:
As per title, in tiny small chunks, move the `transaction::Version` over to `primitives`. Only the type, its associated consts, and its `Display` impl are moved. The two methods are left in an extension trait.
Was originally attempted in #3253
ACKs for top commit:
Kixunil:
ACK c48d9d6523
apoelstra:
ACK c48d9d6523 successfully ran local tests
Tree-SHA512: 83415cf0762dca5c263deb743734fc7abede804a6daac31df3d0101b51c6261e6d54452eb744727ae680cacce9e4ef726a6fa253d86c4e7a5d8ec789b137566c
eccd3c4a3d Automated update to Github CI to rustc nightly-2024-09-01 (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 eccd3c4a3d
Tree-SHA512: 448936db5b2d64204535af3707189e86945c58de239bf4ef861923fe2c0b24866945efd6ec4bcfd41b27bfa23fbbcfb91bb1d86111e795caf7edefa840341c45
We would like to move the `Transaction` type to `primitives`, as a step
towards this move the `transaction::Version` and its trait imps (just
`Display`) over there.
In preparation for adding an extension trait; separate the
`transaction::Version` impl blocks into stuff that will stay here and
stuff that will go to `primitives`.
Refactor only, no logic changes.
30bb93c676 Implement impl_to_hex_from_lower_hex macro for types that implement fmt::LowerHex (Shing Him Ng)
Pull request description:
Created a macro that implements `to_hex` for types that currently have `core::fmt::LowerHex` and called it on types that have `core::fmt::LowerHex` implemented. I put the macro in the `internals` crate since there are types across the whole project that can potentially use this.
Resolves#2869
ACKs for top commit:
Kixunil:
ACK 30bb93c676
apoelstra:
ACK 30bb93c676 successfully ran local tests
Tree-SHA512: d3ebc7b5c0c23f1a8f8eef4379c1b475e8c23845e18ce514cb1e98eb63fc4f215e6bc4425f97c7303053df13374ef931ae9d9373badd7ca1975a55b0d00d0e40
90e073ef3e Set CRATES mechanically (Tobin C. Harding)
Pull request description:
Instead of manually setting the crates list it is less error prone to do so mechanically. This required some changes to the `run_task` script which have now merged, so we update to use the new commit hash at the same time.
- Use shell to set the `CRATES` env var used by `run_task.sh`.
- Use latest revision of rust-bitcoin-maintainer-tools
c71b23d81d Remove bool from cont_assert (Tobin C. Harding)
Pull request description:
It was correctly pointed out during review of #3215 (when we made `const_assert` panic) that using a `bool` added no additional information.
Remove the `bool` and just use unit.
ACKs for top commit:
apoelstra:
ACK c71b23d81d successfully ran local tests; lol sure
Kixunil:
ACK c71b23d81d
Tree-SHA512: be9f4f10ee7d626a082b7ae9f257b79d500824ed3c1f7327391b2ad4d67e60d7da47a14fa7ef8f99d1ea8157967b4658518cbcf1c1bfcf1d8888455f3eb96437
b6371b5801 Fix clippy rustdocs warnings (Tobin C. Harding)
Pull request description:
A new nightly version (`nightly-2024-08-28`) introduces a few warnings because of our rustdocs. These are valid warnings and should be fixed, thanks `clippy` team.
(The `bip152` change is a bit sloppy, open to suggestions.)
ACKs for top commit:
apoelstra:
ACK b6371b5801 successfully ran local tests
Kixunil:
ACK b6371b5801
Tree-SHA512: 503fb9d48772b74a5acdb26c0f77a85c52323c03360f983204fccee0f28bedeff142237b067caa1ce6ea04ea9842cc493e0d06dc141ca00a98151fa002b62392
be13397570 Make hmac & hkdf more robust against buggy `Hash` (Martin Habovstiak)
94c0614bda Enforce that `Hash::Bytes` is an array (Martin Habovstiak)
Pull request description:
This makes sure `Hash::Bytes` is an array. We've discussed this somewhere but I don't remember where.
I'm not sure if the second commit is actually valuable but hopefully shouldn't make things worse.
ACKs for top commit:
apoelstra:
ACK be13397570 successfully ran local tests; yep, this looks like an improvement. Agreed that the second commit has questionable value but doe not make things worse
tcharding:
ACK be13397570
Tree-SHA512: 0fed982084f0f98927c2b4a275cec81cb4bbc0efbf01551a0a4a8b6b39a4504830243ee8d55a5c0418d81b5d4babc7b22332dbacc0609ced8fada84d2961ae71
345d3daa72 fix: deprecate wrong and unused max script num (ChrisCho-H)
Pull request description:
~~Script number can be up to 2^39 - 1 to encode locktime.~~
~~If it's only for the integer operation besides locktime, it must be 2^31 - 1, not 2^31.~~
I agree with apoelstra opinion to deprecate this value.
ACKs for top commit:
apoelstra:
ACK 345d3daa72 successfully ran local tests
Kixunil:
ACK 345d3daa72
Tree-SHA512: df4205415634d8db1412f16a75d1dbc110ed69930cefa085bdaa268a7fcaf47776dd1d8ed6965e6bbb476825b5f80e5c00bb375730ad102bae8868abe5894068
Instead of manually setting the crates list it is less error prone to do
so mechanically. This required some changes to the `run_task` script
which have now merged, so we update to use the new commit hash at the
same time.
- Use shell to set the `CRATES` env var used by `run_task.sh`.
- Use latest revision of rust-bitcoin-maintainer-tools
It was correctly pointed out during review of #3215 (when we made
`const_assert` panic) that using a `bool` added no additional
information.
Remove the `bool` and just use unit.
7ecb8dfa98 Automated update to Github CI to cargo-semver-checks version-0.34.0 (Update cargo-semver-checks Bot)
Pull request description:
Automated update to Github CI workflow `semver-checks.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
ACKs for top commit:
tcharding:
ACK 7ecb8dfa98
Tree-SHA512: b21c513ae0bfceee62f8415402f69347c10b25289246b1d9c742b7385871d1fbf526e7a56b41c83c1fc9b4152e8f1c257a7b2f41ab7d0cb71cedf6c358ea4dc2
fa71b0e044 2024-09-01 automated rustfmt nightly (Fmt Bot)
Pull request description:
Automated nightly `rustfmt` changes by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
ACKs for top commit:
apoelstra:
ACK fa71b0e044 successfully ran local tests; a bit big this week
Tree-SHA512: 875e8d54a06a22ccbef690a21f9d48a0acc1e6ea073a2d16632dd7ce63400cc7f2522339b7cb808653dade407517cd03d4fece4c8dec24abacf5303418aeaa75
9db6234ea9 Show compressed public key in Debug for CompressedPublicKey (Jiri Jakes)
Pull request description:
Currently `CompressedPublicKey` debug produces output of form:
```
CompressedPublicKey(PublicKey(2f8b18dc0adcb73d75f7934d9523ea7347083e41c48115398cb37e295a0a6ffe86e0bf8b1ef65888c880c8d8813a30e69e466380cbe2daec18f3ed1e7a553ff2))
```
Although it shows real internal structure together with inner uncompressed public key, it is not too helpful for the purpose of debugging _compressed_ public key.
After this patch, `Debug` output will be equal to `Display` (it, in fact, delegates rendering to `Display`), prepended by the name of the struct:
```
CompressedPublicKey(02fe6f0a5a297eb38c391581c4413e084773ea23954d93f7753db7dc0adc188b2f)
```
ACKs for top commit:
Kixunil:
ACK 9db6234ea9 unless other maintainers agree to drop the struct name.
apoelstra:
ACK 9db6234ea9 successfully ran local tests; we should keep the struct name since it is canonical for Debug output
Tree-SHA512: e2626678a4144357d3ff4ddd888459ae1283ea50bc7c8ef86deb9902d7d543de79109ac42883e5538f7e2f6a29426224bb43eb44a34f366821e47a9aca563753
3c7c8c44b6 Improve const_assert (Tobin C. Harding)
Pull request description:
Now that we can panic in const context we can improve the `const_assert` macro by adding a message string.
Original idea by Kix:
https://github.com/rust-bitcoin/rust-bitcoin/pull/2972#discussion_r1726328228
ACKs for top commit:
Kixunil:
ACK 3c7c8c44b6 in the sense that it does what it's supposed to with the only tiny issue being that the `bool` looks weird but not broken in any other way I can think of.
apoelstra:
ACK 3c7c8c44b6 successfully ran local tests
Tree-SHA512: 5ff721c0056f87d42c934818da6f780cd945f235291eb4b044752d67405a74f992d7f85853fec129e794ec3fcda1f319cc40daabc6a349d21bbdc977640d2572
a184066660 Move import inside feature gate (Tobin C. Harding)
Pull request description:
The `String` type is only used if the `serde` feature is enabled, move the import statement inside the already feature gated block.
ACKs for top commit:
apoelstra:
ACK a184066660 successfully ran local tests; sure
Kixunil:
ACK a184066660
Tree-SHA512: 688adb1e4489b0242856ac36ed9cf3503b147b84954cf1e6fd34b5e708ed6c1dd92da91e739243face1c1b4e4c8b19ce88e215117bef2a7af2e732859a6e108a
3f244db19d Add Arbitrary to SignedAmount type (yancy)
Pull request description:
While proptesting, I've found Arbitrary for SingedAmount to be useful. Would appreciate adding this as well.
ACKs for top commit:
tcharding:
ACK 3f244db19d
Kixunil:
ACK 3f244db19d
Tree-SHA512: f293215505db85998135c75e147f4cb0031a08c7db619b706e6c7d632c6f23783af56992531396aecc6e3da55b107a7f2c7ec4d68a3e5737ebcf4225f306ed29
4024ba5910 Add unit test for ArrayVec::extend_from_slice (Tobin C. Harding)
798c9cff1c Fix bug in ArrayVec::extend_from_slice (Tobin C. Harding)
Pull request description:
Currently the source slice must be the exact length to fill the array to max capacity, this is an unnecessary restriction since an `ArrayVec` is a variable sized data structure.
Set the destination slice to be the same length as the source slice (still maintain capacity checks).
ACKs for top commit:
Kixunil:
ACK 4024ba5910
apoelstra:
ACK 4024ba5910 successfully ran local tests
Tree-SHA512: b7a510ba6f6e5cf5a535d691f79da98e29558eb9a50b60d41e080769560e9b9f0b8a831806dc9f159c888001951e17b5bbd970363d6a2e57fa9748902548b3ab
Use the newly added requirement that `Hash::Bytes` is an array to
protect the implementation of hmac and hkdf against implementations that
would accidentally return slices of different sizes from the `AsRef`
impl.
In the future we would like to guarantee the correctness of `LEN` which
is currently not entirely possible, so this at least adds a sealed trait
enforcing the `Bytes` type to be an array. Consumers concerned about the
validity of the length can access the `LEN` constant on `Bytes` instead
to get the correct length of the array.