From f94f2ec995f388373b698cec1864e060652b83c4 Mon Sep 17 00:00:00 2001 From: Jayesh Yadav Date: Sun, 31 May 2026 13:18:36 +0530 Subject: [PATCH 1/3] feat(allocator): bound per-rebalance churn and assert idle floor --- src/facets/AllocatorFacet.sol | 56 +++++++++++++++++++- src/libraries/LibAllocator.sol | 5 ++ test/unit/Allocator.t.sol | 93 +++++++++++++++++++++++++++++++++- 3 files changed, 152 insertions(+), 2 deletions(-) diff --git a/src/facets/AllocatorFacet.sol b/src/facets/AllocatorFacet.sol index a2d0ad6..623aa9e 100644 --- a/src/facets/AllocatorFacet.sol +++ b/src/facets/AllocatorFacet.sol @@ -29,6 +29,8 @@ contract AllocatorFacet { error StrategyAlreadyQuarantined(bytes32 strategyId); error StrategyNotQuarantined(bytes32 strategyId); error AllocationToQuarantined(bytes32 strategyId); + error RebalanceDeltaTooLarge(uint256 movementBps, uint16 maxBps); + error IdleReserveBreached(uint256 idleAfter, uint256 requiredIdle); event StrategyRegistered(bytes32 indexed strategyId, LibAllocator.StrategyConfig config); event StrategyRemoved(bytes32 indexed strategyId); @@ -39,6 +41,7 @@ contract AllocatorFacet { event Rebalanced(uint256 totalAssets, uint256 idleAfter); event StrategyQuarantined(bytes32 indexed strategyId); event StrategyReleased(bytes32 indexed strategyId); + event MaxRebalanceDeltaSet(uint16 bps); // ----------------------------------------------------------------------- // Owner-gated governance / risk bounds @@ -143,6 +146,22 @@ contract AllocatorFacet { emit GlobalStrategyCapSet(capBps); } + /// @notice Cap the total churn a single `rebalance` may move — the sum of + /// |delta| across all strategies — as bps of NAV. This bounds *how + /// much* a rebalance can relocate, complementing the role gate + /// (*who*) and the one-per-block throttle (*how often*). + /// @dev Owner-gated risk bound. `0` disables the check. Because a full + /// reshuffle moves each relocated dollar twice (out of one strategy and + /// into another), movement can reach 2x NAV (20_000 bps); the setter + /// therefore admits values up to 2 * BPS_DENOMINATOR. + /// @param bps Max movement in basis points of NAV (0 = disabled). + function setMaxRebalanceDelta(uint16 bps) external { + LibDiamond.enforceIsContractOwner(); + if (bps > 2 * LibAllocator.BPS_DENOMINATOR) revert InvalidBps(bps); + LibAllocator.allocatorStorage().maxRebalanceDeltaBps = bps; + emit MaxRebalanceDeltaSet(bps); + } + /// @notice Isolate a strategy whose accounting can no longer be trusted (a /// failing, exploited, or stuck protocol). A quarantined strategy is /// excluded from `totalAssets` and skipped by the rebalancer and @@ -205,6 +224,28 @@ contract AllocatorFacet { totalCached += cur; } + // Per-call churn bound: sum |delta| across strategies and reject the + // rebalance if it exceeds maxRebalanceDeltaBps of NAV. Evaluated before + // any funds move, so an over-large reshuffle never partially executes. + // 0 disables the bound. + uint16 maxDelta = s.maxRebalanceDeltaBps; + if (maxDelta != 0) { + uint256 totalMovement; + for (uint256 i; i < n; i++) { + bytes32 id = s.strategyIds[i]; + if (s.quarantined[id]) continue; + uint256 target = (totalCached * uint256(s.targetBps[id])) / LibAllocator.BPS_DENOMINATOR; + uint256 cur = currentAssets[i]; + totalMovement += cur > target ? cur - target : target - cur; + } + // Cross-multiply to avoid division and the totalCached == 0 case. + if (totalMovement * LibAllocator.BPS_DENOMINATOR > totalCached * uint256(maxDelta)) { + uint256 movementBps = + totalCached == 0 ? 0 : (totalMovement * LibAllocator.BPS_DENOMINATOR) / totalCached; + revert RebalanceDeltaTooLarge(movementBps, maxDelta); + } + } + // Pass 1: withdraw from over-target strategies. for (uint256 i; i < n; i++) { bytes32 id = s.strategyIds[i]; @@ -227,7 +268,16 @@ contract AllocatorFacet { } } - emit Rebalanced(totalCached, _idleAssetsInternal()); + // Defense-in-depth: the idle reserve floor follows from + // `total + idleReserveBps <= 10_000` in setAllocation, but assert it on + // realized balances too so any accounting/slippage drift that would dip + // idle below the floor reverts the whole rebalance rather than silently + // under-reserving. + uint256 idleAfter = _idleAssetsInternal(); + uint256 requiredIdle = (totalCached * uint256(s.idleReserveBps)) / LibAllocator.BPS_DENOMINATOR; + if (idleAfter < requiredIdle) revert IdleReserveBreached(idleAfter, requiredIdle); + + emit Rebalanced(totalCached, idleAfter); } // ----------------------------------------------------------------------- @@ -277,6 +327,10 @@ contract AllocatorFacet { return LibAllocator.allocatorStorage().lastRebalanceBlock; } + function maxRebalanceDelta() external view returns (uint16) { + return LibAllocator.allocatorStorage().maxRebalanceDeltaBps; + } + function isQuarantined(bytes32 strategyId) external view returns (bool) { return LibAllocator.allocatorStorage().quarantined[strategyId]; } diff --git a/src/libraries/LibAllocator.sol b/src/libraries/LibAllocator.sol index 0aa202c..5ff67db 100644 --- a/src/libraries/LibAllocator.sol +++ b/src/libraries/LibAllocator.sol @@ -34,6 +34,11 @@ library LibAllocator { uint16 idleReserveBps; uint16 globalMaxStrategyCapBps; uint64 lastRebalanceBlock; + /// @dev Per-call rebalance churn bound: caps the sum of |delta| moved + /// across all strategies in a single rebalance, as bps of NAV. So + /// even a compromised curator/AI key can only relocate a bounded, + /// throttled fraction of the vault per block. 0 disables the bound. + uint16 maxRebalanceDeltaBps; /// @dev Strategies flagged here are isolated: excluded from NAV and /// skipped by the rebalancer/harvester, so a single failing protocol /// cannot brick the whole vault. Owner-controlled risk state, kept diff --git a/test/unit/Allocator.t.sol b/test/unit/Allocator.t.sol index c5f9fbc..29a08f7 100644 --- a/test/unit/Allocator.t.sol +++ b/test/unit/Allocator.t.sol @@ -16,6 +16,7 @@ import { OwnershipFacet } from "../../src/facets/OwnershipFacet.sol"; import { IdleStrategyFacet } from "../../src/facets/strategies/IdleStrategyFacet.sol"; import { AllocatorFacet } from "../../src/facets/AllocatorFacet.sol"; import { LibAllocator } from "../../src/libraries/LibAllocator.sol"; +import { LibDiamond } from "../../src/libraries/LibDiamond.sol"; import { MockProtocol } from "../mocks/MockProtocol.sol"; import { MockStrategyFacet } from "../mocks/MockStrategyFacet.sol"; @@ -223,6 +224,94 @@ contract AllocatorTest is Test { assertEq(mockProtocol.balanceOf(address(vault)), 750 * 1e6, "75% deployed"); } + // ----------------------------------------------------------------------- + // setMaxRebalanceDelta — gating & validation + // ----------------------------------------------------------------------- + + function test_SetMaxRebalanceDelta_SetsAndEmits() public { + vm.expectEmit(false, false, false, true, address(vault)); + emit AllocatorFacet.MaxRebalanceDeltaSet(3000); + vm.prank(owner); + AllocatorFacet(address(vault)).setMaxRebalanceDelta(3000); + assertEq(AllocatorFacet(address(vault)).maxRebalanceDelta(), 3000, "stored and readable"); + } + + function test_SetMaxRebalanceDelta_AllowsUpToTwiceBps() public { + vm.prank(owner); + AllocatorFacet(address(vault)).setMaxRebalanceDelta(20_000); // full reshuffle = 2x NAV churn + assertEq(AllocatorFacet(address(vault)).maxRebalanceDelta(), 20_000); + } + + function test_SetMaxRebalanceDelta_RevertsAboveTwiceBps() public { + vm.prank(owner); + vm.expectRevert(abi.encodeWithSelector(AllocatorFacet.InvalidBps.selector, 20_001)); + AllocatorFacet(address(vault)).setMaxRebalanceDelta(20_001); + } + + function test_SetMaxRebalanceDelta_RevertsForNonOwner() public { + vm.prank(alice); + vm.expectRevert(abi.encodeWithSelector(LibDiamond.NotContractOwner.selector, alice, owner)); + AllocatorFacet(address(vault)).setMaxRebalanceDelta(3000); + } + + function test_MaxRebalanceDelta_DefaultsToZeroDisabled() public view { + assertEq(AllocatorFacet(address(vault)).maxRebalanceDelta(), 0, "off by default"); + } + + // ----------------------------------------------------------------------- + // rebalance — per-call churn bound + // ----------------------------------------------------------------------- + + function test_Rebalance_RevertsWhenMovementExceedsBound() public { + _depositToVault(alice, 1000 * 1e6); + vm.prank(owner); + AllocatorFacet(address(vault)).setMaxRebalanceDelta(5000); // cap churn at 50% of NAV + _setSingleAllocation(MOCK_ID, 8000); // wants to move 80% (0 -> 800) in one go + + vm.roll(block.number + 1); + vm.prank(owner); + // 800 moved / 1000 NAV = 8000 bps > 5000 bound. + vm.expectRevert(abi.encodeWithSelector(AllocatorFacet.RebalanceDeltaTooLarge.selector, 8000, 5000)); + AllocatorFacet(address(vault)).rebalance(); + + // Nothing moved — the bound is evaluated before any funds are touched. + assertEq(mockProtocol.balanceOf(address(vault)), 0, "no partial execution"); + assertEq(usdc.balanceOf(address(vault)), 1000 * 1e6, "all assets still idle"); + } + + function test_Rebalance_SucceedsWhenMovementAtBound() public { + _depositToVault(alice, 1000 * 1e6); + vm.prank(owner); + AllocatorFacet(address(vault)).setMaxRebalanceDelta(8000); // exactly the move we make + _setSingleAllocation(MOCK_ID, 8000); + + vm.roll(block.number + 1); + vm.prank(owner); + AllocatorFacet(address(vault)).rebalance(); // 8000 == bound, allowed (strict >) + + assertEq(mockProtocol.balanceOf(address(vault)), 800 * 1e6, "moved exactly to target"); + } + + function test_Rebalance_AllowsSmallIncrementalMoveUnderTightBound() public { + // Seed a 50% position with the bound off. + _depositToVault(alice, 1000 * 1e6); + _setSingleAllocation(MOCK_ID, 5000); + vm.roll(block.number + 1); + vm.prank(owner); + AllocatorFacet(address(vault)).rebalance(); + assertEq(mockProtocol.balanceOf(address(vault)), 500 * 1e6, "seeded at 50%"); + + // Now arm a tight 10% churn bound and nudge 50% -> 55% (5% move). + vm.prank(owner); + AllocatorFacet(address(vault)).setMaxRebalanceDelta(1000); + _setSingleAllocation(MOCK_ID, 5500); + vm.roll(block.number + 1); + vm.prank(owner); + AllocatorFacet(address(vault)).rebalance(); // 50 moved / 1000 = 500 bps < 1000 + + assertEq(mockProtocol.balanceOf(address(vault)), 550 * 1e6, "incremental nudge applied"); + } + // ----------------------------------------------------------------------- // helpers // ----------------------------------------------------------------------- @@ -317,7 +406,7 @@ contract AllocatorTest is Test { } function _allocatorSelectors() internal pure returns (bytes4[] memory s) { - s = new bytes4[](13); + s = new bytes4[](15); s[0] = AllocatorFacet.registerStrategy.selector; s[1] = AllocatorFacet.removeStrategy.selector; s[2] = AllocatorFacet.setAllocation.selector; @@ -331,6 +420,8 @@ contract AllocatorTest is Test { s[10] = AllocatorFacet.idleReserveBps.selector; s[11] = AllocatorFacet.strategyTotalAssets.selector; s[12] = AllocatorFacet.idleAssets.selector; + s[13] = AllocatorFacet.setMaxRebalanceDelta.selector; + s[14] = AllocatorFacet.maxRebalanceDelta.selector; } function _mockSelectors() internal pure returns (bytes4[] memory s) { From f5b07dabb4868a04af4f53bef2d08f89c1c771f2 Mon Sep 17 00:00:00 2001 From: Jayesh Yadav Date: Sun, 31 May 2026 15:41:02 +0530 Subject: [PATCH 2/3] feat(vault): add post-deposit share-lock window --- src/Vault.sol | 22 ++++ src/facets/LockFacet.sol | 42 +++++++ src/libraries/LibLock.sol | 45 ++++++++ test/unit/ShareLock.t.sol | 231 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 340 insertions(+) create mode 100644 src/facets/LockFacet.sol create mode 100644 src/libraries/LibLock.sol create mode 100644 test/unit/ShareLock.t.sol diff --git a/src/Vault.sol b/src/Vault.sol index b091cde..4f64c6a 100644 --- a/src/Vault.sol +++ b/src/Vault.sol @@ -12,6 +12,7 @@ import { Diamond } from "./Diamond.sol"; import { LibAllocator } from "./libraries/LibAllocator.sol"; import { LibFees } from "./libraries/LibFees.sol"; import { LibGuard } from "./libraries/LibGuard.sol"; +import { LibLock } from "./libraries/LibLock.sol"; /// @title Vault Router is a modular ERC-4626 vault on the EIP-2535 Diamond pattern. /// @notice Vault owns the ERC-4626 surface (deposit/withdraw/totalAssets) plus the @@ -84,6 +85,13 @@ contract Vault is Diamond, ERC4626, ReentrancyGuard { // Post-accrue handles the first-deposit bootstrap: now that supply > 0, // initialise HWM and the accrual timestamp. No-op for subsequent deposits. _accrueFees(); + // Lock the freshly minted shares for the configured window so an attacker + // cannot deposit, manipulate, and withdraw within the same block. Skipped + // when the period is 0 (disabled). Re-deposits refresh the window. + LibLock.LockStorage storage l = LibLock.lockStorage(); + if (l.shareLockPeriod > 0) { + l.lockedUntil[receiver] = block.timestamp + uint256(l.shareLockPeriod); + } } function _withdraw( @@ -103,6 +111,20 @@ contract Vault is Diamond, ERC4626, ReentrancyGuard { _accrueFees(); } + /// @dev Single enforcement point for the share lock. Runs on every ERC20 + /// mutation: mints (`from == 0`) are exempt — that is how the lock is + /// armed in `_deposit` — while transfers AND burns of still-locked + /// shares revert. Catching burns here covers withdraw/redeem, and + /// catching transfers stops the transfer-then-withdraw bypass, so no + /// separate check is needed in `_withdraw`. + function _update(address from, address to, uint256 value) internal override { + if (from != address(0)) { + uint256 unlockAt = LibLock.lockStorage().lockedUntil[from]; + if (block.timestamp < unlockAt) revert LibLock.SharesLocked(from, unlockAt); + } + super._update(from, to, value); + } + /// @dev NAV circuit-breaker tripwire run on every deposit/withdraw. Reverts /// when the breaker is latched (`EnforcedPause`) or when the live share /// price deviates beyond the owner-set bound (`SharePriceDeviation`), diff --git a/src/facets/LockFacet.sol b/src/facets/LockFacet.sol new file mode 100644 index 0000000..9860b88 --- /dev/null +++ b/src/facets/LockFacet.sol @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import { LibDiamond } from "../libraries/LibDiamond.sol"; +import { LibLock } from "../libraries/LibLock.sol"; + +/// @title LockFacet +/// @notice Owner-gated configuration and public readers for the share-lock +/// period. The enforcement itself lives in `Vault._update`/`_deposit` +/// (the native ERC-4626 surface needs the ERC20 mutation hooks); this +/// facet owns the configuration surface. +contract LockFacet { + error ShareLockPeriodTooLong(uint64 attempted, uint64 maxPeriod); + + event ShareLockPeriodSet(uint64 period); + + /// @notice Set the share-lock window (seconds). Freshly minted shares from a + /// deposit cannot be moved until this window elapses. + /// @dev Owner-gated risk bound. Capped at `MAX_SHARE_LOCK_PERIOD` so it + /// stays an anti-MEV measure rather than a withdrawal freeze. `0` + /// disables the lock for future deposits. + /// @param period Lock duration in seconds (0 = disabled). + function setShareLockPeriod(uint64 period) external { + LibDiamond.enforceIsContractOwner(); + if (period > LibLock.MAX_SHARE_LOCK_PERIOD) { + revert ShareLockPeriodTooLong(period, LibLock.MAX_SHARE_LOCK_PERIOD); + } + LibLock.lockStorage().shareLockPeriod = period; + emit ShareLockPeriodSet(period); + } + + /// @notice The configured share-lock window in seconds (0 = disabled). + function shareLockPeriod() external view returns (uint64) { + return LibLock.lockStorage().shareLockPeriod; + } + + /// @notice Timestamp until which `account`'s shares are locked. A move is + /// blocked while `block.timestamp` is below this value. + function lockedUntil(address account) external view returns (uint256) { + return LibLock.lockStorage().lockedUntil[account]; + } +} diff --git a/src/libraries/LibLock.sol b/src/libraries/LibLock.sol new file mode 100644 index 0000000..390482d --- /dev/null +++ b/src/libraries/LibLock.sol @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +/// @title LibLock +/// @notice Namespaced storage for the share-lock period — a short window after +/// a deposit during which the freshly minted shares cannot be moved +/// (transferred, withdrawn, or redeemed). This defeats atomic +/// deposit -> manipulate -> withdraw flashloan/MEV attacks: an attacker +/// cannot mint and unwind shares within the same block while the lock +/// holds. +/// @dev Enforcement lives in `Vault._update` (the single ERC20 mutation hook), +/// which catches both transfers and burns so the lock cannot be bypassed +/// by transferring shares out and withdrawing from another account. The +/// lock is set on the deposit receiver in `Vault._deposit`. A +/// `shareLockPeriod` of 0 disables it. +/// Storage location: +/// keccak256(abi.encode(uint256(keccak256("vaultrouter.storage.lock")) - 1)) & ~bytes32(uint256(0xff)) +library LibLock { + /// @dev Upper bound on the lock window: keeps it a short anti-MEV measure, + /// not a withdrawal freeze that could trap user funds. + uint64 internal constant MAX_SHARE_LOCK_PERIOD = 1 days; + + /// @notice Reverted when shares are moved before their lock window elapses. + error SharesLocked(address account, uint256 lockedUntil); + + /// @dev erc7201:vaultrouter.storage.lock + bytes32 internal constant LOCK_STORAGE_SLOT = 0x98796fb009fa2d66e5ccc76be36c65ba72c5853e7fe1b4f23987457f018b5800; + + /// @custom:storage-location erc7201:vaultrouter.storage.lock + struct LockStorage { + /// @dev Seconds that freshly minted shares stay locked after a deposit. + /// 0 disables the lock entirely. + uint64 shareLockPeriod; + /// @dev Timestamp until which an account's shares are locked. A move is + /// blocked while `block.timestamp < lockedUntil[account]`. + mapping(address => uint256) lockedUntil; + } + + function lockStorage() internal pure returns (LockStorage storage s) { + bytes32 slot = LOCK_STORAGE_SLOT; + assembly { + s.slot := slot + } + } +} diff --git a/test/unit/ShareLock.t.sol b/test/unit/ShareLock.t.sol new file mode 100644 index 0000000..e575568 --- /dev/null +++ b/test/unit/ShareLock.t.sol @@ -0,0 +1,231 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import { Test } from "forge-std/Test.sol"; +import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; + +import { Vault } from "../../src/Vault.sol"; +import { IDiamond } from "../../src/interfaces/IDiamond.sol"; +import { IDiamondCut } from "../../src/interfaces/IDiamondCut.sol"; +import { IDiamondLoupe } from "../../src/interfaces/IDiamondLoupe.sol"; +import { IERC173 } from "../../src/interfaces/IERC173.sol"; +import { DiamondCutFacet } from "../../src/facets/DiamondCutFacet.sol"; +import { DiamondLoupeFacet } from "../../src/facets/DiamondLoupeFacet.sol"; +import { OwnershipFacet } from "../../src/facets/OwnershipFacet.sol"; +import { LockFacet } from "../../src/facets/LockFacet.sol"; +import { LibDiamond } from "../../src/libraries/LibDiamond.sol"; +import { LibLock } from "../../src/libraries/LibLock.sol"; + +contract MockUSDC is ERC20 { + constructor() ERC20("USD Coin", "USDC") { } + + function mint(address to, uint256 amount) external { + _mint(to, amount); + } + + function decimals() public pure override returns (uint8) { + return 6; + } +} + +/// @title ShareLockTest +/// @notice Unit coverage for the post-deposit share-lock window (Tier 2.5). +/// Verifies the lock arms on deposit, blocks withdraw/redeem AND +/// transfer within the window (the transfer path closes the +/// transfer-then-withdraw bypass), releases after it elapses, and is a +/// no-op when disabled. +contract ShareLockTest is Test { + MockUSDC internal usdc; + Vault internal vault; + + address internal owner = makeAddr("owner"); + address internal alice = makeAddr("alice"); + address internal bob = makeAddr("bob"); + + uint64 internal constant PERIOD = 300; // 5 minutes + + function setUp() public { + usdc = new MockUSDC(); + vault = _deployVault(); + usdc.mint(alice, 1_000_000 * 1e6); + vm.prank(alice); + usdc.approve(address(vault), type(uint256).max); + } + + // ----------------------------------------------------------------------- + // setShareLockPeriod — gating & validation + // ----------------------------------------------------------------------- + + function test_SetShareLockPeriod_SetsAndEmits() public { + vm.expectEmit(false, false, false, true, address(vault)); + emit LockFacet.ShareLockPeriodSet(PERIOD); + vm.prank(owner); + LockFacet(address(vault)).setShareLockPeriod(PERIOD); + assertEq(LockFacet(address(vault)).shareLockPeriod(), PERIOD, "stored and readable"); + } + + function test_SetShareLockPeriod_RevertsAboveMax() public { + uint64 tooLong = uint64(1 days) + 1; + vm.prank(owner); + vm.expectRevert( + abi.encodeWithSelector(LockFacet.ShareLockPeriodTooLong.selector, tooLong, LibLock.MAX_SHARE_LOCK_PERIOD) + ); + LockFacet(address(vault)).setShareLockPeriod(tooLong); + } + + function test_SetShareLockPeriod_RevertsForNonOwner() public { + vm.prank(alice); + vm.expectRevert(abi.encodeWithSelector(LibDiamond.NotContractOwner.selector, alice, owner)); + LockFacet(address(vault)).setShareLockPeriod(PERIOD); + } + + function test_ShareLockPeriod_DefaultsToZeroDisabled() public view { + assertEq(LockFacet(address(vault)).shareLockPeriod(), 0, "off by default"); + } + + // ----------------------------------------------------------------------- + // Lock arming + // ----------------------------------------------------------------------- + + function test_Deposit_ArmsLockForReceiver() public { + _setPeriod(PERIOD); + uint256 expectedUnlock = block.timestamp + PERIOD; + vm.prank(alice); + vault.deposit(1000 * 1e6, alice); + assertEq(LockFacet(address(vault)).lockedUntil(alice), expectedUnlock, "lock armed to now + period"); + } + + function test_Deposit_NoLockWhenDisabled() public { + // Period defaults to 0; deposit then immediate withdraw must succeed. + vm.startPrank(alice); + vault.deposit(1000 * 1e6, alice); + vault.withdraw(1000 * 1e6, alice, alice); + vm.stopPrank(); + assertEq(LockFacet(address(vault)).lockedUntil(alice), 0, "no lock set when disabled"); + } + + // ----------------------------------------------------------------------- + // Enforcement — withdraw / redeem / transfer within the window + // ----------------------------------------------------------------------- + + function test_Withdraw_RevertsWithinLockWindow() public { + _setPeriod(PERIOD); + uint256 unlockAt = block.timestamp + PERIOD; + vm.startPrank(alice); + vault.deposit(1000 * 1e6, alice); + vm.expectRevert(abi.encodeWithSelector(LibLock.SharesLocked.selector, alice, unlockAt)); + vault.withdraw(500 * 1e6, alice, alice); + vm.stopPrank(); + } + + function test_Redeem_RevertsWithinLockWindow() public { + _setPeriod(PERIOD); + uint256 unlockAt = block.timestamp + PERIOD; + vm.startPrank(alice); + uint256 shares = vault.deposit(1000 * 1e6, alice); + vm.expectRevert(abi.encodeWithSelector(LibLock.SharesLocked.selector, alice, unlockAt)); + vault.redeem(shares, alice, alice); + vm.stopPrank(); + } + + function test_Transfer_RevertsWithinLockWindow() public { + _setPeriod(PERIOD); + uint256 unlockAt = block.timestamp + PERIOD; + vm.startPrank(alice); + uint256 shares = vault.deposit(1000 * 1e6, alice); + // Transfer-then-withdraw bypass is closed: the transfer itself reverts. + vm.expectRevert(abi.encodeWithSelector(LibLock.SharesLocked.selector, alice, unlockAt)); + vault.transfer(bob, shares); + vm.stopPrank(); + } + + // ----------------------------------------------------------------------- + // Release — after the window elapses + // ----------------------------------------------------------------------- + + function test_Withdraw_SucceedsAfterLockWindow() public { + _setPeriod(PERIOD); + uint256 unlockAt = block.timestamp + PERIOD; + vm.startPrank(alice); + vault.deposit(1000 * 1e6, alice); + + vm.warp(unlockAt); // strict `<` check: at == unlockAt the shares are free + uint256 before = usdc.balanceOf(alice); + vault.withdraw(1000 * 1e6, alice, alice); + vm.stopPrank(); + assertEq(usdc.balanceOf(alice) - before, 1000 * 1e6, "withdrawal settles after the window"); + } + + function test_Transfer_SucceedsAfterLockWindow() public { + _setPeriod(PERIOD); + vm.startPrank(alice); + uint256 shares = vault.deposit(1000 * 1e6, alice); + vm.warp(block.timestamp + PERIOD + 1); + vault.transfer(bob, shares); + vm.stopPrank(); + assertEq(vault.balanceOf(bob), shares, "transfer allowed once unlocked"); + } + + // ----------------------------------------------------------------------- + // Helpers + // ----------------------------------------------------------------------- + + function _setPeriod(uint64 period) internal { + vm.prank(owner); + LockFacet(address(vault)).setShareLockPeriod(period); + } + + function _deployVault() internal returns (Vault) { + DiamondCutFacet cut = new DiamondCutFacet(); + DiamondLoupeFacet loupe = new DiamondLoupeFacet(); + OwnershipFacet ownership = new OwnershipFacet(); + LockFacet lock = new LockFacet(); + + IDiamond.FacetCut[] memory cuts = new IDiamond.FacetCut[](4); + cuts[0] = IDiamond.FacetCut({ + facetAddress: address(cut), action: IDiamond.FacetCutAction.Add, functionSelectors: _diamondCutSelectors() + }); + cuts[1] = IDiamond.FacetCut({ + facetAddress: address(loupe), + action: IDiamond.FacetCutAction.Add, + functionSelectors: _diamondLoupeSelectors() + }); + cuts[2] = IDiamond.FacetCut({ + facetAddress: address(ownership), + action: IDiamond.FacetCutAction.Add, + functionSelectors: _ownershipSelectors() + }); + cuts[3] = IDiamond.FacetCut({ + facetAddress: address(lock), action: IDiamond.FacetCutAction.Add, functionSelectors: _lockSelectors() + }); + + return new Vault(IERC20(address(usdc)), "Vault Router", "vUSDC", owner, cuts, address(0), ""); + } + + function _diamondCutSelectors() internal pure returns (bytes4[] memory s) { + s = new bytes4[](1); + s[0] = IDiamondCut.diamondCut.selector; + } + + function _diamondLoupeSelectors() internal pure returns (bytes4[] memory s) { + s = new bytes4[](4); + s[0] = IDiamondLoupe.facets.selector; + s[1] = IDiamondLoupe.facetFunctionSelectors.selector; + s[2] = IDiamondLoupe.facetAddresses.selector; + s[3] = IDiamondLoupe.facetAddress.selector; + } + + function _ownershipSelectors() internal pure returns (bytes4[] memory s) { + s = new bytes4[](2); + s[0] = IERC173.owner.selector; + s[1] = IERC173.transferOwnership.selector; + } + + function _lockSelectors() internal pure returns (bytes4[] memory s) { + s = new bytes4[](3); + s[0] = LockFacet.setShareLockPeriod.selector; + s[1] = LockFacet.shareLockPeriod.selector; + s[2] = LockFacet.lockedUntil.selector; + } +} From 82a5f135a504197705453b08e44246c6a827e13e Mon Sep 17 00:00:00 2001 From: Jayesh Yadav Date: Sun, 31 May 2026 15:41:09 +0530 Subject: [PATCH 3/3] fix(strategies): restrict fund-movers to self-dispatch and require oracle for pendle swaps --- src/facets/strategies/AaveStrategyFacet.sol | 2 + src/facets/strategies/MorphoStrategyFacet.sol | 2 + .../strategies/PendlePtStrategyFacet.sol | 53 ++--- src/libraries/LibDiamond.sol | 15 ++ test/integration/PendleStrategy.fork.t.sol | 16 +- test/unit/MorphoStrategy.t.sol | 70 ++++-- test/unit/PendleStrategy.t.sol | 221 ++++++++++-------- 7 files changed, 235 insertions(+), 144 deletions(-) diff --git a/src/facets/strategies/AaveStrategyFacet.sol b/src/facets/strategies/AaveStrategyFacet.sol index 3443f70..9682c08 100644 --- a/src/facets/strategies/AaveStrategyFacet.sol +++ b/src/facets/strategies/AaveStrategyFacet.sol @@ -73,6 +73,7 @@ contract AaveStrategyFacet { /// @dev Called via diamond fallback by the AllocatorFacet during rebalance. /// Verifies aToken balance increased by at least `amount` after supply. function aaveDeposit(uint256 amount) external { + LibDiamond.enforceIsSelf(); AaveStorage storage s = _as(); if (address(s.pool) == address(0)) revert AavePoolNotConfigured(); IERC20 underlying = IERC20(IERC4626(address(this)).asset()); @@ -87,6 +88,7 @@ contract AaveStrategyFacet { /// @dev Checks the actual amount returned by pool.withdraw() — Aave can return /// less than requested if liquidity is insufficient. function aaveWithdraw(uint256 amount) external { + LibDiamond.enforceIsSelf(); AaveStorage storage s = _as(); if (address(s.pool) == address(0)) revert AavePoolNotConfigured(); IERC20 underlying = IERC20(IERC4626(address(this)).asset()); diff --git a/src/facets/strategies/MorphoStrategyFacet.sol b/src/facets/strategies/MorphoStrategyFacet.sol index 587c5e1..cadf641 100644 --- a/src/facets/strategies/MorphoStrategyFacet.sol +++ b/src/facets/strategies/MorphoStrategyFacet.sol @@ -97,6 +97,7 @@ contract MorphoStrategyFacet { /// `previewDeposit(amount)` predicted. /// @param amount Quantity of underlying asset to allocate to Morpho. function morphoDeposit(uint256 amount) external { + LibDiamond.enforceIsSelf(); MorphoStorage storage s = _ms(); if (address(s.vault) == address(0)) revert MorphoVaultNotConfigured(); IERC20 underlying = IERC20(IERC4626(address(this)).asset()); @@ -112,6 +113,7 @@ contract MorphoStrategyFacet { /// of the user-facing redeem path that also burns shares. /// @param amount Quantity of underlying asset to pull out of Morpho. function morphoWithdraw(uint256 amount) external { + LibDiamond.enforceIsSelf(); MorphoStorage storage s = _ms(); if (address(s.vault) == address(0)) revert MorphoVaultNotConfigured(); // Bound the shares burned: burning more than previewWithdraw predicted diff --git a/src/facets/strategies/PendlePtStrategyFacet.sol b/src/facets/strategies/PendlePtStrategyFacet.sol index edb39fd..ce3b7ca 100644 --- a/src/facets/strategies/PendlePtStrategyFacet.sol +++ b/src/facets/strategies/PendlePtStrategyFacet.sol @@ -66,6 +66,11 @@ contract PendlePtStrategyFacet { /// @notice Thrown when a configured slippage tolerance exceeds 100%. error PendleInvalidSlippage(uint16 bps); + /// @notice Thrown when an AMM swap is attempted with no usable oracle mark + /// (oracle unset or reporting a zero rate). Swapping unpriced would + /// force minOut = 0 and leave the trade fully sandwichable. + error PendleOracleRequired(); + // ----------------------------------------------------------------------- // Events // ----------------------------------------------------------------------- @@ -214,6 +219,7 @@ contract PendlePtStrategyFacet { /// or if zero PT is received. /// @param amount Quantity of underlying asset to spend. function pendleDeposit(uint256 amount) external { + LibDiamond.enforceIsSelf(); PendleStorage storage s = _ps(); if (address(s.router) == address(0)) revert PendleNotConfigured(); if (s.pt.isExpired()) revert PendleMarketExpired(); @@ -242,20 +248,16 @@ contract PendlePtStrategyFacet { // Empty limit order, strategy does not participate in the limit book. IPendleRouter.LimitOrderData memory limit; - // 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; - } - } + // Mandatory oracle: refuse to swap unpriced. The oracle mark is the only + // on-chain reference for bounding minPtOut; without it the trade would + // run with minPtOut = 0 and be fully sandwichable. Invert the PT->asset + // rate to value `amount` in PT, then haircut by the slippage tolerance. + // The router reverts if it cannot meet minPtOut. + if (address(s.oracle) == address(0)) revert PendleOracleRequired(); + uint256 rate = s.oracle.getPtToAssetRate(s.market, s.twapDuration); + if (rate == 0) revert PendleOracleRequired(); + uint256 expectedPt = amount * 1e18 / rate; + uint256 minPtOut = expectedPt * (PENDLE_BPS - _maxSlippageBps(s)) / PENDLE_BPS; (uint256 netPtOut,,) = s.router .swapExactTokenForPt( @@ -280,6 +282,7 @@ contract PendlePtStrategyFacet { /// the AMM discount; post-maturity it is 1:1. /// @param amount PT quantity to liquidate (denominated in underlying units). function pendleWithdraw(uint256 amount) external { + LibDiamond.enforceIsSelf(); PendleStorage storage s = _ps(); if (address(s.router) == address(0)) revert PendleNotConfigured(); @@ -307,18 +310,16 @@ contract PendlePtStrategyFacet { // redeemPyToToken burns PT (YT is implicitly 0 post-maturity). (received,) = s.router.redeemPyToToken(address(this), s.pt.YT(), amount, output); } else { - // 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; - } - } + // Pre-maturity: sell PT on the Pendle AMM. Mandatory oracle — derive + // minTokenOut from the mark (PT->asset rate, haircut by slippage) so + // the router enforces the bound. Refuse to sell unpriced rather than + // run with minTokenOut = 0 (fully sandwichable). Post-maturity redeem + // below needs no oracle: it is 1:1 with a hard 99% dust bound. + if (address(s.oracle) == address(0)) revert PendleOracleRequired(); + uint256 rate = s.oracle.getPtToAssetRate(s.market, s.twapDuration); + if (rate == 0) revert PendleOracleRequired(); + uint256 expected = amount * rate / 1e18; + uint256 minTokenOut = expected * (PENDLE_BPS - _maxSlippageBps(s)) / PENDLE_BPS; IPendleRouter.TokenOutput memory output = IPendleRouter.TokenOutput({ tokenOut: address(underlying), diff --git a/src/libraries/LibDiamond.sol b/src/libraries/LibDiamond.sol index d41ab9a..d897a33 100644 --- a/src/libraries/LibDiamond.sol +++ b/src/libraries/LibDiamond.sol @@ -26,6 +26,7 @@ library LibDiamond { } error NotContractOwner(address caller, address expected); + error NotSelf(address caller); error NoSelectorsProvided(address facetAddress); error CannotAddSelectorsToZeroAddress(bytes4[] selectors); error NoBytecodeAtAddress(address facetAddress, string message); @@ -66,6 +67,20 @@ library LibDiamond { } } + /// @notice Restrict a function to internal diamond dispatch only — callable + /// solely via `address(this).call(...)` from another facet (e.g. the + /// allocator's rebalance or the harvester), never by an external + /// caller. Used to gate strategy fund-movers so they cannot be + /// invoked directly, which would bypass the curator gate, allocation + /// caps, the per-rebalance churn bound, and the rebalance cooldown. + /// @dev Inside a facet running via the diamond's delegatecall, `address(this)` + /// is the diamond; a legitimate self-dispatch arrives with + /// `msg.sender == address(this)`, while any external (EOA/contract) call + /// arrives with its own address preserved through the delegatecall. + function enforceIsSelf() internal view { + if (msg.sender != address(this)) revert NotSelf(msg.sender); + } + event DiamondCut(IDiamond.FacetCut[] _diamondCut, address _init, bytes _calldata); function diamondCut(IDiamond.FacetCut[] memory _diamondCut, address _init, bytes memory _calldata) internal { diff --git a/test/integration/PendleStrategy.fork.t.sol b/test/integration/PendleStrategy.fork.t.sol index a44dffe..8ca4d95 100644 --- a/test/integration/PendleStrategy.fork.t.sol +++ b/test/integration/PendleStrategy.fork.t.sol @@ -16,6 +16,7 @@ import { AllocatorFacet } from "../../src/facets/AllocatorFacet.sol"; import { PendlePtStrategyFacet } from "../../src/facets/strategies/PendlePtStrategyFacet.sol"; import { IPendleRouter } from "../../src/interfaces/external/IPendleRouter.sol"; import { IPPrincipalToken } from "../../src/interfaces/external/IPPrincipalToken.sol"; +import { IPYLpOracle } from "../../src/interfaces/external/IPYLpOracle.sol"; import { LibAllocator } from "../../src/libraries/LibAllocator.sol"; /// @title PendleStrategyForkTest @@ -44,6 +45,12 @@ contract PendleStrategyForkTest is Test { address internal constant ARB_PENDLE_ROUTER = 0x888888888889758F76e7103c6CbF23ABbF58F946; address internal constant ARB_PENDLE_MARKET = address(0); address internal constant ARB_PENDLE_PT = address(0); + // Mandatory oracle for swaps: set to the live Pendle PtYtLpOracle on Arbitrum + // for the chosen market (TODO: confirm address + that the market's oracle is + // ready for ARB_PENDLE_TWAP via getOracleState). Left as address(0) so the + // test skips until wired — pendleDeposit/withdraw now revert without it. + address internal constant ARB_PENDLE_ORACLE = address(0); + uint32 internal constant ARB_PENDLE_TWAP = 900; bytes32 internal constant PENDLE_ID = bytes32("pendle"); @@ -57,8 +64,9 @@ contract PendleStrategyForkTest is Test { vm.skip(true); return; } - // Skip until a real, non-expired Arbitrum Pendle USDC market is wired in. - if (ARB_PENDLE_MARKET == address(0) || ARB_PENDLE_PT == address(0)) { + // Skip until a real, non-expired Arbitrum Pendle USDC market + oracle are + // wired in (the oracle is mandatory for the swap paths). + if (ARB_PENDLE_MARKET == address(0) || ARB_PENDLE_PT == address(0) || ARB_PENDLE_ORACLE == address(0)) { vm.skip(true); return; } @@ -69,6 +77,7 @@ contract PendleStrategyForkTest is Test { vm.startPrank(owner); PendlePtStrategyFacet(address(vault)) .pendleSetConfig(IPendleRouter(ARB_PENDLE_ROUTER), ARB_PENDLE_MARKET, IPPrincipalToken(ARB_PENDLE_PT)); + PendlePtStrategyFacet(address(vault)).pendleSetOracle(IPYLpOracle(ARB_PENDLE_ORACLE), ARB_PENDLE_TWAP); AllocatorFacet(address(vault)).registerStrategy(PENDLE_ID, _pendleStrategyConfig()); _setSingleAllocation(PENDLE_ID, 8000); // 80% to Pendle vm.stopPrank(); @@ -217,7 +226,7 @@ contract PendleStrategyForkTest is Test { } function _pendleSelectors() internal pure returns (bytes4[] memory s) { - s = new bytes4[](10); + s = new bytes4[](11); s[0] = PendlePtStrategyFacet.pendleSetConfig.selector; s[1] = PendlePtStrategyFacet.pendleTotalAssets.selector; s[2] = PendlePtStrategyFacet.pendleDeposit.selector; @@ -228,5 +237,6 @@ contract PendleStrategyForkTest is Test { s[7] = PendlePtStrategyFacet.pendlePT.selector; s[8] = PendlePtStrategyFacet.pendleIsExpired.selector; s[9] = PendlePtStrategyFacet.pendleExpiry.selector; + s[10] = PendlePtStrategyFacet.pendleSetOracle.selector; } } diff --git a/test/unit/MorphoStrategy.t.sol b/test/unit/MorphoStrategy.t.sol index 5da51b4..b2bc57c 100644 --- a/test/unit/MorphoStrategy.t.sol +++ b/test/unit/MorphoStrategy.t.sol @@ -109,13 +109,21 @@ contract MorphoStrategyTest is Test { MorphoStrategyFacet(address(vault)).morphoTotalAssets(); } - function test_Deposit_RevertsWhenUnconfigured() public { - vm.expectRevert(MorphoStrategyFacet.MorphoVaultNotConfigured.selector); + // ----------------------------------------------------------------------- + // Access control — fund-movers are reachable only via diamond self-dispatch + // ----------------------------------------------------------------------- + + function test_Deposit_RevertsForExternalCaller() public { + _configure(); + vm.prank(alice); + vm.expectRevert(abi.encodeWithSelector(LibDiamond.NotSelf.selector, alice)); MorphoStrategyFacet(address(vault)).morphoDeposit(1e6); } - function test_Withdraw_RevertsWhenUnconfigured() public { - vm.expectRevert(MorphoStrategyFacet.MorphoVaultNotConfigured.selector); + function test_Withdraw_RevertsForExternalCaller() public { + _configure(); + vm.prank(alice); + vm.expectRevert(abi.encodeWithSelector(LibDiamond.NotSelf.selector, alice)); MorphoStrategyFacet(address(vault)).morphoWithdraw(1e6); } @@ -125,15 +133,15 @@ contract MorphoStrategyTest is Test { } // ----------------------------------------------------------------------- - // Strategy primitives — called directly on the diamond + // Strategy primitives — exercised through the allocator's self-dispatch + // (the fund-movers are onlySelf, so rebalance is the only legitimate caller) // ----------------------------------------------------------------------- function test_Deposit_MintsSharesAndReportsAssets() public { _configure(); + _register(); uint256 amount = 1000 * 1e6; - usdc.mint(address(vault), amount); // seed the diamond's idle balance - - MorphoStrategyFacet(address(vault)).morphoDeposit(amount); + _deployAll(amount); // 100% target -> rebalance deposits everything assertEq(usdc.balanceOf(address(vault)), 0, "idle underlying fully deployed"); assertGt(morphoVault.balanceOf(address(vault)), 0, "diamond holds Metamorpho shares"); @@ -144,27 +152,37 @@ contract MorphoStrategyTest is Test { function test_Deposit_RevertsOnSlippage() public { _configure(); + _register(); uint256 amount = 1000 * 1e6; morphoVault.setShortchangeBps(100); // mint 1% fewer shares than quoted usdc.mint(address(vault), amount); + _setSingleAllocation(MORPHO_ID, 10_000); + vm.roll(block.number + 1); - // `previewDeposit` is read on the empty vault — same value the facet sees. + // The whole rebalance deposit is `amount` (100% of NAV); previewDeposit + // on the empty vault is the value the facet sees, and MorphoSlippage + // bubbles up through the allocator's self-dispatch. uint256 expected = morphoVault.previewDeposit(amount); uint256 received = (expected * (10_000 - 100)) / 10_000; + vm.prank(owner); vm.expectRevert(abi.encodeWithSelector(MorphoStrategyFacet.MorphoSlippage.selector, expected, received)); - MorphoStrategyFacet(address(vault)).morphoDeposit(amount); + AllocatorFacet(address(vault)).rebalance(); } function test_Withdraw_ReturnsUnderlyingToDiamond() public { _configure(); + _register(); uint256 amount = 1000 * 1e6; - usdc.mint(address(vault), amount); - MorphoStrategyFacet(address(vault)).morphoDeposit(amount); + _deployAll(amount); // all 1000 in Morpho - MorphoStrategyFacet(address(vault)).morphoWithdraw(400 * 1e6); + // Drop the target to 60% -> the next rebalance withdraws 400 back to idle. + _setSingleAllocation(MORPHO_ID, 6000); + vm.roll(block.number + 1); + vm.prank(owner); + AllocatorFacet(address(vault)).rebalance(); - assertEq(usdc.balanceOf(address(vault)), 400 * 1e6, "withdrawn underlying back to idle"); + assertApproxEqAbs(usdc.balanceOf(address(vault)), 400 * 1e6, 1, "withdrawn underlying back to idle"); assertApproxEqAbs( MorphoStrategyFacet(address(vault)).morphoTotalAssets(), 600 * 1e6, 1, "remaining position reported" ); @@ -172,9 +190,8 @@ contract MorphoStrategyTest is Test { function test_TotalAssets_TracksYieldAccrual() public { _configure(); - uint256 amount = 1000 * 1e6; - usdc.mint(address(vault), amount); - MorphoStrategyFacet(address(vault)).morphoDeposit(amount); + _register(); + _deployAll(1000 * 1e6); uint256 beforeYield = MorphoStrategyFacet(address(vault)).morphoTotalAssets(); morphoVault._testAccrueYield(100 * 1e6); // 10% supply yield donated to the vault @@ -186,12 +203,12 @@ contract MorphoStrategyTest is Test { function test_Harvest_IsNoOp() public { _configure(); - uint256 amount = 1000 * 1e6; - usdc.mint(address(vault), amount); - MorphoStrategyFacet(address(vault)).morphoDeposit(amount); + _register(); + _deployAll(1000 * 1e6); uint256 before = MorphoStrategyFacet(address(vault)).morphoTotalAssets(); - MorphoStrategyFacet(address(vault)).morphoHarvest(); // must not revert + // morphoHarvest moves no funds and stays callable (not onlySelf); no-op. + MorphoStrategyFacet(address(vault)).morphoHarvest(); assertEq(MorphoStrategyFacet(address(vault)).morphoTotalAssets(), before, "harvest is a no-op"); } @@ -294,6 +311,17 @@ contract MorphoStrategyTest is Test { AllocatorFacet(address(vault)).registerStrategy(MORPHO_ID, _morphoStrategyConfig()); } + /// @dev Seed `amount` idle into the vault and rebalance it 100% into Morpho + /// via the allocator's self-dispatch — the only path that can move funds + /// now that the mutators are onlySelf. Caller configures + registers first. + function _deployAll(uint256 amount) internal { + usdc.mint(address(vault), amount); + _setSingleAllocation(MORPHO_ID, 10_000); + vm.roll(block.number + 1); + vm.prank(owner); + AllocatorFacet(address(vault)).rebalance(); + } + function _depositToVault(address from, uint256 amount) internal { usdc.mint(from, amount); vm.startPrank(from); diff --git a/test/unit/PendleStrategy.t.sol b/test/unit/PendleStrategy.t.sol index a1f4e89..8374caa 100644 --- a/test/unit/PendleStrategy.t.sol +++ b/test/unit/PendleStrategy.t.sol @@ -129,13 +129,17 @@ contract PendleStrategyTest is Test { assertEq(PendlePtStrategyFacet(address(vault)).pendleTotalAssets(), 0, "zero before config"); } - function test_Deposit_RevertsWhenUnconfigured() public { - vm.expectRevert(PendlePtStrategyFacet.PendleNotConfigured.selector); + function test_Deposit_RevertsForExternalCaller() public { + _configure(); + vm.prank(alice); + vm.expectRevert(abi.encodeWithSelector(LibDiamond.NotSelf.selector, alice)); PendlePtStrategyFacet(address(vault)).pendleDeposit(1e6); } - function test_Withdraw_RevertsWhenUnconfigured() public { - vm.expectRevert(PendlePtStrategyFacet.PendleNotConfigured.selector); + function test_Withdraw_RevertsForExternalCaller() public { + _configure(); + vm.prank(alice); + vm.expectRevert(abi.encodeWithSelector(LibDiamond.NotSelf.selector, alice)); PendlePtStrategyFacet(address(vault)).pendleWithdraw(1e6); } @@ -153,12 +157,15 @@ contract PendleStrategyTest is Test { // Deposit (buy PT) // ----------------------------------------------------------------------- + // The fund-movers are onlySelf, so deposits are driven through the + // allocator's rebalance (the legitimate dispatch path). A mandatory oracle + // is configured first — pendleDeposit refuses to swap unpriced. + function test_Deposit_BuysPtAndReportsAssets() public { - _configure(); + _configureWithOracle(); // par oracle (1e18) + _register(); uint256 amount = 1000 * 1e6; - usdc.mint(address(vault), amount); // seed the diamond's idle balance - - PendlePtStrategyFacet(address(vault)).pendleDeposit(amount); + _deployAll(amount); // 100% target -> rebalance buys PT with all idle assertEq(usdc.balanceOf(address(vault)), 0, "idle underlying fully spent on PT"); assertEq(pt.balanceOf(address(vault)), amount, "diamond holds PT at par"); @@ -168,56 +175,67 @@ contract PendleStrategyTest is Test { } function test_Deposit_AtDiscount_MintsMorePt() public { - _configure(); + _configureWithOracle(); + _register(); router.setDepositRateBps(10_500); // buy PT at a 5% discount -> more PT per USDC uint256 amount = 1000 * 1e6; - usdc.mint(address(vault), amount); - - PendlePtStrategyFacet(address(vault)).pendleDeposit(amount); + _deployAll(amount); uint256 expectedPt = (amount * 10_500) / 10_000; assertEq(pt.balanceOf(address(vault)), expectedPt, "discount captured as extra PT"); - assertEq(PendlePtStrategyFacet(address(vault)).pendleTotalAssets(), expectedPt, "face value reported"); + assertEq(PendlePtStrategyFacet(address(vault)).pendleTotalAssets(), expectedPt, "marked at par == face"); } function test_Deposit_RevertsWhenExpired() public { - _configure(); - vm.warp(expiry); // at/after expiry the market is closed for buys + _configureWithOracle(); + _register(); usdc.mint(address(vault), 1000 * 1e6); + _setSingleAllocation(PENDLE_ID, 10_000); + vm.warp(expiry); // at/after expiry the market is closed for buys + vm.roll(block.number + 1); + vm.prank(owner); vm.expectRevert(PendlePtStrategyFacet.PendleMarketExpired.selector); - PendlePtStrategyFacet(address(vault)).pendleDeposit(1000 * 1e6); + AllocatorFacet(address(vault)).rebalance(); } - function test_Deposit_RevertsWhenZeroPtReceived() public { - _configure(); - router.setDepositRateBps(0); // router mints no PT + function test_Deposit_RevertsWhenNoOracle() public { + // Mandatory oracle: a deposit on an unpriced market is refused outright + // rather than swapped with minOut = 0. + _configure(); // router/market/pt, but NO oracle + _register(); usdc.mint(address(vault), 1000 * 1e6); + _setSingleAllocation(PENDLE_ID, 10_000); + vm.roll(block.number + 1); - vm.expectRevert(abi.encodeWithSelector(PendlePtStrategyFacet.PendleDepositFailed.selector, 0)); - PendlePtStrategyFacet(address(vault)).pendleDeposit(1000 * 1e6); + vm.prank(owner); + vm.expectRevert(PendlePtStrategyFacet.PendleOracleRequired.selector); + AllocatorFacet(address(vault)).rebalance(); } 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); + _register(); + usdc.mint(address(vault), 1000 * 1e6); + _setSingleAllocation(PENDLE_ID, 10_000); + vm.roll(block.number + 1); // 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.prank(owner); vm.expectRevert("MockPendle: minPtOut"); - PendlePtStrategyFacet(address(vault)).pendleDeposit(amount); + AllocatorFacet(address(vault)).rebalance(); } function test_Deposit_SucceedsWhenFillWithinOracleSlippageBound() public { _configure(); _setOracle(0.95e18); + _register(); router.setDepositRateBps(10_500); // 1050 PT, within 1% of the ~1052.6 oracle mark uint256 amount = 1000 * 1e6; - usdc.mint(address(vault), amount); + _deployAll(amount); - PendlePtStrategyFacet(address(vault)).pendleDeposit(amount); assertEq(pt.balanceOf(address(vault)), (amount * 10_500) / 10_000, "fill within tolerance accepted"); } @@ -249,12 +267,15 @@ contract PendleStrategyTest is Test { // ----------------------------------------------------------------------- function test_Withdraw_PreMaturity_SellsPtForUnderlying() public { - _configure(); - uint256 amount = 1000 * 1e6; - usdc.mint(address(vault), amount); - PendlePtStrategyFacet(address(vault)).pendleDeposit(amount); + _configureWithOracle(); + _register(); + _deployAll(1000 * 1e6); // 1000 PT, par oracle - PendlePtStrategyFacet(address(vault)).pendleWithdraw(400 * 1e6); + // Drop target to 60% -> rebalance sells 400 PT on the AMM. + _setSingleAllocation(PENDLE_ID, 6000); + vm.roll(block.number + 1); + vm.prank(owner); + AllocatorFacet(address(vault)).rebalance(); assertEq(usdc.balanceOf(address(vault)), 400 * 1e6, "underlying back to idle"); assertEq(pt.balanceOf(address(vault)), 600 * 1e6, "PT reduced by the sold amount"); @@ -262,63 +283,60 @@ contract PendleStrategyTest is Test { } function test_Withdraw_PostMaturity_RedeemsAtFaceValue() public { - _configure(); - uint256 amount = 1000 * 1e6; - usdc.mint(address(vault), amount); - PendlePtStrategyFacet(address(vault)).pendleDeposit(amount); + _configureWithOracle(); + _register(); + _deployAll(1000 * 1e6); - vm.warp(expiry + 1); // PT now redeems 1:1 via redeemPyToToken + vm.warp(expiry + 1); // PT now redeems 1:1 via redeemPyToToken (no oracle needed) assertTrue(PendlePtStrategyFacet(address(vault)).pendleIsExpired(), "PT expired"); - PendlePtStrategyFacet(address(vault)).pendleWithdraw(amount); + _setSingleAllocation(PENDLE_ID, 0); + vm.roll(block.number + 1); + vm.prank(owner); + AllocatorFacet(address(vault)).rebalance(); - assertEq(usdc.balanceOf(address(vault)), amount, "full face value redeemed"); + assertEq(usdc.balanceOf(address(vault)), 1000 * 1e6, "full face value redeemed"); assertEq(pt.balanceOf(address(vault)), 0, "PT fully burned"); } - function test_Withdraw_RevertsOnInsufficientPt() public { - _configure(); - uint256 amount = 1000 * 1e6; - usdc.mint(address(vault), amount); - PendlePtStrategyFacet(address(vault)).pendleDeposit(amount); - - vm.expectRevert(abi.encodeWithSelector(PendlePtStrategyFacet.PendleInsufficientPt.selector, amount + 1, amount)); - PendlePtStrategyFacet(address(vault)).pendleWithdraw(amount + 1); - } - - function test_Withdraw_RevertsWhenZeroReceived() public { - _configure(); - uint256 amount = 1000 * 1e6; - usdc.mint(address(vault), amount); - PendlePtStrategyFacet(address(vault)).pendleDeposit(amount); - - router.setWithdrawHaircutBps(10_000); // 100% haircut -> zero underlying out + function test_Withdraw_PreMaturity_RevertsWhenNoOracle() public { + // Mandatory oracle on the pre-maturity sell. Build a PT position directly + // (no oracle) so we can reach the sell path, then rebalance to 0%. + _configure(); // no oracle + _register(); + pt.mint(address(vault), 1000 * 1e6); + _setSingleAllocation(PENDLE_ID, 0); + vm.roll(block.number + 1); - vm.expectRevert(abi.encodeWithSelector(PendlePtStrategyFacet.PendleWithdrawFailed.selector, 400 * 1e6, 0)); - PendlePtStrategyFacet(address(vault)).pendleWithdraw(400 * 1e6); + vm.prank(owner); + vm.expectRevert(PendlePtStrategyFacet.PendleOracleRequired.selector); + AllocatorFacet(address(vault)).rebalance(); } 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 + _configureWithOracle(); // par mark: minTokenOut = 99% of PT sold + _register(); + _deployAll(1000 * 1e6); router.setWithdrawHaircutBps(500); // 5% AMM haircut, beyond the 1% tolerance + _setSingleAllocation(PENDLE_ID, 6000); // sell 400 + vm.roll(block.number + 1); + vm.prank(owner); vm.expectRevert("MockPendle: minTokenOut"); - PendlePtStrategyFacet(address(vault)).pendleWithdraw(400 * 1e6); + AllocatorFacet(address(vault)).rebalance(); } function test_Withdraw_SucceedsWhenHaircutWithinSlippageBound() public { - _configure(); - uint256 amount = 1000 * 1e6; - usdc.mint(address(vault), amount); - PendlePtStrategyFacet(address(vault)).pendleDeposit(amount); - _setOracle(1e18); + _configureWithOracle(); + _register(); + _deployAll(1000 * 1e6); router.setWithdrawHaircutBps(50); // 0.5% haircut, within the 1% tolerance - PendlePtStrategyFacet(address(vault)).pendleWithdraw(400 * 1e6); + _setSingleAllocation(PENDLE_ID, 6000); // sell 400 + vm.roll(block.number + 1); + vm.prank(owner); + AllocatorFacet(address(vault)).rebalance(); + assertEq(usdc.balanceOf(address(vault)), (400 * 1e6 * 9950) / 10_000, "fill within tolerance settled"); } @@ -360,37 +378,33 @@ contract PendleStrategyTest is Test { // ----------------------------------------------------------------------- function test_TotalAssets_MarksToMarketWhenOracleSet() public { - _configure(); + _configureWithOracle(); // buy at par + _register(); 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. + _deployAll(amount); + + oracle.setRate(0.95e18); // mark the live position down to a 5% discount + assertEq(pt.balanceOf(address(vault)), amount, "holds PT at face"); assertEq(PendlePtStrategyFacet(address(vault)).pendleTotalAssets(), 950 * 1e6, "marked to market via oracle"); } function test_TotalAssets_FaceValueWhenNoOracle() public { - _configure(); + // The no-oracle valuation branch: hold PT (minted directly, since a + // deposit now requires an oracle) and confirm it reports face value. + _configure(); // no oracle uint256 amount = 1000 * 1e6; - usdc.mint(address(vault), amount); - PendlePtStrategyFacet(address(vault)).pendleDeposit(amount); + pt.mint(address(vault), amount); - // No oracle configured -> face value fallback. assertEq(PendlePtStrategyFacet(address(vault)).pendleTotalAssets(), amount, "face value without oracle"); } function test_TotalAssets_PostMaturityIgnoresOracle() public { - _configure(); + _configureWithOracle(); + _register(); uint256 amount = 1000 * 1e6; - usdc.mint(address(vault), amount); - PendlePtStrategyFacet(address(vault)).pendleDeposit(amount); - _setOracle(0.95e18); // set after the buy; see MarksToMarketWhenOracleSet + _deployAll(amount); + oracle.setRate(0.95e18); // discounted mark vm.warp(expiry + 1); // PT now redeems 1:1 — oracle discount must be bypassed @@ -402,13 +416,12 @@ contract PendleStrategyTest is Test { // ----------------------------------------------------------------------- function test_Harvest_IsNoOp() public { - _configure(); - uint256 amount = 1000 * 1e6; - usdc.mint(address(vault), amount); - PendlePtStrategyFacet(address(vault)).pendleDeposit(amount); + _configureWithOracle(); + _register(); + _deployAll(1000 * 1e6); uint256 before = PendlePtStrategyFacet(address(vault)).pendleTotalAssets(); - PendlePtStrategyFacet(address(vault)).pendleHarvest(); // must not revert + PendlePtStrategyFacet(address(vault)).pendleHarvest(); // pure no-op, stays callable assertEq(PendlePtStrategyFacet(address(vault)).pendleTotalAssets(), before, "harvest is a no-op"); } @@ -433,7 +446,7 @@ contract PendleStrategyTest is Test { // ----------------------------------------------------------------------- function test_Rebalance_RoutesAssetsIntoPendle() public { - _configure(); + _configureWithOracle(); _register(); _depositToVault(alice, 1000 * 1e6); _setSingleAllocation(PENDLE_ID, 8000); // 80% to Pendle @@ -448,7 +461,7 @@ contract PendleStrategyTest is Test { } function test_Rebalance_PullsBackWhenAllocationDrops() public { - _configure(); + _configureWithOracle(); _register(); _depositToVault(alice, 1000 * 1e6); _setSingleAllocation(PENDLE_ID, 8000); @@ -488,6 +501,26 @@ contract PendleStrategyTest is Test { AllocatorFacet(address(vault)).registerStrategy(PENDLE_ID, _pendleStrategyConfig()); } + /// @dev Configure router/market/pt plus a par (1e18) oracle. The oracle is + /// mandatory for deposits/pre-maturity sells now, so most position- + /// building tests need it. + function _configureWithOracle() internal { + _configure(); + _setOracle(1e18); + } + + /// @dev Seed `amount` idle into the vault and rebalance it 100% into Pendle + /// via the allocator's self-dispatch — the only path that can move funds + /// now that the mutators are onlySelf. Caller configures (with oracle) + + /// registers first. + function _deployAll(uint256 amount) internal { + usdc.mint(address(vault), amount); + _setSingleAllocation(PENDLE_ID, 10_000); + vm.roll(block.number + 1); + vm.prank(owner); + AllocatorFacet(address(vault)).rebalance(); + } + function _depositToVault(address from, uint256 amount) internal { usdc.mint(from, amount); vm.startPrank(from);