End-to-end "pp-spartan" verification contract (Step 3) #27
Conversation
| @@ -5,53 +5,14 @@ import "src/blocks/pasta/Vesta.sol"; | |||
| import "src/blocks/pasta/Pallas.sol"; | |||
|
|
|||
| library EqPolinomialLib { | |||
| function evaluateVesta(uint256[] memory r, uint256[] memory rx) public pure returns (uint256) { | |||
There was a problem hiding this comment.
Is it worth doing a similar refactor for evalsPallas and evalsVesta?
There was a problem hiding this comment.
Agree. One thing to note: this kind of evaluation is not used in e2e verification "pp-spartan" flow which is default - as far as I remember it was used in "regular" spartan. I guess it is a normal situation when we have building block which is not used currently but either used previously or may be can be used in future.
src/blocks/Sumcheck.sol
Outdated
| @@ -40,7 +40,7 @@ library PrimarySumcheck { | |||
| uint256 r_i; | |||
| label[0] = 99; // c_label[0] = 99; | |||
| (transcript, r_i) = KeccakTranscriptLib.squeeze(transcript, ScalarFromUniformLib.curveVesta(), label); | |||
|
|
|||
| r_i = Field.reverse256(r_i); | |||
There was a problem hiding this comment.
It looks like in the definition of squeeze you call Field.reverse256 before outputting r_i Unless I'm misunderstanding, could you delete both instances of Field.reverse256 and arrive at the same result?
There was a problem hiding this comment.
nice catch! Thanks
src/blocks/Sumcheck.sol
Outdated
| @@ -87,7 +87,7 @@ library SecondarySumcheck { | |||
| uint256 r_i; | |||
| label[0] = 99; // c_label[0] = 99; | |||
| (transcript, r_i) = KeccakTranscriptLib.squeeze(transcript, ScalarFromUniformLib.curvePallas(), label); | |||
|
|
|||
| r_i = Field.reverse256(r_i); | |||
| @@ -81,7 +81,9 @@ contract KeccakTranscriptContractTest is Test { | |||
| assertEq(scalarVesta, expectedVesta); | |||
| } | |||
|
|
|||
| function testKeccakTranscriptPallas() public { | |||
| // Following test has been ported: https://github.com/lurk-lab/Nova/blob/solidity-verifier-pp-spartan/src/provider/keccak.rs#L138 | |||
| // TODO: For some reason, test vector passes if using Vesta curve parameters, but in reference implementation type of point is Pallas | |||
There was a problem hiding this comment.
Is the TODO still necessary, or did we figure out the Pallas <-> Vesta mixup?
There was a problem hiding this comment.
I think it is still not 100% clear. I mean following test in reference is parameterised by Pallas point. But given test vector outcome is obtained when we use different curve (Vesta) parameterisation at Solidity side. May be my understanding of reference is not 100% correct... @huitseeker, any thoughts?
There was a problem hiding this comment.
I think the situation is a bit confusing for the following reason:
- when Rust tests are parametrized by a point, it can mean that we are using the G parameter because a/ we're indeed digesting a point OR b/ we're using that parameter to link to its associated types, such as
G::Scalar, - when we digest a point in the Nova protocol we are in general calling the function
to_coordinatesfrom the traitGroup, which returns the components of the affine representation of the point. As specified, those coordinates are members of the base field of the point:
https://github.com/microsoft/Nova/blob/200b47edab190c06a44cbd87a63d34a444184a41/src/traits/mod.rs#L79
In the case of a Pallas Point, this will be two coordinates in the Vesta scalar field. - often in tests, we also absorb points from
G::Scalar, which means field elements in the point's own associated scalar field. In the case of a Pallas Point, that's a field element from the Pallas scalar field.
* End-to-end "pp-spartan" verification contract (#25) * Add JSONs generated by pp-spartan * Separate path (src/blocks/) for low-level crypto building blocks * Add e2e verification contract skeleton * Move FieldLib and PolyLib code to Utilities.sol file * Step 1 * Step 2 * CI adjustments * Update README * Update integration testing infrastructure * End-to-end "pp-spartan" verification contract (Step 3) (#27) * Add PolyEvalInstance building block * Refactor building blocks * Add step 3 of verification to the e2e contract * Remove data contracts for step 2 and step 3 * Adjust CI * KeccakTranscript and Sumcheck blocks adjustments * Requested changes * Update Utilities.sol --------- Co-authored-by: Artem Storozhuk <[email protected]>
This PR follows #25 by adding next subsequent part of e2e verification to the main contract. Step 3 actually involves computations of "inner", "outer", "set" and "mem" claims required for comparison. This PR merges Sumcheck block implementation written by @mpenciak , adds PolyEvalInstance block, updates KeccakTranscript block according to recent changes (microsoft/Nova#191) by @huitseeker and carefully integrates all these to the codebase in main branch