bitcoindevkit / bitcoin-ffi

Other
12 stars 7 forks source link

RFC(don't merge): Use proc macro instead of the UDL file #14

Open Davidson-Souza opened 4 weeks ago

Davidson-Souza commented 4 weeks ago

As discussed in one of our last calls, this PR implements everything using uniffi's proc macros rather than a UDL file. It offers the advantage of requiring less code duplication and not requiring knowledge of the weird UDL syntax. Another advantage over using UDL is that you have better control over the scaffolding, so in unusual cases where uniffi can't implement things on its own, you can step in and do it.

I've implemented Transaction and all it subfields, and migrated existing stuff to proc macros. Since this is an exploratory thing, I've used a mix of Records and Objects. Records are cool in that they let consumers access the fields themselves, and without the indirections of an Arc, but can't have associated impl blocks.

I've found some limitations along the way: looks like docstrings from the Rust code aren't used to document the bindings, only UDL docstrings. As per uniffi's docs:

Further, using this capability probably means you still need to refer to the UDL documentation, because at this time, that documentation tends to conflate the UniFFI type model and the description of how foreign bindings use that type model. For example, the documentation for a UDL interface describes both how it is defined in UDL and how Swift and Kotlin might use that interface. The latter is relevant even if you define the interface using proc-macros instead of in UDL.

There's also an odd problem with dictionaries (or Record in proc macros land). If I define one in bitcoin-ffi and make a dependent crate that uses that dict as a field for some other dict, the compiler errs, saying the original dict doesn't implement Lower and Lift. But if I use in the same crate, it works just fine. I've followed their docs and issues, and couldn't find any reason why this didn't work (it does work with UDL definitions).

Edit: Each commit implements one thing (or at least "only the nescessary to implement that thing"). However, if we decide to go this way, this PR obviously needs to be broken down.

reez commented 3 weeks ago

Thanks for the draft PR! I like the idea of using proc macros to avoid UDL duplication and more. I appreciate the one commit for one thing path you're taking now, makes it easier to review, and we can always just squash down at the end etc.

Thoughts on some of the points you brought up:

Pumped to see the work you've done on this!