Open
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
sparrowDom
requested review from
clement-ux,
naddison36 and
shahthepro
as code owners
clement-ux
previously approved these changes
Feb 6, 2026
Collaborator
clement-ux
left a comment
clement-ux
left a comment
There was a problem hiding this comment.
Everything looks good to me.
Note on withdrawAll():
When calling withdrawAll(), the strategy only withdraws what is currently liquid in the Morpho V2 vault (idle USDC + V1 adapter available liquidity). Under high utilization, some funds may remain in the strategy after the call. This is expected behavior, not a bug, but something to keep in mind operationally -- multiple calls or waiting for liquidity to free up may be needed to fully exit the position.
Member
Author
|
good point @clement-ux have expressed this more verbosely in the comment: 1f943d8 |
clement-ux
previously approved these changes
Feb 6, 2026
naddison36
requested changes
Feb 9, 2026
Collaborator
naddison36
left a comment
naddison36
left a comment
There was a problem hiding this comment.
I think we should also change withdrawAll on Generalized4626Strategy so it calls maxRedeem instead of balanceOf. This way it redeems what it can.
This doesn't work for Morpho v2 vaults as they just return zero for all their max functions. For for vaults that are ERC-4626 compliant, the change to Generalized4626Strategy will work.
sparrowDom
force-pushed
the
sparrowDom/morphoV2WithdrawAll
branch
from
ef0a057 to
9ee8188
Compare
Member
Author
|
@naddison36 good thinking thanks. Done here: 9ee8188 |
… possible liquidity in the withdrawAll
* some scaffolding * add basic necessities for unit tests * checkpoint * Fix compiling issues * Add fork test scaffolding * Fix stuffs * Prettify and change salt * Add auto-verification * Fix checkBalance * Make CCTPHookWrapper more resilient * refactor message version and type checks * add some comments * add comment * fix compile errors * Change addresses * Cross chain changes (#2718) * fix deploy files * minor rename * add calls to Morpho Vault into a try catch * refactor hook wrapper * don't revert if withdraw from underlying fails * use checkBalance for deposit/withdrawal acknowledgment * update message in remote strategy * remove unneeded functions * Fix compilation issues * Fix deployment files a bit * Fix Message relayer * Clean up master strategy * Fix deployment file name * move around stuff * Fix CCTP Integrator * clean up fork * Fix race condition (#2720) * Fix race condition * Transfer everything on wtihdrawal * Move destination domain one step above * Cleanup code * decode payloads in fork tests * Add library for message handling * More changes * Add comments and prettify * WIP Unit test setup (#2722) * add cross chain unit test basic files * add basic unit test setup * add header encoding * more tests * Add more fork tests * Add token transfer tests * WIP Unit tests for OUSD Simplified strategy (#2724) * more unit test integration * more tying up ends * fix bug * cleanup * add full round-trip test * cleanup * Fix approve all and prettify --------- Co-authored-by: Shahul Hameed <[email protected]> * Fix master fork tests * linter * add direct withdrawal paths and additional checks * Fix Remote strategy tests * Update comments and clean up code * Fix comment * Fix failing unit test * fix: withdraw only if balance is lower than requested amount * Document crosschain strategy * Update deployment file numbers * adjust the charts * change the function visibility to pure * fix: create2 proxy without using deployer address * fix: impersonate a single deployer on fork * deploy script bug fix * Store create2 proxy addresses * fix: await * more logging * fix opts * Fix env for deploy action * Change writeFileSync to writeFile * add log * Add more logs * fix callback * Add empty file * Cleanup logs * withdraw funds according to the spec * Address Code Review comments (#2732) * Address Code Review comments * Fix initialize method * Fix initialize method * fix remote strat initialize method * Revert 999 * fix comments * add a test that uncovers a withdrawal error (#2733) * remove transferType * correct spelling * rename baseToken to usdcToken * improve error message * simplify code * fix: min withdraw amount is 1e6 * add validations for config * fix: require a min deposit amount of 1e6 * fix: withdrawAll caps withdraw amount * Move around constants * Move decode message header function * fix fork tests * prettify * adjust some comments * have consistent event names * fix: remove redundant check * simplify stuff * adjust comment * fix: variable name * Add TokensBridged and MessageTransmitted events * Add finality threshold checks * add comment regarding fast transfers * Add analytics info * Change error message * Set 1 USDC as min allowed value for deposits * Change comment * Change comment * Update max transfer amount comment * Set nonce(0) as processed during initialization * Use Strategizable for strategist functionality (#2740) * use Strategizable * Add comment --------- Co-authored-by: Shahul Hameed <[email protected]> * set vault address to zero (#2742) * remove unnecessary comments * Add comment * Add nonReentrant for deposit and withdraw methods * Add more checks in constructor * Fix withdrawAll * Update deploy numbers * simplify _withdraw (#2741) * address verification (#2749) * add address verification * remove operator check * Fix merge issue * small adjustement * fix unit test * Bumped the deploy script numbers * remove unused parameter (#2754) * Add unit tests (#2751) * fix unit test * add more unit tests * add more unit tests * prettier * add some more unit tests * add thorough unit test support * Default to Timelock governance * lint * Fix VaultAddress in deployment scripts * Add events for nonce updates (#2755) * Sparrow dom/cctp defender action (#2770) * Add unit tests (#2751) * fix unit test * add more unit tests * add more unit tests * prettier * add some more unit tests * add thorough unit test support * add comment * create a defender task and cleanup * small change * add the ability for the defender relay action to store already processed transactions * update gitignore * prettier * put into a better place * ... * add dry run option, also fix issues with cross chain providers * read cctp domain ids from config * make api a constant * remove finality checks * add custom per chain block lookback * readme change * move all configuration out of cross-chain source file, to more easily support multiple networks and relaying directions * clear the testing addresses * prettier * add test address * add the option not to initialize the implementation contract * Prettify * add comment (#2787) * ignore messages that are too old (#2786) * [OUSD-09] Check burnToken in relay method (#2782) * [OUSD-09] Check burnToken in relay method * Fix: Check burn token is usdc on remote chain * Add tests * Make min transfer amount a constant (#2780) * [OUSD-15] Emit event when withdrawAll is skipped (#2781) --------- Co-authored-by: Domen Grabec <[email protected]> Co-authored-by: Nicholas Addison <[email protected]>
* Refactor OETHVaultCore and VaultCore to simplify asset handling and remove WETH dependencies * Add constructors to MockVault and MockVaultCoreInstantRebase for initialization * Refactor OETHVaultCore and VaultCore to implement async withdrawal functionality * Refactor OETHVaultCore and VaultCore to remove unused functions and simplify total value calculations * Remove unused addWithdrawalQueueLiquidity function from OETHVaultCore and clean up comments in VaultCore * Refactor OETHVaultCore and VaultCore to remove unused imports and functions, simplifying the codebase. * Remove unused pricing functions and related imports in VaultCore to simplify the codebase. * Update constructor parameter in OETHVaultCore to use _weth for clarity * Add OUSDVaultCore contract and implement redeem functionality with access control * Refactor OETHVaultAdmin and VaultAdmin contracts to simplify code and improve clarity by removing deprecated functions and updating asset handling methods. * Refactor vault admin contracts to remove constructors and simplify initialization by using address(0) for backing asset. * Refactor vault contracts to enforce backing asset restrictions and remove deprecated asset handling * Refactor VaultAdmin and VaultStorage contracts to simplify default strategy handling for backing asset * Refactor AbstractHarvester and related contracts to integrate oracle address, simplifying price retrieval and enhancing clarity * Revert changes on Harvester * remove old code * Refactor vault admin contracts to include backing asset address in constructors for improved clarity and functionality * Refactor OETH Vault tests to simplify strategy handling and improve error messages * adjust decimal logic in VaultStorage * wip fix test * Refactor scaling logic in VaultCore to use scaleBy function for improved clarity and consistency * Refactor Compound strategy tests to simplify logic and improve clarity by removing unused variables and consolidating test cases * Refactor Vault tests to simplify logic by removing unused variables and consolidating test cases * Refactor Vault tests to simplify code by removing unused variables and consolidating test cases * Refactor Vault redeem tests to simplify logic by removing unused variables and consolidating test cases * fix buyback fixture * Refactor Reborner contract and tests to use USDC instead of USDS, update mint and redeem amounts for consistency * Refactor compoundingStakingSSVStrategyFixture to simplify default strategy setting * Refactor Convex strategy tests to use USDC instead of USDS and simplify asset handling * Refactor MockVaultCoreInstantRebase constructor to accept backing asset address, update test fixtures to use USDC instead of USDS, and simplify Dripper and VaultValueChecker tests by removing unused variables and consolidating logic. * fix remaining unit test * fix base setup * fix sonic setup * Add slither-disable comments for reentrancy checks in mint functions * Remove collateral swaps test file to streamline testing suite * Add OUSDVaultAdmin contract and deployment script for OUSD Vault upgrade * fix vault fork test * prettier * fix more fork tests * lint * fix more tests * fix sonic fork test * lint + prettier * skip plume test * fix sonic fork test * fix sonic fork test * remove .only * remove old fix * fix decimal conversion * refactor: change contracts to abstract for VaultAdmin, VaultCore, VaultInitializer, and VaultStorage * fix test * WIP add async withdraw test for OUSD * Add lot of test for async withdraw * adjust comment * refactor: remove setDripper function from IVault interface * fix: update redeem function to use correct amount parameter * feat: add deprecated weth asset index to OETHVaultCore contract * increase ousd vault upgrade number * fix: add slither disable comment for constable-states in OETHVaultCore contract * fix: correct drip duration initialization in VaultInitializer contract * feat: deprecate calculateRedeemOutputs and introduce calculateRedeemOutput in VaultCore contract * refactor: rename backingAsset to asset across contracts for consistency * refactor: change visibility of deprecated asset-related mappings to private * fix: improve error message for unsupported asset decimals in VaultStorage constructor * fix: update error message for unsupported asset in minting process * refactor: update comments to clarify AMO strategy references in mint and redeem functions * refactor: standardize terminology by replacing 'backing asset' with 'asset' in contracts and tests * refactor: simplify redeem output calculation by removing deprecated function * refactor: streamline redeem output calculation by consolidating return values * refactor: improve clarity in comments regarding AMO strategy in mint and redeem functions * refactor: mark isSupportedAsset function as deprecated * [Vault] Simplify `mint()` (#2735) * refactor: remove deprecated asset structure and related mappings from VaultStorage * refactor: remove deprecated mint function parameters and related tests * refactor: remove unused parameters from _mint and mint functions * refactor: remove unnecessary blank lines in test files * refactor: change internal mappings to private for better encapsulation * Clement/simplify ousd get rid of old stuff (#2736) * refactor: remove deprecated asset structure and related mappings from VaultStorage * refactor: remove deprecated mint function parameters and related tests * refactor: remove unused parameters from _mint and mint functions * refactor: remove unnecessary blank lines in test files * refactor: change internal mappings to private for better encapsulation * refactor: simplify asset mappings by changing from mapping to single uint256 * [Vault] Remove `redeem()` and simplify inheritance. (#2737) * Remove redeem functionality and related events from vault contracts * Refactor vault contracts to inherit from VaultCore, removing deprecated OETHVaultCore references and relocating legacy code for consistency. * Fix tests after redeem has been removed (#2738) * Remove calculateRedeemOutputs view function from Vault * Removed redeem, fund and yield Hardhat task * FIxed unit tests * Fixed fork tests * Fix Base fork tests * Fixed Base Curve AMO fork tests * Removed redeemFeeBps from Sonic fork tests * Fix Base and Sonic unit tests * Prettier * Remove hardhat fund from node script * Fix OUSD AMO fork tests * Fix AMO fork tests * Remove unused gap variable from OUSDVaultCore contract --------- Co-authored-by: Nick Addison <[email protected]> * Removed Beacon consolidation fork tests * Add OETH vault upgrade deployment script with governance proposal * Remove deprecated redeem tests from OETH vault and bridge helper test files * lint * Add vault upgrade deployment script and skip deprecated tests in bridge helper * prettier * Add deployment script for OSonic vault upgrade with governance actions * Refactor OETH and OSonic VaultAdmin contracts to inherit from VaultAdmin * New gov prop of deploy script 163 * Simplify withdrawal amount calculations in VaultCore contract * fix bug * [Vault] Merge VaultAdmin and VaultCore (#2743) * Refactor vault contracts to remove deprecated implementations and simplify admin structure * Refactor vault contracts to unify admin structure and simplify deployment scripts * move zapper to dedicated folder * Refactor vault upgrade scripts to unify implementation names and simplify deployment actions * Updated hot deploy with new Vault contracts * Update Vault contract diagrams * Add OSVault contract and update deployment script references --------- Co-authored-by: Nicholas Addison <[email protected]> * Removed commented out IOracle lines from Harvester * Changed the OUSD default strategy to the Morpho OUSD v2 strategy * Rename OSonicVault to OSVault in deployment script * Refactor rebase and withdrawal queue liquidity calls in VaultAdmin contract * Add slither disable comments for reentrancy checks in VaultAdmin contract * Removed dependency on OracleRouter from the OUSD and SuperOETH Vaults * Bumped the deploy script numbers * prevent removal of strategies that still have balance after the withdrawal (#2767) * [OUSD-16] Miscellaneous General Comments (#2788) * replace interface to fetch decimals * add comment * add link to readme * removed TODO * rename oUSD to oToken * add function for reverse compatibility and mark deprecated * rename * refactor: update setRebaseRateMax function to clarify APR parameters and improve documentation (#2785) * Remove constant MAX_PRICE_STALENESS from BridgedWOETHStrategy contract (#2784) * remove unused _totalValueInVault function to simplify VaultCore contract (#2779) * [OUSD-13] `removeStrategy()` Does Not Reset Mint Whitelist Flag. (#2778) * Reset mintWhiteList flag when removing strategy. * Add test to reset mint whitelist flag when removing a strategy * [OUSD-11] Missing Zero Amount Validation In `requestWithdrawal()`. (#2777) * Add validation for withdrawal amount in requestWithdrawal function * Fail to allow withdrawal of zero amount in requestWithdrawal function * Add asset support check in approveStrategy function (#2776) * Fix Vault tests (#2790) * Fix base unit tests * Fix Ehereum unit tests * Remove redundant fork tests * fix approval * Fix CI --------- Co-authored-by: Nick Addison <[email protected]> Co-authored-by: Domen Grabec <[email protected]> Co-authored-by: Shah <[email protected]>
* Deploy 166 * Add deployment timestamp * Implement CurvePoolBooster migration script for new version deployment * Update proposalId in CurvePoolBooster migration script
* [CurvePB] Enhance pool reward calculation by adding gauge and LP token details * [CurvePB] Improve CurvePoolBoosterBribesModule - Use single `manageCampaign` call instead of 3 separate calls - Make bridge fee configurable instead of hardcoded - Send ETH from Safe via execTransactionFromModule instead of requiring pool boosters to hold ETH - Add NatSpec documentation * [CurvePB] Make manageBribes fully configurable per pool Add totalRewardAmounts and extraDuration parameters to the parameterized manageBribes overload, allowing per-pool control over reward amounts and campaign duration. The no-arg version retains the previous defaults (use all rewards, +1 period, no maxRewardPerVote update). Remove redundant onlyOperator modifier from internal _manageBribes. Update NatSpec. * [CurvePB] Rename POOLS to pools Follow Solidity naming convention: all-caps is reserved for constants and immutables. Rename mutable storage array to lowercase. Prefix function parameters with underscore to avoid shadowing. * [CurvePB] Revert when removing a pool that does not exist _removePoolBoosterAddress previously did a silent no-op when the address was not found. Now it reverts with "Pool not found" to prevent masking bugs in the caller. * [CurvePB] Add duplicate check and use calldata for pool lists Refactor _addPoolBoosterAddress to accept a single address and revert with "Pool already added" if it already exists, preventing double processing and double bridge fees. Switch addPoolBoosterAddress parameter to calldata for gas efficiency, consistent with removePoolBoosterAddress. * [CurvePB] Make additionalGasLimit configurable Replace the hardcoded 1000000 gas limit with a storage variable that can be set at construction and updated via setAdditionalGasLimit. Add corresponding event. * [CurvePB] Use calldata for manageBribes parameters Switch the parameterized manageBribes overload from memory to calldata for gas efficiency, avoiding unnecessary memory copies. * [CurvePB] Add additionalGasLimit to deployment script Pass the new constructor parameter (1000000) to match the updated CurvePoolBoosterBribesModule constructor signature. * [CurvePB] Fix lint: shorten inline comment exceeding max line length * [CurvePB] Update poolBooster task for new manageBribes signature Update the ABI and manageBribes call to match the new contract interface. Use the no-arg manageBribes() when skipRewardPerVote is true, and the 3-param overload with default totalRewardAmounts (max) and extraDuration (1) otherwise. * [CurvePB] Add zero address check in _addPoolBoosterAddress * [CurvePB] Add max value check for bridge fee * [CurvePB] Add max value check for additional gas limit * [CurvePB] Rename length to pbCount in _manageBribes * [CurvePB] Rename pools to poolBoosters to avoid confusion with AMM pools * Reorder deployment number
* Deploy 173 * prettier + lint
* [Ethereum] [Base] Deploy Crosschain Strategy * Regenerate governance files
* Deploy 174 * Add proposal id
* [CurvePB] Update CurvePoolBoosterBribesModule address Update the CurvePoolBoosterBribesModule contract address to the new deployment. * lint * [CurvePB] Refactor bribes module contract initialization and use constants from ethers
* Run log 17 Feb - deposit SSV to clusters * Added native_staking_3_strat contract
* Add scripts to change Crosschain strategy operator * Run base deployment
* Remove stuff * Remove stuff * Remove more stuff * Fix unit tests * Fix tests * remove more stuff * revert mock files * few more tweaks * more delete * remove older deployment files * Restore mock contract * Fix fixtures * Repo cleanup additions (#2804) * Removed old generated contract docs * Added Curve AMO Strategy docs * Fixed amoStrat Hardhat task for new OETH Curve AMO * Removed ConvexEthMetaStrategy and Generalized4626USDTStrategy * Added new Cross-chain Strategy to OUSD Vault fork test * Fix OUSD vault fork test * Fix Should claim Morpho rewards fork test Removed old Morpho Strategies from fixture * Correct operator ids for second SSV cluster in fork tests * prettier * Fix Curve AMO fork tests * Add proposal id to 177_change_crosschain_strategy_operator * Fix CrossChainMasterStrategy fork tests * Fixed claim CRV rewards fork tests * Fix Merkl Pool Booster fork test * Fix claiming rewards on Yearn's Morpho OUSD v2 Strategy * morphoOUSDv2Fixture to handle outstanding withdrawal requests * fix ForkTest: CrossChainRemoteStrategy fork tests * prettier --------- Co-authored-by: Nick Addison <[email protected]>
* Run log Allocate 100k USDC to the Cross-chain strategy * Updated USDC balance message * Updated withdraw amount
* Updated Vault hierarchy * Updated OUSD contract diagram * Added symbol to snapVault Hardhat task for OUSD Added symbol to queueLiquidity hardhat task for OUSD * regenerated using latest sol2uml just to be safe
* Fix OUSD/USDC prices in amoStrat Hardhat task * Run log - 23 Feb withdraw from Morpho v2 OUSD Strategy * Update script
* Add Auto-Withdrawal Safe Module * Get rid of rate limit logic * Add tests * Address CR comments
* Replace deprecated mint(address,uint256,uint256) with mint(uint256) in IVault Update the IVault interface to use the new single-param mint signature and fix all contract callers: MockNonRebasing, MockRebornMinter, AbstractOTokenZapper, OSonicZapper, and PlumeBridgeHelperModule. * Rewrite EthereumBridgeHelperModule with async withdrawal and remove Plume Remove all Plume/LayerZero bridging code and replace synchronous vault redeem with async withdrawal pattern (requestWithdrawal + claimWithdrawal). Add unwrapAndRequestWithdrawal, claimAndBridgeToBase, and claimWithdrawal. * Update BaseBridgeHelperModule to use async withdrawal Replace synchronous vault redeem with async withdrawal pattern. Remove depositWOETHAndBridgeWETH (no longer single-TX). Add depositWOETHAndRequestWithdrawal, claimWithdrawal, and claimAndBridgeWETH. * Update tests for new mint(uint256) signature and async withdrawal Replace all vault.mint(asset, amount, minAmount) calls with vault.mint(amount) across test files. Update bridge helper tests to match new async withdrawal API. Remove unused variable declarations. * Add deployment scripts for EthereumBridgeHelperModule and BaseBridgeHelperModule Deploy scripts for the updated bridge helper modules on mainnet and Base. Both enable the module on the Safe when running on a fork. * Fix Plume fork tests to use legacy mint/redeem signatures The deployed Plume vault won't be upgraded, so hardcode the old mint(address,uint256,uint256) selector in PlumeBridgeHelperModule and use a legacy ABI contract in fork tests for mint and redeem calls. * Fix review issues: approve check, skipped tests, stale mock signatures - Add missing require(success) on wOETH approve in BaseBridgeHelperModule._depositWOETH - Un-skip Base bridge helper tests and rewrite for async withdrawal flow - Remove unused _asset param from MockNonRebasing.mintOusd - Point MockRebornMinter.redeem() at requestWithdrawal instead of reverting * Fix prettier formatting in bridge-helper base fork test * Add requestWithdrawal boolean to BaseBridgeHelperModule.depositWOETH
pandadefi
approved these changes
Mar 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR: Morpho V2 Liquidity Estimation + Safe WithdrawAll
Summary
This PR updates the Morpho V2 strategy to better estimate immediate liquidity and safely cap
withdrawAll, while acknowledging that only Morpho V1 adapters are supported for extended liquidity checks.Why
Morpho V2 vaults can configure a liquidity adapter. When that adapter points to a Morpho V1 vault, the strategy can query the V1 vault’s
maxWithdrawto include additional immediately available liquidity. If the adapter is something else, the strategy intentionally does not assume additional liquidity. This avoids over‑estimating withdrawable amounts and prevents failed withdrawals.Key Changes
withdrawAllnow accounts for adapter‑backed liquidity when the adapter is a Morpho V1 vault.withdrawAllrespects the actual available liquidity in the V2 vault and its adapter.Notes / Limitations