We have a bunch of unit tests, some use `f` and some use `fee_rate`.
Uniform would be better.
Elect to use the longer form just because there are only 4 instances of
the shorter one (although I personally prefer the shorter form).
a1ce2d1ac8 Use _u32 in FeeRate constructor instead of _unchecked (Tobin C. Harding)
Pull request description:
When constructing an `Amount` it was observed recently that a `u32` is often big enough. For the same reason we can use a `u32` to construct a fee rate instead of overflowing.
Use `_u32`in the constructor and remove the panic.
ACKs for top commit:
apoelstra:
ACK a1ce2d1ac8d646b63532ed9ac459ffea39ec8c20; successfully ran local tests
Tree-SHA512: 339974c954ad613b0be684f7b2fa1402f59fe401969f3785a702ffbb14ac913ecf4c8228240d068ba4b482e38263590154167a071d823ccc9b4d0691036ca484
0a0e23fedf style: use Self:: instead of type aliases in error impls (frankomosh)
Pull request description:
Replace `use FooError as E` with `Self::` , during pattern matching
on the same type in Display/Error trait implementations.
close#4536
ACKs for top commit:
tcharding:
ACK 0a0e23fedf
apoelstra:
ACK 0a0e23fedf3552df677b379d86a1e0ac6b063152; successfully ran local tests
Tree-SHA512: 25c45a890635f990afc3bc09096929f8793007852ac435f061348bff2bd79e3faabf034d1e1e277596f4f7365477f477798f1b1e0c4b918c5b0fa08f8c49e732
395252c6d9 Fix FeeRate::checked_add/sub (Tobin C. Harding)
Pull request description:
Currently the `checked_add` and `checked_sub` functions use a `u64` for the right hand side. This leaks the inner unit because the RHS value is implicitly in the same unit.
Make the functions have `FeeRate` on the RHS.
ACKs for top commit:
apoelstra:
ACK 395252c6d95fdd581ce5e8ad5d7978515aafea23; successfully ran local tests; will one-ACK merge as this is units-only
Tree-SHA512: 1f0893f75c47f720ac741ace0274171ed24efcb6d2724d0fed941899d43b213e165b97aa050d6e40eea78c527af45d090a81b9d6cbd95835ef7105585786fca6
c63f80baec Remove impl From<u64> for FeeRate (Tobin C. Harding)
Pull request description:
This function leaks the inner unit of `FeeRate`. We want to change the unit, best to break downstream so they notice.
ACKs for top commit:
apoelstra:
ACK c63f80baec0780622d70e4c8699369b0a972cb62; successfully ran local tests; will one-ACK merge as this is units-only
Tree-SHA512: 5748f2a4cb29d6554226fe20c5479cb53081da8c2788ac31abfe1a2edb4d17f13a3b3037a840fbdc29e842af3e1accc27835fd5429c7355c1351eb8883943fdc
Currently the `checked_add` and `checked_sub` functions use a `u64` for
the right hand side. This leaks the inner unit because the RHS value is
implicitly in the same unit.
Make the functions have `FeeRate` on the RHS.
We have 4 functions, 2 ceil, 2 floor but they are ordered ceil, floor,
floor, ceil - this causes my head to twitch when I read it.
The logic in the floor versions is easier so put them first, this is
uniform with `fee_rate/mod.rs`.
Code move only.
When constructing an `Amount` it was observed recently that a `u32` is
often big enough. For the same reason we can use a `u32` to construct a
fee rate instead of overflowing.
Use `_u32`in the constructor and remove the panic.
b538a10956 Add deprecated functions to mutants exclude list (Jamil Lambert, PhD)
fd0a756344 Add tests to relative locktime (Jamil Lambert, PhD)
24cc059a78 Add tests to result (Jamil Lambert, PhD)
c1d2f0386d Add tests to block (Jamil Lambert, PhD)
Pull request description:
Weekly mutation testing found new mutants.
Add tests to kill the valid mutants.
Add deprecated functions to the exclude list so they are not mutated.
Closes#4488, Closes#4528
ACKs for top commit:
apoelstra:
ACK b538a1095652f535aeb15f6e3bbc44969db1ea88; successfully ran local tests
tcharding:
ACK b538a10956
Tree-SHA512: 8393ded6c073b2580fbb0fde9a8ce702a3d1e8c581c035870c2ba6a12d718cee577e345c9d92d0761552765248a6fb5ae9bbacbc88cac75e7153516de46de4ca
29b12bec6b Remove Display/FromStr for FeeRate (Tobin C. Harding)
Pull request description:
Parsing and displaying strings is a PITA. `FeeRate` is likely not that heavily used at the moment and users can always just call `to_sat_per_kwu()` to format it.
So we can get the `units-1.0-alpha.0` release out the door just remove the `Display` and `FromStr` impls for now. They can be added back in later when we have time to get #4339 in.
ACKs for top commit:
apoelstra:
ACK 29b12bec6b34148d6df9a4e6207132a667c53b4c; successfully ran local tests
Tree-SHA512: ffb49ab5d4f98be603eb1ca2f2e9d28ff7eaae66607eb9d2d5fef1f98ba2ac284a0007a86c3cae88f06e5b44f1e3e3ecb2014323b82ad4007e8ec59d6d04759b
Parsing and displaying strings is a PITA. `FeeRate` is likely not that
heavily used at the moment and users can always just call
`to_sat_per_kwu()` to format it.
So we can get the `units-1.0-alpha.0` release out the door just remove
the `Display` and `FromStr` impls for now. They can be added back in
later when we have time to get #4339 in.
9a9b41008f units: Make block-related types have private inner fields (Tobin C. Harding)
Pull request description:
We recently added a bunch of types in the `block` module that are wrappers around `u32`. When we did we slapped `pub` on the inner fields.
Lets be more mindful and make the inner fields private. Note all these types have `to_u32`, `from_u32` and `From` in both directions.
ACKs for top commit:
apoelstra:
ACK 9a9b41008f72cb8a00cfed2890d839aea2ea27c7; successfully ran local tests
Tree-SHA512: d4719bef57944000142ec110d4701486b4a7e5c5b24426379ed596ad83555ca1f75191fe618cc62f12b78b9d7a1ac5d18eff96bc407c9688b1210529c25329b3
We recently added a bunch of types in the `block` module that are
wrappers around `u32`. When we did we slapped `pub` on the inner fields.
Lets be more mindful and make the inner fields private. Note all these
types have `to_u32`, `from_u32` and `From` in both directions.
e2d03fef72 units: Manually implement serde traits (Tobin C. Harding)
Pull request description:
For types that have private inner fields we would like to manually implement `serde` traits instead of deriving them.
This hardens the crate against making future mistakes if we want to change the inner fields.
Do so for:
- `Weight`
- `BlockTime`
- `NumberOfBlocks`
- `NumberOf512seconds`
ACKs for top commit:
apoelstra:
ACK e2d03fef72f4fcbdacd1cbecc0c2dddfbdeb3031; successfully ran local tests
Tree-SHA512: 6d3cc4106e44e08d59f8da6f8f3285923408ed05d1c0b2f6f2d83a22718744a38bf67381177856b5713810a5f5318d929daa2d12447c722628e0576fcd075e3d
For types that have private inner fields we would like to manually
implement `serde` traits instead of deriving them.
This hardens the crate against making future mistakes if we want to
change the inner fields.
Do so for:
- `Weight`
- `BlockTime`
- `NumberOfBlocks`
- `NumberOf512seconds`
According to the `serde` docs:
> This is used in error messages. The message should complete the
> sentence “This Visitor expects to receive …”, for example the message
> could be “an integer between 0 and 64”. The message should not be
> capitalized and should not end with a period.
Use a lower case character to start the string.
According to the `serde` docs:
> This is used in error messages. The message should complete the
> sentence “This Visitor expects to receive …”, for example the message
> could be “an integer between 0 and 64”. The message should not be
> capitalized and should not end with a period.
However we have the `expecting` str using the converted type not the
thing the visitor expects.
Use `u64` instead of `FeeRate` since that is what is being parsed. Note
that in `amount` we got it _almost_ correct, subsequent patch will fix
the case.
fe577cd04e Implement Div<NonZeroU64|I64> for Amount, SignedAmount, FeeRate, and Weight (frankomosh)
Pull request description:
The pr implements `Div<NonZeroU64>` and `Div<NonZeroI64>` for the following types in `units` crate: `Amount`, `SignedAmount`, `FeeRate`, `Weight`
For handling owned/borrowed variants, each impl is wrapped in the existing `impl_op_for_references!` macro, which generates: `T / NonZero*` , `&T / NonZero*`, `T / &NonZero*`, `&T / &NonZero*`
close: #4442
ACKs for top commit:
Kixunil:
ACK fe577cd04e
apoelstra:
ACK fe577cd04e64488371aa62b872e2b88050d4948f; successfully ran local tests
tcharding:
ACK fe577cd04e
Tree-SHA512: b9b4a9f46d2fcf559d0a7f62ec397b6c2b174dd8ca9d80b37c0c393894ab4bc32019d64e2afcad1f780442b16aa3d15240ce607fc14b5e17b112243f7556b5b4
Adds an implementation of div by NonZeroU64 for Amount, FeeRate, and Weight
types. Also adds a div by NonZeroI64 for SignedAmount. The operations
helps to prevent div-by-zero errors at compile time, rather than runtime.
It follows same pattern as existing div operations
but leverages safety guarantees offered by non-zero types
a66ff8f8b1 Introduce `map` function for `NumOpResult` (Shing Him Ng)
Pull request description:
Closes#4476
ACKs for top commit:
Kixunil:
ACK a66ff8f8b1
apoelstra:
ACK a66ff8f8b143ebebb80ddc6b052cbc55e8dbb070; successfully ran local tests
Tree-SHA512: 63c79666685895033b9df0c46004fa4b042d038cc61e5ef443f56690be268ac6dd1ba461ab4f7d97c684e68623dfa53cdd37091f40ff7e6a5d3e53920c3fd40c
fd90c8782a Mark method as constant (yancy)
Pull request description:
Allow external const calls to access this method
Followup from https://github.com/rust-bitcoin/rust-bitcoin/pull/4428
ACKs for top commit:
tcharding:
ACK fd90c8782a
apoelstra:
ACK fd90c8782ab602861a715c72ca1481ed963e5c39; successfully ran local tests
Tree-SHA512: 2d04da9bddd58040972942e70096b2714405740236889f0909c00cb6993e38f3ae51ff05bbda13792a31243d3598f5976649590eecb0572c4f00c166f717399d
a7d059151e Assert error type (yancy)
2f7e74da45 Add MathOp helper methods (yancy)
Pull request description:
Follow up from https://github.com/rust-bitcoin/rust-bitcoin/pull/4428 to assert the error type which I agree improves the test.
Added some helper functions since it can be nice to see what type of overflow happened.
ACKs for top commit:
tcharding:
ACK a7d059151e
apoelstra:
ACK a7d059151eb47bf4202302604309c95c0d66371d; successfully ran local tests
Tree-SHA512: e1b3eba640de2e4f98e075270fd797582601c541e1eebef2959a4e609bf51129e8ad38baab1253b40474c39f82ee4802658ec545cc5c3590637f2ddb13f873f7
Name the type exactly what it is. This used to be `Time`, then we tried
`MtpInterval`.
Note that this makes some of the original function names overly verbose
e.g., `NumberOf512seconds::from_512_second_intervals()` but given the
curlyness of locktimes too verbose is better than too terse. Also this
type, along with `NumberOfBlocks` is not going to be in very wide use so
the ergonomic hit is worth the additional clarity.
Name this type exactly what it is. Note for the error we just use
'height' even though this is a bit stale but the general concept is ok
in the error type because the name is long already.
47c77afaac units: delete MtpAndHeight type (Andrew Poelstra)
d82b8c0bcb primitives: stop using MtpAndHeight (Andrew Poelstra)
72d5fbad73 units: stop using MtpAndHeight in locktime::relative is_satisfied_by methods (Andrew Poelstra)
d933c754f5 units: change type of MtpHeight::to_mtp to BlockMtp (Andrew Poelstra)
dcbdb7ca8a units: add checked arithmetic to Block{Height,Mtp}{Interval,} (Andrew Poelstra)
4300271f0c units: add constructor for absolute::Mtp from timestamps (Andrew Poelstra)
4e4601b3d5 units: rename BlockInterval to BlockHeightInterval (Andrew Poelstra)
cb882c5ce1 units: add global `BlockMtpInterval` type (Andrew Poelstra)
4e3af5162f units: add global `BlockMtp` type (Andrew Poelstra)
a3228d4636 units: pull u32 conversions for BlockHeight/BlockInterval into macro (Andrew Poelstra)
Pull request description:
This is a more involved PR than I'd expected but hopefully the individual commits make sense and are well-motivated. Essentially, my goal was to replace `MtpAndHeight` as used by relative locktimes with a pair of `Mtp` and `Height`.
However, relative locktimes, when given a MTP/Height for the UTXO creation and the chain tip, are roughly modeled as "take a diff of MTPs to get a `relative::MtpInterval`, a diff of heights to get a `relative::HeightInterval`, and compare to the locktimes". *However*, we have no standalone MTP type to "take a diff of", and also there are failure modes when creating the diffs (e.g. if the diff would exceed the range of `MtpInterval` or `HeightInterval`).
So I backed up and decided to use the existing `BlockHeight`/`BlockInterval` as the type to "take a diff of". I needed to introduce a `BlockMtp`/`BlockMtpInterval` to work with MTPs. These types have full-u32 range, unlike the similarly-named types in `units::locktimes::absolute`. I then needed to add some conversion methods. Along the way, I cleaned up the APIs and documentation, added checked arithmetic, etc., as needed.
See the individual commit messages for more detail.
I believe the resulting API is much more consistent and discoverable, even though it has more surface than the old API.
I considered splitting this into 2 PRs but I think the first half of the changes aren't well-motivated with out the second half. Let me know.
ACKs for top commit:
tcharding:
ACK 47c77afaac
Tree-SHA512: ebe19a5b1684db8c2d913274347c994026aaa0dcdd79349c237920a82fe55560777278efdbbc7f1b1424c9391d9bbd891ae844db885deea75288000437a8a287
13cbead947 Use NumOpResult instead of Option (yancy)
002a0382aa Mark function constant (yancy)
Pull request description:
Prefer the more descriptive NumOpResult return type over Option where return types are fallible.
Closes https://github.com/rust-bitcoin/rust-bitcoin/issues/4419
ACKs for top commit:
apoelstra:
ACK 13cbead94766987f59482b1fbc1d0ebd0799737c; successfully ran local tests
tcharding:
ACK 13cbead947
Kixunil:
ACK 13cbead947
Tree-SHA512: 1a870962dcafe901a07abd93bd8075e41696341c1a4b3efef615493c73d5e5728bbc2326f8c2c95b9034ab001d0b3c668c9d64793ab03486d3a19f31df907c96
52940d4e12 Prefix unused variables with _ in rustdocs (Jamil Lambert, PhD)
a852aef4b8 Remove unused imports in rustdocs (Jamil Lambert, PhD)
Pull request description:
There is a lint warning about unused variables and imports in the rustdoc examples.
Remove the unused imports and prefix the unused variables with an underscore.
ACKs for top commit:
apoelstra:
ACK 52940d4e1216ad5118f7980cb2e6b8b425c61589; successfully ran local tests
tcharding:
ACK 52940d4e12
Tree-SHA512: 953862d546dc6e0bcd64172e8b383f0fc2a1a851971a1bcad0c1e30cbaeeaea993a0de7dd8b424c4ac1410053e179c52d0b5c90cd1b6560c27123b6b7fa49732
Fmt Bot
Tobin C. Harding
merge-script
frankomosh
Jamil Lambert, PhD
Shing Him Ng
yancy