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

light-client: wasm compilation #463

Closed
3 of 5 tasks
brapse opened this issue Jul 20, 2020 · 23 comments · Fixed by #812
Closed
3 of 5 tasks

light-client: wasm compilation #463

brapse opened this issue Jul 20, 2020 · 23 comments · Fixed by #812
Assignees
Labels
light-client Issues/features which involve the light client structure High level repo-wide structural concerns wasm
Milestone

Comments

@brapse
Copy link
Contributor

brapse commented Jul 20, 2020

Edited due to summarize the discussion bellow

Context

The light client core verification algorithm can verify an application state using only a sequence of headers. This has uses in the light-node where it can perform IO and fetch headers from full nodes. Another use for the light client verification is within the context of IBC where a full node will verify data from foreign chains before including it in local transactions. In this way the light client verification works as a pure function without IO but instead relies on the relayer to feed it data.

Goal

We want to be able to compile our light-client verification to WASM such that it can be used by other chains to verify foreign data. This requires a build target that is independent from WASM incompatible libraries such as net2/socket2 or other IO operations.

Status

@xla xla added the light-client Issues/features which involve the light client label Jul 20, 2020
@xla xla changed the title light-client wasm compilation light-client: wasm compilation Jul 20, 2020
@liamsi
Copy link
Member

liamsi commented Jul 21, 2020

If trying to build to the wasm32 target (either using cargo build --target wasm32-unknown-unknown or wasm-pack build) it currently fails because of an indirect dependency: net2 (and socket2).
Errors look very similar to: rustwasm/wasm-pack#657

more info: https://users.rust-lang.org/t/webassembly-with-rust-problem-compiling-wasm/29575

@brapse
Copy link
Contributor Author

brapse commented Jul 22, 2020

Yes this makes sense. I think this means we need to refactor our crates to have verification completely separate from IO. Will refactor this issue to capture this 👍

@romac
Copy link
Member

romac commented Jul 22, 2020

We might not need a separate crate if we guard the I/O stuff and dependencies behind a feature flag (that would be enabled by default, and toggled off when building for WASM).

@ebuchman ebuchman added the structure High level repo-wide structural concerns label Jul 27, 2020
@romac
Copy link
Member

romac commented Aug 11, 2020

Happy to report that the tendermint crate successfully builds for both wasm32-unknown-unknown and wasm32-wasi targets on Rust nightly with only the following change, followed running cargo update:

diff --git a/tendermint/Cargo.toml b/tendermint/Cargo.toml
index 4bda89e..ca52fed 100644
--- a/tendermint/Cargo.toml
+++ b/tendermint/Cargo.toml
@@ -31,6 +31,9 @@ all-features = true
 [badges]
 codecov = { repository = "..."}

+[lib]
+crate-type = ["cdylib", "rlib"]
+
 [dependencies]
 anomaly = "0.2"
 async-trait = "0.1"
@@ -46,7 +49,7 @@ serde_bytes = "0.11"
 serde_repr = "0.1"
 sha2 = { version = "0.9", default-features = false }
 signatory = { version = "0.20", features = ["ed25519", "ecdsa"] }
-signatory-dalek = "0.20"
+signatory-dalek = { version = "0.20", features = ["nightly"] }
 signatory-secp256k1 = { version = "0.20", optional = true }
 subtle = "2"
 subtle-encoding = { version = "0.5", features = ["bech32-preview"] }

Rust nightly is required because we need the nightly feature of signatory-dalek which is passed down to ed25519-dalek, which in turn passes it down to clear_on_drop to work around rustwasm/wasm-bindgen#763.


wasm32-unknown-unknown

$ cargo build -p tendermint --release --target wasm32-unknown-unknown
   Compiling tendermint v0.15.0 (/Users/coromac/Informal/Code/Current/tendermint-rs/tendermint)
    Finished release [optimized] target(s) in 49.29s

$ file target/wasm32-unknown-unknown/release/tendermint.wasm
target/wasm32-unknown-unknown/release/tendermint.wasm: WebAssembly (wasm) binary module version 0x1 (MVP)

$ du -hs target/wasm32-unknown-unknown/release/tendermint.wasm
1.4M	target/wasm32-unknown-unknown/release/tendermint.wasm

wasm32-wasi

$ cargo +nightly build -p tendermint --release --target wasm32-wasi
   Compiling tendermint v0.15.0 (/Users/coromac/Informal/Code/Current/tendermint-rs/tendermint)
    Finished release [optimized] target(s) in 7.00s

$ file target/wasm32-wasi/release/tendermint.wasm
target/wasm32-wasi/release/tendermint.wasm: WebAssembly (wasm) binary module version 0x1 (MVP)

$ du -hs target/wasm32-wasi/release/tendermint.wasm
1.6M	target/wasm32-wasi/release/tendermint.wasm

@romac
Copy link
Member

romac commented Aug 11, 2020

The light-client crate requires more work because we need to feature-flag the dependencies which transitively pull net2 etc.

@tarcieri
Copy link
Contributor

@romac FYI, it should be possible to remove both signatory-dalek and signatory-secp256k1 as dependencies now, using ed25519-dalek and k256 directly instead: #391 (comment)

I should have a PR to do it later today.

@romac
Copy link
Member

romac commented Aug 11, 2020

It wasn't actually that much work to get the light-client crate to compile for WASM: 2eda9d8

All that was needed was to feature-guard the dependency on the RPC client, both in Cargo.toml and in the code (ie. the production implementations of the Io and EvidenceReporter traits).

@ebuchman ebuchman added this to the v0.18 milestone Aug 28, 2020
@romac romac modified the milestones: v0.18, v0.17.0 Sep 9, 2020
@romac romac self-assigned this Sep 9, 2020
@ethanfrey
Copy link

What is the state here? I'd love to make use of a web assembly light client in a browser app.

#553 seems to have added support, but this issue is open. One obvious missing thing is docs. Like a small spot in the README about wasm client (or link to separate doc if much) and maybe a link to npm package if you have one (I can build it, but it's nice to have an official package to pull in for releases).

@romac
Copy link
Member

romac commented Oct 27, 2020

Aside from docs, the two main things that are needed for an in-browser light client at the moment are:

Happy to help with or even collaborate on any of this :) Will start by adding a section to the README mentioning the current state of WASM support.

@ethanfrey
Copy link

Hi @romac thanks for the response. I'd be happy to collaborate on this.

From our side, we are looking to add light-client support for https://github.com/cosmos/cosmjs and have most of the infrastructure in place already. We have launchpad and stargate clients, doing queries and signing transactions. Closer to this request, we have a tendermint-rpc package that exposes the high-level API and was designed to be future proof (using an adaptor to manage 0.27 -> 0.28 -> 0.31 -> 0.33 -> 0.34 as fields were added, renamed, and changed from integers to string). This has been battle tested almost 2 years.

Closer to the app, we have stargate queriers that either use grpc or make raw/provable queries. The queryVerified function already ensures there is a valid merkle proof that leads to the AppHash in the header .

When checking the merkle proof, we get the next header to check the AppHash via subscription or polling or both.

private async getNextHeader(height?: number): Promise<Header> {
  // ....
    if (headersSubscription) {
      const firstHeader = await firstEvent(headersSubscription);
      // The first header we get might not be n+1 but n+2 or even higher. In such cases we fall back on a query.
      if (firstHeader.height === searchHeight) {
        nextHeader = firstHeader;
      }
    }

    while (!nextHeader) {
      // start from current height to avoid backend error for minHeight in the future
      const correctHeader = (await this.tmClient.blockchain(height, searchHeight)).blockMetas
        .map((meta) => meta.header)
        .find((h) => h.height === searchHeight);
      if (correctHeader) {
        nextHeader = correctHeader;
      } else {
        await sleep(1000);
      }
    }
   // ...
}

But we do not verify this is the valid header yet. The code we wish to replace/upgrade this just the following logic which runs after we get the header for the proper height:

    assert(nextHeader.height === searchHeight, "Got wrong header. This is a bug in the logic above.");
    return nextHeader;

We only assert the height of the header we got is correct, but do not check the signatures at all. Ideally, we would be able to replace this with some logic to prove the header via light-client proofs. We can happily do all the rpc in TypeScript, using websockets where available and falling back to polling where not.

@ethanfrey
Copy link

TL;DR: we would be happy to have a passive light-client verifier that takes some trusted seed upon initialization, and takes a new header + signatures (aka Commit) and returns one of 3 states:

  1. Valid - trust it
  2. Invalid - trash it
  3. Too many changes - bisect and try to prove again

We are happy to do this process in TypeScript, we just need a working verifier. We can also store all valid headers (roots of trust) ourselves.

If there is a fully working wasm blob that takes some initial trusted state and then can replace getNextHeader(height?: number): Promise<Header> doing verification and bisecting and only returning when we have internally verified the proper header, that would work as well. It just seems easier to have a passive wasm code for computation, rather than trying to hook it into the networking and the storage layers.

@webmaster128
Copy link

npm package which exports the light client API to JavaScript via wasm-pack.

This is a hard one. If you want a JS package that bundles Wasm and works in node.js, browsers and WebPack at the same time, you have two options:

  1. Embed Wasm blob is text (base64 or something like that) in a JavaScript file
  2. Use the new new Url("./relative/path.wasm", import.meta.url)

The first is solid but not very efficient because browsers cannot do streaming compilation and there is an encoding overhead. The second option does not work with Webpack 4, only Webpack 5, which means you cannot use it in typical React apps these days. What both solutions have in common is that they are not supported directly in wasm-pack.

@romac
Copy link
Member

romac commented Oct 29, 2020

I am afraid I don't fully understand neither the problem nor the alternatives/potential solutions described above.

Just a shot in the dark: what if we'd provide a WASM blob via wasm-bindgen? Would that be flexible enough to accommodate your use case? If not, how would you recommend we proceed instead?

@webmaster128
Copy link

Happy to take care of the Wasm bundling an JS bindings. I already invested many many hours into this, so it would be doable relatively easy once there is a wasm-bindgen setup.

Just a shot in the dark: what if we'd provide a WASM blob via wasm-bindgen? Would that be flexible enough to accommodate your use case?

That would be sufficient, yes. Have a look at https://github.com/CosmWasm/drand-verify this is a rather simple crate where you can see one approach how this can be. Key points to look at:

  1. There is a "js" feature, which pulls in the wasm-bindgen dependency, such that you can make it optional.
  2. There is a wrapper API that does a bit of type conversion and defines public Wasm interface: https://github.com/CosmWasm/drand-verify/blob/master/src/verify_js.rs. I don't think you can use Vec<u8> or any other complex type directly with wasm-bindgen. We might need to string-encode everything we send to or get from Wasm. Also it uses u32 in some interfaces that usually use u64. This is to be able to use JavaScript number, which can only store integers up to 53 bit.

Once this is done, we can compile the crate on our own, test different targets (see Node.js/browsers instructions in https://github.com/CosmWasm/drand-verify#build-for-js) and do the Wasm bundling.

@webmaster128
Copy link

Here are docs about the usable data types: https://rustwasm.github.io/docs/wasm-bindgen/reference/types.html. Seems like Box<[u8]> can be used for binary data directly.

@romac
Copy link
Member

romac commented Oct 29, 2020

Awesome, thanks a lot for the pointers!

Interestingly, in the time between our two comments, I came up with a very basic prototype that exposes the verifier to JS via wasm-pack/wasm-bindgen and which follows a very similar approach to the one in the repo you linked above: https://github.com/informalsystems/tendermint-rs/tree/romac/wasm-verifier/wasm-client

Rust wrapper: https://github.com/informalsystems/tendermint-rs/blob/romac/wasm-verifier/wasm-client/src/lib.rs
Usage from JS: https://github.com/informalsystems/tendermint-rs/blob/romac/wasm-verifier/wasm-client/app/index.js

To make this work, one currently has to

  1. Run wasm-pack build from within the wasm-client folder
  2. Move the app directory and run npm start
  3. Open http://localhost:8080/ and look at the console
  4. It should display:
{
  "Ok": {
    "Invalid": {
      "NotWithinTrustPeriod": {
        "expires_at": "1970-01-15T00:00:03Z",
        "now": "2020-10-21T12:40:04.160328400Z"
      }
    }
  }
}

Is this good starting point for you to take over? How can I be of more help otherwise?

@webmaster128
Copy link

Very nice! If you prefer a separate crate over a feature, sure. Probably makes sense that way if the API is more than a handful of functions. I would just call it "wasm-verifier" and not include any other client functionality such as networking.

I can look into this more details at some point this month. But it is definitely the right direction and it would be great if you maintain that demo along with the rest of the codebase.

@romac
Copy link
Member

romac commented Oct 29, 2020

I can look into this more details at some point this month. But it is definitely the right direction and it would be great if you maintain that demo along with the rest of the codebase.

Awesome. Feel free to hit me up if I can be any help or if just want to chat about that.

If you prefer a separate crate over a feature, sure. Probably makes sense that way if the API is more than a handful of functions. I would just call it "wasm-verifier" and not include any other client functionality such as networking.

We intend to eventually expose the full light client via WASM so a separate crate makes more sense to me indeed.

We definitely want to leave the networking (and storage) layer out of the exposed bindings, but it's not exactly clear to me how we'd go about providing the Fetch based implementation of it that could then be used in conjunction with the library, but that's something we can figure out at a later stage.

@webmaster128
Copy link

webmaster128 commented Oct 29, 2020

Perfect.

One more thought regarding naming: the demo crate you created is JS specific, not Wasm specific. All the wasm-bindgen stuff means embedding Wasm into JavaScript ("Facilitating high-level interactions between Wasm modules and JavaScript"). You can also compile to Wasm that runs in a different host environment (e.g. Wasmer or look into WASI. In this case, the public Wasm exports and imports would be different. So I suggest thinking of this create as a "js-verifier" or "light-client-js" or something like that.

@ethanfrey
Copy link

It would also be cool to see about compiling this all into wasi. It may work 'out of the box', including rust networking and storage. Not useful for js clients but if you like wasm, it is an interesting alternative to deploying native files. But that is more of a research project.

Both wasmer and wasmtime can run wasi objects from the command line

@webmaster128
Copy link

@romac your demo in the branch looks nice. Can you turn that into a PR for further development?

@romac
Copy link
Member

romac commented Nov 12, 2020

@romac your demo in the branch looks nice. Can you turn that into a PR for further development?

Sure, just opened #671

@thanethomson
Copy link
Contributor

In discussion with the team yesterday, we decided to reduce the scope of this issue by focusing on addressing cosmos/cosmjs#492 and deferring additional work to the following issues:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
light-client Issues/features which involve the light client structure High level repo-wide structural concerns wasm
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants