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>`.
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.
b2344e019d units: Assert roundtrip SignedAmount/str overflows (Tobin C. Harding)
baadcf4c0a units: Test that SignedAmount float conversion overflows (Tobin C. Harding)
d768f25da8 units: Remove duplicate assertion (Tobin C. Harding)
1d536ac8b2 units: Enable parsing Amount from u64::MAX (Tobin C. Harding)
Pull request description:
Our `Amount` type uses an internal `u64` and we maintain no invariants on the inner value. Therefore we should be able to parse `u64::MAX`.
Fix the parsing code by removing the explicit, incorrect check and fix unit tests to mirror this behaviour.
Fix: #2297
ACKs for top commit:
Kixunil:
ACK b2344e019d
apoelstra:
ACK b2344e019d
Tree-SHA512: 944f8d0bfedc559f0444f75eca7d3fba042fbc204c4c032d09ff0edc29be280a3707f5b363dbc04f0d7bdf64701c0c4619e2e0de683d804a2663c2a20ac963f6
c6c5f07880 Add automated labeler job (Martin Habovstiak)
Pull request description:
Rather than modifying the labels manually, which we often forget, we can label the PRs automatically. This will make it easier to search PRs.
ACKs for top commit:
apoelstra:
ACK c6c5f07880
tcharding:
ACK c6c5f07880
Tree-SHA512: d3f7ca8c69cc8734d914f871675568ca293e28c3dc7351ee54b9237645204b87d1b47db02863b27d930900ae4b41d6f169f130ed9f4c750d4afbb765921cfb66
22747149a9 Add convenience constants to `Denomination` (Martin Habovstiak)
Pull request description:
`Denomination::Bitcoin` and `Denomination::Satoshi` are often used, especially in test code so this change adds `BTC` and `SAT` - short, readable constants. Notably this doesn't add the other constants as that would lead to either unidiomatic names or confusing casing (MSAT meaning millisat not megasat) and they are not used that much anyway.
ACKs for top commit:
apoelstra:
ACK 22747149a9
tcharding:
ACK 22747149a9
Tree-SHA512: fb49e61c28f5df14d5432a81ec946a367696fe36e96798d35831cd5cb88b813fe3aa1e7be201997956b4d4f6e5431e4da20d0d24e4d6c036aa199fc9b9c87da1
The errors `SegwitV0Error` and `LegacyScripthashError` contained only
one variant - out of range. There will not be a new one in the future so
this change flattens it to simplify.
We should not be able to roundtrip a `SignedAmount` value greater than
`MAX`, add a test to prove so.
While we are at it document the assertion above that proves we can parse
a float representing an `Amount` greater than `SignedAmount::MAX`.
fe8d559d69 test: add invalid segwit transaction test (startup-dreamer)
Pull request description:
Tries to close#2183
Added the test for invalid segwit transaction (witness flag is set but no witness is present) using [This suggested hex](https://github.com/rust-bitcoin/rust-bitcoin/issues/2183#issuecomment-1901207149) by Kixunil
ACKs for top commit:
Kixunil:
ACK fe8d559d69
apoelstra:
ACK fe8d559d69
Tree-SHA512: 723027e0ad9944a4763fba1e12398d7bcacdef691a40168f0be7cecdb170936f7e0c3690c4de911d086b8c5d42f5a25784f53fe096404f5cf69d6fc75c645d6e
`Denomination::Bitcoin` and `Denomination::Satoshi` are often used,
especially in test code so this change adds `BTC` and `SAT` - short,
readable constants. Notably this doesn't add the other constants as that
would lead to either unidiomatic names or confusing casing (MSAT meaning
millisat not megasat) and they are not used that much anyway.
47b476ec5b Ignore fuzz dir from coverage reports (pool2win)
50ff68550c Add github action for llvm-cov coverage (pool2win)
Pull request description:
There has been discussion around generating coverage reports for tests. See #1853 and #2353.
This PR adds an action using llvm-cov and coveralls.
The action generates lcov coverage report using llvm-cov and uploads the generated report to coveralls. You can see sample reports for my fork here: https://coveralls.io/github/pool2win/rust-bitcoin
I am using the [cargo-llvm-cov](https://github.com/taiki-e/cargo-llvm-cov) wrapper around llvm-cov.
I also use the coveralls official github action to push the coverage report to coveralls. It removes the need to deal with repository secrets etc, making the action easy to run for all contributors on their forks.
We can move this action later to rust.yml if we need to.
ACKs for top commit:
apoelstra:
ACK 47b476ec5b
Kixunil:
ACK 47b476ec5b
tcharding:
ACK 47b476ec5b
Tree-SHA512: 7a7b8caf94d4b4828416bc1e6df49bfe47fede7b8fa4386d905a6f40439a95df1d3fedecdd6d64675f2f6858851f18d2f8863994aa20ac1ab224f97ca48b4af2
Tobin C. Harding
Andrew Poelstra
Martin Habovstiak
yancy
dependabot[bot]