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
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`
There is a range of different wordings used in the docs of constructor
type functions.
Change all to start with `Constructs a new` or `Constructs an empty`.
In functions that act like constructors there is a mixture of the usage
of `creates` and `constructs`.
Replace all occurrences of `creates` with `constructs` in the first line
of docs of constructor like functions.
Currently the `ParseIntError` contains an owned copy of the input
string, this is causing us to have to use `alloc` everywhere.
We already have a alloc-friendly string replacement type, the
`InputString` - use it.
We have a special type for wrapping integer parsing errors, use it.
To test this I added the following tests:
#[test]
pub fn debug_absolute_error_conversion_height() {
let invalid_height = LOCK_TIME_THRESHOLD + 1;
let _ = Height::from_consensus(invalid_height).unwrap();
}
#[test]
pub fn debug_absolute_error_conversion_time() {
let invalid_time = LOCK_TIME_THRESHOLD - 1;
let _ = Time::from_consensus(invalid_time).unwrap();
}
#[test]
#[cfg(feature = "std")]
pub fn debug_absolute_error_conversion_height_string() {
let invalid_height = std::format!("{:x}", LOCK_TIME_THRESHOLD + 1);
let _ = Height::from_hex(&invalid_height).unwrap();
}
#[test]
#[cfg(feature = "std")]
pub fn debug_absolute_error_conversion_time_string() {
let invalid_time = std::format!("{:x}", LOCK_TIME_THRESHOLD - 1);
let _ = Time::from_hex(&invalid_time).unwrap();
}
#[test]
#[cfg(feature = "std")]
pub fn debug_absolute_error_height_invalid_hex_string() {
let _ = Height::from_hex("somerandomshit").unwrap();
}
#[test]
#[cfg(feature = "std")]
pub fn debug_absolute_error_time_invalid_hex_string() {
let _ = Time::from_hex("somerandomshit").unwrap();
}
Which resulted in the following output
Before:
---- locktime::absolute::tests::debug_absolute_error_conversion_height stdout ----
thread 'locktime::absolute::tests::debug_absolute_error_conversion_height' panicked at units/src/locktime/absolute.rs:431:56:
called `Result::unwrap()` on an `Err` value: ConversionError { unit: Blocks, input: 500000001 }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
---- locktime::absolute::tests::debug_absolute_error_conversion_height_string stdout ----
thread 'locktime::absolute::tests::debug_absolute_error_conversion_height_string' panicked at units/src/locktime/absolute.rs:444:51:
called `Result::unwrap()` on an `Err` value: ParseHeightError(Conversion(500000001))
---- locktime::absolute::tests::debug_absolute_error_conversion_time stdout ----
thread 'locktime::absolute::tests::debug_absolute_error_conversion_time' panicked at units/src/locktime/absolute.rs:437:52:
called `Result::unwrap()` on an `Err` value: ConversionError { unit: Seconds, input: 499999999 }
---- locktime::absolute::tests::debug_absolute_error_height_invalid_hex_string stdout ----
thread 'locktime::absolute::tests::debug_absolute_error_height_invalid_hex_string' panicked at units/src/locktime/absolute.rs:457:52:
called `Result::unwrap()` on an `Err` value: ParseHeightError(InvalidInteger { source: ParseIntError { kind: InvalidDigit }, input: "somerandomshit" })
---- locktime::absolute::tests::debug_absolute_error_conversion_time_string stdout ----
thread 'locktime::absolute::tests::debug_absolute_error_conversion_time_string' panicked at units/src/locktime/absolute.rs:451:47:
called `Result::unwrap()` on an `Err` value: ParseTimeError(Conversion(499999999))
---- locktime::absolute::tests::debug_absolute_error_time_invalid_hex_string stdout ----
thread 'locktime::absolute::tests::debug_absolute_error_time_invalid_hex_string' panicked at units/src/locktime/absolute.rs:464:50:
called `Result::unwrap()` on an `Err` value: ParseTimeError(InvalidInteger { source: ParseIntError { kind: InvalidDigit }, input: "somerandomshit" })
After:
---- locktime::absolute::tests::debug_absolute_error_conversion_height stdout ----
thread 'locktime::absolute::tests::debug_absolute_error_conversion_height' panicked at units/src/locktime/absolute.rs:432:56:
called `Result::unwrap()` on an `Err` value: ConversionError { unit: Blocks, input: 500000001 }
---- locktime::absolute::tests::debug_absolute_error_conversion_height_string stdout ----
thread 'locktime::absolute::tests::debug_absolute_error_conversion_height_string' panicked at units/src/locktime/absolute.rs:445:51:
called `Result::unwrap()` on an `Err` value: ParseHeightError(Conversion(500000001))
---- locktime::absolute::tests::debug_absolute_error_conversion_time stdout ----
thread 'locktime::absolute::tests::debug_absolute_error_conversion_time' panicked at units/src/locktime/absolute.rs:438:52:
called `Result::unwrap()` on an `Err` value: ConversionError { unit: Seconds, input: 499999999 }
---- locktime::absolute::tests::debug_absolute_error_conversion_time_string stdout ----
thread 'locktime::absolute::tests::debug_absolute_error_conversion_time_string' panicked at units/src/locktime/absolute.rs:452:47:
called `Result::unwrap()` on an `Err` value: ParseTimeError(Conversion(499999999))
---- locktime::absolute::tests::debug_absolute_error_height_invalid_hex_string stdout ----
thread 'locktime::absolute::tests::debug_absolute_error_height_invalid_hex_string' panicked at units/src/locktime/absolute.rs:458:52:
called `Result::unwrap()` on an `Err` value: ParseHeightError(ParseInt(ParseIntError { input: "somerandomshit", bits: 32, is_signed: false, source: ParseIntError { kind: InvalidDigit } }))
---- locktime::absolute::tests::debug_absolute_error_time_invalid_hex_string stdout ----
thread 'locktime::absolute::tests::debug_absolute_error_time_invalid_hex_string' panicked at units/src/locktime/absolute.rs:465:50:
called `Result::unwrap()` on an `Err` value: ParseTimeError(ParseInt(ParseIntError { input: "somerandomshit", bits: 32, is_signed: false, source: ParseIntError { kind: InvalidDigit } }))
The errors in `units::locktime::absolute` are complex, I'd like to make
them more simple so they are more understandable.
I have no clue why this is implemented - remove it.
Mark the from_consensus and to_consensus methods of the absolute
lock time structs as const. In theory. these methods do some sanity
checking and wrap a u32 value in a newtype. This should be possible
to do in const. Marking the methods as const should not break existing
call sites.
1cce1b5aa6 Remove private prelude module from units crate (Jamil Lambert, PhD)
Pull request description:
The private prelude module has been removed from the units crate and instead imports are stated in full when needed. As discussed in #2926.
ACKs for top commit:
Kixunil:
ACK 1cce1b5aa6
apoelstra:
ACK 1cce1b5aa6
Tree-SHA512: 58b93ff66f74399938bc1a7f59fe8d2a21d0437c7e90e0c190d3d6a8de30f9c9268c8e4288d1db287b4d190624968937b1ad6c6e54d29025370e47e71be925c1
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
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.
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`).
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.
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.
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
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
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.
merge-script
Shing Him Ng
Tobin C. Harding
Jamil Lambert, PhD
Fmt Bot
Christian Lewe
Divyansh Gupta