Current state of rust library API and implementation

[ozgrakkurt]

Currently rust api and implementation of the library isn't idiomatic rust code, which makes it awkward to read and understand.

I think we can change the library so it is like a normal rust library, but wrap it in javascript so It is convenient to use from javascript as well. I am currently working on a concrete proposal/PR to do this.

Also currently running clippy fails on the rust library and running fmt changes so much lines that it is hard to apply to the whole library and not break a lot of people's work.

See also:

• TxBuilder: CIP2 MultiAsset variants added #264 (comment)
• change TransactionBuilderConfigBuilder API for js usage #270
• Fixing tx-builder-config-builder to take references and not values #269

[rooooooooob]

I am currently working on a concrete proposal/PR to do this.

Could you provide some details about this please @ozgrakkurt ?

After writing those comments in #264 I started trying something exploratory so that we can better weigh the options with what I mentioned in #264's comment about using a macro. I'm trying to see what could be done via procedural macros to make wrappers similar to the wrappers I linked in js-chain-libs. My hope is that it could even do parameter and error conversions but I'm a noob at proc macros so we'll have to see how it goes.

I don't want to commit to any approach at this moment but I wanted to explore this option at least to give us a better idea. This would help the usage from rust as mentioned but the usage from js would still be the same as it is now (i.e. not great).

Also I'm linking these wasm-bindgen issues here since they're highly relevant to making the JS experience better:

rustwasm/wasm-bindgen#111
rustwasm/wasm-bindgen#1742

We need to remember that the npm package users are far, far more numerous than the rust crate users as well.

[rooooooooob]

@ozgrakkurt I was thinking about this more. We're going to need some kind of wasm_bindgen interface no matter what, whether that be auto-conversions applied programatically, or whether it be hand-written wrappers (e.g. how js-chain-libs or Ergo's sigma-rust wasm bindings work), or whether we use code-gen to do it for us.

This interface would be necessary whether we use it as the main interface (i.e. now), as well as if we try and write some JS around it to make usage nicer from JS. One important thing to keep in mind for the latter is I still can't think of a way to make memory management not utter garbage since there's no refcounting/destructors. The only way to get around that is if the rust object is temporary (e.g. is created and then detroyed in a single function for serialize() or deserialize(), or for calling crypto code, etc) but then you're still writing the non-(de)serialization code over again in JS + the conversion code between both formats which sounds like a lot. If we do just the first option then the JS is still just as awkward to use, and only the rust API has been improved. So I think unless we have a lot of JS dev time that we're stuck with a subpar JS experience.

I was hesitant about hand-writing wrappers because:

1. You'd have comments in multiple places and that would likely lead to someone changing one but not the other by accident, etc.
2. You have to think not just about now, but what about in the future when we need to add code (e.g. upcoming hardforks)? We've traditionally used cddl-codegen to generate 90%+ of our code from IOHK's binary CDDL spec and then we've added in useful functionality on top.

I guess Point 1. is mostly valid if we're directly consuming the wrappers rather than having a more idiomatic-JS library on top of the direct wasm_bindgen wrappers.

Point 2. has wider-spread implications regardless of how we do the wrappers since we still need to generate code for most structs unless you feel like hand-writing 1000+ lines of CDDL code for hardforks and hoping there's no mistakes. It's worth noting that you could still use the tool to generate the serialization (CBOR) components that are mostly in serialization.rs and then hand-write (or hand-edit the generated code) for everything that would be in lib.rs. If we do that then some of the serialization code would be broken depending on how far we are transforming the library to idiomatic rust.

So that brings an important question: what do we imagine this library looking like once we transform into idiomatic rust code? I assume we stop making those map and wrapper classes like Assets or ScriptHashes which are wrappers around Vec<T> BTreeMap<T, U> or even linked_hash_map::LinkedHashMap<T, U> in some places. What about access? Are we keeping it getters/setters everywhere or are going to change (some/most/all of the cbor structs whose entire definition is basically just their fields?) it to public fields? Changing parameters to be value (so we can move values into instead of cloning)? What about returns? There are traits we could implement that would be helpful for usage from rust too. Do we just axe the BigNum type entirely then and swap it for u64? What else did you have in mind?

As for the auto-wrapper-gen I was looking into proc macros which could wrap the definitions. They could return the same token stream on rust builds and on wasm builds could transform the methods a bit since you get the whole method body AST when parsing with the syn crate which with quote could be inserted into the new body with some kind of wrapper or by a conversion trait implementation. I tried writing one that would auto-implement to do nothing for most traits and only for things like Result<T, E> would have a custom one but it looks like helpful things like having trait default types (not just function bodies) is still unstable. So I think it might be more fruitful to do it just through the macro, although it's very ugly doing so since you're dealing mostly with raw ASTs. syn greatly helps though but only to an extent. Using syn you could do automatic public traits to wasm-compatible getters/setters by wrapping the struct definition in one. Comments are preserved using these macros.

[ozgrakkurt]

@rooooooooob I am currently thinking of it like we make a core rust library which has all the functionality in idiomatic rust.
Then we write a thin wasm wrapper around it, sort of like writing a ffi for it. This means we need comments on the user facing wasm_bindgen layer which should be a small surface.

I think typedefs to BTreeMap<T, U> are fine. But we can make fields public maybe. Then the wrapper can define setters and getters.

Sorry I am not able to make much progress on this now

[rooooooooob]

I guess for the comment-preservation argument you could argue that it's possible there would be some parts of it that would be different between rust/js but I think 98% would be identical.

I am currently thinking of it like we make a core rust library which has all the functionality in idiomatic rust.

It would help if you could list all the planned changes once you think of them so that we can discuss them and see how they would affect writing wasm wrappers (or auto-generating them) over top of them. Depending on the size of the next hardfork it might be worth making minor changes to the cddl-codgen tool to instead of generating the wasm_bindgen/rust compromise to instead generate more idiomatic rust (if it's small changes) and possibly even generate wrappers, depending on how we decide to handle those.

I think typedefs to BTreeMap<T, U> are fine. But we can make fields public maybe. Then the wrapper can define setters and getters.

We'd still have to generate the wasm_bindgen-compatible wrappers since those templated types can't be directly exposed to wasm with the exception of Vec<T> but only when T is a primitive type sendable to wasm like u32. Even Vec<String> isn't allowed for some reason. You'd have to double-check the list of types that implement to ToWasmABI/etc traits in wasm_bindgen though.

Sorry I am not able to make much progress on this now

Don't worry at all! This isn't super urgent and is something that should be very carefully thought through first as it would be a super big refactor/API breakage. I was mostly just concerned by your "I am currently working on a concrete proposal/PR to do this" comment since I didn't want us to jump into any particular approach without comparing all the different possibilities and their pros/cons and having a discussion between other devs familiar with the lib. I wasn't trying to make this happen ASAP but rather the opposite. We shouldn't jump into sudden heavily breaking changes.