Make lagrange basis handling safe #2672
Conversation
|
@mrmr1993, I am working through some linting errors. It looks like you removed the function error[E0599]: no method named `add_lagrange_basis` found for struct `poly_commitment::srs::SRS` in the current scope
--> kimchi/src/circuits/expr.rs:3012:17
|
3012 | srs.add_lagrange_basis(constraint_system.domain.d1);
| ^^^^^^^^^^^^^^^^^^ help: there is a method with a similar name: `get_lagrange_basis`If that function is meant to be removed, what should be done with these references? |
|
@mrmr1993 is there a test case explicitly for the case for which we are implementing this? Would it be easy to whip one up? |
|
@45930 Because the issue was found in o1js, we probably want to create any remaining PRs for the mina / bindings / o1js sides with a test in o1js (probably just one of the few that were raised in o1-labs/o1js#1411) to see the effects of this fix. |
|
@querolita , thanks for the review! Is it possible to test this branch in o1js locally? There is a PR up for o1js: o1-labs/o1js#1857 but I'm not sure how to get these changes into that branch to test with. |
|
@45930 Oh thanks for pointing me to the other PR, I was not aware that it was already being done. I will take a look at it locally to try to test the changes before merging 😄 |
| impl<Key: Hash + std::cmp::Eq, Value> HashMapCache<Key, Value> { | ||
| pub fn new() -> Self { | ||
| HashMapCache { | ||
| contents: Arc::new(Mutex::new(HashMap::new())), |
There was a problem hiding this comment.
Cool! I did something like this in parallel in my working branch: 444e07f#diff-c89c45dc84ecd1f409b94d69ca374ed88678982f04f419b79435fd5ebff85b4fR362
There was a problem hiding this comment.
@mrmr1993 I'd suggest using RWLock though. Unless I'm misunderstanding, Mutex won't allow two concurrent reads, locking several parallel uses (which is the case in mina), and the upgrade to RWLock is easy. Any reasons not to?
This PR adds the concept of
HashMapCacheto remove the potential for errors / panics in code. In particular, this addresses o1-labs/o1js#1411.