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 `AmountOpResult` 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 - somewhat ugly to use, see tests for example usage.
The ops traits return `Self::Output` not `Self`. The current code builds
because `Self` and `Self::Output` are both the same type.
Use `Self::Output` as the return value of `ops::Rem`.
2eccb3897a CI: Remove the API job (Tobin C. Harding)
Pull request description:
It has been argued that the API files provide value for verifying that code changes produce API changes in the expected way. This helps devs and reviewers.
It has also been argued that the CI job adds friction and forces devs to configure their environment unnecessarily.
Both viewpoints have some merit.
Lets remove the API files for now, and when we start doing 1.0-alpha releases and can start to expect minimal API breaking changes (and therefore minimal need to use the tooling)
ACKs for top commit:
Kixunil:
ACK 2eccb3897a
Tree-SHA512: 566edf6cdce6669841d8239bdd46506b6df7884a0a1fd481b4aec0db7134b1109fd7b6216a54e6e92444a4f17e3a740628c0016b028b5966bb460daf84de4b8c
85e0330d7f Run the formatter (Tobin C. Harding)
7be0db730a hashes: Move bench and test code into files (Tobin C. Harding)
665fa9de99 hashes: Pull crypto out into submodule (Tobin C. Harding)
1bfd1e071a hashes: Make module subdirectories (Tobin C. Harding)
Pull request description:
This is an attempt at point 3 in https://github.com/rust-bitcoin/rust-bitcoin/pull/3961#issuecomment-2619946074
However instead of using `api.rs` and `implementation.rs` it uses `crypto.rs` for the cryptography stuff and leaves the rest in `mod.rs`.
This whole PR is internal changes only. Almost entirely code moves, review is easy if you have your `diff` configured nicely.
ACKs for top commit:
Kixunil:
ACK 85e0330d7f
apoelstra:
ACK 85e0330d7f057c9fe447bff5fdb6023150ead319; successfully ran local tests; look great! thanks!
Tree-SHA512: e52e6410e86fc93ec34a72be8c64f02148f46958f8f5c1850075b1a7f41886c220f09144ccd05a98a551c356a5d25524c8884fc8578a205b27f385ec0725f13b
It has been argued that the API files provide value for verifying that
code changes produce API changes in the expected way. This helps devs
and reviewers.
It has also been argued that the CI job adds friction and forces devs
to configure their environment unnecessarily.
Both viewpoints have some merit.
Lets remove the API files for now, and when we start doing 1.0-alpha
releases and can start to expect minimal API breaking changes (and
therefore minimal need to use the tooling).
a7526b6a70 Remove `fee_vb` (yancy)
73b14d03b9 Add `to_fee` in place of `fee_wu` (yancy)
Pull request description:
Rename fee_wu and remove fee_vb
closes https://github.com/rust-bitcoin/rust-bitcoin/issues/3908
ACKs for top commit:
apoelstra:
ACK a7526b6a70642e04f0a8ad8a89aa368031fdb6cd; successfully ran local tests
Kixunil:
ACK a7526b6a70
Tree-SHA512: fd865525e56caddc49158fd94fb125d92fd09654d5c72609a6f8e34370e79a9be4213dbd7e69b0d8498d92fca8a970142262f56b63cebd76c200aca75b6e0ca6
We are trying to make the `hashes` crate easier to read and also
possibly parseable by machines to see what is different where.
Move the test and bench code into separate files. Make special effort to
keep formatting as is so the diff is easier to review. We will run the
formatter in the next patch.
Internal change only.
There are five modules in `hashes` that implement cryptography (i.e.
have a `process_block` function). For each of them create a new
submodule called `crypto` and move the code there.
Code organisational refactor only, no logic changes.
In preparation for removing a bunch of macros move all the modules to
`<mod>/mod.rs`.
Do so by running the following shell:
```bash
for mod in hash160 ripemd160 sha1 sha256 sha256d sha256t \
sha384 sha512 sha512_256 siphash24 hkdf hmac; do
mkdir $mod
mv "$mod.rs" "$mod/mod.rs"
done
```
Internal change only.
Use the more idiomatic to_fee instead of `fee_wu`. Since the method
takes a strongly typed argument, remove `wu` from the method name
to improve clarity.
ebab2d9aa7 Update cargo-public-api version (Jamil Lambert, PhD)
3b350d4f54 Update to rustc (to nightly-2025-01-31) (Jamil Lambert, PhD)
55ab2f34a7 Remove manual implementation of ok (Jamil Lambert, PhD)
30132c56bb Fix rustdoc indentation (Jamil Lambert, PhD)
Pull request description:
Fix the errors found when updating `rustc` to `nightly-2025-01-31`. Then update the version in `nightly-version`.
Update `cargo-public-api` to `v0.43.0` since `v0.42.0` is only compatible with `nightly-2024-10-18 — nightly-2025-01-24`
ACKs for top commit:
apoelstra:
ACK ebab2d9aa7310eb62a6c42bf4dfb266cff441965; successfully ran local tests
tcharding:
ACK ebab2d9aa7
Tree-SHA512: 3c037b8bbfb0fa69be1b6fa1438c7c38636683dc7ffe9bce018afdd6b9033edaa18b5f3e5fa489ebb720221b8780ff628e573b5408270bf6c45edf9ca01a97e7
51cb36881f api: Run just check-api (Tobin C. Harding)
1bcfc80fe7 Add engine function to siphash24::Hash (Tobin C. Harding)
Pull request description:
The other hashes that require initial state (keys etc) both have an `engine` function on the hash type because they cannot use the `GeneralHash::engine` function.
Add an `engine` function to the `siphash24::Hash` type.
ACKs for top commit:
apoelstra:
ACK 51cb36881f5291daa4136a714c286cf01f968cf0; successfully ran local tests
Tree-SHA512: 5cafa5b717235e94167b98a7ff675c3bde5071e736cdc4515ae534b517a5a96eead2bebd25b5dc0123e5e8615bb191c6990ae1ea7dea59b1b58d72547514b5b5
The other hashes that require initial state (keys etc) both have an
`engine` function on the hash type because they cannot use the
`GeneralHash::engine` function.
Add an `engine` function to the `siphash24::Hash` type.
e91cb3ff70 Run just check-api (jrakibi)
134c146748 Add Weight::checked_mul_by_fee_rate method (jrakibi)
Pull request description:
Following up on #3736, this PR adds `checked_mul_by_fee_rate` to `Weight` to mirror the existing `checked_mul_by_weight` method on `FeeRate`.
This makes the fee calculation more symmetrical.
Changes:
- Add `Weight::checked_mul_by_fee_rate` method
- Add corresponding test cases
The new method allows users to calculate fees starting from either Weight or FeeRate. This makes fee calculations more intuitive regardless of which type the user starts with.
Closes#3766
ACKs for top commit:
tcharding:
ACK e91cb3ff70
apoelstra:
ACK e91cb3ff7078277f5b173773263863fcaef47ae3; successfully ran local tests
Tree-SHA512: 10d17763a8b04027e0f650a8fb15245671214e46ae13fe0b0a86c2ee8b79c1542e449cb298bf65f2e30b6837de5ae237b221b9c90575a0318959feb64ab95221
76bc22b496 Automated update to Github CI to cargo-semver-checks version-0.39.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 76bc22b496
Tree-SHA512: 80846a83085b946293feb08817584bb0a8259ee2cb5c5eb2747eb0e5b482228db888ca3bf64ef48ef0a23d180315df84db9e6e8ecf7700ed2cf2a1f49da22c2a
12a1c3c4b7 Add `script` tests (Jamil Lambert, PhD)
Pull request description:
Together with #3971 this fixes all of the mutants in `primitives/src/script` except for those related to `serde`.
ACKs for top commit:
tcharding:
ACK 12a1c3c4b7
apoelstra:
ACK 12a1c3c4b7bc097553fbfb05a0e92320ae42975c; successfully ran local tests
Kixunil:
ACK 12a1c3c4b7
Tree-SHA512: c0b757ae672999799e105befb6a2907b8cecb54c3e6d86dbf3e877ddf54e6fd102ebb2604381d0fa89bce0b3b4f70db6cb9ed3b5c85076e13de8369e81431926
b98c489066 hashes: Move from_engine functions (Tobin C. Harding)
Pull request description:
In order to use the `general_hash_type` macro the must exist a standalone `from_engine` function. Currently this function is in different places in different modules. In an effort to make the `hashes` code easier to reason about put the functions right below the macro.
Code move only, no other changes.
ACKs for top commit:
Kixunil:
ACK b98c489066
apoelstra:
ACK b98c489066e8916a383099e5037e5a24832548ba; successfully ran local tests
Tree-SHA512: 8dfbf2b422d078d687708fa94a478ca597fae141f5c1f0a318a36152ca33f4760bb0545ab67523c558a8c3b8d258356975c5e357600d0ac980d473250a2af20e
c16eab7723 Add examples to `absolute::LockTime` (Jamil Lambert, PhD)
2d73746ad1 Improve `from_consensus` example (Jamil Lambert, PhD)
25a6742573 Add examples to `relative::LockTime` (Jamil Lambert, PhD)
Pull request description:
To conform to API guidelines add examples to both `relative` and `absolute` `LockTime`.
As discussed in #3967 add an example that doesn't round trip in `relative::LockTime::from_consensus`.
ACKs for top commit:
tcharding:
ACK c16eab7723
apoelstra:
ACK c16eab772375a0b8f82789a3453f7e983523fd50; successfully ran local tests
Tree-SHA512: 898db2dbcccfc54971c65bb72030505dbcfffb035aa74eb376cc1eb8d3619c205338899a58fb9f267fca192c11e7ae48fc17a2d9019c5ff4bd34bf029fa9a48f
390cc39d07 Automated update to Github CI to rustc stable-1.84.1 (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 390cc39d07
Tree-SHA512: 9fd472e0e7bd1a2d805b1a0b08108223605708724b68659e24149b5049e3a62fec10de49fe684e363b208ccebe662f7221c2fb321eff3cf00dfd34efcdee8e86
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.
Mirror FeeRate::checked_mul_by_weight functionality by adding a symmetrical method to Weight.
This allows users to calculate fees starting from either Weight or FeeRate with consistent behavior.
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.
In order to use the `general_hash_type` macro the must exist a
standalone `from_engine` function. Currently this function is in
different places in different modules. In an effort to make the `hashes`
code easier to reason about put the functions right below the macro.
Code move only, no other changes.
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
Tobin C. Harding
merge-script
yancy
Peter Todd
Jamil Lambert, PhD
Fmt Bot
jrakibi
Martin Habovstiak