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
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
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
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.
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.
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
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.
a338a61cc3 Remove quadratic algorithm (Tobin C. Harding)
Pull request description:
Currently we loop over transaction inputs within a loop over transaction inputs - ouch.
Cache the `use_segwit_serialization` outside the iteration loop.
Fix: #2357
ACKs for top commit:
Kixunil:
ACK a338a61cc3
apoelstra:
ACK a338a61cc3
Tree-SHA512: 91d0b46b235db57d9c28fc8da5d43a52c76a29916797a4ec44273b91eb120a928050a79cdbd704b922635dd2130db7b6e7863fd10e878eee52882c661af54c11
e356ff6611 Remove the now unused sighash::Error type (Tobin C. Harding)
c17324c574 Introduce segwit sighash error types (Tobin C. Harding)
f0b567313b Introduce sighash::LegacyError (Tobin C. Harding)
a1b21e2f1d Introduce sighash::TaprootError (Tobin C. Harding)
b0f20903a5 Introduce AnnexError (Tobin C. Harding)
a1a2056829 Add tx_in/tx_out accessor methods on Transaction (Tobin C. Harding)
f08aa16e91 Use Self:: in error return type (Tobin C. Harding)
Pull request description:
Improve the error handling in the `sighash` module by adding small specific error types.
Close: #2150
ACKs for top commit:
Kixunil:
ACK e356ff6611
apoelstra:
ACK e356ff6611
Tree-SHA512: e2e98a4caccae4e4acdc0e577e369fc90ee39a2206a8a1451739695fbe33ec2c3a52482b70cec8f9ee6bdb3ad7a2f4f639e8c87031878cd5d816fae24d913c42
61bf462806 Use full path in all macro usage of Result (josibake)
Pull request description:
Follow-up to https://github.com/rust-bitcoin/rust-bitcoin/pull/2355
Couldn't think of a clever way to do this , so just grepped for all instances of `macro_rules` and added the full path for the imports. Wasn't sure if it was necessary for `fmt::Result`, but went ahead and added the full path for consistency.
Tested locally and confirmed this fixes the issue I was seeing.
ACKs for top commit:
apoelstra:
ACK 61bf462806
Kixunil:
ACK 61bf462806
tcharding:
ACK 61bf462806
Tree-SHA512: 8af105b3e6a36723804b290f8254f52e65cd42a61c323f1190e3bcbcb9e4427ff9b026a4530bafcd14aab644ccd9401fed351494457194c3a68a55f11b2a3d04
In a few places in the codebase we want to grab an reference to an input
by index. To reduce code duplication add two methods on `Transaction`,
each to get a reference to an input or output respectively.
These are public methods, do not use them yet internally.
Currently we loop over transaction inputs within a loop over transaction
inputs - ouch.
Cache the `use_segwit_serialization` outside the iteration loop.
Fix: #2357
aa6e5cd342 Use full path in all macro usage of Result (Steven Roose)
Pull request description:
Apparently when someone uses a custom `Result` type and then uses some of these macros, they can get type conflict errors.
(Thanks josibake for finding this using the `sha256t_hash_newtype` macro.)
ACKs for top commit:
josibake:
utACK aa6e5cd342
Kixunil:
ACK aa6e5cd342
apoelstra:
ACK aa6e5cd342
Tree-SHA512: 04e51d6a4da520fd03f8c20c41707e43fc8d909de68533959373afd99e654068cedc5a6ca30bdc867a33e7e42b971a3bba623fad0fd294359948018ed55dc662
Our decoding code reads bytes in very small chunks. Which is not
efficient when dealing with the OS where the cost of a context switch is
significant. People could already buffer the data but it's easy to
forget it by accident.
This change requires the new `io::BufRead` trait instead of `io::Read`
in all bounds.
Code such as `Transaction::consensus_decode(&mut File::open(foo))` will
break after this is applied, uncovering the inefficiency.
This was originally Kix's work, done before we had the `io` crate.
Changes to `bitcoin` were originally his, any new mistakes are my own.
Changes to `io` are mine.
Co-developed-by: Martin Habovstiak <martin.habovstiak@gmail.com>
2dfe455161 Remove mention of core2 (Tobin C. Harding)
Pull request description:
We no longer depend on `core2`, remove stale code comment mention of the crate.
Fix: #2034
ACKs for top commit:
Kixunil:
ACK 2dfe455161
apoelstra:
ACK 2dfe455161
Tree-SHA512: cb723a384cd69e5b1aa70bdb25f53c818092c465783bd8a9b1ec60af488ed013d39f29057b4b09d6347b8bc52911eb6daf609bd088dec172647dbfedc2ea1791
Tobin C. Harding
Andrew Poelstra
yancy
Martin Habovstiak
startup-dreamer
josibake
Steven Roose
Thabokani