Bitcoin 0.33 migration #924
Closed
42Pupusas wants to merge 23 commits into
Closed
Conversation
Start of the 0.33 port. Prior attempt in PR rust-bitcoin#874 is preserved locally as branch pr-874.
Migrates the miniscript library (src/) to bitcoin 0.33.0-beta. The prior commit only bumped Cargo.toml; this commit ports every call site. Highlights: - Introduce crate-level aliases ScriptBuf, Script, ScriptBuilder bound to the new tagged ScriptPubKey variants, keeping the miniscript surface stable across the bitcoin 0.32/0.33 transition. - Hash256 newtype now also invokes impl_hex_for_newtype! and defines its own hash()/all_zeros() since bitcoin_hashes 0.20 no longer derives Display/Debug/FromStr from hash_newtype!. - Add MiniscriptKey and ToPublicKey impls for bitcoin::XOnlyPublicKey (distinct from secp256k1::XOnlyPublicKey in 0.33, now wraps + parity). - Script tagging: route scriptPubKey/witness/redeem/tap scripts through their tagged counterparts (ScriptPubKeyBuf, WitnessScriptBuf, RedeemScriptBuf, TapScriptBuf) and the corresponding *Ext traits for is_p2wsh/to_p2sh/etc. - Remove unused secp arguments from InnerXKey::xkey_fingerprint, SinglePriv::to_public, DescriptorXKey::to_public, DescriptorMultiXKey::to_public, DescriptorXKey::matches, DefiniteDescriptorKey::derive_public_key, Descriptor::derived_descriptor, Descriptor::find_derivation_index_for_spk — tracking the upstream bitcoin 0.33 change of dropping secp from bip32/key APIs. - Rewrite psbt GetKey impls for the new `fn get_key(&self, &KeyRequest)` trait signature and drop the KeySourceLookUp secp field. - Fix PublicKey field privacy (to_inner/compressed()), Transaction.inputs/ outputs, TxOut.amount, bip32 error renames, ecdsa::DecodeError, and misc moves (read_scriptint_non_minimal, MAX_SCRIPT_ELEMENT_SIZE in miniscript limits, Witness iter().rev().nth(1) for second_to_last). - TaprootMerkleBranch is now unsized; switch to TaprootMerkleBranchBuf. Library builds cleanly with no errors and no warnings under the default feature set. Examples, tests, and fuzz targets still need migration in follow-up commits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Apply the project's nightly-only rustfmt config to the files touched in the bitcoin 0.33 migration. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ports the test modules inside src/descriptor/ and the shared src/test_utils.rs to the bitcoin 0.33.0-beta API surface that landed in the preceding library migration commit. Notable changes: - Drop `&secp` from call sites of the APIs whose secp argument was removed in the library: `PrivateKey::public_key`, `Xpriv::fingerprint`, `Xpriv::derive_priv`, `Xpub::derive_pub`, `GetKey::get_key`, `DescriptorXKey::matches`, `Descriptor::derived_descriptor`, and `Descriptor::find_derivation_index_for_spk`. - Route `ms.encode()` through `WitnessScriptBuf::from(...)` so the new `to_p2wsh` method on the witness-script tagged type resolves. - Replace `bitcoin::ScriptBuf::new()` with the appropriate tagged constructor (`ScriptSigBuf::new()` for scriptSig fields, `ScriptBuf` alias for scriptPubKey). - `bitcoin::OutPoint::default()` → `COINBASE_PREVOUT`. - `PublicKey` field literal / `.inner` accesses rewritten as `PublicKey::from_secp(...)` and `.to_inner()` / `.as_inner()`. - Tap spend-info tests: wrap `to_x_only_pubkey()` (secp type) in `UntweakedPublicKey::from(...)` so `.tap_tweak()` is in scope. - `SecretKey::from_slice(&sk[..])` → `from_secret_bytes(sk)` in test_utils.rs, and drop the secp argument from `Keypair::from_secret_key`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ports every file under `examples/` to the bitcoin 0.33.0-beta API
(transaction/txout field renames, tagged scripts, PublicKey accessor
methods, secp-less key/descriptor APIs, etc).
- `Transaction.input/.output` → `.inputs/.outputs`; `TxOut.value` →
`.amount` (wrapping the new `Amount::from_sat` Result);
`OutPoint::default()` → `OutPoint::COINBASE_PREVOUT`;
`TxIn { ..Default::default() }` → explicit field form.
- `bitcoin::ScriptBuf::new()` → `bitcoin::ScriptSigBuf::new()` for
scriptSig fields, `miniscript::ScriptBuf::new()` for scriptPubKey.
- `pk.inner` / `pk.public_key(&secp)` → `pk.to_inner()` /
`pk.public_key()`; construct via `PublicKey::from_secp(...)`.
- `Descriptor::derived_descriptor` no longer takes `&secp` — drop at
call sites in `xpub_descriptors.rs`.
- `Keypair::new(&secp, rng)` → `bitcoin::Keypair::from_secp(
secp256k1::Keypair::new(&mut rand::rng()))`;
`XOnlyPublicKey::from_slice` → `from_byte_array` + `.into()`;
`XOnlyPublicKey::from_keypair` now returns the key directly.
- `bitcoin::hex::{Case, DisplayHex}` → `bitcoin::hashes::hex::prelude::
DisplayHex` and `to_lower_hex_string()`.
- `LockTime::from_time` (deprecated) → `from_mtp` in
`taptree_of_horror`.
- `message` passed owned (not `&message`) into `Secp256k1::verify_ecdsa`
since `&Message` no longer auto-converts.
All example binaries now compile with `cargo check --examples`
(deprecation warnings for `sign_ecdsa`/`verify_ecdsa` remain — those
are API changes to follow up on separately).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ports the test modules across `src/interpreter/`, `src/miniscript/`,
`src/psbt/`, `src/plan.rs`, and `src/policy/`, plus a final tweak to
`src/test_utils.rs` so that `StrXOnlyKeyTranslator` emits
`secp256k1::XOnlyPublicKey` (matching `Tap::Key`) and therefore
`Miniscript::<XOnlyPublicKey, Tap>::decode_consensus` resolves.
- Drop `&secp` from test call sites of APIs whose secp argument was
removed in the library migration (`Xpriv::fingerprint/derive_priv`,
`PrivateKey::public_key`, `Keypair::from_secret_key`, etc).
- Replace `PubkeyHash`/`WPubkeyHash`/`ScriptHash`/`WScriptHash`
constructions that used a hash160/sha256 `.into()` with explicit
`::from_byte_array(..to_byte_array())` so the stricter 0.33
`From<...>` bounds are satisfied.
- `witness.iter().rev().nth(1)` → `witness.get_back(1)` (the new
direct accessor for "second-to-last" stack element).
- Wrap `ms.encode()` as `TapScriptBuf::from_bytes(...)` when calling
`TapLeafHash::from_script` / comparing `tap_scripts` entries.
- `Amount::from_sat(n)` now returns `Result`; add `.expect("in range")`
on test-only call sites. `TxIn::default()` → `TxIn::EMPTY_COINBASE`.
`TransactionVersion::non_standard` → `maybe_non_standard`.
- `pk.inner` → `pk.to_inner()` / `pk.as_inner()`; `PublicKey` struct
literal replaced with `PublicKey::from_secp(...)` in
`miniscript/iter.rs` tests.
- `pk.pubkey_hash().to_raw_hash()` → explicit
`hash160::Hash::from_byte_array(pk.pubkey_hash().to_byte_array())`.
- `SecretKey::from_slice(&[u8])` → `from_secret_bytes([u8; 32])`.
- `secp.sign_schnorr_with_aux_rand(&sighash, ...)` →
`sign_schnorr_with_aux_rand(sighash.as_ref(), ...)` (raw byte input)
against the `bitcoin::Keypair::to_inner()` secp keypair.
All library, test, and example targets now compile cleanly with
`cargo check --lib`, `cargo check --tests`, and
`cargo check --examples`. Deprecation warnings for `sign_ecdsa`,
`verify_ecdsa`, `Message::from_digest_slice`, and `LockTime::from_time`
remain in test code.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The fuzz crate pulls both the current `miniscript` (bitcoin 0.33) and `old_miniscript` 12.3 (bitcoin 0.32) to run regression comparisons. Since those are distinct versions of `bitcoin`, every `PublicKey`, `XOnlyPublicKey`, `hash256::Hash`, etc. from the two sides are *different types even though they look the same* — so the `FuzzPk` impls have to be written twice, against each crate's types. Changes: - `fuzz/src/lib.rs`: split the two `ToPublicKey` impls so each uses its own crate's `PublicKey`, `secp256k1`, and hash types. The new-side `PublicKey` is constructed with `::from_secp(...)` / `::from_secp_ uncompressed(...)` (fields are private); `hash256::Hash` now uses `hash256::Hash::hash(...)` since `from_byte_array` is only exposed via the `Hash` trait. - `fuzz_targets/roundtrip_miniscript_script_tap.rs`: switch to `miniscript::Script::from_bytes` (tagged) and spell the key type as `bitcoin::secp256k1::XOnlyPublicKey` (Tap::Key is still the secp variant even though `bitcoin::XOnlyPublicKey` now wraps a parity). - `fuzz_targets/miniscript_satisfy.rs`: `pk.inner` is private; use `pk.to_inner()`, and wrap the resulting secp xonly with `bitcoin::XOnlyPublicKey::from(...)` for the trait return type. Replace deprecated `schnorr::Signature::from_slice` with `from_byte_array`. - `fuzz_targets/regression_taptree.rs`: the old/new spend_info values aren't directly comparable — compare via serialized bytes: `internal_key().serialize()` (old: `[u8;32]`; new: `([u8;32], Parity)` so index into `.0`), `output_key().serialize()` (both `[u8;32]`), `merkle_root().map(to_byte_array)`, `control_block().serialize()` into a `HashMap<Vec<u8>, HashSet<Vec<u8>>>`. Bridge `LeafVersion` between the two crates via `from_consensus(to_consensus(...))`. `cargo check --manifest-path fuzz/Cargo.toml` passes with no errors or warnings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fixes the test expectations that broke after the bitcoin 0.33 migration
and migrates remaining deprecated 0.32-era calls:
- Opcode display names changed: `OP_PUSHNUM_N` → `OP_N`. Update every
expected-string in `src/miniscript/mod.rs` test helpers.
- `bitcoin::PublicKey`'s `Debug` impl no longer wraps the inner key as
`PublicKey(<hex>)` — it now prints the hex bytes directly. Update the
six expected debug strings in `pk_alias`.
- `miniscript::tests::deserialize` relied on `read_scriptint` rejecting
non-minimal integers; the migration swapped to
`read_scriptint_non_minimal`, which by name does not. Switch the lexer
to the `PushBytes::read_scriptint` method which performs the minimal
check.
- Interpreter's `sat_constraints` test: the interpreter reconstructs
`XOnlyPublicKey` from a 32-byte push with default (`Even`) parity, so
normalise the expected `x_only_pks` produced by `from_keypair` to
`Even` parity before comparison.
- Drop `LockTime::from_time` (deprecated) in favour of `from_mtp` in
`plan.rs` and `policy/semantic.rs`.
- Drop `Script::to_bytes` (deprecated) in favour of `to_vec` in
`interpreter/inner.rs`.
- Migrate `secp.sign_ecdsa` / `verify_ecdsa` /
`sign_schnorr_with_aux_rand` / `verify_schnorr` to the new
`secp256k1::ecdsa::{sign, verify}` / `secp256k1::schnorr::{
sign_with_aux_rand, verify}` free functions (interpreter tests,
`plan.rs`, `descriptor/mod.rs`, `policy/compiler.rs`).
`bitcoin::ecdsa::Signature::verify(msg, pk)` is used for the inherent
method style in the `verify_tx` example.
- `Message::from_digest_slice` → `Message::from_digest(arr32)` in
`plan.rs`.
- Clean up residual unused variables (`secp_ctx` in
`descriptor/mod.rs`, `secp` in `plan.rs`, example secp params) and
unused `Hash` imports in iter/mod tests.
- Fix the two doctests that still passed `&secp` to
`DescriptorXKey::matches` and `Descriptor::derived_descriptor(index)`.
- Drop secp argument from three example helpers in
`examples/xpub_descriptors.rs` now that the underlying API no longer
needs it.
Final state: `cargo check --all-targets` is clean (0 errors,
0 warnings); `cargo test --all-targets` passes all 151 lib tests and
all 9 doctests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduces MasterFingerprintHexError, a small wrapper that captures the displayed form of the upstream `hex-conservative` parse error and implements `std::error::Error`, so the `source()` chain for DescriptorKeyParseError::MasterFingerprint points at the underlying hex-parse error again. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bitcoin 0.33 exposes `PrivateKey::public_key`, `Xpriv::derive_priv`, `Xpub::from_priv`, and `Fingerprint` without requiring a `Secp256k1` context (the crate uses a global verification-only context internally), so the public API can shed the unused parameter. - DescriptorSecretKey::to_public, DescriptorXKey::to_public, and DescriptorMultiXKey::to_public no longer take a Secp256k1 reference. - KeyMap::insert drops its secp parameter and generic. - Descriptor::parse_descriptor drops secp, simplifying both the user entry point and the internal `KeyMapWrapper` translator. - Propagate the signature change through every test, example, and fuzz target that previously constructed a context just to hand to one of these calls. This matches the direction taken by upstream PR rust-bitcoin#874 and removes the last trace of the pre-0.33 "pass a context everywhere" pattern from the public API. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`Miniscript::encode` and `Terminal::encode` now take a type parameter
`T` so callers can produce a tagged `script::ScriptBuf<T>` of whatever
role fits the call-site (ScriptPubKey, RedeemScript, WitnessScript,
TapScript) instead of always producing a `ScriptPubKeyBuf` and then
copying the bytes into another tagged newtype.
Follow-on changes:
- `PushAstElem` is generic over `T`, as is the `impl MsKeyBuilder for
Builder<T>` in `util.rs`, so script construction works for every tag.
- Segwit (`Wsh`, `Wpkh`) surfaces `WitnessScriptBuf` directly from
`inner_script` / `ecdsa_sighash_script_code`; P2SH surfaces
`RedeemScriptBuf`; every `get_satisfaction` return type now
distinguishes `ScriptSigBuf` from `ScriptPubKeyBuf`.
- `Plan::satisfy`, `Descriptor::{unsigned_script_sig, get_satisfaction,
get_satisfaction_mall}`, and the PSBT finalizer propagate
`ScriptSigBuf` through their signatures. The finalizer's
interpreter-check helper takes a borrowed `ScriptSig` and internally
converts to the untagged `Script` view that the interpreter expects.
- `witness_to_scriptsig` now actually returns a `ScriptSigBuf`.
- Tests update their expectations to the new tagged types and annotate
`encode::<T>()` where inference cannot pick a tag on its own.
The one semantic subtlety: `Sh::inner_script` for `Sh(Wsh)` still
returns the inner witness script (not the P2WSH scriptPubKey), matching
the pre-0.33 behavior; `address_fallible` hashes the correct nested
segwit scriptPubKey for `Sh(Wsh)` / `Sh(Wpkh)` so p2sh addresses match
the previous implementation.
`cargo test --all-targets` passes 151 lib tests and 9 doctests; the
fuzz crate and every example still builds.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switches the Tap `ScriptContext::Key` from `bitcoin::secp256k1::XOnlyPublicKey` (raw secp type) to the tagged `bitcoin::XOnlyPublicKey` introduced in bitcoin 0.33. This lines up with upstream PR rust-bitcoin#874 and lets the taproot miniscript layer benefit from the same type-safety distinction the rest of bitcoin 0.33 uses. - `context::Tap::Key = bitcoin::XOnlyPublicKey`. - Seal and implement `ParseableKey` for `bitcoin::XOnlyPublicKey` (it delegates to the secp impl and wraps the result via `from_secp`). The secp variant stays sealed and implementable for callers who want to parse into the raw type. - PSBT finalizer's tap-script hash lookup map is now keyed on `bitcoin::XOnlyPublicKey`; no conversion needed because `tap_key_origins` already yields the tagged type. - `Tapscript` alias in the tests and the decode doctest import the tagged type. - `StrXOnlyKeyTranslator` in `test_utils` produces `bitcoin::XOnlyPublicKey` (via `from_secp`) so Tap miniscripts constructed through it drive through the new context. - Drop the now-unreachable `ToNoChecks` impl for the raw secp type; the existing impl for `bitcoin::key::XOnlyPublicKey` (the same tagged type) already covers the new context. - Fuzz target for tap roundtripping parses into the tagged type. The raw secp type is still the canonical key in non-Tap contexts and still implements `MiniscriptKey`/`ToPublicKey` unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `miniscript::ScriptBuf`, `miniscript::Script` and `miniscript::ScriptBuilder` type aliases in `src/lib.rs` flattened bitcoin 0.33's role-tagged script types back to a single `ScriptPubKeyTag`-specialized alias, hiding the distinction between scriptPubKey / scriptSig / redeemScript / witnessScript / tapScript from every downstream caller. That was always a compromise to keep internal migration mechanical. This commit removes the re-exports and migrates every call site to the tagged types it actually needs: - `scriptPubKey` values (`script_pubkey`, `explicit_script`, `script_code`, the script-pubkey comparisons in the interpreter) become `bitcoin::script::ScriptPubKey` / `ScriptPubKeyBuf`. - `scriptSig` values (every `unsigned_script_sig`, every empty `ScriptBuf::new()` that represented a scriptSig, `witness_to_scriptsig`) become `ScriptSig` / `ScriptSigBuf`. - `witnessScript` returns from `Wsh::inner_script` / `ecdsa_sighash_script_code` stay as `WitnessScriptBuf`. - `redeemScript` returns from `Sh::inner_script` / `ecdsa_sighash_script_code` / `unsigned_script_sig` stay as `RedeemScriptBuf` / `ScriptSigBuf`. - `TapScriptBuf` is threaded through `Placeholder::TapScript`, the tap satisfier helpers, `TrSpendInfo`, the fuzz tap roundtripper, and the PSBT-finalizer tap witness construction. Side effects of dropping the aliases: - `push_astelem` no longer picks the scriptPubKey-tagged builder; every builder construction now spells out `Builder::<TagHere>::new()` (or `ScriptSigTag` / `ScriptPubKeyTag`) so the tag is visible at the construction site. - The PSBT-finalizer module keeps a pair of local aliases (`Script = ScriptPubKey`, `ScriptBuf = ScriptPubKeyBuf`) because a large block of code there is tightly coupled to the "compare against txout scriptPubKey" notion; the aliases are now scoped to that file instead of crate-wide. - The interpreter's lex/decode path uses a file-local alias `ScriptPubKey as Script` so `Miniscript::decode(&Script)` keeps its previous spelling while being explicit about the tag at the use site. Breaking change for downstream: anything that spelled `miniscript::ScriptBuf`, `miniscript::Script`, or `miniscript::ScriptBuilder` must now spell the bitcoin 0.33 tagged type it actually wants. `cargo test --all-targets` passes 151 lib tests and 9 doctests; examples and the fuzz crate both build; `cargo clippy --all-targets -- -D warnings` is clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`verify_sig`, `iter`, `interpreter_check`, all `PsbtExt::finalize_*`, and `extract` no longer take a secp context. Signature verification now routes through the `secp256k1::ecdsa::verify` and `secp256k1::schnorr::verify` free functions instead of the deprecated `Secp256k1::verify_*` / `sign_*` context methods. Cascades through examples and the in-crate test modules. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`ToPublicKey::to_x_only_pubkey` now returns the role-tagged `bitcoin::XOnlyPublicKey` instead of the raw `bitcoin::secp256k1::XOnlyPublicKey`. The untagged secp type is still supported via the `ToPublicKey for bitcoin::secp256k1::XOnlyPublicKey` impl, which upconverts via `bitcoin::XOnlyPublicKey::from_secp`. `.serialize()` on the tagged type returns `([u8; 32], Parity)`; call sites that only want the raw 32 bytes now use `.serialize().0`. Drops redundant `.into()` conversions at call sites in `descriptor/tr/spend_info.rs`, `plan.rs`, and `psbt/mod.rs` now that the return type already matches the expected tagged type. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `Error::AddrP2shError` variant wrapped `bitcoin::script::RedeemScriptSizeError`, which was produced by the fallible `Address::p2sh` in bitcoin 0.33. It was only ever surfaced through `Sh::address_fallible` — an internal helper whose only public caller, `Sh::address`, already panicked with `.expect()` on the error path because the redeem-script-size invariant is enforced at descriptor construction time by `check_global_consensus_validity`. Inline `address_fallible` into `address`, matching the pattern already used by the sibling `script_pubkey` method in the same file. Drop the error variant and its `From<RedeemScriptSizeError>` impl. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`Plan::update_psbt_input` silently dropped the `redeem_script` when `WitnessScriptBuf::to_p2wsh()` failed, producing a quietly-broken PSBT input. Since the witness script was just constructed from a descriptor whose consensus size is already validated at construction time, the failure mode is an unreachable invariant violation, not a user error. Match the established pattern (`Wsh::address` in segwitv0.rs, `Sh::script_pubkey` in sh.rs): panic with an `.expect()` naming the invariant. A silent failure here would leave the PSBT in an unrecoverable state with no diagnostic for the caller. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n 0.33 Builder Two tests in `policy::compiler` compared `ms.encode()` directly against a `script::Builder::new().push_key(...).into_script()` value. Two adjustments for bitcoin 0.33: - `push_key` now takes `PublicKey` by value, not `&PublicKey`. - `push_int` returns `Result<Self, Error>` for minimal-encoding guarantees; the unchecked-size numeric pushes these tests produce should use `push_int_unchecked`. - `Miniscript::encode` is now generic over the script tag. Bind the expected value to an explicit `script::WitnessScriptBuf` so the tag parameter `T` can be inferred, instead of leaving it ambiguous in the `assert_eq!` argument position. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both `Cargo-minimal.lock` and `Cargo-recent.lock` were pinning
`bitcoin = 0.32.6`, which conflicts with the `bitcoin = 0.33.0-beta`
requirement in the working `Cargo.lock` and `Cargo.toml`. Left
unchanged, the `Stable - {minimal,recent}`, `Nightly - {minimal,recent}`,
and `MSRV - {minimal,recent}` matrix jobs would all fail resolution.
Regenerate both files from the current `Cargo.lock` so the CI matrix
resolves against bitcoin 0.33-beta.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`bitcoin::Script` no longer exists in bitcoin 0.33 — it was replaced with role-tagged script types. Update the intra-doc link on `TapTreeIterItem::miniscript` to point at `bitcoin::script::TapScriptBuf`, which is what `Miniscript::encode` actually produces for a tap leaf. Caught by `cargo rbmt docsrs` (the docs.rs CI job) which runs rustdoc with `-D rustdoc::broken-intra-doc-links`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`src/benchmarks.rs` is gated on `#[cfg(bench)]` so it wasn't caught by
the main migration passes. The `keygen` helper used three APIs that
changed in secp256k1 0.32 and bitcoin 0.33:
- `SecretKey::from_slice(&bytes)` → `SecretKey::from_secret_bytes(bytes)`
(takes the array by value now).
- `SecretKey::public_key(&secp)` no longer takes a context, drop it.
- `bitcoin::PublicKey { inner: pk, compressed: true }` → the fields
are no longer public; use `bitcoin::PublicKey::new(pk)` which
constructs a compressed key.
Caught by `cargo rbmt bench` (the Bench CI job). Benchmarks now build
and all 93 measured benches run clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
|
What is the purpose of this please? A 2000 line diff is unmergable. That is the same reason why #874 has been sitting there. |
Contributor
|
Approach NACK a6d1e5a I would have liked to see something more useful from an LLM, like an actual Capacity is low on this repo, and I really want to help out with reviewing the |
Author
mostly the work was done on my local fork so I could use MuSig2 on a personal project. I understand if it's too big of a change to merge , my read on #874 was that the rebase was the big factor holding that back, and thought maybe this was useful, most of the diff is a 1:1 of #874 so I hoped the review burden was less due to that. I also couldnt figure out a proper way to break this up into smaller parts, since bumping the crate breaks a lot of the API surface, so its basically all in or the project doesn't build. Happy to hear suggestions on breaking this up if you see the value in the collaboration, otherwise close |
Author
i spent a while back and forth with the LLM trying to figure out how to make the migration simpler but couldn't really figure it out. the bitcoin crate is just too interweaved across the whole project to reasonably do an incremental migration. Happy to hear suggestions for breaking up the PR |
Member
|
I don't speak for this project but in my mind we are not working on a version of this crate that depends on latest beta releases of bitcoin right now. Thanks for the effort though. |
Member
|
I agree with that sentiment. We want to get #895 and its followups in first. |
Member
|
...and I do speak for this project (I am one of the two primary maintainers). |
Author
|
understood. will close this out. also happy to throw my llm tokens (and my not so experienced eye) at pushing in the right direction for a more appropriate contribution, if you point me towards it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi all! I was working on a related project which required this migration, and after looking through the discussion in #922 and #874, I hope this is a welcome contribution.
Summary
Ports rust-miniscript to bitcoin = 0.33.0-beta and secp256k1 = 0.32.0-beta.2. This is a fresh branch off current master rather than a rebase of #874. That PR had diverged far enough from master that a rebase wasn't practical. PR #874 was used throughout as a reference for the migration shape: where I made a call, I cross-checked it against #874's approach and noted divergences explicitly. The end state is substantively aligned with #874 in public API, but the commit history on this branch is independent.
The work was done with Claude Code assistance. Every change was reviewed by me against PR #874 and the bitcoin 0.33 API surface, but you should treat the commit-by-commit shape as AI-assisted work rather than hand-crafted-from-scratch, and review with that in mind.
Approach
Initial very large "oneshot" commit fully driven by Claude, driving to naively pass compiler checks and tests. After that, small reviewable commits grouped by concern.
Deliberate divergences from PR #874
bitcointo the upcoming0.33.0-betarelease #874 excluded it)CI status
Locally verified green: Stable{minimal,recent}, Nightly{minimal,recent}, Lint, Docs, Docsrs, Bench, Format.
Expected red on GitHub:
bitcointo the upcoming0.33.0-betarelease #874 same),