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
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
7482fcd934 Run just check-api (jrakibi)
bcc38c40e0 Add Amount division by FeeRate (jrakibi)
Pull request description:
Add checked_div_by_fee_rate method to Amount that computes the maximum weight for a transaction with a given fee rate. This complements the existing `fee = fee_rate * weight `and `fee_rate = fee / weight` operations
- Add `checked_div_by_fee_rate` method that returns Option<Weight>
- Implement` Div<FeeRate>` for Amount for operator syntax support
- Use `floor` division to ensure weight doesn't exceed intended fee
This allows calculating the maximum transaction weight possible for a given fee amount and fee rate.
Closes#3814
ACKs for top commit:
apoelstra:
ACK 7482fcd934c09e3cd6cd25fd4328960b02f8e976; successfully ran local tests
tcharding:
ACK 7482fcd934
Tree-SHA512: 622ca42bde1f67782a3c2996efcba0132fedb5e984f594603aece974de6acdeb4b22d63239d29d46fb8623c8082841c33b1d5b9ad2ea51e2f63e6f5d859daa7e
e0028239cf api: Run just check-api (Tobin C. Harding)
5eb5941215 Add FIFTY_BTC const to the amount types (Tobin C. Harding)
Pull request description:
The mining reward for the first epoc is 50 bitcoin. For mainnet this is old news but for regtest it is still relevant.
Add and use a new const `FIFTY_BTC` to the `Amount` type. To keep the amount types uniform also add it to the `SignedAmount`.
ACKs for top commit:
storopoli:
ACK e0028239cf
jamillambert:
ACK e0028239cf
apoelstra:
ACK e0028239cf207660deb2873a92bacfa1315af634; successfully ran local tests
Tree-SHA512: 623ed8b1f5fe8dd95309179308fea83d68be4349becf6305769b0378cc9032961df2c062dc2bf702fec5e2394e8abb7360d2be6f19b6cf505db8769a5ae39e16
9ef8e294ac api: Run just check-api (Tobin C. Harding)
3542803c4e Make Cursor methods const (Tobin C. Harding)
Pull request description:
Mirror the `std::io::Cursor` type by making the same methods `const`.
ACKs for top commit:
jamillambert:
ACK 9ef8e294ac
apoelstra:
ACK 9ef8e294ac81c79f413fbe47372a3f94c926ef12; successfully ran local tests
Tree-SHA512: 60ad8169c9bcc90360fcd3c3439256d86af4a77733a00a66e9b155bdb89580d4971efd55a93f1873363ed9d24f0b2b8f5a0da1857bb5aa8a112b13e27239a984
9396041524 api: Run just check-api (Tobin C. Harding)
ffd8702cb3 units: Hide the remaining public macros (Tobin C. Harding)
Pull request description:
We do not want to commit to any public macros in `units`. Recently we (I at least) learned that adding `doc(hidden)` signals to users that the macro is unstable and should not be relied upon.
Hide the remaining two macros so we can release 1.0 and not worry about later breaking them.
With this applied there are no exported macros that are not hidden. Verify using `git grep -A 1 macro_export`.
ACKs for top commit:
apoelstra:
ACK 9396041524e291203d5c86665639872f9a6246b5; successfully ran local tests; Yeah, let's do it
Tree-SHA512: a3a59897a2fe16276ab2d364ff247f48772a63a25f91eabc17023a37b9fab3860639dc1e09193c938dd73711ba20c95b8d0ad9db9493d269ee9328b2132d61cb
b1399d193f units: Improve rustdocs on macros (Tobin C. Harding)
706c7c9f5f Hide impl_tryfrom_str (Tobin C. Harding)
2675cd1040 units: Re-order impl_parse_str (Tobin C. Harding)
4cc7aae6b9 Hide impl_tryfrom_str_from_int_infallible (Tobin C. Harding)
fc9e40ab1a units: Re-order impl_parse_str_from_int_infallible (Tobin C. Harding)
Pull request description:
Document the public macros in `units::parse`. Done as part of #3632
First 2 patches are trivial preparatory refactor.
ACKs for top commit:
apoelstra:
ACK b1399d193fcb20c09457a1f22c5c1dd8009c84b1; successfully ran local tests
Tree-SHA512: e2b0196adb37b3616963e3e3ded1c2be95f98fe33a4e6edb269b7eca1ddb66b82be139f4edb3269a5cf69a73b3c803845fe83a5f6e300b08abf9fcb0da602084
f5eb8f4747 api: Run just check-api (Tobin C. Harding)
472b1d3ff3 units: Add serde regression test (Tobin C. Harding)
dedae8acf2 Implement custom serde modules for FeeRate (Tobin C. Harding)
d94e5f03e6 Move fee_rate.rs to module (Tobin C. Harding)
c3c1f6f82d Add missing license comment to test file (Tobin C. Harding)
Pull request description:
Implement and enforce explicit unit when serializing. This is as we do for `Amount` (see #3672 for similar).
To test it, and as part of the 1.0 effort; add regression tests for `serde` stuff in `units`.
With this applied one must use attributes to serialize `FeeRate`.
```rust
#[derive(Serialize, Deserialize)]
pub struct Foo {
#[serde(with = "bitcoin_units::fee_rate::serde::as_sat_per_kwu")]
pub fee_rate: FeeRate,
}
```
ACKs for top commit:
apoelstra:
ACK f5eb8f4747a7cd303cad2b7f8f442bb31862c52a; successfully ran local tests; great idea!
Tree-SHA512: 0968ead568b1e3142efd4c0e856192ddde0f441de84215cbb0950b60a56922f1abaf6d4ccfe243b722a6883c0a927d26bcfba979acf3ca84c4f21baba73af764
This macro is a helper for `impl_parse_str_from_int_infallible` used to
reduce code duplication. It does not, and should not, need to be part of
the public API - hide it.
ae129d84ac api: Run just check-api (Tobin C. Harding)
3f433f1cde Improve Cursor ref API (Tobin C. Harding)
Pull request description:
Our `Cursor` is a replacement of sorts for the `std::io::Cursor`. Users of the `Cursor` are likely to expect the same API so to get a reference and/or mutable reference users will likely reach for `get_ref` and `get_mut`. We should provide these.
Deprecate `inner` since it is the same as `get_ref` but less idiomatically named (should have been `as_inner`).
Add `get_ref` and `get_mut` methods to the `Cursor` type.
ACKs for top commit:
apoelstra:
ACK ae129d84ace03fba57bb980cace5aa60ff6bc10e; successfully ran local tests
Tree-SHA512: 664d08cc977c9d6e3319767c49bbabc8f21d681d570f50c514797a7737469cf861af60158ccc1c71d28baed542dabbbca586968144d60691ee0183a549a7b765
2042d91be5 api: Run just check-api (Tobin C. Harding)
04bae4bb91 Use _unchecked in unit tests (Tobin C. Harding)
8e6784dd41 Introduce Amount::from_sat_unchecked (Tobin C. Harding)
f32af7dac6 Use ua_sat throughout test function (Tobin C. Harding)
75f0afd0a3 Add local variable to use as constructor (Tobin C. Harding)
940a244132 Replace Amount::from_sat(0) with ZERO (Tobin C. Harding)
Pull request description:
This is a small-ish changeset in order to mirror what was started in #3769 and reduce the diff in #3794
- Patch 1: Trivial refactor to use `Amount::ZERO`.
- Patch 2: Add the constructor and refactor a single unit test to use it.
- Patch 3: Use the two `_unchecked` constructors throughout `units/src/amount/tests.rs`.
- Patch 4: Add the API text file changes.
ACKs for top commit:
jamillambert:
ACK 2042d91be5
apoelstra:
ACK 2042d91be59a1d47afb986cef0152f9b5250f476; successfully ran local tests
Tree-SHA512: 8f9c3816509bb3fc0707cd4a47426130d001358e82273457b24170522644e10fe36b596e16500ce1778738f5546dfad592f1f3c4de552ec0afcb146442176cf5
87293dfdd3 api: Run just check-api (Tobin C. Harding)
ed387e5f1d hashes: Hide both macros (Tobin C. Harding)
Pull request description:
We have two macro definitions feature gated on `serde`. At some stage we added the `doc(hidden)` attribute to one of them but forgot to add it to the other. This technically makes our features non-additive. This macro is "internal" so its unlikely that this is being used in the wild.
Add `doc(hidden)` to the `serde_impl` macro that is missing it.
Found by `cargo semver-checks` after recent upgrade to 0.38
ACKs for top commit:
storopoli:
ACK 87293dfdd3
apoelstra:
ACK 87293dfdd38be71938ae9d6a9981eb4569de3521; successfully ran local tests
Tree-SHA512: 3773eb42684bce56928f2d3f87993d010dd36634fda8dfa6b18b27a36b6c4bb521d711340a4868270dbd2d0c6a11715f9a1998ff3deb6900bd4227a44722a11e
e13355318e Add From impl (yancy)
364e9ff775 Change method return type (yancy)
fdf3336ed5 Add unchecked variant (yancy)
Pull request description:
Any SignedAmount can now be cast to Amount since the range is the same. Specifically, the range for SignedAmount is (- 21 million, 21 million) while the range for Amount is (0, 21 million). Therefore any value from Amount can be cast to a SignedAmount and it will work. Note it's not the same and still requires checking when going from SignedAmount to Amount since Amount can't handle the negative range.
ACKs for top commit:
tcharding:
ACK e13355318e
Tree-SHA512: c016b51bdd87a12eb09d9c1a82699dad1e866bf96bd3235eeb131f216f02422acb992ddb3a8135af00bbc10e240178fde5e37fb7860d9e6eaf433cf917d4d16a
Any SignedAmount can now be cast to Amount since the range is the same.
Specifically, the range for SignedAmount is (- 21 million, 21 million)
while the range for Amount is (0, 21 million). Therefore any value from
Amount can be cast to a SignedAmount and it will work. Note it's not
the same and still requires checking when going from SignedAmount to
Amount since Amount can't handle the negative range.
As a side effect of changing the return type, TryFrom is no longer valid
and does not compile. Therefore in addition to changing the return
type, TryFrom is also removed.
29811ba82c api: Run just check-api (Tobin C. Harding)
0a16382fa3 Rename rhs to weight (Tobin C. Harding)
Pull request description:
In ops functions we typically use `rhs` but for the more unconventional `checked_*_by_*` functions lets use a more descriptive parameter name.
Interestingly `cargo public-api` thinks this is an API breaking change. This is obviously an internal change but the api files are updated because the parameter names appear in the api text files.
ACKs for top commit:
apoelstra:
ACK 29811ba82cc598d08dc877825ecf8890c48d23b7; successfully ran local tests; sure
sanket1729:
ACK 29811ba82c
Tree-SHA512: b44c958ab3ef024c867d81f12819775afa62f1762b96afb93831bb4857ddb9bc95ae5b5f42f32b1a1d23832c69c3cae55f12a80d109fadda7d6763bc764d06aa
04dfe8dd45 Add api test to check Arbitrary impls (Shing Him Ng)
678fc71b88 Implement Arbitrary for units types (Shing Him Ng)
Pull request description:
Implement Arbitrary for the rest of the types in `units`. Also moved the implementation in `FeeRate` right before the `tests` module
Closes#3705
ACKs for top commit:
apoelstra:
ACK 04dfe8dd45fae9b55dacfe9eb0d73ea306db14ba; successfully ran local tests
tcharding:
ACK 04dfe8dd45
Tree-SHA512: 156bd26d4de85d484711d476df1d2758805387125209f0307aa786dd1585ff9953dbe41b0864b00ae101419176647e3bde7994ed9257c18307d161463b1c8d2e
68f3c3e5d7 api: Run just check-api (Tobin C. Harding)
b956940a6d Add ops unit tests for amount types (Tobin C. Harding)
d88306fb68 units: Implement ops for amount types (Tobin C. Harding)
4a3aa4a08b Add ops unit tests for Weight (Tobin C. Harding)
43ef9a618c units: Implement ops for Weight (Tobin C. Harding)
c5fbc7e117 units: Add assign macros (Tobin C. Harding)
20a79da344 units: Use one level of path for ops traits (Tobin C. Harding)
6ce5385914 units: Add macros for Add and Sub (Tobin C. Harding)
b31ca72b33 units: Use rhs instead of other (Tobin C. Harding)
cd0fa2d1dc Make FeeRate ops impls more terse (Tobin C. Harding)
Pull request description:
Sort out `Add`, `AddAssign`, `Sub`, and `SubAssign` for the following types:
- `Amount`
- `SignedAmount`
- `FeeRate`
- `Weight`
And clean up as we go.
Note that `BlockInterval` isn't touched in this PR - it looks correct already.
The unit tests in the two patches "Add ops tests ..." can be moved if you want to verify that they don't build without the patch they follow.
ACKs for top commit:
apoelstra:
ACK 68f3c3e5d728a60a4b52ca64523770bcdd32f957; successfully ran local tests
Tree-SHA512: a82d3bf288f61b169b8cff498e81bd2cd123c8dcbf534413233ff03f06102a42508e09b2f7e5b268b21f82d4bf2b3612cd88dea1231b4d3e6455c7e99f82e729
f01f071751 api: document the need of cargo nightly (Jose Storopoli)
ff67eadc7f contributing: clarify API changes (Jose Storopoli)
04852958b9 contrib: check if the user has cargo-public-api (Jose Storopoli)
Pull request description:
Closes#3764.
yancyribbens can you test it locally? It works in my end both with and without `cargo-public-api`. Of course without gives the error message that I was supposed to see.
EDIT: You can run it with `just check-api`.
ACKs for top commit:
apoelstra:
ACK f01f071751332bb6aa8b9affa4677f913035de7f; successfully ran local tests
tcharding:
ACK f01f071751
Tree-SHA512: 7acf8332731191de47afc02e24c151bc53c1f38685217eccbea1b64c216674f31729ba703e132765cba67183dff95757aa949d653c5cbe538240371c2db52c14
815330dad2 Clean up weight unit tests (Tobin C. Harding)
3f4365ee0c api: Run just check-api (Tobin C. Harding)
75594c7299 Add Weight::to_kwu_ceil (Tobin C. Harding)
Pull request description:
It is not immediately obvious why we have floor and ceil versions of `to_vbytes` but not for `to_kwu`.
Add a conversion function to get weight by kilo weight units rounding up.
And then clean up the `weight` unit tests.
ACKs for top commit:
apoelstra:
ACK 815330dad2d2026ecf11c998dd560217c8d05d52; successfully ran local tests
Tree-SHA512: 8ba6c255fb9a3bc4a3dd485d6875663976870805a639b0aa309727fd211460029d05154351522d2b753afd478df1187abd92b212512b140c1ddb24b34b7e2bc0