Nftperp Matching Engine audit
A private security review of NFTPerp (NFT perpetuals), conducted with Three Sigma and completed on 29th January 2024 over 5 days. 0xSimao disclosed 15 findings: 6 medium and 9 informational.
What NFTPerp is
Nftperp lets traders take leveraged long and short positions on NFT collection floor prices through perpetual futures settled entirely in ETH against a virtual AMM.
Scope
The second round reviewed the rebuilt matching and pricing layer: mark-price updates, tick-based matching, index-flag heuristics for decreases, liquidity-notional constraints and share minting.
Medium severity findings
- Call _updateMarkPrice() whenever markPrice is changed
- _match() always checks the trigger of the first order of a certain tick, instead of checking i order
- Use a flag to increase or decrease the index of the pool instead of a heuristic
- Notional difference in AmmRouter:removeLiquidty() may revert in certain cases
- It's possible to mint an infinite number of shares without increasing quote or base amounts, due to rounding down
- Price deviation as is can be circumvented by making smaller trades in a loop
Informational findings
- PositionManager:_reversePosition() calculates the notional to reverse but could just use the exchangedQuote
- Duplicate size to fill check in _staticFillToAmm()
- gap is missing the private keyword
- Index underflow is not protected against, although it has no impact as the pool with index type(uint256).max should not be registered
- In AmmRouter:liquidateMaker(), the pools array can safely be deleted from storage
- trimDuplicatePools() is very expensive, having O(n^2) complexity and could be simplified
- Add a 0 address check on the pool when adding liquidity for greater verbosity
- setPools() could use more validation
- Misleading error name
The report