[AIP-12] Improving the ProjectRegistry

[0xKurt]

I would like to initiate a discussion to improve the Registry smart contract, focusing on three key aspects. These improvements aim to enhance the contract's functionality, security, and ease of use. Let's delve into each area of improvement:

1. Use OpenZeppelin's Access Control Library:
One suggestion is to consider incorporating OpenZeppelin's Access Control library into the Registry contract. This library provides a standardized and audited implementation for access control functionalities. By leveraging this library, we can streamline the management of project ownership and permissions within the contract. It offers a robust and well-tested solution, which can enhance the security and reliability of the contract.

2. Use of Unique Identifiers:
Currently, the contract assigns projectIDs as identifiers in a simple incrementing number fashion. I propose using unique identifiers generated by the createProjectIdentifier function throughout the contract, replacing the current projectIDs.
This change simplifies the lookup process within the registry and ensures consistency across our contracts.
By employing unique identifiers, we eliminate the need to know the actual incremented projectID when accessing projects. Additionally, since we already use unique identifiers for round applications, extending their use to the registry improves usability. The createProjectIdentifier function generates a secure and deterministic identifier based on the registry's address, chainId, and projectID. By embracing this change, we streamline project management and enhance the overall efficiency of the Registry contract.

function createProjectIdentifier(
        uint256 projectID
    ) internal view returns (bytes32) {
        return
            keccak256(
                abi.encodePacked(block.chainid, address(this), projectID)
            );
    }

3. Implement Error Messaging:
Another area of improvement is to provide clear and informative error messages using revert statements. Clear error messages are essential for users to understand why a transaction failed. By incorporating descriptive error messages, we can enhance the contract's usability and debugging experience. Users will be able to identify and rectify issues more efficiently, leading to a smoother interaction with the contract.

[gravityblast]

awesome thank you for your work @KurtMerbeth!

I'm completely in with 2 and 3!!

For 1 (Use OpenZeppelin's Access Control Library ) isn't it better to wait for a use case on this?
Since the current implementation is the same of the Gnosis Safe contract where is well tested and audited, I wouldn't change it
just to change the library, unless we want already to introduce different roles.
But I see we will still have 1 single role for each project.

[nfrgosselin]

+1 thank you @KurtMerbeth!

I agree with @gravityblast here — I think 2 and 3 are no-brainers for right now.

With 1, we will 100% want to introduce more sophisticated roles on the project registry, but my preference is to think through project roles holistically and aim for a separate AIP focused on those changes.

[thelostone-mc]

I'd recommend sticking with OpenZeppelin's Access Control Library so that we can reuse the audited library.
We are already using this in the Round and AlloSetting contract and hence keep design consistent.
Curious about what the downfall of using OpenZeppelin's Access Control @nfrgosselin + @gravityblast

We will 100% want to introduce more sophisticated roles on the project registry,

Additionally, if we wanted to do this ( I hadn't accounted for this when I had read this discussion)
It would be quite painful to remove the custom gnosis code for upgradability
Plus the code does become a lot easier to understand + read.

[nfrgosselin]

This pitch has been assigned AIP-12 — initial PR is here

[nfrgosselin]

This PR has been merged and the discussion is now closed.