13c8a709f1 Use nightly toolchain in pre-commit hook (Tobin C. Harding)
Pull request description:
We use the nightly toolchain to run `clippy` now, update the git pre-commit hook to mirror this.
ACKs for top commit:
apoelstra:
ACK 13c8a709f1
Tree-SHA512: 46700641b81a1e5a2e39ff67ca0d31afaedfb58e008bf453c13f0312b2c6936ae38c58548934cae40abc9de0dc1ec4ce4bba4b8142b204d774612bd9721679c9
dda83707a2 Use `unsigned_abs` instead of manual code (Martin Habovstiak)
Pull request description:
The code originally used `if` and incorrectly casted the value into `usize` rather than `u64`. This change replaces the whole thing with `unsigned_abs`.
Closes#1247
ACKs for top commit:
apoelstra:
ACK dda83707a2
tcharding:
ACK dda83707a2
Tree-SHA512: c57bf440705c69917206aab36bbbe5b048ed52d7a96ebd055416b8a249fc3f7ba32ebff3d2251b971e7ef999ff8169ac3db8e599ab38cf0776ecc030447a9a4a
ac26171c32 Clean up `no_std` usage (Martin Habovstiak)
fce03cec85 Provide `Amount` & co in no-alloc (Martin Habovstiak)
Pull request description:
Using the crate without allocation was previously disabled making the
crate empty without the feature. This chage makes it more fine-grained:
it only disables string and float conversions which use allocator. We
could later provide float conversions by using a sufficiently-long
`ArrayString`.
Note that this is API-breaking because we disallow calling the methods of the sealed `SerdeAmount` trait. However I think it should've been obvious that the thing is internal and calling them is not a great idea. (BTW I only learned this trick recently).
Closes#2389
ACKs for top commit:
apoelstra:
ACK ac26171c32 maaybe `private` should be renamed to `internal_api` or something
tcharding:
ACK ac26171c32
Tree-SHA512: 2ca2ff11c3c362f868c3993b5698ace32e6ce2cf2e8028bc43fe65797eb2239b4841c1c722a0184a7c8f4afea3b475b1a049dd77fa30358cb742d60463018e9f
The code originally used `if` and incorrectly casted the value into
`usize` rather than `u64`. This change replaces the whole thing with
`unsigned_abs`.
Closes#1247
61f8bab65f just: Add quick and dirty CI command (Tobin C. Harding)
e185fe46df just: Lint with nightly toolchain (Tobin C. Harding)
Pull request description:
Improve the `justfile` by doing:
- Update linter toolchain
- Add `just sane` to run a minimal set of checks/tests that can be used pre-push but is not as slow as using `DO_FEATURE_MATRIX=true contrib/test.sh`.
ACKs for top commit:
apoelstra:
ACK 61f8bab65f
Kixunil:
ACK 61f8bab65f
Tree-SHA512: 730874f0381db9ae30052ad6511805f76ae5c13c4c5cc5ad272430cf7e67cfbe7b288aa6dc47035ff5e8e42e03d3e4a3bf9d3e9a072c9aa922f9df62c43850b3
Previously the crate used negative reasoning to enable `std` which was
hard to understand, required the `prelude` module and wasn't really
needed because it's only needed when a crate wants to add `alloc`
feature-backwards compatibly and this crate always had the feature.
This cleans up usage to unconditionally use `#[no_std]` and then just
add `extern crate` on top as needed by activated features.
Using the crate without allocation was previously disabled making the
crate empty without the feature. This chage makes it more fine-grained:
it only disables string and float conversions which use allocator. We
could later provide float conversions by using a sufficiently-long
`ArrayString`.
7bf478373a Model `TooBig` and `Negative` as `OutOfRange` (Martin Habovstiak)
54cbbf804f Express `i64::MAX + 1` as `i64::MIN.unsigned_abs()` (Martin Habovstiak)
b562a18914 Move denomination error out of `ParseAmountError` (Martin Habovstiak)
5e6c65bc1a Clean up `unsigned_abs` (Martin Habovstiak)
Pull request description:
Closes#2265Closes#2266
Disclaimer: I did this in December and don't remember why I haven't pushed it. Maybe because it's somehow broken but I don't see how so please review a bit more carefully just in case.
ACKs for top commit:
tcharding:
ACK 7bf478373a
apoelstra:
ACK 7bf478373a
Tree-SHA512: 1f6e9adae9168bd045c9b09f06d9a69efd47ccc7709ac9ecaf48cb86e265b448b9b52a199ac5e6838d5029f5bc7514c5d7deb15a4d7c8a4606a353f390745570
6ddb5cce37 Use Magic::BITCOIN in unit tests (Tobin C. Harding)
Pull request description:
We are currently calling `From` to create the magic bytes, this is unnecessary since `Magic` provides consts.
Refactor only, no logic changes.
ACKs for top commit:
Kixunil:
ACK 6ddb5cce37
apoelstra:
ACK 6ddb5cce37
Tree-SHA512: 20e2e017683f123309e3c0876bba42d86a9411bb225f07c486716184fc79837e04a832338ec8b18874ac76791260f6a4620b932ede92c8b222dac08d468cef8a
5eb2de1660 Remove TODO about rand trait (Tobin C. Harding)
66cc007c2b p2p: Remove TODO comments (Tobin C. Harding)
0b5fb45ea0 consensus: Remove HEX_BUF_SIZE todo (Tobin C. Harding)
579668892a consensus: Remove TODO (Tobin C. Harding)
53beb9db30 Remove ancient todos in test code (Tobin C. Harding)
abe2241828 units: Remove "alloc" TODO (Tobin C. Harding)
5386ef0fd2 psbt: Delete TODO comments (Tobin C. Harding)
14c8a2232b examples: Remove TODO (Tobin C. Harding)
Pull request description:
Done while working on #2368. There are 5 left. Do we want to leave the MSRV ones in there?
```bash
bitcoin/src/blockdata/weight.rs:66: // TODO replace with panic!() when MSRV = 1.57+
bitcoin/src/consensus/serde.rs:101: // TODO: statically prove impossible cases
bitcoin/src/pow.rs:445: // TODO: Use `carrying_mul` when stabilized: https://github.com/rust-lang/rust/issues/85532
units/src/amount.rs:595: // TODO replace whith unwrap() when available in const context.
units/src/amount.rs:599: // TODO replace with panic!() when MSRV = 1.57+
```
ACKs for top commit:
Kixunil:
ACK 5eb2de1660
apoelstra:
ACK 5eb2de1660
Tree-SHA512: 285b1711a6e6fba126e2c4159b25454c7f894122b76fde1d3d29e57b2ec0a6e90230e46ac79d70aa133da177c75d267fc5a13489b69881862649de771027ec8e
6715e93e89 Add Witness::p2tr_key_spend function (Tobin C. Harding)
Pull request description:
Add a function for creating the witness when doing a key path spend for a P2TR output.
This mirrors what we did for P2WPKH when adding `Witness::p2wpkh`.
Includes update to the taproot signing example to use the new constructor.
ACKs for top commit:
Kixunil:
ACK 6715e93e89
apoelstra:
ACK 6715e93e89
Tree-SHA512: aab51329e8fda471442bb9cebd6327636548dd157bb9842fe66993fcdd211bb04b2b829aa9d5962dd619f5c0b73d19644a44529c1a5958df1a6bc892147b44f5
Development for `psbt` has move to another repo, these TODO comments are
over there alread, lets just remove them from `rust-bitcoin` as part of
an effort to remove TODOs from the codebase.
fb81bff61f Add a from impl for ParseIntError (Tobin C. Harding)
2130150df6 absolute: Use Self in error type (Tobin C. Harding)
Pull request description:
While reviewing #2335 I noticed a few places that error code needed some love.
ACKs for top commit:
Kixunil:
ACK fb81bff61f
Harshit933:
ACK [`fb81bff`](fb81bff61f)
apoelstra:
ACK fb81bff61f
Tree-SHA512: 17f4e448862be47534d4c1c2962d5db8f90a471e53226022d7c2e153fc501705cd65b070eb17f3239fb8ad242223340f78fe828ea7df3d43707d3e42fcbef557
3cfd746bbc Add functionality to serialize signatures to a writer (Tobin C. Harding)
Pull request description:
Serializing the ecdsa and taproot `Signature` straight to a writer is a useful thing to be able to do.
Add `to_writer` to both `SerializedSignature`s and also to the `Signature`s (calling through to `SerializedSignature`).
Remove TODO comments from code.
ACKs for top commit:
Kixunil:
ACK 3cfd746bbc
Tree-SHA512: 82eb6d42c7b327cdfe5e89348890e45ea39c664420f7ea17d7826a5c388c7aaae917b1334e3f3df645fc4a81a11b59d97c7d6958e99077fbd67193e2a588f2eb
faa45cf10f Remove stale comment (Tobin C. Harding)
c82f26e960 Use hex-conservative to display pubkey (Tobin C. Harding)
Pull request description:
We introduced `hex-conservative` ages ago, use it to display the `PublicKey`.
ACKs for top commit:
Kixunil:
ACK faa45cf10f
apoelstra:
ACK faa45cf10f
Tree-SHA512: 8ad14c7697314f8393ecb9a287215c505924d0655f7bf3536d4be83af983b142e06a96f802beb4548e2de051f1783549d8d1d1a8ebfb678f372a54010717752e
1c4614ea30 hashes: Remove serde-std feature (Tobin C. Harding)
Pull request description:
As we do for "rand-std" in the `bitcoin` crate we can enable "std" when the "serde-std" feature is enabled. This makes the features more explicit and arguably more ergonomic.
Found while working on #2353, no additional testing added hear (in CI).
ACKs for top commit:
Kixunil:
ACK 1c4614ea30
apoelstra:
ACK 1c4614ea30
Tree-SHA512: b82b72eacdd84cca076747a0eb0c7a625ec6f918701699ca7e7159e0606b089c80182e9d54e1d48b2957815883a36dfa337e353573a6ffe4fa19458615111080
3c4f6850f4 Flatten trivial errors. (Martin Habovstiak)
a4d01d0b6c Factor out `io::Error` from sighash errors (Martin Habovstiak)
Pull request description:
The hadnling of `io::Error` in sighash had a few problems:
* It used `io::ErrorKind` instead of `io::Error` losing inforation
* Changing `io::ErrorKind` to `io::Error` would disable `PartialEq`&co
* The `Io` error wariants were duplicated
It turns out all of these can be solved by moving the `Io` variant into a separate error.
ACKs for top commit:
apoelstra:
ACK 3c4f6850f4
tcharding:
ACK 3c4f6850f4
Tree-SHA512: b7ad6b692062d636ce29e4ebb448a8ac8ea3090feee1d349472e13f905f1f3785decc86e037d2d9658c1331a271e730076139a8d8f6c9b7dadda8b3221f6d434
The error returned when parsing amount had a `Negative` variant which
was weird/unreachable when parsing `SignedAmount`. Also weirdly, parsing
would return `TooBig` when the amount was negative - too low.
To resolve this we merge them into one `OutOfRange` variant that nuges
the consumers to make principled decisions and print error messages as
amounts being more than or less than a specific value which is easier to
understand for the users. Notably, the API still allows getting
information about which type was parsed and which bound was crossed but
in a less obvious way. This is OK since users who have a very good
reason can use this information but most won't.
Closes#2266
The `from_str_in` methods on amounts returned `ParseAmountError` which
contained `InvalidDenomination` among its variants. This one was never
returned because the method doesn't parse denomination.
This change separates the error out.
Closes#2265
Previousle we copied `unsigned_abs` method from `core` because it was
unstable in older MSRV. Our current MSRV allows using the method
directly so this removes our old one and uses the one from standard
library instead.
Add a function for creating the witness when doing a key path spend for
a P2TR output.
This mirrors what we did for P2WPKH when adding `Witness::p2wpkh`.
Includes update to the taproot signing example to use the new constructor.
As is customary add a `From` impl for the `ParseIntError` and use `?`.
While this does not make much difference it saves devs wondering why
there is a `From` impl for one of the variants and not the other.
Serializing the ecdsa and taproot `Signature` straight to a writer is a
useful thing to be able to do.
To both ECDSA and Taproot types:
- Add `SerializedSignature::to_writer`
- Add `Signature::serialize_to_writer`
Remove TODO comments from code.
dae16f052c Use any method on iterator (Tobin C. Harding)
671dc0e9e0 Use better predicate name (Tobin C. Harding)
Pull request description:
- Patch 1: Improve the name.
- Patch 2: Use `any` instead of manual loop.
ACKs for top commit:
Kixunil:
ACK dae16f052c
apoelstra:
ACK dae16f052c
Tree-SHA512: 9b98c21cbb5fa93c011ba8bae88ab0dd2efa807d7cb220f121c56b31fbe5a9e9cd6f29badeca092e22949dc278c1a9d3dd676cd2919a11f13101d0dd83ec9313
20a5f1f35f Use KnowHrp instead of Network (Tobin C. Harding)
Pull request description:
We have a bunch of functions that take `Network` when what they really want is something that can be converted to a `KnownHrp`.
Make `KnownHrp` public and accept `impl Into<KnownHrp>`.
ACKs for top commit:
Kixunil:
ACK 20a5f1f35f
apoelstra:
ACK 20a5f1f35f
Tree-SHA512: d13ae989ca5136523902e938a04357776e00c650ec8699b335f04798a2fb4ea55e596b200b3ba1807d897884362ef9c419a15193ffdbd4ec26be53152a8ac1d3
9eeadaab98 bitcoin: Remove bech32 from the public API (Tobin C. Harding)
Pull request description:
The only place that `bech32` appears in the pubic API is as a pub extern crate re-export. This is totally unnecessary since no other `bech32` functions or types appear in the public API.
Removing `bech32` from the public API allows us to stabilize `rust-bitcoin` without waiting for `bech32` to stabalize - WIN.
ACKs for top commit:
Kixunil:
ACK 9eeadaab98
apoelstra:
ACK 9eeadaab98
Tree-SHA512: f411df7c38b417c1a4b9c175e7f7df7631d25ce23351eae8d77dff5c9aed5a4ae3b3755f0eb6e7d109f040e93d89f6777d74c2306314895f52fd349c91996c95
66352cba98 Add kani test and remove TODO (Tobin C. Harding)
Pull request description:
Add a kani test to check `div_rem`.
ACKs for top commit:
Kixunil:
ACK 66352cba98
apoelstra:
ACK 66352cba98
Tree-SHA512: 2cb277e11d9d853f92b12e3c1e93f4a1094466a7aab9a9a8d0883e015b4c0d876678d8147f107155dee11911954e7b3da6ee25e0e67f39f76a4da45b1dbc5307
1fe90223aa Remove DO_COV (Tobin C. Harding)
Pull request description:
We have test coverage by way of `coveralls` now. Remove the old stale `DO_COV` stuff.
Fix: #1853
ACKs for top commit:
Kixunil:
ACK 1fe90223aa
apoelstra:
ACK 1fe90223aa
Tree-SHA512: d54a0f4566e54f7f6b42474530d76049e64c6a79c6a38746f2cb9d455592f22747dcaaffb2fc8e39340c0861a5e1bfaad222459a7da4474093ed33f83b2f4fcd
We have a bunch of functions that take `Network` when what they really
want is something that can be converted to a `KnownHrp`.
Make `KnownHrp` public and accept `impl Into<KnownHrp>`.