Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

query slotIndex discrepancy between prepareMerklizedQuery and ZKPVerifier smart contract #113

Closed
refi93 opened this issue Aug 1, 2023 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@refi93
Copy link

refi93 commented Aug 1, 2023

Describe the bug
Not sure if a bug, but it has been causing me a query hash mismatch when trying to do the onchain verification based on https://github.com/0xPolygonID/tutorial-examples/tree/main/on-chain-verification

As can be seen from the ZKPVerifier smartcontract used to submit proof requests and verify proofs, the logic that computes the query hash of the proof request has slotIndex hardcoded to 0

https://github.com/0xPolygonID/tutorial-examples/blob/6052cdff6ecdb9b0fe5a297bace7c81b1d03c95f/on-chain-verification/contracts/verifiers/ZKPVerifier.sol#L67

whereas the internal logic of generateProof in the sdk sets the slotIndex either to 2 or 5, which results in a query hash mismatch when trying to submit the proof generated by this function with the AtomicQuerySigV2OnChain circuit

Can you clarify whether this mismatch is intentional? I see earlier iterations of the verifier contract actually allowing to set the slotIndex from outside, so perhaps the generateProof logic just wasn't updated accordingly: https://github.com/codingwithmanny/polygonid-on-chain-verification/blob/211e82ce87f7092d6d8354342593c2c0e24fa9ba/contracts/verifiers/ZKPVerifier.sol#L85

To Reproduce
I managed to reproduce the issue with the following repo where I changed the circuit for AtomicQuerySigV2OnChain in the test fixture kycAgeProofReqSig:

https://github.com/joelamouche/polygon-getProof-test/blob/e93a63dc90bed5020e4d931ed7fa0018add66c67/test/testKYCAgeConstants.ts#L9

which then prints the following proof:

PROOF {
  id: 1,
  circuitId: 'credentialAtomicQuerySigV2OnChain',
  vp: undefined,
  proof: {
    pi_a: [
      '2995257496452039333266672924452187534763353752939957567102513085747788056295',
      '21286006883968543074991929294434532782604772468016390800644084899238403477549',
      '1'
    ],
    pi_b: [ [Array], [Array], [Array] ],
    pi_c: [
      '12992267931549799732997873889763461410538603021736683007685059555543640525061',
      '10376645054694296247228888253097728383852541257117749196283927000898226221711',
      '1'
    ],
    protocol: 'groth16',
    curve: 'bn128'
  },
  pub_signals: [
    '1',
    '23038777784306927280253778898804444169402415964080183736378253638384095746',
    '876854063388751259451046665973179579453696431476710960571891857639659962126',
    '5104537430913137961433130810052482860380035584288621588445755171025855431969',
    '1',
    '47',
    '7035233703809473779787447569724895380693518150459779007479517532546159350854',
    '23169466740316234897070481169520339914935095224269608384492847637518356994',
    '0',
    '5104537430913137961433130810052482860380035584288621588445755171025855431969',
    '1690911471'
  ]
}

note the 876854063388751259451046665973179579453696431476710960571891857639659962126 which is the (presumably wrong) query hash. As soon as I hacked the polygon js sdk to return slotIndex 0 instead of 2/5, the returned query hash changed to 15045271939084694661437431358729281571840804299863053791890179002991342242959 which seems to match successful executions of the verification on chain, e.g. https://mumbai.polygonscan.com/tx/0x477a5df34d390fdd0a0e4f46526dad078292ffb48e248f1c51c1b5efc049d5f6

Expected behavior
slotIndex should be 0 as per the latest ZKPVerifier contract, or perhaps overridable from outside

Screenshots
If applicable, add screenshots to help explain your problem.

** Environment info (please complete the following information):**

  • OS version [e.g. Mac OS]
  • Node version: [e.g. 18.16]
  • Browser [e.g. chrome, safari]
  • Package version [e.g. 1.0.0]
  • Build [e.g. umd, cjs]

Additional context
Add any other context about the problem here.

@refi93 refi93 added the bug Something isn't working label Aug 1, 2023
@vmidyllic
Copy link
Collaborator

thank you @refi93, this is a very nice catch.
It looks like a contradiction between the choice of the value of the slot index in the sdk and ZKPVerifier latest contract.

We will deeply investigate it and fix it asap.

@vmidyllic
Copy link
Collaborator

Fixed in #114 . For merklized credentials, slotIndex in the query must be equal to zero and not a position of merklization root.
It has no influence on check in the off-chain circuits, but it aligns with on-chain verification standard

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants