Since arrays better convey the intention than slices when parsing
fixed-sized bytes we're migrating to them. This deprecates the
`from_slice` method similarly to how we do it elsewhere.
Private keys have statically-known length of 32 bytes and we are
migrating types with known lenths to use `from_byte_array` methods. This
adds the method to `PrivateKey` as well and uses it to implement
`from_slice`.
During upgrade of `secp256k1` a number of function calls needed to be
rewritten to not use `from_slice` but `from_byte_array`. Unfortunately,
the conversions wasn't correct and caused panics on invalid inputs
rather than errors.
This fixes it simply by calling `try_into` on the entire slice and
converting the error.
cf12ba262a Move taproot back to bitcoin crate (Tobin C. Harding)
Pull request description:
I don't know what I was thinking when I move the taproot hash types to `primitives`. As correctly pointed out by Kix we agreed to only have blockdata in `primitives`.
Move the taproot hash types back to `bitcoin::taproot` and remove the extension traits.
ACKs for top commit:
Kixunil:
ACK cf12ba262a
apoelstra:
ACK cf12ba262a646a6341098ee3f4c178a52fc90211; successfully ran local tests
Tree-SHA512: 0c5eabf395c05a93603a46b277c6ea2cc547f3894eef182fceb80f309123d67fe457936a388bac0249ec24cae7521eaef3bf8bd8facca5282e4ce2ea6fafd5f7
I don't know what I was thinking when I move the taproot hash types to
`primitives`. As correctly pointed out by Kix we agreed to only have
blockdata in `primitives`.
Move the taproot hash types back to `bitcoin::taproot` and remove the
extension traits.
For private WIF keys corresponding to a compressed address,
the last byte of the key needs to be 0x01, but the API
doesn't enforce this when using PrivateKey::from_wif(). So,
invalid keys can be accepted.
Thus we check if the last byte is equivalent to 0x01
if the key's length is 34 (which indicates it's
compressed).
In #3847 we added an `InvalidCharError` into one of the variants of
`ParsePublicKeyError` but we forgot to update the trait
implementations.
Fix the `error::Error` and `Display` implementations for
`ParsePublicKeyError`. While we are at it match on `*self` as is
typical in this codebase.
With this applied #3835 is fully resolved.
Close: #3835
Rust macros, while at times useful, are a maintenance nightmare. And
we have been bitten by calling macros from other crates multiple times
in the past.
In a push to just use less macros remove the usage of the
`impl_from_infallible` macro in the bitcoin, units, and internals crates
and just write the code.
945fcd0920 fix ParsePublicKeyError using hex::InvalidCharError (Innocent Onyemaenu)
Pull request description:
Replaced the InvalidChar variant u8 with hex::InvalidCharError
Resolves#3835
changed InvalidChar variant of the ParsePublicKeyError from `u8` to `hex::InvalidCharError`
```
pub enum ParsePublicKeyError {
...
/// Hex decoding error.
InvalidChar(hex::InvalidCharError),
...
}
Also,
modified the test cases to accommodate the new variant
Why:
- hex::InvalidCharError includes both the invalid character and its position.
- This improves debugging and makes error messages more actionable.
ACKs for top commit:
apoelstra:
ACK 945fcd09209120ef8869a2e4165e866328cc9bd5; successfully ran local tests; I like it
clarkmoody:
utACK 945fcd0920
Tree-SHA512: c13446c099cb02b4f253f9cc559a860aff3288a2cc5eac96d3cf910bf63e78957741bbdff69b936b16b36e46b366841a5c94876d16cbc0c41aea2a70866a6e45
What:
- Replaced the InvalidChar variant u8 with hex::InvalidCharError
Why:
- hex::InvalidCharError includes both the invalid character and its position.
- This improves debugging and makes error messages more actionable.
There is a loose convention in Rust to not use `test_` prefix. The
reason being that `cargo test` outputs 'test <test name>' using the
prefix makes the output stutter.
This patch smells a bit like code-churn but having the prefix in some
places and not others is confusing to new contributors and is leading me
to explain this many times now. Lets just fix it.
Remove the prefix unless doing so breaks the code.
These lints are valuable, lets get at em.
Changes are API breaking but because the changes make functions consume
self for types that are `Copy` downstream should not notice the breaks.
For the `hashes` crate we would like to make `hex` an optional
dependency. In preparation for doing so do the following:
- Remove the trait bounds from `GeneralHash`
- Split the hex/string stuff out of `impl_bytelike_traits` into a
separate macro.
The `impl_bytelike_traits` macro is public and it is used in the
`hash_newtype` macro, also public.
Currently if a user calls the `hash_newtype` macro in a crate that
depends on `hashes` without the `serde` feature enabled and with no
`serde` dependency everything works. However if the user then adds a
dependency that happens to enable the `serde` feature in `hashes` their
build will blow up because `serde` code will start getting called from
the original crate's call to `hash_newtype`.
Pull the serde stuff out of `hash_newtype` and provide a macro to
implement it `impl_serde_for_newtype`.
1649b68589 Standardize wording to `constructs a new` (Jamil Lambert, PhD)
27f94d5540 Replace `creates` with `constructs` (Jamil Lambert, PhD)
Pull request description:
As discussed in issue #3575 there are various ways of saying a new object is created.
These have all be standardized to the agreed version.
Close#3575
ACKs for top commit:
apoelstra:
ACK 1649b68589834dfe9d5b63812da3e9f0e5930107; successfully ran local tests
tcharding:
ACK 1649b68589
Tree-SHA512: 0ed9b56819c95f1fc14da1e0fdbbe03c4af2d97a95ea6b56125f72913e8d832db5d2882d713ae139d00614e651f3834a4d72528bdf776231cceb6772bf2f9963
There is a range of different wordings used in the docs of constructor
type functions.
Change all to start with `Constructs a new` or `Constructs an empty`.
In functions that act like constructors there is a mixture of the usage
of `creates` and `constructs`.
Replace all occurrences of `creates` with `constructs` in the first line
of docs of constructor like functions.
The private_key_debug_is_obfuscated test is removed because it belongs
in the secp256k1 library, not in the bitcoin library. Keeping it here
is redundant and creates unnecessary maintenance.
As we have been doing in other places deprecate `to_bytes` in favour of
`to_vec`. Note this is only for functions that return a `Vec`, the `key`
module has `CompressedKey::to_bytes` still as it returns an array.
30bb93c676 Implement impl_to_hex_from_lower_hex macro for types that implement fmt::LowerHex (Shing Him Ng)
Pull request description:
Created a macro that implements `to_hex` for types that currently have `core::fmt::LowerHex` and called it on types that have `core::fmt::LowerHex` implemented. I put the macro in the `internals` crate since there are types across the whole project that can potentially use this.
Resolves#2869
ACKs for top commit:
Kixunil:
ACK 30bb93c676
apoelstra:
ACK 30bb93c676 successfully ran local tests
Tree-SHA512: d3ebc7b5c0c23f1a8f8eef4379c1b475e8c23845e18ce514cb1e98eb63fc4f215e6bc4425f97c7303053df13374ef931ae9d9373badd7ca1975a55b0d00d0e40
`s.parse` is more idiomatic and produces more helpful error messages.
This has been changed repo wide in the main codebase, not including
examples, rustdocs, and in the test module.
`use std::str::FromStr;` has been removed where this change makes
it unnecessary.
c72069e921 Bump MSRV to 1.63 (Martin Habovstiak)
Pull request description:
The version 1.63 satisfies our requirements for MSRV and provides significant benefits so this commit bumps it. This commit also starts using some advantages of the new MSRV, namely namespaced features, weak dependencies and the ability to use trait bounds in `const` context.
This however does not yet migrade the `rand-std` feature because that requires a release of `secp256k1` with the same kind of change - bumping MSRV to 1.63 and removing `rand-std` in favor of weak dependency. (Accompanying PR to secp256k1: https://github.com/rust-bitcoin/rust-secp256k1/pull/709 )
Suggested plan:
* merge both PRs
* at some point release `hashes` and `secp256k`
* remove `rand-std` from `bitcoin`
* release the rest of the crates
ACKs for top commit:
apoelstra:
ACK c72069e921
tcharding:
ACK c72069e921
Tree-SHA512: 0b301ef8145f01967318d3ed1c738d33e6cf9e44f835f3762122b460a536f926916dbd6ea39d6f80b4f95402cd845e924401e75427dbb0731ca5b12b4fa6915e
The version 1.63 satisfies our requirements for MSRV and provides
significant benefits so this commit bumps it. This commit also starts
using some advantages of the new MSRV, namely namespaced features, weak
dependencies and the ability to use trait bounds in `const` context.
This however does not yet migrade the `rand-std` feature because that
requires a release of `secp256k1` with the same kind of change - bumping
MSRV to 1.63 and removing `rand-std` in favor of weak dependency.
We would like to move the `Script` type to `primitives` without moving
any key stuff, including pubkey hashes. We may later, before releasing
`primitives`, move the `WPubkeyHash` at which time this patch can be
reverted or re-implemented on `ScriptBuf`.
The `ScriptBuf::p2wpkh_script_code` function does not take `self` as a
parameter but it does return `Self` - this can trivially be made into a
standalone function.
Make `ScriptBuf::p2wpkh_script_code` a standalone function.
d099b9c195 Remove wildcard from prelude import (Jamil Lambert, PhD)
Pull request description:
This patch replaces `prelude::*` wildcard imports with the types actually used. In a couple of cases `DisplayHex` was previously imported by the wildcard but was only used in the test module, an additional import was added to the test module instead of at the top where it causes an unused import warning.
Close: #2875
ACKs for top commit:
Kixunil:
ACK d099b9c195
tcharding:
ACK d099b9c195
Tree-SHA512: d59dfac0961d2649d509039a11c1b5574d81d05fef567a624cf15be2f587de796ea960ba5a08bef788199331c2f790fb06f7b393182538c7d8b1891ded119efc
dc10a49876 api: Run just check-api (Tobin C. Harding)
5e8f204581 Pass sigs and associated types by value (Tobin C. Harding)
Pull request description:
We should pass `Copy` types by value not by reference.
Currently this is not done in secp, but lets do it here in bitcoin.
Pass by value:
- `SerializedSignature`
- bitcoin sigs
- secp sigs
- secp `Message`
This is a continuation of the work to split up #2404 into manageable PRs.
ACKs for top commit:
apoelstra:
ACK dc10a49876
Kixunil:
ACK dc10a49876
Tree-SHA512: 8736eba067c74edb951c92357f5b3d0fc99c4fa6dc3beea579c10b3150873b74e8ec46c2c01f18818b37fca6e77c6b6edddeb6340edde6a9d8c28a4e69164c8c
Wildcards have been replaced with what is actually used.
In a couple of cases an additional use statement was added to the test
module to import `DisplayHex` which is only used in test, but
previously imported with the wildcard at the top.
In the next commits we are going to stop exposing the ability to hash
arbitrary data into wrapped hash types like Txid etc. In preparation for
this, stop using these methods internally.
This makes our internal code a little bit uglier and less DRY. An
alternative approach would be to implement the from_engine and engine
methods, but privately (and maybe having a macro to provide this). But I
think this approach is more straightforward.
The one exception is for the Taproot hashes, which are tagged hashes and
currently do not have their own engine type. I will address these in a
later PR because this one is already too big.
We should pass `Copy` types by value not by reference.
Currently this is not done in secp, but lets do it here in bitcoin.
Pass by value:
- `SerializedSignature`
- bitcoin sigs
- secp sigs
- secp `Message`
the `blockdata` directory is code organisation thing, all the
types/modules are re-exported from other places. In preparation for, and
to make easier, the `primitives` crate smashing work - remove all
explicit usage of `blockdata`.
Note that the few instances remain as they seem required e.g.,
`pub(in crate::blockdata::script)`
Refactor only, no logic changes.
9f01871c11 api: Run just check-api (Tobin C. Harding)
7929b51640 Pass keys by value (Tobin C. Harding)
Pull request description:
We should pass `Copy` types by value not by reference. Pass the key types by value.
This is patch 1 from #2404
ACKs for top commit:
apoelstra:
ACK 9f01871c11 this will annoy some people but I think we should do it
Tree-SHA512: 18afab537edf4ade4dc1c1e5992e50060b8935531f1e3cbe1d3b94b2fcb87aafa39947f342e0e762835bda3b4091dd35b3b74ea79f4dbb3b21660ffd21d1f82e
Martin Habovstiak
19年梦醒
merge-script
Tobin C. Harding
leopardracer
Fmt Bot
ndungudedan
Innocent Onyemaenu
Shing Him Ng
Jamil Lambert, PhD
Diogo Canut
Jiri Jakes