f61e93ccf1 Properly deprecate Hash::from_slice (Tobin C. Harding)
50c0af7138 Stop using Hash::from_slice (Tobin C. Harding)
Pull request description:
The `hashes::error::FromSliceError` error is only returned from `from_slice`. We attempted to deprecate this function but it seems we only did half a job at it.
- deprecate _all_ instances of the method/function
- deprecate the error type
- stop using the deprecated functions in `bitcoin`
Close: #4053
ACKs for top commit:
apoelstra:
ACK f61e93ccf1db7e7e3c9604fdb09b4e25195d88b2; successfully ran local tests
Tree-SHA512: 61a0e5127019859776ffac66bd4d320c86b8462bb1e908127d0bf42896aaa8df85fd2b06850342b694ca1cd68ed50355c81cad6ae3e9a5fd6e3933efe85498ad
cdece244d5 Update to latest maintainer-tools (Tobin C. Harding)
Pull request description:
We have made a couple of improvements to the `run_task` script lately (*cough* actually test code with `--all-features`).
Lets update to use it.
This PR explicitly tests https://github.com/rust-bitcoin/rust-bitcoin-maintainer-tools/pull/20 because it uses the commit hash created in that PR.
ACKs for top commit:
apoelstra:
ACK cdece244d5b660c658461e58093090d982e4835e; successfully ran local tests; will one-ack merge this since it is trivial
Tree-SHA512: 97597d01beb3b0a87fb0c279ba0e57093f4898a38e7373040cfa18b16c62f9cbceb41ac53429a2e05f70f2ea9fdd7d409e9925131575b3f66d00c5395ba6bfc1
4259dab93a Remove rust-ordered dependency (Tobin C. Harding)
d392cdbd7d Remove PartialOrd from locktimes (Tobin C. Harding)
Pull request description:
These two things are related so we remove them both in the same PR. Please see commit logs for full explanation.
For more context see discussion below and also:
- #2500
- #4002
- #3881Close: #4029
ACKs for top commit:
Kixunil:
ACK 4259dab93a
apoelstra:
ACK 4259dab93ab52795305db4b889b9151595bcee51; successfully ran local tests
Tree-SHA512: 7526d4faaa9edf8017d2af412c41a33f33d851ad5130c9a745bba86d9c71dc1db7f20d07377aaf3a25fec2c0de79f3ffabc2c538a5a366e415c7a6eaa730153c
da8b85ed7c Implement Debug for Hkdf (Tobin C. Harding)
85652359e8 hashes: Derive Copy and Clone for Hkdf (Tobin C. Harding)
Pull request description:
Currently the `Hkdf` type does not derive any traits.
Derive `Copy` and `Clone` and implement `Debug` based on secret obfuscation algo in `rust-secp` (in the `secret` module).
ACKs for top commit:
apoelstra:
ACK da8b85ed7cf34c0510c0b64c67477d3819bee369; successfully ran local tests
Kixunil:
ACK da8b85ed7c
Tree-SHA512: 8ae0e8857ea0e32ad5ef8f544979eeb9d530beb1b6f046ce28a286ca2231f8f696a9f4f8d9ea219d3389c4216d6b69766dbd96edbb27e7489803ac583bf3b200
The `hashes::error::FromSliceError` error is only returned from
`from_slice`. We attempted to deprecate this function but it seems we
only did half a job at it.
- deprecate _all_ instances of the method/function
- deprecate the error type
It has turned out that the `rust-ordered` crate and it's
`ArbitraryOrd` trait are only useful for locktimes and only marginally
useful for them at best.
Remove the `ArbitraryOrd` impls and the `rust-ordered` dependency.
This topic was discussed in various places including:
- #2500
- #4002
- #3881Close: #4029
Currently it is possible to write
```rust
if this_locktime < that_locktime {
do_this();
}
```
with the hope that this code means if a locktime is satisfied by the
value in the other locktime. This is a footgun because locktimes are
incommensurate.
We provide the `is_satisfied_by` API to help users do locktime
comparisons.
Remove the `PartialOrd` implementation from both locktime types.
8d8edd2c77 make Debug representation of Witness to be slice of hex-encoded bytes strings (Erick Cestari)
Pull request description:
This PR updates the Debug implementation for the Witness type to improve its readability by displaying the witness data as a slice of hex-encoded strings rather than a concatenated blob or list of raw u8 values. The changes include:
- Improved Output:
The debug output now shows pseudo-fields such as the number of elements and the total length of all elements, making it easier to understand the underlying data without exposing internal indices like indices_start.
- Hex-Encoding:
Each witness element is displayed as a hex-encoded string, similar to Bitcoin Core's output style, which enhances clarity during debugging sessions.
These changes should provide a more developer-friendly view of the witness data and align with similar patterns used elsewhere in the ecosystem.
Closes#4023.
Example display:
```
Witness {
num_elements: 3,
total_bytes: 5,
elements: [
0b,
1516,
1f20,
],
}
```
```
Witness { num_elements: 3, total_bytes: 5, elements: [0b, 1516, 1f20] }
```
ACKs for top commit:
tcharding:
ACK 8d8edd2c77
Kixunil:
ACK 8d8edd2c77
apoelstra:
ACK 8d8edd2c77de9b0423533fc70802171803761fcd; successfully ran local tests
Tree-SHA512: ffcdf67542049f405317eecd74876b51972d27ec552eec8e9c7b6324f18f31f4721fc4d2be1e596232c39af90a8d169c082f9b0636e5aa1a80fe1b063d645456
914730d7f1 Add inline to small functions in result module (Jamil Lambert, PhD)
Pull request description:
Close#4064
ACKs for top commit:
Kixunil:
ACK 914730d7f1
apoelstra:
ACK 914730d7f1cb5e032cce155440bbe4f064a09533; successfully ran local tests
Tree-SHA512: eb6d743df462462a048686f803d687756b230fdbd6b20c3bccd210f839a37dae1a390ff732efd0686f1b1df36d771b8147c5da87010025af3b1964b5b2c65751
8c2439550a hashes: Test macros in function scope (Tobin C. Harding)
Pull request description:
The two main public macros can be used in function scope - prove it.
While we are at it prove that additional attributes are supported by them both as well as visability keywords.
ACKs for top commit:
apoelstra:
ACK 8c2439550acdf3cb52c78d69eb55160f9d03a139; successfully ran local tests
Kixunil:
ACK 8c2439550a
Tree-SHA512: 66d6a9fd142966f6e68e9a5cb849345a77357dc6e415a10507418425a4cf6a1440deaf47515db43b9ec5b5d337d53164c617db165cb2ff782cf6a6e4ff195c77
This function is deprecated, stop using it in favour of
`Hash::from_byte_array`.
Patch only touches test code, I'm guessing that is why lint warnings
were no showing up.
Currently the `Hkdf` type does not derive any traits.
We would like to derive the common set of traits but there are a bunch
reasons we can't;
- Don't want to leak secrets in `Debug`.
- Don't want to enable timing attacks with Eq/Ord and friends.
For now just derive `Copy` and `Clone`. We will then implement `Debug`
manually.
f433b36ee1 Automated update to Github CI to rustc nightly-2025-02-14 (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 f433b36ee1
Tree-SHA512: e3e97e4a5aa924e8020c7d0909e541fa3507a10e49045db8bb1b03986e422c10dfd8cfe4e32da3fa3e076bb6b8c19ac306a9ad5cb4ba46a0a7122e58d135666e
The two main public macros can be used in function scope - prove it.
While we are at it prove that additional attributes are supported by
them both as well as visability keywords.
f99847d6ec Do not enable `soft-float` when running `miri` (Martin Habovstiak)
Pull request description:
To ensure that all target features are tested we simply enable all of them. However some of them are problematic. We already have an exception to not enable `crt-static`. This change also adds `soft-float` to the list because `miri` was warning about it being UB.
ACKs for top commit:
apoelstra:
ACK f99847d6eca0977d6f987baa7846c62a0613ba35; successfully ran local tests; thanks for digging into this!
tcharding:
ACK f99847d6ec
Tree-SHA512: c016179bca1fafae22f5229b232fe190e22942c9e71b719bfcafc33ab269c52f5ff5a834b194974b8f0e6ab4e0ba93aa226231ce0f2618a6c6221e363cd3c333
6244cb75fa Introduce monadic AmountOpResult (Tobin C. Harding)
Pull request description:
We would like to return an error when doing math ops on amount types. We cannot however use the stdlib `Result` or `Option` because we want to implement ops on the result type.
Add an `NumOpResult` type. Return this type from all math operations on `Amount` and `SignedAmount`.
Implement `core::iter::Sum` for the new type to allow summing iterators of amounts.
Note please this removes `AddAssign` impls for amount types.
ACKs for top commit:
apoelstra:
ACK 6244cb75faf62aed4b47d63a59d14cb766e4e7a8; successfully ran local tests; let's do it -- but definitely want the followup issues addressed
Kixunil:
ACK 6244cb75fa
Tree-SHA512: 7a105acb1aa492ab3e97d94ae182ac4c30a364edd183f71cc320cf80d85060049e8caf1e5736ef6d1af32f39c3376f21382afe35ac65ea1b8c15130c622d9d64
7c12d92bc3 Only enable hex/std, alloc when hex is (Jamil Lambert, PhD)
11770cac1c Add hashes to io dev-depencies and require hex (Jamil Lambert, PhD)
e7c6564d38 Add missing hex feature gate (Jamil Lambert, PhD)
Pull request description:
`hex/std` and `hex/alloc` should only be included if optional dependency `hex` is enabled. A bunch of tests that need `hex` relied on an `alloc` feature gate to ensure `hex/alloc` was enabled.
Add feature gates to the tests that require `hex`.
Add `?` so `hex/alloc` or `hex/std` are only included if the optional feature `hex` is enabled.
Audit rest of crates `Cargo.toml` files: no other cases found.
Close#4035
ACKs for top commit:
Kixunil:
ACK 7c12d92bc3
apoelstra:
ACK 7c12d92bc3011e1305238f0f5d3b155a01875814; successfully ran local tests; Played with cargo a bit; `cargo tree` seems to confirm that you can mix dev and non-dev dependencies like this.
Tree-SHA512: bd752d3c074dd8c93b09aa318d98ccb761b2f9baa2797566dd1d5f486349a92fbcd05787ddeb53d1f05ef0b29fbf559152f6373674fbec01e1251e74ad61e61f
ab2f709181 Implement Default for Script (jrakibi)
Pull request description:
This PR implements Default for `Script` to return an empty script.
*Note: ScriptBuf already has `#[derive(Default)]` in the code so it's already handled*
Resolves#3735
ACKs for top commit:
tcharding:
ACK ab2f709181
Kixunil:
ACK ab2f709181
apoelstra:
ACK ab2f7091814333b20669d41f1f78e0e52795df08; successfully ran local tests; neat!
Tree-SHA512: c06ba98d9bf8568e323ef9082a7f06756586360d6bef2b93721db7f6e28a777852e494c86319c97b0fd5444a0010d6c679625753534c0e1c8116e452ce8fa9cc
6cecc40ae4 Test LockTime PartialOrd (Jamil Lambert, PhD)
Pull request description:
Add tests to kill the mutants in both relative and absolute PartialOrd.
ACKs for top commit:
tcharding:
ACK 6cecc40ae4
apoelstra:
ACK 6cecc40ae4d52b711f58998315155bc8c6b19d7b; successfully ran local tests; thanks!
Tree-SHA512: dba7d90e3f6e62f0d3417bacc09d38145dd29bf654f84c2d3bc68af30c0e65b105146466a384bd35ef4326913ca414fd31f92daa3d7ffe3ff409c49bd1c05d96
43a7c66f50 units: Remove alloc from fee module (jrakibi)
Pull request description:
This PR removes the `alloc` feature gating from fee module
Closes#3815
ACKs for top commit:
tcharding:
ACK 43a7c66f50
apoelstra:
ACK 43a7c66f50b663aee503c958c5158127fa0b8d5c; successfully ran local tests; nice!
Tree-SHA512: 645d50cd6cde915972a576d7282a5dfc9aa27a8c3a3b44d3f3eb7a7f066cb3a697bed7e757bc86766498d92cc534607960caf20c90a1ac6fabf9246db4b30249
854a4cf511 Kill mutant in checked_mul_by_fee_rate (Jamil Lambert, PhD)
44df39e72c Skip deprecated functions in mutants.toml (Jamil Lambert, PhD)
Pull request description:
Recent changes to units created new mutants.
Skip mutants from deprecated functions.
Modify existing test to kill `checked_mul_by_fee_rate` mutant.
Close#4026
ACKs for top commit:
tcharding:
ACK 854a4cf511
apoelstra:
ACK 854a4cf51189b4be596c4f048420fc9b1212d714; successfully ran local tests
Tree-SHA512: 62343984d16d7a6f2287454a21c4ec5333387c940ce97819c2610da08f709ba3e806e024dc2f352e309b4ed5555c13e7b02fdf0c41aae5961f0e0c3c35d049a3
00cd247bc4 Validate compressed WIF keys (ndungudedan)
Pull request description:
For private WIF keys corresponding to a compressed address, the last byte of the key needs to be 0x01, but the API doesn't enforce this when using PrivateKey::from_wif(). So, invalid keys can be accepted.
Thus we check if the last byte is equivalent to 0x01 if the key's length is 34 (which indicates it's
compressed).
resolves#3773
ACKs for top commit:
Kixunil:
ACK 00cd247bc4
tcharding:
ACK 00cd247bc4
apoelstra:
ACK 00cd247bc49451fe8c2cbf537ed042b0bb5c3340; successfully ran local tests
Tree-SHA512: 8d2cfc13f713dd4ae47d4f55d87f783167b5b3755f4f902cd009ebe0c5da19b0d3bf31393570a804e22010685bf97381922d21c53a71a9c144aa53ff7aac2167
`hex/std` and `hex/alloc` should only be included if optional
dependency `hex` is enabled`.
Add `?` so it is only included if the optional feature `hex` is enabled.
The tests in `hash` require hex. Previously this was enabled when
`alloc` was enabled. Now `alloc` doesn't enable `hex` add `hashes` to
`dev-dependecies` and set it to require `hex`
To ensure that all target features are tested we simply enable all of
them. However some of them are problematic. We already have an exception
to not enable `crt-static`. This change also adds `soft-float` to the
list because `miri` was warning about it being UB.
7e66091e1e Add from impl tests (Jamil Lambert, PhD)
2f95064cfd Add from_parts test (Jamil Lambert, PhD)
3ee66c5bb8 Modify push test (Jamil Lambert, PhD)
Pull request description:
Add tests to kill the mutants in `primitives/src/witness.rs`
ACKs for top commit:
tcharding:
ACK 7e66091e1e
apoelstra:
ACK 7e66091e1e8b6cdd3e40d001ea1824125f7175e7; successfully ran local tests
Tree-SHA512: 57b2b0e4dbd93023d1a6a9709a02fa843e3ef9b25e7293ad641726b9c335e220a4ed87b717ec5dda999217677a916b86ac7daa9aaaec077afbfee4789836344e
435750f292 Add a parse_vout test (Jamil Lambert, PhD)
957be3c978 Add OutPoint test (Jamil Lambert, PhD)
a4ef027134 Add tests of transaction functions (Jamil Lambert, PhD)
Pull request description:
Add tests to kill the mutants in `primitives/src/transaction.rs`
ACKs for top commit:
tcharding:
ACK 435750f292
Tree-SHA512: 78baf40ad6ed1cd5b3a33346b4702c3a7efbb03ae6eec9a802d3dea99910373cf4e8053c73e9fdc02ce54852d3ee43253f2ff0c149c86870ecfed2fc909e5bcf
For private WIF keys corresponding to a compressed address,
the last byte of the key needs to be 0x01, but the API
doesn't enforce this when using PrivateKey::from_wif(). So,
invalid keys can be accepted.
Thus we check if the last byte is equivalent to 0x01
if the key's length is 34 (which indicates it's
compressed).
3c12d4eb1f Add primitives to mutants.toml (Jamil Lambert, PhD)
32b05132b5 Reorganize mutants.toml (Jamil Lambert, PhD)
Pull request description:
All of the mutants found by `cargo mutants` in primitives have been killed in: #3948, #3971, #3987, #4012, #4019, #4020, #4024, #4033, #4036, #4037.
Add `deserialize` as a general exception. The mutant is changing the return value to Ok(Default::default()), and a test to kill it doesn't prove anything other than the function can return something.
Reword comments to make them clearer.
Add the required exceptions for cases in primitives that need to be skipped.
Add primitives to the examine paths.
ACKs for top commit:
tcharding:
ACK 3c12d4eb1f
apoelstra:
ACK 3c12d4eb1fcdfc8d7569ef3f4024811017abd15c; successfully ran local tests
Tree-SHA512: 8848164bfea8f44c49c3055d3c6daa9bbc6d65de86b36b1f2022e6cb872602ee878a1dcf62c10da615b26c906d97a606ab4b96cf12d1a2bced2984ec03cc6d67
0acd2b5e89 primitives: store transaction::Version as u32 instead of i32 (Andrew Toth)
Pull request description:
Closes#4039.
ACKs for top commit:
Kixunil:
ACK 0acd2b5e89
apoelstra:
ACK 0acd2b5e894246d0b0cf7f78bb820a8940c41c26; successfully ran local tests
Tree-SHA512: 8e207c77eec3db3ab85de842ab03ac8692ac66f0359121fbc736068b8f7ce66ef3049ad0be53a534cba08d32ea62e76a624ca2713dd5b8b6f115fd51826a81ce
fd4586eaae Invert dependency between io and hashes (Tobin C. Harding)
9833ca32ce primitives: Do not depend on bitcoin_io (Tobin C. Harding)
Pull request description:
Currently in order to release `hashes v1.0` we need to 1.0 `io` as well. For multiple reasons, many out of our control, the `io` crate may not stabalise any time soon.
Instead we can invert the dependency between the two crates.
This is an ingenious idea, props to Kixunil for coming up with it.
Notes
- This work highlights that we cannot call `hash_reader` on a siphash.
- We cannot implement `Hmac::hash_reader` without silently ignoring the key - this would be a footgun
- This work also highlights that our current `hash_reader` code is not well tested.
ACKs for top commit:
Kixunil:
ACK fd4586eaae
apoelstra:
ACK fd4586eaae22cafb5a04cdc55919a4ec4aa6151a; successfully ran local tests
Tree-SHA512: b3fbaf800142cd9e3cc9612e10999b222d5c81556129b6e89b05b36866b551cfb3303968b2554908955bad3ff122b22ebe2098660ead8ba7d6b6c26432e37544
Cargo mutants found mutants in witness.
Add to the existing test `push` to kill the mutants from `is_empty` and
`third_to_last`.
Change the dummy values to make the progression of `elements` down the
test easier to follow.