Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jayeshy14 merged 1 commit into from
May 30, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions src/Vault.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ pragma solidity ^0.8.24;
import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import { ERC4626 } from "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { ReentrancyGuard } from "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
import { Math } from "@openzeppelin/contracts/utils/math/Math.sol";

import { IDiamond } from "./interfaces/IDiamond.sol";
import { Diamond } from "./Diamond.sol";
Expand All @@ -19,7 +21,7 @@ import { LibGuard } from "./libraries/LibGuard.sol";
/// @dev Inflation attack mitigation comes from OZ ERC-4626's `_decimalsOffset`
/// virtual shares. The ERC-4626 surface is native (non-facet) and therefore
/// non-upgradeable, so it cannot be altered by a later diamondCut.
contract Vault is Diamond, ERC4626 {
contract Vault is Diamond, ERC4626, ReentrancyGuard {
error StrategyTotalAssetsCallFailed(bytes32 strategyId);

constructor(
Expand Down Expand Up @@ -70,7 +72,7 @@ contract Vault is Diamond, ERC4626 {
// Fee-accrual hooks
// -----------------------------------------------------------------------

function _deposit(address caller, address receiver, uint256 assets, uint256 shares) internal override {
function _deposit(address caller, address receiver, uint256 assets, uint256 shares) internal override nonReentrant {
// Circuit breaker: revert if paused, or if the current share price has
// moved beyond the configured bound since the last checkpoint.
_guard();
Expand All @@ -93,6 +95,7 @@ contract Vault is Diamond, ERC4626 {
)
internal
override
nonReentrant
{
_guard();
_accrueFees();
Expand Down Expand Up @@ -148,10 +151,13 @@ contract Vault is Diamond, ERC4626 {

// Performance fee — proportional to share-price gain above HWM.
if (f.performanceFeeBps > 0 && sharePrice > f.highWaterMark) {
// Full-precision mulDiv throughout: each step multiplies at 512-bit
// width before dividing, so no intermediate truncation feeds the next
// multiplication (fixes the divide-before-multiply rounding).
uint256 profitPerShare = sharePrice - f.highWaterMark;
uint256 profitValue = (profitPerShare * supply) / 1e18;
uint256 feeValue = (profitValue * uint256(f.performanceFeeBps)) / LibFees.BPS_DENOMINATOR;
if (ta > 0) feeShares += (feeValue * supply) / ta;
uint256 profitValue = Math.mulDiv(profitPerShare, supply, 1e18);
uint256 feeValue = Math.mulDiv(profitValue, uint256(f.performanceFeeBps), LibFees.BPS_DENOMINATOR);
if (ta > 0) feeShares += Math.mulDiv(feeValue, supply, ta);
f.highWaterMark = sharePrice;
}

Expand Down
6 changes: 5 additions & 1 deletion src/facets/strategies/MorphoStrategyFacet.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,11 @@ contract MorphoStrategyFacet {
function morphoWithdraw(uint256 amount) external {
MorphoStorage storage s = _ms();
if (address(s.vault) == address(0)) revert MorphoVaultNotConfigured();
s.vault.withdraw(amount, address(this), address(this));
// Bound the shares burned: burning more than previewWithdraw predicted
// means a worse-than-quoted price. Mirrors the morphoDeposit slippage check.
uint256 expected = s.vault.previewWithdraw(amount);
uint256 shares = s.vault.withdraw(amount, address(this), address(this));
if (shares > expected) revert MorphoSlippage(expected, shares);
}

/// @notice No-op for Metamorpho — supply yield auto-compounds into the
Expand Down
80 changes: 68 additions & 12 deletions src/facets/strategies/PendlePtStrategyFacet.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ contract PendlePtStrategyFacet {
/// zero TWAP duration.
error PendleInvalidOracle();

/// @notice Thrown when a configured slippage tolerance exceeds 100%.
error PendleInvalidSlippage(uint16 bps);

// -----------------------------------------------------------------------
// Events
// -----------------------------------------------------------------------
Expand All @@ -73,13 +76,22 @@ contract PendlePtStrategyFacet {
/// @notice Emitted when the mark-to-market oracle is set (or cleared).
event PendleOracleSet(address indexed oracle, uint32 twapDuration);

/// @notice Emitted when the max AMM slippage tolerance is set.
event PendleSlippageSet(uint16 maxSlippageBps);

// -----------------------------------------------------------------------
// Storage
// -----------------------------------------------------------------------

/// @dev erc7201:vaultrouter.strategy.pendle
bytes32 internal constant PENDLE_STORAGE_SLOT = 0xb0e016db49ce2cfbe35770c2200cbf5f1a9b502bca57dbaaddf328cb9e0cef00;

/// @dev Basis-points denominator.
uint16 internal constant PENDLE_BPS = 10_000;

/// @dev Slippage tolerance (bps) used when none is explicitly configured: 1%.
uint16 internal constant DEFAULT_MAX_SLIPPAGE_BPS = 100;

struct PendleStorage {
/// @notice PendleRouterV4 — handles all swap and redemption paths.
IPendleRouter router;
Expand All @@ -92,6 +104,9 @@ contract PendlePtStrategyFacet {
IPYLpOracle oracle;
/// @notice TWAP window (seconds) passed to the oracle.
uint32 twapDuration;
/// @notice Max AMM slippage tolerance (bps) applied against the oracle
/// mark when deriving minOut for swaps. Zero => 1% default.
uint16 maxSlippageBps;
}

function _ps() internal pure returns (PendleStorage storage s) {
Expand Down Expand Up @@ -146,6 +161,24 @@ contract PendlePtStrategyFacet {
emit PendleOracleSet(address(oracle), twapDuration);
}

/// @notice Set the maximum AMM slippage tolerance (bps) for pre-maturity
/// swaps, applied against the oracle mark when deriving minOut.
/// @dev Owner-gated. Only enforceable when an oracle is configured — without
/// an on-chain mark there is no reference to bound the swap against.
/// @param bps Tolerance in basis points (<= 10_000). Zero selects the 1% default.
function pendleSetSlippage(uint16 bps) external {
LibDiamond.enforceIsContractOwner();
if (bps > PENDLE_BPS) revert PendleInvalidSlippage(bps);
_ps().maxSlippageBps = bps;
emit PendleSlippageSet(bps);
}

/// @dev Effective slippage tolerance: the configured value, or the 1% default.
function _maxSlippageBps(PendleStorage storage s) private view returns (uint16) {
uint16 bps = s.maxSlippageBps;
return bps == 0 ? DEFAULT_MAX_SLIPPAGE_BPS : bps;
}

// -----------------------------------------------------------------------
// Strategy surface (pendle* prefix)
// -----------------------------------------------------------------------
Expand Down Expand Up @@ -209,20 +242,32 @@ contract PendlePtStrategyFacet {
// Empty limit order, strategy does not participate in the limit book.
IPendleRouter.LimitOrderData memory limit;

uint256 ptBefore = s.pt.balanceOf(address(this));
// Derive an on-chain minimum from the oracle mark when available: invert
// the PT->asset rate to value `amount` in PT, then haircut by the
// slippage tolerance. The router reverts if it cannot meet minPtOut.
// Without an oracle there is no mark to bound against, so we fall back to
// 0 (unchanged behaviour for unpriced markets) and rely on the post-call
// zero check.
uint256 minPtOut;
if (address(s.oracle) != address(0)) {
uint256 rate = s.oracle.getPtToAssetRate(s.market, s.twapDuration);
if (rate > 0) {
uint256 expectedPt = amount * 1e18 / rate;
minPtOut = expectedPt * (PENDLE_BPS - _maxSlippageBps(s)) / PENDLE_BPS;
}
}

s.router
(uint256 netPtOut,,) = s.router
.swapExactTokenForPt(
address(this), // PT receiver is the vault itself
s.market,
0, // minPtOut: checked post-call below
minPtOut,
approx,
input,
limit
);

uint256 ptReceived = s.pt.balanceOf(address(this)) - ptBefore;
if (ptReceived == 0) revert PendleDepositFailed(ptReceived);
if (netPtOut == 0) revert PendleDepositFailed(netPtOut);
}

/// @notice Return `amount` of underlying from the Pendle position to the vault.
Expand All @@ -242,10 +287,11 @@ contract PendlePtStrategyFacet {
if (amount > ptBalance) revert PendleInsufficientPt(amount, ptBalance);

IERC20 underlying = IERC20(IERC4626(address(this)).asset());
uint256 underlyingBefore = underlying.balanceOf(address(this));

IERC20(address(s.pt)).forceApprove(address(s.router), amount);

uint256 received;

if (s.pt.isExpired()) {
// Post-maturity: PT redeems 1:1. minTokenOut = 99% (dust tolerance).
IPendleRouter.TokenOutput memory output = IPendleRouter.TokenOutput({
Expand All @@ -259,13 +305,24 @@ contract PendlePtStrategyFacet {
});

// redeemPyToToken burns PT (YT is implicitly 0 post-maturity).
s.router.redeemPyToToken(address(this), s.pt.YT(), amount, output);
(received,) = s.router.redeemPyToToken(address(this), s.pt.YT(), amount, output);
} else {
// Pre-maturity: sell PT on the Pendle AMM.
// minTokenOut = 0 here; slippage is validated post-call.
// Pre-maturity: sell PT on the Pendle AMM. Derive minTokenOut from the
// oracle mark when available (PT->asset rate, haircut by slippage) so
// the router itself enforces the bound; fall back to 0 only when no
// oracle is configured (unchanged behaviour for unpriced markets).
uint256 minTokenOut;
if (address(s.oracle) != address(0)) {
uint256 rate = s.oracle.getPtToAssetRate(s.market, s.twapDuration);
if (rate > 0) {
uint256 expected = amount * rate / 1e18;
minTokenOut = expected * (PENDLE_BPS - _maxSlippageBps(s)) / PENDLE_BPS;
}
}

IPendleRouter.TokenOutput memory output = IPendleRouter.TokenOutput({
tokenOut: address(underlying),
minTokenOut: 0,
minTokenOut: minTokenOut,
tokenRedeemSy: address(underlying),
pendleSwap: address(0),
swapData: IPendleRouter.SwapData({
Expand All @@ -275,10 +332,9 @@ contract PendlePtStrategyFacet {

IPendleRouter.LimitOrderData memory limit;

s.router.swapExactPtForToken(address(this), s.market, amount, output, limit);
(received,,) = s.router.swapExactPtForToken(address(this), s.market, amount, output, limit);
}

uint256 received = underlying.balanceOf(address(this)) - underlyingBefore;
if (received == 0) revert PendleWithdrawFailed(amount, received);
}

Expand Down
81 changes: 78 additions & 3 deletions test/unit/PendleStrategy.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,52 @@ contract PendleStrategyTest is Test {
PendlePtStrategyFacet(address(vault)).pendleDeposit(1000 * 1e6);
}

function test_Deposit_RevertsWhenFillBelowOracleSlippageBound() public {
_configure();
_setOracle(0.95e18); // oracle: 1 PT = 0.95 USDC -> a buy should yield ~1.053x PT
uint256 amount = 1000 * 1e6;
usdc.mint(address(vault), amount);

// Router fills at par (1000 PT) — ~5% worse than the oracle-implied amount,
// beyond the default 1% tolerance. The router enforces our derived minPtOut.
vm.expectRevert("MockPendle: minPtOut");
PendlePtStrategyFacet(address(vault)).pendleDeposit(amount);
}

function test_Deposit_SucceedsWhenFillWithinOracleSlippageBound() public {
_configure();
_setOracle(0.95e18);
router.setDepositRateBps(10_500); // 1050 PT, within 1% of the ~1052.6 oracle mark
uint256 amount = 1000 * 1e6;
usdc.mint(address(vault), amount);

PendlePtStrategyFacet(address(vault)).pendleDeposit(amount);
assertEq(pt.balanceOf(address(vault)), (amount * 10_500) / 10_000, "fill within tolerance accepted");
}

// -----------------------------------------------------------------------
// Slippage config — gating & validation
// -----------------------------------------------------------------------

function test_SetSlippage_SetsAndEmits() public {
vm.expectEmit(false, false, false, true, address(vault));
emit PendlePtStrategyFacet.PendleSlippageSet(250);
vm.prank(owner);
PendlePtStrategyFacet(address(vault)).pendleSetSlippage(250);
}

function test_SetSlippage_RevertsAboveBps() public {
vm.prank(owner);
vm.expectRevert(abi.encodeWithSelector(PendlePtStrategyFacet.PendleInvalidSlippage.selector, 10_001));
PendlePtStrategyFacet(address(vault)).pendleSetSlippage(10_001);
}

function test_SetSlippage_RevertsForNonOwner() public {
vm.prank(alice);
vm.expectRevert(abi.encodeWithSelector(LibDiamond.NotContractOwner.selector, alice, owner));
PendlePtStrategyFacet(address(vault)).pendleSetSlippage(100);
}

// -----------------------------------------------------------------------
// Withdraw — pre-maturity (sell on AMM) vs post-maturity (redeem 1:1)
// -----------------------------------------------------------------------
Expand Down Expand Up @@ -252,6 +298,30 @@ contract PendleStrategyTest is Test {
PendlePtStrategyFacet(address(vault)).pendleWithdraw(400 * 1e6);
}

function test_Withdraw_RevertsWhenAmmHaircutExceedsSlippageBound() public {
_configure();
uint256 amount = 1000 * 1e6;
usdc.mint(address(vault), amount);
PendlePtStrategyFacet(address(vault)).pendleDeposit(amount);
_setOracle(1e18); // par mark: minTokenOut derived as 99% of PT sold

router.setWithdrawHaircutBps(500); // 5% AMM haircut, beyond the 1% tolerance
vm.expectRevert("MockPendle: minTokenOut");
PendlePtStrategyFacet(address(vault)).pendleWithdraw(400 * 1e6);
}

function test_Withdraw_SucceedsWhenHaircutWithinSlippageBound() public {
_configure();
uint256 amount = 1000 * 1e6;
usdc.mint(address(vault), amount);
PendlePtStrategyFacet(address(vault)).pendleDeposit(amount);
_setOracle(1e18);

router.setWithdrawHaircutBps(50); // 0.5% haircut, within the 1% tolerance
PendlePtStrategyFacet(address(vault)).pendleWithdraw(400 * 1e6);
assertEq(usdc.balanceOf(address(vault)), (400 * 1e6 * 9950) / 10_000, "fill within tolerance settled");
}

// -----------------------------------------------------------------------
// Oracle config — gating & validation
// -----------------------------------------------------------------------
Expand Down Expand Up @@ -291,10 +361,14 @@ contract PendleStrategyTest is Test {

function test_TotalAssets_MarksToMarketWhenOracleSet() public {
_configure();
_setOracle(0.95e18); // PT marked at a 5% discount pre-maturity
uint256 amount = 1000 * 1e6;
usdc.mint(address(vault), amount);
// Acquire the position at par, then mark it down. The oracle drives
// totalAssets() valuation, not deposit execution, so it is set after the
// buy — a discounted mark against a par fill would otherwise (correctly)
// trip pendleDeposit's slippage guard.
PendlePtStrategyFacet(address(vault)).pendleDeposit(amount);
_setOracle(0.95e18); // PT marked at a 5% discount pre-maturity

// Face value is 1000 USDC, but the oracle marks it down to 950.
assertEq(pt.balanceOf(address(vault)), amount, "holds PT at face");
Expand All @@ -313,10 +387,10 @@ contract PendleStrategyTest is Test {

function test_TotalAssets_PostMaturityIgnoresOracle() public {
_configure();
_setOracle(0.95e18);
uint256 amount = 1000 * 1e6;
usdc.mint(address(vault), amount);
PendlePtStrategyFacet(address(vault)).pendleDeposit(amount);
_setOracle(0.95e18); // set after the buy; see MarksToMarketWhenOracleSet

vm.warp(expiry + 1); // PT now redeems 1:1 — oracle discount must be bypassed

Expand Down Expand Up @@ -512,7 +586,7 @@ contract PendleStrategyTest is Test {
}

function _pendleSelectors() internal pure returns (bytes4[] memory s) {
s = new bytes4[](13);
s = new bytes4[](14);
s[0] = PendlePtStrategyFacet.pendleSetConfig.selector;
s[1] = PendlePtStrategyFacet.pendleTotalAssets.selector;
s[2] = PendlePtStrategyFacet.pendleDeposit.selector;
Expand All @@ -526,5 +600,6 @@ contract PendleStrategyTest is Test {
s[10] = PendlePtStrategyFacet.pendleSetOracle.selector;
s[11] = PendlePtStrategyFacet.pendleOracle.selector;
s[12] = PendlePtStrategyFacet.pendleTwapDuration.selector;
s[13] = PendlePtStrategyFacet.pendleSetSlippage.selector;
}
}