3f244db19d Add Arbitrary to SignedAmount type (yancy)
Pull request description:
While proptesting, I've found Arbitrary for SingedAmount to be useful. Would appreciate adding this as well.
ACKs for top commit:
tcharding:
ACK 3f244db19d
Kixunil:
ACK 3f244db19d
Tree-SHA512: f293215505db85998135c75e147f4cb0031a08c7db619b706e6c7d632c6f23783af56992531396aecc6e3da55b107a7f2c7ec4d68a3e5737ebcf4225f306ed29
4024ba5910 Add unit test for ArrayVec::extend_from_slice (Tobin C. Harding)
798c9cff1c Fix bug in ArrayVec::extend_from_slice (Tobin C. Harding)
Pull request description:
Currently the source slice must be the exact length to fill the array to max capacity, this is an unnecessary restriction since an `ArrayVec` is a variable sized data structure.
Set the destination slice to be the same length as the source slice (still maintain capacity checks).
ACKs for top commit:
Kixunil:
ACK 4024ba5910
apoelstra:
ACK 4024ba5910 successfully ran local tests
Tree-SHA512: b7a510ba6f6e5cf5a535d691f79da98e29558eb9a50b60d41e080769560e9b9f0b8a831806dc9f159c888001951e17b5bbd970363d6a2e57fa9748902548b3ab
dae42bef9d do not enable bitcoin-io by default (Antoni Spaanderman)
a14cdaf859 don't enable std by default when testing (Antoni Spaanderman)
e83830dcfc use slice instead of array to not have to hardcode the length (Antoni Spaanderman)
55749d6f61 use `hash.to_byte_array` to check equality with `test.output` (Antoni Spaanderman)
969864e3b0 use fixed size array if possible, otherwise `&'static [u8]` (Antoni Spaanderman)
28ccf70fa6 remove unnecesarry borrow operator (`&`) (Antoni Spaanderman)
fa3a3afd02 remove unnecessary slicing (Antoni Spaanderman)
22e42ab86c fix test code being unnecessarily feature gated (Antoni Spaanderman)
Pull request description:
- remove 2 unnecessary cfg attributes from tests left over from #3167 (it made them not dependent on `alloc` anymore)
- simplify assertion logic by removing unnecessary conversions before comparing
- make tests `no_std` compatible by adding imports to alloc or std
- feature gate tests behind the `alloc` feature if they use anything from the alloc crate (like the `format!` macro)
- `schemars` feature enables `alloc` because (for example) its trait wants implementations to return `String`
- fix `bitcoin-io` always enabling when `std` is enabled (only useful if people depend on `hashes` only, `bitcoin` depends on `bitcoin-io` already)
ACKs for top commit:
tcharding:
ACK dae42bef9d
Kixunil:
ACK dae42bef9d
apoelstra:
ACK dae42bef9d successfully ran local tests
Tree-SHA512: 622fd4963ef21530a98af89bcfc71abe8723aac12d363ab88d9bd30dcf2f75392711bec10e2901fab5f1a30e11897d1aae36e22892738aa1e5670166f91fddd4
Currently the source slice must be the exact length to fill the array to
max capacity, this is an unnecessary restriction since an `ArrayVec` is
a variable sized data structure.
Set the destination slice to be the same length as the source
slice (still maintain capacity checks).
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
- make tests no_std compatible by adding imports to alloc or std
- feature gate tests behind the 'alloc' feature if they use anything
from 'alloc' (like the `format!` macro)
- schemars feature enables alloc
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
Jiri Jakes
merge-script
yancy
Tobin C. Harding
Jamil Lambert, PhD
Martin Habovstiak
elsirion
Antoni Spaanderman
Chris Hyunhum Cho
Update Nightly Rustc Bot
Jose Storopoli