Some of our CI shell scripts are meant only to be sourced and not
run directly however they include an initial shebang line, implying that
they should be run.
Remove the shebang line from `crates.sh` and the various `test_vars.sh`
scripts. Add a `shellcheck` directive to inhibit the no-shebang warning.
Fix: #2764
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.
30a482504b bump nightly-version (Andrew Poelstra)
5ad7c245e3 cargo: whitelist all cfgs used in this repo (Andrew Poelstra)
814786b0a6 crypto: enable and fix accidentally disabled unit test (Andrew Poelstra)
Pull request description:
https://github.com/rust-lang/rust/issues/124800 has been fixed and we can update our nightly version by whitelisting all cfgs that are used.
There was one place where we had an old `cfg(feature = "no-std")` despite having removed the feature. By removing that cfg check we re-enabled a previously disabled test.
ACKs for top commit:
tcharding:
ACK 30a482504b
Tree-SHA512: d25bed819091db74b9d47cb2c23caa3ceb0d7be323b37831326e2ec1608cb1577d41aad2e1cdf59d66df69397537bc3e17a3c2872935d5a4f46f4dc55b5e613c
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`.
26b9782d8b CI: Re-write run_task.sh (Tobin C. Harding)
Pull request description:
Recently we re-wrote CI to increase VM level parallelism, in hindsite this has proved to be not that great because:
- It resulted in approx 180 jobs
- We are on free tier so only get 20 jobs (VMs) at a time so its slow to run
- The UI is annoying to dig through the long job list to find failures
Have another go at organising the jobs with the main aim of shortening total run time and making it easier to quickly see fails.
Re-write the `run_task.sh` script, notable moving manifest handling to the workflow. Also don't bother testing with beta toolchain.
### Note on review
The diff is hard to read for `rust.yml`, I tried splitting out a bunch of separate patches but it resulted in the same thing (because there are so many identical lines in the yaml file). I suggest just looking at the yaml file and not the diff.
ACKs for top commit:
apoelstra:
ACK 26b9782d8b
sanket1729:
ACK 26b9782d8b.
Tree-SHA512: 1b0a0bab5cf729c5890f7150953499b42aebd3b1c31a1b0d3dfa5b5e78fda11e17a62a2df6b610ab4a950d5709f3af6fff1ae64d9e67379338903497ab77ae0e
Recently we re-wrote CI to increase VM level parallelism, in hindsite
this has proved to be not that great because:
- It resulted in approx 180 jobs
- We are on free tier so only get 20 jobs (VMs) at a time so its slow to run
- The UI is annoying to dig through the long job list to find failures
Have another go at organising the jobs with the main aim of shortening
total run time and making it easier to quickly see fails.
Re-write the `run_task.sh` script, notable moving manifest handling
to the workflow. Also don't bother testing with beta toolchain.
WASM Note
Removes the `cdylib` and `rlib` from the manifest patching during wasm
build - I do not know the following:
- Why this breaks on this PR but not on other PRs
- Why I can't get wasm test to run locally on master but PRs are passing
- What the `cdylib` and `rlib` were meant to be doing
This is the docs from: https://doc.rust-lang.org/reference/linkage.html
* --crate-type=cdylib, #![crate_type = "cdylib"] - A dynamic system
library will be produced. This is used when compiling a dynamic library
to be loaded from another language. This output type will create *.so
files on Linux, *.dylib files on macOS, and *.dll files on Windows.
* --crate-type=rlib, #![crate_type = "rlib"] - A "Rust library" file
will be produced. This is used as an intermediate artifact and can be
thought of as a "static Rust library". These rlib files, unlike
staticlib files, are interpreted by the compiler in future linkage. This
essentially means that rustc will look for metadata in rlib files like
it looks for metadata in dynamic libraries. This form of output is used
to produce statically linked executables as well as staticlib outputs.
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
We just did a minor bug fix to error code in the `amounts` module. This
change did not effect the public API but improved the display of two
error types from that module.
In preparation for doing a point release bump the version number and add
a changelog entry.
We have 2 crates that require an allocator, `bitcoin` and `base58ck` -
these crates should enable the "alloc" feature when depending on
`internals`.
For `units` we use the `internals::error::InputString` but do not enable
the "alloc" feature - this is a bug, it means that the parsed string is
being lost from the error types that use `InputString`.
Enable "alloc" for `bitcoin`, `base58ck`, and `units`.
- `bitcoin` and `base56ck` is just for good measure so we don't get
bitten later on.
- `units` is a bug fix and requires a point release.
In preparation for the initial release add a changelog. Note the version
number is already set to `v0.1.0` and this does not conflict with the
release currently on crates.io `v0.0.0`.
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.
af6dc1db02 internals: Bump version to 0.3.0 (Tobin C. Harding)
Pull request description:
In preparation for release add a changelog and bump the version number.
Please note, the changelog is pretty terse.
ACKs for top commit:
apoelstra:
ACK af6dc1db02
sanket1729:
ACK af6dc1db02
Tree-SHA512: b70d4b9de7de90aba3cbff90dd7f25c5ac801d020dbdfe3e64af4c079347cba726aa783a94fc777e7bf177db8402b54948c2dfd4a766d90c1a7a7a6bdfd36136
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.
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
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.
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
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
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
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.
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.
Tobin C. Harding
Andrew Poelstra
Fmt Bot
jamil.lambert
Jose Storopoli
Divyansh Gupta
448 OG