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

The erc20VerifierFlowV2() in the testcase seems to not actually execute the Cross Chain State Proof validation logic on the contract side. #58

Open
yushihang opened this issue Jan 8, 2025 · 2 comments

Comments

@yushihang
Copy link

yushihang commented Jan 8, 2025

My testing method is as follows.

  • Run tests
node -v 
#v22.12.0

git clone git@github.com:0xPolygonID/contracts.git
cd contracts

npm i
npm i @iden3/contracts

npx hardhat test test/verifiers/erc20-with-universal-verifier.ts

The testcase can run successfully.

    ✔ Requests count
    ✔ Example ERC20 Verifier: set zkp request Sig validator + submit zkp response (103ms)
    ✔ Example ERC20 Verifier: set zkp request Mtp validator + submit zkp response (103ms)
    ✔ Example ERC20 Verifier: set zkp request Sig validator + submit zkp response V2 (97ms)
    ✔ Example ERC20 Verifier: set zkp request Mtp validator + submit zkp response V2 (99ms)


  5 passing (2s)
  • Modify the value of the Cross Chain State Proof in the testcase (in a real environment, this data should be obtained from the oracle).

https://github.com/0xPolygonID/contracts/blob/f523723a190f5718291b9792c27fef22cc46cb2a/test/verifiers/erc20-with-universal-verifier.ts#L185C1-L204C7

    const globalStateMessage = {
      timestamp: BigInt(Math.floor(Date.now() / 1000)),
      idType: '0x01A1',
      root: 0n,
      replacedAtTimestamp: 0n
    };

    const identityStateMessage1 = {
      timestamp: BigInt(Math.floor(Date.now() / 1000)),
      id: 25530185136167283063987925153802803371825564143650291260157676786685420033n,
      state: 4595702004868323299100310062178085028712435650290319955390778053863052230284n,
      replacedAtTimestamp: 0n
    };

    const identityStateUpdate2 = {
      timestamp: BigInt(Math.floor(Date.now() / 1000)),
      id: 25530185136167283063987925153802803371825564143650291260157676786685420033n,
      state: 16775015541053109108201708100382933592407720757224325883910784163897594100403n,
      replacedAtTimestamp: 1724858009n
    };

After I modify the values of these states or IDs, the testcase can still run successfully. But as I understand it, they should not pass the contract's logic check.

  • The log printing on the smart contract side.

By importing hardhat/console.sol on the smart contract side and using console.log to output logs, I found that when the testcase is executed, the function does not reach the Cross Chain Proof validation logic.

https://github.com/iden3/contracts/blob/9d4e443b5f1144c8d00133df57eb72066ea3f884/contracts/state/State.sol#L460C1-L480C6

    /**
     * @dev Retrieve the timestamp when the GIST root was replaced by another root.
     * @param idType Id type
     * @param root GIST root
     * @return replacedAt The timestamp when the GIST root was replaced by another root
     */
    function getGistRootReplacedAt(bytes2 idType, uint256 root) external view returns (uint256) {
        if (isIdTypeSupported(idType)) {
            if (_gistData.rootExists(root)) {
                return _gistData.getRootInfo(root).replacedAtTimestamp;
            }
            revert("GIST root entry not found");
        } else {
            StateCrossChainStorage storage $ = _getStateCrossChainStorage();
            uint256 replacedAt = $._rootToGistRootReplacedAt[idType][root];
            if (replacedAt != 0) {
                return replacedAt;
            }
            revert("Cross-chain GIST root not found");
        }
    }
  • I tried setting up my own oracle using https://github.com/0xPolygonID/driver-did-polygonid and used VP + Cross Chain State Proof data associated with the Polygon Amoy network. I successfully ran these test cases and confirmed that the smart contract also executed the cross-chain validation logic.
    Thank you for your hard work and for sharing your open-source code

However, I am not completely sure if my understanding of the original data in the testcase is correct, so I would like to consult with you.

@AndriianChestnykh
Copy link
Contributor

@yushihang thank you for the request and I'm grateful that you provided the full context of the problem which does not demand any additional questions.
It can be reproduced from my side also. Investigating...

@daveroga
Copy link
Contributor

daveroga commented Jan 13, 2025

@yushihang we have reviewed the issue and due to some security update in State contract, identities with the same idType as the ones supported in State contract are being checked only with the Gist data that is in the State contract instead of checking the cross-chain proof data provided.

This is produced in this line of code:
https://github.com/iden3/contracts/blob/95545f996f2b4b0ee4277ade509a421eccf83122/contracts/state/State.sol#L467

In the test you mention we are providing proofs in some json files that are generated from a user that is in one of the supported IdTypes of the State contract deployed in the test network.

? require('./common-data/valid_sig_user_non_genesis_challenge_address.json')

As a result of this we will always check the Gist data to this State contract instead of checking the cross-chain proof.

We will fix this test with some positive and negative cross-chain proof submission from a different user of different idType so it can be tested properly and let you know.

Thanks.

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

4 participants
@AndriianChestnykh @yushihang @daveroga and others