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

Extract all liquidity sources into separate crate #395

Closed
2 of 6 tasks
sunce86 opened this issue Jul 28, 2022 · 5 comments
Closed
2 of 6 tasks

Extract all liquidity sources into separate crate #395

sunce86 opened this issue Jul 28, 2022 · 5 comments

Comments

@sunce86
Copy link
Contributor

sunce86 commented Jul 28, 2022

This issue initiates discussion about moving out the liquidity sources data fetching into separate crate / service.

Main motivation for this is for external contributors/teams to be able to easily add new liquidity source.

Open question is: should we extract the sources into separate crate and use it from the driver, or should a separate service be built on top of new crate. In latter, data fetching would be moved from driver to solvers and solvers would have a higher flexibility to choose type/s of liquidity to fetch.

Additionally, there is a need for two mode data fetching service: one for fast retrieval of latest liquidity and second for historical retrieval of a liquidity at a specific block.

Related issue: Adding UniV3 liquidity source

Data fetching part is implemented in a separate repo: https://github.com/cowprotocol/cow-native-liquidity
Also, other already existing liquidity sources are copied over to the same repo.
There is a draft PR for using UniV3 PoolFetcher in driver and liquidity collector: #388 Note that UniV3 data fetching code is copy-pasted from cow-native-liquidity repo - this is done in order to provide UniV3 liquidity to Quasimodo solver ASAP.

To sumarize, here is a track for both issues

  • Implement data fetching for Uniswap V3 via subgraph in cow-native-liquidity
  • Extract all liquidity sources into cow-native-liquidity
  • Integrate Uniswap V3 into driver (PR drafted - almost done)
  • Improve Uniswap V3 data fetching (probably using events although Nick mentioned self hosting subgraph)
  • Delete all liquidity sources from driver and use cow-native-liquidity instead
  • Build service on top of cow-native-liquidity

Seeking approval for the direction I'm taking.

@MartinquaXD
Copy link
Contributor

Open question is: should we extract the sources into separate crate and use it from the driver, or should a separate service be built on top of new crate.

I'm a little conflicted. I understood the options as liquidity collection 1) within the driver process and 2) as a separate process.
With the solver + driver co-location in mind I think it would probably be easier to get started if the liquidity collection happens in the driver process (less stuff to set up for solver maintainers).

On the other hand I have the feeling that the competitiveness of the solvers is heavily influenced by their sources
of liquidity. That would mean in the long run we should focus on making the liquidity collection as easily extensible for solver maintainers as possible. That argument would suggest a separate process with some mechanism specifically designed to enable them to extend it easily (ideally in whatever language they want to).

Judging by your checklist I think we share the same views (start quick and dirty to get the ball rolling and then build a new tailor made component for liquidity collection).

@fleupold
Copy link
Contributor

fleupold commented Aug 1, 2022

My concern about keeping the liquidity in the driver process is that if I understand correctly in that case we also need to be opinionated on how the relevant state will be serialized to the solvers (currently as part of the instance.json)? I worry that e.g. serializing full uni v3 state for a bunch of orders over I/O on every batch may be too much overhead (do we have benchmarks on that?).
If solvers end up interfacing with the liquidity service directly (e.g. by invoking a "calcInGivenOut" method via a native binding) it might make sense to have it run as a separate crate.

@nlordell
Copy link
Contributor

nlordell commented Aug 1, 2022

My concern about keeping the liquidity in the driver process

We talked about this quite a bit, and what process the logic runs seems like an implementation detail to me. In fact, if we are worried about I/O and batch sizes, we should stop sending all liquidity to external solvers (and not just UniV3).

@nlordell
Copy link
Contributor

nlordell commented Aug 1, 2022

On the other hand I have the feeling that the competitiveness of the solvers is heavily influenced by their sources of liquidity. That would mean in the long run we should focus on making the liquidity collection as easily extensible for solver maintainers as possible

I think one big aspect of the long-term idea of the liquidity service is to make it so protocol teams can implement new liquidity that would "Just Work™" with solvers that use the liquidity abstraction (like Quasimodo). So having unrelated teams provide logic for new liquidity would be a way to extend solvers without directly modifying them.

Whether or not this runs in the driver, at least to me, isn't super important (as long as its designed in a way that can be easily pulled out in case we want it in a separate process).

@sunce86
Copy link
Contributor Author

sunce86 commented Jan 12, 2024

I failed magnificently at this long time ago. AFAIR it took forever to kill the dependencies. Maybe wait for some better times.

@sunce86 sunce86 closed this as completed Jan 12, 2024
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

No branches or pull requests

4 participants