Univ2 LP Deposit After Univ3 Trade #18
Conversation
|
This is basically ready for review, pending discussion on reference price fetching architecture. The current code assumes there could be multiple reference price sources ( Note that a fixed price quoter or quoters based on Chronicle/Chanlink can be supported, as long as they conform to I'd also like us to consider renaming the kiln instances to a general name with a running index (e.g |
… router Previousluy the v2 deposit price limits check used amountAMin and amountBMin. That has been changed to an explicit price check after the deposit. The reason the previous v2 deposit price limits check is not good enough is that it uses amountADesired/amountBDesired its reference price and not the quote reference price as we would have wanted. The code did attemp to make up for that by "shifting" amountAmin and amountBmin to center around the quote reference, but that doesn't fully work since the v2 router price limit check validates only one of the min or max prices and not both. The router chooses whether to validate the min or max price according to the direction that the price moves vs the price of amountADesired / amountBDesired (which is in our case the v3 price we got on the first swap). Can see that in the amountAMin and amountBMin parameters documentation here: https://docs.uniswap.org/contracts/v2/reference/smart-contracts/router-02#addliquidity Thus there would be no limit enforcemenet for some situations. For example: [ <-quote-> ] v3 v2 Where `quote` represent the quote reference price, the range around it is `zen`, v3 is the price we got on univ3 and v2 is the v2 deposit price. In such a case since the v2 price is out of the zen range the tx should fail, but since the price went up in comparison to v3, the price check would only be to the down side (and would therefore succeed).
|
|
||
| uint256 _zen = zen; | ||
| uint256 depositToQuote = (depositB * WAD / depositA) * WAD / (quote * WAD / _halfIn); | ||
| require(depositToQuote >= _zen && depositToQuote <= (2 * WAD - _zen), |
There was a problem hiding this comment.
This will need some tests against USDC and other tokens with lt 18 decimals to ensure it holds up.
|
|
||
| // If not all buy tokens were used, send the remainder to the receiver | ||
| if (bought > depositB) { | ||
| unchecked { GemLike(buy).transfer(receiver, bought - depositB); } |
There was a problem hiding this comment.
This is the only part that still has me a little uncomfortable.
If the purpose of using the kiln is to get rid of sell token in a permissionless way, dropping the buy token back on the user in dusty amounts means they'll have to find another way to get rid of them if what they really want is the LP token.
Need to think about whether it may be possible to query the UniV2 router first and adjust down the amount we're buying so that we can only spend the amount necessary to fill the deposit order in V3.
I think it would be better to leave sell token in the contract that could be used in the next trade instead of overbuying buy token and having to send it to the deployer.
|
|
No description provided.