feat(acir static analysis): EcAdd constraints #20297
Conversation
| return Bool<CircuitBuilder>{ result_idx, bool_ct::from_witness_index_unsafe(&builder, result_idx) }; | ||
| } | ||
| } | ||
| throw std::runtime_error("No normalization gate found for bool " + std::to_string(a_idx)); |
There was a problem hiding this comment.
I don't use optional, because it is way too verbose to handle every var
There was a problem hiding this comment.
You can choose to throw or return after the first finding in a higher-level functions, but putting just a throw is not a very good practice. What if we decide to parallelize discovery? We'll get different exceptions each time
… sn/acir_ec_add_constraints_partial_check
Flakey Tests🤖 says: This CI run detected 1 tests that failed, but were tolerated due to a .test_patterns.yml entry. |
Rumata888
left a comment
Rumata888
left a comment
There was a problem hiding this comment.
Please replace throws with optional at the helper level.
You are using the same mechanism for searching the arithmetic gates in several places. It can be implemented once and then reused.
The algorithm you are using will have total complexity O(kn), where k is the number of elliptic gates and n the size of the circuit. This is bad. You should first go through the whole circuit and store a mapping (variable index -> vector (gate, witness index)). Then you'll be able to quickly retrieve the gates which contain those witnesses and only go through them.
For the constant gates it's better to parse them once and then just check against the hashmap.
| return Bool<CircuitBuilder>{ result_idx, bool_ct::from_witness_index_unsafe(&builder, result_idx) }; | ||
| } | ||
| } | ||
| throw std::runtime_error("No normalization gate found for bool " + std::to_string(a_idx)); |
There was a problem hiding this comment.
You can choose to throw or return after the first finding in a higher-level functions, but putting just a throw is not a very good practice. What if we decide to parallelize discovery? We'll get different exceptions each time
| FF q_r = FF::zero(); | ||
| for (size_t gate_idx = 0; gate_idx < builder.blocks.arithmetic.size(); gate_idx++) { | ||
| bool condition = true; | ||
| condition &= builder.blocks.arithmetic.w_l()[gate_idx] == a_idx; |
There was a problem hiding this comment.
The complexity for these checks is O(n). That's bad
Rumata888
left a comment
Rumata888
left a comment
There was a problem hiding this comment.
Add some descriptions, please
| #include <vector> | ||
|
|
||
| namespace cdg { | ||
| template <typename CircuitBuilder, typename FF> class FilterFunctionBuilder { |
There was a problem hiding this comment.
There are 0 comments in the file. What is it used for?
3 participants
This PR introduces EcAdd constraints processing
Currently we only check that gates, created by is_on_curve check exist
Also introduced helpers for boomerang_value_detection, which might help to find the default gate results (field_t: +, *, madd, add_two, conditional_assign; and for bools) from the witnesses