-
Notifications
You must be signed in to change notification settings - Fork 5
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
Uni v3 Zap #1179
Uni v3 Zap #1179
Conversation
Pull Request Test Coverage Report for Build 11282077538Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Hyperdrive Gas Benchmark
This comment was automatically generated by workflow using github-action-benchmark. |
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.
Great work! I just had a few qs and some nits
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.
Looks awesome, really great work on the documentation... super digestable
* forge install: v3-periphery v1.3.0 * Added a basic Uniswap v3 zap contract * Added a Uni v3 zap for `closeLong` and improved the zap for `openLong` * Added a zap for `addLiquidity` * Cleaned up the validation logic for Uni v3 zaps * Adds zaps for `removeLiquidity` and `redeemWithdrawalShares` * Added the remaining zaps * Started writing tests for `addLiquidityZap` * Updated the zap to use multi-hop routing * Wrote some more tests * Refactored the `addLiquidityZap` tests * Added tests for `openLongZap` * Added a full test suite for `openLongZap` * Broke out the zap tests and added test for `openShortZap` * Added support for zaps out starting with ETH * Added tests for `removeLiquidity` * Added tests for `redeemWithdrawalSharesZap` * Added tests for `redeemWithdrawalSharesZap` * Added tests for `closeLongZap` * Added tests for `closeShortZap` * Cleaned up the tests * Bump the version * Remaining cleanup * Fixed the UniV3Zap * Added wrapping tests * Some cleanup * Some cleanup * Addressed review feedback from @jrhea * Addressed remaining review feedback from @jrhea * Appeased the linter gods * Addressed review feedback from @mcclurejt
Description
This PR adds a zap contract that supports generalized swapping between assets using Uniswap V3. Each Hyperdrive trading or LP function has a direct anologue in the zap contract that exposes all of the parameters as well as a few extra parameters for things like the Uniswap swap object (
ISwapRouter.ExactInputParams
) and special-case parameters to handle different edge-cases in the zap.Extensive tests were added that should test the different paths through these functions. Special care was taken to verify that ETH, WETH, and rebasing tokens are properly handled by the zap contract. Since Uniswap v3 doesn't really support ETH as a first-class citizen in all situations, there are several flows within the zap contract where ETH will be converted to WETH or vice-versa. For the most part, this is unopinionated and will support all use-cases, but in the case of zapping funds out of the zap, WETH will always be unwrapped into ETH before sending it to the user.
Some notable things to watch out for are:
openShortZap
's lack of precision compared to the other flows. This is unavoidable because of the way that Hyperdrive'sopenShort
logic functions.