AztecProtocol / barretenberg

Apache License 2.0
143 stars 93 forks source link

Refactor(pedantic): Can we rename points? #355

Open iAmMichaelConnor opened 1 year ago

iAmMichaelConnor commented 1 year ago

"Element" isn't a clear enough word, imo.

affine_element -> affine_point element -> jacobian_point (if I recall correctly)

And should we have a dinstinct class for when a point is in montgomery form, to make it clear? (Maybe we already do... I'm doing a lot of uninformed backseat driving by opening this issue, sorry).

(We could even go mad and give the names PascalCase).

kevaundray commented 1 year ago

Montgomery form would be for curves which have an Edwards form, so it would not apply for bn254 or grumkin.

Maybe you meant when the point's co-ordinates are in montgomery form, if so then I think we should not expose whether a field element is in montgomery form as its an implementation detail that should not leak through to the proving system. This is because a field element could use barret reduction to implement modular arithmetic and then you need to change your proving system code because of a change you made in your field element class. The change here would be getting rid of the to_montgomery and from_montgomery methods.

I opened an issue on gnark-crypto here for the same problem.

I think I brought up the field elements exposing montgomery form with @codygunton or maybe it was @ledwards2225

kevaundray commented 1 year ago

For replacing Element I think it depends on the usecases we are using. I think thats what most other libraries do. ie the concrete struct is AffinePoint, JacobianPoint and then they have an interface called Element or GroupElement which AffinePoint and JacobianPoint satisfies.