f0562504b7 Exclude deprecated fn from mutation testing (Jamil Lambert, PhD)
4d31b141a8 Improve is_too_precise test (Jamil Lambert, PhD)
a2bae3bb0b Add test for impl Display for Script (Jamil Lambert, PhD)
Pull request description:
Weekly mutation testing found new mutants. There are also some untested mutants in match statements that will be included in future mutation testing once #4654 goes in.
- Add a regression test for the mutants in the `Display` impl for `Script`.
- Improve the existing test for `is_too_precise` to check all four cases.
- Exclude the two deprecated functions that have untested mutants.
Closes#4646
ACKs for top commit:
benalleng:
**tACK** no missed mutants on f056250 I ran with 63b61e9497 rebased in to make sure the match arm and match guard mutations were included
tcharding:
ACK f0562504b7
Tree-SHA512: d109e30be91da2ab243a152b9ef17147337328282ac418fa9d2eebd17c2e2d9b6f7ee095d91ccf58e287c9620cb71a090b0d929c9bf35011feb26e6f28457dd3
We have `Add` and `Sub` implemented for `NumOpResult<T>` but because the
impls are generic they are not grouped in the file with the other
add/sub impls. This makes it hard to see if they are supported.
Add to the `add` and `sub` unit tests to verify support is implemented.
`cargo publish` does not allow publishing a crate without all
dependencies having explicit version numbers. We discovered recently
that having patching the root manifest was annoying when downstream
testing so we removing the version numbers in #4284 in favor of `path`.
It turns out we can include both.
Props to nyonson for discovering that.
ref: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#multiple-locations
Use an explicit version number to the `internals` dep in `units`.
70221ffa08 Fix link to use new name (Tobin C. Harding)
ebf92fcb01 Use ASCII in rusdocs (Tobin C. Harding)
64ece63f19 Add missing whitespace character to rustdoc (Tobin C. Harding)
64e10686de Improve rustdoc examples for absolute locktime (Tobin C. Harding)
Pull request description:
In preparation for release go over the locktime rustdocs.
Close: #4463
ACKs for top commit:
jamillambert:
ACK 70221ffa08
apoelstra:
ACK 70221ffa085e3cd0a50683f788f5e78cdfae04ae; successfully ran local tests
Tree-SHA512: 67bc51400056e08018f92f8e7ff91c8f8db9e6b575e83f8937a382bde45541a31e75277e200694180fa5098b681e200858fb7b0c9330065af0019955ed126a61
ef56baa696 Improve fee_rate test to kill a mutant (Jamil Lambert, PhD)
5edcc5dad4 Remove repeated fee_rate test (Jamil Lambert, PhD)
bd50943234 Add a roundtrip test to kill a mutant (Jamil Lambert, PhD)
0b4b17307d Test SignedAmount edge case to kill mutant (Jamil Lambert, PhD)
f3338655f1 Test OutPoint edge case to kill mutant (Jamil Lambert, PhD)
Pull request description:
Weekly mutation testing found new mutants in units and primitives.
- Add tests or improve existing tests to kill all of the mutants.
- Remove a duplicate test in `fee_rate`
Closes#4601, Closes#4622
ACKs for top commit:
tcharding:
ACK ef56baa696
apoelstra:
ACK ef56baa69670f9c2e453cb981d6f3488247d000c; successfully ran local tests
Tree-SHA512: b5301ae76e6b1d94831054d348d7eb544ab052178a1ce362cff2e6876fd4eb243c842d158e7d174b7ec4700083367ed0e3c6f6d8e1f6c0a4c888d5bb3205f72f
d8377d90dd units: Remove serde derive feature (Tobin C. Harding)
1031851da4 units: Manually implement serde traits for block types (Tobin C. Harding)
Pull request description:
Currently we only need the `derive` feature of `serde` in test code.
Observe:
- We do not need the error testing logic because `ParseAmountError` is already exhaustively tested.
- The rest of the `serde` test logic in `amount` can be done using the public API so it can be moved to the integration test directory.
Move the unit test code to `tests/` excluding the error testing logic. Remove the `derive` feature from the `serde` dependency. Add a `dev-dependency` on `serde` that enables the `derive` feature.
ACKs for top commit:
apoelstra:
ACK d8377d90dd8d6bd066f51f45c23ffdfe2d0ab694; successfully ran local tests; nice!
Tree-SHA512: 03eb24ae1917e838a2e20c3c62ef9381e2a1eaccdb6474f60a2db59af98d9533054227af4c404013ea8deb4cfe4d57075ae4890857f8af283ebb4338ddb4ed3f
f746aecb61 Use NumberOfBlocks in rustdoc (Tobin C. Harding)
dc5249aae6 Use new type names instead of deprecated aliases (Tobin C. Harding)
Pull request description:
Recently we changed the names of some locktime types but kept aliases to the original names. To assist with ongoing maintenance lets use the new names already.
~Internal change only.~
Added a patch that does the same in some rustdocs.
ACKs for top commit:
jamillambert:
ACK f746aecb61
apoelstra:
ACK f746aecb61456f1eb97280e1d091434250832e72; successfully ran local tests
Tree-SHA512: f3304fcc62069f72fa6c49b4aee5579e4ef6e81cb466b9c7e79ddd1a04b63f2f4d3f0ffdcb9303c3bee2501beead3fceb9be79cefe35d7615223738291535152
2b07f59545 units: Fix up the api test (Tobin C. Harding)
a6ab5c9fd0 Implement Arbitrary for result types (Tobin C. Harding)
Pull request description:
A lot has changed over the last few months. Fix up the API integration test to match the current state of the crate.
Includes a patch to implement `Arbitrary` for types from the new private `result` module.
ACKs for top commit:
apoelstra:
ACK 2b07f59545de16b66b3bf8ee988757c2d0c14afb; successfully ran local tests
Tree-SHA512: fa9bd5b67ea6217cfb11a985183d2d83ae126678fa9fbe5b540fcec31efc0fbafb3e6010c7e831c84c9161300ad5ccc771f848875fdcfe09e072bb0d2881a104
There is a mutant found in `FeeRate` `to_sat_per_kvb_floor` and `ceil`.
Add to the existing test so that all 6 to_sat floor and ceil functions
are tested.
Currently we only need the `derive` feature of `serde` in test code.
Observe:
- We do not need the error testing logic because `ParseAmountError` is
already exhaustively tested.
- The rest of the `serde` test logic in `amount` can be done using the
public API so it can be moved to the integration test directory.
Move the unit test code to `tests/` excluding the error testing logic.
Remove the `derive` feature from the `serde` dependency. Add a
`dev-dependency` on `serde` that enables the `derive` feature.
Recently we changed the names of some locktime types but kept aliases to
the original names. To assist with ongoing maintenance lets use the new
names already.
Internal change only.
We typically do not want to have public constructors on error types.
Currently we do on the `TimeOverflowError` so that we can call it in
`primitives` but it turns out we can just use `NumberOf512Seconds`
constructors instead.
We now have constructors that take an arbitrary size fee
rate (`Amount`). The `from_sat_per_foo` constructors can be made
infallible by taking a `u32` instead of `u64`. This makes the API more
ergonomic but limits the fee rate to just under 42 BTC which is plenty.
Note we just delete the `from_sat_per_vb_u32` function because it is
unreleased, in the past we had `from_sat_per_vb_unchecked` so we could
put that back in if we wanted to be a bit more kind to downstream. Can
be done later, we likely want to go over the public API before release
and add a few things back in that we forgot to deprecate or could not
for some reason during dev.
Fuzz with a new function that consumes a `u32`.
As we did for `Amount` change the fee functions on `FeeRate` and
`Weight` to return `NumOpResult`.
This change does not add that much value but it does not make things
worse and it makes the API more uniform. Also reduces lines of code.
Currently we call the `Amount` fee calculation functions `div_by_foo`
and return an `Option` to indicate error. We have the `NumOpResult`
type that better indicates the error case.
Includes a bunch of changes to the `psbt` tests because extracting the
transaction from a PSBT has a bunch of overflow paths that need testing
caused by fee calculation.
The `Amount` and `FeeRate` types are not simple int wrappers, as such we
do not need to mimic the stdlib too closely. Having the `checked_`
prefix to the functions that do fee calcualtions adds no additional
meaning. Note that I'm about to change the return type to use
`NumOpResult` further justifying this change.
Note we leave the 'Checked foo' in the functions docs title because
the functions are still checked.
e7c90c57e7 Remove reachable unreachable call in psbt (Tobin C. Harding)
c736e73ae0 Add unwrap_or and unwrap_or_else to NumOpResult (Tobin C. Harding)
Pull request description:
A bunch of changes have been implemented lately in the fee calculation logic. As a result of this a at once time `unreachable` statement is now reachable - bad rust-bitcoin devs, no biscuit.
Saturate to `FeeRate::MAX` when calculating the fee rate, check against the limit arg, and win.
(Patch 1 adds the new combinators to `NumOpResult`. This was pulled out of #4610.)
ACKs for top commit:
apoelstra:
ACK e7c90c57e7b5ee20a2c523706cc4ea9957594857; successfully ran local tests
Tree-SHA512: d33b3d5d4442fb17cae4c52880bc5dafdd611d686c6923744bc5f218761e8bff32fc5ab169af9f2ed2f02011516ed850ac8c6bd4ce1d6de40c0ac0b17486bbb4
The `fee` module is now empty as far as API surface. It still holds the
`core::ops` impls for fee calculation. It also does have useful
rustdocs which are not currently visible online because module is
private.
Make the module public. Functionally all this does is provide a place
for the module level docs under a discoverable name.
Improve the docs by adding a bunch of links to fee related functions.
A while back we move all the 'fee' stuff into a separate module
because I thought it would help with clarity - I was wrong.
Move the checked mul function back into the `weight` module on the main
`Weight` impl block.
Internal change only - code move.
A while back we move all the 'fee' stuff into a separate module
because I thought it would help with clarity - I was wrong.
Move the checked mul and to fee functions back into the `fee_rate`
module on the main `FeeRate` impl block.
Internal change only - code move.
A while back we move all the 'fee' stuff into a separate module
because I thought it would help with clarity - I was wrong.
Move the checked div functions back into the `unsigned` module on the
main `Amount` impl block.
Internal change only - code move.
Two useful combinators, add them.
I copied the function signature and docs from stdlib and wrote the
functions myself - thereby operating within licensing requirements.
Some users may find it more ergonomic to pass in an `Amount` when
constructing fee rates. Also the strong type adds some semantic meaning
as well as imposes the `Amount::MAX` limit.
Add an equivalent constructor for each of the existing ones that uses an
argument of type `Amount` instead of `u64` sats.
7fbe07a6e0 Use uniform docs for overflow (Tobin C. Harding)
153a6a2f3c Make Weight docs uniform with FeeRate (Tobin C. Harding)
c87f7292be Fix rustdocs on Weight (Tobin C. Harding)
02b523a8ad Remove whitespace from encapsulate module (Tobin C. Harding)
Pull request description:
Make a sweep of the `units` crate's rustdocs.
ACKs for top commit:
apoelstra:
ACK 7fbe07a6e0b3e398aca845d64ec86f3f0068edf4; successfully ran local tests
Tree-SHA512: ba50f3afb94dda43f89d04eb53c6e85df302292d4647fe81a20e3f7d1ca75e8ee8cdf6548864b2f3c33ed661205d109dbd763db1061ea45a59eab25f134191f8
The `block` module does not follow the `encapsulate` pattern but we can
still use the getters instead of accessing the inner field directly.
Refactor, no logic change.
afc0ce6175 units: Add must_use to checked arithmetic functions (Tobin C. Harding)
Pull request description:
The checked arithmetic functions all consume self so we use `must_use` to help users not miss this point.
Most are done, add the missing ones.
ACKs for top commit:
apoelstra:
ACK afc0ce617554303d7fd25e052b039af44b6efc1c; successfully ran local tests
Tree-SHA512: 7105affff43827ed47a1c0b6e41a996aa538c7d53b891faf03e79a83164706d7e86db5fb184ac740fdf57bb43f8401a496cc64ea4da0da71eaa8c8cca16444c7
Andrew Poelstra
Jamil Lambert, PhD
Tobin C. Harding
Fmt Bot