Skip to content
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

Add Uniswap pool fee as functions arg #400

Merged
merged 19 commits into from
May 4, 2022
Merged

Conversation

haythemsellami
Copy link
Member

@haythemsellami haythemsellami commented Apr 29, 2022

Add Uniswap pool fee as functions arg

Description

This will allow choosing different uniswap pool from fe/user.

Fixes #294

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Document update

How Has This Been Tested

Please describe how to test to verify the changes. Provide instructions so we can reproduce.

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • Added video recordings if it is a UI change

@vercel
Copy link

vercel bot commented Apr 29, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
continuouscall ✅ Ready (Inspect) Visit Preview May 4, 2022 at 7:57PM (UTC)

@haythemsellami haythemsellami changed the title [WIP] Add Uniswap pool fee as functions arg Add Uniswap pool fee as functions arg Apr 29, 2022
@haythemsellami haythemsellami requested a review from aleone April 29, 2022 09:43
Base automatically changed from fix/excess-amounts to main April 29, 2022 11:48
haythemsellami and others added 10 commits April 29, 2022 14:57
Copy link
Contributor

@alpinechicken alpinechicken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Contributor

@aleone aleone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm generally, but think we should add in logic to replace the diamond storage for wPowerPerpPool and have it be a param, like the pool fee for at least non vault based actions (batchMintLp, rebalanceWithoutVault, etc). That way someone can use the ControllerHelper to LP in non 0.3% pools. Seems pretty straightforward and makes the ControllerHelper fully agnostic to pools (LPing and trading). Note that (if easier) it doesn't need to support arbitrary pools for vault based actions (flashloan mint and LP for example) as they will fail as Controller will not accept them as collateral.

@haythemsellami haythemsellami merged commit 223dc22 into main May 4, 2022
@haythemsellami haythemsellami deleted the feat/add-pool-address branch May 4, 2022 20:43
@haythemsellami haythemsellami restored the feat/add-pool-address branch May 4, 2022 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants