✨ add EC and modexp precompiles #143
Conversation
| >( | ||
| builder: CsBuilder<T, F, GC, TB>, | ||
| ) -> CsBuilder<T, F, impl GateConfigurationHolder<F>, impl StaticToolboxHolder> { | ||
| // TODO: Maybe some gates are not actually needed since it was copy-pasting from the previous secp256k1 ecmul implementation. |
There was a problem hiding this comment.
you can check if this is the case or not by running a full proof of the circuit and then printing the gate stats - it should give you a tally of how often each gate is called
| (Some(TARGET_CIRCUIT_TRACE_LENGTH), Some(1 << 26)) | ||
| } | ||
|
|
||
| fn add_tables<CS: ConstraintSystem<F>>(cs: &mut CS) { |
There was a problem hiding this comment.
are any of these tables actually necessary? i thought ecadd only uses gates and it seems useless to add byte splits, bitwise ops and fixed base muls
| (Some(TARGET_CIRCUIT_TRACE_LENGTH), Some(1 << 26)) | ||
| } | ||
|
|
||
| fn add_tables<CS: ConstraintSystem<F>>(cs: &mut CS) { |
There was a problem hiding this comment.
i assume it's the same case with gates and tables here but these should be checked and removed before merging on each new circuit
| ECAdd(T), | ||
| ECMul(T), | ||
| ECPairing(T), |
There was a problem hiding this comment.
i think its best to keep the order of when the circuit was added, so they should be at the bottom of the list, it will break backwards compatibility with the old numeric_circuit_types
| ZkSyncBaseLayerStorage::ECAdd(..) => "Elliptic Curve points addition", | ||
| ZkSyncBaseLayerStorage::ECMul(..) => "Elliptic Curve point multiplication by a scalar", | ||
| ZkSyncBaseLayerStorage::ECPairing(..) => "Elliptic Curve pairing on BN254", |
There was a problem hiding this comment.
same goes then for all these instances of course
src/tests/run_manually.rs
Outdated
| match el { | ||
| ZkSyncBaseLayerCircuit::ECAdd(_) => { | ||
| base_test_circuit(el); | ||
| } | ||
| _ => { | ||
| continue; | ||
| } | ||
| } | ||
| // base_test_circuit(el); |
There was a problem hiding this comment.
ensure this is changed back before merging
NikitaMasych
force-pushed
the
feature/ec-and-modexp-precompiles
branch
from
370c42a to
12a6b87
Compare
|
Rebased to |
What ❔
Add tests for ECMul, ECAdd, ECPairing and Modexp.
Why ❔
The above were absent.
Checklist
zk fmtandzk lint.