<!-- canonical: https://0xsimao.com/reports/nftperp-exchange -->

# Nftperp Exchange audit

Three Sigma · NFT perpetuals · 2nd January 2024

A private security review of NftPerp (NFT perpetuals), conducted with Three Sigma and completed on 2nd January 2024 over 25 days. 0xSimao disclosed 20 findings: 2 high severity, 2 medium, 7 low and 9 informational.

## What NftPerp is

Nftperp lets traders take leveraged long and short positions on NFT collection floor prices, Milady, Pudgy Penguins and the rest, through perpetual futures settled entirely in ETH against a virtual AMM.

## Scope

This first round covered the exchange core: margin accounting as positions are reduced or merged, funding settlement, notional bookkeeping, and the maker/taker fill machinery of the order flow.

## Findings

### High

- [reduceOnly limit order update does not take into account that a new amm may be selected, which may lead to loss of funds](https://0xsimao.com/findings/nftperp-i-reduce-account-amm-selected)
- [Realized pnl not returned to trader on _mergeDecrease()](https://0xsimao.com/findings/nftperp-i-realized-returned-trader-merge)

### Medium

- [_getPriceToTick() reverts if the price is smaller than 1e10, which may happen in _search() as it assumes the final tick as going entirely to the amm](https://0xsimao.com/findings/nftperp-i-price-reverts-happen-entirely)
- [setFundingPeriod() can be DoSed due to calling settleFunding()](https://0xsimao.com/findings/nftperp-i-funding-period-calling-settle)

### Low

- [In fillPair(), when the taker is the maker, the reduceOnly order mapping is not being deleted](https://0xsimao.com/findings/nftperp-i-fill-taker-maker-deleted)
- [_getPositionNotional() crops the size of the position to the available reserves, which may lead to unexpected profits/losses](https://0xsimao.com/findings/nftperp-i-notional-crops-unexpected-profits)
- [getTriggerOrders() should be paginated or it may revert if enough orders are created](https://0xsimao.com/findings/nftperp-i-orders-paginated-revert-created)
- [Partially removing liquidity may underflow if the price of the pool has changed significantly](https://0xsimao.com/findings/nftperp-i-partially-underflow-price-significantly)
- [removeLiquidity() in the amm calculates marginToRemove without updating the margin with the funding payment](https://0xsimao.com/findings/nftperp-i-calculates-margin-updating-payment)
- [PriceFeed: Two step owner transfers are safer](https://0xsimao.com/findings/nftperp-i-price-two-transfers-safer)
- [Total Position Size isn't updated properly](https://0xsimao.com/findings/nftperp-i-size-isn-updated-properly)

### Informational

- [State changes should always emit events](https://0xsimao.com/findings/nftperp-i-state-changes-emit-events)
- [Swapping amounts that would send the price below/above the bounds of the amm underflows without a reason](https://0xsimao.com/findings/nftperp-i-price-below-bounds-underflows)
- [PriceFeed: First owner isn't set as valid keeper](https://0xsimao.com/findings/nftperp-i-price-isn-valid-keeper)
- [Structs can be packed to save gas](https://0xsimao.com/findings/nftperp-i-structs-packed-save-gas)
- [Contracts should inherit their interfaces](https://0xsimao.com/findings/nftperp-i-contracts-inherit-their-interfaces)
- [Bug in _reversePosition() might lead to future exploits](https://0xsimao.com/findings/nftperp-i-bug-reverse-future-exploits)
- [Using low level pop is not recommended](https://0xsimao.com/findings/nftperp-i-low-level-pop-recommended)
- [Errors could include relevant arguments whenever possible, making it easier to debug](https://0xsimao.com/findings/nftperp-i-include-relevant-easier-debug)
- [mul() and div() in NFTPMath are misleading due to scaling by 1e18 underneath](https://0xsimao.com/findings/nftperp-i-nftp-scaling-1e18-underneath)

## The report

[Download the report (PDF)](https://0xsimao.com/reports/2024-01-02-nftperp-i.pdf)
