Skip to content

[VaultBundles] IKR#15

Open
QGarchery wants to merge 51 commits into
mainfrom
feat-ikr
Open

[VaultBundles] IKR#15
QGarchery wants to merge 51 commits into
mainfrom
feat-ikr

Conversation

@QGarchery

@QGarchery QGarchery commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

@QGarchery QGarchery self-assigned this Jun 16, 2026
@QGarchery QGarchery changed the title [VaultBundles] IKR (WIP) [VaultBundles] IKR Jun 16, 2026
@QGarchery QGarchery marked this pull request as ready for review June 16, 2026 16:47
@QGarchery QGarchery requested a review from bhargavbh June 16, 2026 17:03
Comment thread src/vault-ikr/VaultIkrBundlesV1.sol Outdated
Comment thread src/vault-ikr/VaultIkrBundlesV1.sol Outdated
Comment thread src/vault/VaultBundles.sol Outdated
Comment thread src/vault/VaultBundles.sol Outdated
Comment thread src/vault-force-withdraw/VaultForceWithdrawBundlesV1.sol Outdated
Comment thread src/vault/VaultBundles.sol Outdated
Comment thread src/vault-ikr/VaultIkrBundlesV1.sol Outdated
Comment thread src/vault-ikr/VaultIkrBundlesV1.sol Outdated
Comment thread src/vault-ikr/VaultIkrBundlesV1.sol Outdated
/// @dev The sender must have given enough allowance over vault shares to this bundler. Using max allowance makes sure that this condition is met.
/// @dev The deallocatedAssets amount is floor(forceWithdrawAssets * WAD / (WAD + penalty)).
/// @dev Requires Morpho Blue to have more than the deallocated assets in liquidity.
/// @dev Requires the sender to have enough shares to withdraw ceil(assets * penalty / WAD) and then assets, for each market in the list, where the sum of the assets is equal to deallocatedAssets.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are multiple roundings (up) right? so it could be worse than this I think

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's accurate, it requires to be able to call withdraw twice, which takes into account all the rounding errors that happen inside a given vault interaction

Comment thread src/vault-force-withdraw/VaultForceWithdrawBundlesV1.sol
Comment on lines +144 to +145
// Markets not enabled in the vault are skipped.
if (!IMetaMorpho(vault).config(marketParams.id()).enabled) continue;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe revert instead?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want the curator to be able to remove it from the list (by calling updateWithdrawQueue, not timelocked) and make the transaction revert. Maybe ok, since the number of markets is limited, but it doesn't cost much to just skip the market here

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will be very hard to reproduce (only doable once per market). it sounds ok to me

Comment thread src/vault-ikr/VaultIkrBundlesV1.sol Outdated
Comment thread src/vault-ikr/VaultIkrBundlesV1.sol Outdated
Comment thread src/vault-ikr/interfaces/IVaultIkrBundlesV1.sol Outdated
Comment thread src/vault-force-withdraw/interfaces/IVaultForceWithdrawBundlesV1.sol Outdated
Comment thread src/vault-force-withdraw/VaultForceWithdrawBundlesV1.sol
uint256 assets = UtilsLib.min(availableToWithdraw, assetsToDeallocate);

IVaultV2(vault).forceDeallocate(adapter, abi.encode(marketParams), assets, msg.sender);
IVaultV2(vault).withdraw(assets, msg.sender, msg.sender);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not a single withdraw at the end? the deallocated liquidity couldn't be stolen by reentrancy right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No strong reason, it was more to align with the other vault v2 function, and the motivation in this message don't hold that well in this case: it's less gas gain, and the rounding that it prevents is not the same as in vault v1.
Maybe still interesting to factorize though, what do you think ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants