Skip to content

Conversation

@apoelstra
Copy link
Member

See commit messages for more detail.

@apoelstra apoelstra changed the base branch from master to 0.25.x August 28, 2025 16:11
@LeoComandini
Copy link
Contributor

Tried to update locally in LWK to this branch
Blockstream/lwk@e92059b

however I'm getting few errors related to SighashCache
https://docs.rs/elements/latest/elements/sighash/struct.SighashCache.html

error[E0271]: type mismatch resolving `<T as Deref>::Target == Transaction`
    |
878 | /     fn sighash_msg<T: Deref<Target = elements::Transaction>>(
879 | |         &self,
880 | |         idx: usize,
881 | |         cache: &mut SighashCache<T>,
882 | |         tapleaf_hash: Option<TapLeafHash>,
883 | |         genesis_hash: elements::BlockHash,
884 | |     ) -> Result<PsbtSighashMsg, SighashError> {
    | |_____________________________________________^ expected `elements::transaction::Transaction`, found `elements::Transaction`
    |
note: two different versions of crate `elements` are being used; two types coming from two different versions of the same crate are different types even if they look the same
...

@apoelstra
Copy link
Member Author

Thanks for testing! Lemme check on this. I swear I kept the sighash module for exactly this reason..

@apoelstra
Copy link
Member Author

...no, I think this got lost. Sorry, I restarted this PR 4 or 5 times and must have gotten sloppy. Will update.

In 0.26.0 we made two breaking changes: we changed the signature of
some blinding functions in PartiallySignedTransaction, and we removed
the & from a couple utility methods in pset::raw::ProprietaryKey and
in TxOut.

Therefore, those three types, and any type which refers to them in their
public API, have semver-breaking changes. But no other type does! So we
can re-export everything else in a minor release of 0.25.x.

The types that we must preserve are:

* PartiallySignedTransaction
* TxOut
* Transaction (contains Vec<TxOut> in a public field)
* Block (contains Vec<Transaction> in a public field)

There are then several modules which have impl blocks on these types,
which we need to keep; and the PSET `Output` type has accessors for
a `TxOut`, so we need to keep that, which means that we need to keep
its whole `pset::Serialize` impl, which means we need to keep the
whole private `Map` trait. Frustrating.

Now, this commit is very large and may be hard to review. However, much
of it is easy:

* First, we entirely delete the `address`, `blech32`, `confidential`,
  `dynafed`, `encode`, `error`, `ext`, `fast_merkle_root`, `hash_types`,
  `hex`, `issuance`, `locktime`, `opcodes`, `parse`, `schnorr`, `script`,
  and `taproot` modules and replace them with re-exports.

  To confirm these changes are OK, you ought to check their API on
  crates.io (or by just grepping) and see that none of the types listed
  above appear there.

* Then, the `block` and `transaction` modules have every type except for
  `TxOut`, `Transaction` and `Block` removed. So that's probably also
  easy to review, though again it's good to check that these types don't
  appear in the API of anything that was removed.

* Then `blind.rs` and `pset/elip100.rs` have impls on `Transaction` and
  `PartiallySignedTransaction`, so they had to stay.

* Then `sighash.rs` has some functions which are conditioned on T with
  T: Deref<Transaction> or Borrow<TxOut> so these had to be kept; we
  also had to keep the private method split_anyonecanpay_flag and move
  it to a standalone function.

* Finally, the whole PSET module was kept, except that I removed the
  Serialize trait and a couple utility structs, including the errors,
  in a somewhat ad-hoc way.

PROBABLY it's fine to just skim this and we'll release it and see if
anything breaks.
@apoelstra apoelstra force-pushed the 2025-08/semver-0.25 branch from 0a6729e to 3dc8cd2 Compare August 28, 2025 18:05
@apoelstra
Copy link
Member Author

@LeoComandini sorry, should be fixed. Can you try now?

@apoelstra
Copy link
Member Author

....sigh, crap. We bumped the MSRV in 0.26.0 but not in 0.25.x. I think we just can't do this.

@LeoComandini
Copy link
Contributor

@LeoComandini sorry, should be fixed. Can you try now?

@apoelstra compiles now locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants