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

chore: minor adjustment to the Solidity verify() function signature and integration of Solidity verification with the CI #114

Merged
merged 14 commits into from
Feb 25, 2023

Conversation

weijiekoh
Copy link
Contributor

This PR does two main things:

  1. Change the ABI of the verify() function in Solidity to:
    function verify(
        uint256[] memory pubInputs,
        bytes memory proof
    ) public view returns (bool)

Previously, the size of pubInputs had to be specified. This change makes it easier for the CI tests to run as the number of inputs is not known at compile-time.

  1. Enable Solidity verification in the CI

The CI flow now includes verification of the generated Solidity verifiers for the models which can be verified in Solidity (the others create contracts that are too large to work).

@alexander-camuto alexander-camuto changed the title Minor adjustment to the Solidity verify() function signature and integration of Solidity verification with the CI chore: minor adjustment to the Solidity verify() function signature and integration of Solidity verification with the CI Feb 24, 2023
@alexander-camuto alexander-camuto marked this pull request as ready for review February 24, 2023 13:33
@alexander-camuto alexander-camuto self-requested a review February 24, 2023 13:34
Copy link
Collaborator

@alexander-camuto alexander-camuto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a few small comments regarding error handling, otherwise looks perfect !

src/execute.rs Outdated Show resolved Hide resolved
src/execute.rs Outdated Show resolved Hide resolved
src/execute.rs Outdated Show resolved Hide resolved
@alexander-camuto alexander-camuto self-requested a review February 25, 2023 11:34
Copy link
Collaborator

@alexander-camuto alexander-camuto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !

@alexander-camuto alexander-camuto merged commit 0a5d1aa into zkonduit:main Feb 25, 2023
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

Successfully merging this pull request may close these issues.

2 participants