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

State of ergo-lib-wasm #663

Open
ross-weir opened this issue Dec 19, 2022 · 4 comments
Open

State of ergo-lib-wasm #663

ross-weir opened this issue Dec 19, 2022 · 4 comments

Comments

@ross-weir
Copy link
Collaborator

Want to highlight some pain points/things I've encountered while using the wasm library and some suggestions on how I think we might be able to improve the situation. The team probably has a lot of other work/higher priorities but maybe one day we can work on improving the package 😄

Pain Point: Library size/bloat - ergo-lib-wasm is approaching 3mb size, that is really non-trivial for frontend applications in particular

Possible solution to this could be breaking up the monolithic ergo-lib-wasm package into a monorepo/workspaces structure that mirrors the wrapped rust crates and publish each as their own npm package, i.e @ergoplatform/merkle-tree, @ergoplatform/nipopow, etc.

As an example, I was looking to use NiPoPoW functionality which is a small subset of ergo-lib-wasm but in order to do so I need to pull in almost 3MB of extra unneeded code, which greatly impacts one of NiPoPoWs use cases of running on constrained devices

Library is published as browser and nodejs versions leaving it up to consumers to try and figure out how to correctly use it in their projects

Looks like we may have looked into this in the past as we're defining browser and main fields in package.json but still end up publishing 2 packages, it looks like its possible to start publishing as one package, some refs:

API is not ergonomic and doesn't follow JS conventions

Some examples:

  • Functions taking integers require you to perform a dance to convert a JS number into a I64 type
  • Collections of types (ie. ErgoBoxes, ErgoBoxCandidates) being their own types instead of arrays of ErgoBox with less functionality than array counterparts make them awkward to work with
  • We don't follow JS naming conventions and use Class.new() functions instead of constructors, etc

I think exposing an API that makes more use of js_sys for types (and then converts them under the hood on the rust side where needed) would greatly reduce awkwardness. For the conventions stuff, making more use of wasm_bindgen annotations like wasm_bindgen(constructor), wasm_bindgen(getter, setter), wasm_bindgen(js_name = renamedToCamelCase) I think would go a long way to improving usability and making the library more familiar for JS devs

@greenhat
Copy link
Member

I really appreciate this!

Pain Point: Library size/bloat - ergo-lib-wasm is approaching 3mb size, that is really non-trivial for frontend applications in particular

Agree.

Possible solution to this could be breaking up the monolithic ergo-lib-wasm package into a monorepo/workspaces structure that mirrors the wrapped rust crates and publish each as their own npm package, i.e @ergoplatform/merkle-tree, @ergoplatform/nipopow, etc.

As an example, I was looking to use NiPoPoW functionality which is a small subset of ergo-lib-wasm but in order to do so I need to pull in almost 3MB of extra unneeded code, which greatly impacts one of NiPoPoWs use cases of running on constrained devices

https://github.com/ross-weir/ergo-lib-wasm looks awesome! I looked into splitting up the ergo-lib-wasm myself but got cold feet since we're exposing a wrapped rust type to JS and I got stuck on how to pass these wrapped types (think box, tx, etc.) between different Wasm runtimes. I see you used serde ser/deser to pass types through Rust<->JS border. I like this and the performance penalties can be tolerated. I probably made a mistake to not have chosen this model initially.

Besides that, I'm planning to migrate to no_std which should also shave off a significant chunk.

Library is published as browser and nodejs versions leaving it up to consumers to try and figure out how to correctly use it in their projects

Looks like we may have looked into this in the past as we're defining browser and main fields in package.json but still end up publishing 2 packages, it looks like its possible to start publishing as one package, some refs:

* [give option target=all which generates pkg supporting commonjs and esm rustwasm/wasm-pack#313](https://github.com/rustwasm/wasm-pack/issues/313)

* [Both rustwasm/wasm-bindgen#326 (comment)](https://github.com/rustwasm/wasm-bindgen/pull/326#issuecomment-415991338)

Totally agree. My fear is that might leave some edge cases unsupported (like using the node version in an electron app, etc.). If that's not the case I'm all in.

API is not ergonomic and doesn't follow JS conventions

Yep.

Some examples:

* Functions taking integers require you to perform a dance to convert a JS `number` into a `I64` type

Well, in my defense, back then I did not find a better way to avoid 53-bit mantissa limit of JS Number. I'm open to all suggestions.

* Collections of types (ie. `ErgoBoxes`, `ErgoBoxCandidates`) being their own types instead of arrays of `ErgoBox` with less functionality than array counterparts make them awkward to work with

This was(still is?) the limitation of wasm-bindgen. But with serde serialization we can do a native arrays anyway.

* We don't follow JS naming conventions and use `Class.new()` functions instead of constructors, etc

Yep, although we have some wasm_bindgen(constructor) attributes set, overall it's pretty bad.

I think exposing an API that makes more use of js_sys for types (and then converts them under the hood on the rust side where needed) would greatly reduce awkwardness. For the conventions stuff, making more use of wasm_bindgen annotations like wasm_bindgen(constructor), wasm_bindgen(getter, setter), wasm_bindgen(js_name = renamedToCamelCase) I think would go a long way to improving usability and making the library more familiar for JS devs

I 100% agree!

@ross-weir
Copy link
Collaborator Author

I see you used serde ser/deser to pass types through Rust<->JS border. I like this and the performance penalties can be tolerated. I probably made a mistake to not have chosen this model initially.

If it means anything, this is also how Deno handles it so that adds some weight to the perf penalties being acceptable I guess 😄

Totally agree. My fear is that might leave some edge cases unsupported (like using the node version in an electron app, etc.). If that's not the case I'm all in.

The JS ecosystem is a mess when it comes to distribution but I'd be surprised if we encounter a edge case that is insurmountable - the edge case you mention should work with the example repo (I'll validate)

I did not find a better way to avoid 53-bit mantissa limit of JS Number

I'd suggest using BigInt JavaScript type and also providing methods that take string (as hex, etc) or Uint8Array (bytes) for compatibility with older environments, popular bigint JS libraries like bn.js use strings / byte arrays so we'd be compatible with them too

This was(still is?) the limitation of wasm-bindgen. But with serde serialization we can do a native arrays anyway.

Disappointing this is still a issue in wasm-bindgen. A problem with serde will be it serializes to arrays of basic JSObjects (not the class) so you can't call methods on the resulting js objects (AFAIK). I have some ideas how to work around it but yeah this a tricky one

Well, in my defense

None of this is meant as a criticism of you, I'm impressed by how much you've accomplished across so many tech stacks. We've learnt a heap about wasm bindings over the last couple of years so can probably put it to use!


I have some ideas with how we proceed if you'd like to discuss at some point (after the holidays or whenever suits you)

@ross-weir
Copy link
Collaborator Author

ross-weir commented Dec 21, 2022

I'll just put my ideas on proceeding here

I'm thinking:

  • Continue making updates here (maybe just small ones like Add public API for Diffie-Hellman Tuples transaction signing #664) until new packages are usable
  • Split ergo-lib-wasm into a new repo. It would be good to use tooling to manage versioning/publishing of monorepos like changesets, not sure how this would go in a monorepo within a monorepo and I feel like things could get messy/hard to manage if we keep it here
  • Start splitting ergo-lib-wasm into packages in the new mono repo, I'm thinking something along the lines of
    • @ergoplatform/blockchain types/entities in the ergo blockchain like Block, Box, etc
    • @ergoplatform/wallet operations/types for wallets, UnsignedTransaction, Wallet.signTx, etc
    • @ergoplatform/address operations for working with ergo addresses & encoding
    • @ergoplatform/merkle-tree
    • @ergoplatform/nipopow
    • Others as we get to them

I'm assuming ergo still has access to the ergoplatform npm account so we can publish under the @ergoplatform npm scope

@greenhat
Copy link
Member

greenhat commented Dec 21, 2022

Sounds great!

I agree with the need for new repo and your take on package split. I think @deadit should have access to the ergoplatform account on npm.

EDIT: I created https://github.com/ergoplatform/ergo-lib-wasm and sent you an invite. Feel free to set things up.

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

No branches or pull requests

2 participants