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
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
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
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
Andrew Poelstra
Tobin C. Harding
yancy
Martin Habovstiak
startup-dreamer