Skip to content

Add psbt ffi bindings#36

Merged
benalleng merged 1 commit into
bitcoindevkit:masterfrom
benalleng:rob-psbt-10-15
Oct 28, 2025
Merged

Add psbt ffi bindings#36
benalleng merged 1 commit into
bitcoindevkit:masterfrom
benalleng:rob-psbt-10-15

Conversation

@benalleng
Copy link
Copy Markdown
Collaborator

This add some basic ffi bindings for psbts including serialization
and deserizlization as well as a few methods used in payjoin
integration tests that are needed for downstream bindings there.

supersedes #22 as it build off the work @rustaceanrob did

@benalleng benalleng force-pushed the rob-psbt-10-15 branch 3 times, most recently from 9626734 to 9615770 Compare April 25, 2025 15:59
@benalleng
Copy link
Copy Markdown
Collaborator Author

@DanGould I chose these bindings for now based on some integration tests from rust-payjoin.

namely, .fee(), .extract_tx(), and .combine()

https://github.com/payjoin/rust-payjoin/blob/518d8d2600c0a57fc3ef7ab3b75a5e6b1c7f89aa/payjoin/tests/integration.rs#L902-L905

Comment thread src/lib.rs Outdated
Comment thread src/error.rs
Comment on lines +173 to +174
#[derive(Debug, thiserror::Error, uniffi::Error)]
pub enum PsbtError {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe remote error types can be used with uniffi::remote(Enum). Then I'd guess they don't need to derive thiserror::Error, or require a conversion, or require the explicit display #[error] attributes. Not sure If it has been tried in this repo before. I think an enum still needs to be written with each variant but some of the other boilerplate that might get out of sync can go. I am not certain of this but the documentation suggests it.

https://mozilla.github.io/uniffi-rs/latest/types/remote_ext_types.html

Copy link
Copy Markdown
Collaborator Author

@benalleng benalleng May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the Psbt Error Enum there are a lot of complex error payload types, wouldn't these all still need to be converted for this to be easily used as a remote(Enum)? I'm just not sure its worth it to avoid error messages getting out of sync unless there is a way to more easily avoid those conversions.

Copy link
Copy Markdown
Contributor

@DanGould DanGould May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think #[uniffi(flat_error)] might be able to keep the upstream payload types but inherit debug/display implementations (perhaps with #[uniffi::export(Debug), uniffi::export(Display)], not sure to what extent these macros can be combined). You may need to rely on the bitcoin/std feature so errors are std::error::Error in order to make sure UniFFI knows the remote PsbtError can be thrown as an error.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Errors have been a pain in our side for a long time, but I'm curious to see if some of the new features in uniffi might fix some of those pain points for us. In particular, simply using the remote error types would be awesome instead of reimplementing errors.

For a comprehensive dive into this from a year ago (some things have changed since then so I still have hope!) see this issue: bitcoindevkit/bdk-ffi#509.

Copy link
Copy Markdown
Member

@thunderbiscuit thunderbiscuit May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I remember our problem was mostly about having to choose between offering the complex types in the payload vs correct and interesting error messages for users (which they expect in their target languages).

I think if you have complex types in the payload, your Display becomes a stringified version of the type, and that's what you get in Kotlin for your exception.message field (where you expect a good message). Same thing for Swift (I think it's the description field). If you want to instead fix this message, you cannot have fields in your enum (something like that). The issue as far as I remember is that for some errors you'd much rather have the message, and for others you'd rather have the data... 😅

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that you cannot use #[uniffi::export(Display)] on enums (you get the error unsupported item: only functions and impl blocks may be annotated with this attribute).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that message is related? I definitely use #[uniffi::export(Display)] on this struct which is neither a function nor an impl block

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok weird. Maybe I got this wrong. I tried a bunch of stuff 2 weeks ago to play with the errors a bit more and got to the same place we ended up so far.

Let me know how you make out I'm definitely interested in better errors!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bumping this so we get it merged (or merge a new pr if that's what ben wants to do instead, up to you @benalleng )

@thunderbiscuit
Copy link
Copy Markdown
Member

thunderbiscuit commented Oct 21, 2025

Using 0.30.0 of uniffi your errors should start getting a toString/description/__str__ from the Display trait (doesn't work yet for Kotlin but should be fixed at some point soon, see mozilla/uniffi-rs#2699).

Still need to test to make sure that the thiserror crate will work as expected and populate your Display, but that should in theory clean up your errors quite a bit if you're willing to upgrade to 0.30.0.

@thunderbiscuit
Copy link
Copy Markdown
Member

We are also doing some work exposing the Psbt type in bdk-ffi, so if you want to see some of that take a look at our implementation. The lead on this is @ItoroD

@benalleng benalleng force-pushed the rob-psbt-10-15 branch 3 times, most recently from 0269d9e to 3d9384a Compare October 27, 2025 19:31
@benalleng
Copy link
Copy Markdown
Collaborator Author

I plan on merging this as-is and then addressing the Error serialization after uniffi 0.30 has been setup in the rest of our ffi layer

This adds support for the Psbt struct in our ffi layer. The methods I
have added are only the ones we actually needed in our upstream ffi
though we will likely want to add some methods like sign at some point
in the future.
@reez
Copy link
Copy Markdown
Collaborator

reez commented Oct 27, 2025

I plan on merging this as-is and then addressing the Error serialization after uniffi 0.30 has been setup in the rest of our ffi layer

im good w that 👍

@benalleng benalleng merged commit 73416a5 into bitcoindevkit:master Oct 28, 2025
7 checks passed
@benalleng benalleng deleted the rob-psbt-10-15 branch October 28, 2025 17:36
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.

4 participants