A new nightly version (`nightly-2024-08-28`) introduces a few warnings
because of our rustdocs. These are valid warnings and should be fixed,
thanks `clippy` team.
(The `bip152` change is a bit sloppy, open to suggestions.)
cf129ad314 fix: re-implement (de)serialization from/to readers/writers (elsirion)
Pull request description:
Fixes#3250.
The serialization is less than ideal and still allocates a lot. I can understand not wanting to (ab)use the consensus encoding traits, but they have a pretty good interface, copying it and creating some `EncodePsbt` and `DecodePsbt` traits with similar interfaces would have been nice imo.
ACKs for top commit:
Kixunil:
ACK cf129ad314
apoelstra:
ACK cf129ad314 successfully ran local tests; LGTM -- I believe this is non-breaking, as does cargo-semver-checks, so we can backport this to 0.32
Tree-SHA512: d7f218164d772db3a9fb4436953c3b5fd3677b92078d0843233197629df7d852d80615a3ff38c5b70771381ba1aeb30defdc98ee63653e570bb75dc553400cad
`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.
There are a couple of issues around the ECDSA signature decoding /
parsing code. We have duplicate code in `from_str` and `from_slice`
and both use the same error type even though it is impossible to get a
hex error in `from_slice`.
Create two errors:
- A `DecodeError` returned by `from_slice`
- A `ParseSignatureError` that has a decode variant and a hex variant
Call through to `from_slice` after parsing hex into a byte vector.
Removes an instance of `unreachable!`.
Fix: #1193
In preparation for moving the `ScritpBuf` type to `primitives` add a
public and private extension trait for the functions we want to leave
here in `bitcoin`.
Note, includes a change to the `difine_extension_trait` metavariable
used on `$gent` from `ident` to `path` to support the generic
`AsRef<PushBytes>`.
In preparation to move script types to `primitives` we replace impl
block with extension traits by replacing the temporary modules with
`define_extension_trait`.
We already explicitly do not support 16 bit machines.
Also, because Rust supports `u182`s one cannot infallibly convert from a
`usize` to a `u64`. This is unergonomic and results in a ton of casts.
We can instead limit our code to running only on machines where `usize`
is less that or equal to 64 bits then the infallible conversion is
possible.
Since 128 bit machines are not a thing yet this does not in reality
introduce any limitations on the library.
Add a "private" trait to the `internals` crate to do infallible
conversion to a `u64` from `usize`.
Implement it for all unsigned integers smaller than `u64` as well so
we have the option to use the trait instead of `u32::from(foo)`.
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.
2169b75bba Use lower case error messages (Jamil Lambert, PhD)
Pull request description:
Error messages should be lower case, except for proper nouns and variable names. These have all been changed.
~~They should also state what went wrong. Some expect error messages were positive, giving the correct behaviour or correct input. These have been changed so that they are now negative, i.e. saying what went wrong.~~
EDIT: After further discussion it was decided not to change the expect messages.
ACKs for top commit:
Kixunil:
ACK 2169b75bba
tcharding:
ACK 2169b75bba
Tree-SHA512: 92442c869e0141532425f6fca5195fd319b65026f68c4230a65ad70253565d98931b2b44ee202975c307280525c505147e272297dc81207312e40c43d007021c
64c31cfb97 Move locktimes and Sequence to primitives (Tobin C. Harding)
Pull request description:
The `absolute` and `relative` locktimes as well as the `Sequence` are all primitive bitcoin types.
Move the `Sequence`, and `locktime` stuff over to `primitives`.
There is nothing surprising here, the consensus encoding stuff stays in `bitcoin` and we re-export everything from `blockdata`.
Note please `Sequence` is no longer publicly available at `bitcoin::transaction::Sequence` but it is available at `bitcoin::Sequence`.
ACKs for top commit:
Kixunil:
ACK 64c31cfb97
apoelstra:
ACK 64c31cfb97
Tree-SHA512: 968aa595bfb53e8fcfa860dae797ec5381ed49c67326a8ef9494086ec65d737502dffe4b24143e159042d07c59c5109d50103029082f87e1c3c26671e975f1b3
The `absolute` and `relative` locktimes as well as the `Sequence` are
all primitive bitcoin types.
Move the `Sequence`, and `locktime` stuff over to `primitives`.
There is nothing surprising here, the consensus encoding stuff stays in
`bitcoin` and we re-export everything from `blockdata`.
164b72e07b Update Key documentation (Shing Him Ng)
Pull request description:
The documentation for `Key` was a bit confusing since `<key> := <keylen> <keytype> <keydata>` was right above the `key` field, when in reality the `key` field represents the key data. Moving the documentation that describes a `Key` as a whole to the struct itself and specifying that `key` represents key data will hopefully clear things up a bit
ACKs for top commit:
Kixunil:
ACK 164b72e07b
tcharding:
ACK 164b72e07b
Tree-SHA512: 33c2b24d3006a0ed85a5d96351e0a01c1365c8ea01472233b024de54941319cdbefa0126f8b9538385e8f33ba7e2e3895f0dc5b6a36a1c501c8b97ebbede6502
Done in preparation for moving the script types to `primitives`.
The script types have a bunch of functionality to support scriptPubkeys,
and scriptPubkeys are an address thing.
Create a module under `address` and in it create a bunch of extension
traits to hold all scriptPubkey functionality.
Includes adding an ugly-as-hell macro to create the traits.
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.
8ee1744b9b Make 'use core::fmt' calls consistent (Shing Him Ng)
Pull request description:
I started taking a look at #2869 and looked for everything that was implementing the `Display` trait:
```rust
impl fmt::Display for _
```
but found some places where the imports weren't consistent:
```rust
impl Display for _
```
There were only a few instances of the latter, so I went ahead and cleaned those up before starting #2869
I started pulling this thread when I saw the same thing was happening for `fmt::Debug` and `fmt::Formatter` so I updated the rest of the `use core::fmt::*` statements with a few exceptions:
- No updates to `use core::fmt::*` if it was being called from within a function since I felt like the function scope was small enough to not cause confusion
- No updates to `use core::fmt::{self, Write as _};`
ACKs for top commit:
Kixunil:
ACK 8ee1744b9b
tcharding:
ACK 8ee1744b9b
Tree-SHA512: 33eb6ea0c4e808ef78bc87de6547144b756bde206c50d80488f740e97cd8d11f1abcb8936c487d7bfd29be5e21c7f40ff88f82acdaaec9aacb4b6362ffc4c680
2bde5d002e api: Run just check-api (Jose Storopoli)
d1f84329e4 psbt: implement const for PsbtSighashType::ALL (Jose Storopoli)
Pull request description:
Closes#2751.
I only did the `ALL` which is by far the most common case.
ACKs for top commit:
tcharding:
ACK 2bde5d002e
apoelstra:
ACK 2bde5d002e
Tree-SHA512: 693575de758657a3e172d86ba5114ec0bf3b12b82df598e38c6a7916c99c20cfb5c4e74442108b51ae4e7bb1f1e940fd4a7269145e3f9838f727675c7711c890
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.
4652ce20ed API changes for "delete `all_zeros`" (Andrew Poelstra)
8869f35a69 hashes: drop the `all_zeros` method on arbitrary hashes (Andrew Poelstra)
9f8797f486 API changes for constification of hash constructors (Andrew Poelstra)
154e91af8c hashes: constify a bunch of constructors (Andrew Poelstra)
c155cbf8b2 hashes: use workaround to get constfns on tagged hashes with MSRV (Andrew Poelstra)
Pull request description:
I think these changes are both uncontroversial but they have fairly large diffs so I am PRing them together before making more invasive changes.
ACKs for top commit:
tcharding:
ACK 4652ce20ed
Tree-SHA512: 4560fa397deab50448598894b9364f9d8f8b48169901a84db6a44168cdba795ab69b48ad2cac61caebcee5e227a03271335b405cf5514265290a4d1f2fdf52a2
433fd6bf7e api: Run just check-api (Tobin C. Harding)
8fd583b069 Pass hash types by value (Tobin C. Harding)
Pull request description:
We should pass `Copy` types by value not by reference. Pass the hash types by value.
Second step in the pass-copy-types-by-value work, pulled out of #2404.
ACKs for top commit:
apoelstra:
ACK 433fd6bf7e
Kixunil:
ACK 433fd6bf7e
Tree-SHA512: 999d12f60550cacc4ae19b4cbf505b25c1eed803820f22b1a706e9f95da1b7e7b422f393f4115d579927c0c476cd504036a39b3cdc06a1d6befbcff5513f7433
39df0a9fbe update api (Divyansh Gupta)
3a5f2932a4 create constants for ChildNumber enum (Divyansh Gupta)
Pull request description:
this aims to fix#2750
ACKs for top commit:
tcharding:
ACK 39df0a9fbe
Tree-SHA512: e1c38568facd2b9aa55b1b1ec0d5d5f68ff38ca3fe68962bc316c060a062299935aa51bcfc1c255a7f5c9ad97435cab22e2c160d3fd3f52a46f6b5cbb7d5743f
Manually implement it for Wtxid, Txid and BlockHash, where the all-zero
"hash" has a consensus meaning. But in general we should not be
implementing this method unless we have a good reason to do so. It can
be emulated or implemeted in terms of from_byte_array.
The use of Wtxid::all_zeros is obscure and specific enough that I am
tempted to drop it. But for txid and blockhash, the 0 hash appears in
actual blockdata and we should keep it.
All other uses of all_zeros were either in test code or in places where
the specific hash was not important and [u8; 32] was a more appropriate
type.
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.
There are two limits that the Bitcoin network enforces in regard to
hashing scripts
- For P2SH the redeem script must be less than 520 bytes
- For P2WSH the witness script must be less than 10,000 bytes
Currently we are only enforcing the p2sh limit when creating an address
with `Address::p2sh`.
There are various ways to create addresses from script hashes and if
users manually hash a script then use the `ScriptHash` (or
`WScritpHash`) our APIs assume the script that was hashed is valid. This
means there is the potential for users to get burned by creating
addresses that cannot be spent, something we would like to avoid.
- Add fallible constructors to `ScriptHash` and `WScriptHash`
- Add `TryFrom` impls as well to both types
- Remove the `From` impls
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
Currently we have a trait `Hash` that is required for `Hmac`, `Hkdf`,
and other use cases. However, it is unegonomic for users who just want
to do a simple hash to have to import the trait.
Add inherent functions to all hash types including those created with
the new wrapper type macros.
This patch introduces some duplicate code but we are trying to make
progress in the hashes API re-write. We can come back and de-dublicate
later.
Includes making `to_byte_array`,`from_byte_array`, `as_byte_array`, and
`all_zeros` const where easily possible.
9e4b092fce psbt: Use macro instead of function (Tobin C. Harding)
Pull request description:
We have a private function that makes use of the `Hash` trait to generically hash map entries. This usage makes patching the `hashes` module difficult. We can achieve the same thing by using a macro and passing in the concrete type.
This is an internal change, no effect on logic or public API.
ACKs for top commit:
apoelstra:
ACK 9e4b092fce
Tree-SHA512: 8b788fa91d21bbae556c746c2e55e6e9395e022bedf13193555ef7482109b6ef5032b233c5f37543a31ebda49d9b4761c161ca0db501472047eb661a48e944b7
6ba7758b30 Improve array macros (Tobin C. Harding)
Pull request description:
Currently we have two macros used when creating array wrapper types, one is in `internals` and the other in `bitcoin::internal_macros`. It is not immediately obvious what is what and why there are two.
Improve the macros by:
- Move the inherent functions to `impl_array_newtype`
- Use `*_byte_array` for the names instead of `*_bytes`
- Re-name the other macro to match what it now does
ACKs for top commit:
apoelstra:
ACK 6ba7758b30
Tree-SHA512: 36ed0fae0d28f24d29287062eb05bbc1e9e8b565f4ff41fd893503a25404ed8e185a34d75e398a8a660923ffda3b832b6157011598d5a75a5c4aafdffc74af2a
We have a private function that makes use of the `Hash` trait to
generically hash map entries. This usage makes patching the `hashes`
module difficult. We can achieve the same thing by using a macro and
passing in the concrete type.
This is an internal change, no effect on logic or public API.
802af8e417 Removed //! spare line at end of headers (jamil.lambert)
Pull request description:
Some of the headers had a //! at the end but most didn't. They have all been removed in bitcoin/src/ to make the files consistent
ACKs for top commit:
apoelstra:
ACK 802af8e417
Tree-SHA512: a1eb0dda76af68cb96352f6b31231fa5391d49e11df924065e76871f82231ec0d5751190663f142240e5d757975937387243d1fdac3684d9bdbd7e2362dbd0a7
Currently we have two macros used when creating array wrapper types,
one is in `internals` and the other in `bitcoin::internal_macros`. It
is not immediately obvious what is what and why there are two.
Improve the macros by:
- Move the inherent functions to `impl_array_newtype`
- Use `*_byte_array` for the names instead of `*_bytes` for functions
that return arrays
- Add `as_bytes` to return a slice
- Add `to_bytes` to return a vector
- Re-name the other macro to match what it now does
d353be4546 bip32: derive_xpriv should not return a Result (Jose Storopoli)
Pull request description:
We discussed in #2752 that `derive_priv` never fails.
This PR addresses that issue.
ACKs for top commit:
apoelstra:
ACK d353be4546
tcharding:
ACK d353be4546
sanket1729:
ACK d353be4546
Tree-SHA512: 3a3d09027c6079581636b51a506fac2b325e592d182167c0a3d8676b41fb2bef59a85a404ad12b2e14d73e58fd4b6d8f5923a3dc76a4b9724033097fc08b36ac
Tobin C. Harding
merge-script
Jamil Lambert, PhD
elsirion
Martin Habovstiak
Fmt Bot
Shing Him Ng
Jose Storopoli
Divyansh Gupta