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

Serve Solver List I: Fetch & Store #15

Closed
wants to merge 7 commits into from
Closed

Serve Solver List I: Fetch & Store #15

wants to merge 7 commits into from

Conversation

bh2smith
Copy link
Contributor

@bh2smith bh2smith commented Jul 8, 2022

As part of an effort to make our solver list universally accessible via our own API service. This is the service that would periodically fetch the most recent solver list and store it as a json file.

As a second step, we would add a route to the API (here in this project) which serves the JSON content.

Pending the existence of view_solverstable for Gnosis Chain (PR - here) we would like to store an additional field "chainID" in the json file and merge the solver lists from all networks. Alternatively we could store two separate files. Not sure which is preferred (cc @alfetopito)

@bh2smith bh2smith requested a review from a team July 8, 2022 13:36
@bh2smith bh2smith marked this pull request as draft July 11, 2022 08:53
@bh2smith bh2smith changed the title Store Solver List Serve Solver List I: Fetch & Store Jul 11, 2022
Copy link

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

Minor comments regarding the code below.

Regarding this comment:

Pending the existence of view_solverstable for Gnosis Chain (PR - duneanalytics/spellbook#1268) we would like to store an additional field "chainID" in the json file and merge the solver lists from all networks. Alternatively we could store two separate files. Not sure which is preferred (cc @alfetopito)

I don't really mind how the data is stored, but I would rather be able to consume it per network rather than all in the same response with a chainId parameter, to be consistent with every other API method, e.g.:

  • Mainnet: api.cow.fi/mainnet/api/v1/...
  • Gchain: api.cow.fi/xdai/api/v1/...

Then, to provide this behaviour with the minimum amount of transformation required on the API side, it might be worth to store them as a separated file.

dune_api_scripts/store/solver_list.py Outdated Show resolved Hide resolved
dune_api_scripts/store/solver_list.py Outdated Show resolved Hide resolved
@josojo
Copy link
Contributor

josojo commented Jul 12, 2022

Ben, the dune v2 promised to have a direct API. Is that coming or not? Do you know roughly any timeline?
If it is coming soon (likely not), we could direct the request form the front-end directly to dune without middleware.

Otherwise, the PR looks good

@bh2smith bh2smith marked this pull request as ready for review July 12, 2022 08:38
@bh2smith
Copy link
Contributor Author

Ben, the dune v2 promised to have a direct API. Is that coming or not?

Still waiting on this. I will ping them again and ask. But for now I think this is in good shape.

Copy link
Contributor

@josojo josojo left a comment

Choose a reason for hiding this comment

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

LGTM

dune_api_scripts/store/solver_list.py Outdated Show resolved Hide resolved
@bh2smith
Copy link
Contributor Author

Update - Now that we have the Alpha API key for Dune, we may not need this (since we can query directly through their API). We may want to keep this as a backup - it also offers the ability to refresh (which is not yet available with Dune's supported API).

@bh2smith
Copy link
Contributor Author

Not sure exactly what the status is here. @alfetopito is this something you'd like to see go live as is for your purposes?

@alfetopito
Copy link

Not sure exactly what the status is here. @alfetopito is this something you'd like to see go live as is for your purposes?

We are closing the solvers page design, and implementation will start soon.
Still, I don't really care where the data comes from.
If you say we can replace this with Dune API, go for it.

What's the earliest that this info can be provided, be it via this or Dune API?

@bh2smith
Copy link
Contributor Author

Well we could merge this and then expose and endpoint in our existing dune bridge api. I haven't exactly worked with the routing and serving part of this project, but I would imagine it could be ready in a staging environment in less than a week from here.

@alfetopito
Copy link

it could be ready in a staging environment in less than a week from here.

That would be great!

@bh2smith
Copy link
Contributor Author

bh2smith commented Aug 2, 2022

Hey, @alfetopito - we chatted about this today in the sync and I think this approach might be soon outdated and maybe a bit of over engineering. I recall you suggested that you could parse the file from dune abstractions. Do you think you could set yourself up for this and in the meantime I can get make the solver list available via the DUNE V2 api

Who knows, maybe I can whip something up really quick.

Cc @josojo

@bh2smith
Copy link
Contributor Author

bh2smith commented Aug 2, 2022

Actually, for example (you already have a Dune API key - for beta testing) and this query returns the solvers (by name) based on the abstractions view. I can lift the condition of being active, although I suspect this is what you want...

Mainnet: https://dune.com/queries/575593
Gnosis Chain: https://dune.com/queries/1129021

@alfetopito
Copy link

Given that we might soon have a solution coming from Dune API as you mention, I'm fine using a hardcoded file locally in the Explorer until it is exposed from the backend.

Those 2 queries are perfect for creating this hardcoded file for now

@bh2smith
Copy link
Contributor Author

bh2smith commented Aug 2, 2022

@alfetopito - you don't need to hard code the content (you can use the Dune API that we gave you for the volume and num traders to fetch this from their Beta V2 Engine API).

@alfetopito
Copy link

@alfetopito - you don't need to hard code the content (you can use the Dune API that we gave you for the volume and num traders to fetch this from their Beta V2 Engine API).

But if we consume that directly on the frontend, the API key will be exposed, unlike cow.fi where it's not since it's rendered server side.

@bh2smith
Copy link
Contributor Author

bh2smith commented Aug 2, 2022

Understood, then raw file parsing works for now. Sorry for the misunderstanding.

@bh2smith bh2smith closed this Aug 2, 2022
@bh2smith bh2smith deleted the store-solvers branch August 2, 2022 10:15
@github-actions github-actions bot locked and limited conversation to collaborators Aug 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants