Skip to content
This repository was archived by the owner on Oct 31, 2023. It is now read-only.

Change verify function visibility from external to public to enable inheritance#176

Closed
Turupawn wants to merge 1 commit intonoir-lang:masterfrom
Turupawn:master
Closed

Change verify function visibility from external to public to enable inheritance#176
Turupawn wants to merge 1 commit intonoir-lang:masterfrom
Turupawn:master

Conversation

@Turupawn
Copy link

@Turupawn Turupawn commented May 11, 2023

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.

@TomAFrench
Copy link
Member

Hi @Turupawn, the verifier contracts aren't actually developed/tested in this repository but in https://github.com/AztecProtocol/aztec-verifier-contracts and so we'd like to ensure that we don't introduce any changes relative to that repository. For that reason I'm going to close this PR as it stands.

However please feel free to open an issue/PR on that repository to discuss with the team which develops the verifier smart contracts. We'll then update to pull down any changes they make.

@TomAFrench TomAFrench closed this May 11, 2023
@TomAFrench
Copy link
Member

Sorry for any time wasted by this, I'll make a note in the readme about the repository for the verifier contracts.

@Turupawn
Copy link
Author

Thanks @TomAFrench , moved it there:
AztecProtocol/aztec-verifier-contracts#24
AztecProtocol/aztec-verifier-contracts#23

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants