f5cae1cddd Comment from_str methods (yancy)
Pull request description:
Follow up from https://github.com/rust-bitcoin/rust-bitcoin/pull/3346
ACKs for top commit:
tcharding:
ACK f5cae1cddd
apoelstra:
ACK f5cae1cddd successfully ran local tests
Tree-SHA512: 2b95381e5281754e2b3a49aa8dfaac5742c244970fb54f68024dc23b61a74955ae95b9a0e7ae848095ac0686df5faf93faf7de3371b2f341b108cc10e5d4a9cd
894f82e7cc Add a condition for parsing zero from string when not denominated. (yancy)
Pull request description:
closes https://github.com/rust-bitcoin/rust-bitcoin/issues/3307
ACKs for top commit:
Kixunil:
ACK 894f82e7cc
tcharding:
ACK 894f82e7cc
apoelstra:
ACK 894f82e7cc9eb459a297d43e82734621e0824610; successfully ran local tests.
Tree-SHA512: 6d32847c74ccedc3355d615838e2dd1e08add29cc47ff69d9ef22683eda93049d41131dd0c992c8d7be3a018f5438856c415a3d2f3cb9de9c986534b268ee386
b6371b5801 Fix clippy rustdocs warnings (Tobin C. Harding)
Pull request description:
A new nightly version (`nightly-2024-08-28`) introduces a few warnings because of our rustdocs. These are valid warnings and should be fixed, thanks `clippy` team.
(The `bip152` change is a bit sloppy, open to suggestions.)
ACKs for top commit:
apoelstra:
ACK b6371b5801 successfully ran local tests
Kixunil:
ACK b6371b5801
Tree-SHA512: 503fb9d48772b74a5acdb26c0f77a85c52323c03360f983204fccee0f28bedeff142237b067caa1ce6ea04ea9842cc493e0d06dc141ca00a98151fa002b62392
3f244db19d Add Arbitrary to SignedAmount type (yancy)
Pull request description:
While proptesting, I've found Arbitrary for SingedAmount to be useful. Would appreciate adding this as well.
ACKs for top commit:
tcharding:
ACK 3f244db19d
Kixunil:
ACK 3f244db19d
Tree-SHA512: f293215505db85998135c75e147f4cb0031a08c7db619b706e6c7d632c6f23783af56992531396aecc6e3da55b107a7f2c7ec4d68a3e5737ebcf4225f306ed29
A new nightly version (`nightly-2024-08-28`) introduces a few warnings
because of our rustdocs. These are valid warnings and should be fixed,
thanks `clippy` team.
(The `bip152` change is a bit sloppy, open to suggestions.)
`s.parse` is more idiomatic and produces more helpful error messages.
This has been changed repo wide in the main codebase, not including
examples, rustdocs, and in the test module.
`use std::str::FromStr;` has been removed where this change makes
it unnecessary.
The use of links in the rustdocs was inconsistent.
Links have been added when missing.
[`locktime::absolute::Height`] and [`locktime::relative::Height`] did
not work and `(crate::locktime)` was appended to fix it.
Created headings for a couple of function error descriptions to be
consistent with the rest of the crate.
Added a description explaining why Mega is not allowed in a
denomination.
3196c271ac Deprecate `Amount::fmt_value_in` (Martin Habovstiak)
467546fc0c Round `Amount` when requested precision is too low (Martin Habovstiak)
7c95a777c1 Fix `Amount` decimals handling (Martin Habovstiak)
Pull request description:
Displaying with minimal number of zeros is the default in Rust and we want to follow it. This was originally implemented in #716 but #2604 reversed it claiming this is common, however it broke people who rely on minimal display (e.g. BIP21) without fixing the root cause of #2136.
This reverts commit d887423efc adds a test to prevent this change and also fixes the problem with `{:0.8}` not working.
Closes#2136Closes#2948
Can we backport this one too?
ACKs for top commit:
tcharding:
ACK 3196c271ac
apoelstra:
ACK 3196c271ac I also really like how this reduces the complexity of the module, basically passing everything through to `display_in`
Tree-SHA512: 3221f83086ac55af3d4caad03fe2b619be303533bba12096040419d119600c8597938809e171662f11b515d469156b083b2072b901d445e4fdfc7b1062cf7b6a
`fmt_value_in` was added when `display_in` wasn't available. However
common usage patterns seem to favor `display_in`. It can be used within
format strings and supports formatting options.
Removing it will simplify the codebase, so this deprecates it.
Low precision was previously not considered because it was believed it
could lead to misleading data being displayed. However it's quite
possible that people using low precision know what they are doing and
it's sometimes useful to show rounded numbers.
To enable low precision we just compute what the rounded number would be
and display that one instead.
This actually fully closes#2136 since this issue was mentioned there
along with previously-fixed displaying of higher precisions.
Displaying with minimal number of zeros is the default in Rust and we
want to follow it. This was originally implemented in #716 but #2604
reversed it claiming this is common, however it broke people who rely on
minimal display (e.g. BIP21) without fixing the root cause of #2136.
This reverts commit d887423efc adds a test
to prevent this change and also fixes the problem with `{:0.8}` not
working.
Closes#2136Closes#2948
The comment in `FromStr` says that any combination of upper and lower case is considered valid, but the code only allowed a set list of variations. It also had a non exhaustive list of `CONFUSING_FORMS`.
The comment and code has been changed to only consider all upper case or all lower case denominations as valid or where BTC/btc is prefixed with a lower case c, m or u. `CONFUSING_FORMS` has been changed to contain all combinations of an upper case prefix on an otherwise valid denomination.
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.
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
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.
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.
merge-script
yancy
Fmt Bot
Shing Him Ng
Tobin C. Harding
Jamil Lambert, PhD
Martin Habovstiak
448 OG
Liam Aharon
yancy