QA Report

[code423n4]

1. Use safeTransfer/safeTransferFrom consistently instead of transfer/transferFrom

It is good to add a require() statement that checks the return value of token transfers or to use something like OpenZeppelin’s safeTransfer/safeTransferFrom unless one is sure the given token reverts in case of a failure. Failure to do so will cause silent failures of transfers and affect token accounting in contract.

Instances:

PuttyV2.sol #L336

PuttyV2.sol:336: IWETH(weth).transfer(order.maker, msg.value);

Reference:

This similar medium-severity finding from Consensys Diligence Audit of Fei Protocol.

Recommended Mitigation Steps:

Consider using safeTransfer/safeTransferFrom or require() consistently.

---

---

2. _safeMint() should be used rather than _mint() wherever possible

_mint() is discouraged in favor of _safeMint() which ensures that the recipient is either an EOA or implements IERC721Receiver. Both open OpenZeppelin and solmate have versions of this function so that NFTs aren’t lost if they’re minted to contracts that cannot transfer them back out.

Instances

PuttyV2.sol #L303 PuttyV2.sol #L303

PuttyV2.sol: 303: _mint(order.maker, uint256(orderHash));
PuttyV2.sol: 308: _mint(msg.sender, positionId);

Recommendations:

Use _safeMint() instead of _mint().

[outdoteth]

2. _safeMint() should be used rather than _mint() wherever possible

Duplicate: Contracts that can’t handle ERC721 tokens will lose their Putty ERC721 position tokens: https://github.com/code-423n4/2022-06-putty-findings/issues/327

[HickupHH3]

Disagree with (1), checking return value for WETH is unneeded. Since (2) is upgraded, I'm invalidating this report.