Refactor and simplify the design of traits #189
+1,336
−2,082
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current codebase has pretty complicated trait bounds / type parameters, and this is mostly due to the requirement of types that are more specific / powerful than the ones provided by arkworks.
For example, when using
CurveGroup
, we often need itsBaseField
to be aPrimeField
, itsScalarField
andBaseField
to beAbsorb
able, and an in-circuit typeCurveVar
to be associated with the curve.This PR aims to simplify the interfaces of complex traits, thereby improving the DX of sonobe before the incoming release. Specifically:
SonobeField
andSonobeCurve
, are introduced, in order to remove type parameters likeGC{1, 2}: CurveVar
and trait bounds likeAbsorb
andPrimeField
.AbsorbNonNative
are adjusted in order to fixto_native_sponge_field_elements
cant be called with PrimeField #170.to_constraint_field
to obtain the hash preimage that corresponds to an in-circuit variable,to_{native_}sponge_field_elements
is now preferred because it is semantically more meaningful.(Note that we cannot eliminate
to_constraint_field
calls forCurveGroup
because arkworks does not implementto_sponge_field_elements
for it, and it is also impossible for us to do the implementation as both traits are foreign.)Inputize
trait was originally added to replace arkworks'ToConstraintField::to_field_elements
, since we have too many methods similar to the latter, whose naming is also less clearer than the former.Now, this PR separates the "inputization" of non-native variables from native variables, resulting in a new trait
InputizeNonNative
. The benefit is that. we can now simultaneously handle types with two representations in-circuit (e.g.,CurveGroup
whose in-circuit version can be eitherCurveVar
orNonNativeAffineVar
).Furthermore,
Inputize
for a typeT
is now guaranteed to return underlying field elements in the same order as howT::Var
is allocated in-circuit. In comparison, arkworks' implementation ofToConstraintField::to_field_elements
for EC group elements (which essentially returnsx
,y
,infinity
of the affine form) in fact does not match the allocation of in-circuit EC variables (which is handled in the order ofx
,y
,z
of the projective form).ToEth
trait for arkworks types, so that we can avoid verbosity when implementingprepare_calldata
for on-chain deciders for HyperNova, ProtoGalaxy, etc. in the future.Thanks in advance for reviewing this (yet another) huge PR. It would be great to hear your feedback!