6dd5af9678 Add missing links (Jamil Lambert, PhD)
47e367f011 Standardize error headings (Jamil Lambert, PhD)
75f317a689 Fix rustdoc grammar (Jamil Lambert, PhD)
573f8ce724 Add backticks to rustdoc links (Jamil Lambert, PhD)
Pull request description:
The rustdocs in the `units` crate has been reviewed and improved.
Some of the links were missing backticks, these have been added.
Some grammatical changes have been made to improve the documentation.
The use of links was inconsistent and has been changed to have links everywhere that seemed appropriate.
A couple of error headings were added and a description as to why a capital M is not accepted in the denomination for MegaSatoshi or MegaBitcoin.
ACKs for top commit:
apoelstra:
ACK 6dd5af9678
tcharding:
ACK 6dd5af9678
Tree-SHA512: f5481a7c3aa99d7882cda9ccda9df9e27f092ff91ef584f07dc887f38bfe12e5ea801cfa7f42bb4022301860489ee6be55557752a8cbe70932f258ea753495dc
7fa53440dc Move serde_round_trip macro to internals (Tobin C. Harding)
Pull request description:
We currently duplicate the serde_round_trip macro in `units` and `bitcoin`, this is unnecessary since it is a private test macro we can just throw it in `internals`.
While we are at it lets improve the macro by testing a binary encoding also, elect to use the `bincode` crate because we already have it in our dependency graph.
Add `test-serde` feature to `internals` to feature gate the macro and its usage (preventing the transient dependency on `bincode` and `serde_json`).
ACKs for top commit:
Kixunil:
ACK 7fa53440dc
apoelstra:
ACK 7fa53440dc
Tree-SHA512: f40c78bf2539940b7836ed413d5fe96ce4e9ce59bad7b3f86d831971320d1c2effcd23d0da5c785d6c372a2c6962bf720080ec4351248fbbdc0f2cfb4ffd602c
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.
We currently duplicate the serde_round_trip macro in `units` and
`bitcoin`, this is unnecessary since it is a private test macro we can
just throw it in `internals`.
While we are at it lets improve the macro by testing a binary encoding
also, elect to use the `bincode` crate because we already have it in
our dependency graph.
Add `test-serde` feature to `internals` to feature gate the macro and
its usage (preventing the transient dependency on `bincode` and
`serde_json`).
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
Wildcards have been replaced with what is actually used.
In a couple of cases an additional use statement was added to the test
module to import `DisplayHex` which is only used in test, but
previously imported with the wildcard at the top.
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.
Add two simple integer wrapper types for abstracting over block
height (from genesis block) and block interval.
This does not include hex because block height is typically written in
decimal.
These types are very thin wrappers, their usecase is to assist in code
readability instead of enforcing any logic.
d2a597c90d unit: Group re-exports (Tobin C. Harding)
d242125ae4 units: Fix error re-exports (Tobin C. Harding)
Pull request description:
First patch is the meat and potatoes, second one is just a trivial refactor to the same code, done separately so as to make the changes in the first patch more clear.
From patch 1
```
units: Fix error re-exports
Currently we re-export two error types at the crate root, this is
surprising because:
- Why not none or all the rest?
- Why these two?
Observe that the `ParseIntError` is special in that it is used by
other modules so its good to have at the crate root (other errors are
expected to be used with a module prefix eg, `amount::ParseError`).
There is no obvious reason why `ParseAmountError` is re-exported.
Comment and doc inline the `ParesIntError`, remove the re-export of
the `ParseAmountError`.
```
ACKs for top commit:
apoelstra:
ACK d2a597c90d
Tree-SHA512: 38d3f590357e66d07cbd7fedff134c39e0920e076ea99cb34ba276749a14695d428345d7b0f9ec8222f7899cb57e7c97068d3b6e7b2a9be25a0278e0a1abf762
11bb1ff6ff Standardize function doc Safety, Returns and Parameters (jamil.lambert)
df83016c98 Standardize function doc Errors (jamil.lambert)
d219ceb68e Standardize function doc Examples (jamil.lambert)
233a9133d8 Standardize function doc Panics (jamil.lambert)
Pull request description:
The subheadings in the rustdocs have been standardized according to [./CONTRIBUTING.md](https://github.com/rust-bitcoin/rust-bitcoin/blob/master/CONTRIBUTING.md):
```rust
impl FooBar {
/// Constructs a `FooBar` from a [`Baz`].
///
/// # Errors
///
/// Returns an error if `Baz` is not ...
///
/// # Panics
///
/// If the `Baz`, converted to a `usize`, is out of bounds.
pub fn from_baz(baz: Baz) -> Result<Self, Error> {
...
}
}
```
ACKs for top commit:
apoelstra:
ACK 11bb1ff6ff
tcharding:
ACK 11bb1ff6ff
Tree-SHA512: 163af3cd1cfb47cea3e55eddeaeb6843ff7ec89c57354e3247d6bae85e756b183e8045c2555cfcf87e8c23c1388ff9d7592cfb6a951a37a9ec41d27263e5a2e4
Add to `units::parse` the complete suit of hex unit parsing functions:
- remove prefix
- assert without prefix
- parse with or without prefix
- parse with prefix
- parse without prefix
- parse prefix unchecked
Refactor `bitcoin` to use the exact function we need, removing code
duplication.
This is a breaking change to `units`, it does however keep the current
re-exports from the public, now empty, `bitcoin::error` module.
Currently we re-export two error types at the crate root, this is
surprising because:
- Why not none or all the rest?
- Why these two?
Observe that the `ParseIntError` is special in that it is used by
other modules so its good to have at the crate root (other errors are
expected to be used with a module prefix eg, `amount::ParseError`).
There is no obvious reason why `ParseAmountError` is re-exported.
Comment and doc inline the `ParesIntError`, remove the re-export of
the `ParseAmountError`.
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
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.
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.
merge-script
Jamil Lambert, PhD
Fmt Bot
Tobin C. Harding
yancy
Martin Habovstiak
Divyansh Gupta