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

Add finalize to Psbt #580

Closed
wants to merge 1 commit into from

Conversation

andreasgriffin
Copy link
Contributor

@andreasgriffin andreasgriffin commented Jul 31, 2024

Description

Add finalize() to Psbt. See #469

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

@andreasgriffin
Copy link
Contributor Author

@thunderbiscuit : Do you know how to solve the electrum-client errors in the CI-tests?

@thunderbiscuit
Copy link
Member

A rebase of this on master should solve your issues! We're finally ready to start merging new stuff. Feel free to rebase and I'll take a look today/tomorrow.

@andreasgriffin andreasgriffin force-pushed the finalize branch 2 times, most recently from c877ced to 14f3b75 Compare September 5, 2024 17:04
@andreasgriffin
Copy link
Contributor Author

@thunderbiscuit : Could you take a look if it is ready to be merged?

@rustaceanrob
Copy link
Contributor

rustaceanrob commented Sep 13, 2024

You'll want to squash this into a single commit by using git rebase -i HEAD~5 and changing pick to squash. You will also want to rename your commit message using conventional commits. Although, I am afraid this work may also be in vein as adding PSBT to bitcoin-ffi is planned here. If I pick up PSBT as a type to add I will be sure to include finalize

clippy

map to OtherPsbtErr

otherwise uniffi is giving a warning

clippy
@andreasgriffin
Copy link
Contributor Author

If I pick up PSBT as a type to add I will be sure to include finalize

Great!

@rustaceanrob rustaceanrob mentioned this pull request Nov 16, 2024
8 tasks
@rustaceanrob
Copy link
Contributor

#630 closes this. Thanks @andreasgriffin

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.

3 participants