transmissions11/solmate

`previewWithdraw` logic

tess3rac7 opened this issue · 3 comments

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

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

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

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.