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

skipClaimRevocationCheck is defined in CredentialAtomicQueryV3 but not used? #248

Open
yushihang opened this issue Jul 5, 2024 · 5 comments
Assignees

Comments

@yushihang
Copy link

  1. skipClaimRevocationCheck is not used in CredentialAtomicQueryV3Validator

    should it be verifier with isRevocationChecked together?

  2. Or should skipClaimRevocationCheck be removed from struct CredentialAtomicQueryV3

@yushihang yushihang changed the title skipClaimRevocationCheck is defined in CredentialAtomicQueryV3 but not used? skipClaimRevocationCheck is defined in CredentialAtomicQueryV3 but not used? Jul 5, 2024
@AndriianChestnykh
Copy link
Collaborator

Thanks for the question @yushihang

isRevocationChecked and skipClaimRevocationCheck can't be used together, because of different logic.
As it is stated in the V3 circuit code

    // check issuer non revocation state only if we need it:
    // 1. if Sig proof is provided we need to check non revocation of authClaim always
    // AND non revocation of issuerClaim only if isRevocationChecked = 1
    // 2. if MTP proof is provided we need to check non revocation of claim only if isRevocationChecked = 1

However, the skipClaimRevocationCheck can't be removed from struct CredentialAtomicQueryV3 as in case of MTP and isRevocationChecked == false (it's public but need to set it to _setInputToIndex("isRevocationChecked", <input index>);) in the circuit, the call to the _checkClaimNonRevState() do can be omitted.

@AndriianChestnykh
Copy link
Collaborator

The above is something for us to check and fix.

@yushihang
Copy link
Author

However, the skipClaimRevocationCheck can't be removed from struct CredentialAtomicQueryV3 as in case of MTP and isRevocationChecked == false

@AndriianChestnykh Thanks for your explanation,I am not sure that is there some judgement logic about skipClaimRevocationCheck in solidity code for v3? I found some code in v2 version, but not found in v3.

https://github.com/iden3/contracts/blob/07b28d7cdfe54770169ec8a213d92670085f3e49/contracts/validators/CredentialAtomicQueryV2ValidatorBase.sol#L104C1-L104C40

@vmidyllic
Copy link
Contributor

isRevocationChecked is included to the query hash for v3 circuit. If user didn't do revocation check for proof generation it query hash will be not the same.
https://github.com/iden3/circuits/blob/develop/circuits/lib/utils/queryHash.circom

@AndriianChestnykh
Copy link
Collaborator

Thanks @vmidyllic. Yes, and in V2 it is not included into the query hash, that's why we are checking it outside ZK Verifier there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants