6836de9ee6 Remove catch all pattern (Tobin C. Harding)
Pull request description:
The `PushBytes` type enforces len is less than 0x100000000 so we do not need to panic in a catch all pattern after explicitly matching against less than 0x100000000.
Refactor only because of the invariant on `PushBytes` - no logic changes.
ACKs for top commit:
apoelstra:
ACK 6836de9ee6 successfully ran local tests
Kixunil:
ACK 6836de9ee6
Tree-SHA512: a7cdb31683a8c00eecbdd0879886bec48133f9029f899b5279f1f5294ef40320592db196bfcafdeef4507636fc785a7ab87879b25b6d1b4905ae573b545f1ff4
There are a number of issues with the two `const_hash` functions in the
`sha256` module:
- The two `const_hash` functions in the `sha256` module differ slightly,
one finalizes the hash and one is for computing the midstate.
- They are inefficient and provided for usage for const context only.
Fix both issues by renaming the functions as discussed in #3075.
Close: #3075
Add a function `hash_reader` that uses the `BufRead` trait to read
bytes directly into the hash engine.
Add the functionality to:
- as a trait method in the `GeneralHash` trait with default implementation
- as inherent functions to all the hash types
Close: #3050
Currently we have duplicate code in inherent functions that also occurs
in the default implementation of the `GeneralHash` trait methods, this
is unnecessary because we can call through to the trait methods.
The `PushBytes` type enforces len is less than 0x100000000 so we do
not need to panic in a catch all pattern after explicitly matching
against less than 0x100000000.
Refactor only because of the invariant on `PushBytes` - no logic
changes.
84df3438ca Fix markdown list items (Tobin C. Harding)
0a45c68cf8 Introduce helper function name policy (Tobin C. Harding)
Pull request description:
As much as it hurts the C hacker inside me we have settled on using `_internal` to mark private function names that clash with a public function of the same name.
Introduce a policy section and rename one instance, I did not grep the codebase looking for other violations.
This came up because I had to look at what `_inner` implied when reading the function name somewhere else.
ACKs for top commit:
storopoli:
ACK 84df3438ca
apoelstra:
ACK 84df3438ca successfully ran local tests; sgtm. should probably update rust-miniscript which uses a `real_` prefix
Tree-SHA512: aece73fac54c4f34bdba72c08721586eb6d76dc9191442bbca43301f2af3d3b3e3217383f1ad20752fa7654e7f5d09927d1a4b72602aa064e76d550b5d6e81bd
ea9f6e8f97 Cast after calling min (Tobin C. Harding)
342fe18ad0 Use From in map_to_range (Tobin C. Harding)
d9331794f1 bip158: Fix typo (Tobin C. Harding)
Pull request description:
Patch 1 is a typo fix, other two are cast improvements. Done as part of #2941.
Leaves a cast to `u64` in there, will be removed once we get to the `ToU64` stuff. One other remaining cast in the module is obviously bit twiddling.
ACKs for top commit:
Kixunil:
ACK ea9f6e8f97
apoelstra:
ACK ea9f6e8f97 successfully ran local tests
Tree-SHA512: 92fa6225621f6c1eea4864472a5b9c60cb244e62cdfbee4074067da43b8f2bd4e3ed1a249ba40b994cad824e97585f3140747b03677ef1596c80be3007c94a7c
Done in an effort to reduce the cognitive load of reading the loop.
The base68 decode and encode algorithm uses a `u32` intentionally for
multiplication and a cast to `u8` intentionally when carrying.
Use `From` where possible and document the cast.
a90c8b1929 Automated update to Github CI to rustc nightly-2024-08-04 (Update Nightly Rustc Bot)
Pull request description:
Automated update to Github CI workflow `rust.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
ACKs for top commit:
tcharding:
ACK a90c8b1929
Tree-SHA512: 272f60a156cdfa21799af43282655804e4a5723c55976bf742aa0e1fbe1bfc4127225ecc9b965a69a0efb54a21b8c32cdbc48d4ba802181685d2fdb46c7c2f3a
Fix list items to use capital letters because the list items are
sentences (have trailing full stop already).
Also, use a long single line because it is [subjectively] easier to read
the list.
As much as it hurts the C hacker inside me we have settled on using
`_internal` to mark private function names that clash with a public
function of the same name.
Introduce a policy section and rename one instance, I did not grep the
codebase looking for other violations.
This came up because I had to look at what `_inner` implied when reading
the function name somewhere else.
5a91719755 Rename Siphash::as_u64 to Siphash::to_u64 (Shing Him Ng)
Pull request description:
Resolves#3114
ACKs for top commit:
tcharding:
However, ACK 5a91719755
Kixunil:
ACK 5a91719755
apoelstra:
ACK 5a91719755 successfully ran local tests
Tree-SHA512: 89b5b8575444e1cef82ba85c633104559923109c9b46026c1293d4776c8d55ea853b84e88acc91ab2fde0d378a0d7bae930ae6981fa8e6c87385dbe1aa083647
In an effort to reduce the cognitive load of reading code we are
removing casts unless they are useful or obvious.
Move the cast onto the call to `min` and comment it for good measure.
This allows us to call infallible `from` for conversion when needed.
Refactor only, no logic changes.
In an effort to remove unnecessary casts use `u128::from` to convert
from `u64`s. Leave the cast to `u64` in there because it is right after
a shift right and is brain-dead obvious.
`d58` is the iterator value from `Bytes` (iter returned by
`String::bytes`). As such we can infallibly convert it using `from`.
Internal change only, no external changes.
2969b032f9 Push up the Default bound on HashEngine (Nick Johnson)
Pull request description:
Backstory for this patch is in #3084, and this might be enough to close that issue, but I think some follow up work might be required.
I took Kixunil's advice and pushed up the `Default` bound on `HashEngine` all the way to the default methods themselves on the `GeneralHash` trait. I initially placed the bound just on the assocated type `Engine` of `GeneralHash`, which is a tad cleaner and keeps the bound from "leaking" downstream into things like `Hmac` and `Hkdf` implementations. However, that restricts `GeneralHash` implementations to just unkeyed hash functions and I believe there will be value in having `Poly1305` and `SipHash24` still leverage the interface. I struggled a bit on the best spots to put these bounds (even found the syntax a little jarring at times, I found [RFC2289](https://rust-lang.github.io/rfcs/2289-associated-type-bounds.html) helpful if others do too).
Refactored the existing keyed hash function, `SipHash24`, to no longer have `Default` functions with zero-value keys. I kept the test coverage though by just hardcoding the zero-value keys over in the tests.
Refactoring the `hash_type` macro for keyed hashes was getting a little hairy, so backed off and just wrote the bare minimum for `SipHash24` inline. Once `Poly1305` lands there will be two keyed hash functions and I think it will make more sense to then generalize over them.
ACKs for top commit:
Kixunil:
ACK 2969b032f9
apoelstra:
ACK 2969b032f9
Tree-SHA512: 9ca8f8baa6d60c36627eb3564f5faafcddd0a69fed5e29965404e405d3d09cb08bec2e05f14e177b8bfa2a488efba9947c0e0c158db6b00b44c8c11869328f5a
3bd59a5497 Automated update to Github CI to rustc nightly-2024-07-31 (Update Nightly Rustc Bot)
Pull request description:
Automated update to Github CI workflow `rust.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
ACKs for top commit:
tcharding:
ACK 3bd59a5497
Tree-SHA512: 713a576da3050759e186b275aa49f0106765161f4dc78283721af96dce5a6cfd5317b0493c994512ba8de0c3621687306cf31c4348b33b09848f4b27d45746d0
* The Default bound only makes sense for unkeyed hash functions which
can fire up a new engine without a key. Keyed hash functions, like
SipHash24 or Poly1305 require a secret key to be initialized and
should not implement a default engine generator.
* SipHash24 tests updated to the previous default key "0".
298b96c579 Add an extension trait for script validation (Tobin C. Harding)
c1aa33ed89 Use impl syntax instead of generic (Tobin C. Harding)
Pull request description:
Add an extension trait for the validation logic in preparation for moving the `Script` type to `primitives`.
ACKs for top commit:
Kixunil:
ACK 298b96c579
apoelstra:
ACK 298b96c579
Tree-SHA512: 6282bf7bd5657f0ec68e1369150969daf51f97dc6ff72a419fe823d60ab8a993f1e6d56b1cffa114580d388b36fe2bcbf7b9865776f98c46d68b7368168a07ee
A single trait bound can be expressed using the `impl` style. This is a
breaking change because callers can no longer use turbofish. In this
case that probably does not matter because users are likely just passing
an integer in and letting the compiler infer the type.
Done in preparation for moving logic into an extension trait so that the
functions can be parsed by the `define_extension_trait` macro.
ref: https://doc.rust-lang.org/reference/types/impl-trait.html
842fc9b24d Automated update to Github CI to rustc nightly-2024-07-28 (Update Nightly Rustc Bot)
Pull request description:
Automated update to Github CI workflow `rust.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
ACKs for top commit:
tcharding:
ACK 842fc9b24d
Tree-SHA512: 3763f634bf6209207a45564ce1e7ac8ee042eb1fe7bfe32721daebcc191aaff0928dbb0a21827f83c4ac5b7a87f420b56524effd0829e9c7a678ce8fdc221167
c72069e921 Bump MSRV to 1.63 (Martin Habovstiak)
Pull request description:
The version 1.63 satisfies our requirements for MSRV and provides significant benefits so this commit bumps it. This commit also starts using some advantages of the new MSRV, namely namespaced features, weak dependencies and the ability to use trait bounds in `const` context.
This however does not yet migrade the `rand-std` feature because that requires a release of `secp256k1` with the same kind of change - bumping MSRV to 1.63 and removing `rand-std` in favor of weak dependency. (Accompanying PR to secp256k1: https://github.com/rust-bitcoin/rust-secp256k1/pull/709 )
Suggested plan:
* merge both PRs
* at some point release `hashes` and `secp256k`
* remove `rand-std` from `bitcoin`
* release the rest of the crates
ACKs for top commit:
apoelstra:
ACK c72069e921
tcharding:
ACK c72069e921
Tree-SHA512: 0b301ef8145f01967318d3ed1c738d33e6cf9e44f835f3762122b460a536f926916dbd6ea39d6f80b4f95402cd845e924401e75427dbb0731ca5b12b4fa6915e
f76f68217b ci: pin cargo-semver-checks version and cron job (Jose Storopoli)
8a91015769 ci: harden zip command with -n (Jose Storopoli)
4edd504cb6 ci: pin stable in semver-checks and updates weekly (Jose Storopoli)
1948995443 ci: semver-check for non-additive cargo features (Jose Storopoli)
b5180732e6 io: add not_unwind_safe as PhantomData (Jose Storopoli)
Pull request description:
Closes#3001.
Closes#3023.
## Summary
Adds new CI Job that checks for semver breaks in `no-default-features` against `all-features`.
A fail would represent that we are inserting non-additive cargo features.
Credits to @Kixunil for the amazing idea.
This PR does:
1. fix a non-additive feature in `bitcoin-io`;
1. move the current `semver-checks` to `semver-checks-pr` since it checks if the PR is introducing classical semver-checks;
1. adds a new `semver-checks-feature`;
1. Add `-n` to `unzip` in `semver-checks`;
1. pins `rustc` stable and updates in a cron job weekly on Fri; and
1. pins `cargo-semver-checks` to `0.33.0` and updates in a cron job weekly on Sat.
## Implementation notes
We don't need nightly (and somehow it fails) but if we incorporate the ENV [`RUSTC_BOOTSTRAP=1` as hardcoded in the cargo-semver-checks codebase](50b93599df/src/rustdoc_cmd.rs (L110C17-L110C39)) and run `cargo` with the stable toolchain it works.
To get the `cargo-semver-checks` version we use the [`crates.io` API](https://crates.io/api/v1/crates/cargo-semver-checks) along with some quite intelligible `jg` magic (NOTE: `jq` is available in GH's `ubuntu-latest` image).
ACKs for top commit:
Kixunil:
ACK f76f68217b
apoelstra:
ACK f76f68217b At some point we should pull the stable-version to the root with the nightly-version and use it everywhere; but can be in a separate PR
Tree-SHA512: 9949350fb7d14853ed9b24c43c1b8c28d1178cc623fd4bb1a71b40f0e32a56052596f40ab55b1c26d17f4f4538a7a321b34cae2a65c110ed1939cc5cf0fc55ce
353af24ffe Set `diff=rust` for `*.rs` in `.gitattributes` (Martin Habovstiak)
Pull request description:
This slightly improves the diffs shown by git commands.
ACKs for top commit:
tcharding:
utACK 353af24ffe
apoelstra:
ACK 353af24ffe
Tree-SHA512: 2b74efa362943e91823e0ad03efaa92f577ab73aabb0e0eecc28cbe1841d2e4a3ce14e94d2a6664e472f7bc5a2d6174f848ee2bd2dbb54a1b992b7874254c51d
feef34fdea Make ScriptBuf::p2wpkh_script_code stand alone (Tobin C. Harding)
Pull request description:
We would like to move the `Script` type to `primitives` without moving any key stuff, including pubkey hashes. We may later, before releasing `primitives`, move the `WPubkeyHash` at which time this patch can be reverted or re-implemented on `ScriptBuf`.
The `ScriptBuf::p2wpkh_script_code` function does not take `self` as a parameter but it does return `Self` - this can trivially be made into a standalone function.
Make `ScriptBuf::p2wpkh_script_code` a standalone function.
ACKs for top commit:
Kixunil:
ACK feef34fdea
apoelstra:
ACK feef34fdea
Tree-SHA512: 9de43bb274480b85d328ddd4cb6d79b501a525837307ca399dea1c3d39e9a7c0d8719dbb0e5d852629351d2b1fe09ce0244df82c53ff9ef2342be85a71887329
98fe6179db Use "unfinalized" in header of Midstate (Tobin C. Harding)
86de586898 Use const to construct Midstate (Tobin C. Harding)
dcb18bfa7a Add length to sha256::Midstate (Tobin C. Harding)
db0502d3cd Use third person in rustdoc (Tobin C. Harding)
34dd95f909 Debug Midstate forwards (Tobin C. Harding)
1d0e70b1da Add regression test for Midstate debug output (Tobin C. Harding)
ca823945fc Manually implement AsRef (remove Borrow) (Tobin C. Harding)
7dc68b62e9 Remove serde from sha256::Midstate (Tobin C. Harding)
07e8e5d3a6 Stop using macro for Midstate (Tobin C. Harding)
37b54dd54c Move from_midstate function (Tobin C. Harding)
9efe4cea9d Move impl block under struct (Tobin C. Harding)
5941008733 Import sha256t in docs builds (Tobin C. Harding)
Pull request description:
"overhaul the midstate API" might be a better description. PR is broken up into many patches to the extreme to make sure all API the changes are easily noticed.
Resolve: #2918
ACKs for top commit:
Kixunil:
ACK 98fe6179db
apoelstra:
ACK 98fe6179db
Tree-SHA512: 1384cdeea52f6a0d5b41bcfdc9127f9b983f57bb4b15c7749af2a4a40388598a9fe1ab07d0a6507ffd2f38983459f9dd400c5d3f927f6d37c1f9d80a5c7f9a6e
a7fa237d32 githooks: Added post-merge githook to check if hooks changed (Ryan Breen)
Pull request description:
Referring back to [this post](https://github.com/rust-bitcoin/rust-bitcoin/pull/2980#discussion_r1673140172) in #2980, there was discussion that a post-merge githook should be added to check if the user's copied githooks have changed.
In order to distinguish between a githook that a user installed themselves and a rust-bitcoin githook, I chose to add a tag at the beginning of the project's hooks.
ACKs for top commit:
Kixunil:
ACK a7fa237d32
apoelstra:
ACK a7fa237d32 I have never used `git pull` but I also have not ever used githooks, so I guess this is good
Tree-SHA512: 90314fea81a93e5a2c698fad4837f7661dd90f9fb2ecd015cbe6472fbef8647e06a1595e2aab4f557a1cc770050b828d430eabda04500954c0e4c9995098237c
The version 1.63 satisfies our requirements for MSRV and provides
significant benefits so this commit bumps it. This commit also starts
using some advantages of the new MSRV, namely namespaced features, weak
dependencies and the ability to use trait bounds in `const` context.
This however does not yet migrade the `rand-std` feature because that
requires a release of `secp256k1` with the same kind of change - bumping
MSRV to 1.63 and removing `rand-std` in favor of weak dependency.
82a0656b57 Direct link to ./CONTRIBUTING.md (Joao Leal)
Pull request description:
For convenience.
ACKs for top commit:
tcharding:
ACK 82a0656b57
Kixunil:
ACK 82a0656b57
Tree-SHA512: ef2e0941dfb5d9c341be2f8f80286e8d8d654728262a6324a8bfdef81cfa62cc1b3f864b8926d90205b9d1deac4a7805cf70fe4a9ebd68dc58200586cec8a204
29b213daca Move validation module to consensus_validation (Tobin C. Harding)
Pull request description:
The `consensus` module is currently doing two things, validation and encoding. These two things are orthogonal.
Move the `consensus::validation` module to `consensus_validation`. Remove the function re-exports from `consensus`.
This was originally discussed here: https://github.com/rust-bitcoin/rust-bitcoin/issues/2779
ACKs for top commit:
Kixunil:
ACK 29b213daca
apoelstra:
ACK 29b213daca
Tree-SHA512: 3bd0e43c220b0d89a47e9df0e0c92b776ccc65f5f60d57f413db834acc8e86269379bc9fdd688f8c4f0138db22f8eb8983770afa2d7d53d51acf063f2302121c
991c73cdf9 primitives: Re-export everything from units (Tobin C. Harding)
7a44908aee primitives: Use wildard re-export in locktimes (Tobin C. Harding)
d1c876eda5 Remove rustfmt attribute (Tobin C. Harding)
e3d9376a9b units: Remove serde re-export (Tobin C. Harding)
Pull request description:
Re-export `units` types by doing:
- Patch 1: Remove the public re-export of `serde` so that it does not shadow the private one in `primitives`
- Patch2: Fix formatting (remove attribute and format)
- Patch 3 and 4: Use wildcard re-exports
ACKs for top commit:
Kixunil:
ACK 991c73cdf9
apoelstra:
ACK 991c73cdf9
Tree-SHA512: dd71aa97fe5718a1229bfe26b776a72ebdd745a15a0e8ce8dae421c3a4c49f0f1e27d676056b24c35318cc3643a2ce712b4a69cc09f42b7c43259680bea931f3
33d0a95b8c Automated update to Github CI to rustc nightly-2024-07-24 (Update Nightly Rustc Bot)
Pull request description:
Automated update to Github CI workflow `rust.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
ACKs for top commit:
tcharding:
ACK 33d0a95b8c
Tree-SHA512: a33ed623396222f73f145187297a7f6f44532790328d4d6d6821067e6d5fa3a21d7d285e420aec5101af9e5b54c778f12812cfff418bc1526d63000f007e3c26
merge-script
Tobin C. Harding
Update Nightly Rustc Bot
Shing Him Ng
Nick Johnson
Martin Habovstiak
Jose Storopoli
Ryan Breen