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
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
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
de9c2bc43d p2p: Improve nonce documentation (Tobin C. Harding)
Pull request description:
Better describe what the nonce is used for.
Note this file has not had its docs manicured so the line length is 80 still, just use the same line length instead of the conventional 100.
Fix: #575
ACKs for top commit:
Kixunil:
ACK de9c2bc43d
apoelstra:
ACK de9c2bc43d
Tree-SHA512: ab18d9893fff4e673373125e607a4a60843a98cf84dc336fba9b6423da24ea3ad4c5fe5846ae8bcef51962dc2f3017157f2d7301c2c2cd1a81a37c3da6823552
The effective_value method is useful for coin selection algorithms. By
providing this effective value method, the effective value of each
output can be known during the coin selection process.
Better describe what the nonce is used for.
Note this file has not had its docs manicured so the line length is 80
still, just use the same line length instead of the conventional 100.
Fix: #575
518f0970c9 Implement ArbitaryOrd for absolute::LockTime (Tobin C. Harding)
Pull request description:
At times we would like to provide types that do not implement `PartialOrd` and `Ord` because it does not make sense. I.e we do not want users writing `a < b`. This could range from kind-of-iffy to down-right-buggy (like comparing absolute locktimes).
However this decision effects downstream users who may not care about what the ordering means they just need to use it for some other reason e.g., to use as part of a key for a `BTreeMap` (as we do in `miniscript` requiring the `AbsLockTime` type).
A solution to this problem is to provide a wrapper data type that adds `PartialOrd` and `Ord` implementations. I wrote the `ordered` crate is for this very purpose.
ACKs for top commit:
apoelstra:
ACK 518f0970c9
Kixunil:
ACK 518f0970c9
Tree-SHA512: 05c753e650b6e2f181caf7dc363c4f8ec89237b42883bd695a64da0661436c9a7e715347f8fcf4fb19ce069cbf75a93032052e946f05fd8029f61860cf9c6225
Applies to both `ecdsa::Signature` and `taproot::Signature`.
Re-name the `Signature` fields with more descriptive names. The
names used were decided upon in the issue discussion.
Impove rustdocs while we are at it.
Note, the change to `sign-tx-segwit-v0` is refactor only, the diff does
not show it but we have a local variable already called `sighash_type`
that is equal to `EcdsaSighashType::All`.
Includes a function argument rename as well, just to be uniform.
Fix: #2139
Andrew Poelstra
yancy
Tobin C. Harding
Martin Habovstiak
startup-dreamer
josibake
Steven Roose
Thabokani