feat: add redis caching support for getBalance #881

james-a-morris · 2023-10-25T21:27:04Z

This PR adds the ability for redis to be employed by the vercel edge functions.

vercel · 2023-10-25T21:27:11Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
frontend-v2	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 19, 2023 2:28pm
goerli-frontend-v2	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 19, 2023 2:28pm

james-a-morris · 2023-10-25T21:27:27Z

api/_imports.ts

This is required to polyfill fetch at a global level.

james-a-morris · 2023-10-25T21:28:03Z

api/_utils.ts

-  );
-  // Return the balance
-  return BigNumber.from(response.data.balance);
+  const key = `balance_${chainId}-${account}-${token}`.toLowerCase();


Our keys are in checksum case, but I don't think it would hurt to make the key case insensitive.

james-a-morris · 2023-10-25T21:28:36Z

api/account-balance.ts

We no longer need this endpoint.

Corollary - we can get rid of the cached coingecko endpoint as well.

james-a-morris · 2023-10-25T21:29:06Z

api/limits.ts

We gain the ability to go back to a single function interface with getBalance

james-a-morris · 2023-10-25T21:29:38Z

api/_utils.ts

@@ -35,6 +35,7 @@ import {
  BLOCK_TAG_LAG,
 } from "./_constants";
 import { PoolStateResult } from "./_types";
+import { kv } from "@vercel/kv";


The setup/destroy of this redis is fully handled by vercel. It is federated in SFO, TX, and Singapore currently.

dohaki

Left a few comments and questions

dohaki · 2023-10-26T07:45:33Z

api/_imports.ts

+import * as fetch from "node-fetch";
+
+if (!globalThis.fetch) {
+  (globalThis as any).fetch = fetch;


Just curious: Why do we need to polyfill this? Does the SDK depend on fetch instead of axios?

Yep - unfortunately @vercel/kv relies on fetch. There are pros/cons to trying to adapt, but the major pro here is that Vercel handles all the setup/teardown of these connections when we use their official package.

dohaki · 2023-10-26T07:49:13Z

api/_utils.ts

@@ -1,3 +1,4 @@
+import "./_imports";


NIT: The file name _imports feels not very descriptive. Maybe renaming to _polyfills is better

yea agreed we should name it to something way more intuitive

dohaki · 2023-10-26T08:00:04Z

api/_utils.ts

- * @returns A promise that resolves to the BigNumber of the balance
- */
-export const getCachedTokenBalance = async (
+export const getBalance = async (


NIT: It feels somewhat unintuitive to me that this function getBalance returns cached balances per default. Maybe it makes sense to rename it back to getCachedTokenBalance?

dohaki · 2023-10-26T08:02:46Z

api/_utils.ts

+      BLOCK_TAG_LAG // We should do this for consistency
+    );
+    await kv.set(key, balance.toString(), {
+      ex: 60 * 5, // 5 minutes


Is it safe to set the expiry time to 5 minutes? We might also make this a parameter.

This is currently what we have in our balance endpoint

dohaki · 2023-10-26T08:03:21Z

api/suggested-fees.ts

+      const isRecipientAContract =
+        (await sdk.utils.isContractDeployedToAddress(
+          recipientAddress,
+          provider
+        )) || true;


Hmm doesn't this always return true? Maybe a leftover from manual testing?

This was for testing - it's since resolved.

nicholaspai · 2023-10-25T21:47:28Z

api/_utils.ts

+      getProvider(Number(chainId)),
+      BLOCK_TAG_LAG // We should do this for consistency
+    );
+    await kv.set(key, balance.toString(), {


Should we try catch this?

Good call - we should be able to continue even if the cache mechanism fails.

api/suggested-fees.ts

nicholaspai · 2023-10-30T15:44:14Z

api/_utils.ts

@@ -1,3 +1,4 @@
+import "./_imports";


yea agreed we should name it to something way more intuitive

nicholaspai · 2023-10-30T15:44:22Z

api/_utils.ts

- * @returns A promise that resolves to the BigNumber of the balance
- */
-export const getCachedTokenBalance = async (
+export const getBalance = async (


dohaki

Nice 👍

dohaki · 2024-09-13T09:18:12Z

Closing as this was part of the performance improvements PR #1194

james-a-morris requested review from nicholaspai, dohaki and amateima October 25, 2023 21:27

james-a-morris commented Oct 25, 2023

View reviewed changes

vercel bot deployed to Preview – goerli-frontend-v2 October 25, 2023 21:34 View deployment

vercel bot deployed to Preview – frontend-v2 October 25, 2023 21:35 View deployment

vercel bot deployed to Preview – goerli-frontend-v2 October 25, 2023 22:20 View deployment

vercel bot deployed to Preview – frontend-v2 October 25, 2023 22:21 View deployment

dohaki reviewed Oct 26, 2023

View reviewed changes

feat: add redis caching support for getBalance

7d49221

james-a-morris force-pushed the james/add-redis branch from f7a8b88 to 7d49221 Compare December 14, 2023 20:20

james-a-morris changed the base branch from james/acx-1571-estimate-fill-gas-costs-with-non-empty-message-fields to master December 14, 2023 20:20

vercel bot had a problem deploying to Preview – frontend-v2 December 14, 2023 20:25 Failure

vercel bot had a problem deploying to Preview – goerli-frontend-v2 December 14, 2023 20:25 Failure

james-a-morris added 4 commits December 14, 2023 15:34

fix: merge issues

407b770

improve: change name

087a0ec

chore: change name

5701faa

improve: bump lock

83bd8b8

vercel bot deployed to Preview – goerli-frontend-v2 December 14, 2023 21:01 View deployment

improve: bump nodefetch

6050bc4

james-a-morris requested a review from dohaki December 14, 2023 21:03

vercel bot had a problem deploying to Preview – frontend-v2 December 14, 2023 21:10 Failure

vercel bot deployed to Preview – goerli-frontend-v2 December 14, 2023 21:11 View deployment

james-a-morris added 3 commits December 15, 2023 15:54

fix: resolve VC

dcf535b

fix: resolve VC

2661210

docs: add comments

cdd79b3

vercel bot deployed to Preview – goerli-frontend-v2 December 15, 2023 21:03 View deployment

vercel bot deployed to Preview – frontend-v2 December 15, 2023 21:11 View deployment

nicholaspai approved these changes Dec 18, 2023

View reviewed changes

improve: catch error if cache balance fails

4b9ebc1

vercel bot deployed to Preview – goerli-frontend-v2 December 19, 2023 14:28 View deployment

vercel bot deployed to Preview – frontend-v2 December 19, 2023 14:28 View deployment

dohaki approved these changes Dec 19, 2023

View reviewed changes

dohaki closed this Sep 13, 2024

dohaki deleted the james/add-redis branch September 13, 2024 09:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add redis caching support for getBalance #881

feat: add redis caching support for getBalance #881

james-a-morris commented Oct 25, 2023

vercel bot commented Oct 25, 2023 •

edited

Loading

james-a-morris Oct 25, 2023

james-a-morris Oct 25, 2023

james-a-morris Oct 25, 2023

james-a-morris Oct 25, 2023

james-a-morris Oct 25, 2023 •

edited

Loading

dohaki left a comment

dohaki Oct 26, 2023

james-a-morris Dec 14, 2023

dohaki Oct 26, 2023

nicholaspai Oct 30, 2023

james-a-morris Dec 14, 2023

dohaki Oct 26, 2023

nicholaspai Oct 30, 2023

james-a-morris Dec 14, 2023

dohaki Oct 26, 2023

james-a-morris Dec 18, 2023

dohaki Oct 26, 2023

james-a-morris Dec 14, 2023

nicholaspai Oct 25, 2023

james-a-morris Dec 19, 2023

nicholaspai Oct 30, 2023

nicholaspai Oct 30, 2023

dohaki left a comment

dohaki commented Sep 13, 2024

feat: add redis caching support for getBalance #881

feat: add redis caching support for getBalance #881

Conversation

james-a-morris commented Oct 25, 2023

vercel bot commented Oct 25, 2023 • 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

james-a-morris Oct 25, 2023 • edited Loading

Choose a reason for hiding this comment

dohaki 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dohaki left a comment

Choose a reason for hiding this comment

dohaki commented Sep 13, 2024

vercel bot commented Oct 25, 2023 •

edited

Loading

james-a-morris Oct 25, 2023 •

edited

Loading