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
8783d526bd fix : adds the arrayvec dependency (harshit933)
Pull request description:
This commit adds the arrayvec dependency to the sortKey.
Potential fix#2276
ACKs for top commit:
Kixunil:
ACK 8783d526bd
apoelstra:
ACK 8783d526bd
Tree-SHA512: 35f28ade02dd526ce5dfa2f42578b36cd5af29a5a9f409da70a775bc12046674737e9bce9fabcc87f1b4669080ad10465c75601342f280c11eab11f791f44c36
We only use the network to serialize and deserialize from WIF.
For this we only really need network kind since WIF only differentiates
between mainnet and non-mainnet.
a92d49fe33 Implement `CompressedPublicKey` (Martin Habovstiak)
Pull request description:
P2WPKH requires keys to be compressed which introduces error handling even in cases when it's statically known that a key is compressed. To avoid it, this change introduces `CompressedPublicKey` which is similar to `PublicKey` except it's statically known to be compressed.
This also changes relevant code to use `CompressedPublicKey` instead of `PublicKey`.
ACKs for top commit:
tcharding:
ACK a92d49fe33
apoelstra:
ACK a92d49fe33
Tree-SHA512: ff5ff8f0cf81035f042dd8fdd52a0801f0488aea56f3cdd840663abaf7ac1d25a0339cd8d1b00f1f92878c5bd55881bc1740424683cde0c28539b546f171ed4b
P2WPKH requires keys to be compressed which introduces error handling
even in cases when it's statically known that a key is compressed. To
avoid it, this change introduces `CompressedPublicKey` which is similar
to `PublicKey` except it's statically known to be compressed.
This also changes relevant code to use `CompressedPublicKey` instead of
`PublicKey`.
There is no advantage in having `io::Read` as opposed to `Read` and
importing the trait. It is surprising that we do so.
Remove `io::` path from `io::Read` and `io::Write`. Some docs keep the
path, leave them as is. Add import `use io::{Read, Write}`.
Refactor only, no logic changes.
When we use the `fmt::Write` trait it is just to call its methods, we
can therefore, without any change to the logic, use `as _` when
importing the trait. This prevents naming conflicts.
Done in preparation for importing the `io::Write` trait.
Generic types can be single letters, and a writer is conventionally, in
this codebase at least, called `W`.
Use `W` instead of `Write` with no loss of clarity.
1ee989a3af Remove private fmt_internal function (Tobin C. Harding)
923ce7402d Remove Network from AddressInner (Tobin C. Harding)
3490433618 Return error from wpubkey_hash (Tobin C. Harding)
f7ab253ce4 Remove stale comment (Tobin C. Harding)
Pull request description:
An `AddressInner` struct (contains `Network` field) is created when parsing address strings however address strings do not map 1:1 to `Network` because signet and testnet use the same bech32 prefix "tb".
We can fix this by inlining the `Payload` variants into `AddressInner` and adding prefix enums for legacy addresses and an `Hrp` for bech32 addresses.
Fix: #1819
ACKs for top commit:
Kixunil:
ACK 1ee989a3af
apoelstra:
ACK 1ee989a3af
Tree-SHA512: 1c2749dc929a1e9ad9b9feb01bec5c96b5aec07c6d646d88652deca7abe485907403116e9e29a0ab7dc06223254c4b49a384043284ec0a68fd76f9ab551e9e8a
3d17031725 Derive Debug for PrivateKey for no-std builds (Tobin C. Harding)
Pull request description:
Currently we derive `impl Debug for PrivateKey` for "std" builds and manually implement an obfuscated version for "no-std" builds. Since we enable the `hashes` feature of `rust-secp` this is unnecessary because secp takes care of obfuscating the secret for us.
ACKs for top commit:
apoelstra:
ACK 3d17031725
Kixunil:
ACK 3d17031725
Tree-SHA512: 0ce394c6517c51e8964290a980cddd20186d19bcc6cbb8c71aa09b7485d6a0df373960798418184971e1c6e5a6b8f725dd44ebfa7184e31b63faf105dea69725
801c72e056 Add deprecation comment to hash_types module (Tobin C. Harding)
61351c917f Move impl_asref_push_bytes to internal_macros (Tobin C. Harding)
2b4b66dee3 Move impl_hashencode to internal_macros (Tobin C. Harding)
2a0ac1258a Move the bip158 filter hash types (Tobin C. Harding)
3107f80aac Move transaction hash types (Tobin C. Harding)
61c02ff202 Move block hash types (Tobin C. Harding)
Pull request description:
Move hash types out of `hash_types` and into the modules where they are primarily used. Adds deprecated re-export so this is not a breaking change.
Is an alternate solution to #2072Resolves: #2072
ACKs for top commit:
apoelstra:
ACK 801c72e056
Kixunil:
ACK 801c72e056
Tree-SHA512: 4ccac63553de3f7d417213429c0f5c2b7ebc3c2d77a9feb6d4a7daa233565fc62617edf6426a421d251eadc0841235a719bd7fd3f980302c7a2bf3dacb8b4a61
98ce46c009 Update docs on witness_mut (Tobin C. Harding)
Pull request description:
Recently during the rust-bitcoin workshop at TABConf devs were thrown off by the example on `witness_mut`. We have some work going on to add examples and a cookbook that all demonstrate usage of `witness_mut`.
Remove the docs on `witness_mut` and direct devs to the `examples/sign-tx-*` files.
ACKs for top commit:
apoelstra:
ACK 98ce46c009
Kixunil:
ACK 98ce46c009
Tree-SHA512: e662213db4cbdaa53f6927cc1b10c1b6276f538cc6ad0d4bfff6dfcbf042f287a14bf5bfc88eeba7a32646c3d6741c5e09d11bb76666572a12a2043db55a2f38
Recently during the rust-bitcoin workshop at TABConf devs were thrown
off by the example on `witness_mut`.
Attempt to improve the docs on `witness_mut`.
Previously `taproot::Signature` could be only serialized into `Vec<u8>`
which forced allocation. This adds a `SerializedSignature` type which
acts like `Box<u8>` but is on stack.
Note: the code was copied from `secp256k1::ecdsa::serialized_signature`
with minimal changes.
We are emptying the `hash_types` module. `impl_asref_push_bytes!` is an
internal macro, as such it can live in the `internal_macros` module.
While we are at it import the macro and call it without any qualifying
path, this is typical for our usage of other internals/internal_macros
usage.
An `AddressInner` struct is created when parsing address strings however
address strings do not map 1:1 to `Network` because signet and testnet
use the same bech32 prefix "tb".
We can fix this by inlining the `Payload` variants into `AddressInner`
and adding prefix enums for legacy addresses and a `KnownHrp` for bech32
addresses.
Also enables removing the `AddressEncoding` struct as we can display the
`AddressInner` struct directly. (The `Display` impl is on `AddressInner`
and not directly on address to ignore the `NetworkValidation` wrapper,
may be able to be simplified still further.)
Calling `wpubkey_hash` on a key that is uncompressed is flat out an
error, really it is a programmer error at build time because a segwit
key should never be compressed, however, for historical reasons we do
not enforce this in the type system. As a step towards clarity make it
an error to call `wpubkey_hash` on a an uncompressed pubkey. This adds
documentation and potentially might assist debugging for newer devs.
Currently we derive `impl Debug for PrivateKey` for "std" builds and
manually implement an obfuscated version for "no-std" builds. Since
we enable the `hashes` feature of `rust-secp` this is unnecessary
because secp takes care of obfuscating the secret for us.
add371d263 Remove `core2` dependency entirely (Matt Corallo)
b7dd16da99 [IO] Use our own io::Error type (Matt Corallo)
c95b59327a Explicitly use `std::io::Error` when implementing `std` traits (Matt Corallo)
9e1cd372cb Use `io::Error::get_ref()` over `std::error::Error::source()` (Matt Corallo)
3caaadf9bb [IO] Replace the `io::Cursor` re-export with our own `Cursor` (Matt Corallo)
141343edb4 [IO] Move to custom `Read` trait mirroring `std::io::Read` (Matt Corallo)
7395093f94 Stop relying on `Take`'s `by_ref` method (Matt Corallo)
2364e1a877 Stop relying on blanket Read impl for all &mut Read (Matt Corallo)
6aa7ccf841 [IO] Replace `std::io::Sink` usage with our own trivial impl (Matt Corallo)
7eb5d65bda [IO] Provide a macro which implements `io::Write` for types (Matt Corallo)
ac678bb435 [IO] Move to custom `Write` trait mirroring `std::io::Write` (Matt Corallo)
5f2395ce56 Add missing `?Sized` bounds to `io::Write` parameters (Matt Corallo)
2348449d2a Stop relying on `std::io::Write`'s `&mut Write` blanket impl (Matt Corallo)
5e0209569c Use `io::sink` rather than our custom `EmptyWrite` utility (Matt Corallo)
a0ade883b6 [IO] Move io module into selected re-exports (Matt Corallo)
27c7c4e26a Add a `bitcoin_io` crate (Matt Corallo)
Pull request description:
In order to support standard (de)serialization of structs, the
`rust-bitcoin` ecosystem uses the standard `std::io::{Read,Write}`
traits. This works great for environments with `std`, however sadly
the `std::io` module has not yet been added to the `core` crate.
Thus, in `no-std`, the `rust-bitcoin` ecosystem has historically
used the `core2` crate to provide copies of the `std::io` module
without any major dependencies. Sadly, its one dependency,
`memchr`, recently broke our MSRV.
Worse, because we didn't want to take on any excess dependencies
for `std` builds, `rust-bitcoin` has had to have
mutually-exclusive `std` and `no-std` builds. This breaks general
assumptions about how features work in Rust, causing substantial
pain for applications far downstream of `rust-bitcoin` crates.
This is mostly done, I'm still finalizing the `io::Error` commit at the end to drop the `core2` required dep in no-std, but its getting there. Would love further feedback on the approach or code-level review on these first handful of commits.
ACKs for top commit:
tcharding:
ACK add371d263
apoelstra:
ACK add371d263
Kixunil:
ACK add371d263
Tree-SHA512: 18698ea8b1b65108ee0f695d5062d2562c8df2f50bf85d93442648da3b35a4184a5d5d2a493aed0adaadc83f663f0cd2ac735c34941cc9a6fa58d826e548e091
7d695f6b41 Improve public re-exports (Tobin C. Harding)
33774122e0 Remove public re-exports from private module (Tobin C. Harding)
Pull request description:
Improve the public exports in two ways:
1. Inline re-exports into the docs of the module that re-exports them.
2. Separate public and private use statements
Recently we discussed a way to separate the public and private import statements to make the code more clear and prevent `rustfmt` joining them all together.
Separate public exports using a code block and `#[rustfmt::skip]`. Has the nice advantage of reducing the number of `#[doc(inline)]` attributes also.
1. Modules first, as they are part of the project's structure.
2. Private imports
3. Public re-exports (using `rustfmt::skip` to prevent merge)
Use the format
```rust
mod xyz;
mod abc;
use ...;
pub use {
...,
};
```
This patch introduces changes to the rendered HTML docs.
ACKs for top commit:
apoelstra:
ACK 7d695f6b41
Tree-SHA512: dc9121c0fe282e3035d862beadb89e2d5a374a7dab6b1c3147a9b5960f8bc2f5af49892f0f713f55c645c46f53464c32daf390c11d85c75553b3ea7e0efc8246
Since we are no longer relying on the blanket `io::Write` impl for
`&mut io::Write`, we should now ensure that we do not require
`Sized` for our `io::Write` bounds, as its unnecessarily
restrictive and can no longer be worked around by simply adding an
`&mut`.
`std::io::Write` is implemented for all `&mut std::io::Write`. This
makes it easy to have APIs that mix and match owned `Write`s with
mutable references to `Write`s.
However, in the next commit we add our own `Write` trait which we
intend to implement for all `std::io::Write`. Sadly, this is
mutually exclusive with a blanket implementation on our own
`&mut Write`, as that would conflict with an `std::io::Write`
blanket impl.
Thus, in order to use the `Write for all &mut Write` blanket impl
in rust-bitcoin, we'd have to bound all `Write`s by
`std::io::Write`, as we're unable to provide a blanket
`Write for &mut Write` impl.
Here we stop relying on that blanket impl in order to introduce the
new trait in the next commit.
e21ee381bc Split Prevouts errors out into specific error types (Tobin C. Harding)
Pull request description:
Done as part of the great error clean up.
Currently we are returning a general `Error` from `Prevouts` functions, this is un-informative, we can do better by returning specific types that indicate the exact error path.
ACKs for top commit:
Kixunil:
ACK e21ee381bc
apoelstra:
ACK e21ee381bc
Tree-SHA512: 2a4900f9e31584ad2b6faafa17ea98742fff9206ee1bf77ed29624e0c7b05e655b3b6bf3710e2da26b0b2b8bd5eb36fdd81decbb1f55b41f153f0fbcc4a9165e
Improve the public exports in two ways:
1. Inline re-exports into the docs of the module that re-exports them.
2. Separate public and private use statements
Recently we discussed a way to separate the public and private import
statements to make the code more clear and prevent `rustfmt` joining
them all together.
Separate public exports using a code block and `#[rustfmt::skip]`. Has
the nice advantage of reducing the number of `#[doc(inline)]` attributes
also.
1. Modules first, as they are part of the project's structure.
2. Private imports
3. Public re-exports (using `rustfmt::skip` to prevent merge)
Use the format
```rust
mod xyz;
mod abc;
use ...;
pub use {
...,
};
```
This patch introduces changes to the rendered HTML docs.
Done as part of the great error clean up.
Currently we are returning a general `Error` from `Prevouts` functions,
this is un-informative, we can do better by returning specific types
that indicate the exact error path.
We have a new API function available with recent version of `secp256k1`
to create a `Message` directly from a sighash byte array.
Use `Message::from_digest(sighash.to_byte_array())` to construct
messages ready to sign.
Upgrade the `secp256k1` dependency to the newly released `v0.28.0`.
FTR this includes two simple changes:
- Use `Message::from_digest_slice` instead of `Message::from_slice`.
- Use `secp256k1::Keypair` instead of `secp256k1::KeyPair`.
In preparation for updating the secp dependency to v0.28.0, which
includes a change of `KeyPair` to `Keypair`, change our identifier usage
to indicate that "keypair" is a single word.
Deprecate the old forms.
8eff4d0385 Remove private hex test macro (Tobin C. Harding)
Pull request description:
We have this macro in `hex-conservative` now, remove the version here.
This patch does not change the public API and only touches test code.
ACKs for top commit:
apoelstra:
ACK 8eff4d0385
clarkmoody:
ACK 8eff4d0385
Tree-SHA512: 93a08fff778930071cd1a28c19202e4a94ca8881b2e873538de2e942b71c2cd6184ed6364c572538a8a699295a71761c6f836accaf251a15683138b71f148fab
On our way to v1.0.0 we are defining a standard for our error types,
this includes:
- Uses the following derives (unless not possible, usually because of `io::Error`)
`#[derive(Debug, Clone, PartialEq, Eq)]`
- Has `non_exhaustive` unless we really know we can commit to not adding
anything.
Furthermore, we are trying to make the codebase easy to read. Error code
is write-once-read-many (well it should be) so if we make all the error
code super uniform the users can flick to an error and quickly see what
it includes. In an effort to achieve this I have made up a style and
over recent times have change much of the error code to that new style,
this PR audits _all_ error types in the code base and enforces the
style, specifically:
- Is layed out: definition, [impl block], Display impl, error::Error impl, From impls
- `error::Error` impl matches on enum even if it returns `None` for all variants
- Display/Error impls import enum variants locally
- match uses *self and `ref e`
- error::Error variants that return `Some` come first, `None` after
Re: non_exhaustive
To make dev and review easier I have added `non_exhaustive` to _every_
error type. We can then remove it error by error as we see fit. This is
because it takes a bit of thinking to do and review where as this patch
should not take much brain power to review.
We would like the codebase to be optimized for readability not ease of
development, as such code that is write-once-read-many should not use
macros.
Currently we use the `impl_std_error` macro to implement
`std::error::Error` for struct error types. This makes the code harder
to read at a glance because one has to think what the macro does.
Remove the `impl_std_error` macro and write the code explicitly.
There is no logical default for the transaction version number, there is
only pre-bip68 (v1) and post-bip68 (v2). Uses should specify the version
they want not rely on us making the choice.
(I originally added this impl to support testing, this was in hindsight
the wrong thing to do, props to Sanket for noticing.)
BIP-68 activated a fair while ago (circa 2019) and since then only
transaction versions 1 and 2 have been considered standard.
Currently in our `Transaction` struct we use an `i32`, this means users
can construct a non-standard transaction if they do not first look up
what the value should be. We can help folk out here by abstracting over
the version number.
Since the version number only governs standardness elect to make the
inner `i32` public (ie., not an invariant). The aim of the type is to
make life easy not restrict what versions are used.
Add transaction::Version data type that simply provides two consts `ONE`
and `TWO`.
Add a `Default` impl on `Version` that returns `Version::TWO`.
In tests that used version 0, instead use `Version::default` because the
test obviously does not care.
Tobin C. Harding
Fmt Bot
Andrew Poelstra
harshit933
Martin Habovstiak
Matt Corallo
Steven Roose