-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
EIP-4626: Tokenized Vault Standard #4626
Changes from 3 commits
dfc160b
90c4fbb
8adc0d9
b9b6eec
636f8e2
e4cb510
39a614e
e6abeeb
f198798
9de5af5
b0381e6
b30ef7a
98d9490
efb8c0a
46b47bf
50fe1da
295f442
8a0ff46
6fb327b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,155 @@ | ||
# Yield Bearing Vault Standard | ||
|
||
Joeysantoro marked this conversation as resolved.
Show resolved
Hide resolved
|
||
eip: 4626 | ||
title: Yield Bearing Vault Standard | ||
description: A standard for yield bearing vaults | ||
Author: Joey Santoro (@joeysantoro), t11s (@transmissions11), Jet Jadeja (@JetJadeja) | ||
Status: Draft | ||
Type: Standards Track | ||
Category: ERC | ||
Created: 2021-12-22 | ||
Joeysantoro marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Joeysantoro marked this conversation as resolved.
Show resolved
Hide resolved
|
||
## Simple Summary | ||
|
||
A standard for yield bearing vaults. | ||
|
||
## Abstract | ||
|
||
The following standard allows for the implementation of a standard API for yield bearing vaults within smart contracts. This standard provides basic functionality for depositing and withdrawing tokens and reading balances with an optional extension for tokenized vaults using ERC-20. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is the ERC20 tokenized vault optional? I understand that there might be an interest to describe the Yield-Bearing Vault specification separately, to be able to combine it later on with ERC-20, ERC-721, ERC-1155, or some other custom tokenization standard. However, the EIP does a halfway job of dissociating itself from ERC-20, stating it as optional, which turns it confusing. I would suggest to either remove the optionality of tokenized vaults, and specify a pure Yield-Bearing Vault EIP, or make ERC-4626 Vaults to inherit from ERC-20 as a requirement. Either case would make the EIP clearer and more robust. My preference would be to make ERC-4626 Yield-Bearing Vaults to compulsorily inherit from ERC-20, and to provide a complete standard that also matches closely the existing implementations. I think it will be quite hard to specify a Yield-Bearing Vault with no relationship to any tokenizing standard, yet with enough meaning to be useful. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i am ok with enforcing tokenization if consensus is there but we strayed away from specifying it cuz there are many different approaches to doing it beyond a simple non-rebasing erc20 and we want the core deposit and withdraw logic to be adopted widely There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that you can require the vault to be an ERC20 in which balances represent ownership of the vault (shares), without specifying how shares are allocated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In favor, will enforce ERC20 compatibility |
||
|
||
## Motivation | ||
|
||
Yield bearing vaults have a lack of standardization leading to diverse implementation details. Some various examples include lending markets (Compound, Aave, Fuse), aggregators (Yearn, Rari Vaults, Idle), and intrinsically interest bearing tokens (xSushi). This makes integration difficult at the aggregator or plugin layer for protocols which need to conform to many standards. This forces each protocol to implement their own adapters which are error prone and waste development resources. | ||
|
||
A standard for yield bearing vaults will allow for a similar cambrian explosion to ERC-20, unlocking access to yield in a variety of applications with little specialized effort from developers. | ||
|
||
|
||
## Specification | ||
|
||
### Methods | ||
lightclient marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
#### deposit | ||
|
||
`function deposit(address _to, uint256 _value) public returns (uint256 _shares)` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any way that address to send shares to can be the second parameter? In yearn vaults, we have them in this order because in decreasing order of usage:
Depositing everything to another person is rare There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cool with me, makes sense in languages (vyper) that have optional args There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a decent preference for the other order given it follows ERC20 spec. Wouldn't yearn need to still make adapters for the remaining functions anyway? What is the gain by switching if its only limited backward compatibility There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a side note, the likelihood any protocol makes an adapter is quite low, I didn't make the suggestion for backwards compatibility reasons. Anyways, I don't think ERC20 plays a huge role here in this spec in terms of driving the interface of these operations. Merely consider that the addition of override functions might be a good way to reduce gas costs for common operations, if the ordering is correct. So adding: function deposit() public returns (uint256 _shares) // Deposit all to self
function deposit(uint256 _value) public returns (uint256 _shares) // Deposit some to self
function deposit(uint256 _value, address _to) public returns (uint256 _shares) // Deposit some to other As another side note... I really wish people would start writing ERC specifications in YAML per EIP-2069 but that never really took off 😞 |
||
|
||
Deposits `_value` tokens into the vault and grants ownership of them to `_to`. | ||
|
||
MAY return a pro-rata ownership `_shares` value corresponding `_value`, if not MUST return `0`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As it stands now, the inputs to an EIP-4626 pool must be the underlying, which is the unit for valuing the pool and the user's position. I would like to make a case to allow the inputs to be something else, called the deposit token. There are a few cases where the deposit token could be different from the underlying:
To allow for these use cases, we just need to make one additional view function and three semantic changes:
In short: These semantic changes do not alter how things currently work but will make the EIP more general and future-proof. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is no need for this, as what the vault does with the deposited tokens internally is completely irrelevant to this ERC interface. underlying signifies what comes in and out of the vault and what totalHoldings() and such is measured in, we do not specify what happens to deposited underlying tokens. you can impl the vaults you proposed without additional metadata. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's say we want to implement this vault A:
Option 1: If we use the current standard:
Option 2: if we apply the proposed semantic changes here, it can be:
If we take the perspective of let's say a UI to show the details of this vault, it makes more sense to show the user balance in USDC using balanceOfUnderlying(), and easier to compare this vault with other vaults with the same USDC underlying. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that explicitly specifying a multi-layer approach adds complexity to the standard, which should be kept as simple as possible. Not all existing vaults will be immediately compliant and that must be accepted. As such, I don't think that a However On specifying inputs based on
This follows the pattern seen in AMMs that also deal with two tokens (in this case the underlying and the vault tokens). Externally, the vault does the same job of exchanging between the two, and it is irrelevant the internal mechanism used to determine the exchange rate. I'm not arguing to provide an interface identical to any existing AMM, but a familiar set of methods, proven to work in an equivalent use case, would definitely help with integration. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it makes a lot of sense to add a "mint" method which does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mrenoon Perhaps whatever wants the USDC value can go through two interfaces. First determine SHARES/aUSDC value via ERC-4626 and then determine aUSDC/USDC via say a DEX or another ERC-4626. This is kind of composing two "valuation" interfaces which have "calculate" functions. Could that be standardized independent of the rest of EIP-4626? Both ERC-4626 and a DEX could implement this "valuation" interface. Then the logic which composes two valuation interfaces could be wrapped up into its own valuation interface, achieving your goal of going through this one interface which calculates the SHARES/USDC valuation directly. Edit: Alternatively, one could wrap an ERC-4626 using aUSDC as its underlying into another ERC-4626 which behaves "as if" USDC is the underlying. This implementation may go through two other ERC-4626s or similar to above, may go through a DEX. Side note: You could nearly argue that a DEX could implement ERC-4626 (who says it needs to be a new shares token? One token in the pair acts as an investment into the pool where one expects the number of the other token to increase over time). |
||
|
||
MAY represent `_shares` using internal accounting or an ERC-20 token. | ||
|
||
If pro-rata shares ownership is implemented, the vault SHOULD implement `balanceOf`, `redeem`, `totalSupply` and `exchangeRate`. | ||
|
||
|
||
#### withdraw | ||
|
||
`function withdraw(address _to, uint256 _value) public returns (uint256 _shares)` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I love the idea of standardizing Vaults. But every single Vault Product in DeFi uses Additionally some / most shares end up being a token of their own and people think of them that way, (bDigg, bveCVX, yveCRV, etc...) I recommend having For context: Badger Vaults: Mushroom Finance: Inverse Finance: The only notable example that doesn't follow this pattern is Idle Finance, which has a unique interface: As an addendum, to avoid further invalidating already existing projects, there could be multiple overloaded withdraw signatures similarly to how Yearn V2 did which would allow most projects to comply to the standard without changing it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am open to reordering or making Wdyt @transmissions11? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree on this, I think the norm of most protocols is to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, see above. I think that both the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Happy to agree to disagree, I'm not making any statement about what I think the code should look like. The code that is being used in production by the majority of protocols looks like the following Happy to go into detail as to why that is the case, but this interface has survived over hundreds of forks on all EVM compatible chains, and as such shows wisdom in its simplicity There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of the cToken-based protocols (Compound, Rari, Cream, etc) use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You can't withdraw to someone else either, the bank analogy falls flat There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
As for withdrawing $100 dollars, the reason why fork over fork never killed the Illustrated by literally the first withdraw I found on Rari's smart contract https://etherscan.io/tx/0xcada09b35d35aaee76fdaf2a00b8f275b07ced7eb8c80cf48cc5dac42c4832ea Did the depositor really want to keep those 0.02 tokens? Or is the implementation that forces them into something they don't want? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Definitions:
It doesn't seem to say anything about where the money ends up (the destination), so I think you can withdraw into someone else's account. On apps like Binance (a crypto exchange), you just withdraw to some arbitrary wallet address, Binance doesn't care who owns the destination address. If anything, withdraw is commonly used to refer to a separate destination than your own. But, most importantly, withdrawals have always historically referred to the actual asset (the underlying asset). I agree that redeem would make more sense here (when it's in terms of the wrapper token/shares), but I can understand your point of view since many protocols have used the term withdraw here (IMO) incorrectly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Regardless, the term "redeem" is perhaps better-fitted than "withdraw". I agree, there definitely may be an added complexity when supporting |
||
|
||
Withdraws `_value` tokens from the vault and transfers them to `_to`. | ||
|
||
MAY return a pro-rata ownership `_shares` value corresponding to `_value`, if not MUST return `0`. | ||
|
||
|
||
#### totalHoldings | ||
`function totalHoldings() public view returns (uint256)` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There ought to be some documentation here or elsewhere about whether this view should rely on external state. One There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The interface is unopinionated in the same way totalSupply is unopinionated for ERC20. We expect most implementations to use a shares based model. It will always be up to the integration layer to control for "safe" vaults based on their own security model There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like totalUnderlying as well. @transmissions11 @JetJadeja wdyt? |
||
|
||
Returns the total amount of underlying tokens held/managed by the vault. | ||
|
||
|
||
#### balanceOfUnderlying | ||
`function balanceOfUnderlying(address _owner) public view returns (uint256)` | ||
|
||
Returns the total amount underlying tokens held in the vault for `_owner`. | ||
|
||
#### underlying | ||
`function underlying() public view returns (address)` | ||
|
||
Returns the address of the token the vault uses for accounting, depositing, and withdrawing. | ||
|
||
SHOULD return a token implementing the ERC-20 standard. | ||
|
||
|
||
#### totalSupply | ||
|
||
`function totalSupply() public view returns (uint256)` | ||
|
||
Returns the total number of unredeemed vault shares in circulation. | ||
|
||
OPTIONAL - This method is only needed for vaults that implement a pro-rata share mechanism for deposits. | ||
|
||
#### balanceOf | ||
|
||
`function balanceOf(address _owner) public view returns (uint256)` | ||
|
||
Returns the total amount of vault shares the `_owner` currently has. | ||
|
||
OPTIONAL - This method is only needed for vaults that implement a pro-rata share mechanism for deposits. | ||
|
||
#### redeem | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given compound's I think this makes the method's use case a bit more clear at a glance, though There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup its a 1-1 of But if there is no Again, I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fair points
thats t11s for ya ;P |
||
|
||
`function redeem(address _to, uint256 _shares) public returns (uint256 _value)` | ||
|
||
Redeems a specific number of `_shares` for underlying tokens and transfers them to `_to`. | ||
|
||
MAY return a pro-rata ownership `_shares` value corresponding `_value`, if not MUST return `0`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the documentation here is reversed, the return parameter here is rightfully value There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yup |
||
|
||
OPTIONAL - This method is only needed for vaults that implement a pro-rata share mechanism for deposits. | ||
|
||
#### exchangeRate | ||
|
||
`function exchangeRate() public view returns (uint256)` | ||
|
||
The amount of underlying tokens one `baseUnit` of vault shares is redeemable for. | ||
Joeysantoro marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
e.g. `_shares * exchangeRate() / baseUnit() = _value`. | ||
Joeysantoro marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
`exchangeRate() * totalSupply()` MUST equal `totalHoldings()`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we need to divide by the right scalar cuz totalHoldings decimals is scaled by underlying decimals There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great call, should we remove this MUST condition or add the decimals scalar? Leaning former There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. im not too opinionated either way tbh, just want to make sure we clarify the decimals one way or another There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i do think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's easier and more general to use a Likewise, you should have the symmetrical This pattern has been used in the yield oracles with great success, and partially in the lido staked ether contract where you have also symmetrical functions for asset conversions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To give another example, if a vault is implemented with a fee on deposits or withdrawals, it won't be possible for an integrating contract to know the deposit or withdrawal output, without specific knowledge of those fees. By putting any conversion logic behind the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this suggestion as part of the OPTIONAL interface instead of or in addition to exchangeRate(). As an integrating contract, the only time I'd want to use exchangeRate() would be to calculate one of these two conversions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would suggest this as instead If you would still be interested in the exchange rate for a given use case, you can obtain it by passing one full unit as the amount parameter. Furthermore, using two parameterized functions removes the constraint on how tokens are converted for shares, which the EIP forces to a linear, no-fee formula (otherwise If still unconvinced, I'd like to hear the arguments in favour of a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm writing a generic ERC4626 router and all of your advice has become very appropriate as I encounter some of the nuances of the implementation details between vaults of different types |
||
|
||
OPTIONAL - This method is only needed for vaults that implement a pro-rata share mechanism for deposits. | ||
|
||
#### baseUnit | ||
|
||
`function baseUnit() public view returns(uint256)` | ||
|
||
The decimal scalar for vault shares and operations involving `exchangeRate()`. | ||
|
||
OPTIONAL - This method is only needed for vaults that implement a pro-rata share mechanism for deposits. | ||
Joeysantoro marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
### Events | ||
|
||
#### Deposit | ||
|
||
MUST be emitted when tokens are deposited into the vault. | ||
|
||
`event Deposit(address indexed _from, addres indexed _to, uint256 _value)` | ||
Joeysantoro marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Where `_from` is the user who triggered the deposit and approved `_value` underlying tokens to the vault, and `_to` is the user who is able to withdraw the deposited tokens. | ||
|
||
|
||
#### Withdraw | ||
|
||
MUST be emitted when tokens are withdrawn from the vault by a depositor. | ||
|
||
`event Withdraw(address indexed _owner, addres indexed _to, uint256 _value)` | ||
Joeysantoro marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Where `_from` is the user who triggered the withdrawal and held `_value` underlying tokens in the vault, and `_to` is the user who received the withdrawn tokens. | ||
|
||
|
||
## Rationale | ||
|
||
The vault interface is designed to be optimized for minimal implementation and integration logic while maintaining flexibility for both parties. Details such as accounting and allocation of deposited tokens are intentionally not specified, as vaults are expected to be treated as black boxes on-chain and inspected off-chain before use. | ||
|
||
lightclient marked this conversation as resolved.
Show resolved
Hide resolved
|
||
## Reference Implementation | ||
|
||
[Solmate Minimal Implementation](https://github.com/Rari-Capital/solmate/pull/88) - a tokenized vault using the ERC-20 extension with hooks for developers to add logic in deposit and withdraw. | ||
|
||
[Rari Vaults](https://github.com/Rari-Capital/vaults/blob/main/src/Vault.sol) are an implementation that is nearly ready for production release. Any discrepancies between the vaults abi and this ERC will be adapted to conform to the ERC before mainnet deployment. | ||
|
||
## Security Considerations | ||
|
||
This specification has similar security considerations to the ERC-20 interface. Fully permissionless yield aggregators, for example, could fall prey to malicious implementations which only conform to the interface but not the specification. | ||
|
||
## Copyright | ||
|
||
Copyright and related rights waived via [CC0](https://creativecommons.org/publicdomain/zero/1.0/). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please follow the template found at https://raw.githubusercontent.com/ethereum/EIPs/master/eip-template.md
The formatting on this header is incorrect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done