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

WIP restore the JS build #2

Merged
merged 6 commits into from
Feb 2, 2024
Merged

WIP restore the JS build #2

merged 6 commits into from
Feb 2, 2024

Conversation

protz
Copy link
Collaborator

@protz protz commented Feb 1, 2024

This restores the JS build in its full glory up to a logical error in the lookup API. Notes:

  • this relies on HACL-wasm, and assumes that there is a checkout of HACL* located at HACL_HOME
  • this relies on hacl-packages, and assumes that there is a checkout of hacl-packages located at HACL_PACKAGES_HOME

Right now, running the tests require performing the following manual steps:

  • dune build --profile=release (important to use release profile)

Then, from the js/ subdirectory:

  • ./import.sh, in order to
    • bring hacl-wasm and the JS bindings into the wasm/ subdirectory
    • bring the MLS.js file (precisely, MLS_JS.bc.js) into the js/ subdirectory
  • followed by node index.js to run the tests.

The copy is required because the wasm/js bindings live in two different places. Maybe there's a way to do this more easily with just hacl-packages, after a successful build of hacl-packages.

I could use help with the following things:

  • first and foremost, it looks like the process_welcome_message API is no longer working; see index.js, notably, the line that says "adding to B's store", and the line that says "looking into B's store". Previously, this used to be the same hash in both places, but now it's different. This is the main issue right now.
  • concerning the build, I don't know how you'd like things to work out in a clean and modular fashion -- I can write some Makefile rules to assemble everything in a separate subdirectory if that's easier, let me know. In any case, some guidance on how to follow the path of Nix enlightenment appears much needed
  • once that's figured out, we should figure out how to add node to the build followed by node index.js to CI

@TWal
Copy link
Collaborator

TWal commented Feb 1, 2024

Indeed, the MLS API was creating a hash on the leaf package, but was querying the key database with key package refs, hence the inconsistency.

While doing so, I found that we are using the same key for the init key and the leaf key, they should be different and we would probably need to adapt the top-level API accordingly… For now we can leave that for later.

I did some nix fu to add the js tests to the CI in this PR, but the build system is getting nastier and nastier, mainly because of my decision of building tests only when running make check, which made sense at some point, but may not be relevant anymore.

@protz
Copy link
Collaborator Author

protz commented Feb 1, 2024

I agree that at least extracting the tests should be part of the main build invocation. It's too complicated otherwise.

@TWal TWal merged commit 6d5d638 into main Feb 2, 2024
1 check passed
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.

3 participants