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

Add simple deserialization example showing inability to deserialize p… #2

Open
wants to merge 6 commits into
base: testnet3
Choose a base branch
from

Conversation

evanmarshall
Copy link

@evanmarshall evanmarshall commented Feb 10, 2023

…rover keys from wasm

Overview

For the wallet and many other applications, we would like to generate proofs from the WASM. Currently, this is not possible mainly due to the serialization and deserialization of the prover/verify keys.

This PR is a simple test to illustrate the issue.

As I believe it's crucial to enable proof generation from the wasm, I'm open to any feedback. My hope is that we can agree on an approach and I'm happy to implement it. If I'm misunderstanding anything about the issue, please let me know. I hope I'm wrong and the fix is simple.

How to run

  1. Put this snarkVM branch in the same parent directory: Make Marlin key types public snarkVM#2
    .../aleo-dev/snarkVM & .../aleo-dev/aleo

  2. Update this line to the location your transfer prover file

  3. Run the rust test:
    cargo test --package aleo-wasm --lib -- program::proving_key::tests::test_deserialize_key --exact --nocapture

It should output the first 1000 bytes of the transfer prover file.

  1. Run the wasm test:
    wasm-pack test --node --lib -- program::proving_key::tests::test_deserialize_key_wasm

It should fail with the error:

panicked at 'called `Result::unwrap()` on an `Err` value: the call expects empty flags', wasm/src/program/proving_key.rs:35:64

Stack:

Error
    at /Users/evanmarshall/Documents/aleo/target/wasm32-unknown-unknown/wbg-tmp/wasm-bindgen-test.js:1216:17
    at logError (/Users/evanmarshall/Documents/aleo/target/wasm32-unknown-unknown/wbg-tmp/wasm-bindgen-test.js:157:18)
    at module.exports.__wbg_new_abda76e883ba8a5f (/Users/evanmarshall/Documents/aleo/target/wasm32-unknown-unknown/wbg-tmp/wasm-bindgen-test.js:1215:65)
    at console_error_panic_hook::hook::h01dee5383346a78b (wasm://wasm/324a6c9a:wasm-function[575]:0xf5a9b)
    at core::ops::function::Fn::call::h5b6ca970811401b8 (wasm://wasm/324a6c9a:wasm-function[2046]:0x141621)
    at std::panicking::rust_panic_with_hook::hd000e9fb43b5781d (wasm://wasm/324a6c9a:wasm-function[1250]:0x1316b6)
    at std::panicking::begin_panic_handler::{{closure}}::he16e52e9a7dddeb1 (wasm://wasm/324a6c9a:wasm-function[1381]:0x138133)
    at std::sys_common::backtrace::__rust_end_short_backtrace::h227361e053771d9e (wasm://wasm/324a6c9a:wasm-function[1710]:0x13f9c1)
    at rust_begin_unwind (wasm://wasm/324a6c9a:wasm-function[1636]:0x13e8c2)
    at core::panicking::panic_fmt::h9d972fcdb087ce21 (wasm://wasm/324a6c9a:wasm-function[1661]:0x13ef3d)
  1. The error throws here: https://github.com/AleoHQ/snarkVM/blob/testnet3/fields/src/macros.rs#L277

Background & Explanation

  1. The bytes of a proving file look like:
0000 1000 0000 0000 0000 e1c0 0000 0000
0000 e1c0 0000 0000 0000 50dc 0100 0000
0000 5966 0200 0000 0000 eedf 0100 0000
0000 0c00 0000 0000 0000 d3c0 073d 5cd9
c5ec 75f4 ede3 8d59 1c9e c4a1 d48f eaf0
73d6 2a89 e70e 948a 628b d794 dc9e bbd1
6a8f 1fd8 1b9c 9033 6b00 facd 0e8c 8f0d
5579 4c1b 5299 ff0a 26a6 a908 f2c4 07b8
6792 e21f d5cf 3214 24e6 1f2c d783 cda3
0258 bc99 fbf1 c0fb c680 fe48 2d47 d975
5367 8437 e054 3aaf 638e e392 22d6 9b23
bc1f 2c58 6e2d 88c9 0950 6159 e5f9 61ff
9e2f 5d1d 51ef 5d5d 0900 47c1 151b b9c4
c4e5 4d4d aa99 0504 3d5e b60a 3a78 dcf7
2650 06c6 3556 cce8 ad5e 8116 266c 3f6f
292d e94b fad5 8972 fe80 99ea f0c8 1580
7e89 8a74 67c8 05ef 25b4 bccc eb0a 4898
bc75 22de 7c9a c290 c5ca 08cd 7bd6 13ab
....
  1. When running from rust and deserializing a field element from FP384 (from KZGCommitment <- CircuitInfo <- CircuitVerifyingKey <- CircuitProvingKey).

The reader gets:
d3c0 073d 5cd9 c5ec 75f4 ede3 8d59 1c9e c4a1 d48f eaf0 73d6 2a89 e70e 948a 628b d794 dc9e bbd1 6a8f 1fd8 1b9c 9033 6b00 00

But when running from the wasm, the reader gets:
5966 0200 0000 0000 eedf 0100 0000 0000 0c00 0000 0000 0000 d3c0 073d 5cd9 c5ec 75f4 ede3 8d59 1c9e c4a1 d48f eaf0 73d6 00

The wasm is missing an additional 24 byte offset.

  1. As an experiment, if you just read those 24 bytes here: https://github.com/AleoHQ/snarkVM/blob/testnet3/algorithms/src/snark/marlin/data_structures/circuit_proving_key.rs#L57

The prover key will continue to deserialize successfully for about a couple of KB until it hits a similar error where once again it tries to deserialize a field but is missing an offset:
Should be:
[255, 191, 61, 21, 31, 87, 78, 184, 34, 113, 126, 20, 187, 13, 181, 251, 187, 25, 62, 187, 56, 30, 239, 1, 36, 225, 174, 5, 251, 139, 214, 34, 58, 141, 133, 53, 119, 214, 140, 129, 159, 111, 59, 65, 57, 56, 100, 1, 0]
But is instead:
[0, 0, 0, 0, 255, 191, 61, 21, 31, 87, 78, 184, 34, 113, 126, 20, 187, 13, 181, 251, 187, 25, 62, 187, 56, 30, 239, 1, 36, 225, 174, 5, 251, 139, 214, 34, 58, 141, 133, 53, 119, 214, 140, 129, 159, 111, 59, 65, 0]

Speculative Cause

I believe this is caused by the dependence of the CanonicalSerialization & CanonicalDeserialization on the AST:
https://github.com/AleoHQ/snarkVM/blob/testnet3/utilities/derives/src/canonical_serialize.rs#L74 but I'm not 100% sure.

As the WASM is able to deserialize individual types successfully provided the correct offset, it doesn't seem to be a problem with incompatible types in WebAssembly.

Since the wasm feature flags generate a different AST, deserializing a key created with a different AST breaks.

This explanation would also explain why the offsets are always missing reads, as in the Rust version always adds additional bytes in comparison before trying to deserialize some type.

Potential Fixes

Assuming I'm accurately describing the problem, I'm not sure if there's a simple fix. Here are some ideas:

  1. Create a new serialization format, not dependent on the AST using something like serde that can be used by the wasm. The potential here is that adding a new serialization format could change the AST enough as to require new generation of default keys like the credits program, inclusion keys, and fee keys.

  2. Change the default serialization of keys to not be dependent on the AST. There are a few different ways of doing this but a simple one would be adding an unique ID to each type and using that in the serialization instead of the Ident. This would require regeneration of the keys.

@Pratyush
Copy link

Maybe it's a usize-size difference (u32 vs u64)?

@evanmarshall
Copy link
Author

Thank you @Pratyush . That did the trick. Most of the changes were here: demox-labs/snarkVM#2
I'll turn this into a proper PR and open against the AleoHQ branches.

This could also serve as an optimization as well for the file size if we could use 4 bytes instead of 8 for the usize as it appears quite frequently in the output file.

@evanmarshall
Copy link
Author

Update on the size, it doesn't save very much. This is for the transfer prover key.
Size before: 209_282_964 bytes
Size after: 207_674_406 bytes

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

Successfully merging this pull request may close these issues.

2 participants