search

bosonprotocol / boson-protocol-contracts

Boson Protocol V2 (latest)
https://bosonprotocol.io/
GNU General Public License v3.0
32 stars 8 forks source link

Remove supportsInterface function from diamond proxy #247

Closed mudgen closed 2 years ago

mudgen commented 2 years ago

I suggest removing the supportsInterface function from the diamond proxy contract here: https://github.com/bosonprotocol/boson-protocol-contracts/blob/main/contracts/diamond/ProtocolDiamond.sol#L62

The supportsInterface function does need to be in the diamond proxy contract or added to the diamond in the constructor function when implementing ERC721 because OpenSea's ERC721 validator looks for supportsInterface when a contract is created. But this diamond isn't implementing ERC721, so it doesn't need to be in the diamond proxy contract.

I suggest adding the supportsInterface function in the constructor function of the diamond, the same way the diamondCut and loupe functions are being added here: https://github.com/bosonprotocol/boson-protocol-contracts/blob/9639040566d9a6b355355009a2a6d5ef7be9327e/scripts/util/deploy-protocol-diamond.js#L16

If the supportsInterface function is left in diamond proxy contract then it still needs to be emitted in the DiamondCut event and returned by the Diamond Loupe functions, to be compliant with EIP2535 Diamonds.

cliffhall commented 2 years ago

We can refactor/move the supportsInterface method to a standalone facet and deploy that facet when we deploy the diamond.

cliffhall commented 2 years ago

The reasoning here is from the standard:

All immutable functions must be emitted in the DiamondCut event as new functions added. And the loupe functions must return information about immutable functions if they exist. The facet address for an immutable function is the diamond’s address. Any attempt to delete or replace an immutable function must revert.

cliffhall commented 2 years ago

Closing this one as it has been addressed in PR #263