OpenZeppelin / openzeppelin-labs

A space for the community to interact and exchange ideas on the OpenZeppelin platform. Do not use in production!
https://openzeppelin.com/
MIT License
376 stars 116 forks source link

Package registry PoC #131

Open facuspagnuolo opened 5 years ago

facuspagnuolo commented 5 years ago

Fixes https://github.com/zeppelinos/labs/issues/132

spalladino commented 5 years ago

Looks great @facuspagnuolo! I've yet to understand better the standards around ENS to comment in-depth about the PR, but the architecture looks good. I like having a base registrar, with different "interfaces" depending on the access control for zOS or aOS (though maybe we could switch from Ownable to RBAC down the road).

I have one question pending regarding ENS: can we be sure that, under the new registrar to be deployed next year, there is no way for the domain backing the package registry to be lost? For instance, if we register thebestpackageregistry.eth, and we lose it a year after due to a higher bid, or someone forgetting to pay for the renewal, we are dealing with quite a single point of failure. From what I understand though, domain name assignments are to be permanent, but I wanted to check.


Moving on to a different subject, which is the Registry contract. I different ways of working here:

1- We have an APMRegistry and a ZOSRegistry, both entirely different contracts, but they are both backed by the same Registrar, tied to the same domain. This would allow both organizations, at first, to share the domain where packages are registered, and then move forward by integrating the subsequent contracts.

2- We aim at a single Registry contract that covers the use cases for both. We can extend the APMRegistry contract to have registerRepo methods, that will allow any kind of contract to be registered, including zOS Packages. This would remove the need for having two different contracts writing to the same Registrar, and we could actually upgrade the APMRegistry to this SharedRegistry contract.

Either way, this opens the door for arbitrary contracts to be registered in the registry, not just valid Repos or Packages. We should keep a centralised control to remove malicious registry entries during the beta phase.


Another topic is the Repo itself, which tracks different versions of a lib, and is the aOS equivalent of zOS' Package. Regardless of its validations, its heavily tied to semver, and we should decide whether we want zOS Package to adopt that naming scheme as well.

1- If we modify Package to work only under semver (which I'd not recommend to do now, but later down the road), then we could have a similar public interface to Repo, even if we don't enforce the same validations.

2- Otherwise, we accept the fact that, during the initial beta period, there will be two different kind of animals registered in the same registry, which have different interfaces.

Note that (2) means that a lib developer who wants to develop for both aOS and zOS would need to register two different packages under two different names, at least during this beta phase. On the other hand, even if we do implement a shared interface, then the expected contractAddress linked for a version of the Repo/Package would also need to be different, as it'd be an ImplementationProvider for zOS, and an AragonApp for aOS, until we figure out a common interface at that level for both OSs (which is not that far-fetched, actually).

luisivan commented 5 years ago

Nice work @facuspagnuolo! I like the simplification work you have done. I have a question: where is the Registry contract? I guess it is still on the works, but right now there would be no way to push new Repos/Packages to the registry. I see a mention of ZOSRegistry but didn't find it anywhere.

Other points I wanted to touch upon:

Rich governance

I see most of the logic has been taken from APM, but without the auth modifiers for newVersion and newRepo. I think it is important to have rich governance built in into the package manager, and that's why we made APM a DAO from the start. A decentralized package manager run by a single entity doesn't make sense, since that'd be centralized and there are already plenty of centralized package managers out there. I think the package manager being a DAO is key for rich governance, since it makes it dead easy. For example, in Zeppelin's case, if the package manager was a DAO, you could deploy a registry controlled by ZEP holders very easily in X steps:

  1. Deploy the package manager DAO
  2. Deploy the Voting app in Aragon, and give permission to the ZEP holders to cast votes
  3. Give permission to the Voting app to call newVersion and newRepo

With just those 3 steps, we give ZEP token holders right to vote on new versions and repos inside the registry. You could get fancier and create new Token Manager instances for different groups of stakeholders, different Voting apps with different consensus thresholds for different repos, etc. This wouldn't be possible if the package manager wasn't a DAO. We recently wrote a blog post about this.

Upgradeability of the repos

Right now, the contracts for the repo and the registry are not upgradeable, right? I think they should be (no need to argue why upgradeability is important between us haha). Also one of the cool things you get for free if the package manager is a DAO is that you can upgrade all repos in a registry. Otherwise, you'd need to upgrade each one individually. But if it's a DAO, you can just upgrade the reference to the Repo app in the kernel, and all repos in a registry are automatically upgraded.

Repo vs Package naming

Although Package is more intuitive (you push new versions of a package) the technically correct term would be Repo (you push a package to a repo). I guess that calling it Package means that you push new versions to a package, but the new versions pushed packages themselves, so you cannot push a package to a package. That's why I think that, although not as sexy, Repo is the most accurate term. It's also used by package managers in Linux, etc. That being said Package feels more friendly, so I don't have hard feelings, but wanted to point out that question about naming.


That's it! Solid work on this, looking forward to your thoughts.

facuspagnuolo commented 5 years ago

Hey @luisivan! thanks a lot for your feedback! 😄

Regarding the Registry contract, it would be simple to have some similar contract to the ENSRegistry that acts as a registry for us. But that would mean that either you guys duplicate the effort of publishing Repos in this new Registry given that you're already using ENS, or that you stop using ENS to register Repos and start using the new Registry. Based on what I spoke with @izqui you guys are interested in keeping the ENS integration, so we decided to focus simply on what will be the common interface of Package/Repos that fit for both projects.

Governance

I really like the idea of decentrilizing the governance of the Package/Repos, do you think it should be part of the minimum interface?

Upgradeability of the Repos

AFAIK, given that a Aragon's Repo are an AragonApp I think those are upgradeable. Regarding Zeppelin's Package I haven't tested that in this PoC, but it is on our plans. I do think both should be upgradeable, but I think it shouldn't be part of the standard, do you agree?

Repo vs Package naming

I like your thoughts on this topic :) I'll discuss this with the rest of the team and let you guys know

luisivan commented 5 years ago

Got it, makes sense, thanks for the clarification @facuspagnuolo!

Regarding governance and upgradeability, I think it doesn't need to be a part of the interface, but a part of any of the implementations.

The minimum viable path is to work on a common interface and spec, which I think we are doing a good job at. But ideally we would also work on a canonical implementation that we can all use and build upon. For the spec and interface, we don't need to get into governance or upgradeability, but for the implementation, I think it's very necessary.