Skip to content

Base facet and app facets override same contracts which is prone to bugs#

Informational

contract BaseFacet is
    ERC721AUpgradeable,
    MinimalOwnableRoles,
    ERC2981,
    OperatorFilterer, 
    DiamondLoupeFacet
{


contract DropFacet is InternalOwnableRoles, InternalERC721AUpgradeable {

contract ApeDropFacet is InternalOwnableRoles, InternalERC721AUpgradeable {

contract EditionFacet is InternalOwnableRoles, InternalERC721AUpgradeable {

They both share some inheritance (because InternalOwnableRoles is an internal modified version of MinimalOwnableRoles and InternalERC721AUpgradeable is an internal modified version of ERC721AUpgradeable).

This has been done so that app facets can easily call functions and use storage from ERC721A and OwnableRoles.

But this design requires attention to the risks of having an overriden function in BaseFacet which is not overriden in an app facet. In that case, when a function is called from an app facet it would execute original code instead of overriden one which is undesired and leads to bugs.

A bug has been found during the audit which was caused exactly by this fact.

The function: 

    function _startTokenId() internal pure override returns (uint256) {
        return 1;
    }

Is an override in BaseFacet, allowing to have token ids starting at 1 instead of 0.

But the function was not overriden in facets, and the consequence was that max supply could not be reached.

Solution to the bug was to implement the same override in each app facet.

Fortunately, bug was found and fixed.

Recommendations#

There are a few different ways to share code between facets. We recommend these ways:

  • Write internal functions in Solidity libraries and import and call those functions in facets.
  • Put common internal functions in a contract that is inherited by multiple facets. Internal functions defined with the virtual keyword can be overriden. Consider not using the virtual keyword to ensure shared internal functions are the same between facets.

More information about sharing code between facets and ways to do it are in this article: How to Share Functions Between Facets of a Diamond