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

previewWithdraw logic #305

Open
tess3rac7 opened this issue Aug 11, 2022 · 3 comments
Open

previewWithdraw logic #305

tess3rac7 opened this issue Aug 11, 2022 · 3 comments

Comments

@tess3rac7
Copy link

Hi there, here's previewWithdraw as coded in the solmate ERC4626 mixin:

function previewWithdraw(uint256 assets) public view virtual returns (uint256) {
    uint256 supply = totalSupply; // Saves an extra SLOAD if totalSupply is non-zero.

    return supply == 0 ? assets : assets.mulDivUp(supply, totalAssets());
}

If totalSupply is 0 (meaning no vault shares exist), it returns the assets passed in.

However, here's previewWithdraw as coded in fubuloubo's repo (comparing these two since they're both listed as reference implementations on the official EIP-4626 page):

function previewWithdraw(uint256 assets) external view returns (uint256) {
    uint256 shares = convertToShares(assets);
    if (totalSupply() == 0) return 0;
    return shares;
}

In this case, if totalSupply() is 0, it returns 0.

How do we reconcile the two? Which is the correct behavior as per the spec? It makes more sense to me to return 0 since the spec says:

MUST return as close to and no fewer than the exact amount of Vault shares that would be burned in a withdraw call in the same transaction

@zoey-t
Copy link

zoey-t commented Sep 9, 2022

I think the idea is when the vault is empty, the share and asset ratio is default 1:1.

@zoey-t
Copy link

zoey-t commented Sep 9, 2022

for oz's erc4626, it adds a function that allows you to customize the initial ratio OpenZeppelin/openzeppelin-contracts#3639

@Akshat-sGit
Copy link

To reconcile the two you can return 0 instead of assets value if the totalSupply is zero. This behavior may not satisfy the requirement of returning the exact amount of Vault shares that would be burned in a withdraw call.

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

No branches or pull requests

3 participants