i think there’s a typo in the final spec, see - typo in 4626 · Issue #5084 · ethereum/EIPs · GitHub
I find the spec for the convert
methods somewhat confusing. It says these methods must not reflect slippage or “other on-chain conditions”. Obviously the price will depend on on-chain conditions to some degree and conversion has to reflect that in some way. So what is the point that the spec is trying to make?
Would it be correct for convert methods to behave like a spot price?
The security considerations section says that convert
methods are “allowed” to be inexact and that they “can” be implemented as robust price oracles like TWAPs. Can you confirm that this is not mandatory? I think there is some abiguity or potential for confusion by consumers of the spec.
Amount the convertion methods, I’m also wondering if they should reflect the average price of existing shares, or if they should reflect the local tangent to the price curve.
These might be two different things in the case of bounding curve.
Suppose we want to have a ERC-4626 contract which takes in an asset A and then swaps it for another token B on deposit. That token B itself is yield bearing. Similarly, for withdraw, it swaps back B to A.
What do we return for totalAssets()
in this case? Is this standard a good fit for this use case?
The wording could be more explicit that the convert*
methods are supposed to be robust against on-chain manipulation, and cheaper that the preview*
methods, at the expense of precision.
Robust against on-chain manipulation:
Each specific implementation has some freedom here, but if you see that the conversion rate in your vault can be manipulated, and you intend to provide a conversion function that is resistant to such manipulation, convert*
methods are where that logic goes.
It’s important to see this against the goal of the preview*
methods, that are expected to be exact to the wei, and therefore often manipulable with flash loans and the like.
If you are an integrator looking for a price feed, for example to determine collateralization levels, you know that you need to get your data from convert*
to have the best chances of not being exploited.
Cheaper than the preview*
methods
preview*
methods are expected to be exact to the wei to allow for tight integrations, and this comes at a cost in gas. When calling preview*
as a given user all on-chain conditions will be taken into account so that the result will be exact. For this reason, preview*
methods can be expensive.
convert*
methods don’t take an account
parameter, so they can’t take individual conditions into account, reducing gas cost. Likewise, they could and should ignore a number of other conditions to obtain an approximate value that is still fit for purpose, but cheap to use.
This is still not clear to me what the convert*
method should use as a base reference:
- the price of historical assets (basically the ratio between
totalSupply()
andtotalAssets()
) - the current marginal price (local tangent to the price curve)
The two might be significantly different
There is definitely some degree of ambiguity here. In non-bonding curve pro rata use cases, these two are the same.
I would consider using the ratio between totalSupply and totalAssets as this is a conservative bound on the price if a massive redemption were to occur.
if the vault is denominated in A on input and output, totalAssets should return the total value of A that would be received by liquidating the position in B.
Is B a derivative of A like cDAI:DAI?
Thanks for taking the time!
A is WETH and B is icETH, and the swapping happens through a DEX. icETH has a debt component of WETH and an equity component of stETH.
So, we would need an oracle to get DEX prices.
ideally the vault would report underlying in WETH as I mentioned above. Make sure the share price is manipulation resistant (an oracle would be a good way to do this).
What should be the flow for the native currency (ETH) as an underlying?
Is it better to change the deposit/withdraw to payable
or have an additional function for handling ETH deposits (depositETH/withdrawETH)?
Its a bit late now that the ERC is finalized, but I’m wondering why “minOut” / “maxIn” parameters were not included in the deposit
, mint
, withdraw
and redeem
functions.
While the preview functions give an accurate prediction, this may change between the moment a user signs a transaction and the moment its mined. Frontrunning in particular make this ERC possibly dangerous without these additional checks.
I would recommend this ERC gets extended to include
function deposit(uint256 assets, address receiver, uint256 minShares) external returns (uint256 shares) {
shares = deposit(assets, receiver);
require(shares >= minShares);
}
function mint(uint256 shares, address receiver, uint256 maxAssets) external returns (uint256 assets) {
assets = mint(shares, receiver);
require(assets <= maxAssets);
}
function withdraw(uint256 assets, address receiver, address owner, uint256 maxShares) external returns (uint256 shares) {
shares = withdraw(assets, receiver, owner);
require(shares <= maxShares);
}
function redeem(uint256 shares, address receiver, address owner, uint256 minAssets) external returns (uint256 assets) {
assets = redeem(shares, receiver, owner);
require(assets >= minAssets);
}
As per the spec,
totalAssets
“Total amount of the underlying asset that is “managed” by Vault.”
Should the totalAssets return the underlying asset deposited + extra assets including airdrops, donation etc OR only assets deposited to the vault ?
If its latter, then only returning vault balance of asset is not enough.
At mStable we are building a number of different EIP-4626 vaults. One of them deposits assets in an AMM. We have implemented a mechanism to protect against sandwich attacks by limiting the amount of slippage allowed on operational functions like deposit
, mint
, withdraw
and redeem
.
My question is in regards to the preview functions. Should the preview functions revert if there is too much slippage like the operational functions?
For previewDeposit
, the EIP states “MUST NOT revert due to vault specific user/global limits. MAY revert due to other conditions that would also cause deposit to revert.”
So is too much slippage a “vault specific user/global limit”?
Or is it “other conditions that would also cause deposit to revert”?
Personally, don’t think the preview functions should revert if there is more slippage than what the vault allows.
Should totalAssets
include slippage or not?
The EIP for convertToShares
and convertToAssets
states
MUST NOT reflect slippage or other on-chain conditions, when performing the actual exchange.
Does this same statement apply to totalAssets
?
BTW, I don’t like the “or other on-chain conditions” statement. Transactions can only see on-chain conditions so it’s impossible for convertToShares
and convertToAssets
not to reflect on-chain conditions. eg the amount of shares and assets in the vault is an on-chain condition.
Personally, I think totalAssets
needs to be consistent with convertToShares
and convertToAssets
and not include slippage or fees.
I have another clarification to the preview functions.
The specification for previewDeposit
states
Allows an on-chain or off-chain user to simulate the effects of their deposit at the current block, given current on-chain conditions.
The “current block” implies that the on-chain conditions is at a block level and not transaction level. That is, previewDeposit
needs to allow for other transactions being included before it in the block. For vaults that have slippage, that would mean previewDeposit
would have to return the worst case of a sandwich attacked transaction before it reverted with a slippage error.
But the previewDeposit
specification also states
MUST return as close to and no more than the exact amount of Vault shares that would be minted in a
deposit
call in the same transaction. I.e.deposit
should return the same or moreshares
aspreviewDeposit
if called in the same transaction.
This statement implies that the on-chain conditions is at a transaction level and not block level.
Personally, I think the preview functions should be at a transaction level rather than block level. I would recommend changing the first line of the previewDeposit
specification to
Allows an on-chain or off-chain user to simulate the effects of their deposit at the current transaction, given current on-chain conditions.
The same also applies to mint
, redeem
and withdraw
.
100% agree with that.
I think its for the caller to decide what outcomes are good for him.
I would like to suggest an extension of EIP-4626 that allows for more cost-efficient redemptions. Currently, redemptions return the underlying asset of a vault. However, in cases where yield is generated by taking positions in assets other than the vault’s underlying asset, it would be more efficient for an EOA to source their own liquidity upon redemption, for example via an off-chain RfQ.
A solution would be a multi-asset redeem function that returns a basket of tokens. The return values of such a function and the corresponding preview would be dynamic arrays address[] and uint256[] for the tokens and amounts (to be) returned.