solidstate-network / solidstate-solidity

💠 Upgradeable-first Solidity smart contract development library 💠
https://discord.gg/BnvwfM6bRe
MIT License
441 stars 89 forks source link

incorrect implementation of Diamond #141

Closed wiasliaw closed 2 years ago

wiasliaw commented 2 years ago

Diamond contract is a proxy of lookup table of addresses and function selectors. Some of the functions are specified in EIP, but they can be removed. However, current implementation inherit the DiamondReadable and DiamondWriteable, which cause function shadowing and make them unable to be removed. Otherwise, calling these functions doesn't use delegate call (due to function shadowing).


From eip2535 substack, there is a term called Finished Diamond.

Finished Diamond: A finished diamond was an upgradeable diamond and had a number of upgrades. Then its diamondCut function and/or other upgrade functions were removed and upgrades are no longer possible. It is no longer possible to add/replace/remove functions. It has become an immutable diamond.

ItsNickBarry commented 2 years ago

The SolidStateDiamond contract is the "recommended" configuration for most users. In my opinion, the need for a "finished diamond" is very rare, so SolidStateDiamond does not support that feature. However, it is still a valid diamond implementation - all functions inherited into the proxy are properly registered as "immutable functions".

A user who wishes to implement a "finished diamond" can do so manually by using DiamondBase as the proxy, and DiamondWritable as a facet.

I will consider adding another full implementation that can be a "finished diamond", but at the moment I still don't think it's necessary.