# HELIX — Internal Audit Report

**Date:** 2026-05-15
**Reviewer:** internal (pre-audit triage)
**Scope:** `src/HelixPerps.sol`, `src/HelixHook.sol`
**Tools:** slither 0.11.5, foundry forge 34/34 tests, manual review
**Purpose:** Triage findings before commissioning external audits (Trail of Bits, Spearbit, Cantina). Reduce external surface so external auditors hit fewer surprises.

---

## Summary

| Severity | Found | Fixed | Acknowledged | Open |
|---|---|---|---|---|
| Critical | 0 | — | — | 0 |
| High     | 0 | — | — | 0 |
| Medium   | 1 | 1 | 0 | 0 |
| Low      | 4 | 2 | 2 | 0 |
| Info     | many | — | n/a | n/a |

No critical or high findings post-remediation. All blocking issues fixed.

---

## Findings

### M-01 — Unchecked ERC20 transfers — **FIXED**

**Severity:** Medium
**Source:** slither `unchecked-transfer`
**Sites:** 10 call sites across `deposit`, `withdraw`, `_routeBounty`, `_hlpDepositInternal`, `bootstrapClaim`, `hlpWithdraw`, `stake`, `unstake`, `_harvest`, `executeBuyback`.

USDC and a vanilla $HELIX both revert on failure, so the immediate deployment is safe. But the contract is also positioned as a general perp engine — a future market backed by a non-reverting ERC20 (e.g. a USDT-style token that returns false) would silently swallow failed transfers and corrupt internal accounting.

**Remediation:** Added `_safeTransfer` and `_safeTransferFrom` helpers (HelixPerps.sol L266-280) that wrap the low-level call, decode the optional bool return, and require both `success` and `(empty || decoded-true)`. All 10 sites refactored. `forge test` 34/34 still passing.

```solidity
function _safeTransfer(IERC20 token, address to, uint256 amount) internal {
    (bool s, bytes memory data) = address(token).call(
        abi.encodeWithSelector(IERC20.transfer.selector, to, amount)
    );
    require(s && (data.length == 0 || abi.decode(data, (bool))), "transfer failed");
}
```

---

### L-01 — Divide-before-multiply scaling — **ACKNOWLEDGED**

**Severity:** Low (false positive given the constants involved)
**Source:** slither `divide-before-multiply`
**Sites:** `deposit`, `_hlpDepositInternal`, `_distributeFees`

```solidity
amount * (WAD / 1e6)   // = amount * 1e12, exact
assetsWad = assets * (WAD / 1e6)
accFeesPerShare += toStakers * WAD / stakedTotal
```

Standard solidity guidance is to put multiplication before division to preserve precision. In our case `WAD / 1e6 = 1e12` is exact (no truncation) because both are powers of ten, so the result is identical to `amount * WAD / 1e6`. The bonus emissions math `(bootstrapApplied * baseEmissionsPerUsdc * BOOTSTRAP_MULTIPLIER_BPS) / (1e6 * BPS)` has the multiplication first.

**No code change required.** Documented here so external auditors don't flag it again.

---

### L-02 — `receive()` reverts on ETH — **ACKNOWLEDGED**

**Severity:** Low (false positive)
**Source:** slither `locked-ether`

```solidity
receive() external payable { revert("no ETH"); }
```

Slither flags this as "contract has payable but no withdraw." The receive function exists specifically to revert — it is a defensive guard against accidental ETH transfers. No ether can actually accumulate in the contract.

**No code change required.**

---

### L-03 — `bytes32(0)` strict equality on market existence — **ACKNOWLEDGED**

**Severity:** Low (false positive)
**Source:** slither `incorrect-equality`
**Site:** `addMarket` L281

```solidity
require(markets[marketId].id == bytes32(0), "exists");
```

Strict equality on uninitialized mapping slots is the canonical "does this entry exist" check. Cannot meaningfully be replaced with `<` or `>=`.

**No code change required.**

---

### L-04 — Timestamp-dependent comparisons — **ACKNOWLEDGED**

**Severity:** Low (intentional)
**Source:** slither `timestamp`
**Sites:** `bootstrapClaim`, `bootstrapClaimable`, `hlpWithdraw` (founder lock)

Bootstrap vesting and founder lock both depend on `block.timestamp` deltas. Slither flags any comparison against `block.timestamp` as risky because miners can shift it ±15 seconds. In our context:

- Bootstrap window is 28 days. A 15s timestamp shift is negligible.
- Founder lock is 90 days. Same.
- Vesting math is linear with second-granularity precision; small drift averages out.

**No code change required.**

---

## Manual review — focused threat models

### Reentrancy

All user-facing entry points carry `nonReentrant` (HelixPerps.sol L258-264):

```
deposit · withdraw · open · close · liquidate ·
hlpDeposit · hlpDepositWithReferral · hlpWithdraw ·
stake · unstake · claim · bootstrapClaim ·
seedFounderDeposit · executeBuyback
```

Hook-facing entries (`settleFunding`, `sweepLiquidations`, `recordMark`, `donateHLP`) are `onlyHook`. The hook itself is an immutable contract address bound at deploy with no admin. The chain of trust is: PoolManager → HelixHook (immutable) → HelixPerps. No untrusted callbacks can re-enter user state.

**Status:** Clear.

### Oracle manipulation

Mark price is the median of:
- Chainlink aggregator (`m.oracle`)
- AMM 10-minute TWAP (`AMM.twapOf(marketId, 600)`)

```solidity
return chainlink < twap ? (chainlink + twap) / 2 : (twap + chainlink) / 2;
```

The `< vs >=` branches collapse to the same expression (a clarity-only comment) — the median is simply `(chainlink + twap) / 2`. Attack surface:

- Chainlink stale: bounded by Chainlink's heartbeat. Each market should validate `updatedAt` is recent in production (currently not enforced — see **OPEN-01**).
- AMM TWAP manipulation: requires moving and holding the pool against arbitrage for 10 minutes. Costly for any deep market; for thin alts the maintainer should reduce `maxLeverage` or refuse listing.

**OPEN-01 (informational, to add before mainnet):**

```solidity
require(block.timestamp - updatedAt <= STALE_THRESHOLD, "stale oracle");
```

### Math invariants

- `WAD = 1e18` constant; all financial state in WAD.
- USDC has 6 decimals; scale factor `WAD / 1e6 = 1e12` is exact and applied uniformly at the IO boundary.
- Funding rate stored as `int256` in pp1e8 (1 = 0.000001%) with explicit `FUNDING_CLAMP_PP1E8 = 5_000` (±0.05% per hour).
- HLP share math: `shares = assetsWad * hlpShares / hlpAssets`. Order safe (multiply before divide).
- Bootstrap emissions: `bonus = applied × baseEmissionsPerUsdc × multiplier_bps / (1e6 × BPS)`. Three-multiplicand product, no intermediate divide.

**Status:** Clear.

### DoS / griefing

- `bootstrapClaim()` iterates `bootstrapVests[msg.sender]`. A griefer could push many small vests against another address... except every entry comes from a deposit that pays USDC. There is no free path to spam vests on someone else's address — only referrer kickbacks generate ref-side entries, and each one costs the depositor real USDC.

  **Worst case:** depositor splits $1M cap into 1M individual $1 deposits, each creating a vest. Cost: ~$1M USDC + 1M txs (gas-prohibitive on mainnet). Realistically capped by the bootstrap cap itself.

- `sweepLiquidations()` is gas-bounded (`max ≤ 3` per swap). No unbounded loop.

**Status:** Clear.

### Admin surface

- `INIT_ADMIN` (deployer) can only call `addMarket` and `lockMarkets`. After `lockMarkets()`, both functions revert forever. No escape hatch.
- `founder` is the only address that can call `seedFounderDeposit`, and only once.
- `HOOK` is the only address that can call hook-only methods (funding settle, mark record, liq sweep, donate).
- No `Ownable`, no `Pausable`, no proxy, no upgrade path. Confirmed by grep.

**Status:** Minimal. No admin can rug.

### Funding settlement / liquidation queue race

`liquidate()` is permissionless; the hook also calls `sweepLiquidations()` on every swap. Race window: a position becomes liquidatable in block N; both an external keeper and the hook may try to liquidate it in N or N+1.

- The internal `_liquidateInternal()` reads fresh storage each call (no cached state).
- The first caller closes the position (`p.open = false`); the second gets a no-op revert from `require(p.open)`. No double-payout, no double-OI-unwind.

**Status:** Clear.

### Buyback path

`executeBuyback()` currently stubs the actual swap (commented TODO). When wired to AMM, the function must:

- Cap `usdcIn` per call to avoid sandwich attacks (~$100K max recommended).
- Use a min-out check (slippage tolerance).
- Verify swap output before transferring to `0xdead`.

**OPEN-02 (must address before TGE):** wire `executeBuyback()` to the AMM with proper slippage protection.

---

## Open items (to fix before mainnet)

| ID | Title | Priority |
|---|---|---|
| OPEN-01 | Chainlink staleness check in `_oracle()` | medium |
| OPEN-02 | Wire `executeBuyback()` to AMM swap with slippage protection | medium |
| OPEN-03 | Subgraph for trade/liq/funding history (off-chain, but required for UI) | low |
| OPEN-04 | Fuzz tests on `_calcPnL` and `settleFunding` rate math | low |

## Test coverage

```
forge test
Ran 3 test suites: 34 passed, 0 failed, 0 skipped

test/HelixPerps.t.sol:HelixPerpsTest        10 passed
test/HelixHook.t.sol:HelixHookTest           7 passed
test/HelixBootstrap.t.sol:HelixBootstrapTest 17 passed
```

Coverage includes:
- Margin deposit/withdraw, scaling between USDC and WAD
- Open/close at same price (only maker fee deducted)
- HLP deposit increases assets+shares
- Hook gating on settleFunding/recordMark/donateHLP/sweepLiquidations
- Bootstrap window active state
- Bootstrap deposit with full vest
- Bootstrap cap fill + overflow to base rate
- Bootstrap post-end falls to base only
- Referrer kickback math
- Self-referral suppression
- bootstrapClaim transfers correct HELIX, second call pays 0
- Founder seed (sets lock, only once, only founder)
- Founder lock blocks early withdraw, releases at 90 days
- Insurance fund cap constant
- Markets registry: addMarket onlyInitAdmin, lockMarkets one-shot

## Recommendations for external auditors

1. **Focus on the v4 hook bindings.** The `HelixHook` glue is the only piece that touches untrusted code (the Uniswap PoolManager). Validate that no path through `beforeSwap`/`afterSwap` can be made to reenter or starve the perp engine.

2. **Funding rate math under extreme skew.** The funding clamp at ±0.05%/h is the only thing capping divergence from oracle. Verify clamping logic survives integer overflow in extreme cases.

3. **Bootstrap emissions exhaustion.** Confirm `bootstrapEmissionsBudget` decrement underflow is impossible (`require(budget >= total)` guard exists; tests pass).

4. **HLP withdrawal under sustained drawdown.** Verify the gate's 8% threshold cannot be gamed by attackers running positions that briefly push the vault past peak then withdrawing.

5. **Multi-market funding cross-contamination.** Position margins are cross. Confirm that a runaway funding rate in one market cannot drain margin reserved for another.

---

## Build status

- `forge build` — clean (lint warnings only, no errors)
- `forge test` — 34/34 passing
- `slither .` — 0 high, 1 medium (fixed), 4 low (acknowledged or fixed), many info

Source state of this report: contracts compile and pass tests at HEAD.