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

Feature: transparency updates #2

Merged
merged 3 commits into from
Sep 13, 2024
Merged

Feature: transparency updates #2

merged 3 commits into from
Sep 13, 2024

Conversation

bezze
Copy link

@bezze bezze commented Sep 10, 2024

I've been following summa-dev/summa-solvency#299 and figured a good approach would be to first make the verifying key accessible. I modified the contract template to support a new method getVerifyingKey (which can optionally take a contract address in the decoupled contracts architecture).

After this I envision a simple cli tool (probably JS or wasm for browser compatibility) that calculates the digest using

fn vk_transcript_repr(vk: &VerifyingKey<bn256::G1Affine>) -> bn256::Fr {

Summary of changes:

  • Re-factoring verifying key into constants in the Halo2Verifier
  • Added getVerifyingKey() method, which returns vkey parameters
  • Added auxiliary askama formatting methods

* Re-factoring verifying key into constants in the Halo2Verifier
* Added getVerifyingKey() method, which returns vkey parameters
* Added auxiliary askama formatting methods
@jkktom
Copy link

jkktom commented Sep 12, 2024

Hi, This is a very interesting approach. As I was also looking into the same issue, I would like to fully understand your approach.
So to Summarize your solution is, making vk visible by view function in solidity, and then, users can verify themselves by manually (1:1 comparison) or simple CLI tools?

Copy link
Member

@sifnoc sifnoc left a comment

Choose a reason for hiding this comment

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

This is a good approach compared to using a universal reading contract, as mentioned in summa-dev/summa-solvency#301 (comment).
However, the vk data should not be duplicated to avoid increasing the contract size. More importantly, it's crucial to prevent malicious actions, such as a user receiving different vk data through the getVerifyingKey method than what is stored in the verifyProof function.

templates/Halo2Verifier.sol Show resolved Hide resolved
templates/Halo2Verifier.sol Outdated Show resolved Hide resolved
templates/Halo2Verifier.sol Outdated Show resolved Hide resolved
@bezze
Copy link
Author

bezze commented Sep 12, 2024

I'll experiment with the formatting as I think there are some rust versioning issues. I managed to run the examples using rust-toolchain "nightly-2023-07-11", which I think I took from the zk-prover repo, who is the one that uses this library internally. I will also try running the zk-prover with my changes.

I agree that the value should not be duplicated, and I took care in following that. Allow me to explain.

In the single contract approach the key parameters are defined as constants and then referred from the

  • view key function:
    function getVerifyingKey() public view returns (uint256[{{ vk_len / 0x20 }}] memory) {
    return [
    {%- for (name, chunk) in vk_.constants %}
    {{ self::vk_var_name(name)|left_pad(24) }},
    {%- endfor %}
    {%- for (x, y) in vk_.fixed_comms %}
    {{ self::index_coord("fixed_comms", loop.index0, "x")|left_pad(24) }},
    {{ self::index_coord("fixed_comms", loop.index0, "y")|left_pad(24) }},
    {%- endfor %}
    {%- for (x, y) in vk_.permutation_comms %}
    {{ self::index_coord("permutation_comms", loop.index0, "x")|left_pad(24) }},
    {{ self::index_coord("permutation_comms", loop.index0, "y")|left_pad(24) }}{%- if loop.index < vk_.permutation_comms.len() %},{%- endif %}
    {%- endfor %}
    ];
    }
  • verifier function:
    {%- for (name, chunk) in vk.constants %}
    mstore({{ vk_mptr + loop.index0 }}, {{ self::vk_var_name(name)|left_pad(24) }}) // {{ name }}
    {%- endfor %}
    {%- for (x, y) in vk.fixed_comms %}
    {%- let offset = vk.constants.len() %}
    mstore({{ vk_mptr + offset + 2 * loop.index0 }}, {{ self::index_coord("fixed_comms", loop.index0, "x")|left_pad(24) }}) // fixed_comms[{{ loop.index0 }}].x
    mstore({{ vk_mptr + offset + 2 * loop.index0 + 1 }}, {{ self::index_coord("fixed_comms", loop.index0, "y")|left_pad(24) }}) // fixed_comms[{{ loop.index0 }}].y
    {%- endfor %}
    {%- for (x, y) in vk.permutation_comms %}
    {%- let offset = vk.constants.len() + 2 * vk.fixed_comms.len() %}
    mstore({{ vk_mptr + offset + 2 * loop.index0 }}, {{ self::index_coord("permutation_comms", loop.index0, "x")|left_pad(24) }}) // permutation_comms[{{ loop.index0 }}].x
    mstore({{ vk_mptr + offset + 2 * loop.index0 + 1 }}, {{ self::index_coord("permutation_comms", loop.index0, "y")|left_pad(24) }}) // permutation_comms[{{ loop.index0 }}].y

When the verification key is in a separated contract, there are no new constant definitions

@bezze
Copy link
Author

bezze commented Sep 12, 2024

Hi, This is a very interesting approach. As I was also looking into the same issue, I would like to fully understand your approach. So to Summarize your solution is, making vk visible by view function in solidity, and then, users can verify themselves by manually (1:1 comparison) or simple CLI tools?

Yes, then users would simply get the embedded keys by calling the contract, and check if the digest matches the one from executing the zk-prover. I'm not sure how to convince the user without them having to recompile the circuits.

@bezze
Copy link
Author

bezze commented Sep 12, 2024

I'll experiment with the formatting as I think there are some rust versioning issues. I managed to run the examples using rust-toolchain "nightly-2023-07-11", which I think I took from the zk-prover repo, who is the one that uses this library internally. I will also try running the zk-prover with my changes.

Ok, I performed some checks following these steps:

  1. Went to the zk_prover directory, replaced the halo2-solidity-verifier dependency with a local path to my fork.
  2. Ran (successfully) the examples: gen_commitment, gen_inclusion_verifier, gen_inclusion_proof.
  3. The result of the previous step produces commitment_solidity_calldata.json, inclusion_proof_solidity_calldata.json and the ../contracts/src/InclusionVerifier.sol with the PR changes.
  4. Went to the contracts directory and ran the hardhat tests (successfully).

@sifnoc
Copy link
Member

sifnoc commented Sep 13, 2024

I'm not sure how to convince the user without them having to recompile the circuits.

It will be handled by the issue assigners at summa-dev/summa-solvency#299.

Great job! I verified that it worked perfectly in the V2 implementation as well. I will merge your PR in 5 mins.

@sifnoc sifnoc merged commit e6833d1 into summa-dev:main Sep 13, 2024
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.

3 participants