Josselin Feist, from Trail of Bits, wrote an article with a number of concerns about EIP-2535 Diamonds and an implementation of it. I'll address concerns he brought up and correct some things along the way.
One thing to mention is that EIP-2535 Diamond and its implementations and tools and information is a community effort. There is a growing group of people making the standard and its growing ecosystem what it is.
I invite Trail of Bits and other people to help fill in the gaps of information, tests, procedures, ideas and tools to help make diamonds and upgrades better and easier to use for everyone.
The Diamond Discord is here: https://discord.gg/kQewPw2
EIP-2535 Diamonds Is Not Just For Upgrades
Josselin's article describes EIP-2535 Diamonds as an "upgradeable standard" only. This is a fundamental misunderstanding of the standard. EIP-2535 Diamonds is more than just a standard for upgrading smart contracts. In addition “upgradeability” is an optional feature of the standard.
EIP2535 Diamonds is a general purpose smart contract framework for building multi-contract systems. Diamonds can be upgradeable or not. Diamonds provide a way to have smart contract functionality that exceeds the 24kb smart contract size limit at a single Ethereum address. It provides a standard way to organize the source code of a large smart contract system. It provides a mechanism for on-chain code reuse and modularity. It provides mechanisms for transparency and tooling.
Yes, it also enables upgrades (if desired), in such a way that a project can be deployed and then be extended as needed in a systematic and organized way, without limit. And diamonds provide fine-grained upgrades so code that doesn't need to be changed doesn't have to be redeployed.
Josselin's Article Is Biased Against EIP2535 Diamonds With False Data
Josselin's article is not an objective, honest review of EIP2535 Diamonds. It contains outdated code and false data. His article brings up security concerns that are addressed and handled by the standard and related diamond documentation.
A prominent and obvious technical advantage of EIP2535 Diamonds is that it provides a way for a smart contract to exceed the 24kb smart contract size limit. Josselin's article fails to mention that and other benefits of the standard.
About Upgrades
Josselin says an upgradability standard should have the following things:
A clear, simple implementation. Standards should be easy to read to simplify integration with third-party applications.
Understood, this has been done with the diamond-1-hardhat implementation.
A thorough checklist of upgrade procedures. Upgrading is a risky process that must be thoroughly explained.
A list of things to watch out for and rules to follow to ensure safe upgrades has been written and exists here: https://eip2535diamonds.substack.com/p/diamond-upgrades
On-chain mitigations against the most common upgradeability mistakes, including function shadowing and collisions. Several mistakes, though easy to detect, can lead to severe issues.
The diamond reference implementations contain no function shadowing and provides a solution for handling that. The standard prevents function selector clashes. Diamond storage helps prevent contract storage corruption or collisions. Additional tools and ways to do this are welcome. There is also information about upgradeable contracts and their pitfalls that can be found online.
I have written many articles (and there are others online) about how to correctly and safely write smart contracts and diamonds. These can be found here: https://eip2535diamonds.substack.com/ and here: https://dev.to/mudgen
A list of associated risks. Upgradeability is difficult; it can conceal security considerations or imply that risks are trivial.
A list of things to watch out for and rules to follow to ensure safe upgrades has been written and exists here: https://eip2535diamonds.substack.com/p/diamond-upgrades
Also this blog post about Pros and Cons of diamonds is recommended reading: https://eip2535diamonds.substack.com/p/answering-some-diamond-questions
Tests integrated with the most common testing platforms. The tests should highlight how to deploy the system, how to upgrade a new implementation, and how an upgrade can fail.
The diamond reference implementations have a suite of tests to test them. The reference diamond implementations and other implementations show ways to deploy and upgrade diamonds.
hardhat-deploy provides builtin support for deploying and upgrading diamonds and includes documentation about it.
Contract Standardization
One of the main purposes of a contract standard is to provide standard interfaces with implementation details so that implementations can be made that interoperate with other contracts and tools and software.
EIP-2535 Diamonds does this by providing a standard function diamondCut for upgrades, a standard event DiamondCut for recording what functions are added/replaced/removed and 4 standard functions that are used to show what functions a diamond has.
The standard provides information about how to implement these functions and other useful information. All of this is useful and being used in development and production successfully.
Diamond Implementation
Josselin shows part of an implementation of the diamondCut function that is used for upgrades. In several places he mentions that gas-optimizations make the code more complicated:
Additionally, much of the code complexity is due to optimization in locations that don’t need it.
The gas-optimizations that were made in that implementation save gas and it is really up to the people building an application whether or not the gas optimizations are needed or wanted or not.
People who aren't interested in gas-optimizations can use the much simpler diamond-1-hardhat implementation.
Josselin's article is badly out of date. The code shown for the upgrade function was removed and replaced by simplified code 7 weeks before the date Josselin's article was published. The code he shows does not exist in any of the current diamond reference implementations. I have asked Josselin to update his article with code from a current implementation, but he has not done that.
Josselin was hired from Trail of Bits back in 2020 to help us find bugs and improve our code. And he did and we fixed and improved things. Then he published an article featuring our old, unfixed code to represent EIP-2535 Diamonds. To be accurate he should provide information and code for current implementations.
Also note that the EIP-2535 Diamond is a smart contract standard. It is not an implementation. There can be good or bad implementations of it. Josselin's article conflates an outdated implementation with the standard.
Diamond Terminology
- Diamond A contract that gets its external functions from other contracts called facets. A diamond has a fallback function that delegates function calls to facets. A diamond implements the Specification section of EIP-2535 Diamonds.
- Facet A contract that supplies external functions to a diamond. In the diamond industry a facet is a side of a diamond.
- Loupe Four standard functions that show what functions and facets a diamond has. In the diamond industry a loupe is a magnifying glass that is used to look at diamonds.
-
Upgradeable Diamond A diamond that has the
diamondCutexternal function so functions can be added/replaced/removed. -
Finished Diamond A diamond that was an upgradeable diamond but then its
diamondCutfunction was removed so functions can no longer be add/replaced/removed. It can't be upgraded anymore. -
Single Cut Diamond A diamond that adds all of its functions and facets in its constructor function, and does not add the
diamondCutor other upgrade function. A single cut diamond is fully formed upon deployment, is immutable and can't be upgraded.
Josselin says:
EIP 2535 introduces “diamond terminology,” wherein the word “diamond” means a proxy contract, “facet” means an implementation, and so on. It’s unclear why this terminology was introduced.
Let me clear this up.
Originally EIP-2535 Diamonds was called the Transparent Contract Standard, but that name clashed with something from OpenZeppelin and they asked me to change the name.
When making transparent contracts it was noticed that each of its facets/implementations could handle a different aspect or set of functionality and it was useful to organize code this way. It was like each facet/implementation was a different side or facet of a transparent contract.
Physical diamonds have facets that share a common center. EIP-2535 diamonds have facets that share the same contract storage space and a common Ethereum address.
The terminology helps conceptualize how diamonds work and how to build applications with them.
The Loupe Functions
The loupe functions are four standard functions defined by EIP-2535 Diamonds that return what functions a diamond has and the facet addresses they come from.
The verified source code of a diamond does not show what functions it has or where it gets them. And a diamond's ABI doesn't include what functions it has. To help get this information the DiamondCut event or the loupe functions are used.
The loupe functions are being used successfully by the application https://louper.dev/ to display and make diamonds transparent. It is like etherscan but for diamonds.
One of the loupe functions is facetFunctionSelectors. Josselin says this about it:
facetFunctionSelectorsreturns all the function selectors of an implementation. This information will only be useful for off-chain components, which can already extract the information from the contract’s events. There’s no need for such a feature on-chain
An important reason the loupe functions (including facetFunctionSelectors) exist is because functions on smart contracts are more reliable than events. These functions make diamonds robust. This ensures their transparency and interoperability with software.
Some event systems fail, some are inconsistent and fragile. Events work better on some blockchains than others. For example on Polygon where block times are fast querying events on nodes will fail if trying to query too far back in time. Basing a system on an unreliable event system makes a system also unreliable or harder to deal with or program to make reliable. This is why diamonds use on-chain loupe functions for transparency. It is robust.
Currently the loupe functions are being used in unit tests to test diamonds. They are being used in code that deploys and upgrades diamonds. They are used by hardhat-deploy to deploy and upgrade diamonds.
It is true that DiamondCut events can be queried and processed to get the same data that the loupe functions return. This means there are two ways to get data about diamonds. Both ways can get data about diamonds and either way can be used.
So why have two ways? Because in some cases it will be easier and better to use function calls, and in other cases it will be easier and better to use events.
For example a blockchain explorer website may want to use events to show facet and function information about diamonds because it is already using event tooling and infrastructure to capture and process events for contracts.
An upgrade script may want to check that functions don't already exist in a diamond before trying to add them. Using the facets loupe function for this is simple and easy and reliable.
A simple static website can show information about any diamond by calling the loupe functions and then using the returned data to query services like Etherscan to get and show verified source code, ABI information and other information. This could be done with events, but it would be easier, simpler and probably run faster if done with loupe functions.
Having loupe functions and events give people and software choice to use what is best for their use.
There is also a question about reliability. Querying and processing events is not as reliable, available and performant as contract function calls. Likewise, a connection to an Ethereum node may not always be available in every system but there may be a connection to an events database that can be used to retrieve diamond information. Having both events and loupe functions makes diamonds robust.
The loupe functions add some complexity to diamonds. Generally it is a good idea to push complexity out of contracts into off-chain software. But every case is different and requires evaluation. In the case of diamonds there is another principle which is optimize for simplicity for common use cases. A diamond is implemented and deployed once but will be queried many times and may be used by or integrated with a lot of software. The loupe functions are a simple, reliable and direct way to query diamonds and integrate with software. The loupe functions are read-only, tested and security audited. The loupe functions can simply be added and used in every diamond without touching application specific smart contract code.
In addition the loupe functions can be deployed once and be reused on-chain by many diamonds.
Someone suggested the idea of making the loupe functions optional in the standard. The problem with that is it would break interoperability. Some diamonds would work with some software and not with other software.
I wrote another article about loupe functions that provides more information: Diamonds Loupe Functions
Addressing False Data
Many Interfaces
While the proposal only needs a lookup table, from the function signature to the implementation’s address, the EIP defines many interfaces that require storage of additional data:
This statement is a lie. It is easily verifiably false. You can easily fact check it yourself.
Josselin says the EIP "defines many interfaces that require storage of additional data". If you look at EIP-2535 you will see that the EIP only defines two interfaces in the entire standard. So unless you think "many" means two the statement is false. But still, only one of the interfaces requires storing additional data besides a lookup table. One definitely does not mean "many".
The IDiamondLoupe interface requires storing function selectors in an array and their positions in the array. That is all the additional data that needs to be stored.
Why does Josselin lie about this? If it is just a mistake then why doesn't he fix it?
In general I suggest that you verify the technical details for yourself with regards to Josselin's article. The diamond reference implementations can be found from here: https://github.com/mudgen/diamond
Safe Struct Storage Pointers with Diamond Storage
EIP-2535 Diamonds shows and encourages a contract storage technique called Diamond Storage.
Diamond Storage enables different facets to share or use their own contract storage space within a diamond and access it with a namespace. The namespace is a unique, human-readable and meaningful string that represents the area of data. For example "com.mywebsite.myproject.mytoken". The string is hashed and used as a pointer to a struct in contract storage. More information about this and examples are in this article: How Diamond Storage Works.
There is no risk of accidentally or maliciously colliding diamond storage with other contract storage because of using hashes of unique and meaningful strings for struct storage pointers.
OpenZeppelin plans to use Diamond Storage in their next version of upgradeable contracts.
Josselin's article shows an example of colliding contract storage data. However his storage pointer is not a hash of a unique and meaningful string. His storage pointer is not even a hash of a string but is a hash of a hex literal of a specific but random looking value. It is easy to see that it is not a namespace to an area of contract storage.
Malicious storage pointers are obvious and easy to spot. Here is a comparison:
Namespace to an area of contract storage:
bytes32 storagePosition = keccak256("myproject.mytoken")
Malicious pointer that collides with existing contract storage data:
bytes32 storagePosition = keccak256(
hex"78c8663007d5434a0acd246a3c741b54aecf2fefff4284f2d3604b72f26"
);
Someone adding code to their contract or diamond with a malicious pointer means that the person is adding code that they didn't look at or understand. If that is the case then any kind of malicious code could be added.
Diamond Storage Not Required
The EIP proposes that every implementation have an associated structure to hold the implementation variables, and a pointer to an arbitrary storage location where the structure will be stored.
EIP-2535 Diamonds does show, explain and encourage using diamond storage. But it does not require it.
Diamonds and facets are free to use or mix any contract storage strategy such as inherited storage etc.
To comply with the standard and be a diamond an implementation must implement the Specification section of the standard, which does not mandate any contract storage strategy or mechanism.
Function Shadowing
Upgradeable contracts often have functions in the proxy that shadow the functions that should be delegated. Calls to these functions will never be delegated, as they will be executed in the proxy. Additionally, the associated code will not be upgradeable.
None of the diamond reference implementations have any functions shadowing any other functions. Function shadowing is not possible in diamonds that follow the standard.
It is possible to have external functions directly in a diamond in a safe way. They are called immutable functions in EIP-2535 Diamonds. They are functions that can't be replaced or removed. The diamond loupe functions provide information about them and they are emitted in the DiamondCut event. In the reference implementations calling diamondCut to replace or remove immutable functions will revert. See the standard for more info.
Contract Existence Check
Another common mistake is the absence of an existence check for the contract’s code. If the proxy delegates to an incorrect address, or implementation that has been destructed, the call to the implementation will return success even though no code was executed (see the Solidity documentation). As a result, the caller will not notice the issue, and such behavior is likely to break third-party contract integration.
Note: Josselin's article shows a fallback function without the contract code check he talks about.
The fallback function in the diamond reference implementations do not check for contract code. It does not do this because of the gas cost and because the diamondCut function prevents any functions from being added from facets that do not have code.
The rule which is documented is don't write selfdestruct in facet source code and don't use facets with selfdestruct in them unless it specifically makes sense for some use case. It is unnecessary and useless (and costs gas) to check for contract existence on contracts that can never be deleted.
OpenZeppelin's proxy contracts also do not perform the check in the fallback function. Like the diamond reference implementations they prevent implementation contracts from being added that don't have code.
EIP-2535 Diamonds does not prevent people from adding this check in the diamond fallback function so it can be added by people who want it. Contract existence check in the diamond fallback function is not part of the EIP2535 Diamonds standard so it is up to the implementer to implement it or not.
Top comments (2)
Hello Readers,
I am the author of the original post from Trail of Bits. Nick: thank you for acknowledging that you agree with most of the issues pointed out in our review of your proposal.
Regarding the new reference implementations, I think the complexity has got even worse than what we initially reviewed and blogged about. There are now three reference implementations, which makes everything even more confusing for users, and further review of the proposal more difficult.
The "low complexity" version is far from having a low complexity. For example, see the following code and then compare it to the upgrade function in OpenZeppelin's library. This level of complexity is not necessary to update a key-value pair in a mapping.
The direction that the proposal is taking now highlights its primary fundamental issues:
The proposal and implementations would be suitable for a custom solution but do not meet the maturity required for a standard.
I think it would be beneficial to re-start the proposal, create a simple API, use existing common vocabulary, and avoid the pitfalls shown in our blogpost.
-Josselin
Hi Josselin,
I don't agree with the points of your article that are outdated or inaccurate which I address in my article. I think your article is unnecessarily negative. I think you can help with some of the informational/procedure parts you mention in your article and that would be great.
EIP-2535 Diamond Standard is a good and useful standard that applications can use.
OpenZeppelin's upgrade function can't be compared to the Diamond Standard's because they do different things. The Diamond Standard provides more functionality than OpenZeppelin's proxy. With the Diamond Standard you can have functions from multiple implementations/facets, which is a core part of what the Diamond Standard is about.
The
diamondCutfunction that is linked to provides an important safety feature: You have to be explicit about what functions you are adding or replacing or removing. The function then checks and enforces that you don't make a mistake about that. This safety feature is a good thing and protects users.The
diamondCutcode that is linked to is easy to read and is straightforward.Having three different implementations does not affect the complexity of the code, since each implementation is separate.
I think that if the most complicated thing about the Diamond Standard is that people have to choose which implementation to use, that's pretty good.
There are three different implementations to meet the needs and wants of different people. It is not hard to sort out which implementation to use, and in many cases I don't think it matters much which one is used because they do the same things. These are implementations of the standard, not the standard itself. People are free to write their own implementations or use other implementations if they want to.
People that want a simple implementation should use the diamond-1 implementation:
github.com/mudgen/diamond-1
People that want a more gas-efficient implementation should use the diamond-2 implementation: github.com/mudgen/diamond-1
People willing to pay a little more gas and want the simplest loupe functions should use the diamond-3 implementation: github.com/mudgen/diamond-3
More info about this here: github.com/mudgen/diamond
Also people can contact me if they have questions.