AztecProtocol / aztec-verifier-contracts

28 stars 8 forks source link

verify function visibility should be public to enable inheritance #24

Open Turupawn opened 1 year ago

Turupawn commented 1 year ago

Hello, I'm writing educational content about Noir in spanish and @critesjosh has been guiding me on the process. I stumbled into following issue while writing a guide on how to create a dApp. I got redirected here from acvm-backend-barretenberg.

I did a PR #23 that solves this. Happy to hear your comments.

The problem

It should be common for devs to inherit from UltraVerifier while creating a dApp. This is to keep the codebase organized and readable. A normal implementation should look like the following example:

//SPDX-License-Identifier: MIT
pragma solidity >=0.8.19;

contract MyDApp is UltraVerifier {
    // Custom logic
    function proveStuff(bytes calldata _proof, bytes32[] calldata _publicInputs) public {
        verify(_proof, _publicInputs));
        // More custom logic
    }
}

However this is currently not possible due to the verify function being external. This means that this function is not visible to inherited contracts.

The solution

I think the verify visibility should be changed to public. This will enable verify being called from inherited contracts like the example above.

Maddiaa0 commented 1 year ago

As mentioned here: https://github.com/AztecProtocol/aztec-verifier-contracts/pull/23#issuecomment-1544299197 we purposely have the verifier calls as external library calls to ensure that they are writing into clean memory.

The solution should be to swap the contract syntax to be a library instead, we will get this updated within noir

The resulting syntax when the verifier is a library should be something like this:


import {UltraVerifier} from "...path...";

contract MyDApp  {
    // Custom logic
    function proveStuff(bytes calldata _proof, bytes32[] calldata _publicInputs) public {
        UltraVerifier.verify(_proof, _publicInputs));
        // More custom logic
    }
}
Turupawn commented 1 year ago

Noted :memo: , yeah a library would be nice. I'll stick with interfaces for the mean time. Thanks for the explanation!