18619a6d0b api: Run just check-api (Tobin C. Harding)
1eb8f1f9e0 Add a Hmac::engine function (Tobin C. Harding)
c352d376ed Do not implement Default for HmacEngine (Tobin C. Harding)
Pull request description:
The `HmacEngine` should be created using a key. Currently we are providing a `Default` impl that uses `&[]` as the key. This is, I believe, a hangover from when we had a `Default` trait bound somewhere else. It is incorrect and an API footgun - remove it.
Note this PR includes changes to the bench code in `hmac` that highlights the footgun - pity the poor user we even shot ourselves.
Patch 2 adds a constructor `Hmac::engine` and uses it in the bench code.
ACKs for top commit:
Kixunil:
ACK 18619a6d0b
apoelstra:
ACK 18619a6d0b0bca7b7e3603e260b254b4aae6cebf; successfully ran local tests
Tree-SHA512: c96640e7ffba52d5b13b76a6dd9e1381788efcf56ee76300c5111541466bab8018d2546bcecf237c42dbd82e9372a0e43e1ecec37147508e879365d92a4c1451
53bcdefba5 api: Run just check-api (Tobin C. Harding)
dd2df2bf10 Delete `TxOut::NULL` (Martin Habovstiak)
a9ffb1571c Stop using `TxOut::NULL` in tests (Martin Habovstiak)
313406d6ab Optimize `encode_signing_data_to_inner` (Martin Habovstiak)
Pull request description:
This removes `TxOut::NULL` from our API and replaces the very few occurrences in our code. The PR has three commits so that the first one provably doesn't break anything, and the second one could be dropped if anyone complains about not deprecating but I really hope that won't happen.
As a side effect this also improves signing performance.
Closes#3975
ACKs for top commit:
tcharding:
ACK 53bcdefba5
apoelstra:
ACK 53bcdefba5f60878cc7049d6e157e21e985bb72c; successfully ran local tests; thanks!! No need to deprecate. Nobody was using this thing except maybe as a test dummy value
Tree-SHA512: c603512c2df6b43f6bbc107e5a40f64fa6a5f48a96e192cf2304179e3fd76925a8b3c5f03cd64f4c684ee086fd6abdcebfcf9e5785f0c95b5df9b173f8050e7a
5134025180 api: Run just check-api (Tobin C. Harding)
9b81a8a2ed hashes: Remove sha256t::Hash Default impl (Tobin C. Harding)
Pull request description:
The other hash types do not implement Default but the tagged one does still - bad bitcoin devs, no biscuit.
ACKs for top commit:
Kixunil:
ACK 5134025180
apoelstra:
ACK 513402518016bde0d55ec66f8be573bd3e6209ee; successfully ran local tests
Tree-SHA512: 15a98a42e4e06b9d66c1c07a6eda7d013026700e227b6664a462a035c3ffae0ca034d8b0bfd2a94aecd28f2fb73bfead8f88e29934e6d571e93b385e09f05236
The `HmacEngine` cannot be constructed by way of the `GeneralHash` trait
method because it requires a key. However we can still add an inherent
function to the type to construct an engine.
Add the engine constructor and use it in bench code.
The `HmacEngine` should be created using a key. Currently we are
providing a `Default` impl that uses `&[]` as the key. This is, I
believe, a hangover from when we had a `Default` trait bound somewhere
else. It is incorrect and an API footgun - remove it.
We've stopped using `TxOut::NULL` in the code and we want to restrict
`Amount` to 21M BTC, so we are now deleting this constant without
deprecation. Deprecation can be backported if needed.
We want to get rid of this constant, so we replace it in tests with 0
amount, empty script. Notably, the tests were already using it as a
dummy value - the exact amount was irrelevant, so this change doesn't
break anything.
The `encode_signing_data_to_inner` function previously constructed a
transaction internally, requiring a bunch of allocations, which it'd
then consensus-serialize into a writer (hasher). It also used a dummy
`TxOut::NULL` value which we want to get rid of.
To get rid of both allocations and the NULL value we serialize the
transaction on-the-fly. Because the encoding doesn't involve witnesses
it's not too complicated and the consensus encoding will never change so
there are no desync bugs possible. We may later change this to an
abstract transaction though.
6cde537d9b Add `ScriptBuf` tests (Jamil Lambert, PhD)
566a6e5da6 Add `Script` tests (Jamil Lambert, PhD)
Pull request description:
Add tests to both `primitives/src/script/borrowed.rs` and `primitives/src/script/owned.rs` to kill the mutants found by running `cargo mutants`.
ACKs for top commit:
tcharding:
ACK 6cde537d9b
apoelstra:
ACK 6cde537d9ba9bd56495ca47afb3f9a560e9b4358; successfully ran local tests
Kixunil:
ACK 6cde537d9b
Tree-SHA512: fc36c1e9249753ebcb0f72e8195c85a7eccd3a0b8b3e4e753102d405494e1f72cd1cdfb6f12af41a9aa6f2ff10142b95827039fa428997b96510a5e262007b25
cb5ffde9ee Change `#[must_use]` to be the same as stdlib (Jamil Lambert, PhD)
Pull request description:
Change the iterator `#[must_use]` message to be the same as stdlib uses.
Message taken from https://doc.rust-lang.org/src/core/iter/traits/iterator.rs.html#38Close#3962
ACKs for top commit:
Kixunil:
ACK cb5ffde9ee
tcharding:
ACK cb5ffde9ee
Tree-SHA512: 4866d9bddccdc28e2ba2b15cb4041851abb1512cea0317f20d2e2306df6dcd1addb828d61a84a2083d3ef47abb5cf9276e857cf098de2d28b4706ecd0a1f24fa
bc8a378187 Rename `day_` variables to `time_` (Jamil Lambert, PhD)
922392268f Combine satisfied by into singe tests (Jamil Lambert, PhD)
33a7a35bbb Make naming consistent in tests (Jamil Lambert, PhD)
Pull request description:
As mentioned in https://github.com/rust-bitcoin/rust-bitcoin/pull/3948#discussion_r1927690822 the variable naming in the tests in both `relative::LockTime` and `absolute::LockTime` is inconsistent and possibly confusing.
Make variable names consistent.
In `absolute::LockTime` incorporate the two standalone tests `satisfied_by_same` into `satisfied_by` to simplify the tests and conform to what is done in `relative::LockTime` .
ACKs for top commit:
tcharding:
ACK bc8a378187
apoelstra:
ACK bc8a378187b9fade96223c8ae9578b78f941f449; successfully ran local tests
Tree-SHA512: 643498e05a9fce16d2880c1fdd39422e239bfc60b959d9207d704e9d1de7bc6508761da602bc53217b74c8b7f1986f6d5a0f769507918235ddba72d5f77ca15f
fe09896c7d Update `README.md` (Jamil Lambert, PhD)
Pull request description:
`.github/workflows/README.md` is stale
Add the new `API` check and reorder jobs to match `rust.yml`.
Close#3942
ACKs for top commit:
Kixunil:
ACK fe09896c7d
tcharding:
ACK fe09896c7d
apoelstra:
ACK fe09896c7d87a0ae6a1b6d6c2525bd5fdff4f3ea; successfully ran local tests
Tree-SHA512: bbd9c57f20022a1383541ce574adc2db3cf02994b71749543ad080ecb585fae7c3577a4d832351948f749edebb5525da50ff79ba6354e1510e83081ba36481f3
75303b331e Automated update to Github CI to rustc nightly-2025-01-24 (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 75303b331e
Tree-SHA512: 81cd6635d412697256c01a6b149631b02555847ceda359823ae20dd78393c9e9d129bc29ceba3e95c4c9e0af4a874419e148db13b828d81f7ae89282327ca41a
71fab82a1b io: Improve docs on macro (Tobin C. Harding)
9c81d5e747 io: Move macro_export below docs (Tobin C. Harding)
b109177e11 io: Improve rustdocs (Tobin C. Harding)
07d8703a00 io: Add SPDX identifier (Tobin C. Harding)
b844637935 io: Remove rustdoc for private module (Tobin C. Harding)
Pull request description:
As part of #3643 improve the public documentation on the `io` crate.
While doing this I also checked:
- C-FAILURE
- C-LINK
- C-METADATA
- C-RELNOTES
- C-HIDDEN
ACKs for top commit:
apoelstra:
ACK 71fab82a1b32e6077adb896be9d8ac12d5513e0a; successfully ran local tests
Tree-SHA512: 91b8807102277fb7f6602837b7d0e64e4276c9c5bf748ab875ea0e4f1f7f91bc989413acd25e4412d72d6f3744a22b746ed63cbac20d9b42217cd3164c7e6847
5f75bfaa63 Improve examples on Denomination (Tobin C. Harding)
Pull request description:
Reduce the noise in the examples.
ACKs for top commit:
apoelstra:
ACK 5f75bfaa63309c7526136d430ca8092197ab7c8e; successfully ran local tests; yeah, agreed, this is nicer to read
Tree-SHA512: 01c5863f8712a8ca3b38d3f96be9d08078ca28d8cfc3dd8e8528c388e5f82406a0d43def552b7b53f034c9bf440f7d2d0fec6a760cf69a245b109d0ce4e288c3
4dad4730a8 Add unreleased changelog entry (Tobin C. Harding)
08bb57e499 Use MAX_MONEY in serde regression test (Tobin C. Harding)
Pull request description:
We plan on enforcing MAX_MONEY in the amount types. In preparation use MAX_MONEY in the serde regression test instead of the arbitrary, and too large DEADBEEFCAFEBABE value.
ACKs for top commit:
apoelstra:
ACK 4dad4730a81f9f08a565f196e5699f054e28f977; successfully ran local tests
Tree-SHA512: 10fefdb75289f4c93537e031410eb729420729563b262d6104948a0176a78c4f6130956b550cc08ffc924b1bce1c42f0544542879a4d44165022196ef695718a
13a3f490b8 Use Self instead of amount type (Tobin C. Harding)
34e3049ae0 Use sats instead of satoshi (Tobin C. Harding)
00b71a670f Use from_sat_unchecked for hardcoded ints (Tobin C. Harding)
8fdec67f7d Change local var ua to sat (Tobin C. Harding)
c6f056672b Change local var sa to ssat (Tobin C. Harding)
f3e853e07a units: Do trivial refactor of amount::tests (Tobin C. Harding)
dbec9807f9 Shorten identifiers by removing _in_sats (Tobin C. Harding)
154a4420fc Stop using FQP on Amount type (Tobin C. Harding)
8e16a48252 Run the formatter (Tobin C. Harding)
Pull request description:
Do a bunch of refactorings to tease out changes from #3794.
The first 8 are uncontroversial. The 9th one is subjective. The last one is unusual but IMO worth doing because of the relationship between the two amount modules.
Do note that this PR is 100% internal changes - please please don't bike shed this to death.
ACKs for top commit:
apoelstra:
ACK 13a3f490b80e4c8f8e1753111a914315eefd73e6; successfully ran local tests; lgtm
Tree-SHA512: e2ef0e7fbdaaf632a9840920a227a901fbeb55a29398013cd6cb764b1ff7c0a7c5a1648fd8f606e8b5f7523943886f5eff54cf4054d24349feb72f0b4de05b91
I claim that if the two amount modules are coded as similarly as
possible it will be easier to ensure that we have the API's uniform and
bug free. To make auditing the modules easier and less error prone use
`Self` instead of the explicit type. This makes it easier to see
differences in the modules and to ensure the differences are correct and
required.
Internal change, no logic changes whatsoever.
420e8c043e Fix mutants in units (Jamil Lambert, PhD)
Pull request description:
Cargo mutant found missed mutants in the bitcoin constants in both signed and unsigned amounts.
Add a test to check their values.
ACKs for top commit:
tcharding:
ACK 420e8c043e
apoelstra:
ACK 420e8c043e34817fa51939c0484ed77cb972ad46; successfully ran local tests; sure, this is reasonable enough
Tree-SHA512: 7a3834b9ac8a6c4742405d67e7a5d5b4eecdb6ec262d7525ba9538201c84fd4aef1a900c43a7dfc041cc09741dfc6fa34e7664ae371b66e9554718af0a85083b
We plan on enforcing MAX_MONEY in the amount types. In preparation use
MAX_MONEY in the serde regression test instead of the arbitrary, and too
large DEADBEEFCAFEBABE value.
We have an `_unchecked` amount constructor that makes no assumptions
about the argument. We would like to start enforcing MAX_MONEY but the
diff to introduce this is massive. In an effort to make it smaller we
can do all the hardcoded ints first. We did this already but a bunch
more snuck in or were missed.
In any amount constructor that passes in a hardcoded const as a decimal
integer (i.e., not hex) use the `_unchecked` version.
Done in preparation for enforcing MAX_MONEY.
4dcdf73cfa Add `µBTC` and `µbtc` to tests (Jamil Lambert, PhD)
afba28e188 Change `uBTC` to `µBTC` in rustdocs (Jamil Lambert, PhD)
2ca24f00f2 Add `µBTC` as a recognized `str` form of `uBTC` (Jamil Lambert, PhD)
Pull request description:
`µ` is the correct letter for the SI unit micro but is not on most standard keyboards. `u` was used instead because it looks similar.
Add `µBTC` to the list of recognized strings for MicroBitcoin. This is an addition only, `uBTC` still works as normal.
Change `uBTC` to `µBTC` in the rustdocs. The examples have been left as `uBTC` since this is easier for most people to use.
Add `µBTC` and `µbtc` to the tests.
Close#3941
ACKs for top commit:
apoelstra:
ACK 4dcdf73cfa896b2c095cda9064c6e0a0e9aeec2b; successfully ran local tests
storopoli:
ACK 4dcdf73cfa
tcharding:
ACK 4dcdf73cfa
Tree-SHA512: 0f6e8b8b9c04f1a4dc6536c0420b2ded568ab96d2301b7d488807cb26003b91a787a6cf9023705c731682580f73ae5247f3f3b1e8646e4eb720c5a65da582933
6d0c6c61ef Run just check-api (Jamil Lambert, PhD)
20d3f16a54 Fix clippy lint in new rustc nightly (Jamil Lambert, PhD)
Pull request description:
Automated update #3922 failed due to a new clippy lint and a change in the `cargo-public-api` format.
Update nightly to 2025-01-16, fix the clippy lint and run `just check-api`.
ACKs for top commit:
tcharding:
ACK 6d0c6c61ef
apoelstra:
ACK 6d0c6c61ef759ac4bfc3b16d32e93f1b483b9365; successfully ran local tests
Tree-SHA512: a4fb8c3dd253acc0d587399771d4ce7893fc5e16cd9637c4968fd730e101456ecaad786506dc83a3d8447b1ac35617ec649d0af2ca52f1d7deb79b04c5366b56
merge-script
Tobin C. Harding
Martin Habovstiak
Jamil Lambert, PhD
Fmt Bot