Recently in #2451 we disallowed bip32 derivation paths with the leading
'm' variable.
There is some confusion as to what exactly the bip specifies however
Bitcoin Core RPC call `getaddressinfo` returns a derivation path with a
leading "m/". This means we need to be able to parse it irrespective of
what the bip says.
Be more liberal in what we accept as a derivation path, including both
with and without the leading 'm/'.
Leave the full investigation of the bip to a later date.
Change back some of the test strings as makes sense and include test
strings to showcase the full current behaviour.
We are currently using the `base58::Error` type to create errors in
`bitcoin`, these are bitcoin errors not `base58` errors.
Note that we add what looks like duplicate
`InvalidBase58PayloadLengthError` types but they are different because
of the expected length. This could have been a field but I elected not
to do so for two reasons:
1. We will need to do so anyways if we crate smash more
2. The `crypto::key` one can have one of two values 33 or 34.
With this applied we can remove the now unused error variants from
`base58::Error`.
d3d5ee1047 Improve error handling in errors emmited by `keys` (harshit933)
Pull request description:
For now I have tried to group those functions which can produce more than one error and changed the functions which were generating single error from `Key::Error` to the respective error. Let me know if this needs to be changed.
Also in `psbt/error.rs` I have changed the `InvalidPublicKey(crate::crypto:🔑:Error)` to `InvalidPublicKey(crate::crypto:🔑:FromSliceError)`. What should be done here?
Changes -
- in `from_slice` changed the `error` to `FromSliceError`.
- in `verify` changed to `secp256k1::Error` as it can return only one error.
- in `from_str` changed to `FromSliceError`.
- in `CompressedPublicKey` changed `verify` from `Error` to `secp236k1::Error` as it only returns one error.
- introduces CompressedPublicKeyError
- Removes impl from `bip32.rs`
Potential fix#2291
ACKs for top commit:
Kixunil:
ACK d3d5ee1047
tcharding:
ACK d3d5ee1047
Tree-SHA512: 21681bbf87c37eb0caaefe4b356a8a5e1d9b17de3207a0c9294de66b367ab348a7dda1916eb866fe4382e852af14ccab7b9f25a279291cd5beb56bb60b2523c2
In BIP0032, m is used as a variable for the root extended key. It is not
meant to be used as a constant prefix when serializing paths.
Update the DerivationPath parser to no longer require the m prefix.
Remove the m prefix from the unit tests and the bip32, ecdsa-psbt,
and taproot-psbt examples.
close#2449
Changes -
- in `from_slice` changed the `error` to `FromSliceError`.
- in `verify` changed to `secp256k1::Error` as it can return only one error.
- in `from_str` changed to `FromSliceError`.
- in `CompressedPublicKey` changed `verify` from `Error` to `secp236k1::Error` as it only returns one error.
- introduces CompressedPublicKeyError
- Removes impl from `bip32.rs`
- introduces `ParsePubKeyError` to return errors while generating publickey from string
BIP-32 only differentiates between mainnet and some testnet when
encoding and decoding xpubs and xprivs. As such we can use the new
`NetworkKind` type instead of `Network` throughout the `bip32` module.
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.
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`.
In the 0.31.0 release we renamed the bip32 extended key types without
leaving the originals in there marked as deprecated. This makes for a
bad experience for devs, add them back in.
We have a convention in `rust-bitcoin` to use external crates directly
when importing them not via `crate::foo`.
Update all the import paths for `io` to use this form.
Not sure what happened here but our release job didn't catch this? We
should have updated this to "since = 0.31.0" before release. Since we
only deprecate for one release lets go ahead and remove this.
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`.
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.
As we have recently been doing, move the declaration of the hash type to
where it is used.
Move the `XpubIdentifier` hash declaration to the `bip32` module.
This is an API breaking change.
f18f684ad2 Add version bytes consts (Tobin C. Harding)
Pull request description:
BIP-32 defines 4 4-byte consts used as version bytes; currently we are hardcoding the version bytes in multiple places.
Add BIP-32 version bytes consts and use them throughout the module.
ACKs for top commit:
apoelstra:
ACK f18f684ad2
RCasatta:
utACK f18f684ad2
Tree-SHA512: 50bf2d26f0f8e3528642ffcc621c03b82f536994deb808a6c84225676b4b8849db8e0d16e46f3819e0810296a422b31cf90d0595739910afdb92fb768ef7696e
BIP-32 defines 4 4-byte consts used as version bytes; currently we are
hardcoding the version bytes in multiple places.
Add BIP-32 version bytes consts and use them throughout the module.
The BIP-32 extended public key and extended private key exist in the
Bitcoin vernacular as xpub and xpriv. We can use these terms with no
loss of clarity.
Rename our current BIP-32 types
- `ExtendedPubKey` to `Xpub`
- `ExtendedPrivKey` to `Xpriv`
This patch is a mechanical search-and-replace, followed by running the
formatter, no other manual changes.
The `network` module deals with data types and logic related to
internetworking bitcoind nodes, this is commonly referred to as the p2p
layer.
Rename the `network` module to `p2p` and fix all the paths.
As part of an ongoing effort to make our error types stable and useful
add a stand set of derives to all error types in the library.
`#[derive(Debug, Clone, PartialEq, Eq)]`
Add `Copy` if possible and the error type does not include
`#[non_exhaustive]`.
If an error type includes `io::Error` it only gets `#[derive(Debug)]`.
We have just released the `hex-conservative` crate, we can now use it.
Do the following:
- Depend on `hex-conservative` in `bitcoin` and `hashes`
- Re-export `hex-conservative` as `hex` from both crate roots.
- Remove all the old hex code from `hashes`
- Fix all the import statements (makes up the bulk of the lines changed
in this patch)
Currently we have a mishmash of attribution lines accompanying the SPDX
identifier. These lines are basically meaningless because:
- The date is often wrong
- The original author attributed is not the only contributor to a file
- The term "rust bitcoin developers" is basically just noise
Just remove all the attribution lines and be done with it. While we are
at it add an SPDX line to the few files missing it, whether this license
nonsense is even needed is left as an argument for another day.
fabcde036f Use package in manifest and shorten import (Tobin C. Harding)
Pull request description:
We can use `package` to rename `bitcoin_hashes` to `hashes` and `bitcoin_internals` to `internals`. This makes imports more terse with no loss of meaning.
ACKs for top commit:
apoelstra:
ACK fabcde036f
Kixunil:
ACK fabcde036f
Tree-SHA512: bc5bff6f7f6bf3b68ba1e0644a83da014081d8c6c9d578c21cb54fdd56a018f68733dd1135d05b590ba193ed9efd12fa9019182c1fed347e604d8548f6ef9103
If we use `#![cfg_attr(docsrs, feature(doc_auto_cfg))]` instead of
`#![cfg_attr(docsrs, feature(doc_cfg))]` we no longer need to manually
mark types with `#[cfg_attr(docsrs, doc(cfg(feature = "std")))]`.
Sweeeeeet.
We can use `package` to rename `bitcoin_hashes` to `hashes` and
`bitcoin_internals` to `internals`. This makes imports more terse with
no loss of meaning.
Various formatting issues have crept into the codebase because we do not
run the formatter in CI.
In preparation for enabling formatting checks in CI run `cargo +nightly
fmt` to fix current formatting issues. No changes other than those
create by the formatter.
Recently we used an error type that holds only one expected hex string
length when parsing but for `PublicKey`s we have two (66 and 130). Add a
new error variant to express the error. Requires adding a variant to
`bip32` for the same thing.
Fix: #1281
This makes the code less noisy and is a preparation for changing it to
`const`-based literal. Because of the preparation, places that used
variables to store the hex string were changed to constants.
There are still some instances of `Vec::from_hex` left - where they
won't be changeable to `const` and where `hex!` is unavailable
(integration tests). These may be dealt with later.
See also #1189
This fixes several API bugs:
* Use `TryFrom` instead of `From` for fallible conversions
* Move byte conversion methods from `impl_array_newtype` to
`impl_bytes_newtype`
* Add missing trait impls like `AsRef`, `Borrow`, their mutable versions
and infallible conversions from arrays
Closes#1336
In preparation for removing the `util` module move the `base58` module
to the crate root. This is likely not the final resting place for this
module but it is a step in the right direction.
Includes addition of rustfmt attribute to skip formatting the digits
array. No other changes to the `base58` module.
Done as part of flattening util.
Currently in `util` module we have a bunch of modules that provide
cryptography related functionality.
Create a `crypto` module and move into it the following:
- ecdsa
- schnorr
- key
To improve uniformity and ergonomics, do the following re-names while we
are at it:
- EcdsaSig -> ecdsa::Signature
- SchnorrSig -> schnorr::Signature
- EcdsaSigError -> ecdsa::Error
- SchnorrSigError -> schnorr::Error
- InvalidSchnorrSigSize -> InvalidSignatureSize (this is an error enum variant)
4e9ff972ad Improve checksum documentation (Tobin C. Harding)
0f01cb9f51 Use rustdoc summary (Tobin C. Harding)
6151d4c841 base58: Rename public functions (Tobin C. Harding)
a94af5c052 base58: Re-order code (Tobin C. Harding)
d362e6286a base58: Improve rustdocs (Tobin C. Harding)
a43234e7ab base58: Make SmallVec methods private (Tobin C. Harding)
27f2cba623 base58: Use alternate form to print hex (Tobin C. Harding)
f659a7aca3 base58: Remove key related errors (Tobin C. Harding)
Pull request description:
Do some clean up work to the `base58` module in preparation for splitting it out into its own crate.
- Patches 1-6: Basic clean up.
- Patch 7: Re-names the public API functions.
- Patch 8: Fixes rustdoc comment as suggested during review.
- Patch 9: Improves documentation on checksum, also as suggested during review.
ACKs for top commit:
apoelstra:
ACK 4e9ff972ad
sanket1729:
ACK 4e9ff972ad. Left some naming nits.
Tree-SHA512: 0fb1e5a964bd197fcb3ef5e9ecd6f8c6b35439af46528e8dbe654d9d10f7c8ed3ca1461593caf6efd0be1cd3a1c24fed1a176931114846a394b396bed6a2411d
Recently we added a bunch of deprecated re-exports while moving things
out of the util module. Turns out while the code reads like it works,
`deprecated` actually only works for functions, not types or modules
etc.
Remove the non-functional deprecated lines and elect to _not_ re-export
things we moved. Release 0.30 is going to break a lot of code but there
is no real nice way to resolve that. We will need good release notes and
a public apology probably :)
Fix import statements that still rely on `util::bip32` - these should
have been fixed when we moved the `bip32` module.
Tobin C. Harding
Fmt Bot
Liam Aharon
geekvest
Andrew Poelstra
josibake
harshit933
Martin Habovstiak
Marko Bencun
startup-dreamer
Lorenzo Maturano