This requires updating a couple serde test vectors -- we are tightening
validation so that some existing serde-serialized PSBTs no longer
deserialize. I believe this is the correct thing to do.
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.
d9cf7270eb Move `bitcoin/p2p` into `p2p` (rustaceanrob)
Pull request description:
Could use a rebase on #4568
Somehow lost the branch of my original draft so some of the review context is removed, but all comments were addressed in separate PRs.
First commit moves everything required to implement encoding within `p2p`, so we have a reference for functions to add to future moves to `internals`. Second commit does the relocation and third commit removes the `bitcoin/p2p` module.
ACKs for top commit:
apoelstra:
ACK d9cf7270eb457fd660fa505701895ab4756e394d; successfully ran local tests
tcharding:
ACK d9cf7270eb
Tree-SHA512: 31bf960788f45bb60b04dd73793a06828a5540e1e6118376776494bad9330dc8ebcb57749bc84b70a00e4d1d2a5686506e089269833ed99c4f9fa0c3e3b2e5b7
f6dea36e31 units: Make error constructor private (Tobin C. Harding)
Pull request description:
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.
ACKs for top commit:
apoelstra:
ACK f6dea36e313c59c3dca534aba2b7de459f094e9f; successfully ran local tests
Tree-SHA512: f8d4615f2adb506278f20352c97e2b6698d3f81fe27d5204f215faeb8fe208fc849ad6c82cd40963736e85e669287d2f0f0c2cbc0a0d5858c2677e693c1582ae
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.
7dc66e3476 impl LowerHex, UpperHex, Octal, and Binary for ChildNumber (vicjuma)
Pull request description:
apoelstra Each trait forwards formatting to the inner u32 index. For Hardened variants, a suffix is appended based on whether alternate formatting is used per the discussion.
See discussion: https://github.com/rust-bitcoin/rust-bitcoin/pull/4620#issuecomment-2974023604
ACKs for top commit:
tcharding:
ACK 7dc66e3476
apoelstra:
ACK 7dc66e3476c3ba08eb341a09f3c85a795b005159; successfully ran local tests
Tree-SHA512: 49e4acc82198d3a2c807bf8977413dd260a7081b83e8ad2c2b97d8fcaf85b78a54552098a5fe3020c78fe7b908d90a3bab86be20f330d3de569b5eba69e23348
Moves all of the content from `bitcoin/p2p` into `p2p`.
`TryFrom<Network>` must be implemented for `Network -> Magic` now that
`Network` is a foreign, non-exhaustive type. Ser/de test is updated
accordingly, as well as the `Magic::from_network` constructor, which I
have opted to return an `Option<Self>`. The `TryFrom` implementation
uses a new `UnknownNetworkError(Network)` that mirrors the `Magic ->
Network` error.
The example handshake does not generate a random nonce for the version
message, as there is no `rand` dependency in this crate and the program
only makes a single, user-defined connection.
It appears we can do better than copying and pasting the consensus
encoding macros and functions from `bitcoin` without doing weird
cross-crate macros that require some special knowledge of the crate to
know when they will compile.
4284deed29 DerivationPath: support 'h' in Display output for hardened components (vicjuma)
Pull request description:
DerivationPath now supports a display format using both 'h' a single quote (') to indicate hardened components.
This aligns it with its ChildNumber's output style.
Resolves: #4618
ACKs for top commit:
apoelstra:
ACK 4284deed29114c5e31ef7c29e28e352b860f74e7; successfully ran local tests
tcharding:
ACK 4284deed29
Tree-SHA512: 24e053c22ec94b851935debbab83d15ad9f41ccfca0a7c34a061450989b176295512e8ffb187b2a54c6c6926f82ec7e4a4186ccdba2ec2616a6e0603d90d2a9b
0ff8d82193 Make FeeRate from sat constructors infallible (Tobin C. Harding)
3b0286bd56 Return NumOpResult for FeeRate and Weight (Tobin C. Harding)
15065b78c7 Return NumOpResult when calculating fee on Amount (Tobin C. Harding)
75106e6d82 Remove checked_ prefix from fee functions (Tobin C. Harding)
Pull request description:
This was 14dc950b54 from #4610.
Remove the `checked_` prefix from `fee` functions then make them all return `NumOpResult`.
ACKs for top commit:
apoelstra:
ACK 0ff8d82193ef562c1aedabe98a6283d09f803c0c; successfully ran local tests; nice!
Tree-SHA512: 62f2af6701f2a0e17b9f0a0ee132ca4a9289fe00e22c94f746602ed3c1f3758bb22323b224362cb659432ff5297391f3106ee460e9a8f65a39f904d72bc98aeb
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
20c84ce444 units: Make fee module public (Tobin C. Harding)
251e6a85da Inline checked mul function back into weight module (Tobin C. Harding)
a8610a937b Inline checked mul / to fee back into fee_rate module (Tobin C. Harding)
e17c391a3c Inline checked div functions back into unsigned module (Tobin C. Harding)
Pull request description:
Move all the `fee` functions back onto the respective impl blocks for the associated type.
Finally, make the `fee` module public so users get the docs. If this is not like we can drop the last patch.
Close: #4609
ACKs for top commit:
apoelstra:
ACK 20c84ce444fe68577bf14842ebaffd796572d91f; successfully ran local tests
Tree-SHA512: e4c9701245fcd0c7881baaf01a7f9d0be1ed80bc3fa848a289fbc6104375d8db5a2a092e9f1aa0b556dcc45349d053418cff8d1c281dda3cefc1e38c6ac2ba85
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.
6ed3fd6234 Add fee rate constructors that take Amount as arg (Tobin C. Harding)
c1a760bf60 units: Use singular in rustdoc (Tobin C. Harding)
Pull request description:
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.
(This was pulled out of #4610.)
Close: #4734
ACKs for top commit:
apoelstra:
ACK 6ed3fd6234a6d9e44d2cf187108cc22955471d27; successfully ran local tests
Tree-SHA512: 92609457ba181370e1484c08028cec9158d6e1fb2ee984e4e8caeacc5c9828bc70febeb4fd3601d9a2fee57c65d5cee0af0ef95ea15e2387d5fd886879c2afb1
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.
Co-developed-by: Andrew Poelstra <apoelstra@wpsoftware.net>
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.
Andrew Poelstra
Fmt Bot
Tobin C. Harding
Jamil Lambert, PhD
rustaceanrob
vicjuma