Currently the macro ast_arith! evaluates the ast right to left, regardless what the operator is. So the second expression of lookup is actually evaluated as:
l_last * (z_x * (z_x - z_x))
So it becomes 0 every time, to fix it just wraps the z_x2 in parentheses. Perhaps we should make the ast_arith! left to right to make it more intuitive?
han0110
commented
2 years ago
Hi! While trying to aggregate a circuit using lookup, I get incorrect aggregated proof and found these 2 bugs:
zeroandoneare swapped mistakenlyhttps://github.com/scroll-tech/halo2-snark-aggregator/blob/c6623bb091c1208e058fc7d1c721d0db9c543089/halo2-snark-aggregator-api/src/systems/halo2/params.rs#L132-L146
The second expression of lookup argument is evaluated in a wrong order
https://github.com/scroll-tech/halo2-snark-aggregator/blob/c6623bb091c1208e058fc7d1c721d0db9c543089/halo2-snark-aggregator-api/src/systems/halo2/lookup.rs#L96-L97
Currently the macro
ast_arith!evaluates the ast right to left, regardless what the operator is. So the second expression of lookup is actually evaluated as:So it becomes
0every time, to fix it just wraps thez_x2in parentheses. Perhaps we should make theast_arith!left to right to make it more intuitive?