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

[SQS-Routing] API to force quote over a given route #132

Open
p0mvn opened this issue Mar 18, 2024 · 7 comments
Open

[SQS-Routing] API to force quote over a given route #132

p0mvn opened this issue Mar 18, 2024 · 7 comments
Assignees

Comments

@p0mvn
Copy link
Member

p0mvn commented Mar 18, 2024

We have frequent requests claiming that another route might be more performant without a streamlined way to confirm it.

To make the discussions more productive and to avoid investigating the details of route differences, we should have a Swagger API that allows to force quote from token A to token B over given pool IDs / routes.

Acceptance Criteria

  • Add an API to achieve this
  • Test it
  • Expose via swagger
  • Deploy to stage
  • Deploy to production
  • Add add alerts if applicable
  • Add Grafana dashboards if applicable
  • Backwards compatibility with custom-direct-quote . Simply extending it to support more pools
@p0mvn p0mvn changed the title API to force quote over a given route [SQS-Routing] API to force quote over a given route Apr 17, 2024
@p0mvn p0mvn added the Onboarding-Calvin label Apr 17, 2024 — with Linear
@linear linear bot assigned p0mvn Apr 17, 2024
@p0mvn p0mvn removed their assignment Apr 17, 2024
@p0mvn
Copy link
Member Author

p0mvn commented May 30, 2024

@deividaspetraitis would you be interested in this one?

Instead of creating a new endpoint, we can modify the existing one:

e.GET(formatRouterResource("/custom-direct-quote"), handler.GetDirectCustomQuote)

However, we would need to ensure backwards compatibility

@deividaspetraitis
Copy link
Collaborator

Sure! Please feel free to assign me and let me take a close look once I'm back to my PC, I think I might have a few questions.

@deividaspetraitis
Copy link
Collaborator

hey @p0mvn, I just wanted to clarify a few details to make sure I understand task and scope correctly.

As per my understanding the idea is to have a Swagger API that allows to force quote from token A to token B over given pool IDs / routes, which is basically the functionally of router/custom-direct-quote endpoint as you have pointed. Is my understanding correct that it's basically following that needs to be implemented ( within SQS code base ):

  1. Extend router/custom-direct-quote endpoint to support more pools ( with backwards compatibility ).
  2. Add relevant annotations for router/custom-direct-quote so it's available in Swagger API UI ( here ).

In case I am on the right track with above, I was wondering what specific new pools do we need add to support? Is that about Custom CosmWasm Pools? I am sorry for my ignorance. :)

@p0mvn
Copy link
Member Author

p0mvn commented May 30, 2024

You're pretty much spot on here.

Please note that, currently, we support poolID query param.

Note, that this poolID allows forcing a swap over a specific pool. If the given pool ID does not contain the token in or out denoms, the query would fail

I suggest that we add poolIDs that takes a comma-separated list of IDs and constructs/forces routes over them but keep poolID for backwards compatibility.

In terms of how this would work, consider an example where we may want to force an OSMO -> USDC swap over pool IDs 1, 5, 7.

Assume pool 1 contains tokens OSMO/ATOM, pool 5 ATOM/KUJI and 7 KUJI/USDC.

As a result, we can construct and force a route over such pool.

If, on the other hand, pool 5 was ATOM/AKT, the logic would fail since we cannot get to the final token out by going through this pool.

I suggest implementing a new method similar to this that would iterate over the given pools and try to use GetCustomDirectQuote under the hood while traversing.

Please make sure to add table-driven unit tests. Check out the routertesting package on how the tests are setup.

Once the core logic is complete, exposing swagger is another component that is in scope of this work.

I don't think you need to worry about the custom CosmWasm pools for the context of this task but lmk if you have any specific questions about them.

If something is missing context/docs, please don't hesitate to ping

@deividaspetraitis
Copy link
Collaborator

deividaspetraitis commented May 31, 2024

Thank you for detailed explanation, it's clear for now and I'm on it then!

@deividaspetraitis
Copy link
Collaborator

Hey @p0mvn, Just a few more clarifying questions, to make sure I am on the right track.

In terms of how this would work, consider an example where we may want to force an OSMO -> USDC swap over pool IDs 1, 5, 7.
Assume pool 1 contains tokens OSMO/ATOM, pool 5 ATOM/KUJI and 7 KUJI/USDC. > As a result, we can construct and force a route over such pool.

I will keep building on your example, so if the client provides pool IDs of 1,5,7 and wants to swap OSMO to USDC it is within the scope of this task to figure out whether pool 1 contains OSMO/ATOM, pool 5 ATOM/KUJI and 7 KUJI/USDC ( or any other permutation that would lead to OSMO -> USDC ), is that right?

If what was said above is a case, I was wondering, is there already any algorithm in place for swapping tokens through given specific route? As per my understanding this functionality just calculates a quote, maybe we can reuse some of the logic from actual implementation if any.

How final output to the client should look like? Is it something like following ( basically just adding more entries to the pools arrray ):

{
  "amount_in": {
    "denom": "uosmo",
    "amount": "1000000"
  },
  "amount_out": "98645",
  "route": [
    {
      "pools": [
        {
          "id": 1,
          "type": 2,
          "balances": [],
          "spread_factor": "0.000500000000000000",
          "token_out_denom": "ATOM",
          "taker_fee": "0.001000000000000000"
        },
        {
          "id": 5,
          "type": 2,
          "balances": [],
          "spread_factor": "0.000500000000000000",
          "token_out_denom": "KUJI",
          "taker_fee": "0.001000000000000000"
        },
        {
          "id": 7,
          "type": 2,
          "balances": [],
          "spread_factor": "0.000500000000000000",
          "token_out_denom": "USDC",
          "taker_fee": "0.001000000000000000"
        }
      ],
      "has-cw-pool": false,
      "out_amount": "98645",
      "in_amount": "1000000"
    }
  ],
  "effective_fee": "0.001500000000000000",
  "price_impact": "-0.000504853187504351",
  "in_base_out_quote_spot_price": "0.098793620017714781"
}

@p0mvn
Copy link
Member Author

p0mvn commented Jun 3, 2024

Hey @deividaspetraitis .

I will keep building on your example, so if the client provides pool IDs of 1,5,7 and wants to swap OSMO to USDC it is within the scope of this task to figure out whether pool 1 contains OSMO/ATOM, pool 5 ATOM/KUJI and 7 KUJI/USDC ( or any other permutation that would lead to OSMO -> USDC ), is that right?

This should be abstracted away by the existing logic that we should be able to reuse.

We have this function that can be utilized.

It does the checks for token presence in the pool, simplifying the scope of work needed to achieve the final result.

I recommend the following change: make current GetCustomDirectQuoteMultiPool that takes a pool ID slice as an input and calls the GetCustomDirectQutote for each pool while updating the local tokenIn and tokenOutDenom at each iteration.

The output should be of the type domain.Quote. We should be able to modify the output of GetCustomDirectQuote during each iteration to construct the final domain.Quote result.

For testing, I suggest referencing this example and try running it with a debugger. Note the mainnetState thing that mocks out the production environment from json files located here

Let me know if you have any follow up questions

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

2 participants