In a kani test we are checking if the result of `to_unsigned()` is an
error but setting `is_signed` to `true`, this field represents the
signed-ness of the return type of `to_unsigned` (`Amount`) so should be
`false`.
Fix kani daily job.
The tests currently include the word "add" but they test addition as
well as subtraction. Elect to keep the multiple assertions per test and
just make the names more general.
`cargo` is reporting a redundant import warning:
warning: the item `TryInto` is imported redundantly
Remove the import statement from the `verification` module.
051c358bcb Remove deprecated legacy numeric methods (Divyansh Gupta)
Pull request description:
As `rustc 1.79.0-nightly (9d79cd5f7 2024-04-05)` is released which solves the issue mentioned , but the release has deperacted legacy numeric methods.
Thus replaced `u16::max_value()` etc with `u32::MAX` & `core::u16` to directly `u16`.
fix#2639
ACKs for top commit:
tcharding:
ACK 051c358bcb
apoelstra:
ACK 051c358bcb thanks! I will remove an equivalent commit from my #2669
Tree-SHA512: c08c856f7f3b281417c29283351eac5e0f75cc1c8d23d9aae58d969219a327b2337fe57932053e53773ebb9dbec04254f90149266b6639a66c5c09f2ad1675ef
As `rustc 1.79.0-nightly (9d79cd5f7 2024-04-05)` is released which solves the issue mentioned , but the release has deperacted legacy numeric methods.
Thus replace `u16::max_value()` etc with `u32::MAX` & `core::u16` to directly `u16`.
fix#2639
We just did a minor bug fix to error code in the `amounts` module. This
change did not effect the public API but improved the display of two
error types from that module.
In preparation for doing a point release bump the version number and add
a changelog entry.
We have 2 crates that require an allocator, `bitcoin` and `base58ck` -
these crates should enable the "alloc" feature when depending on
`internals`.
For `units` we use the `internals::error::InputString` but do not enable
the "alloc" feature - this is a bug, it means that the parsed string is
being lost from the error types that use `InputString`.
Enable "alloc" for `bitcoin`, `base58ck`, and `units`.
- `bitcoin` and `base56ck` is just for good measure so we don't get
bitten later on.
- `units` is a bug fix and requires a point release.
In preparation for the initial release add a changelog. Note the version
number is already set to `v0.1.0` and this does not conflict with the
release currently on crates.io `v0.0.0`.
Make an attempt to improve the ergonomics and docs clarity of the
`units` crate.
- Don't inline error type re-exports, this keeps them up in the
"Re-exports" section and saves cluttering the other inlined docs.
- Re-export and inline the docs for `FeeRate` and `Weight` same as we do
for `Amount`. This makes the "Structs" section of the docs nice except
for the exclusion of the locktime types (which cannot be helped).
Move the `strip_hex_prefix` helper function to below where it is called.
Note that I was the original author of this helper so there is no excuse
for it being above - bad Tobin no biscuit.
This lint triggers when parsing a reference to a large struct as a
generic argument, which is wrong.
Allow it crate wide because [subjectively] this lint never warns for
anything useful.
af6dc1db02 internals: Bump version to 0.3.0 (Tobin C. Harding)
Pull request description:
In preparation for release add a changelog and bump the version number.
Please note, the changelog is pretty terse.
ACKs for top commit:
apoelstra:
ACK af6dc1db02
sanket1729:
ACK af6dc1db02
Tree-SHA512: b70d4b9de7de90aba3cbff90dd7f25c5ac801d020dbdfe3e64af4c079347cba726aa783a94fc777e7bf177db8402b54948c2dfd4a766d90c1a7a7a6bdfd36136
af49841433 Hide base58::Error internals (Tobin C. Harding)
4f68e79da0 bitcoin: Stop using base58 errors (Tobin C. Harding)
669d5e8fc6 base58: Add InvalidCharacterError for decoding (Tobin C. Harding)
ec8609393b base58: Add error module (Tobin C. Harding)
42fabbab03 base58: Run the formatter (Tobin C. Harding)
Pull request description:
Improve the error code in the new `base58` crate.
ACKs for top commit:
apoelstra:
ACK af49841433
sanket1729:
ACK af49841433
Tree-SHA512: c05479f02a9a58c7c98fd5987e760288562372e16cceeeb655f0a5385b4a8605945a3b6f7fcf473a7132a40f8dc90d204bc5e9e64fd2cc0bdc37dbcabb4ddc5c
Adds constructors to allow directly creating locktimes from time or
block counts; adds a flooring constructor to Time to match the ceiling
one; adds an explicit constructor to Height since the From<u16> was not
very discoverable.
Currently we are deriving the serde traits for the `absolute::{Height,
Time}` types, this is incorrect because we maintain an invariant on
the inner `u32` of both types that it is above or below the threshold.
Manually implement the serde traits and pass the deserialized `u32` to
`from_consensus` to maintain the invariant.
Close: #2559
When creating the ParseIntError in `hex_u32` I (Tobin) just cargo cult
programmed the generic stuff without thinking.
- The `is_signed` field is used to denote whether we were attempting to
parse a signed or unsigned integer, it should be `false`.
- The `bits` field should be directly set to 32.
d887423efc Enforce displaying Amount with trailing zeros (448 OG)
Pull request description:
It is common to display bitcoins using trailing zeros upto 8 decimals. This commit enforces:
- Displaying Amount in BTC with trailing zeroes by eight decimal places if a precision on the Amount is not specified.
- Displaying Amount in BTC upto the precision specified truncating the insignificant zeros.
- Displaying amount in BTC without any decimals if the remainder of the amount divided by the satoshis in 1 BTC is equal to zero using formula `satoshis.rem_euclid(Amount::ONE_BTC.to_sat()) != 0`
These are not breaking changes and all previous tests pass.
A testcase is added to for changes introduced.
Resolves: #2136
ACKs for top commit:
sanket1729:
ACK d887423efc
apoelstra:
ACK d887423efc
Tree-SHA512: c32e41216477f60a8d95f164bf4a1f6644ea14adc7e169743ce419b0f26ecb6603f3a516f9b18d6508c04ce658f6a0a78ff3b0b062aeb7101b28bbb1e9d522bc
It is common to display bitcoins using trailing zeros upto 8 decimals.
This commit enforces:
- Displaying Amount in BTC with trailing zeroes by eight decimal places
if a precision on the Amount is not specified.
- Displaying Amount in BTC upto the precision specified truncating the insignificant zeros.
- Displaying amount in BTC without any decimals if the remainder of the amount
divided by the satoshis in 1 BTC is equal to zero using formula `satoshis.rem_euclid(Amount::ONE_BTC.to_sat()) != 0`
These are not breaking changes and all previous tests pass.
A testcase is added to for changes introduced.
Resolves: #2136
cbee9781e8 Move unit types to units (Tobin C. Harding)
5bd0d7194b Remove unused absolute::Error (Tobin C. Harding)
Pull request description:
Move the following unit types to the new `units` crate:
- `locktime::absolute::{Height, Time}`
- `locktime::relative::{Height, Time}`
- `FeeRate`
- `Weight`
Also move the `parse` module as well as constants as required.
Do minimal changes to get things building:
- Feature gate on "alloc" as needed.
- Remove rustdocs that use `bitcoin` types.
- Re-export units types so this is a non-breaking change.
- Fix import paths.
Patch 1 was originally #2526, putting it in via this PR to try and speed up the process.
Close: #2282
ACKs for top commit:
sanket1729:
ACK cbee9781e8
apoelstra:
ACK cbee9781e8 lgtm. this is a good start. I think the LockTime types should follow Height and Time
Tree-SHA512: 6b0d63c7b054008598d7fa81be7d8c112f2778883b5529d79d446617b94b3c196c9ac735f840d1dfb488700894d3161c6976d44ab0e12ac3af4008068eac5f87
No clue how this got into master; found with `just sane`.
Clippy emits:
useless conversion to the same type: `amount::ParseAmountError`
As suggested, remove the useless conversion.
Move the following unit types to the new `units` crate:
- `locktime::absolute::{Height, Time}`
- `locktime::relative::{Height, Time}`
- `FeeRate`
- `Weight`
Also move the `parse` module as well as constants as required.
Do minimal changes to get things building:
- Feature gate on "alloc" as needed.
- Remove rustdocs that use `bitcoin` types.
- Re-export units types so this is a non-breaking change.
- Fix import paths.
e3db95226a Tidy description (yancy)
Pull request description:
I noticed `uncheced_add` looked really bad with two spaces (my mistake). Fixed some others as well.
ACKs for top commit:
apoelstra:
ACK e3db95226a oops, we should have caught this. Thanks for the fix!
tcharding:
ACK e3db95226a
Tree-SHA512: 8a2e7f85262f17063bea6ac22855ae45d99a1559a2d30f2627ffba1108f0fd8ebd0b541b50fe746b5af2ebb013cb3e9ea432987b90f37c046120388a808c5443
Sometimes people don't remember the exact number of decimal places
supported by denomination or don't want to count (e.g. when converting
fiat to BTC the calculator may yield too precise value). It's helpful to
say in error message at which digit the precision is too high.
This adds `TooPreciseError` struct containing the information and
improves the error message to display it.
This variant lacked pretty important information: what is the limit. We
could just write it in the error message but it's even better to move it
to a separate struct which also says how many characters exceed the
limit - this helps users know how many need to be deleted.
The `InvalidFormat` variant was pretty bad: it didn't make it clear what
exactly is wrong with the input string, especially since it was used
when the denomination was missing. Not only was this unhelpful to the
users who don't know they have to write the denomination when the amount
was otherwise correct but it was also attributed to a problem with the
amount rather than a problem with denomination.
To fix this the variant is replaced by `MissingDigitsError`,
`MissingError` and `InvalidCharError` - all the cases `InvalidFormat`
was originally used in.
`InvalidCharError` is effectively the same as the existing variant but
it was made into a separate error to enable special casing `.` and make
extensions possible. Further this opportunity was used to add a special
case for `-` as well.
`MissingDigitsError` currently contains special casing for empty string
and a string only containing minus sign. It's currently unclear if it's
useful so this change makes this distinction private and only makes it
affect error messages.
As opposed to the previous two variants, `MissingDenominationError` was
added to `ParseError`. The struct is currenly empty and may be extended
in the future with e.g. span information.
df1d2f6eb5 Add unchecked variants to Amount and SignedAmount (yancy)
Pull request description:
The checked variants have worse performance than the unchecked variants due to the additional branching operations. To improve performance where overflow is either not possible or not a concern, unchecked variants of `Amount` and `SignedAmount` are introduced for addition, subtraction and multiplication.
Note, it seems the default behavior for the test framework is to panic on overflow, so I haven't figured out a good way to add tests for this. Marking as a draft for now.
closes: https://github.com/rust-bitcoin/rust-bitcoin/issues/2434
ACKs for top commit:
Kixunil:
ACK df1d2f6eb5
apoelstra:
ACK df1d2f6eb5 gonna go ahead and merge this, we can revisit if necessary when we look at `units` overflow behavior in general
Tree-SHA512: 3fbb0ec81a758b350569226c44e25f6ca49e551566bee83c05c1c2b343874ef657d63a36b5f51c41582d8a8e36466275c574ebff6d363ed7c112ac8b4d5376fa
The checked variants have worse performance than the unchecked variants
due to the additional branching operations. To improve performance where
overflow is either not possible or not a concern, unchecked variants
of Amount and SignedAmount are introduced.
251579f4ef feat: implement TryFrom trait to SignedAmount and Amount (Sumit Kumar)
Pull request description:
Closes: #2245
Adds `TryFrom<SignedAmount> for Amount` and `TryFrom<Amount> for Amount` in units module
ACKs for top commit:
tcharding:
ACK 251579f4ef
Kixunil:
ACK 251579f4ef
apoelstra:
ACK 251579f4ef
Tree-SHA512: 3e58d7a891019ccd272417eadc977037167439e3385a7b47c060fe93eba9c47fc37cdc0ca5551174ef2d93b256b2804ad7c01f5f2470ef9e9b7b912877aed11c
c69caafefc Remove attribute comments (Tobin C. Harding)
3e83ef9276 Remove consensus error wrapper TODO (Tobin C. Harding)
bfabea94e9 Remove unwrap comment (Tobin C. Harding)
8bdaf4a34d Remove carrying_mul TODO (Tobin C. Harding)
Pull request description:
Add issues and remove the TODOs from the code.
Resolves: #2368
ACKs for top commit:
apoelstra:
ACK c69caafefc
Kixunil:
ACK c69caafefc
Tree-SHA512: b10a3de8da7ace890735023f8441605dd11b0227c27a2357556b8aaa8276a7f34ed220e3bcbc93aad4b35357319318ff7de27210e8f60dd90f6c55af23e21470
Tobin C. Harding
Andrew Poelstra
Fmt Bot
Divyansh Gupta
448 OG
Liam Aharon
yancy
Martin Habovstiak
Sumit Kumar