Pieces Protocol

Description

Your contract can receive Ether in buyOrder; however, if someone sends Ether directly via a plain transfer (.transfer(), .send(), or sendTransaction from outside), the contract will reject it since there is no receive() or fallback() function. This is mostly an operational/usability note rather than a direct security vulnerability.

Impact

• Any direct or accidental transfer of Ether outside buyOrder will revert.

Recommendation

• If you want to allow direct deposits of Ether, implement a simple receive() function that reverts or logs the deposit to prevent confusion.

8. Minor Findings & Best Practices

1. Repeated onlyNftOwner(nftAddress, tokenId): In divideNft, you have the same modifier repeated: onlyNftOwner(nftAddress, tokenId) onlyNftOwner(nftAddress, tokenId). This is likely a typo or redundant.

2. Use of _ in Constructors: Ownable(msg.sender) in the constructor is valid, but ensure that it is not overshadowed if you use a newer version of OpenZeppelin’s Ownable.

3. Checks-Effects-Interactions: Throughout the contract, ensure all external calls (transfers, .call{value: ...}, etc.) are placed after state modifications to reduce re-entrancy risk.

4. Zero Price Sell Orders: The code does not revert if price == 0. Be sure that is intended. Otherwise, you can put a minimum floor or revert if price == 0.

5. Documentation: Thoroughly document how fees are calculated and how fraction owners must approve the contract prior to calling transferErcTokens.