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

feat: v2 + v3 factory address tracked for better discovery debugging / analysis #215

Merged
merged 3 commits into from
Oct 12, 2024

Conversation

HilliamT
Copy link
Contributor

@HilliamT HilliamT commented Sep 4, 2024

Motivation

A factory_address field in V2 and V3 proves helpful in understand what pools from which factories are discovered via the PairCreated / PoolCreated event. This can also prove helpful in analysing the contributions from which V2-like / V3-like protocols in pathing e.g 0x-style.

image

Solution

  • Added factory_address to UniswapV2Pool and UniswapV3Pool
  • Updated constructors
  • Updated tests

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@0xKitsune
Copy link
Collaborator

0xKitsune commented Sep 5, 2024

Thanks for the PR, this change makes sense to me!

Thoughts on making the factory_address optional? There are some cases where a user may want to initialize a handful of pools with new_from_address without needing to know the factory.

@HilliamT
Copy link
Contributor Author

HilliamT commented Sep 14, 2024

@0xKitsune Apologies for the delay - makes sense for factory_address to be optional.

One later function that can be added as a filter is to filter in only e.g v2 / v3 AMMs with a whitelisted factory_address, especially after discovering other AMMs from merely logs. This does require more thinking of how other AMM cases should be handled though, but can be done later when better thought out.

@0xOsiris 0xOsiris merged commit b6e8d38 into darkforestry:main Oct 12, 2024
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants