7cdc90565f Mutate mul_u64 with mutagen (Tobin C. Harding)
Pull request description:
Add the `mutate` attribute to mutate `mul_u64`. Add non-doc comments listing the two false positives. These are identical but we list them twice so when devs grep for `mutagen false pos` the same number of lines for each function is displayed as is displayed by the `mutagen` run. This coding false positives thing is also introduced in PR #1655.
ACKs for top commit:
apoelstra:
ACK 7cdc90565f
sanket1729:
utACK 7cdc90565f
Tree-SHA512: d066beb2f9ba198f5af36258ba15cfbd2c7c9ce7596f6340ed1fe2f62a2b0b5296eeb2cb4be30146d019671f1858521c29db917936895b5b3fd36bdb0bd07e57
8e6f953aa7 Expose valid (min, max) difficulty transition thresholds (Wilmer Paulino)
Pull request description:
Once `U256` was made private, we lost the ability to check whether a valid difficulty transition was made in the chain, since `Target` doesn't expose any operations. We only choose to expose `Shl<u32>` and `Shr<u32>` such that we can compute the min and max target thresholds allowed for a difficulty transition.
This is something we realized was missing after bumping to `rust-bitcoin v0.30.0` in `rust-lightning`, specifically for our `lightning-block-sync` crate. It may also be worth having a helper in `rust-bitcoin` that checks a header properly builds upon the previous, but that can be left for future work.
ACKs for top commit:
Kixunil:
ACK 8e6f953aa7
sanket1729:
ACK 8e6f953aa7 . Sorry, was confused by some details.
apoelstra:
ACK 8e6f953aa7
Tree-SHA512: 740dd64089426463dc6a19726d5a562276bd0966e0e31af8e1e67b28d18945644ac0e50be3cf0cc7fa604acc3d2c5b912a77a7caa69d8cff85f70fd57e5328c5
Once `U256` was made private, we lost the ability to check whether a
valid difficulty transition was made in the chain, since `Target`
no longer exposes any arithmetic operations.
Our previous MSRV did not support MIN/MAX associated consts so we had
methods min/max_value. Now that our MSRV is Rust 1.48.0 we can use the
consts.
Deprecate min/max_value methods in favor of MIN/MAX associated conts.
We currently use the functions `min_value` and `max_value` because the
consts were not available in Rust 1.41.1, however we recently bumped the
MSRV so we can use the consts now.
Currently we have a mishmash of attribution lines accompanying the SPDX
identifier. These lines are basically meaningless because:
- The date is often wrong
- The original author attributed is not the only contributor to a file
- The term "rust bitcoin developers" is basically just noise
Just remove all the attribution lines and be done with it. While we are
at it add an SPDX line to the few files missing it, whether this license
nonsense is even needed is left as an argument for another day.
fabcde036f Use package in manifest and shorten import (Tobin C. Harding)
Pull request description:
We can use `package` to rename `bitcoin_hashes` to `hashes` and `bitcoin_internals` to `internals`. This makes imports more terse with no loss of meaning.
ACKs for top commit:
apoelstra:
ACK fabcde036f
Kixunil:
ACK fabcde036f
Tree-SHA512: bc5bff6f7f6bf3b68ba1e0644a83da014081d8c6c9d578c21cb54fdd56a018f68733dd1135d05b590ba193ed9efd12fa9019182c1fed347e604d8548f6ef9103
If we use `#![cfg_attr(docsrs, feature(doc_auto_cfg))]` instead of
`#![cfg_attr(docsrs, feature(doc_cfg))]` we no longer need to manually
mark types with `#[cfg_attr(docsrs, doc(cfg(feature = "std")))]`.
Sweeeeeet.
We can use `package` to rename `bitcoin_hashes` to `hashes` and
`bitcoin_internals` to `internals`. This makes imports more terse with
no loss of meaning.
Add the `mutate` attribute to mutate `mul_u64`. Add non-doc comments
listing the two false positives. These are identical but we list them
twice so when devs grep for `mutagen false pos` the same number of lines
for each function is displayed as is displayed by the `mutagen` run.
This coding false positives thing is also introduced in PR #1655.
This adds a conversion function to U256 to get an f64. We use the method shown in the following blog post. https://blog.m-ou.se/floats/
Target::MAX was converted to a f64 and set as a const that is verified in a unit test.
Various formatting issues have crept into the codebase because we do not
run the formatter in CI.
In preparation for enabling formatting checks in CI run `cargo +nightly
fmt` to fix current formatting issues. No changes other than those
create by the formatter.
Currently we have an associated type on hash types `Inner` with
accompanying methods `into_inner`, `from_inner`, `as_inner`. Also, we
provide a way to create new wrapped hash types. The use of 'inner'
becomes ambiguous with the addition of wrapped types because the inner
could be the inner hash type or the `Inner` byte array of the inner
wrapped hash type.
In an effort to make the API more clear and uniform do the following:
- Rename `Inner` -> `Bytes`
- Rename `*_inner` -> `*_byte_array`
- Rename the inner hash to/from methods to `*_raw_hash`
Correct method prefix `into_` -> `to_` because theses methods convert
owned `Copy` types.
Add the trait Bound `Copy` to the `Bytes` type because we rely on this
trait bound for the conversion methods to be correctly named according
to convention.
Because of the dependency hole created by `secp256k1` this patch changes
the secp dependency to a git tag dependency that includes changes to the
hashes calls required so that we can get green lights on CI in this
repo.
When we added `Target` and `Work` types we implemented multiplication
and division by anything `Into<u64>`, this is not typically done in the
Rust stdlib and also is semantically incorrect for the types.
Remove `Mul` and `Div` impls from `Target` and `Work`. Also remove
`Mul<T>` for `T: Into<u64>` from the private `U256` type.
The `max_value` and `min_value` functions only exist to be
compatible/uniform with Rust 1.41.1 they will never change and they just
return a constant value. They can therefore be made const functions.
Recently we introduced some mutation testing to the `pow` module but
testing is never done - add more `mutate` attributes and add unit tests
to ensure all mutants are killed.
Of note, the `from_compact` and `to_compact_lossy` functions are not
done, doing so results in a bunch of surviving mutants.
The `ToHex` trait was replaced by either simple `Display`/`LowerHex`
where appropriate or `DisplayHex` from `bitcoin_internals` which is
faster.
This change replaces the usages and removes the trait.
Add kani verification for `U256::mul_u64`, doing so uncovered a bug in
the current implementation due to overflow.
Re-write the `mul_u64` method.
Props to Elichai for the algorithm.
Co-authored-by: Elichai Turkel <elichai.turkel@gmail.com>
Introduce mutation testing by way of mutagen [0]
- Conditionally add the dev-dependency `mutagen` (using `do_mutate`
flag)
This flag is not very well named but `mutagen` and `mutate` are already
taken?
- Mutate all methods of the `U256` struct that do not require additional
unit tests.
Uses `cfg(all(test, do_mutate), mutate)` - I cannot workout why we need
to check on `test` as well i.e., I don't understand why we cannot use
`cfg(do_mutate, mutate)`?
With this applied test can be run as usual with a stable toolchain. To
run mutagen we use `RUSTFLAGS='--cfg=do_mutate' cargo +nightly mutagen`.
[0] https://github.com/llogiq/mutagen
Adds new module `string` to be later converted to its own
crate. The module currently contains the FromHexStr trait and an error
type to be used for implementing hex parsing on types. This change
also adds implementations of FromHexStr for types with a single u32
member such as `Sequence(pub u32)`. All structs that match the
following regex have been given this implementation
`\(u32\)` and `\(pub u32\)`. All implementations have associated
unit tests matching all possible cases. NonStandardSighashType has
been ommitted from this change as it is an error and should not be
constructed using the methods added in this change.
Adds parse::hex_u32 for future use to be made generic to allow
different sizes of integers to be parsed from hex strings.
The error type FromHexError implements required traits such as
Display and std::error::Error
Currently the types in the block module have longer names than
necessary, "header" and "version" identifiers contain the word "block",
this is unnecessary because we can write `block::Header` instead of
`BlockHeader` when context is required. This allows us to use the naked
type `Header` inside the `block` module with no loss of clarity.
We are stuck with `BlockHash` because the type is defined along with all
the other hash types in `hash_types`, leave it as is for now but
re-export it from the `block` module to assist in putting types that are
used together in scope in the same place, making import statements more
ergonomic.
A zero default value for `Target` and `Work` has no significance and/or
usecase, remove the derived `Default` implementation.
Includes making `Target::ZERO` public.
Currently we use the `Uint256` type to represent two proof of work
integers, namely target and difficulty (work).
It would be nice to not have a public integer type that is not fully
implemented (i.e., does not implement arithmetic etc as do integer types
in stdlib). Instead of implementing all the stdlib functions we can
instead add two new wrapper types, since these are not general purpose
integers they do not need to implement anything we do not need to use.
- Add a `pow` module.
- Put a modified version of `Uint256` to `pow`.
- Add two new wrapper types `Target` and `Difficulty`.
- Only implement methods that we use on each type.
Note this patch does not remove the original `Uint256`, that will be
done as a separate patch.
sanket1729
Andrew Poelstra
Wilmer Paulino
Tobin C. Harding
junderw
Martin Habovstiak
connormullett
Jiri Jakes