hash_to_curve and derivate_value_base

[XuyangSong]

[simonmasson]

That is pretty nice!

I had a look at the map_to_curve part of the circuit, and there are several parts where I would like some clarifications.

Conditions of the type is_zero()

In several places, we need to check that a value is zero or not: ta.is_zero(), num_x1.is_zero(), and also in subroutines, e.g. for a square root computation, we check the condition (x - sqrt_x²).is_zero(). In order to check X.is_zero(), we store X_inverse = X.inverse() in an advice column and then check that 1 - X * X_inverse polynomial condition is satisfied. But could we simply write X to be the polynomial? (maybe it does not make sense).

Inverse

When we need to compute inverses (e.g. in sqrt_ratio()) we store the value of the inverse in an advice column and check 1-x*x_inverse (similar to the is_zero() case.

Square roots

For a square root computation, we store the square root in an advice column, but then we need to store 1/(x-sqrtx²) in order to check (x - sqrtx²).is_zero().

Other advice columns

There are other advice columns that seems to be "useless" (in the sense that we could compute them with adds and mults). For example, ta = zu²(1+zu²) could be computed in-circuit instead of storing it in an advice column, no? I guess you store it because you have some space in the advice columns, and so it is more efficient?

[XuyangSong]

But could we simply write X to be the polynomial?

If X is a bool, we can use X as the conditional expression in the following constraints. If not, I'm afraid that we need to figure out a way to get the bool expression of X_is_zero, like I used in code.

Inverse and Square roots

For inverse and square root, I think it's easier to check relation than to compute the result, right?

I guess you store it because you have some space in the advice columns, and so it is more efficient?

That's what we discussed in Slack. "It seems that we can’t use long poly expression in one single constraint? The performance goes back to normal when I divided the long expression" The commit 1e1e8ba is to fix the performance problem, although I haven't dived into the basic cause

[vveiln]

Didn't look properly at the PR yet, but it seems there is a typo and derivate_value_base should be derive_value_base

[joebebel]

Didn't look properly at the PR yet, but it seems there is a typo and derivate_value_base should be derive_value_base

I had to look it up in the dictionary since I didn't remember, but yes, derive is the action and derivate is the result.