Skip to content

Fast-Forward Merge with the upstream rust-bitcoin repository #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 872 commits into from
Mar 26, 2025

Conversation

Calisto-Mathias
Copy link
Collaborator

No description provided.

apoelstra and others added 30 commits February 24, 2025 16:37
cb270ea Make transaction::Version field private (jrakibi)
6c69b66 Use Version constant (jrakibi)

Pull request description:

  This commit addresses rust-bitcoin#4041 by making the `transaction::Version` field private.

  This forces people to either use the associated constants (`Version::ONE/TWO/THREE`) or the `non_standard`/`from_consensus` methods for any other transaction version.

  This aligns with our approach to `block::Version`

ACKs for top commit:
  tcharding:
    ACK cb270ea
  Kixunil:
    ACK cb270ea

Tree-SHA512: dcf5b50dfeda04e56fec350acd735dcb7099989b552afce4261d559a8a846c0eb3705369ad635ef9bbbfb2373d203a2c3641178925de6685426aa91245db9a8c
0596867 2025-02-23 automated rustfmt nightly (Fmt Bot)

Pull request description:

  Automated nightly `rustfmt` changes by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action

ACKs for top commit:
  apoelstra:
    ACK 0596867; successfully ran local tests

Tree-SHA512: 84a3b7c79939c1bcd0c3e30b117e0f681923a8c0825ee8852845abc0ec0677f3a6033c6133eba7742b9bb4e67d635614be5cd2d983d07c5866396142a8d8c3bc
The `taproot_control_block` did not properly detect whether it deals
with script spend or key spend. As a result, if key spend with annex was
used it'd return the first element (the signature) as if it was a
control block.

Further, the conditions identifying which kind of spend it was were
repeated multiple times but behaved subtly differently making only
`taproot_control_block` buggy but the other places confusing.

To resolve these issues this change adds a `P2TrSpend` enum that
represents a parsed witness and has a single method doing all the
parsing. The other methods can then be trivially implemented by matching
on that type. This way only one place needs to be verified and the
parsing code is more readable since it uses one big `match` to handle
all possibilities.

The downside of this is a potential perf impact if the parsing code
doesn't get inlined since the common parsing code has to shuffle around
data that the caller is not intersted in. I don't think this will be a
problem but if it will I suppose it will be solvable (e.g. by using
`#[inline(always)]`).

The enum also looks somewhat nice and perhaps downstream consumers could
make use of it. This change does not expose it yet but is written such
that after exposing it the API would be (mostly) idiomatic.

Closes rust-bitcoin#4097
The previous commit fixed a bug when `taproot_control_block` returned
`Some` on key-spends. This adds a test case for it which succeeds when
applied after the previous commit and fails if applied before it.
We already have `tapscript` method on `Witness` which is broken because
it doesn't check that the leaf script is a tapscript, however that
behavior might have been intended by some consumers who want to inspect
the script independent of the version. To resolve the confusion, we're
going to add a new method that returns both the leaf script and, to
avoid forgetting version check, also the leaf version.

This doesn't touch the `tapscript` method yet to make backporting of
this commit easier. It's also worth noting that leaf script is often
used together with version. To make passing them around easier it'd be
helpful to use a separate type. Thus this also adds a public POD type
containing the script and the version. In anticipation of if being
usable in different APIs it's also generic over the script type.
Similarly to the `tapscript` method, this also only adds the type and
doesn't change other functions to use it yet. Only the newly added
`taproot_leaf_script` method uses it now.

This is a part of rust-bitcoin#4073
5680b4e Refer to `Script{Buf}` as `Self` where relevant (Martin Habovstiak)
ce55dd5 Make `ScriptHash` & `WScriptHash` obey sanity rule (Martin Habovstiak)
9ec9adc Add a note about Electrum's script hashes (Martin Habovstiak)
82f553a Expose `ScriptBuf`'s `capacity` (Martin Habovstiak)
6b9d439 Remove stale FIXME comments (Martin Habovstiak)
0567e6f Put `#[inline]` on most `Script{Buf}` methods (Martin Habovstiak)
b7e2af1 Implement `Arbitrary` for `&'a Script` (Martin Habovstiak)
bca2864 Move `Deref{Mut}` from common module to `owned` (Martin Habovstiak)
3b15e90 Add `const` to some `Script` methods (Martin Habovstiak)
277223d Make `Script` and `ScriptBuf` obey sanity rules (Martin Habovstiak)

Pull request description:

  This implements various improvements related to `Script`. Please refer to the individual commits for details.

  This is a part of rust-bitcoin#4059

ACKs for top commit:
  tcharding:
    ACK 5680b4e
  apoelstra:
    ACK 5680b4e; successfully ran local tests

Tree-SHA512: 5daa8bf6c0b439a579d31d23944077e4a7fa89e14052003d2b81c745f225147f8f6f693d068e0567830027cefea7dda2516596da632bc817199352fa29af0a9b
Bitcoin block headers have a timestamp. Currently we are using a
`u32`. while this functions correctly it gives the compiler no chance
to enforce type safety.

Add a `Timestamp` newtype that is a thin wrapper around a `u32`.
Document it and test the API surface in `api.rs`.
We release `v0.32.4` and `v0.32.5` already but forgot to merge the
changelog entries back into master.

Grab the missing changelog entries from the `0.32.x` release branch.
…ompatibility with Bitcoin Core

7ab2f5b Add test for sighash_single_bug incompatility fix (Liu-Cheng Xu)
5d38073 Fix `is_invalid_use_of_sighash_single()` incompatibility with Bitcoin Core (Liu-Cheng Xu)

Pull request description:

  Close rust-bitcoin#4112

ACKs for top commit:
  tcharding:
    ACK 7ab2f5b
  Kixunil:
    ACK 7ab2f5b
  apoelstra:
    ACK 7ab2f5b; successfully ran local tests

Tree-SHA512: d47143d188653d3e845951e64e9b410fdbdac8e51906f33532b8d71519f0bd1454a46135dfdd6073a6d1ced9854dc3e13f3c35de60b7fdd45c22ef37f9a0fc75
The `NumOpResult` type is way more ergonomic to use if it derives
`Copy`. This restricts the `NumOpResult` to being `Copy` as well.

This does restrict what we can include in the error type in the future.

Derive Copy for `NumOpResult` and `NumOpResult`.
This macro can generally handle a lot of different cases where we
implement "the same trait but on references". We introduce it here and
use it in two places. We will use it in many more, but I wanted to make
the diff small on this commit, which introduces the actual macro code
and might take a bit of reading to understand.

You may want to use --color-moved-ws=allow-indentation-change to review
this, and the next commit.

The next set of changes will mechanically delete other macros that are
made redundant by this.
This is not too complicated a change to support and it will reduce the
noise in the following commits a fair bit.
The next commit changes a lot of code, but almost entirely by moving and
indenting it. We try to do the moves here ahead of time, so it the diff
for the next commit will be just deletions and indentations.
Looks like a large diff but if you run

    git show --color-moved-ws=allow-indentation-change

you will see that it's 100% moves (though moves of code into the
reference macro). May be easier to just look at src/amount/result.rs
after this; it's pretty short now.
This is a bit ugly and requires that we put our where-clauses in
parentheses because the macro_rules parser sucks, but it allows us to
move the blanket-impls on NumOpResult into the macro.

This commit moves one instance and updates the macro; the next commits
will change the rest.
Add a few missing impls to the `impl_op_for_references` macro.

Includes a minor whitespace change so that traits are grouped together.
Macroise the tests improving coverage along the way.

Includes a TODO to remind us to do `Neg` more fully.
…2025-02-21)

ca8bd53 Automated update to Github CI to rustc nightly-2025-02-21 (Update Nightly Rustc Bot)

Pull request description:

  Automated update to Github CI workflow `rust.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action

ACKs for top commit:
  tcharding:
    ACK ca8bd53

Tree-SHA512: 39f9ee068cdafde4d50cebfbf9fd3b96377d12e0c98a059e835f61bfbe57f77f4e84a629082dc38aa40aabb190e66dbd95ab4ba6423f816796534fe28f89e6f8
As part of the 1.0 effort and forward maintainability hide the internals
of the two error types in the `relative` locktime module. Doing so
allows us to remove the `non_exhaustive` attribute. Add getters to get
at the error innards.
As part of the 1.0 effort and forward maintainability hide the internals
of the two error types in the `script` module. Add getters to get at the
invalid size.
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.
erickcestari and others added 16 commits March 21, 2025 11:28
… signing

This commit enhances PSBT signing functionality by:

1. Added new KeyRequest::XOnlyPubkey variant to support direct retrieval using XOnly public keys
2. Implemented GetKey for HashMap<XOnlyPublicKey, PrivateKey> for more efficient Taproot key management
3. Modified HashMap<PublicKey, PrivateKey> implementation to handle XOnlyPublicKey requests by checking both even and odd parity variants

These changes allow for more flexible key management in Taproot transactions.
Specifically, wallet implementations can now store keys indexed by either
PublicKey or XOnlyPublicKey and successfully sign PSBTs with Taproot inputs.

Added tests for both implementations to verify correct behavior.

Added test for odd parity key retrieval.

Closes rust-bitcoin#4150
db9ec3b Remove From<newtype> for $hash (Tobin C. Harding)
6b2b89c Remove From<hash> for not-general-hash types (Tobin C. Harding)
200ff47 Use compute_merkle_root (Tobin C. Harding)

Pull request description:

  The `hash_newtype` macro is explicitly designed to produce a hash that is not a general purpose hash type to try and prevent users hashing arbitrary stuff with it. E.g., `Txid` isn't meant to be just hash arbitrary data. However we provide a `From` impl that will convert any instance of the inner hash type into the new type. This kind of defeats the purpose. We provide `from_byte_array` and `to_byte_array` to allow folk to 'cast' from one hash type to another if they really want to and its ugly on purpose.

  Also, it is becoming apparent that we may be able to remove the `hashes` crate from the public API of `primitives` allowing us to stabalise `primitives` without stabalising `hashes`.

  For both these reasons remove the `From` impl from the `hash_newtype` macro. Note that deprecating doesn't seem to work so we just delete it.

ACKs for top commit:
  Kixunil:
    ACK db9ec3b
  apoelstra:
    ACK db9ec3b; successfully ran local tests

Tree-SHA512: 90bc325821cd2d72bbaef5b3cfef2d299192d1e7999cd4f96b6b69b8872e419964e431e91674c59bfdd2e9a5959dbc13ee89d5f87d03e96785044c616db19d72
This commit standardizes the function signatures in the Amount and SignedAmount
implementations by consistently using Self as the return type instead of the concrete
type names. This makes the code more consistent, easier to maintain, and follows Rust's
idiomatic practices.

Changes:

Replace all occurrences of -> Amount with -> Self in unsigned.rs
Replace all occurrences of -> SignedAmount with -> Self in signed.rs
Make similar replacements for Option/Result return types
Use Self:: instead of the explicit type name for static method calls
…2025-03-21)

1c13627 Automated update to Github CI to rustc nightly-2025-03-21 (Update Nightly Rustc Bot)

Pull request description:

  Automated update to Github CI workflow `rust.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action

ACKs for top commit:
  tcharding:
    ACK 1c13627

Tree-SHA512: 291dd7e0db15a8a3d187dc3e5a63105c8f0530cdf8dc0c87cb34f94f33766a1c8717bf7be400895d20a631537382de6aa281e0b6df303af62dfa2715e9c3fa75
`Witness` was missing conversions from arrays (and variations) which was
annoying when creating known-sized witnesses. These come up when
spending statically-known inputs and in tests.
Since `Witness` is semantically equivalent to `&[&[u8]]` they should
also be comparable. However we only had the impl to compare `Witness`
with itself. Being able to compare `Witness` with other containers is
particularly needed in tests.
Accessing the internals of tested object is problematic because it makes
changes to layout harder, it makes tests harder to read and it checks
implementation details rather than semantics of the API (behvaior).

We had such tests in `primitives::witness` so this changes them to use
the newly added APIs instead. However, it still leaves
`from_parts__unstable` which needs to be dealt with separately.
The `Witness`-related tests were constructing `Witness` in
over-complicated way by serializing `Vec<Vec<u8>>` and then
deserializing `Witness` even though they were not supposed to test
serialization but Taproot accessor methods. This was difficult to
understand and maintain.

This change simplifies them to just construct the `Witness` from array
of `Vec<u8>`s using the recently-added constructors. Note that we
already have serialization tests written separately so we're not losing
meaningful coverage here.
d0bead6 docs: fix LICENCE link (jike)

Pull request description:

  ![image](https://github.com/user-attachments/assets/9c08a57e-e958-487f-ba9d-978bd6fde913)

ACKs for top commit:
  tcharding:
    ACK d0bead6
  Kixunil:
    ACK d0bead6

Tree-SHA512: 148ea7c35ca8872e314e12889cab325af6965f533abea062c49d03809d57de3b476a97d39e2ae4e0c8479da263b7959c9cf2409050e3888af33b308c022391b1
…ieval and improve Taproot signing

069d2fd Add XOnlyPublicKey support for PSBT key retrieval and improve Taproot signing (Erick Cestari)

Pull request description:

  The `bip32_sign_schnorr` function was previously only attempting to retrieve private keys using `KeyRequest::Bip32`, which limited the ability to sign Taproot inputs with key maps that don't support BIP32 derivation paths.

  ## Changes
  - Added new `KeyRequest::XOnlyPubkey` variant to support direct retrieval using XOnly public keys
  - Implemented `GetKey` for `HashMap<XOnlyPublicKey, PrivateKey>` for more efficient Taproot key management
  - Modified `HashMap<PublicKey, PrivateKey>` implementation to handle XOnlyPublicKey requests by checking both even and odd parity variants
  - Added comprehensive tests for both key map implementations

  These improvements enable wallet implementations to store keys indexed by either `PublicKey` or `XOnlyPublicKey` and successfully sign PSBTs.

  Closes rust-bitcoin#4150

ACKs for top commit:
  Kixunil:
    ACK 069d2fd
  apoelstra:
    ACK 069d2fd; successfully ran local tests

Tree-SHA512: 0ae07309b772f1a53e7da45073f7e2337cc332ab2335925d623d0e1ad1503aab77673bbbd64e5533ae7fc8d57f3577db0ae7ac3b05279de92d3b34ab8eeae90f
… macro Array implementations

e744347 Make usage of Self and type uniform across both modules (Erick Cestari)
dfb49f0 Rename impl_try_from_array to impl_from_array (Erick Cestari)

Pull request description:

  This PR makes two main changes:

  1. Standardizes the function signatures in the `Amount` and `SignedAmount` implementations by consistently using `Self` as the return type instead of the concrete type names. This improves code consistency, maintainability, and follows Rust's idiomatic practices.
  2. Renames `impl_try_from_array` to `impl_from_array` to better reflect its functionality.

  ### Changes
  **Consistent usage of Self instead of concrete types**

  - Replace all occurrences of `-> Amount` with `-> Self `in unsigned.rs
  - Replace all occurrences of `-> SignedAmount` with `-> Self` in signed.rs
  - Make similar replacements for Option/Result return types
  - Use `Self::` instead of explicit type name for static method calls

  **Function rename**

  Renamed `impl_try_from_array` to `impl_from_array` for better clarity

  ### Related Issues

  Closes rust-bitcoin#4210

  Closes rust-bitcoin#4241

ACKs for top commit:
  Kixunil:
    ACK e744347
  tcharding:
    ACK e744347
  apoelstra:
    ACK e744347; successfully ran local tests

Tree-SHA512: 3113f3ccf595b298afe6b23514f1de790284df7fcb55a13658aabe3ef4fcea0e401b65b0a2c67ac18da87a1bcd247bd1f1484856fe03470b98dfa2614958a3bb
3ae21d5 Use impl_add/sub_assign for block interval (Tobin C. Harding)
9d55922 Use impl_op_for_references for block height/interval (Tobin C. Harding)
f5e1791 Move Assign impls together (Tobin C. Harding)
cc66838 units: Remove unnecessary code comments (Tobin C. Harding)

Pull request description:

  Improve the ops impls in the `block` module using the already present macros.

ACKs for top commit:
  apoelstra:
    ACK 3ae21d5; successfully ran local tests; nice!

Tree-SHA512: 6565426a06bb47d337d21cf5c59acca43e69228dbec8319fc95373025d220d8ec6273c54f214f312c4229603c455d08e4c6a8c108663c6db5086df36266979de
84bee2f Simplify `Witness` construction in tests (Martin Habovstiak)
3551ec2 Don't access internalls of `Witness` in tests (Martin Habovstiak)
c807836 Impl `PartialEq` between `Witness` and containers (Martin Habovstiak)
587a66d Add a bunch of missing conversions for `Witness` (Martin Habovstiak)

Pull request description:

  This is supposed to go in front of rust-bitcoin#4250

  `Witness` lacked a bunch of APIs that were making it harder to use and test, so this also adds them in addition to cleaning up tests. (I only realized they are missing when I tried to clean up tests and got a bunch of errors.)

ACKs for top commit:
  tcharding:
    ACK 84bee2f
  apoelstra:
    ACK 84bee2f; successfully ran local tests

Tree-SHA512: 7973f2a56b070babba7b4c632f45858154ccd00f8e77956ad2d28cb66e1fd18ff60d92c031ba3b76d0958e4acd34adfca10607fa26ec569dfd52ba1c1e2c79eb
Copy link

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Check semver breaks / PR Semver - stable toolchain" job then expand "Run semver checker script" and scroll to the end of the section.

@mcelrath mcelrath merged commit e56d4a7 into braidpool:master Mar 26, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.