kopy-kat / ethdenver-aa Goto Github PK

Great to see this implementation that uses EIP-2535 Diamonds!

I noticed something that I would like to address. I noticed on twitter the description of this project using "using (modified) Diamond Proxies (ERC-2535).". And I noticed in the README the text, " we created a custom implementation of diamond proxies".

The diamond reference implementations are not the standard, they are only working (and security audited) examples of implementations of EIP-2535. The standard can be implemented in various ways with various different functionality. There is no such thing as a "custom" implementation of EIP-2535 Diamonds. There is only implementations that implement the standard or don't.

If you take an existing diamond reference implementation, like you did, and make your own changes to it, that does not mean that you created a "modified" or "custom" diamond proxy. Because a reference implementation is just an example of an implementation and the standard is VERY flexible. As long as you follow the Implementation Points stated in the standard then your implementation is a standard implementation of EIP-2535 Diamonds.

I looked briefly at your code and I only saw one deviation of the standard which is that you did not add the "immutable" functions to the diamond. Immutable functions are external functions that are defined directly in the diamond proxy or inherited. They also need to be added to the diamond using diamondCut. This is an interoperability point --- the loupe functions and DiamondCut event will be missing data if you don't do this -- and it is a security point. This is an easy fix: add your immutable functions to the diamond, just like you would any other external functions. But you will find that a better design is to move all your external functions to their own facet that is only deployed once. That design reduces the cost of deploying diamonds (because you can just reuse already deployed facets) and gives you more flexibility to add/replace external functions.

Check out this article about EIP-2535 Diamonds compliance: https://eip2535diamonds.substack.com/p/compliance-with-eip-2535-diamonds