EIP-4626: Yield Bearing Vault Standard

maybe it’s
vault.calculateUnderlying(10**vault.decimals()) ?

That’s correct, and that’s intentional.

A function such as exchangeRate returns a factor, and there is no standard as to which how many decimals a factor should have. Compound notoriously defines this as an unsigned integer, scaled by 1 * 10^(18 - 8 + Underlying Token Decimals).

If you want to know how much underlying you need to pay for one share, you ask for that, and that’s what you get with no further guessing.

Note that if the vault would have no decimals, the query would still work fine.

Also, in multi-hop oracle conversions, as we do for Yield, you don’t need to worry about decimals at all, it just works.

https://github.com/yieldprotocol/vault-v2/blob/master/contracts/oracles/composite/CompositeMultiOracle.sol

3 Likes

Also, when there is a fee on transfer during deposit, the amount of underlying token received by the vault will be less than the actual value of underlyingAmount in function deposit(address to, uint256 underlyingAmount), so when we account for the real underlyingAmount received (let’s call it realUnderlyingAmount), should we emit a Deposit event with realUnderlyingAmount or underlyingAmount. See code:

/// @notice Deposit a specific amount of underlying tokens.
/// @param to The address to receive shares corresponding to the deposit
/// @param underlyingAmount The amount of the underlying token to deposit.
function deposit(address to, uint256 underlyingAmount) external virtual returns (uint256 shares) {
    uint exchangeRate_ = exchangeRate();
    uint initialBalance = totalHoldings();
    // Transfer in underlying tokens from the user.
    underlying.safeTransferFrom(_msgSender(), address(this), underlyingAmount);
    // Determine the real amount of underlying token received .
    uint realUnderlyingAmount = totalHoldings() - initialBalance;
    shares = realUnderlyingAmount.fdiv(exchangeRate_, baseUnit);
    // Determine the equivalent amount of shares and mint them.
    _mint(to, shares);
    // Should we state realUnderlyingAmount or underlyingAmount in the Deposit event
    emit Deposit(_msgSender(), to, underlyingAmount);
    // This will revert if the user does not have the amount specified.
    
    afterDeposit(underlyingAmount);
}

Before you can deposit an asset, all of the existing protocols (Compound, Aave, Yearn, etc) first require you to approve the contract to spend the underlying asset. Can we somehow extend this standard and prevent every depositor from having to make two transactions? I believe EIP-2612 tried to accomplish this.

When you deposit into Aave or Rari or Origin or the (now deprecated) dYdX lending protocol on L1, you include the address of the underlying asset as one of the function arguments. This allows for a general lending pool contract that isn’t necessarily restricted to one specific reserve and allows for more than one reserve as the underlying asset. Can we include this as an optional argument? Here is how Aave is doing it.

Aave and Idle include a referral code with every deposit. This allows for future broker programs. Something to consider as an optional argument. Here is how Idle is doing it.

I think the withdraw() and redeem() functions need an argument for allowable slippage. For a lot of protocols this will could be ignored, though Yearnv2 for instance has a slippage argument. There are a lot of less-liquid strategies that wouldn’t be possible without the slippage argument.

Maybe this is the wrong EIP for this but having the ability to set up vaults where positions were represented by NFTs, e.g. as in UNIv3 LPs, might be really useful from how vaults are viewed from a tax perspective.

Whether or not the underlying uses EIP-2612 is irrelevant to this proposal, as it only cares about the allowances post any approval logic.

This proposal is for a tokenized vault on a single underlying. Making it too general hurts many use cases as it helps others.

Referrals are great, applications can add their own referral logic as an overloaded method in addition to the core spec. This is too peripheral to include in the spec.

slippage should be handled at the router layer. For example see [DRAFT] router here: ERC4626-Contracts/src/ERC4626Router.sol at main · ERC4626-Alliance/ERC4626-Contracts · GitHub

Probably the wrong EIP for this. A pseudo-ERC4626 could implement the core interface without implementing ERC20 to avoid certain tax treatment.

Has anyone put this into deployable code?

There are inconsistencies in the naming of function and return parameters

  • deposit, mint, withdraw and redeem refer to the amount of underlying tokens as value, while calculateShares and calculateUnderlying uses underlyingAmount. The Deposit and Withdraw events also use value.

    I prefer underlyingAmount over value as its clearer on what it is, but I also think just underlyings will do. The unit256 type makes it clear it’s an amount, not the address of the underlying token. Using the plural also makes it more obvious that its an amount.

  • deposit, mint, withdraw and redeem refer to the amount of shares as shares, while calculateShares and calculateUnderlying uses shareAmount.

    I suggest just shares is used across all function these calls.

Still mulling over the spec, a few comments:

  1. exchangeRate() was a nice addition to the standard. With Yearn v2 we use pricePerShare() which is perhaps more clear semantically that the underlying amount is per 1 unit of shares (10**decimals). “Exchange Rate” to me seems to express that a like-kind exchange is not happening, when actually it is (I am swapping the underlying for shares the represent similar value of the underlying I deposit)

  2. Whenever I’ve written code that interacts with the Vault, I prefer to directly measure the balance of the underlying via the token’s own balanceOf function, because that is a better guarantee of accuracy in the amount. totalUnderlying and balanceOfUnderyling can be set by the user of this ERC in a fairly arbitrary way, but that might be a good thing. Either way, you are accessing at least two SLOADs and probably 2 STATICCALLs as well. We’ve found with Yearn v2 that storing the balance locally prevents the type of issue that occured during the most recent Cream hack where the Yearn vault share price was boosted via airdrop. Still mulling this bit over, not sure which direction I’d advocate for.

  3. underlying as a term is perhaps a bit verbose. I also question whether it is a word that translates well to other languages. Yearn v3 uses token for this same concept. Consider tokens vs. underylings (which I don’t think is even a word) or underylingAmounts as a simpler term.

  4. Having to support both mint/redeem and deposit/withdraw (and the similar preview* methods) is additional overhead for methods that do essentially the same thing. A good spec would have one way of doing a thing, since you could replicate other ways quite easily. If you still prefer supporting both, consider making one (or both) of them extensions (selecting one of both as a “mandatory extension”?). That might make it more complicated overall though.

  5. As @SmoothBot noted, a slippage argument was very handy for us with Yearn v2, because often when withdrawing there are contextual issues that leads to there being not a 1-to-1 relationship between the share price and the actual amount available. In general, there are a lot of contextual issues on deposit and/or withdraw where one or both might not work the way the user expects (slippage, deposit limits, etc.). Perhaps a way to support this is to add a bool success return value to the preview* methods that show whether depositing or withdrawing is not allowed. You can also measure the preview* methods return amount against the similar way of computing share price to see if there is a loss you do not expect. A vault with a withdrawal fee would naturally have a % loss in the preview method that could be checked. I’m not sure how you’d specify a slippage loss though (would it be success = false?), but you could also measure the difference there to show the owner of the shares what the likely effects would be. In general, I like the idea of being able to simulate the outcome.

  6. Some Vaults have the concept of locked up withdrawals. I sort of mentioned that in a previous comment, but it is really an important point to make, and I’m not sure how else to make it. I suppose the preview* method returning success = false is one way to represent that, although there might be a partial amount available for withdrawals.

  7. In general, the amounts of shares/tokens to deposit/withdraw should be considered a “maximum” amount, if there is a deposit limit or a limited amount of funds to withdraw from, it should take the lesser of that or the user’s specified amount. In Yearn v2 we often use MAX_UINT256 to specify “take as much as possible”, which makes it the lesser of the user’s holdings or the available deposit limit/withdrawal amount. We also use overloading there, so when the amount argument is not specified, this is assumed.

  8. Lastly, I understand why the ordering of the arguments in the methods are they way they are (to evoke similarity to the ERC20 transfer/transferFrom methods), but here the context is much different. With Yearn v2, we optimized the ordering of arguments according to their frequency of use, for example deposit(amount, to) would be a better ordering because the overloaded methods deposit() (deposit everything I have and keep the shares) and deposit(amount) (deposit a specific amount and keep the shares) would be used much more often than deposit(amount, to) (deposit a specific amount and send the shares to someone else). This of course assumes there is overloading, which there is not currently in this ERC. If we stay away from rampant overloading, this is probably okay to keep as is.

I still have to think about how this might work. I like the concept behind the spec, and it will be something that Yearn very seriously considers for future iterations of the protocol.

1 Like

maybe something more like bool fullExchangeProcessed to show that a partial or full exchange occured.

Other ways of doing this would be extensions for deposit limit, deposit/withdrawal fee, locked withdrawals, etc.

I’ve been thinking about how to simplify this proposal down more, so that it has one obvious flow to it (without extensions or duplicate APIs), and also how to reflect various issues with yield-bearing vaults (from my perspective working with Yearn’s Strategies there are many things to consider: deposit limits, slippage on withdrawal, withdrawal lockups, fee accounting, etc.), and I came up with this:

Please feel free to read this and leave comments about format directly on it, or we can discuss/move parts of the design to this ERC discussion. I’d like to contribute to the design of this ERC with everything that we have learned in the past 2+ years of supporting production Vaults ourselves, and integrations with various types of them.

That’s a good point, I’d be inclined to emit events from the point of view of the contract, so that it’s state can be reproduced from them.

For example if there is a fee on transfer Deposit should show the amount received by the contract, after the fee is subtracted. Likewise, on a Withdraw it should show the amount deduced from the contract. The counterparties on a transfer should also emit events reflecting their own state changes.

Thanks for the suggestion, I think it has been corrected in later versions of the EIP

Lots of good points here, my own 2c:

  1. I also prefer pricePerShare(), but not a hill I will die on.
  2. Some vaults might do a difference between the tokens received, and the tokens managed, much like Uniswap v2 does. In those cases totalUnderlying reflects tokens that the contract recognizes as being managed. I.E. donations don’t count towards it.
  3. token is fairly vague. We could call it yeet if we go that route so that at least it’s not overloaded with meaning. With its verbosity, underlying is fairly well understood.
  4. There are good use cases for all four methods. deposit and withdraw are denominated in underlying, as in the user deciding how much USDC to deposit to or how much USDC to withdraw from his position. If that same user decides to completely close his position, he would need to make a reverse calculation to find out exactly how much USDC to withdraw as to not leave any xUSDC. The redeem method enables closing a position in a much easier way. Expanding on this, shares gain their own life via composability, and it is previsible that someone might want to exchange an exact number of shares for whatever underlying they are worth (within limits). Likewise, it is conceivable for someone to want to mint an exact number of shares, for example to pay off a share-denominated debt.
  5. I lost you here, could you please rephrase it?
  6. I think that in the case of an specific function call not being possible, both the mutable and the preview should revert. In other contexts we have implemented separate functions that compute limits, which are used in the frontend. Not sure about applicability here, though.
  7. That’s a good point, although in that case the preview functions should return both the amount that was taken from the user, and the amount that was given to the user.
  8. You should blame Fabian and Vitalik for this. While your parameter ordering makes sense, I feel it breaks convention too much.

If preview methods return both tokens taken and tokens given, a (0, 0) would easily represent that it is not allowed. Maybe a revert is still better, though.

I feel like I’ve seen this mentioned before but I can’t find it.

Does this EIP support vaults where withdrawal is a two-step process with a waiting period in between?

Would it be compliant to require an initiateWithdraw function call in order for withdraw/redeem to work?