60f2089dcd Clean up possibly confusing (Tobin C. Harding)
Pull request description:
Put the list of possibly confusing forms in the same order as the enum and fix the rustdocs to show the actual error variants returned.
ACKs for top commit:
apoelstra:
ACK 60f2089dcdd697b8ab53c9f96b2192ca030237ac; successfully ran local tests
Tree-SHA512: 109fd3e9b72860c3e2a1268aeafab30d6b2b6fac5d5835db6c216a434b9e175911e6445c187f6b80ab2d8c54184df9338230985a4e030aa146cd9fa410971216
7725ca77c5 Rename private module to sealed (Tobin C. Harding)
Pull request description:
There are two `private` modules in `amount` but they do slightly different things. One provides a private `Token` and one is for trait sealing. We have various other trait sealing modules in the codebase and they are all called `sealed` not `private`. Also the seal trait is called `Sealed`.
Rename the `private` module and the trait to be uniform with the rest of the codebase.
ACKs for top commit:
apoelstra:
ACK 7725ca77c589a5215c50f1634f1b095d3e268540; successfully ran local tests; sure
Tree-SHA512: 5953686d7d22daaad8d2d59eff2338db3bb2a7765a37c08ed02ae1a4622509d628dbcb971781a6e85d750afa58609f5b058dfce6d5b4066f4f0d8ded45375b5b
a4a0f2921c units: Add regression tests for Default impls (Tobin C. Harding)
f5c2248a31 units: Derive Default for BlockInterval (Tobin C. Harding)
Pull request description:
Done while looking into [C-TOR](https://rust-lang.github.io/api-guidelines/predictability.html#c-ctor)
- Derive `Default` for `BlockInterval`
- Add regression test for all types that implement `Default`
ACKs for top commit:
apoelstra:
ACK a4a0f2921cd2425fa46a8ade42f888d5268e6709; successfully ran local tests
Tree-SHA512: ce39c2bcb37fc0fa70bd2553dd7843d9ac8f9528631706056ba89fda9623defadf32974091832b7a087121290b3f883bc8a8dcca070f1d90670eeee6b541de01
a2cab6f925 units: Add _export::_core (Tobin C. Harding)
Pull request description:
As we do in `bitcoin` add a module for usage in macros to prevent naming clashes if a module called `core` exists.
Overly paranoid yes but this is bitcoin after all.
ACKs for top commit:
apoelstra:
ACK a2cab6f925530b7e920362149ac4a472abf035c8; successfully ran local tests; lol sure
Tree-SHA512: f20d5b7aae55370891a2943cf3be6e04cb24a93f570093252eefee548f3e13931d03cfaef14613df2a75c477476c98f33173bd4a6da3a15beb7dc4e01648f574
38c329c1b7 Add additional catagories (Tobin C. Harding)
Pull request description:
Add `no-std` category to the soon-to-be-released leaf crates. Also add `cryptography` to the `bitcoin_hashes` crate.
Close: #3731
ACKs for top commit:
apoelstra:
ACK 38c329c1b7e393b040da02447c5edfe8946c50cf; successfully ran local tests
Tree-SHA512: e4481c61028b42876ac4ff62d7f1f97116c49a72f99aa0f75fe158c524d021518bf3334ea181fdc9979765bbe3ef2167a7999396898471e88299814e292a9949
4b926e1908 Change`MAX and `MIN` to equal `MAX_MONEY` (Jamil Lambert, PhD)
Pull request description:
To prevent rounding errors converting to and from f64 change `SignedAmount` `MAX` and `MIN` to +/- `MAX_MONEY` which are within the limit in f64 that has issues.
Add checks to `from_str_in`, `checked_add`, `checked_sub` and `checked_mul` that the result is within MIN and MAX.
Modify tests to work with new `MIN` and `MAX`.
Discussed in #3688 and #3691
ACKs for top commit:
tcharding:
ACK 4b926e1908
apoelstra:
ACK 4b926e1908f72af98d24cc64d7e1eef44d624e4e; successfully ran local tests
Tree-SHA512: 9053e761b3b74a7a9d826ae7271ced41cf7919752ac8ed8977a20b5b1a746ac8a6bfff68159f4a0dea733ea00f49cf41c0422de53c7aff39efd8482f8cba6069
There are two `private` modules in `amount` but they do slightly
different things. One provides a private `Token` and one is for trait
sealing. We have various other trait sealing modules in the codebase and
they are all called `sealed` not `private`. Also the seal trait is
called `Sealed`.
Rename the `private` module and the trait to be uniform with the rest of
the codebase.
A block interval is a relative thing so it makes sense to default to
zero. This is the same as how we derive `Debug` for `relative::Height`
but not `absolute::Height`.
As we do in `bitcoin` add a module for usage in macros to prevent naming
clashes if a module called `core` exists.
Overly paranoid yes but this is bitcoin after all.
2fd8614f5d units: Add additional must_use (Tobin C. Harding)
79a229b391 units: Add pedantic lint return_self_not_must_use (Tobin C. Harding)
Pull request description:
Enable return_self_must_use and also run the linter locally with must_use_candidate.
Add must_use attribute as required excluding obvious functions (conversion, getters, etc).
Done as part of https://github.com/rust-bitcoin/rust-bitcoin/issues/3185
ACKs for top commit:
apoelstra:
ACK 2fd8614f5d091377f179440b04ec71f4853fd187; successfully ran local tests; nice! will one-ACK merge
Tree-SHA512: 90868846d6877830ca2b6931e8b94208434acc5a7b21808a32e2e1568c0ad33185644b931cae500e6619b8b4f19c39bce6922502d9233173722ef0e6455edba6
adaf4ac086 Set avoid-breaking-exported-api to false (Tobin C. Harding)
Pull request description:
These lints are valuable, lets get at em.
ACKs for top commit:
apoelstra:
ACK adaf4ac086553674803fadfde776d6dd013a7964; successfully ran local tests; will probably be a problem repeatedly for the next few days until we get through all currently-open PRs
Tree-SHA512: 56617a2f4aeafceef72008232cee817d45b62c040d7f1938631291c5d73627851cfbefc4b4000cd380ecb5c7e1379d1022f6cc90a4b68c819c78fb883bee0b3a
To prevent rounding errors converting to and from f64 change
`SignedAmount` `MAX` and `MIN` to +/- `MAX_MONEY` which are within the
limit in f64 that has issues.
Add checks to `from_str_in`, `checked_add`, `checked_sub` and
`checked_mul` that the result is within MIN and MAX.
Modify tests to work with new `MIN` and `MAX`
c27f443520 Add basic unit tests for Amount serde (Tobin C. Harding)
22530f6a2b Support serde serializing Amount as string (Tobin C. Harding)
Pull request description:
Sometimes JSON parsers may munge floats. Instead of using `f64` we can serialize BTC amounts as strings.
Close: #894
ACKs for top commit:
apoelstra:
ACK c27f4435208cc3ca7b98580fd7e2784e089b545e; successfully ran local tests
sanket1729:
utACK c27f443520.
Tree-SHA512: 084669a0622557b75fceae732fb485e7139ecada48c0b65642d122e1a02f6f7e41564c3579fd10adbf3aa14c82c9f10abc3f9201858e50b729852140b31a4216
These lints are valuable, lets get at em.
Changes are API breaking but because the changes make functions consume
self for types that are `Copy` downstream should not notice the breaks.
Use the `must_use_candidate` clippy lint to find all functions that are
candidates for having `must_use`.
Add `must_use` attribute but exclude obvious functions like `from_`,
`to_`, and `new`.
This patch is subjective.
ac74ed2144 Range check against SignedAmount::MAX instead of i64::MAX (yancy)
Pull request description:
Future proof this check by using SignedAmount::MAX in the case where the MAX SignedAmount changes to something other then i64::MAX.
ACKs for top commit:
tcharding:
ACK ac74ed2144
apoelstra:
ACK ac74ed2144e785fef7c395388a4fb7fb394e833e; successfully ran local tests; nice. Simple and obviously an improvement
Tree-SHA512: 4003a2f3b34e03330c57125622cab5e55a235b1a610dda622035c071bc5530811e275c2e25f40e1309cecf1c3bef35070ae690fa57fdf3e2c1b5c3f75ca5d29e
ffdd63fa8e units: test for C-SEND-SYNC (Tobin C. Harding)
Pull request description:
Done as part of the Rust API guidelines checklist.
Add a unit test for `Send` and one for `Sync`.
ACKs for top commit:
apoelstra:
ACK ffdd63fa8ec3575bc3087241ebdcbccc71818ab7; successfully ran local tests; ooh, I like how extensible this API test framework is!
Tree-SHA512: 9227ad487f77596964c728deee97c62a04490510a8ab69fd3fc29a3e400b37114e27c777cf868fe58de870a9ff0aca3d234ccf2bb93d69a709e8c9b85d44fc61
6950c0a7b5 Change `Amount::MAX` to equal `MAX_MONEY` (Jamil Lambert, PhD)
Pull request description:
As discussed in #3688 and #3691 using `u64::MAX` causes errors when converting to `f64` so `MAX_MONEY` is should be used as the maximum `Amount`.
- `Amount::MAX` changed to equal `MAX_MONEY`
- Add a check to add, multiply and from_str functions
- Change tests to account for new lower maximum
Different approach to the existing PR #3692. I have only done Amount and not SignedAmount until there is feedback on which approach to use.
ACKs for top commit:
tcharding:
ACK 6950c0a7b5
apoelstra:
ACK 6950c0a7b507f9d70c1ebdab540634482f73b247; successfully ran local tests
Tree-SHA512: 03ebf39c47b19ba88d95235538039f28bfa29f4499618fab25c9b627684c348ed41caef682e8f0e01ca62cf9ced8a1183fe3ed861bffeb9609b09440ddfb1c92
To prevent rounding errors converting to and from f64 change
`Amount::MAX` to `MAX_MONEY` which is below the limit in f64 that has
issues.
Add checks to `from_str_in`, `checked_add` and `checked_mul` that the
result is below MAX, where previously a u64 overflow was relied on.
Change tests to account for new lower MAX that is within the range of
SignedAmount and does not overflow so easily
Remove overflow tests
`Amount::MAX` is now below `u64::MAX` and within the range of values for
`SignedAmount`. These tests therefore do not overflow.
In effective_value there is no error with `Amount::MAX` and the correct
value is returned.
In psbt the removed test is effectively the same as the previous test.
Modify `Amount` tests to work with new `MAX`
Tests need to be changed that checked values above the new `MAX` or
`Amount::MAX` was out of range for `SignedAmount` which it isn't anymore
Needs release notes but there are 20 pages of them, that feels like too
many to go through manually.
Includes a missing changelog entry in `units` for an already released
change that is included in this `bitcoin` release.
Sometimes JSON parsers may munge floats. Instead of using `f64` we can
serialize BTC amounts as strings.
Includes addition of `alloc` feature gate to `DisplayFullError` to
remove lint warnings when building with `--no-default-features`.
Close: #894
fd2a5c1ec7 Close amounts error types (Tobin C. Harding)
23c77275b1 Reduce code comment lines (Tobin C. Harding)
d595f421c6 Remove whitespace between enum variants (Tobin C. Harding)
Pull request description:
Close the two pubic enum error types in `units::amounts`. All the other structs are closed already because they either have private fields or marked `non_exhaustive`.
ACKs for top commit:
apoelstra:
ACK fd2a5c1ec79f337fb3695c030c9fb6b4671468f2; successfully ran local tests; thanks!
Tree-SHA512: f8d68ef821449e0829c926cf527df4b226b29c8d1d41b320a016fbf70b4b39cc54c8c218955caa0c3776158eeeae0ebacc1cc89dab67bafc399b94063324ab0e
Close the two pubic enum error types in `units::amounts`. All the other
structs are closed already because they either have private fields or
marked `non_exhaustive`.
433f70939c Implement iter::Sum for BlockInterval (Tobin C. Harding)
0369e64b56 Implement Sum for an iterator of references to amounts (Tobin C. Harding)
31f883ac00 Implement iter::Sum for FeeRate (Tobin C. Harding)
Pull request description:
Enables summing an iterator of values. Note that this does not include either `LockTime`s. `absolute::LockTime` should not be added and for `relative::LockTime` we have https://github.com/rust-bitcoin/rust-bitcoin/issues/3676Close: #1638
ACKs for top commit:
apoelstra:
ACK 433f70939c3ecc10702ab6502e3f9bcd94dab739; successfully ran local tests; nice!
sanket1729:
utACK 433f70939c
Tree-SHA512: 1eda00f3bbbc61f795198ce8525a5a9b690478a8abc268da6d2e40de7d91decc28dd8211df0c6abeaf30148c7ec3907b85e3c5351972c354590569840e84d562
22769268f3 units: Close the hex parse errors (Tobin C. Harding)
Pull request description:
As part of the 1.0 effort close the errors in the `units::parse` module.
ACKs for top commit:
apoelstra:
ACK 22769268f34b45c0bd86c548eb13cfe1e290f0d7; successfully ran local tests; thanks!
sanket1729:
ACK 22769268f3
Tree-SHA512: 598ba236f8c08c3f750112e0d8b8e0aa0568be2700a328e4a2d8999ca59eada8a16a6a1d9e9121e7f42ce9bbe3da3f87221ba67c36056996a687e489f4c6007c
c49f40fd9a units: Test C-COMMON-TRAITS (Tobin C. Harding)
Pull request description:
Add structs to the `api` integration test file that verify the set of common traits.
ACKs for top commit:
apoelstra:
ACK c49f40fd9af1b9446a7c9a35aefdc02b30801275; successfully ran local tests; good idea!
Tree-SHA512: bc60ed8912e705d4c07714e9d19c0eee4fb2eb6be17a6ee67c2bb922ea27ef3a737eda3f7de690fa3a9fa364c34029989f20361d03f706fd3152ec4f20f33aab
7e17eaf21c units: Add stringy regression tests (Tobin C. Harding)
Pull request description:
Add regression tests for `Display` and `FromStr` impls. Exclude error types and helper types (`amount::Display`).
Done as part of #2498 and also as part of the 1.0'ing effort.
ACKs for top commit:
apoelstra:
ACK 7e17eaf21c479338d5161b493f02578ef4186e62; successfully ran local tests; nice
Tree-SHA512: 6484806777501fe38557987c9c39b377f972fd4406cf3cfd8ac36f8426041484caab45d4ccff87221dbc5c34507d1be6a7b23839367bd3010855c92fd898c835
The `Amount` and `SignedAmount` were not supposed to implement `serde`
traits by design because doing so implicitly uses sats. We provide two
modules `as_sat` and `as_btc` to allow users to explicitly serialize in
their preferred format.
In commit: `d57ec019d5 Use Amount type for TxOut value field` derives
were added for `serde` and we did not notice it during review.
7b8369f381 units: Add integration test of API surface (Tobin C. Harding)
Pull request description:
In an effort to check off items in the Rust API guidelines checklist (#3632) add an integration test file that tests:
- The location of re-exports for various typical usage styles.
- Regressions in the API surface (things being accidentally moved).
- All public types implement Debug (C-DEBUG).
- For all non-error types:
- `Debug` representation is never empty (C-DEBUG-NONEMPTY)
- For all error types:
- Derive standard traits as defined by `rust-bitcoin` policy.
- All data structures implement `serde` traits (C-SERDE).
I used the `cargo check-api` script we have laying around from ages ago (#2986) to parse `units` and get a list of the public types.
ACKs for top commit:
apoelstra:
ACK 7b8369f381c3f3fc15ed536d5316e86e02914b4e; successfully ran local tests
Tree-SHA512: 6fa2a61f6b67a6b5a56950b87e6df68b761b69bd2a49e7aac48aa238cb84441ce04acd85097d28ae4055058052a7491eccda3da218812149a896e548b4018aaa
17ca5018eb units: Comment alloc feature (Tobin C. Harding)
Pull request description:
Add a comment to the `serde` module about why we have all the `alloc` feature gating.
Done as part of #3556 - auditing the whole crate for use of `alloc` feature.
ACKs for top commit:
apoelstra:
ACK 17ca5018ebb28d5d392d61aee62170fb3e4f2a5b; successfully ran local tests
Tree-SHA512: 6d13129903a9c530db811e9f64ae4fa653dbcc952a42502c53c6c622a80c1e58b3037edd0638de78867823ca370b3a4595a08a9ec07b84c71afb6200454f5d6d
In an effort to check off items in the Rust API guidelines
checklist (#3632) add an integration test file that tests:
- The location of re-exports for various typical usage styles.
- Regressions in the API surface (things being accidentally moved).
- All public types implement Debug (C-DEBUG).
- For all non-error types:
- `Debug` representation is never empty (C-DEBUG-NONEMPTY)
- For all error types:
- Derive standard traits as defined by `rust-bitcoin` policy.
I used the `cargo check-api` script we have laying around from ages
ago (#2986) to parse `units` and get a list of the public types.
Jamil Lambert, PhD
merge-script
Tobin C. Harding
yancy
Fmt Bot