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

Improve speed of last price endpoint #138

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SebastienGllmt
Copy link
Contributor

This re-implementation of the last price endpoint has the following differences:

  1. It now returns information about the block where the last price change happened so you can know how long ago it was
  2. It's about 100x faster than the old query

Unfortunately there are no tests for this endpoint, so I instead manually tested some queries and it gave the same output. It would be good to test this against a fully sync'd instance and compare the result to know truths (real DEX prices)

@nicarq
Copy link
Contributor

nicarq commented Jun 21, 2023

@gostkin @GrzegorzDrozda can we merge this one if it's working as expected?

@SebastienGllmt
Copy link
Contributor Author

@nicarq it's not working as expected. There was some query we found during testing that returned the wrong result that @GrzegorzDrozda found

@GrzegorzDrozda
Copy link
Contributor

Seems I found why this doesnt work:
https://github.com/dcSpark/carp/blob/improve-last-price-speed/webserver/server/app/models/dex/sqlDexLastPrice.sql#L38-L51

       INNER JOIN "AssetIdPairs" as "Asset1"
        ON
              COALESCE("Dex".asset1_id, -1) = COALESCE("Asset1".asset1_id, -1) 
              AND
              COALESCE("Dex".asset2_id, -1) = COALESCE("Asset1".asset2_id, -1)
              AND
              "Dex".operation = :operation1
        -- Add swap for another direction
        INNER JOIN "AssetIdPairs" as "Asset2"
        ON
              COALESCE("Dex".asset2_id, -1) = COALESCE("Asset2".asset2_id, -1)
              AND
              COALESCE("Dex".asset1_id, -1) = COALESCE("Asset2".asset1_id, -1)
              AND "Dex".operation = :operation2

Here We have statement which will never return any rows when we have two times INNER JOIN and two times the same condition differs only with operation.

Not sure what should be the result - if all rows where asset1_id or asset2_id one of values from given pair we can do something like this:

        INNER JOIN "AssetIdPairs" as "Asset1"
        ON
              (
			  (COALESCE("Dex".asset1_id, -1) = COALESCE("Asset1".asset1_id, -1) AND COALESCE("Dex".asset2_id, -1) = COALESCE("Asset1".asset2_id, -1))
			  OR
              (COALESCE("Dex".asset1_id, -1) = COALESCE("Asset1".asset2_id, -1) AND COALESCE("Dex".asset2_id, -1) = COALESCE("Asset1".asset1_id, -1))
			  )
			  AND
              ("Dex".operation = 1 OR "Dex".operation = 0);

@SebastienGllmt
Copy link
Contributor Author

Concretely, a query that gives the wrong result is

WITH
  "AssetPairs" AS (
      SELECT policy_id1, asset_name1, policy_id2, asset_name2
      FROM
        unnest(
          /*
          Aparrently, we can't make pgtyped understand that these are actually (bytea | NULL)[].
          We will pass in ('', '') instead of (NULL, NULL) for ADA and do the NULL->'' conversion
          below when filtering the assets (see the COALESCE).
          */
          ('{"\\xaf2e27f580f7f08e93190a81f72462f153026d06450924726645891b"}')::bytea[],
          ('{"\\x44524950"}')::bytea[],
          ('{"\\xa0028f350aaabe0545fdcb56b039bfb08e4bb4d8c4d7c3c7d481c235"}')::bytea[],
          ('{"\\x484f534b59"}')::bytea[]
        ) x(policy_id1, asset_name1, policy_id2, asset_name2)
  ),
  "AssetIdPairs" AS (
        SELECT "AssetPairs".*, "Asset1".id as "asset1_id", "Asset2".id as "asset2_id"
        FROM "AssetPairs"
        LEFT JOIN "NativeAsset" as "Asset1" ON "Asset1".policy_id = "AssetPairs".policy_id1 AND "Asset1".asset_name = "AssetPairs".asset_name1
        LEFT JOIN "NativeAsset" as "Asset2" ON "Asset2".policy_id = "AssetPairs".policy_id2 AND "Asset2".asset_name = "AssetPairs".asset_name2
  )
        SELECT
        "Asset1".policy_id1 AS "policy_id1?",
        "Asset1".asset_name1 AS "asset_name1?",
        "Asset2".policy_id2 AS "policy_id2?",
        "Asset2".asset_name2 AS "asset_name2?",
        "Dex".asset1_id,
        "Dex".asset2_id,
        "Dex".amount1,
        "Dex".amount2,
        "Dex".dex,
        "Dex".id,
        "Dex".tx_id
        FROM "Dex"
        INNER JOIN "AssetIdPairs" as "Asset1"
        ON
              COALESCE("Dex".asset1_id, -1) = COALESCE("Asset1".asset1_id, -1) 
              AND
              COALESCE("Dex".asset2_id, -1) = COALESCE("Asset1".asset2_id, -1)
              AND
              "Dex".operation = '1'
        -- Add swap for another direction
        INNER JOIN "AssetIdPairs" as "Asset2"
        ON
              COALESCE("Dex".asset2_id, -1) = COALESCE("Asset2".asset2_id, -1)
              AND
              COALESCE("Dex".asset1_id, -1) = COALESCE("Asset2".asset1_id, -1)
              AND "Dex".operation = '0';

The reason seems to be that, in its current form, the new last price endpoint only works when operation: 2 for both cases (which is true for PriceType.Mean which is the only case that I had tested)

We could try and fix the query (ideal solution), but also we could just disable passing other PriceType values for the last price query since I believe it's all we need for Flint

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