forgenie / diamond-foundry Goto Github PK

NFT facet should be implemented first #33
Instead of tokenIds we can create incremental nonces for each diamond

There is no need to keep them as separated facets, since Introspection has only one function anyways

Pull Request #6

Currently all the facets are merged into a single one Base facet, so that it implements all the basic requirements of smart contracts and EIP2535.

We need to split them into separate facets as multiple projects do it. In this way we can upgrade in isolated manner.

It would be good, if we can compose the base facets into a single one, so that every created diamond complies with requirements.

Take ERC721AUpgradeable and remove simple ERC721A dependency

currently diamondCut is not protected by owner

Instead of rewriting our own facets like ERC20 and others, we should use the OpenZeppelin library for security guarantees.

https://eips.ethereum.org/EIPS/eip-1820

Replace the error naming convention <Contract>_<method>_<name> for <Contract>_<name>

That includes structs, events and errors

Make DiamondFactory implement IBeacon interface that points to the most recent Diamond implementation. Create a deployDiamondBeacon method that creates a proxy to this diamond.

Also, consider a deployDiamondClone that deploys minimalistical non-upgradeable clones that point to the current diamond implementation.

We need this in order to be compatible with ERC1967.

Also, research other Proxy factories and outline functionalities that we need to support, example:https://github.com/maple-labs/proxy-factory/tree/main/contracts.

Add document generation

supportsInterface should return false for facets that are removed, our options:

1. We store supported interfaces for each facet in registry, and diamond removes by fetching interfaces from there.
   supportedInterfaces(address facet, bytes4[] interfaceIds)
2. We have noDelegateCall method on each facet, diamond directly asks the facet, on add and on remove. We can get rid of some initializers, but get possible compatibility issue.
   supportedInterfaces(bytes4[] interfaceIds)

ERC721 Facet with mint, burn functionality

We need to implement a library for EnumerableBytes4Set similar to OZ EnumerableBytes32Set where we can store 8 selectors in one storage slot.

We need a way to initialize a diamond when adding multiple facets, which potentially contain their own initializers.

We could it similarly to Multicall3 but with delegatecall, but it should be permitted only to diamonds during the deployment process. As delegating to unknown addresses can induce unforeseen consequences.

Check implementation for simple calls here https://github.com/mds1/multicall

For example, we could make another fallback that resolves to some default behavior if no Facet was found in the diamond.
Other fallback approaches are also welcome