Skip to content

fix: enforce strategy slippage bounds, precise fee math, and vault reentrancy guard#11

Merged
jayeshy14 merged 1 commit into
mainfrom
fix/slither-findings
May 30, 2026
Merged

fix: enforce strategy slippage bounds, precise fee math, and vault reentrancy guard#11
jayeshy14 merged 1 commit into
mainfrom
fix/slither-findings

Conversation

@jayeshy14
Copy link
Copy Markdown
Owner

Summary

Slither-MCP-driven hardening pass. Fixes the four substantive findings (the rest of the detector output was triaged as false positives — trusted-protocol balance-delta reentrancy, zero-guard strict-equality, accumulator uninitialized-locals — and intentionally left).

Fixes

  • Pendle slippage (PendlePtStrategyFacet): added a configurable maxSlippageBps (default 1%) + owner setter pendleSetSlippage. pendleDeposit/pendleWithdraw now derive a real minPtOut/minTokenOut from the oracle mark so the router enforces the bound, and use the router's netPtOut/netTokenOut returns instead of stale balance deltas. Falls back to prior behaviour only when no oracle is configured (unpriced markets).
  • Fee-math precision (Vault._accrueFees): performance-fee math rewritten with Math.mulDiv, eliminating the divide-before-multiply rounding.
  • Reentrancy (Vault): added OZ ReentrancyGuard; _deposit/_withdraw are now nonReentrant. Standard guard (OZ 5.0.2 has no transient variant); safe because the ERC-4626 surface is the non-upgradeable native contract and facets use namespaced storage.
  • morphoWithdraw unused return (MorphoStrategyFacet): capture withdraw's returned shares and bound against previewWithdraw, mirroring the existing morphoDeposit slippage check.

Tests

  • +6 cases covering the new slippage guards (deposit/withdraw revert beyond tolerance, succeed within it) and pendleSetSlippage gating; wired the new selector into the test diamond cut.
  • Two pre-existing totalAssets tests reordered to set the oracle after the seed buy — the new guard correctly rejects buying PT at par against a discounted mark.

Verification

  • forge build clean; forge fmt clean.
  • forge test --no-match-path 'test/integration/*'128 passed, 0 failed.
  • diamond-detectno storage collisions, 8 namespaced regions each on its own slot.

@jayeshy14 jayeshy14 merged commit 09c55d9 into main May 30, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant