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

Decide if we want to use Celo Registry contract #2

Closed
Tracked by #79 ...
karlb opened this issue Aug 1, 2023 · 3 comments
Closed
Tracked by #79 ...

Decide if we want to use Celo Registry contract #2

karlb opened this issue Aug 1, 2023 · 3 comments

Comments

@karlb
Copy link

karlb commented Aug 1, 2023

On Optimism Most contracts are proxied and the proxies are managed by the ProxyAdmin contract. In contrast to the Celo Registry contract, no names are given to the contracts which could be used to look up a specific contract. Instead, contracts are assigned a fixed address in the 0x42000000000000000000000000000000000000xx range. All addresses in this range are reserved for the OP-stack by deploying a proxy at each address within this range.

If we want to use OP’s ProxyAdmin instead of our CeloRegistry, we would at least have to provide a contract at the old Celo Registry’s address the resolves name lookups to the new fixed proxy addresses. Other points to check:

  • How different is the proxy setup? I haven’t seen an initialize method in OP contracts.
  • Is OP’s layout locking sufficient to allow compatible contract upgrades or do we need to port our checks?
  • What happens with the GoldToken? We can’t just move it to a new fixed address and geth relies on the registry to get the gold token’s address.
  • Certainly more… Somebody who knows in detail how contract updates are handled on Celo should refine this.
@pahor167
Copy link

pahor167 commented Aug 6, 2023

ProxyAdmin uses something called AddressManager to help name contracts and find them by name. When comparing our Registry with ProxyAdmin, a big difference is that ProxyAdmin can do things like upgrading and changing owners. For this to work, ProxyAdmin has to be the owner of all the contracts it's looking after.

There are three types of proxies that ProxyAdmin can work with: ERC1967, CHUGSPLASH, and RESOLVED. But none of these match our Proxy. If we decide to use ProxyAdmin, we'd have add support of our type of Proxy or implement their Proxy.sol and change the addresses of our main contract proxies.

  • Optimism's Proxies: Optimism uses proxy called Proxy.sol. It's a bit different from ours, but it does a lot of the same things. Unlike our Proxies, theirs doesn't use special initialize function but rather allow owner (admin) to call upgrade implementation and in same call also admin can choose to call whichever function on implementation smart contract.

  • Layout Locking: This is something that seems pretty similar to what we're already doing. If we decide to use their way of doing things, we'd need to take a good look at our current setup.

  • I believe this would be very risky move since lots of protocols most probably use hardcoded Celo address.

One more thing to think about is how we handle releases. Right now, we have tools that take care of upgrading our core contracts. These tools can compare different releases, create proposals, and do other things. If we decide to switch to Optimism's way of doing things, we'd have to make some changes to these tools.

@karlb
Copy link
Author

karlb commented Aug 7, 2023

There are three types of proxies that ProxyAdmin can work with: ERC1967, CHUGSPLASH, and RESOLVED. But none of these match our Proxy. If we decide to use ProxyAdmin, we'd have add support of our type of Proxy or implement their Proxy.sol and change the addresses of our main contract proxies.

We might be able to swap out the proxies when migrating the existing state to cel2. Not sure if this is a reasonable thing to do, but it should be possible and would allow us to keep the existing proxy addresses.

palango pushed a commit that referenced this issue Aug 16, 2023
@karlb
Copy link
Author

karlb commented Sep 19, 2023

Yes, we keep the original Celo registry.

@karlb karlb closed this as completed Sep 19, 2023
@carterqw2 carterqw2 mentioned this issue Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants