The `untis::error` module is just a code organisation thing it should
never have been public. We already re-export all the error types and
this is verified by the `units/tests/api.rs` test file.
Make the module private and remove it from the paths in the `api` test.
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
f4617e71f5 kani: Verify no out of bounds for ArrayVec (Tobin C. Harding)
e378cdd8fa kani: Don't bother checking signed to unsigned conversion (Tobin C. Harding)
50224eecc2 kani: Don't overflow the tests (Tobin C. Harding)
Pull request description:
PR does two things because a recent upgrade of `kani` broke our setup. I'm not sure why it just showed up.
We fix the verification for `amount` types which recently broke because of `MAX_MONEY` changes and then we add a verification function to `internals` because build fails and one fix is to just add something.
- Patch 1: units: Assumes correct values
- Patch 2: units: Removes a stale check now that MAX_MONEY is being used for MAX
- Patch 3: Add verification to `internals::ArrayVec`.
ACKs for top commit:
apoelstra:
ACK f4617e71f5fd074000d1e3a9376644c744210562; successfully ran local tests
Tree-SHA512: dfef05a7bbb5372415efa8acab7f79801aa7326ac298c007b173786f00bcccd0b1b81d327113723c359fb2797895414a586cc3fb86e495476a03fcac02a96899
f08d8741d3 Test types MIN/MAX instead of i64::MIN/i64::MAX (yancy)
Pull request description:
The MIN/MAX for SignedAmount recently changed from i64::MIN and i64::MAX to MAX_MONEY/MIN_MONEY. Update the tests to reflect this new MIN/MAX since it is no longer valid to create a value above or bellow MAX_MONEY/MIN_MONEY.
ACKs for top commit:
apoelstra:
ACK f08d8741d39685b636830680bb891bd414826e88; successfully ran local tests
tcharding:
ACK f08d8741d3
Tree-SHA512: 563408240dffaf95f88a9d570e56f9b9b161b422cb59a89828c18b9c784e7acb717f57fe55c80411f104443ac2a3f908f2a98ab1a4b34edab69b6946a723b30c
2513e05501 api: Run just check-api (Tobin C. Harding)
9619f68956 units: Move excluded lints to manifest (Tobin C. Harding)
5290a93a38 units: Add all pedantic lints (Tobin C. Harding)
Pull request description:
Add all pedantic clippy lints but leave a few marked as TODO.
The coment on the list claims to it is an exhaustive list of pedantic lints, that claim is going to go stale. I would be nice if we had tooling to catch new lints as they were added.
ACKs for top commit:
apoelstra:
ACK 2513e05501e3a014c097f24eb9178c291785db81; successfully ran local tests
Tree-SHA512: 33b2a7448d49d6a5571c9e4e9922b6042ab03aaaa9f7acad243a926f8a03a0ffed75d4f5f37be4705f23862c32f96879582214cd17c7e5ab81e47517a84745e0
Amount add and sub now enforce the MAX_MONEY invariant when doing
addition and subtraction. We need to tell kani to assume we don't
overflow before doing actual tests.
Note also that `ops::Add` calls through to `checked_add` and
`ops::Sub` calls through to `checked_sub` so separate kani tests for
these are unnecessary.
Add all the pedantic lints to the repository by way of the repository
manifest. Then enable these lints in the `units` manifest.
Some things worth mentioning:
- Fix `needless_pass_by_value` by adding derives to `FormatOptions`.
- Fix lint `cast_lossless` using `cargo clippy --fix``
- While fixing `lint enum_glob_use` introduce a new style to the
codebase; import enums using a single character. Doing so prevents
namespace clashes, improves clarity, and maintains terseness.
Audit:
Use the following lints locally and audit all the warnings, they produce
many false positives so we can't enable them permentently.
- `cast_possible_truncation`
- `cast_possible_lint`
- `cast_sign_loss`
The MIN/MAX for SignedAmount recently changed from i64::MIN and
i64::MAX to MAX_MONEY/MIN_MONEY. Update the tests to reflect this new
MIN/MAX since it is no longer valid to create a value above or bellow
MAX_MONEY/MIN_MONEY.
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
183ecf1fe6 Add units tests for cBTC (Tobin C. Harding)
Pull request description:
We added the "cBTC" denomination but forgot to test it.
Add units test for "cBTC" as we do for the other denominations. Put all the tests lines of code in the same order as the enum variants so a reader can see easily that all cases are tested.
ACKs for top commit:
apoelstra:
ACK 183ecf1fe6944187151036e36d6726fae9b64392; successfully ran local tests
Tree-SHA512: 93a3f3e5430f7b6f525d7c55ad3d318f8b30fef8cdef24ea92a1687e898f73d3829875861f026af70dde013743d310d87331b5654b524f4ee90c20f7e6ed7f7c
Add macros for implementing `ops::Add` and `ops::Sub` for various
combinations of references. Use the macro in `fee_rate`.
These are internal macros so no need to protect `core` using
`$crate::_export::_core`.
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
f97c04cf07 api: Run just check-api (Tobin C. Harding)
99b32c95b8 Change paramater type used for whole bitcoin (Tobin C. Harding)
Pull request description:
We have multiple constructors for `Amount`, one of which accepts an integer representing whole bitcoin.
In an attempt to make the API crystal clear change the parameter type used for whole bitcoin to `u32` instead of a `u64` which may well be mapped to sats in the mind of developers (it is in mine anyway).
ACKs for top commit:
apoelstra:
ACK f97c04cf07c2ac5acb5d312b573d100afa009b57; successfully ran local tests
Tree-SHA512: fb22f08f182a96ff8d067337e32174137e3d73acd2faeddc2bac974ea41f8de9ec06baa2e6fb76a2fa1a7afa7bf6b4c8f606666d966413a14d39b129e36d0674
We added the "cBTC" denomination but forgot to test it.
Add units test for "cBTC" as we do for the other denominations. Put all
the tests lines of code in the same order as the enum variants so a
reader can see easily that all cases are tested.
We have multiple constructors for `Amount`, one of which accepts an
integer representing whole bitcoin.
In an attempt to make the API crystal clear change the parameter type
used for whole bitcoin to either `Into<u64>` or `u32` instead of a
`u64` which may well be mapped to sats in the mind of developers (it
is in mine anyway).
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.
These functions do not add value because one can just call `to_sat` and
`from_sat` if doing ops in a tight loop.
Note we need to remove these deprecated functions before we release
`units v1.0`.
55092aab47 Use Self in amount consts (Tobin C. Harding)
Pull request description:
Currently the consts in the `amount` modules repeat the type. For `Amount` this isn't a big deal but for `SignedAmount` its quite noisy.
Use `Self` as the type in const declarations inside `Amount` and `SignedAmount`.
Internal change, no logic changes.
ACKs for top commit:
apoelstra:
ACK 55092aab47051e740c1ee43de73e367a2f9299a3; successfully ran local tests
Tree-SHA512: 8220541f3bac85f0a40382e9545500709dac6d96496cfae76c0ff41afd937b6452e3fa12390b38f80e586a2b52baa384c580c5fe3e8ac9e80ea394b3b3ee26db
01b58d668c Add rustdoc examples (Jamil Lambert, PhD)
b881f1b54e Add `# Errors` to rustdocs (Jamil Lambert, PhD)
66f36b3048 Correct doc errors and add links in `unsigned` (Jamil Lambert, PhD)
Pull request description:
Update rustdocs in `units::Amount` to conform to the API guidelines.
`# Examples` API guideline quote:
> "is not always to show how to use the item. ... Rather, an example is often intended to show why someone would want to use the item."
This guideline is subjective, and in the cases in `Amount` the why is often trivial. Where the example is trivial it has been included once, and for similar functions left off e.g. included for `from_sat` but not `from_btc`, etc.
ACKs for top commit:
tcharding:
ACK 01b58d668c
apoelstra:
ACK 01b58d668cf03dda6aeff7c556e359436ccc4132; successfully ran local tests
Tree-SHA512: 5dde89949e0091f166b9903d28ca44275ca3d283de4ef1b6142dba88f0c45e9ba0a8f7cefa0c36a0f1e3e9e2b61d4992f0d3324d30c09b9877b481e9f343002a
4cccbfdf76 units: Seal the Integer trait (Tobin C. Harding)
Pull request description:
This trait is an internal thing, users should never implement it.
ACKs for top commit:
sanket1729:
utACK 4cccbfdf76
apoelstra:
ACK 4cccbfdf7628442cc198f6399846ad4e98938494; successfully ran local tests
Tree-SHA512: f48e8ef96d15135990976c77e933ddc735dda577464548478526e37fff49bf453aaacf966967d98a0d5598588b531199627af8e353f845d4c7781f1e1d8de6db
463fbd16f1 Update to arbitrary v1.4 (Tobin C. Harding)
Pull request description:
Out with the old in with the new, update to the latest minor version of arbitrary.
No real reason to do this except that I wanted to add the minor version instead of using just `1` so that we don't accidentally pull a new minor version in during a patch release (after we 1.0).
(I'm unsure if this is necessary and did not test it against MSRV.)
ACKs for top commit:
apoelstra:
ACK 463fbd16f1431febe7e4fc50abd7415886542109; successfully ran local tests
Tree-SHA512: 88913f993b97abba224ba94ee9d5057894bb7142fef07e557de03a423da314a9f21632932fb64c4c27c676051bbb6e90d9b90bfa944af810e834bf42707d3cff
3c8c956511 Remove Weight::from_wu_usize function (Tobin C. Harding)
Pull request description:
This constructor is an anomaly in this repo. Also it is ugly to have the type parameter hard coded in the function name.
The problem the constructor is trying to solve is that we don't like casts that don't obviously loose data. We have a solution for that already in `internals::ToU64`. This trait cannot be used in const contexts though so we do introduce a single cast in this patch.
ACKs for top commit:
apoelstra:
ACK 3c8c95651169fb9329ceb380162721c2d2f9b3aa; successfully ran local tests
Tree-SHA512: 168196edb7c151378d425b96ea3c9cdb0a4f6a879543e89facd02ed1fdf9bc69bde8ef862ffa0959b7c5ca21d6f4fe5ae38a933c379e7e88a946ca7cb68d61ec
edfdd575d5 Test all the valid denomination forms (Tobin C. Harding)
Pull request description:
Exhaustively ...
ACKs for top commit:
apoelstra:
ACK edfdd575d5af1c676c0640f1e38056d7da9168e0; successfully ran local tests
Tree-SHA512: 76d078988b68f68cf8d4f18439b94cfef6b8b222596aa0b64e5270797c1d7d0f15b6376fb81d1655638e2617c33747d8a5623d3f52db0c95061723a3e36ce7de
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
Currently the consts in the `amount` modules repeat the type. For
`Amount` this isn't a big deal but for `SignedAmount` its quite noisy.
Use `Self` as the type in const declarations inside `Amount` and
`SignedAmount`.
Internal change, no logic changes.
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
Tobin C. Harding
merge-script
Shing Him Ng
Jamil Lambert, PhD
yancy
Fmt Bot