Skip to content
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

Import finalizer logic from miniscript #5

Merged
merged 17 commits into from
Nov 23, 2023
Merged

Conversation

tcharding
Copy link
Owner

@tcharding tcharding commented Nov 14, 2023

Import the psbt module from rust-miniscript while tip of master was at a280447. Then go wild refactoring the miniscript stuff.

Import the `psbt` module from `rust-miniscript` while tip of master was:

  a280447 Merge rust-bitcoin/rust-miniscript#617: Improve `TapTree` API

Only do changes required to get things building, however this includes
upgrade of the bitcoin dependency.

Feature gate the miniscript stuff because we want this crate to be a
drop in replacement for the `psbt` module in `rust-bitcoin`, users may
not want to have `rust-miniscript` in there dependency tree.
Clippy emits:

  warning: this expression creates a reference which is immediately
  dereferenced by the compiler

Remove reference.
Run `cargo +nightly fmt`, no other manual changes.
There is a whole bunch of error code, move it to a newly created
submodule.

Code move only, no logic changes.
Now that the `Psbt` type is defined in the same crate as the
`miniscript` extension traits we can implement them directly instead of
by way of extension traits.

Refactor only, no logic changes.
Refactor the import statements using the same coding conventions we use
in `rust-bitcoin`.

Refactor only, no logic changes.
Move the `PsbtInputSatisfier` to a newly create `satisfy` submodule and
re-export the type from the original module.

Code move only, no logic changes.
Code is [subjectively] easier to read if important things come first.

The `sanity_check` is a private helper function, put it down the bottom
of the file.

Code move only, no logic changes.
We do not need to be so explicit, the lifetime identifier `'a` is
perfectly descriptive.
We only need to use path qualifiers when more context is needed.
Re-order the `miniscript` `Psbt` pubic functions putting the common case
first.

1. finalize
2. finalize_mut
3. finalize_mall
4. finalize_mall_mut

The diff looks complicated but this patch is code move only.
The majority of the functions in the  `finalizer` module take a `Psbt`
as the first paramater, this means they can be implemented as methods on
`Psbt`. The remaining functions can be moved as well and made private to
the `miniscript` module.
Currently we have lots of logic for implementing the Extrator role. To
make this clear pull the logic out into separate modules and explicitly
mark it as WIP.
@tcharding tcharding marked this pull request as ready for review November 23, 2023 03:43
@tcharding
Copy link
Owner Author

tcharding commented Nov 23, 2023

Only change was to use the newly released version of miniscrpt (v11.0.0) and remove the patch section from the manifest.

EDIT: Added a patch to fix the docs build. All the docs need going over anyways so its just rough.

@tcharding tcharding merged commit 33fd386 into master Nov 23, 2023
4 checks passed
@tcharding tcharding deleted the import-finalizer branch November 23, 2023 04:06
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.

1 participant