Wasm crate refactor

[grod220]

We should split up the current Rust wasm crate into two:

A) penumbra-wasm Should have only the wasm-bindgen functions that expose utility functions

B) penumbra-extension Should have the penumbra-extension specific functionality here.

(Do these in later issues) C̶o̶n̶s̶i̶d̶e̶r̶a̶t̶i̶o̶n̶s̶:̶ -̶ ̶I̶n̶d̶e̶x̶e̶d̶d̶b̶ ̶g̶l̶o̶b̶a̶l̶ ̶v̶s̶ ̶d̶e̶p̶e̶n̶d̶e̶n̶c̶y̶ ̶i̶n̶j̶e̶c̶t̶i̶o̶n̶ -̶ ̶N̶P̶M̶ ̶p̶u̶b̶l̶i̶s̶h̶i̶n̶g̶ ̶w̶o̶r̶k̶f̶l̶o̶w̶ -̶ ̶T̶e̶s̶t̶i̶n̶g̶ ̶s̶t̶r̶a̶t̶e̶g̶y̶

[hdevalence]

I think the penumbra-wasm code should continue to live in the core protocol repo, and the penumbra-extension code should move into this repo. This way, the parts of the extension that use typescript and that use Rust can be updated in lockstep.

[grod220]

the penumbra-extension code should move into this repo

We should think through this because there are significant trade offs.

Current flow:

• Wasm crate lives in core repo
• Web code consumes crate npm package published on new tags. Mid-cycle npm publishing done via manual github action runs.
• Dev experience for changes require local compiling and pointing web monorepo npm dependencies to the build output on disk.
• Breaking changes in core require fixing wasm crate simultaneously. A kind of core-team maintenance commitment. Though not all things are caught (e.g. fee tiers implemented in pcli, but not wasm crate).

Migrate to web monorepo

• Crate would be an additional package (would require to have a package.json that mapped build/lint commands from javascript to rust)
• Web code consumes compiled wasm not from crates.io, but from the local crate and local npm dependencies pointing to the build output.
• Penumbra core would be required to publish all crates to crates.io so it can be remotely consumed by relocated wasm crate. Published on tags.
• Core team members would not fix breaking changes in relocated wasm crate. Upon a new release, web team is responsible for pulling latest dependencies and making fixes (similar to spec changes now).

====

From @conorsch's statement, it sounded like it was previously segmented off but then decided best to merge it in core. What was the reasoning behind this?

In the end it feels like it would be a more convenient dev environment on the web side. Though, is it that impactful? It saves some overhead with 1) npm publishing (not needed anymore) and 2) local dev needing to change npm deps to the local directory. But what we trade off is the maintenance commitment from the core team. All of those breaking changes that would have been done as they happen, get backloaded to the web team that has to do them all at once upon a release. It feels quite similar to the current web releases where we have to go and do some code archaeology in the penumbra-core repo to see what changed and why. From the core-team side, I'm sure it's nice not to have maintain the wasm crate, but on net, it will be more work for the web team.

[Valentine1898]

In fact, wasm has never been separate from core. It's just that at the beginning of development there was a fork core repository penumbra with wasm, which was maintained by zpoken, and later merged.

[hdevalence]

Penumbra core would be required to publish all crates to crates.io so it can be remotely consumed by relocated wasm crate. Published on tags.

Why can't penumbra-extension use a git dependency in its Cargo.toml?

Core team members would not fix breaking changes in relocated wasm crate. Upon a new release, web team is responsible for pulling latest dependencies and making fixes (similar to spec changes now).

No, the wasm crate would stay in the monorepo. The code that would move would be the extension-specific code.

[Valentine1898]

Theoretically, there is a way in which we can get rid of Indexed-db global simply by passing all data as parameters to the wasm function.

1. view_server.rs View server (aka block scanner) uses Indexed-db only to save and read advices. The solution is that we should add advices discovery as one of the conditions for calling flush_updates(). Currently we only call flush_updates() when we detect new notes and swaps. Also in this case we will need to add advices to ScanBlockResult and also pass advices already stored in indexed-db when initializing view server. My assumptions:
   • we won't lose in scanning speed because we won't actually call flush_updates() more often than we do now because in blocks where we detect advices we are also guaranteed to detect swaps and call flush_updates() anyway
   • We will not pass more data during initialization than we do now. We can easily recognize already used advices from unused ones. Used advices can be ignored and not passed during view server initialization, and considering that swap claim will almost always automatically occur in the next block after swap, in practice advices will be unused for only one block, and will almost never be passed during view server initialization.
2. tx.rs We use reading from indexed-db to construct transaction view and transaction perspective. The point is that only during the TxV and TxP generation itself we can determine which notes and assets data we need. This is the main reason why we have to read data from indexed-db directly and we can't prepare it on TS side and pass it as parameter, and passing all notes and swaps records as parameters seems like a very bad idea. The solution is to decompose the transaction_info() wasm function into several smaller helper wasm functions, and move the TxV and TxP generation itself to the TS.
   • This does not lead to additional code duplication. We already have 2 different functions for TxV and TxP generation for wasm and pcli.
3. wasm_planner.rs For swap_claim(), there is no difficulty to pass SwapRecord as a parameter, having previously prepared the data on the TS side. For plan(), we can move the implementation of this function to the TS side. First we call planner.notes_requests() and get NotesRequest then, based on the received NotesRequest, we prepare notes data on the TS side.
   And finally call plan_with_spendable_and_votable_notes() with already prepared data.

I think I could quickly test the proposed solution for View server, and if it works, then other steps to get rid of Indexed-db are definitely possible. @grod220

[grod220]

Helpful writeup @Valentine1898. We probably should have created another issue for the indexeddb global thing. After we migrate things over, this is worthwhile prioritizing next as code improvements. cc'd @TalDerei

[Valentine1898]

moved to separate issue #488

[grod220]

@TalDerei it appears these are candidates for staying in core:

• build.rs
• dex.rs
• error.rs
• keys.rs (except for load_proving_key())
• note_record.rs
• planner.rs (though this is somewhat complex)
• swap_record.rs
• utils.rs
• tx.rs (things without indexeddb dependency)

These are the candidates for moving:

• storage.rs
• tx.rs (things with indexeddb dependency)
• view_server.rs
• wasm_planner.rs (later, if we can remove indexeddb dependency, should be moved to core)

...but another question entirely is: "Why not just move everything?". Do we anticipate more consumers of this crate?

One challenge will be that our web code will now have two .wasm dependencies unless we import all of the core functions and re-export them from the penumbra-extension code.

[TalDerei]

Why not just move everything?

We don't anticipate more consumers of this crate at the moment, and the core team shouldn't need to maintain any part of the wasm stack. Henry green lighted moving the entire wasm crate to the web to simplify the dependency chain relation.