Commit Graph

53 Commits

Author SHA1 Message Date
Andrew Poelstra f27e675e1e
relative locktime: add "obvious" constructors
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.
2024-03-21 22:07:25 +00:00
Andrew Poelstra 2ff5085e70
locktimes: run cargo fmt 2024-03-21 22:07:25 +00:00
Tobin C. Harding 911f8cbd6a
fix: Manually implement serde traits
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
2024-03-20 11:33:14 +11:00
Tobin C. Harding 290e4418e6
units: Fix cargo cult programming
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.
2024-03-19 09:59:11 +11:00
Andrew Poelstra 93ca42cb4d
Merge rust-bitcoin/rust-bitcoin#2604: Enforce displaying Amount with trailing zeros
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
2024-03-17 17:50:50 +00:00
448 OG d887423efc
Enforce displaying Amount with trailing zeros
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
2024-03-17 17:55:59 +03:00
Andrew Poelstra 750b4dfb8b
Merge rust-bitcoin/rust-bitcoin#2569: Move types to `units`
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
2024-03-15 22:43:51 +00:00
Tobin C. Harding 62f726d5a8
Remove useless convertion
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.
2024-03-12 12:56:07 +11:00
Tobin C. Harding cbee9781e8
Move unit types to units
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.
2024-03-12 11:59:39 +11:00
Liam Aharon b9f7462958
Implement infallible for errors
Creates a new macro `impl_from_infallible`, and applies it to custom
error types in the codebase.

Closes #1222.
2024-03-08 16:48:34 +11:00
Andrew Poelstra 6f14a1031a
Merge rust-bitcoin/rust-bitcoin#2527: Tidy description
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
2024-02-29 21:14:31 +00:00
yancy e3db95226a Tidy description
Add one space between header and description body
2024-02-29 15:21:46 +01:00
Martin Habovstiak 08f83898a3 Report the position of an invalid char in amount
It can be helpful to report the exact position where the invalid
character was encountered. This change adds he feature.
2024-02-24 09:42:10 +01:00
Martin Habovstiak 73b325aec5 Report position of the first "too precise" digit
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.
2024-02-24 08:55:32 +01:00
Martin Habovstiak 28d83551eb Improve `ParseAmountError::InputTooLarge`
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.
2024-02-24 08:55:32 +01:00
Martin Habovstiak b7689a7d60 Split up `ParseAmountError::InvalidFormat`
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.
2024-02-24 08:55:32 +01:00
Tobin C. Harding 9187bf3a65
Fix new nightly warnings/errors
The latest nightly toolchain introduced a whole bunch of new warnings
and errors, mostly to do with import statements - fix them all.
2024-02-21 14:13:49 +11:00
Andrew Poelstra bf29a76d89
Merge rust-bitcoin/rust-bitcoin#2436: Add unchecked variants to Amount and SignedAmount
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
2024-02-15 15:04:28 +00:00
yancy df1d2f6eb5 Add unchecked variants to Amount and SignedAmount
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.
2024-02-13 23:46:51 +01:00
Andrew Poelstra 8f7cc4d6b3
Merge rust-bitcoin/rust-bitcoin#2462: feat: implement TryFrom trait to `SignedAmount` and `Amount`
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
2024-02-12 23:11:41 +00:00
Sumit Kumar 251579f4ef
feat: implement TryFrom trait to SignedAmount and Amount 2024-02-13 01:22:36 +05:30
Tobin C. Harding 7d538c830d
units: Implement ops::Neg for SignedAmount
Its useful to be able to do `let x = -btc_amount;`

Implement `core::ops::Neg for SignedAmount`, returning a `SignedAmount`.

Fix: #2470
2024-02-12 13:06:34 +11:00
Andrew Poelstra 343510d3a0
kani: fix Amount overflow test 2024-02-05 18:52:13 +00:00
Andrew Poelstra a3c4194c3f
Merge rust-bitcoin/rust-bitcoin#2428: Remove the remaining TODOs
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
2024-02-02 23:39:16 +00:00
Tobin C. Harding c69caafefc
Remove attribute comments
Add an issue and remove the TODO from the code as well as the attribute
comments, leave a single comment as an explanation of why the unusual
code block.

ref: https://github.com/rust-bitcoin/rust-bitcoin/issues/2427
2024-02-02 06:22:02 +11:00
Martin Habovstiak 5c15ed5441
CI: Epic overhaul
Re-write the whole CI pipeline.

Co-developed-by: Martin Habovstiak <martin.habovstiak@gmail.com>
2024-02-02 05:57:23 +11:00
Andrew Poelstra 7b937acf17
Merge rust-bitcoin/rust-bitcoin#2403: Make crate level attributes uniform
0997382772 io: Enable alloc from std (Tobin C. Harding)
ba1166a63b Make crate level attributes uniform (Tobin C. Harding)

Pull request description:

  Make the trait level attributes uniform across all released crates in the repo. Excludes things that are obviously not needed, eg, bench stuff if there is not bench code.

  - Remove `uninhabited_references` - this is allow by default now.
  - Remove `unconditional_recursion` and mark the single false positive we have with an `allow`.

  Note, this does not add `missing_docs` to the `io` crate. There is an open PR at the moment to add that along with the required docs.

ACKs for top commit:
  apoelstra:
    ACK 0997382772
  Kixunil:
    ACK 0997382772

Tree-SHA512: ef1f638aca171536287cce369be98998e871d26468ad2d8c39d9004db610b406471809c283540a4a19bcede78b12b8976a1bb37e5d431fbff8c8a3e53a64d4e3
2024-02-01 14:17:16 +00:00
Tobin C. Harding 96d3bbd065
Fix kani test
Recently (in #2379) we patched the `ParseAmountError` but we don't check
kani code on every pull request so we broke it.

Fix kani test to use the new `OutOfRangeError`.

Close: #2424
2024-02-01 15:13:50 +11:00
Tobin C. Harding bfabea94e9
Remove unwrap comment
Add an issue and remove the TODO from the code.

ref: https://github.com/rust-bitcoin/rust-bitcoin/issues/2426
2024-02-01 12:32:42 +11:00
Tobin C. Harding ba1166a63b
Make crate level attributes uniform
Make the trait level attributes uniform across all released crates in
the repo. Excludes things that are obviously not needed, eg, bench stuff
if there is not bench code.

- Remove `uninhabited_references` - this is allow by default now.
- Remove `unconditional_recursion` and mark the single false positive we
  have with an `allow`.

Note, this does not add `missing_docs` to the `io` crate. There is an
open PR at the moment to add that along with the required docs.
2024-01-31 11:32:46 +11:00
Martin Habovstiak ac26171c32 Clean up `no_std` usage
Previously the crate used negative reasoning to enable `std` which was
hard to understand, required the `prelude` module and wasn't really
needed because it's only needed when a crate wants to add `alloc`
feature-backwards compatibly and this crate always had the feature.

This cleans up usage to unconditionally use `#[no_std]` and then just
add `extern crate` on top as needed by activated features.
2024-01-27 13:25:40 +01:00
Martin Habovstiak fce03cec85 Provide `Amount` & co in no-alloc
Using the crate without allocation was previously disabled making the
crate empty without the feature. This chage makes it more fine-grained:
it only disables string and float conversions which use allocator. We
could later provide float conversions by using a sufficiently-long
`ArrayString`.
2024-01-27 12:46:55 +01:00
Andrew Poelstra e2b9555070
Merge rust-bitcoin/rust-bitcoin#2370: Improve units
7bf478373a Model `TooBig` and `Negative` as `OutOfRange` (Martin Habovstiak)
54cbbf804f Express `i64::MAX + 1` as `i64::MIN.unsigned_abs()` (Martin Habovstiak)
b562a18914 Move denomination error out of `ParseAmountError` (Martin Habovstiak)
5e6c65bc1a Clean up `unsigned_abs` (Martin Habovstiak)

Pull request description:

  Closes #2265
  Closes #2266

  Disclaimer: I did this in December and don't remember why I haven't pushed it. Maybe because it's somehow broken but I don't see how so please review a bit more carefully just in case.

ACKs for top commit:
  tcharding:
    ACK 7bf478373a
  apoelstra:
    ACK 7bf478373a

Tree-SHA512: 1f6e9adae9168bd045c9b09f06d9a69efd47ccc7709ac9ecaf48cb86e265b448b9b52a199ac5e6838d5029f5bc7514c5d7deb15a4d7c8a4606a353f390745570
2024-01-26 13:18:57 +00:00
Tobin C. Harding abe2241828
units: Remove "alloc" TODO
Remove the TODO and add an issue:

  https://github.com/rust-bitcoin/rust-bitcoin/issues/2389
2024-01-25 16:59:55 +11:00
Martin Habovstiak 7bf478373a Model `TooBig` and `Negative` as `OutOfRange`
The error returned when parsing amount had a `Negative` variant which
was weird/unreachable when parsing `SignedAmount`. Also weirdly, parsing
would return `TooBig` when the amount was negative - too low.

To resolve this we merge them into one `OutOfRange` variant that nuges
the consumers to make principled decisions and print error messages as
amounts being more than or less than a specific value which is easier to
understand for the users. Notably, the API still allows getting
information about which type was parsed and which bound was crossed but
in a less obvious way. This is OK since users who have a very good
reason can use this information but most won't.

Closes #2266
2024-01-24 11:37:46 +01:00
Martin Habovstiak 54cbbf804f Express `i64::MAX + 1` as `i64::MIN.unsigned_abs()`
This better conveys the intention that we're checking the lower bound.
2024-01-24 11:25:09 +01:00
Martin Habovstiak b562a18914 Move denomination error out of `ParseAmountError`
The `from_str_in` methods on amounts returned `ParseAmountError` which
contained `InvalidDenomination` among its variants. This one was never
returned because the method doesn't parse denomination.

This change separates the error out.

Closes #2265
2024-01-24 11:25:09 +01:00
Martin Habovstiak 5e6c65bc1a Clean up `unsigned_abs`
Previousle we copied `unsigned_abs` method from `core` because it was
unstable in older MSRV. Our current MSRV allows using the method
directly so this removes our old one and uses the one from standard
library instead.
2024-01-24 11:25:09 +01:00
Andrew Poelstra d08d3efdfa
Merge rust-bitcoin/rust-bitcoin#2336: units: Enable parsing Amount from `u64::MAX`
b2344e019d units: Assert roundtrip SignedAmount/str overflows (Tobin C. Harding)
baadcf4c0a units: Test that SignedAmount float conversion overflows (Tobin C. Harding)
d768f25da8 units: Remove duplicate assertion (Tobin C. Harding)
1d536ac8b2 units: Enable parsing Amount from u64::MAX (Tobin C. Harding)

Pull request description:

  Our `Amount` type uses an internal `u64` and we maintain no invariants on the inner value. Therefore we should be able to parse `u64::MAX`.

  Fix the parsing code by removing the explicit, incorrect check and fix unit tests to mirror this behaviour.

  Fix: #2297

ACKs for top commit:
  Kixunil:
    ACK b2344e019d
  apoelstra:
    ACK b2344e019d

Tree-SHA512: 944f8d0bfedc559f0444f75eca7d3fba042fbc204c4c032d09ff0edc29be280a3707f5b363dbc04f0d7bdf64701c0c4619e2e0de683d804a2663c2a20ac963f6
2024-01-22 19:13:12 +00:00
Tobin C. Harding b2344e019d
units: Assert roundtrip SignedAmount/str overflows
Add a unit test to prove that attempting to roundtrip a `SignedAmount`
greater than `MAX` through a string fails.
2024-01-22 09:14:26 +11:00
Tobin C. Harding baadcf4c0a
units: Test that SignedAmount float conversion overflows
We should not be able to roundtrip a `SignedAmount` value greater than
`MAX`, add a test to prove so.

While we are at it document the assertion above that proves we can parse
a float representing an `Amount` greater than `SignedAmount::MAX`.
2024-01-22 09:08:54 +11:00
Tobin C. Harding d768f25da8
units: Remove duplicate assertion
Unit test has a duplicate assertion, remove it.
2024-01-22 09:06:26 +11:00
Martin Habovstiak 22747149a9 Add convenience constants to `Denomination`
`Denomination::Bitcoin` and `Denomination::Satoshi` are often used,
especially in test code so this change adds `BTC` and `SAT` - short,
readable constants. Notably this doesn't add the other constants as that
would lead to either unidiomatic names or confusing casing (MSAT meaning
millisat not megasat) and they are not used that much anyway.
2024-01-20 22:12:52 +01:00
Tobin C. Harding 1d536ac8b2
units: Enable parsing Amount from u64::MAX
Our `Amount` type uses an internal `u64` and we maintain no invariants
on the inner value. Therefore we should be able to parse `u64::MAX`.

Fix the parsing code by removing the explicit, incorrect check and fix
unit tests to mirror this behaviour.

Fix: #2297
2024-01-15 08:56:57 +11:00
yancy 278229def5 Add allow for out of bounds indexing
Out of bounds indexing is a workaround for const panic until MSRV +1.57
2024-01-01 10:35:52 +01:00
Fmt Bot 5af7727250 2023-12-17 automated rustfmt nightly 2023-12-17 00:59:05 +00:00
Andrew Poelstra 2a6b4c1f43
Merge rust-bitcoin/rust-bitcoin#2262: Clean up `io` usage
f06d12455f bitcoin: Remove the custom sink (Tobin C. Harding)
b503aa1544 Run the formatter (Tobin C. Harding)
3ca55fb163 Remove qualifying path from Read and Write (Tobin C. Harding)
ebeb21fa7a Import fmt::Write using underscore (Tobin C. Harding)
e2dbcb1d28 Use W for writer generic type (Tobin C. Harding)
8704d9f0ae docs: Fix grammar (Tobin C. Harding)

Pull request description:

  A few cleanups to how we use the `io` crate, this is reasonably trivial but commit `a6c7e696 Remove qualifying path from Read and Write` is big, I have however gone to some effort to make sure it is easy to flick through the diff.

  Done in preparation for another go at the `BufRead` stuff.

ACKs for top commit:
  apoelstra:
    ACK f06d12455f
  Kixunil:
    ACK f06d12455f

Tree-SHA512: 751c489c67901c7563f1cc91f7761a4e3c276ae1981010338134e8c13200720ba69fcc74948c1dc1e6e65390197da0da27b2b69b86034029748321b404142cba
2023-12-12 20:07:06 +00:00
Jiri Jakes daa47b2061
Allow `SignedAmount` parse values equal to i64::MIN
Previously, parsing such textual value returned 'too big' error. This
change fixes it and adds relevant tests.
2023-12-12 18:33:44 +08:00
Tobin C. Harding ebeb21fa7a
Import fmt::Write using underscore
When we use the `fmt::Write` trait it is just to call its methods, we
can therefore, without any change to the logic, use `as _` when
importing the trait. This prevents naming conflicts.

Done in preparation for importing the `io::Write` trait.
2023-12-12 11:48:29 +11:00
Tobin C. Harding ae07bdbdbc
Remove ToOwned from prelude
We are not using the `ToOwned` trait, remove it.

Found by clippy.
2023-12-12 08:55:03 +11:00