Candles v2 (time windows) #128

grod220 · 2024-11-05T19:16:33Z

Adds new db query to get window-based candles from new db
Removes old candle route + dependency on old events-based db
Assortment of refactoring
Note the design is not quite like the figma. Follow up UI work will be needed.

compressed.mov

grod220 · 2024-11-05T19:18:45Z

src/pages/trade/api/book.ts


-export const useBook = (symbol1: string | undefined, symbol2: string | undefined) => {
+export const useBook = () => {
+  const { baseSymbol, quoteSymbol } = usePathSymbols();


Realizing a few hooks are relying upon async data fetching for the path symbols. This is not necessary because they are synchronously available. So created a new usePathSymbols(); for that.

grod220 · 2024-11-05T19:19:40Z

src/pages/trade/api/book.ts

+      const jsonRes = (await res.json()) as RouteBookApiResponse;
+      if ('error' in jsonRes) {
+        throw new Error(jsonRes.error);
+      }


Because nextjs can return an error field, we need to parse it and re-throw if we see it

grod220 · 2024-11-05T19:21:15Z

src/pages/trade/redirect.tsx

+export const RedirectToPair = () => {
+  const { data, isLoading, error } = useQuery({
+    queryKey: ['redirectSymbols'],
+    retry: 1,
+    queryFn: redirectSymbolsQueryFn,
+  });
+
+  if (error) {
+    return <div className='text-red-600'>{String(error)}</div>;
+  } else if (isLoading || !data) {
+    return <div className='text-white'>Loading...</div>;
+  } else {
+    redirect(`/trade/${data.baseAsset}/${data.quoteAsset}`);
+  }
+};


Was finding it a bit annoying that we had a hard-coded pair here. This causes issues when going back and forth between testnet/mainnet. When ahead and created a hook to pull the highest priority pair and show that as the default.

Love the idea, looks like this'd resolve #92 too. However when testing locally I get a pretty bizarre error that looks like the assets are being interpolated incorrectly:

Locally I was using commit c507d1b and had updated my env vars to point to a pindexer db, rather than the old cometbft db.

Definitely a bug in the implementation. I should have been filtering out delegation tokens, fixed! Also, added some sample priority scores: prax-wallet/registry#109.

Nice, confirming resolved on my end:

Looks like the styling is sub-optimal but we can handle that in time.

Indeed, but we have an issue for that: #107

grod220 · 2024-11-05T19:23:19Z

src/shared/api/indexer/connector.tsx

-// TODO: Delete when possible, this is deprecated
-export class IndexerQuerier {
-  private pool: Pool;


Bye bye old db client

grod220 · 2024-11-05T19:23:52Z

src/shared/api/server/book/index.ts

@@ -72,10 +74,10 @@ export async function GET(req: NextRequest): Promise<NextResponse<AllResponses>>
    output: baseAssetMetadata.penumbraAssetId,
  });

-  const simQuerier = new SimulationQuerier({ grpcEndpoint });
+  const client = createClient(grpcEndpoint, SimulationService);


Finding the query classes to be unnecessary, we can just create a client in line when we want to query

grod220 · 2024-11-05T19:25:39Z

src/shared/api/server/candles/utils.ts

Nearly this whole file was copied over from the previous implementation

grod220 · 2024-11-05T19:26:37Z

src/shared/database/index.ts

+  async candles(
+    baseAsset: AssetId,
+    quoteAsset: AssetId,
+    start: Date,
+    end: Date,
+    window: DurationWindow,
+  ) {
+    return this.db
+      .selectFrom('dex_ex_price_charts')
+      .innerJoin('dex_ex_candlesticks', 'candlestick_id', 'dex_ex_candlesticks.id')
+      .select(['start_time', 'open', 'close', 'low', 'high', 'swap_volume', 'direct_volume'])
+      .where('the_window', '=', window)
+      .where('asset_start', '=', Buffer.from(baseAsset.inner))
+      .where('asset_end', '=', Buffer.from(quoteAsset.inner))
+      .where('start_time', '<', start)
+      .where('start_time', '>=', end)
+      .execute();


@cronokirby

grod220 · 2024-11-05T19:27:41Z

@conorsch after this is merged, the nextjs app will no longer have a dependency on the old database and only on the new one fyi.

conorsch

Encountered one large bug related to the initial route redirect, see screenshot shared above. Standing by to assist with backend changes once we're happy with the diff.

conorsch · 2024-11-05T22:57:08Z

src/pages/trade/api/candles.tsx

+import { OhlcData } from 'lightweight-charts';
+import { DurationWindow } from '@/shared/database/schema.ts';
+
+const DEX_ENABLED_DATE = '2024-08-01';


The DEX_ENABLED_DATE const is applicable only to the penumbra-1 chain; in general I'd prefer to avoid assumptions about chain id in any of the frontend code bases. Practically speaking, I don't expect this specific assumption to have adverse affects for e.g. testnet chains, but at the same time, are we really gaining anything from it? The genesis date is not far behind the dex-enable date for penumbra-1, and as time goes on, the comparative savings on the query will be minuscule.

Is the motivation for this cut-off date to make queries more efficient, or does the cut-off date avoid the need for error handling or similar?

Good catch. We should make this chain agnostic. Given we have such little data, it's hard for me to determine what is the upper bound for this query. Aka, should we only be pulling data from the last year? Alas, I'm going to remove the start/end date logic and just pull whatever candlestick data is in the database. Think it's worthwhile revisiting this topic in a year.

conorsch · 2024-11-05T22:59:24Z

src/pages/trade/redirect.tsx

+export const RedirectToPair = () => {
+  const { data, isLoading, error } = useQuery({
+    queryKey: ['redirectSymbols'],
+    retry: 1,
+    queryFn: redirectSymbolsQueryFn,
+  });
+
+  if (error) {
+    return <div className='text-red-600'>{String(error)}</div>;
+  } else if (isLoading || !data) {
+    return <div className='text-white'>Loading...</div>;
+  } else {
+    redirect(`/trade/${data.baseAsset}/${data.quoteAsset}`);
+  }
+};


Love the idea, looks like this'd resolve #92 too. However when testing locally I get a pretty bizarre error that looks like the assets are being interpolated incorrectly:

Locally I was using commit c507d1b and had updated my env vars to point to a pindexer db, rather than the old cometbft db.

grod220 · 2024-11-06T17:52:52Z

(@conorsch OOO today, but gave the approval ✅ offline)

grod220 added 4 commits November 5, 2024 07:23

Query Pindexer db for candles

6c7db97

Add symbol hook

7f50851

working candles w/ buttons

8102bd1

Remove old candle api route

04c2020

grod220 commented Nov 5, 2024

View reviewed changes

remove more

c507d1b

grod220 force-pushed the candle-refactor branch from 1b541d1 to c507d1b Compare November 5, 2024 19:24

grod220 commented Nov 5, 2024

View reviewed changes

src/shared/api/server/candles/utils.ts Outdated

Copy link

Contributor Author

grod220 Nov 5, 2024

Choose a reason for hiding this comment

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

Nearly this whole file was copied over from the previous implementation

grod220 commented Nov 5, 2024

View reviewed changes

grod220 requested a review from a team November 5, 2024 19:27

conorsch requested changes Nov 5, 2024

View reviewed changes

docs: update database reqs to pindexer

b1b4336

grod220 force-pushed the candle-refactor branch from 38fb560 to bffa031 Compare November 6, 2024 00:25

Review updates

9c5c982

grod220 force-pushed the candle-refactor branch from bffa031 to 9c5c982 Compare November 6, 2024 00:27

grod220 requested a review from conorsch November 6, 2024 00:28

grod220 merged commit b245695 into main Nov 6, 2024
2 checks passed

grod220 deleted the candle-refactor branch November 6, 2024 17:52

conorsch mentioned this pull request Nov 13, 2024

db: app depends on non-existent dex_ex_summary table #126

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Candles v2 (time windows) #128

Candles v2 (time windows) #128

grod220 commented Nov 5, 2024 •

edited

Loading

grod220 Nov 5, 2024

grod220 Nov 5, 2024

grod220 Nov 5, 2024

conorsch Nov 5, 2024

grod220 Nov 6, 2024

conorsch Nov 6, 2024

grod220 Nov 6, 2024

grod220 Nov 5, 2024

grod220 Nov 5, 2024

grod220 Nov 5, 2024

grod220 Nov 5, 2024

grod220 commented Nov 5, 2024

conorsch left a comment

conorsch Nov 5, 2024

grod220 Nov 6, 2024

conorsch Nov 5, 2024

grod220 commented Nov 6, 2024

Candles v2 (time windows) #128

Candles v2 (time windows) #128

Conversation

grod220 commented Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grod220 commented Nov 5, 2024

conorsch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grod220 commented Nov 6, 2024

grod220 commented Nov 5, 2024 •

edited

Loading