Skip to content
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.

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

MultiStrategyVault #145

Merged
merged 2 commits into from
Jan 29, 2024
Merged

MultiStrategyVault #145

merged 2 commits into from
Jan 29, 2024

Conversation

RedVeil
Copy link
Contributor

@RedVeil RedVeil commented Jan 26, 2024

No description provided.

Comment on lines 328 to 361
function _withdrawStrategyFunds(uint256 amount) internal {
// Get the Vault's floating balance.
uint256 float = IERC20(asset()).balanceOf(address(this));

// If the amount is greater than the float, withdraw from strategies.
if (amount > float) {
// We'll start at the tip of the stack and traverse backwards.
uint256 currentIndex = strategies.length - 1;

// Iterate in reverse so we pull from the stack in a "last in, first out" manner.
// Will revert due to underflow if we empty the stack before pulling the desired amount.
for (; ; currentIndex--) {
uint256 missing = amount - float;

IERC4626 strategy = strategies[currentIndex];

uint256 withdrawableAssets = strategy.previewRedeem(
strategy.balanceOf(address(this))
);

if (withdrawableAssets >= missing) {
strategy.withdraw(missing, address(this), address(this));
break;
} else if (withdrawableAssets > 0) {
strategy.withdraw(
withdrawableAssets,
address(this),
address(this)
);
float += withdrawableAssets;
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

we can save gas by sending the funds directly to the user:

if float > amount: send all funds from vault's own balance, if float < amount: withdraw from strat and send directly to user. Vault's funds stay in the vault.

That way we'd minimize the amount of ERC20 tranfers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i mean if we have enough float the function should return and we have the transfer in the withdrawal function. Or am i missing smth?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah but if we don't have enough float we pull from the strats to the vault and then transfer from the vault to the user.

So ERC20 flows:
strat1 -> vault
strat2 -> vault
strat3 -> vault
vault -> user

if we do:
strat1 -> user
strat2 -> user
strat3 -> user

we have 1 less ERC20 transfer which is about ~50k gas I think

Copy link
Contributor

Choose a reason for hiding this comment

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

That'll be the default flow for withdrawals anyways since the vault will only hold assets when paused or strats are changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

aaaah gotcha! makes sense. ill change that

src/vaults/MultiStrategyVault.sol Show resolved Hide resolved
@RedVeil RedVeil merged commit 2f88e26 into main Jan 29, 2024
1 check failed
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.

2 participants