Diamond vs UUPS

[rndquu]

Right now we use we diamond proxy pattern for most of our contracts to make them upgradable.

However we can't put all of our contracts in a single diamond because of selector clashing (i.e. diamond proxy supports only unique function names). So some of our contracts are not upgradable right now. This should be fixed.

We have 2 options:

1. UUPS
2. Create separate diamond contracts with a single facet for each of the contracts here

So UUPS is simpler to implement but we end up with code inconsistency when part of our contracts is implementing diamond and the other part is implementing UUPS. I would prefer consistency and use diamond for all of the contracts (i.e. for each of the contracts here create a separate diamond with a single facet).

@zgorizzo69 @0xcodercrane @molecula451 @pavlovcik

What do you think?

[0x4007]

The last time I discussed this with @zgorizzo69 I pointed out that we should have another solution for our token related contracts, so perhaps UUPS.

I think code consistency is nice but it's not clear to me how to use Diamond for token related contracts if they all have similar function signatures. So I guess that leaves UUPS which I also don't know all too much about.

[rndquu]

how to use Diamond for token related contracts if they all have similar function signatures

This way would have multiple diamond contracts each with their own facests:

• CoreDiamond (this is basically the single diamond contract that we have now)
  • AccessControlFacet
  • ChefFacet
  • CreditNftManagerFacet
  • etc...
• UbiquityDollarDiamond
  • UbiquityDollarFacet
• UbiquityCreditTokenDiamond
  • UbiquityCreditTokenFacet
• etc...

[0x4007]

I think we should not use the Diamond Standard for tokens because it seems really redundant to have a new Diamond per token.

[molecula451]

If we therefore, integrate, UUPS into this same repo, then we need to DEBUG hard the interaction between that proxy and diamond

[molecula451]

To keep consistency with the project/codebase I agree in keep developing thru diamonds, and face it's underlying issues, with ideas, ticket issues, and solutions and perhaps give room to a new repo within the org to implement UUPS on some of the contracts, but i think we should not mix both.

So the 2nd. makes sense to me!

The 1st, since we are not production ready. There's openness for issue tickets, and new repo to implement the tokens on a local dev test env

[0x4007]

If possible I'd like to preserve the history of the original tokens. At least the governance token. Do you think its possible to continue to use the same contracts?

[molecula451]

I'd love to. I find UUPS, cleaner and simpler for that purpose, but we need to explore out the compatibility along the diamond. I think we may pull something

[rndquu]

If possible I'd like to preserve the history of the original tokens. At least the governance token. Do you think its possible to continue to use the same contracts?

You mean to continue to use already deployed UBQ ERC20 token contract?

[0x4007]

I'm not sure if my reaction sends a notification, but yes, I mean to continue to use already deployed UBQ ERC20 token contract.

[rndquu]

We can't make an already deployed contract upgradeable. The best thing we can do is to create some kind of an upgradeable wrapper over the Governance token which will proxy all calls to the Governance token preserving some space for future upgrades. This will not cover all possible cases for upgrades but at least adds some space to custom logic.

There are also some drawbacks with this approach:

• hard to maintain (upgradeable wrapper over ERC20 token)
• ambiguity on the frontend (we will have to keep in mind that we're using a wrapper and the main storage is on the Governance token side)

Anyway right now we don't have any features that we should add to the Governance token so I think that it is simpler to refactor the deployment script and the tests to use already deployed Governance token. We can add an upgradeable wrapper any time in the future.

[rndquu]

Ok, so there is no consensus regarding Diamond vs UUPS. I'm actually ok with both approaches (with UUPS we get simplicity, with Diamond we get code consistency). I remember @zgorizzo69 mentioned some points in favour of UUPS. So the winner seems to be UUPS (2.5 votes vs 1.5 votes)