QA Report

[code423n4]

Unsafe ERC20 Operation(s)

Information : L001 - Unsafe ERC20 Operation(s)

Instances include :

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

Recommendation

It is recommended to always use OpenZeppelin's SafeERC20 library, for example :

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

---

_safeMint() should be used instead of _mint() if possible

Instances include :

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

Recommendation

It is recommended to always use _safeMint() instead of _mint().

---

Block timestamp

Information : Block timestamp

Instances include :

PuttyV2.fillOrder(PuttyV2.Order,bytes,uint256[]) (src/PuttyV2.sol#268-380) uses timestamp for comparisons
    Dangerous comparisons:
    - require(bool,string)(block.timestamp < order.expiration,Order has expired) (src/PuttyV2.sol#290)
PuttyV2.exercise(PuttyV2.Order,uint256[]) (src/PuttyV2.sol#389-458) uses timestamp for comparisons
    Dangerous comparisons:
    - require(bool,string)(block.timestamp < positionExpirations[uint256(orderHash)],Position has expired) (src/PuttyV2.sol#401)
PuttyV2.withdraw(PuttyV2.Order) (src/PuttyV2.sol#466-520) uses timestamp for comparisons
    Dangerous comparisons:
    - require(bool,string)(block.timestamp > positionExpirations[longPositionId] || isExercised,Must be exercised or expired) (src/PuttyV2.sol#481)

Recommendation

Avoid relying on block.timestamp.

---

Calls inside a loop

Information : Calls inside a loop

Instances include :

PuttyV2.fillOrder(PuttyV2.Order,bytes,uint256[]) (src/PuttyV2.sol#268-380) has external calls inside a loop: IWETH(weth).deposit{value: msg.value}() (src/PuttyV2.sol#358)
PuttyV2._transferERC721sIn(PuttyV2.ERC721Asset[],address) (src/PuttyV2.sol#610-614) has external calls inside a loop: ERC721(assets[i].token).safeTransferFrom(from,address(this),assets[i].tokenId) (src/PuttyV2.sol#612)
PuttyV2._transferFloorsIn(address[],uint256[],address) (src/PuttyV2.sol#622-630) has external calls inside a loop: ERC721(floorTokens[i]).safeTransferFrom(from,address(this),floorTokenIds[i]) (src/PuttyV2.sol#628)
PuttyV2.fillOrder(PuttyV2.Order,bytes,uint256[]) (src/PuttyV2.sol#268-380) has external calls inside a loop: IWETH(weth).deposit{value: msg.value}() (src/PuttyV2.sol#335)
PuttyV2.fillOrder(PuttyV2.Order,bytes,uint256[]) (src/PuttyV2.sol#268-380) has external calls inside a loop: IWETH(weth).transfer(order.maker,msg.value) (src/PuttyV2.sol#336)

Recommendation

Favor pull over push strategy for external calls.

---

[HickupHH3]

_safeMint() should be used instead of _mint() if possible

not upgrading to med because lacking description and details about why _safeMint() should be used.

Calls inside a loop

same reasoning. what are the implications (loss of assets)? lacking details