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

[uniswap-v3] Add Pilot Protocol #4

Merged
merged 14 commits into from Aug 10, 2021
Merged

[uniswap-v3] Add Pilot Protocol #4

merged 14 commits into from Aug 10, 2021

Conversation

anassohail99
Copy link
Contributor

Changes proposed in this pull request:

  • Added Uniswap V3 compatibility
  • Added Pilot Protocol

@bonustrack
Copy link
Member

@anassohail99 Also i think it would make more sense to move the Uniswap v3 calculation to a more generic strategy, this strategy could be used by many others projects on Snapshot.

@anassohail99
Copy link
Contributor Author

@anassohail99 Also i think it would make more sense to move the Uniswap v3 calculation to a more generic strategy, this strategy could be used by many others projects on Snapshot.

@bonustrack Updated

@bonustrack
Copy link
Member

@anassohail99 Can you create a generic strategy with the name "uniswap-v3" just for the uniswap calculations? And do separated strategy "unipilot"?

src/strategies/index.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@bonustrack bonustrack changed the title Add Pilot Protocol [unipilot] Add Pilot Protocol Aug 7, 2021
@ChaituVR
Copy link
Member

ChaituVR commented Aug 7, 2021

@anassohail99 Hey, Also update your fork branch with the latest changes from master :)

@ChaituVR ChaituVR changed the title [unipilot] Add Pilot Protocol [uniswap-v3] Add Pilot Protocol Aug 7, 2021
@ChaituVR
Copy link
Member

@anassohail99 Please resolve conflicts, also would be good to install these two dependencies with npm instead in yarn, because yarn on this repo is not tested yet - may break other dependents

@anassohail99
Copy link
Contributor Author

@ChaituVR I've updated can you review it and merge it thanks.

@TimDaub
Copy link

TimDaub commented Aug 10, 2021

IMO there could be a comment somewhere explaining what options "tokenReserve" means. I understand that it's selecting the token0 of a Uniswap V3 pool for the number of votes, correct?

@ChaituVR ChaituVR merged commit e819e76 into snapshot-labs:master Aug 10, 2021
3 checks passed
ChaituVR added a commit that referenced this pull request Aug 10, 2021
@ChaituVR
Copy link
Member

Strategy is live here: https://snapshot.org/#/strategy/uniswap-v3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants