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

Fix Lagrange bases caching on the web #313

Merged
merged 2 commits into from
Nov 19, 2024

Conversation

mitschabaude
Copy link
Collaborator

@mitschabaude mitschabaude commented Nov 19, 2024

companion of o1-labs/o1js#1906

this fixes a case where compile() got stuck on the web version and kept spinning without error.

diagnosis:

  • when the recursive verifier circuit needed access to fp lagrange bases, it ended up in lagrangeCommitment() in srs.ts
  • there, in the case of a cache miss, we attempted to call getLagrangeBasis() to get the entire lagrange basis and store it in the cache
  • however, in the browser, there is an additional layer behind methods using concurrency, like caml_fq_srs_get_lagrange_basis which getLagrangeBasis() aliases to. this additional layer (see web-backend.js > overrideBindings()) was not able to handle the Uint32Array return value of caml_fq_srs_get_lagrange_basis(). instead, that return value was interpreted in a way that caused the method to never return (even though the lagrange basis had been successfully created!)

the minimal fix is twofold:

  1. in the web worker layer, throw an error when calling one of the methods that would never return.
  2. in the SRS caching layer, only attempt to return the full LB if we even have a writable cache. otherwise, call a different method that will store the LB internally but not attempt to return it.

The fix circumvents the problem in the most typical case, where we don't have a writable cache in the browser. In the case where we do have a writable cache, the error message and comments added here will make it very obvious what the problem is.

The full solution (which I don't have time for) is outlined in code comments

@45930 45930 merged commit e172bf0 into o1-labs:main Nov 19, 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.

2 participants