A new nightly version (`nightly-2024-08-28`) introduces a few warnings
because of our rustdocs. These are valid warnings and should be fixed,
thanks `clippy` team.
(The `bip152` change is a bit sloppy, open to suggestions.)
f8edbd1f45 Add Arbitrary to Weight (yancy)
Pull request description:
Adds Arbitrary to the Weight type. I had meant to add this with the initial Arbitrary PR but I overlooked it.
ACKs for top commit:
tcharding:
ACK f8edbd1f45
Kixunil:
ACK f8edbd1f45
Tree-SHA512: b886f78285b5cd06b2531a1e53272cad623a8e43e76e8108abe90668b45d00674b6a867c5ff980ee7f0a34919e1d6bb534624bd0bc67a53c8c9a41ea2ed504e8
cf129ad314 fix: re-implement (de)serialization from/to readers/writers (elsirion)
Pull request description:
Fixes#3250.
The serialization is less than ideal and still allocates a lot. I can understand not wanting to (ab)use the consensus encoding traits, but they have a pretty good interface, copying it and creating some `EncodePsbt` and `DecodePsbt` traits with similar interfaces would have been nice imo.
ACKs for top commit:
Kixunil:
ACK cf129ad314
apoelstra:
ACK cf129ad314 successfully ran local tests; LGTM -- I believe this is non-breaking, as does cargo-semver-checks, so we can backport this to 0.32
Tree-SHA512: d7f218164d772db3a9fb4436953c3b5fd3677b92078d0843233197629df7d852d80615a3ff38c5b70771381ba1aeb30defdc98ee63653e570bb75dc553400cad
9fce57b738 Change T::from_str(s) to s.parse::<T>() in tests (Jamil Lambert, PhD)
4ad86148c7 Change Address::from_str(s) to s.parse (Jamil Lambert, PhD)
64e668f99e Change from_str(s) to parse::<T>() in Examples (Jamil Lambert, PhD)
c835bb9eab Change T::from_str(s) to s.parse::<T>() in docs (Jamil Lambert, PhD)
Pull request description:
This PR follows on from #3256 changing `T::from_str(s)` to `s.parse::<T>()` in the rest of the repository, i.e. in examples, docs and tests that were not done in the previous PR.
Address was done in a separate patch because the formatting was different and required a different macro to make the change. Also in the rustdocs it gives 3 variants for parsing strings to addresses (`bitcoin/src/address/mod.rs` ll266-301) and I chose variant 3, but I can change to 1 if preferred.
``` rust
// variant 1
let address: Address<NetworkUnchecked> = "32iVBEu4dxkUQk9dJbZUiBiQdmypcEyJRf".parse().unwrap();
let address: Address<NetworkChecked> = address.require_network(Network::Bitcoin).unwrap();
// variant 2
let address: Address = Address::from_str("32iVBEu4dxkUQk9dJbZUiBiQdmypcEyJRf").unwrap()
.require_network(Network::Bitcoin).unwrap();
// variant 3
let address: Address<NetworkChecked> = "32iVBEu4dxkUQk9dJbZUiBiQdmypcEyJRf".parse::<Address<_>>()
.unwrap().require_network(Network::Bitcoin).unwrap();
```
Issue #3234 still needs `contributing.md` to be updated before closing.
ACKs for top commit:
Kixunil:
ACK 9fce57b738
apoelstra:
ACK 9fce57b738 successfully ran local tests; whew!
Tree-SHA512: 1e8abfa4a62c681c902827b8a7503ed5a10721c0adc68a97701ab00923dea7d62049ba530609b8bc6590209ad2e11698a761453c00f80965835677f6d3a7aee3
a76d76eca1 Change `T::from_str(s)` to `s.parse::<T>()` (Jamil Lambert, PhD)
Pull request description:
As mentioned in issue #3234 `s.parse::<T>()` is more idiomatic and produces more helpful error messages.
This has been changed in the main codebase, not including examples, rustdocs, and in the `test` modules.
`use std::str::FromStr;` has been removed where this change makes it unnecessary.
To close the issue it may also need to be changed in the examples and the `test` modules and `contributing.md` updated.
ACKs for top commit:
Kixunil:
ACK a76d76eca1
storopoli:
ACK a76d76eca1
tcharding:
ACK a76d76eca1
Tree-SHA512: 18be389579b5c2d0ba567dfb02b9c8d71c108a749ec5168c60a50e7af9d7f498e364c8f4c0ce1698d960e7d146486899cd10214ebc9c96708ed158a04dfff425
83831e6363 Show output of failed semver checks (Martin Habovstiak)
Pull request description:
Debugging the API breaks without seeing what failed is pretty much impossible. This simply prints the output when a check fails.
ACKs for top commit:
tcharding:
ACK 83831e6363
apoelstra:
ACK 83831e6363 successfully ran local tests
Tree-SHA512: d3acd41ef745e8e19c0be8ca9c205f038a72d6d66c459564f6ea35cdef9402c07026e497e6ff6c8e1b80b4856fedfbf1d6f74adb40789b30bffd6e2a1ee76a2b
b61adf7ca4 Introduce TxOutExt trait (Tobin C. Harding)
8b089ffe40 Run the formatter (Tobin C. Harding)
02c7d504fa Add tmp module around TxOut impl block (Tobin C. Harding)
fa946796eb Create a separate TxOut impl block (Tobin C. Harding)
Pull request description:
This one is pretty easy. Leaves the actual move of `TxOut` for later.
ACKs for top commit:
Kixunil:
ACK b61adf7ca4
apoelstra:
ACK b61adf7ca4 successfully ran local tests
Tree-SHA512: 99c23f1cebdade0f4298bf46b6af66c02594f79096b82550a7d17d5a7d058afba3738c676deac2712cfec43798813a6e49ac53e39760677777c6910ccedf5bc8
`s.parse` is more idiomatic and produces more helpful error messages.
This has been changed repo wide in the main codebase, not including
examples, rustdocs, and in the test module.
`use std::str::FromStr;` has been removed where this change makes
it unnecessary.
The two `TxOut` fields are public and there are no construtors or
getters to move, only the associated const `NULL`.
Add a tmp module around the big impl block so we can trick the formatter
into indenting before we add the extension trait.
441aac0a08 fix: vec! macro enabled only for test module (Chris Hyunhum Cho)
a050618fd8 feat: remove zeroed vector by pushing front (Chris Hyunhum Cho)
Pull request description:
Remove useless zeroed vector following https://github.com/rust-bitcoin/rust-bitcoin/issues/3131.
1. Init vector with capacity not to realloc when pushing.
2. If vector is empty(the first iteration), push data from the front.
3. Iterate vector from the front, and copy it from the end.
ACKs for top commit:
apoelstra:
ACK 441aac0a08 successfully ran local tests
jamillambert:
ACK 441aac0a08
Kixunil:
ACK 441aac0a08
Tree-SHA512: dee934f675786e6977216de9516ecb7f03fa17304891f060b33d46ff3a02342b817628b68dc429c65730a52e1d99c3d0d5e9d47afee5b727b06d3c800767369e
bab2fc44b6 Bump units version (yancy)
Pull request description:
The master branch version of units is behind that of crates: https://crates.io/crates/bitcoin-units
This breaks the ability for those of us trying to patch and work off of master since Cargo ignores the patched version 0.1.1 which is technically ahead.
I'm not sure how this got out of sync and if there's a preferable way/commit to fix this.
closes https://github.com/rust-bitcoin/rust-bitcoin/issues/3171
ACKs for top commit:
Kixunil:
ACK bab2fc44b6
apoelstra:
ACK bab2fc44b6 successfully ran local tests
Tree-SHA512: 0243309dbe0ff2e650f88c136e7e3c22c798bcc744419d5ef72b7929d045acc6015c8710af2861e8782ea5ae3446a9a4c78ae18c394c6e605cd2afc3d7f71bdc
8fb9bf8262 ci: fix cron update-cargo-semver-checks (Jose Storopoli)
Pull request description:
typo in the comparison inside jq
Closes#3229
ACKs for top commit:
Kixunil:
ACK 8fb9bf8262 looks like something like `!.yanked` ought to work but I don't care enough to check and force rewrite.
tcharding:
ACK 8fb9bf8262
apoelstra:
ACK 8fb9bf8262 successfully ran local tests
Tree-SHA512: 8be05d08969e2b13eeb77c202352d736b076f5bc023f9144818c247c139e51c5187e331264723709ff67dcdc638f97403fbbe2c77b0a18b1b37c201d0d647183
8ae156fe8b Automated update to Github CI to rustc nightly-2024-08-25 (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 8ae156fe8b
Tree-SHA512: 25ea39dd579630dba3683469f518f825b7b0078fa6def4cc3cce428944cc5d276f5d649b004e77aac0fdc9bcf610ab6fd69c45f7e65f3d2db08c1c16e8129aee
3e034d5ede Add Arbitrary dependency (yancy)
Pull request description:
Adds an example draft showing what is needed to use Arbitrary for coin selection.
Shot out to how nice Arbitrary is for fuzzing a target by taking unstructured randomness and creating structured rust-bitcoin types for fuzzing. Is there a way we could add this to rust-bitcoin for structuring the fuzz data needed?
This is then the example to fuzz test a SRD algo (after applying this PR to rust-bitcoin) using rust-bitcoin types :)
```
#![no_main]
use arbitrary::Arbitrary;
use bitcoin::{Amount, FeeRate};
use bitcoin_coin_selection::{select_coins_srd, WeightedUtxo};
use libfuzzer_sys::fuzz_target;
use rand::thread_rng;
#[derive(Arbitrary, Debug)]
pub struct Params {
target: Amount,
fee_rate: FeeRate,
weighted_utxos: Vec<WeightedUtxo>,
}
fuzz_target!(|params: Params| {
let Params { target: t, fee_rate: f, weighted_utxos: wu } = params;
select_coins_srd(t, f, &wu, &mut thread_rng());
});
```
ACKs for top commit:
tcharding:
ACK 3e034d5ede
Kixunil:
ACK 3e034d5ede
apoelstra:
ACK 3e034d5ede successfully ran local tests
Tree-SHA512: accd565815de3b37730d2ff12a24fcfc84e52ad357e5c940b1500a1e0bb17f4ff5fd6e52d31e8e96bb5290ee4fa050cfd2a9bbd6bbae13fc378f43093b64177f
9fb5edb39e ecdsa: Improve error types (Tobin C. Harding)
Pull request description:
There are a couple of issues around the ECDSA signature decoding / parsing code. We have duplicate code in `from_str` and `from_slice` and both use the same error type even though it is impossible to get a hex error in `from_slice`.
Create two errors:
- A `DecodeError` returned by `from_slice`
- A `FromStrError` that has a decode variant and a hex variant
Call through to `from_slice` after parsing hex into a byte vector.
Removes an instance of `unreachable!`.
Fix: #1193
ACKs for top commit:
Kixunil:
ACK 9fb5edb39e
apoelstra:
ACK 9fb5edb39e successfully ran local tests
Tree-SHA512: 3b3ae31887d603f1739d261b491b99f7847987f94dbbfefb9aa84d4250736eba2d007d28746bbb064946d3055e4cca01510677bf2cdbb11bbf83d7388dbd2620
c427d8b213 bitcoin: Compile time assert on index size (Tobin C. Harding)
49a6acc1a0 internals: Remove double parenthesis in const_assert (Tobin C. Harding)
2300b285ef units: Remove compile time pointer width check (Tobin C. Harding)
Pull request description:
3 patches in preparation for other size related work, this PR does not touch the `ToU64` issue which will be handled separately.
- Patch 1: Don't check pointer width in `units` because its not consensus code
- Patch 2: Modify internal macro `const_assert`
- Patch 3: Use index size to enforce not building on a 16 bit machine
ACKs for top commit:
Kixunil:
ACK c427d8b213 though I think the last commit was kinda a waste of time and it should have been adding the trait instead or leave it for later.
apoelstra:
ACK c427d8b213 successfully ran local tests; unsure if we want to merg this or wait for #3215
Tree-SHA512: 823df5b6a5af3265bce2422c00d287f45816faeb5f965685650ac974a1bd441cf548e25ac2962591732ff221bee91a55703da936382eb166c014ca5d4129edf8
a2be82c0c9 Use TBD in deprecated attribute (Tobin C. Harding)
Pull request description:
Our `release` job checks for 'TBD', I can't remember exactly why but I thought we introduced `0.0.0-NEXT-RELEASE` because CI was failing when we used TBD - clearly this is not the case now because we have a bunch of `TBD`s in the code base.
Change all the instances of `0.0.0-NEXT-RELEASE` to be `TBD`.
ACKs for top commit:
Kixunil:
ACK a2be82c0c9
apoelstra:
ACK a2be82c0c9 successfully ran local tests
Tree-SHA512: b383cc4095484291a7b4dca593ad5e017e3a9de9bfae9d6e9447ae36da32aa1c0d1fd593f49fd52c04db5ca5cdbaae8b30a772f792df13542f0a157a86295746
6e5bd473a6 Improve siphash's `as_u64` -> `to_u64` rename (Martin Habovstiak)
1a91492204 Clean up the siphash mess (Martin Habovstiak)
Pull request description:
Never break SSOT. Never break SSOT. Never break SSOT. Never break SSOT. Never break SSOT. Never break SSOT. Never break SSOT. ...
Previously we had removed `Default` impl on `siphash24::HashEngine` by
reimplementing the type manually. This was a really bad idea as it
inevitably led to API differences that broke the build which we didn't
notice because of unrelated bug. It should've just split the macro from
the start as was suggested but it was claimed to be difficult, I don't
think was the case as can be seen by this PR.
This commit does what the previous one should've done: it renames the
macro to have `_no_default` suffix, creates another one of the original
name that calls into `_no_default` one and moves anything related to
`Default`. This cleanly ensures all previous hashes stay the same while
siphash gets `Default` removed. This also removes all now-conflicting
impls from `siphash24` module which makes the module almost identical to
what it looked like before the change. The only differences are removed
`Default`/`new`, fixes in tests and recent rename of `as_u64` to
`to_u64`.
FTR both of our recent bugs were caused by breaking SSOT. It confirms again my life experience that breaking it will almost certainly bite one in the ass. If there is the most important principle in programming it's quite likely this one.
I was trying to be more "friendly" towards contributors breaking it but I no longer feel that this is a good choice, especially for complicated PRs. Thus I plan to be more aggressive here and NACK pretty much all breaks of SSOT that aren't provably unsolvable.
ACKs for top commit:
apoelstra:
ACK 6e5bd473a6 successfully ran local tests
Tree-SHA512: 03f413f302a41d07b52ac4645552231bb136aad9eb15010758fbad935ac2a686287d1adab7637372dd2629c37434b6ff9a085fba6792b8b6ec6c2a357e99538e
96e0e720fd feat(bip158): compute canonical filter hash (Rob N)
Pull request description:
From [BIP-157](https://github.com/bitcoin/bips/blob/master/bip-0157.mediawiki#filter-headers)
> The canonical hash of a block filter is the double-SHA256 of the serialized filter.
If a user forgets the "double" in double-SHA256 they will be computing a nonsensical filter hash when this is easily handled by the API.
ACKs for top commit:
Kixunil:
ACK 96e0e720fd
tcharding:
ACK 96e0e720fd
Tree-SHA512: 5fc0b1632e2327adacbd0ab56b4cd7edce0774f8be2f819782519c51b9691a748f4b3c01887f3bf1146dee49a35f9dfac833f53ae69ee7a53858bd2cedcec01a
There are a couple of issues around the ECDSA signature decoding /
parsing code. We have duplicate code in `from_str` and `from_slice`
and both use the same error type even though it is impossible to get a
hex error in `from_slice`.
Create two errors:
- A `DecodeError` returned by `from_slice`
- A `ParseSignatureError` that has a decode variant and a hex variant
Call through to `from_slice` after parsing hex into a byte vector.
Removes an instance of `unreachable!`.
Fix: #1193
Our `release` job checks for 'TBD', I can't remember exactly why but I
thought we introduced `0.0.0-NEXT-RELEASE` because CI was failing when
we used TBD - clearly this is not the case now because we have a bunch
of `TBD`s in the code base.
Change all the instances of `0.0.0-NEXT-RELEASE` to be `TBD`.
The previous change was just a dumb rename with no deprecation and it
also kept the `self` type which should be taken by value since the hash
is `Copy`. This improves on it by adding a deprecated method of the
original name and changing the type to be `self` instead of `&self`.
Previously we had removed `Default` impl on `siphash24::HashEngine` by
reimplementing the type manually. This was a really bad idea as it
inevitably led to API differences that broke the build which we didn't
notice because of unrelated bug. It should've just split the macro from
the start as was suggested but it was claimed to be difficult, I don't
think was the case as can be seen by this PR.
This commit does what the previous one should've done: it renames the
macro to have `_no_default` suffix, creates another one of the original
name that calls into `_no_default` one and moves anything related to
`Default`. This cleanly ensures all previous hashes stay the same while
siphash gets `Default` removed. This also removes all now-conflicting
impls from `siphash24` module which makes the module almost identical to
what it looked like before the change. The only differences are removed
`Default`/`new`, fixes in tests and recent rename of `as_u64` to
`to_u64`.
d03e5456a3 ci: fix `rev` to be `ref` when pinning bitcoin-maintainer-tools (Andrew Poelstra)
d04b6aabe5 bitcoin: add a couple missing prelude imports (Andrew Poelstra)
30f6bd43a4 Add primitives to the CRATES list (Andrew Poelstra)
e1e0230827 primitives: Enable alloc from serde (Tobin C. Harding)
ce36a37d68 primitives: Test ordered feature (Tobin C. Harding)
Pull request description:
This is a rebase of #3195 on master (since #3200 was merged) and adds one commit which fixes a few missing imports, which it appears was missed in the other "fix CI" PRs.
ACKs for top commit:
Kixunil:
ACK d03e5456a3
tcharding:
ACK d03e5456a3
Tree-SHA512: 6f5dc5c2cbb42716e83e46e70dbf4d644f9e80f6b3d2e75a276e37d33d461f4a86e67eb5c32444dbd23bb9767abedc88bfe958b1beba39852c382cf73736711f
Apparently the `checkout` action will just grab random shit in the case
that you configure it with an unknown key. There is a warning in the
middle of the CI output so ok, I guess there's that.
This commit does not change the pinned version of
rust-bitcoin-maintainer-tools, though we do want to update the pin,
because I want to make sure that the pin is actually working.
This should have been done when we introduced the `primitives`
crate - epic fail.
Intentionally put it before `fuzz` because of the known bug in the CI
script where nothing after `fuzz` gets tested.
Currently the `serde` feature requires an allocator, this is a hang over
from code being written for the `bitcoin` crate.
This was not found because we are not testing `primitives` correctly in
CI (there is the `fuzz` bug and also `primitives` is not even in the
CRATES list).
60b3cabb41 Panic in const context (Tobin C. Harding)
Pull request description:
Now that our MSRV is past 1.57 we can panic in const contexts.
Fix: #2427
ACKs for top commit:
Kixunil:
ACK 60b3cabb41
apoelstra:
ACK 60b3cabb41 successfully ran local tests
Tree-SHA512: 705a8b7d52a11826e6084684706cb7e01dfaa554e4e369739e64e64263537b0c8c0e518b04e96249ecdeea1f22b534594ffd2a89e17ebba85b369d896e820239
c97389596b Remove stale docs from sha256t_hash_newtype (Tobin C. Harding)
39f7dcb816 Reduce API surface of tagged wrapped hash types (Tobin C. Harding)
Pull request description:
Recently we made it so that wrapper types created with `hash_newtype` were not general purpose hash types i.e., one could not easily hash arbitrary data into them. We would like to do the same for tagged wrapped hash types.
In `hashes` do:
- Create a new macro `sha256_tag` that does just the tag/engine stuff out of the `sha256t_hash_newtype` macro.
- Deprecate the `sha256t_hash_newtype` macro.
In `bitcoin` do:
- Use a combination of `sha256_tag` and `hash_newtype` to create tagged wrapped hash types.
Note that we do not add private helper functions `engine` and `from_engine` to the tagged wrapper types as we do for legacy/segwit in `sighash`. Can be done later if wanted/needed.
Fix: #3135
ACKs for top commit:
Kixunil:
ACK c97389596b
apoelstra:
ACK c97389596b successfully ran local tests
Tree-SHA512: d937a8eac1a77298231f946f9dfbc2f7739af8da00f2075b0b54803b4111c0cec810bc6564515153769193056cf102a9c954e216664f055b249d4a6153b14bca
Tobin C. Harding
merge-script
Jamil Lambert, PhD
yancy
Martin Habovstiak
elsirion
Chris Hyunhum Cho
Update Nightly Rustc Bot
Jose Storopoli
Rob N