It could happen that we unknowingly depend on a new version of a crate
without updating `Cargo.toml`. This could cause resolution issues for
downstream users. It's also unclear for outsiders to see which
dependencies we tested the crate with.
This change commits two lock files: `minimal` and `recent`. `minimal`
contains minimal depdendency versions, while `recent` contains
dependency versions at the time of making the change.
Further, this adds CI jobs to test with both lock files, CI job for
`internals` crate, removes old `serde` pinning and prints a warning if
`recent` is no longer up to date. (We may have to override it somehow if
any crate breaks MSRV.)
The documentation is also updated accordingly.
Co-developed-by: Tobin C. Harding <me@tobin.cc>
Closes#1230
dd4ad9444e Hardcode expected weight in txin_txout_weight_tests (Peter Todd)
Pull request description:
Rational: the expected weight is fixed so this both ensures we don't accidentally change it somehow, and makes it easier to re-use these test cases in other codebases (eg python-bitcoinlib).
ACKs for top commit:
apoelstra:
ACK dd4ad9444e
tcharding:
ACK dd4ad9444e
Kixunil:
ACK dd4ad9444e
Tree-SHA512: 4769a4bb8695f4f4c95e258bb5f06a232090b14c3d9159d6d5de2d09d7fc934a1b920b90cc09677a88fc0cf37ac21ed27794692dff2c73df4252c9551dc10fc2
2860aae1a5 fuzz: don't fuzz hashes against RustCrypto (Andrew Poelstra)
6467728202 fuzz: disable tests unless 'cfg(fuzzing)' is passed; update README for reproducing failures (Andrew Poelstra)
6e2ee5be66 fuzz: run 'cargo fmt' on all the fuzz targets (Andrew Poelstra)
9cfc0fcd81 fuzz: add contrib/test.sh so we at least 'cargo test' it in CI (Andrew Poelstra)
933ecb19e1 fuzz: fix warnings, clippy lints, 1.48.0 failures (Andrew Poelstra)
fd88e48696 fuzz: remove AFL support (Andrew Poelstra)
ab467cb091 fuzz: make hongfuzz fuzzing the default feature (Andrew Poelstra)
6f754df231 fuzz: add fuzzing README (Andrew Poelstra)
f093765efe fix fuzz.sh and cycle.sh to use generated lists of targets (Andrew Poelstra)
6534f22362 fuzz: auto-generate CI and Cargo.toml files (Andrew Poelstra)
8021034d86 rename travis-fuzz.sh to fuzz.sh; partially patch CI (Andrew Poelstra)
0be75f7edc move hashes/fuzz into main fuzz/ directory (Andrew Poelstra)
5a891dec2d move bitcoin fuzz targets into bitcoin/ subdirectory (Andrew Poelstra)
e3111c748b move bitcoin/fuzz into repo root; add to workspace (Andrew Poelstra)
Pull request description:
Several big changes here:
* Moves fuzzing to its own workspace with a `contrib/test.sh` etc so that CI will check that it compiles
* FIx all warnings, clippy lints, MSRV problems, etc.; mostly move to Rust 2018
* Merge `hashes/` fuzztests into workspace
* Rewrite all scripts; add file that auto-generates CI fuzz job and Cargo.toml so we don't have to manually keep these in sync
* Remove bitrotted and partial AFL support.
Supercedes #1422
I suspect the hashes fuzztests will actually fail since we haven't touched them in so long. Will address that if CI fails here.
ACKs for top commit:
sanket1729:
ACK 2860aae1a5
tcharding:
ACK 2860aae1a5
Tree-SHA512: b1aa3d6fac75fee51966f1d3f3245784e331bdea2a3fa7d6609bc4196c34f81acb7701faf8f269c3741568ea100438f24a2f06e75c8d01cb84c8b22d7886f1dd
We should probably restore this in the future, but we need to rethink
how we fuzz hashes -- right now when cfg(fuzzing) is set, we break all
the hash functions in a way that won't match any other library.
We should probably make this breakage opt-in but this will require
buy-in from rust-lightning and maybe others.
AFAICT we literally never used this; it was available only on the
bitcoin targets and not the honggfuzz ones; AFL has a broken dep
tree (or at least, requires some more MSRV pins that I did not care
to investigate).
Just remove it entirely.
Rational: the expected weight is fixed so this both ensures we don't
accidentally change it somehow, and makes it easier to re-use these test
cases in other codebases (eg python-bitcoinlib).
d37845197f embedded: Remove error handler (Tobin C. Harding)
Pull request description:
Seems we no longer need an explicit error handler, remove it.
I did not grok the reason (long thread link below) but just removed it and checked that the embedded crates still ran correctly.
https://github.com/rust-lang/rust/issues/51540Fix: #1813
ACKs for top commit:
Kixunil:
ACK d37845197f
apoelstra:
ACK d37845197f
Tree-SHA512: 76ce3f346e7e4e62595c6a6c415476b0cabcf27ded8e79a3e0b692a98b12ff9e90bec2841d18790bb62a16c553ad60492fc09e1aa4bf550c7070cd29b5ac1702
Seems we no longer need an explicit error handler, remove it.
I did not grok the reason (long thread link below) but just removed it
and checked that the embedded crates still ran correctly.
https://github.com/rust-lang/rust/issues/51540)
a54e1ceab1 Apply rustfmt (The rustfmt Tyranny)
38d11ce3da ci: Make release CI search for NEXT.RELEASE instead (Steven Roose)
dad3abd20f transaction: Rename is_coin_base to is_coinbase (Steven Roose)
Pull request description:
Alternative to https://github.com/rust-bitcoin/rust-bitcoin/pull/1795.
Keep the old method as deprecated and add doc alias. Also change internal usage of the method.
ACKs for top commit:
tcharding:
ACK a54e1ceab1
apoelstra:
ACK a54e1ceab1
Tree-SHA512: 52d9729bf83da164556d960f8867cb836ff57a0f619da3dd3620efffb28a974aac23b8085863ab0e072a4bdb2f13ac576efa43ad2eec9a271ad044227f4d00a4
53d75c7b94 extended_tests: Remove stale docs (Tobin C. Harding)
Pull request description:
We no longer need to pin `syn`; remove stale docs.
This is a follow up to #1696
ACKs for top commit:
Kixunil:
ACK 53d75c7b94
apoelstra:
ACK 53d75c7b94
Tree-SHA512: 9e939860cfb3731942e92a611632f0b24acc88e06a96e905ee2ae50e489d1610f7b9c79e20384196482aad4d5da182f6f919d18726e079a98a667532806d5aae
6c61e1019e Fix pinning (schemars and MSRV) (Tobin C. Harding)
c8e38d6a5a hashes: Implement JsonSchema for sha256t::Hash<T> (Tobin C. Harding)
Pull request description:
This has grown due to now including pinning work also done in https://github.com/rust-bitcoin/rust-bitcoin/pull/1736, I decided to do this because the PRs conflict and doing it all here saves accidentally getting out of sync. And https://github.com/rust-bitcoin/rust-bitcoin/pull/1764 requires this PR.
- Patch 1 is unchanged
- Patch 2 now fixes pinning in bitcoin and hashes CI scripts and in the docs of both as well as the manifest stuff relating to `schemars` - phew.
Fix: #1687
ACKs for top commit:
Kixunil:
ACK 6c61e1019e
apoelstra:
ACK 6c61e1019e
Tree-SHA512: eae4aa9700817bab6ad444e07709e8b1a4ffb1625e08be6ba399abde38bf6f8e5ea216a0836e2e26dfaddc76c392802cd016738ea6e753a1bca2584d9d2a9796
Done as is single patch to make sure all the docs and CI are in sync and
correct.
We currently pin the `schemars` dependency using `<=0.8.3` as well as a
the `dyn-clone` transient dependency in the manifest (`hashes` and the
extended test crate). This is incorrect because it makes usage of the
crate klunky (or possibly impossible) if downstream users wish to use a
later version of `schemars`.
Observe also that we do not have to pin `schemars`, we do however have to pin
the `serde` crate if either `serde` or `schemars` features are enabled.
Do so in CI and document in the readme file within hashes.
Currently we have a pin remaining from the old MSRV (`syn` due to use
of `matches!`).
Fix pinning by:
- Remove pin in manifest for `schemars`
- Fix pinning for MSRV in CI and docs (this includes documenting pinning
requirements for `schemars` feature because it is related to the other
pin of `serde`) in both `hashes` readme and main repo readme.
8f6317fbab Add predict_weight test for witness address types (yancy)
Pull request description:
Add a predict_weight test for address types with witness data
ACKs for top commit:
Kixunil:
ACK 8f6317fbab
apoelstra:
ACK 8f6317fbab
Tree-SHA512: 954908f068d6b0e8f7cc6bc6bffd110d490d7165cf2544ff0f936264761cb1f8e4da1e37fd5a6a34fd8b05f8d72817a3b340bbf968b29c9f538f10853347fdf0
3d524e06e4 Add Inventory::network_hash() method (Steven Roose)
Pull request description:
I'm not positive we won't ever had inv items that are not `sha256d::Hash`, though. I would expect them to stay like that, but we never know.
Would accept that as a reasonable objection against this helper.
ACKs for top commit:
apoelstra:
ACK 3d524e06e4
tcharding:
ACK 3d524e06e4
Tree-SHA512: 97d64b08ff99dbec2d907bdb98aa41ede4a15b988551ad8dd87a10ac5c9750ae4d516bb558c1e4f2171612777355c2baf1ffbe671cb92320a1af64d73bd150dd
b3b57518a0 ci: Check for remaining NEXT_RELEASE in the release script (Steven Roose)
Pull request description:
The intention here is that we can use the deprecation tag
`#[deprecated(since = "NEXT_RELEASE")]` and fill in the version number
at release time.
Not sure if there is a good place to mention this somewhere in developer docs. I looked into CONTRIBUTING.md but didn't really found a suitable section for this.
ACKs for top commit:
tcharding:
utACK b3b57518a0
apoelstra:
ACK b3b57518a0
Tree-SHA512: 7c077ee6569d38cf4c518cd5003ba0c09fdf7f98f4fb14fd28b49eb2e1b43eec7833de7abd1e35b9b14451f37de8e36a740a6445e3a5feb34a79cbfb7e63ee9f
e223cbe6df CI: Only run release job for release PRs (Tobin C. Harding)
Pull request description:
Currently we run the release jobs for all PRs, this leads to red CI runs any time we have unreleased changes in one crate used by another i.e., basically days after a release comes out.
Add a `release.sh` script that has more smarts to try and figure out if the patch series currently ontop of tip of mater contains changes that imply its a release PR. To do so we check for changes to the version field in the manifest of each crate.
ACKs for top commit:
stevenroose:
ACK e223cbe6df
apoelstra:
ACK e223cbe6df
Tree-SHA512: cc5503f68f9d275989acf8d2e8ff3b2b4ceb73606b8b823e408e4cf0bccbbb6dce0c583917d4dcb4b1b7f1b0bc34293251247d088943a5a6d5e06bf5af2ee8c3
088fa24be8 hashes: Derive traits on the sha512::Hash type (Tobin C. Harding)
Pull request description:
For all the other hash types we use derive, by way of the `hash_type!` macro, unless I'm missing something we can do the same for `sha512::Hash`.
ACKs for top commit:
apoelstra:
ACK 088fa24be8
Kixunil:
ACK 088fa24be8
Tree-SHA512: 3849261a8208c8c6108f4ecc5643940d38289f791d1b360ef4b13d60f72b365cd29f07f625a2d320f188defde848ddd64c8532cae5979e54afbf62f741927185
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
29cb34eed7 Refactor Address struct and its methods (Harshil Jani)
Pull request description:
Closes#1755
In this PR the `as_unchecked` is added to the Address struct, which returns a reference to the same address but with the type Address<NetworkUnchecked>. Similarly, the `assume_checked_ref` is added to Address<NetworkUnchecked>, which returns a reference to the same address but with the type Address.
ACKs for top commit:
tcharding:
ACK 29cb34eed7
apoelstra:
ACK 29cb34eed7
Tree-SHA512: 75ba40883d9fb31026b0e94e8d3fdcf808ff38a4d10f61f99a1e14ddc48358da86bad44cd78563dc67aa5d39382a5493e73c03891317f361f590e39b6257cb84
Currently we run the release jobs for all PRs, this leads to red CI runs
any time we have unreleased changes in one crate used by another i.e.,
basically days after a release comes out.
Add a `release.sh` script that has more smarts to try and figure out if
the patch series currently ontop of tip of mater contains changes that
imply its a release PR. To do so we check for changes to the version
field in the manifest of each crate.
91f45a214f Replace hardcoded values with compile-time hashing (Martin Habovstiak)
095b7958dd Make `sha256t_hash_newtype!` evocative of the output. (Martin Habovstiak)
Pull request description:
The Rust API guidelines state that macros should be evocative of the
output, which is a sensible recommendation. We already had this for
`hash_newtype!` macro but didn't for sha256t version.
This changes the macro to have this syntax:
```rust
sha256t_hash_newtype! {
// Order of these structs is fixed.
/// Optional documentation details here. Summary is auto-generated.
/*pub*/ struct Tag = raw(MIDSTATE_BYTES, LEN);
/// Documentation here
#[hash_newtype(forward)] // optional, default is backward
/*pub*/ struct HashType(/* attributes allowed here */ _);
}
```
Closes https://github.com/rust-bitcoin/rust-bitcoin/issues/1427
Depends on #1769
How do you like the syntax? Is weird `struct Foo = bar(..);` acceptable?
ACKs for top commit:
tcharding:
ACK 91f45a214f
apoelstra:
ACK 91f45a214f
Tree-SHA512: f6b555b20311a4c1cb097bc296c92459f6fbe16ba102d8c977eb2383dbcae3cc8ffce32e3cb771e7e22197fda26379971aa4feaadc5e2e70d37fa690ae8b8859
873501a85f Use slice patterns (Martin Habovstiak)
Pull request description:
Some code looks better with slice patterns. This changes `bip32` to use them.
ACKs for top commit:
apoelstra:
ACK 873501a85f
tcharding:
ACK 873501a85f
Tree-SHA512: 2e91b21d0f943c04f592112f241444ba3b1b1dad47ea6bea162f62cfbcaf98e383257cad144072b3807aaa0c122dabab638ac10b1cc1f5179727a5f05aac0659
42d3ae05be Implement computing SHA256 in const context (Martin Habovstiak)
Pull request description:
Computing hashes in const fn is useful for esily creating tags for `sha256t`. This adds `const fn` implementation for `sha256::Hash` and the algorithm for computing midstate of tagged hash in `const` context.
Part of #1427
I'd like to make sha256t evocative of the output after this which would also do the thing we want to do with #1427 (awkward language to bypass GH automatically closing it...) I'm thinking of this:
```rust
sha256t_newtype_hash! {
// optional attrs
/*pub*/ struct TagName = "tag_str"; // weird syntax but I guess OK-ish?
// optional attrs
/*pub*/ struct HashName(_); // allowed to elide the type because it's implied; maybe also allow writing it?
}
```
What do you think? I was also thinking about this alternative:
```rust
sha256t_newtype_hash! {
// optional attrs
/*pub*/ struct TagName;
const TAG: Tag = from_str("tag_str"); // allows for having from_raw([...]) or even from_hex([...])
// optional attrs
/*pub*/ struct HashName(_); // allowed to elide the type because it's implied; maybe also allow writing it?
}
```
Which looks more natural but maybe too much boilerplate? Which one do you like? Any better ideas? Heads up sanket1729
Also FYI tcharding this is the kind of post-1.48 change I was thinking about. :)
ACKs for top commit:
apoelstra:
ACK 42d3ae05be
tcharding:
ACK 42d3ae05be
Tree-SHA512: 437f3160e53104fcdf94c6e09ca6722ea2fe1032429b4701e578dded665cb9cab45cc68ee718b8925075c05dff231c0b898dc03949ef57b831666c68e38950a6
8b4280d5ef Update clippy MSRV configuration (Martin Habovstiak)
Pull request description:
We've upgraded MSRV but didn't update clippy config, so some things that could be improved aren't caught by clippy. This updates the config and fixes the new issues.
I also `rg '1\.41\.1'`ed for interesting changes and found one additional improvement.
ACKs for top commit:
apoelstra:
ACK 8b4280d5ef
tcharding:
ACK 8b4280d5ef
Tree-SHA512: 337ca099ca1a4b8b5d230c802bc455a5e0bbc3436066e41577c5b67f8d7fedede6f7a446b208da067ffc2cc2cebcbc187404846c295e9d89798f11dc66d34e34
This commit refactors the Address struct and its methods to improve
its functionality and usability.The AddressInner struct now holds
the payload and network, and the PhantomData<V> type is used to track
the network validation state.
Also as_unchecked and assume_checked_red methods are added to allow
conversion between checked and unchecked network validation state.
Signed-off-by: Harshil Jani <harshiljani2002@gmail.com>
Martin Habovstiak
Tobin C. Harding
Andrew Poelstra
Peter Todd
The rustfmt Tyranny
yancy
Steven Roose
Harshil Jani