-
Notifications
You must be signed in to change notification settings - Fork 130
Fix lagrange basis functions for web environments #3325
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
base: master
Are you sure you want to change the base?
Conversation
- Add caml_fp_srs_get_lagrange_basis_ptr: Returns pointer for web worker communication - Add caml_fp_srs_get_lagrange_basis_read_from_ptr: Extracts data from pointer - Follows existing working pattern from lagrange_commitments_whole_domain functions - Enables zkProgram.compile() with cache writing in web environments - Part 1 of 2: Fp functions (Fq functions to follow)
- Add caml_fq_srs_get_lagrange_basis_ptr: Returns pointer for web worker communication - Add caml_fq_srs_get_lagrange_basis_read_from_ptr: Extracts data from pointer - Completes pair with Fp functions added in previous commit - Uses same proven pattern as existing lagrange_commitments_whole_domain functions - Part 2 of 2: Now both Fp and Fq are ready for web environments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What a fix! I am sure this one bug was hard to find, well done 💯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🆗
@@ -320,6 +320,33 @@ pub mod fp { | |||
}); | |||
basis.iter().map(Into::into).collect() | |||
} | |||
|
|||
/// Web-compatible version that returns pointer for worker communication |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both ways are web compatible, it just doesn't work with our current worker implementation. For clarity, I would change this comment to something that better reflects what it does: Calculates and returns a pointer to the lagrange basis
@@ -400,4 +427,31 @@ pub mod fq { | |||
}); | |||
basis.iter().map(Into::into).collect() | |||
} | |||
|
|||
/// Web-compatible version that returns pointer for worker communication |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
Summary
This PR fixes the issue where
zkProgram.compile()
with cache writing hangs indefinitely in web browsers. The problem was thatcaml_fp_srs_get_lagrange_basis
andcaml_fq_srs_get_lagrange_basis
functions returnWasmVector
types that cannot be transferred over the web worker synchronization API.Changes
Added new pointer-based functions for web compatibility:
Fp Functions
caml_fp_srs_get_lagrange_basis_ptr
- Returns pointer for worker communicationcaml_fp_srs_get_lagrange_basis_read_from_ptr
- Extracts data from pointerFq Functions
caml_fq_srs_get_lagrange_basis_ptr
- Returns pointer for worker communicationcaml_fq_srs_get_lagrange_basis_read_from_ptr
- Extracts data from pointerImplementation Details
lagrange_commitments_whole_domain_ptr
Box::into_raw()
andBox::from_raw()
for pointer managementRelated PRs
This is part 1 of 3 for the complete fix:
Files Changed
plonk-wasm/src/srs.rs
- Added 4 new pointer-based functions