zkevm-circuits
zkevm-circuits copied to clipboard
Add unit tests for math gadgets
In src/evm_circuit/util/math_gadget.rs we have several math gadgets that are used in opcode gadgets. While the opcode gadgets have unit tests (which indirectly test the math gadgets), we don't have unit tests for the math gadgets. Adding these unit tests would be very useful because it will allow us to test the math gadgets individually with their own edge cases.
Doing these unit tests may require creating some testing functionality because these gadgets use the ConstraintBuilder which is associated to EVM steps; but for unit testing them there's no actual step.
The list of math gadgets that require unit testing:
- [ ]
IsZeroGadget<F> - [ ]
IsEqualGadget<F> - [ ]
BatchedIsZeroGadget<F, const N: usize> - [ ]
AddWordsGadget<F, const N_ADDENDS: usize, const CHECK_OVREFLOW: bool> - [ ]
MulWordByU64Gadget<F> - [ ]
RangeCheckGadget<F, const N_BYTES: usize> - [ ]
LtGadget<F, const N_BYTES: usize> - [ ]
LtWordGadget<F> - [ ]
ComparisonGadget<F, const N_BYTES: usize> - [ ]
PairSelectGadget<F> - [ ]
ConstantDivisionGadget<F, const N_BYTES: usize> - [ ]
MinMaxGadget<F, const N_BYTES: usize> - [ ]
MulAddWordsGadget<F> - [ ]
ShrWordsGadget<F> - [ ]
MulAddWords512Gadget<F> - [ ]
ModGadget<F>
Thinking about this:
-
What about a special opcode
TEST=A5, only activated when compiled in a special way, with the only propose to test gadgets? e.g. The first word in the stack is the function to test, and there's 16 words in the stack as input and 16 words as output. It the cell step becomes too much big, we can also use other opcodes, we haveA5-EFthat are empy. -
Another way could be putting this as precompiles, maybe is cleaner.
We are working on it.
Some of the math gadgets that were also re-defined in the gadgets crate already have unit tests:
https://github.com/privacy-scaling-explorations/zkevm-circuits/tree/main/gadgets/src
So @xiaodino can refer them :)
@smtmfft IMO we should also refactor the math_gadget module into modules per gadget in it, mainly to lower the lines-of-code in the entire module. What do others think about this? If yes, I'm happy to do this in a separate PR, or this can be done within #865 as well.
Hi, @roynalnaruto You mean make each math gadget has its own file?? I think that's better structure, If the code does not change too much, I think this test would still works for them and I can add them to those files later on.
Hi, @roynalnaruto Which branch are you working on? Maybe I could create PRs to that one if necessary.
Hi, @roynalnaruto , I might misunderstood sth. Is there already a math gadget refactory task which is being done by someone else, or I can do that one by one just like https://github.com/privacy-scaling-explorations/zkevm-circuits/tree/main/gadgets/src??
@smtmfft I am at the moment not working on the math gadget or its refactoring. This issue is only for adding tests for each gadget in there, but I mentioned that along with that, it would be good to also refactor those gadgets into their own modules, so that the lines-of-code in math_gadget.rs are reduced and we can manage the entire module better.
Cool, I will create a task issue and self assign if no body else has concern :)
Unit tests added as part of #920