feat(perf): improvements (#852)

* feat(perf): improvements

* refactor(score): improve readability

* refactor(getBlockNum): simplify logic

* test(utils): change tests for modified files

.husky/pre-commit

@@ -2,3 +2,4 @@
 . "$(dirname -- "$0")/_/husky.sh"
 
 yarn lint-staged
+yarn test:unit

jest.config.unit.ts

@@ -13,8 +13,7 @@ export default {
   preset: 'ts-jest',
   testEnvironment: 'jest-environment-node-single-context',
   moduleFileExtensions: ['js', 'ts'],
-  testMatch: ['<rootDir>/test/unit/**/?(*.)+(spec|test).(ts|js)'],
-  testPathIgnorePatterns: ['dist/', 'node_modules/'],
+  testMatch: ['<rootDir>/src/**/?(*.)+(spec|test).[jt]s?(x)'],
   verbose: true,
   globals: {
     'ts-jest': {

src/methods.test.ts

@@ -0,0 +1,165 @@
+import { getVp } from './methods';
+import snapshot from '@snapshot-labs/strategies';
+import { getBlockNum } from './utils';
+import * as redisModule from './redis';
+
+jest.mock('@snapshot-labs/strategies');
+jest.mock('./utils');
+jest.mock('./redis', () => ({
+  __esModule: true,
+  default: {
+    hGetAll: jest.fn(),
+    multi: jest.fn(() => ({
+      hSet: jest.fn(),
+      exec: jest.fn()
+    })),
+    connect: jest.fn(),
+    on: jest.fn()
+  }
+}));
+
+const mockRedis = redisModule.default;
+
+describe('getVp function', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('should set snapshot to "latest" if it is not a number', async () => {
+    const expectedSnapshotNum = 'latest';
+    const params = {
+      address: '0x123',
+      network: '1',
+      strategies: [],
+      snapshot: 'not-a-number',
+      space: 'testSpace'
+    };
+
+    (snapshot.utils.getVp as jest.Mock).mockResolvedValue({ vp_state: 'pending', vp: 100 });
+    (getBlockNum as jest.Mock).mockResolvedValue(expectedSnapshotNum);
+
+    // @ts-expect-error
+    await getVp(params);
+
+    expect(getBlockNum).toHaveBeenCalledWith(expectedSnapshotNum, params.network);
+    expect(params.snapshot).toBe('latest');
+  });
+
+  it('should call getBlockNum if snapshot is not "latest"', async () => {
+    const params = {
+      address: '0x123',
+      network: '1',
+      strategies: [],
+      snapshot: 1000,
+      space: 'testSpace'
+    };
+
+    (getBlockNum as jest.Mock).mockResolvedValue(900);
+
+    await getVp(params);
+
+    expect(getBlockNum).toHaveBeenCalledWith(1000, '1');
+  });
+
+  it('should throw an error for disabled networks or spaces', async () => {
+    const params = {
+      address: '0x123',
+      network: '1319',
+      strategies: [],
+      snapshot: 12345,
+      space: 'testSpace'
+    };
+
+    try {
+      await getVp(params);
+      fail('Expected getVp to throw an error'); // This will fail the test if no error is thrown
+    } catch (error) {
+      expect(error).toMatch('something wrong with the strategies');
+    }
+  });
+
+  it('should call snapshot.utils.getVp with correct parameters', async () => {
+    const params = {
+      address: '0x123',
+      network: '1',
+      strategies: [],
+      snapshot: 12345,
+      space: 'testSpace'
+    };
+
+    (snapshot.utils.getVp as jest.Mock).mockResolvedValue({ vp_state: 'pending', vp: 100 });
+    (getBlockNum as jest.Mock).mockResolvedValue(params.snapshot);
+
+    await getVp(params);
+
+    expect(snapshot.utils.getVp).toHaveBeenCalledWith(
+      '0x123',
+      '1',
+      [],
+      12345,
+      'testSpace',
+      undefined
+    );
+  });
+
+  it('should use cache if conditions are met', async () => {
+    const params = {
+      address: '0x123',
+      network: '1',
+      strategies: [],
+      snapshot: 1000,
+      space: 'testSpace'
+    };
+
+    (getBlockNum as jest.Mock).mockResolvedValue(900);
+    mockRedis.hGetAll.mockResolvedValue({ vp_state: 'pending', vp: 100, vp_by_strategy: '{}' });
+
+    const result = await getVp(params);
+
+    expect(mockRedis.hGetAll).toHaveBeenCalled();
+    expect(result.cache).toBe(true);
+    expect(result.result.vp).toBe(100);
+  });
+
+  it('should return correct result values', async () => {
+    const params = {
+      address: '0x123',
+      network: '1',
+      strategies: [],
+      snapshot: 1000,
+      space: 'testSpace'
+    };
+
+    mockRedis.hGetAll.mockResolvedValue();
+    (snapshot.utils.getVp as jest.Mock).mockResolvedValue({
+      vp_state: 'pending',
+      vp: 100,
+      vp_by_strategy: '{}'
+    });
+    (getBlockNum as jest.Mock).mockResolvedValue(params.snapshot);
+
+    const result = await getVp(params);
+
+    expect(result.cache).toBe(false);
+    expect(result.result.vp).toBe(100);
+    expect(result.result.vp_state).toBe('pending');
+  });
+
+  it('should handle the delegation parameter correctly', async () => {
+    const params = {
+      address: '0x123',
+      network: '1',
+      strategies: [],
+      snapshot: 1000,
+      space: 'testSpace',
+      delegation: true
+    };
+
+    (snapshot.utils.getVp as jest.Mock).mockResolvedValue({ vp_state: 'pending', vp: 100 });
+    (getBlockNum as jest.Mock).mockResolvedValue(params.snapshot);
+
+    await getVp(params);
+
+    expect(snapshot.utils.getVp).toHaveBeenCalledWith('0x123', '1', [], 1000, 'testSpace', true);
+  });
+});

src/methods.ts

@@ -21,31 +21,31 @@ interface ValidateRequestParams {
   params: any;
 }
 
+const disabledNetworks = ['1319'];
 const disableCachingForSpaces = ['magicappstore.eth', 'moonbeam-foundation.eth'];
 
 export async function getVp(params: GetVpRequestParams) {
   if (typeof params.snapshot !== 'number') params.snapshot = 'latest';
 
-  if (params.snapshot !== 'latest') {
-    const currentBlockNum = await getBlockNum(params.snapshot, params.network);
-    params.snapshot = currentBlockNum < params.snapshot ? 'latest' : params.snapshot;
-  }
+  params.snapshot = await getBlockNum(params.snapshot, params.network);
 
   const key = sha256(JSON.stringify(params));
   const useCache =
     redis && params.snapshot !== 'latest' && !disableCachingForSpaces.includes(params.space);
   if (useCache) {
     const cache = await redis.hGetAll(`vp:${key}`);
+
     if (cache?.vp_state) {
       cache.vp = parseFloat(cache.vp);
+
       cache.vp_by_strategy = JSON.parse(cache.vp_by_strategy);
       return { result: cache, cache: true };
     }
   }
 
-  if (['1319'].includes(params.network) || disabled.includes(params.space))
+  if (disabledNetworks.includes(params.network) || disabled.includes(params.space)) {
     throw 'something wrong with the strategies';
-
+  }
   const result = await snapshot.utils.getVp(
     params.address,
     params.network,

src/rpc.ts

@@ -131,19 +131,13 @@ router.post('/api/scores', async (req, res) => {
 
   let result;
   try {
-    result = await scores(
-      {
-        requestId,
-        strategyNames
-      },
-      {
-        space,
-        network,
-        snapshot,
-        strategies,
-        addresses
-      }
-    );
+    result = await scores({
+      space,
+      network,
+      snapshot,
+      strategies,
+      addresses
+    });
   } catch (e: any) {
     capture(e, { params, strategies });
     // @ts-ignore

test/unit/scores.test.ts → src/scores.test.ts

@@ -1,10 +1,10 @@
-import scores from '../../src/scores';
-import { get, set } from '../../src/aws';
-import { getBlockNum, sha256 } from '../../src/utils';
+import scores from './scores';
+import { get, set } from './aws';
+import { getBlockNum, sha256 } from './utils';
 import snapshot from '@snapshot-labs/strategies';
 
-jest.mock('../../src/utils');
-jest.mock('../../src/aws');
+jest.mock('./utils');
+jest.mock('./aws');
 jest.mock('@snapshot-labs/strategies');
 
 describe('scores function', () => {
@@ -25,8 +25,8 @@ describe('scores function', () => {
 
   it('should deduplicate requests', async () => {
     (snapshot.utils.getScoresDirect as jest.Mock).mockResolvedValue(mockScores);
-    const firstCall = scores(null, mockArgs);
-    const secondCall = scores(null, mockArgs);
+    const firstCall = scores(mockArgs);
+    const secondCall = scores(mockArgs);
 
     const firstResult = await firstCall;
     const secondResult = await secondCall;
@@ -39,7 +39,7 @@ describe('scores function', () => {
     process.env.AWS_REGION = 'us-west-1';
     (get as jest.Mock).mockResolvedValue(mockScores);
 
-    const result = await scores(null, mockArgs);
+    const result = await scores(mockArgs);
 
     expect(result).toEqual({
       cache: true,
@@ -54,16 +54,17 @@ describe('scores function', () => {
     (get as jest.Mock).mockResolvedValue(null); // Not in cache
     (snapshot.utils.getScoresDirect as jest.Mock).mockResolvedValue(mockScores);
 
-    await scores(null, mockArgs);
+    await scores(mockArgs);
 
     expect(set).toHaveBeenCalledWith(key, mockScores);
   });
 
   it('should return uncached results when cache is not needed', async () => {
     process.env.AWS_REGION = 'us-west-1';
+    (getBlockNum as jest.Mock).mockResolvedValue('latest');
     (get as jest.Mock).mockResolvedValue(null); // Not in cache
     (snapshot.utils.getScoresDirect as jest.Mock).mockResolvedValue(mockScores);
-    const result = await scores(null, { ...mockArgs, snapshot: 'latest' }); // "latest" should bypass cache
+    const result = await scores({ ...mockArgs, snapshot: 'latest' }); // "latest" should bypass cache
 
     expect(result).toEqual({
       cache: false,
@@ -75,8 +76,9 @@ describe('scores function', () => {
 
   it("shouldn't return cached results when cache is not available", async () => {
     process.env.AWS_REGION = '';
+    (getBlockNum as jest.Mock).mockResolvedValue(mockArgs.snapshot);
     (snapshot.utils.getScoresDirect as jest.Mock).mockResolvedValue(mockScores);
-    const result = await scores(null, mockArgs);
+    const result = await scores(mockArgs);
 
     expect(result).toEqual({
       cache: false,
@@ -88,9 +90,10 @@ describe('scores function', () => {
 
   it('should restrict block number by `latest`', async () => {
     (snapshot.utils.getScoresDirect as jest.Mock).mockResolvedValue(mockScores);
-    (getBlockNum as jest.Mock).mockResolvedValue('100');
+    (getBlockNum as jest.Mock).mockResolvedValue('latest');
 
-    const result = await scores(null, { ...mockArgs, snapshot: '99999999' });
+    const args = { ...mockArgs, snapshot: '99999999' }; // block in the future
+    const result = await scores(args);
 
     expect(result).toEqual({
       cache: false,

src/scores.ts

@@ -5,29 +5,29 @@ import serve from './requestDeduplicator';
 
 const broviderUrl = process.env.BROVIDER_URL || 'https://rpc.snapshot.org';
 
-async function calculateScores(parent, args, key) {
+async function calculateScores(args, key) {
   const withCache = !!process.env.AWS_REGION;
   const { space = '', strategies, network, addresses } = args;
-  let snapshotBlockNum = 'latest';
-
-  if (args.snapshot !== 'latest') {
-    const currentBlockNum = await getBlockNum(args.snapshot, network);
-    snapshotBlockNum = currentBlockNum < args.snapshot ? 'latest' : args.snapshot;
-  }
+  const snapshotBlockNum = await getBlockNum(args.snapshot, network);
 
   const state = snapshotBlockNum === 'latest' ? 'pending' : 'final';
+
   let scores;
+  let cache = false;
 
-  if (withCache && state === 'final') scores = await get(key);
+  if (withCache && state === 'final') {
+    cache = true;
+    scores = await get(key);
+  }
 
-  let cache = true;
   if (!scores) {
     cache = false;
+    const provider = snapshot.utils.getProvider(network, { broviderUrl });
     scores = await snapshot.utils.getScoresDirect(
       space,
       strategies,
       network,
-      snapshot.utils.getProvider(network, { broviderUrl }),
+      provider,
       addresses,
       snapshotBlockNum
     );
@@ -44,8 +44,8 @@ async function calculateScores(parent, args, key) {
   };
 }
 
-export default function scores(parent, args) {
+export default function scores(args) {
   const id = JSON.stringify(args);
   const cacheKey = sha256(id);
-  return serve(id, calculateScores, [parent, args, cacheKey]);
+  return serve(id, calculateScores, [args, cacheKey]);
 }