a0febf7eed githooks: remove unnecessary shellcheck disables (Jose Storopoli)
Pull request description:
We don't need to worry about nested quoting in SC2046.
Thanks to Kixunil in https://github.com/rust-bitcoin/rust-secp256k1/pull/697 for pointing that out.
ACKs for top commit:
Kixunil:
ACK a0febf7eed
tcharding:
ACK a0febf7eed
apoelstra:
ACK a0febf7eed successfully ran local tests
Tree-SHA512: 83cdcc55c7e7922c05f5e78305086595a96745f34ea68ee99ed1c08c97683bd45d3c62e8d8b4bcba6b0572c9c65178ff4e21abd7178e2e8b0c9e42f4b25508fb
3b5bcd0983 primitives: Fix alloc feature (Tobin C. Harding)
Pull request description:
We have an `alloc` feature but we are unconditionally using `extern crate alloc`, this is broken - clearly we need to add a `no-std` build for `primitives` in CI.
Feature gate the `alloc` crate.
While we are at it just pull types in from `alloc` in the `prelude` - I have no idea why we do not do this in `bitcoin`?
ACKs for top commit:
Kixunil:
ACK 3b5bcd0983
apoelstra:
ACK 3b5bcd0983 successfully ran local tests
Tree-SHA512: f042dfb368dc0bde16237764a0dd09def7eeeb59291a5dbb1b5bd5f1dcd49b3adf66c12508c3be4f0ff48b52cd31cbb20edea68d9d55760e99c506ac5b114781
34e8212594 Replace &self with self: &Self (Tobin C. Harding)
Pull request description:
`foo(&self)` is syntax sugar for `foo(self: &Self)`.
The `define_extension_trait` is currently large, ugly, and not that expressive. If we use `self: &Self` then the macro is greatly simplified.
(Also consuming version `self: Self`)
De-sugar only, no logic changes.
ACKs for top commit:
apoelstra:
ACK 34e8212594 successfully ran local tests; lol this looks so much better
Kixunil:
ACK 34e8212594
Tree-SHA512: 7ec81bee99ede328d73a661c9e683a774ba14404ceb89ecb06765bedddf04dc9721672775b9ad3a9e3356d510c76681848f24ce4392a59d53216d23e6a27d9ad
579b76b7cb Introduce ToU64 conversion trait (Tobin C. Harding)
Pull request description:
The idea for this was pulled out of Steven's work in #2133
We already explicitly do not support 16 bit machines.
Also, because Rust supports `u182`s one cannot infallibly convert from a `usize` to a `u64`. This is unergonomic and results in a ton of casts.
We can instead limit our code to running only on machines where `usize` is less that or equal to 64 bits then the infallible conversion is possible.
Since 128 bit machines are not a thing yet this does not in reality introduce any limitations on the library.
Add a "private" trait to the `internals` crate to do infallible conversion to a `u64` from `usize`.
Implement it for all unsigned integers smaller than `u64` as well so we have the option to use the trait instead of `u32::from(foo)`.
ACKs for top commit:
Kixunil:
ACK 579b76b7cb
apoelstra:
ACK 579b76b7cb successfully ran local tests
Tree-SHA512: 2eaddfff995987a346e052386c6dfef3510e4732e674e3a2cfab60ee391b4cce1bf7ba4fb2dfd4926f8203d7251eea2198ccb61f0b40332e624c88fda4fa7f48
191897f9ea Manually format (Tobin C. Harding)
Pull request description:
Run `rustfmt` and manually fix the places where comments are moved to the wrong place.
ACKs for top commit:
Kixunil:
ACK 191897f9ea
apoelstra:
ACK 191897f9ea successfully ran local tests
Tree-SHA512: f977ff373d1d410012734208c090bfcd8f9dbda414d0b19400acf8f552df481b4a2bc20d77c61538895a6fb66197be13cbdadf74956d67fd4d055b99ba8ab356
abe7b3f202 Remove build cfg for versions less than MSRV (Tobin C. Harding)
Pull request description:
Recently we upgraded the MSRV but forgot to remove the Rust version specific `cfg`s.
ACKs for top commit:
Kixunil:
ACK abe7b3f202
apoelstra:
ACK abe7b3f202 successfully ran local tests
Tree-SHA512: eabfdeb3217a5af8eae69e6f3589044f71b649c23b411525fb0401c2c3866dcd4c64f22ef927765f12584c223186ff850a60e71ee065476d39d5d557c5807e92
We already explicitly do not support 16 bit machines.
Also, because Rust supports `u182`s one cannot infallibly convert from a
`usize` to a `u64`. This is unergonomic and results in a ton of casts.
We can instead limit our code to running only on machines where `usize`
is less that or equal to 64 bits then the infallible conversion is
possible.
Since 128 bit machines are not a thing yet this does not in reality
introduce any limitations on the library.
Add a "private" trait to the `internals` crate to do infallible
conversion to a `u64` from `usize`.
Implement it for all unsigned integers smaller than `u64` as well so
we have the option to use the trait instead of `u32::from(foo)`.
108047ce96 Automated update to Github CI to rustc nightly-2024-08-07 (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 108047ce96
Tree-SHA512: 70114c69b98ff4d769af3ca37097bbbffb6f0b4e429ba7fe4d8a13786e3a70ec5e68c92cd4c05781e5ad8a3096d2151befbb312d06f74cf5ea74d104b4ebf313
We have an `alloc` feature but we are unconditionally using
`extern crate alloc`, this is broken - clearly we need to add a `no-std`
build for `primitives` in CI.
Feature gate the `alloc` crate.
While we are at it just pull types in from `alloc` in the `prelude` - I
have no idea why we do not do this in `bitcoin`?
`foo(&self)` is syntax sugar for `foo(self: &Self)`.
The `define_extension_trait` is currently large, ugly, and not that
expressive. If we use `self: &Self` then the macro is greatly
simplified.
De-sugar only, no logic changes.
4b66a479b0 base58: Use u32 instead of usize (Tobin C. Harding)
3f8cf1b335 base58: Use from and document cast (Tobin C. Harding)
121b435a9b base58: Use from to cast u8 (Tobin C. Harding)
Pull request description:
Done as part of #2941.
Remove or document casts in the `base58` crate. Two separate patches because to assist review, with the hope that reviewers are able to just read the diff without going to the crate's code.
ACKs for top commit:
Kixunil:
ACK 4b66a479b0
apoelstra:
ACK 4b66a479b0 successfully ran local tests
Tree-SHA512: e120e844af6e41eb29cc1538a9bad21a4e3ea8b27b90b7221a09eb5112093c72248ba3d72cd91d950ea8420032cb2e66790c24f9e15af7f2c0554f6cac3854c0
e7762e0612 hashes:: Rename const_hash functions (Tobin C. Harding)
Pull request description:
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
ACKs for top commit:
Kixunil:
ACK e7762e0612
apoelstra:
ACK e7762e0612 successfully ran local tests
Tree-SHA512: 2b765bbbaa596d060a555495582b24175f660bf630de489cf0e0199f1c589f13f46dde5c9735bffece10a1ff116a70472f821df66c62a97fffb424f16e5568f9
975f22f399 hashes: Call through to trait methods (Tobin C. Harding)
Pull request description:
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.
ACKs for top commit:
Kixunil:
ACK 975f22f399
apoelstra:
ACK 975f22f399 successfully ran local tests
Tree-SHA512: 74d8905a20d75536abf477dd2226e3cb12d8bd7330b1769e520840df1538362c6cbec6a976dfeb771797732b1f9259ee4f1970cadb69eca67b8b9bbe956ceeca
0a045d87ea hashes: Add a new hash_reader function (Tobin C. Harding)
Pull request description:
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
ACKs for top commit:
Kixunil:
ACK 0a045d87ea
apoelstra:
ACK 0a045d87ea successfully ran local tests
Tree-SHA512: 225f1d72f7a6119313d36422a3f7dc026ddcd27de9c8712c5734ea6056bb21e4857814761dbf2383a7a87fa82573ffc2097f67d08a0785a93e691c1745d0db8c
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
Update Stable Rustc Bot
merge-script
Jose Storopoli
Tobin C. Harding
Shing Him Ng
Nick Johnson