Close the two pubic enum error types in `units::amounts`. All the other
structs are closed already because they either have private fields or
marked `non_exhaustive`.
7b8369f381 units: Add integration test of API surface (Tobin C. Harding)
Pull request description:
In an effort to check off items in the Rust API guidelines checklist (#3632) add an integration test file that tests:
- The location of re-exports for various typical usage styles.
- Regressions in the API surface (things being accidentally moved).
- All public types implement Debug (C-DEBUG).
- For all non-error types:
- `Debug` representation is never empty (C-DEBUG-NONEMPTY)
- For all error types:
- Derive standard traits as defined by `rust-bitcoin` policy.
- All data structures implement `serde` traits (C-SERDE).
I used the `cargo check-api` script we have laying around from ages ago (#2986) to parse `units` and get a list of the public types.
ACKs for top commit:
apoelstra:
ACK 7b8369f381c3f3fc15ed536d5316e86e02914b4e; successfully ran local tests
Tree-SHA512: 6fa2a61f6b67a6b5a56950b87e6df68b761b69bd2a49e7aac48aa238cb84441ce04acd85097d28ae4055058052a7491eccda3da218812149a896e548b4018aaa
17ca5018eb units: Comment alloc feature (Tobin C. Harding)
Pull request description:
Add a comment to the `serde` module about why we have all the `alloc` feature gating.
Done as part of #3556 - auditing the whole crate for use of `alloc` feature.
ACKs for top commit:
apoelstra:
ACK 17ca5018ebb28d5d392d61aee62170fb3e4f2a5b; successfully ran local tests
Tree-SHA512: 6d13129903a9c530db811e9f64ae4fa653dbcc952a42502c53c6c622a80c1e58b3037edd0638de78867823ca370b3a4595a08a9ec07b84c71afb6200454f5d6d
In an effort to check off items in the Rust API guidelines
checklist (#3632) add an integration test file that tests:
- The location of re-exports for various typical usage styles.
- Regressions in the API surface (things being accidentally moved).
- All public types implement Debug (C-DEBUG).
- For all non-error types:
- `Debug` representation is never empty (C-DEBUG-NONEMPTY)
- For all error types:
- Derive standard traits as defined by `rust-bitcoin` policy.
I used the `cargo check-api` script we have laying around from ages
ago (#2986) to parse `units` and get a list of the public types.
a8379bf005 Mark `checked_` functions const in bitcoin. (Jamil Lambert, PhD)
fe10ff2eb7 Mark functions `const` in `units` (Jamil Lambert, PhD)
Pull request description:
Following on #3608, #3627 and #1174 the rest of the `checked_` functions in all `rust-bitcoin` have been marked as const. Except for tests and traits.
ACKs for top commit:
apoelstra:
ACK a8379bf0053e66cf5984ce449d19e54e529b6b70; successfully ran local tests; thanks! I find this much easier to read
tcharding:
ACK a8379bf005
Tree-SHA512: b8d97b7a3d9fc33b57349f418ccc5aac2f3e8c4145a73bf0e24e85547217d41214696e9a80b1fb5a1bae7e766aad1c2b5df6044564772fe76cec9b3628bcd8e5
Add a comment to the `serde` module about why we have all the `alloc`
feature gating.
Done as part of #3556 - auditing the whole crate for use of `alloc`
feature.
Mark `checked_` functions const. Replace `map()` and `?` operators,
which are not allowed in const context, with match statements.
Use descriptive variable names in ceiling division to make
it easier to follow.
4223655829 Add test case (yancy)
Pull request description:
Since there are test cases for other functions here, add test case as follow up for https://github.com/rust-bitcoin/rust-bitcoin/pull/3604. I forgot to request this during review.
ACKs for top commit:
apoelstra:
ACK 422365582964cf405529de3a1cf63d766b0d0e2e; successfully ran local tests
tcharding:
ACK 4223655829
Tree-SHA512: 8629cfff3e3eb800aa4d4625627e0a777c389566f4125bc63e67fe4c5a597ca63b9fb92da42f6c9dab12e6c63839eef35a13957f98606a37ed687eb96d896c2e
0348801ef3 Remove Amount::fmt_value_in (Shing Him Ng)
Pull request description:
Saw this mentioned in #3556Fixes#2952
ACKs for top commit:
apoelstra:
ACK 0348801ef30d1cdfdc40cd2316671532d4c06141; successfully ran local tests
tcharding:
ACK 0348801ef3
Tree-SHA512: 29a1d230672a22d059d2ef0870e68dec0fe6d6ae9d3fed51275d9b6779d097578a38a514544df11bdf23908c2d8e3c5fd5c6de2e460aab9792770d6b9e396fc6
4c94695942 Mark funtions const (yancy)
Pull request description:
May as well mark these const as well. Follow up to https://github.com/rust-bitcoin/rust-bitcoin/pull/3605
ACKs for top commit:
apoelstra:
ACK 4c94695942de41e5eca4ac120db2e069c0bb037d; successfully ran local tests; yeah, I think this are all worth consting
tcharding:
ACK 4c94695942
sanket1729:
ACK 4c94695942
Tree-SHA512: bf41e416cb8a2eb90e26d7bd640df5c2a5a868e3b721bde7cc8a3b97c687bab4d1cb4ab1c4b04ed625e8657e25c59e386720cb0f9d735ffc492e98f5b8b2ca3c
1db4b723dc Mark function as const (yancy)
Pull request description:
We discussed marking from_vb as const here: https://github.com/rust-bitcoin/rust-bitcoin/issues/3602
However, from what I can tell, map() isn't const and I don't see a tracking issue for changing it. Also, the question mark operator isn't available in const context either, although there is a tracking issue for that: https://github.com/rust-lang/rust/issues/74935. It will be a long while before that makes it into this projects MSRV if/when it lands.
There are some other functions in this module that could use the same re-write to make them const as well it looks like.
ACKs for top commit:
tcharding:
ACK 1db4b723dc
apoelstra:
ACK 1db4b723dc59568c14c76598e7c63c58651ed1cd; successfully ran local tests
Tree-SHA512: 62b612791dd3ce2f6ebf27f586a1262633a46566b9fd3583984171f62209714ad979439345fe86d8ef87d2f78a2cee21d619e2eb3621689faf59d81640e9f77c
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.
81d8699b55 units: Put no_std up top (Tobin C. Harding)
3e332c3839 amount: Fix docs on FromStr (Tobin C. Harding)
5bec76aa51 amount: Fix rustdocs (Tobin C. Harding)
41c80cc476 amount: Improve docs on div by weight (Tobin C. Harding)
2b4a61739a Add type to debug output for Amount (Tobin C. Harding)
42e5043b33 Add from_int_btc group of functions (Tobin C. Harding)
Pull request description:
Improve the `amount` module by doing:
- Patch 1: Add/update `from_int_btc` and `from_int_btc_const` functions
- Patch 2: Add type to debug output for `Amount`
- Patch 3: Fix incorrect docs
- Patch 4: Make docs use correct style and be uniform across the two amount types
- Patch 5: Fix docs on `FromStr`
ACKs for top commit:
apoelstra:
ACK 81d8699b55dad570247cb14d2b3eea64270a83cf; successfully ran local tests
jamillambert:
ACK 81d8699b55
Tree-SHA512: ddd6e02b4cd9a9aa8cbdd2d2f7ae289a6bcca9eb002ef0c6d83a6eca950b2cd08f5918286bb33e814f321e3bfe83fdf75ae051cf8f91ee45d3e4abe76c1c2a4c
The `no_std` attribute has significant ramifications, as opposed to the
clippy attributes etc.
Put the `no_std` attribute up top so it catches the eye. This is also
uniform with other crates in this repo.
The docs on `FromStr` say "string slice is zero" but actually mean to
document the behaviour when the value represented is zero.
Also the fact that `FromStr` returns an error if parsing fails is self
evident.
Fix the rustdocs on `Amount` and `SignedAmount` by doing:
- Make them uniform
- Use correct style (looks like `SignedAmount` got left behind when we
improved `Amount`)
Add/update the from_int group of functions to provide one that errors
and one that is const and panics (errors in const context are not useful
because one cannot call `unwrap` in const context).
In an effort to reduce requirement for `alloc`; remove the `String` and
use `InputString` in the hex prefix related error types.
Tested with:
(Note in test code we have to use `"cafe".to_owned()` before this patch
is applied.)
rust```
#[test]
fn hex_prefix_errors() {
let err = hex_remove_prefix("cafe").unwrap_err();
std::println!("{}", err);
std::println!("{:?}", err);
let err = hex_check_unprefixed("0xcafe").unwrap_err();
std::println!("{}", err);
std::println!("{:?}", err);
}
```
Before:
hex string is missing a prefix (e.g. 0x): cafe
MissingPrefixError { hex: "cafe" }
hex string contains a prefix: 0xcafe
ContainsPrefixError { hex: "0xcafe" }
After:
failed to parse 'cafe' as hex because it is missing the '0x' prefix
MissingPrefixError { hex: InputString("cafe") }
failed to parse '0xcafe' as hex because it contains the '0x' prefix
ContainsPrefixError { hex: InputString("0xcafe") }
3f2e760d1f Replace String with InputString in ParseIntError (Tobin C. Harding)
aa5c78430c Replace invalidInteger with ParseIntError (Tobin C. Harding)
9b7a706bfd Remove From<ParseIntError> (Tobin C. Harding)
c90f4b6033 Fix bug in error output (Tobin C. Harding)
c986b2f620 internals: Move error.rs to error/mod.rs (Tobin C. Harding)
Pull request description:
This PR hopefully clears the way for removing many of the `alloc` feature gates in `units` and `primitives`
The three final patches were tested by adding the following test to `units::locktime::absolute`:
```rust
#[test]
pub fn debug_absolute_error_conversion_height() {
let invalid_height = LOCK_TIME_THRESHOLD + 1;
let err = Height::from_consensus(invalid_height).unwrap_err();
std::println!("{:?}", err);
std::println!("{}", err);
let invalid_time = LOCK_TIME_THRESHOLD - 1;
let err = Time::from_consensus(invalid_time).unwrap_err();
std::println!("{:?}", err);
std::println!("{}", err);
let invalid_height = std::format!("{:x}", LOCK_TIME_THRESHOLD + 1);
let err = Height::from_hex(&invalid_height).unwrap_err();
std::println!("{:?}", err);
std::println!("{}", err);
let invalid_time = std::format!("{:x}", LOCK_TIME_THRESHOLD - 1);
let err = Time::from_hex(&invalid_time).unwrap_err();
std::println!("{:?}", err);
std::println!("{}", err);
let err = Height::from_hex("somerandomshit").unwrap_err();
std::println!("{:?}", err);
std::println!("{}", err);
let err = Time::from_hex("somerandomshit").unwrap_err();
std::println!("{:?}", err);
std::println!("{}", err);
}
```
Gives the following output (the last four lines is the bit that changes, the rest just proves we don't break other variants)
On commit: `d47ff1c25 Remove From<ParseIntError>`
ConversionError { unit: Blocks, input: 500000001 }
invalid lock time value 500000001, expected lock-by-blockheight (must be < 500000000)
ConversionError { unit: Seconds, input: 499999999 }
invalid lock time value 499999999, expected lock-by-blocktime (must be >= 500000000)
ParseHeightError(Conversion(500000001))
block height 500000001 is above limit 499999999
ParseTimeError(Conversion(499999999))
block height 499999999 is below limit 500000000
ParseHeightError(InvalidInteger { source: ParseIntError { kind: InvalidDigit }, input: "somerandomshit" })
failed to parse somerandomshit as block height
ParseTimeError(InvalidInteger { source: ParseIntError { kind: InvalidDigit }, input: "somerandomshit" })
failed to parse somerandomshit as block time
On commit: `0155a0d9a Replace invalidInteger with ParseIntError`
ConversionError { unit: Blocks, input: 500000001 }
invalid lock time value 500000001, expected lock-by-blockheight (must be < 500000000)
ConversionError { unit: Seconds, input: 499999999 }
invalid lock time value 499999999, expected lock-by-blocktime (must be >= 500000000)
ParseHeightError(Conversion(500000001))
block height 500000001 is above limit 499999999
ParseTimeError(Conversion(499999999))
block height 499999999 is below limit 500000000
ParseHeightError(ParseInt(ParseIntError { input: "somerandomshit", bits: 32, is_signed: true, source: ParseIntError { kind: InvalidDigit } }))
failed to parse somerandomshit as block height
ParseTimeError(ParseInt(ParseIntError { input: "somerandomshit", bits: 32, is_signed: true, source: ParseIntError { kind: InvalidDigit } }))
failed to parse somerandomshit as block time
On Commit: `3f2e760d1 Replace String with InputString in ParseIntError`
ConversionError { unit: Blocks, input: 500000001 }
invalid lock time value 500000001, expected lock-by-blockheight (must be < 500000000)
ConversionError { unit: Seconds, input: 499999999 }
invalid lock time value 499999999, expected lock-by-blocktime (must be >= 500000000)
ParseHeightError(Conversion(500000001))
block height 500000001 is above limit 499999999
ParseTimeError(Conversion(499999999))
block time 499999999 is below limit 500000000
ParseHeightError(ParseInt(ParseIntError { input: InputString("somerandomshit"), bits: 32, is_signed: true, source: ParseIntError { kind: InvalidDigit } }))
failed to parse 'somerandomshit' as absolute Height/Time (block height)
ParseTimeError(ParseInt(ParseIntError { input: InputString("somerandomshit"), bits: 32, is_signed: true, source: ParseIntError { kind: InvalidDigit } }))
failed to parse 'somerandomshit' as absolute Height/Time (block time)
ACKs for top commit:
apoelstra:
ACK 3f2e760d1fef2951f93a2554cd53340b0d7a6e0b; successfully ran local tests; nice!
Tree-SHA512: f7fd55acfb83082419db22c24a6b375c20e2631263401e500410c5b5659463f06dc4bdb145621e475dc15d75e764668cdcbf8f88006a487248a05fdb237ad136
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.
This has been fixed and we use nightly to lint so we have access to the
merged fix.
Removing the attribute uncovers a bunch of real lint warnings, fix
them while we are at it.
In an effort to make the `amount` module more readable move the
`SignedAmount` type to a private submodule.
Re-export everything so this is not a breaking change.
Code move and re-exports only.
When we move `SignedAmount` to a submodule linking to `self` introduces
a clippy warning, I'm not exactly sure why but lets remove the link in
preparation for the move.
In an effort to make the `amount` module more readable move the `Amount`
type to a private submodule.
Re-export everything so this is not a breaking change.
Code move and re-exports only.
Tobin C. Harding
merge-script
Jamil Lambert, PhD
Fmt Bot
yancy
Shing Him Ng
Rob N