Annex

In the 1st part of the audit from 7 March to 14 March, some issues were found and fixed.

We present the most notable ones in this annex.

EIP-2535 Diamonds compliancy

See EIP-2535 Diamonds Implementation Points.

Immutable functions are external functions defined directly in a diamond proxy contract or inherited by it. The EIP-2535 Diamonds standard requires information about immutable functions be returned by the loupe functions and emitted in the DiamondCut event.

Information about immutable functions in the diamond proxy contract (DiamondCollection.sol) were not returned by the loupe functions and were not emitted in the DiamondCut event. This was fixed by putting all the immutable functions in a separate facet (BaseFacet.sol) which is now cut in the diamond proxy constructor.

The fix also made the code clearer, more modular and upgradable.

Replay attacks possible

Signatures in NiftyKitV3.sol and EditionFacet.sol were replayable on another chain. ChainId parameter has been added which fixed the issue.

NiftyKitV3 Withdraw function could be called by anybody

This was a low issue because Ether would be sent to treasury and not the transaction sender. But we can imagine a scenario where treasury would be changed (because compromised for example), and the current treasury address would frontrun the setTreasury function by calling Withdraw just before. By doing so, the compromised treasury would get the ethers in the contract before being prevented from doing it.

It has been changed to OnlyOwner.

The preventTransfers modifier could not block tokens if TransferMode is OperatorsOnly

It was due to some logic issue in the modifier which has been fixed.