QA Report

[code423n4]

Unused import of OpenZeppelin String.sol in PuttyV2Nft.sol

PuttyV2Nft.sol is importing import "openzeppelin/utils/Strings.sol"; but is never using that library inside the code.

Consider removing it.

PuttyV2Nft.sol miss natspec documentation

Natspec documentation are useful for internal developers that need to work on the project, external developers that need to integrate with the project, auditors that have to review it but also for end users given that Etherscan has officially integrated the support for it directly on their site.

The PuttyV2Nft contract is missing natspec documentation for all the function. A well detailed natspec documentation would be important to describe the current implementation difference for _mint, transferFrom and balanceOf functions.

Consider adding natspec documentation to the contract.

Orders should not be cancellable multiple times or after being filled

The cancelledOrders mapping state variable is only used by fillOrder to prevent to fill an already cancelled order.

To prevent confusion, the protocol should prevent:

1. the order.maker to cancel order multiple times and as a result emitting multiple time the CancelledOrder event
2. the order.maker cancel an order after being filled. Even if it does not impact any logic inside the contract, it still could confuse users interacting directly with the contract/websites that use that information or even with monitoring tools that rely on those events/state variables

Consider preventing order.maker to call the cancel order if the order had been filled or have been already cancelled.

Note: If the protocol implement this solution, they should also consider to "skip" the cancel call inside acceptCounterOffer if the originalOrdr has been cancelled, otherwise the acceptCounterOffer tx would revert.