From ca8a0b2e97ab4ebff91056bf8401e29ddaa129a6 Mon Sep 17 00:00:00 2001 From: Wan <495709+wa0x6e@users.noreply.github.com> Date: Fri, 17 Nov 2023 14:06:58 +0800 Subject: [PATCH] fix: check for invalid arguments in `validate` (#929) * fix: check for invalid arguments in `validate` * fix: return a Promise rejection on error, instead of throwing * refactor: extract validation functions for easier reuse * Update src/utils.ts * Update src/utils.ts * Update src/utils.spec.js --------- Co-authored-by: Chaitanya --- src/utils.spec.js | 164 ++++++++++++++++++++++++++++++++++++++++++++++ src/utils.ts | 29 ++++++++ 2 files changed, 193 insertions(+) create mode 100644 src/utils.spec.js diff --git a/src/utils.spec.js b/src/utils.spec.js new file mode 100644 index 000000000..3f4ad4ed0 --- /dev/null +++ b/src/utils.spec.js @@ -0,0 +1,164 @@ +import { describe, test, expect, vi, afterEach } from 'vitest'; +import * as crossFetch from 'cross-fetch'; +import { validate } from './utils'; + +vi.mock('cross-fetch', async () => { + const actual = await vi.importActual('cross-fetch'); + + return { + ...actual, + default: vi.fn() + }; +}); +const fetch = vi.mocked(crossFetch.default); + +describe('utils', () => { + afterEach(() => { + vi.resetAllMocks(); + }); + + describe('validate', () => { + const payload = { + validation: 'basic', + author: '0xeF8305E140ac520225DAf050e2f71d5fBcC543e7', + space: 'fabien.eth', + network: '1', + snapshot: 7929876, + params: { + minScore: 0.9, + strategies: [ + { + name: 'eth-balance', + params: {} + } + ] + } + }; + + function _validate({ + validation, + author, + space, + network, + snapshot, + params, + options + }) { + return validate( + validation ?? payload.validation, + author ?? payload.author, + space ?? payload.space, + network ?? payload.network, + snapshot ?? payload.snapshot, + params ?? payload.params, + options ?? {} + ); + } + + describe('when passing invalid args', () => { + const cases = [ + [ + 'author is an invalid address', + { author: 'test-address' }, + /invalid author/i + ], + ['network is not valid', { network: 'mainnet' }, /invalid network/i], + ['network is empty', { network: '' }, /invalid network/i], + [ + 'snapshot is smaller than start block', + { snapshot: 1234 }, + /snapshot \([0-9]+\) must be 'latest' or greater than network start block/i + ] + ]; + + test.each(cases)('throw an error when %s', async (title, args, err) => { + await expect(_validate(args)).rejects.toMatch(err); + }); + }); + + describe('when passing valid args', () => { + test('send a JSON-RPC payload to score-api', async () => { + fetch.mockReturnValue({ + json: () => new Promise((resolve) => resolve({ result: 'OK' })) + }); + + expect(_validate({})).resolves; + expect(fetch).toHaveBeenCalledWith( + 'https://score.snapshot.org', + expect.objectContaining({ + body: JSON.stringify({ + jsonrpc: '2.0', + method: 'validate', + params: payload + }) + }) + ); + }); + + test('send a POST request with JSON content-type', async () => { + fetch.mockReturnValue({ + json: () => new Promise((resolve) => resolve({ result: 'OK' })) + }); + + expect(_validate({})).resolves; + expect(fetch).toHaveBeenCalledWith( + 'https://score.snapshot.org', + expect.objectContaining({ + method: 'POST', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json' + } + }) + ); + }); + + test('can customize the score-api url', () => { + fetch.mockReturnValue({ + json: () => new Promise((resolve) => resolve({ result: 'OK' })) + }); + + expect( + _validate({ options: { url: 'https://snapshot.org/?apiKey=xxx' } }) + ).resolves; + expect(fetch).toHaveBeenCalledWith( + 'https://snapshot.org/?apiKey=xxx', + expect.anything() + ); + }); + + test('returns the JSON-RPC result property', () => { + const result = { result: 'OK' }; + fetch.mockReturnValue({ + json: () => new Promise((resolve) => resolve(result)) + }); + + expect(_validate({})).resolves.toEqual('OK'); + }); + }); + + describe('when score-api is sending a JSON-RPC error', () => { + test('rejects with the JSON-RPC error object', () => { + const result = { error: { message: 'Oh no' } }; + fetch.mockReturnValue({ + json: () => new Promise((resolve) => resolve(result)) + }); + + expect(_validate({})).rejects.toEqual(result.error); + }); + }); + + describe('when the fetch request is failing with not network error', () => { + test('rejects with the error', () => { + const result = new Error('Oh no'); + fetch.mockReturnValue({ + json: () => { + throw result; + } + }); + + expect(_validate({})).rejects.toEqual(result); + }); + }); + }); +}); diff --git a/src/utils.ts b/src/utils.ts index 860496a46..2aee4b2ba 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -31,6 +31,7 @@ export const SNAPSHOT_SUBGRAPH_URL = delegationSubgraphs; const ENS_RESOLVER_ABI = [ 'function text(bytes32 node, string calldata key) external view returns (string memory)' ]; +const EMPTY_ADDRESS = '0x0000000000000000000000000000000000000000'; const scoreApiHeaders = { Accept: 'application/json', @@ -333,6 +334,19 @@ export async function validate( params: any, options: any ) { + if (!isValidAddress(author)) { + return Promise.reject(`Invalid author: ${author}`); + } + + if (!isValidNetwork(network)) { + return Promise.reject(`Invalid network: ${network}`); + } + if (!isValidSnapshot(snapshot, network)) { + return Promise.reject( + `Snapshot (${snapshot}) must be 'latest' or greater than network start block (${networks[network].start})` + ); + } + if (!options) options = {}; if (!options.url) options.url = 'https://score.snapshot.org'; const init = { @@ -530,6 +544,21 @@ export function getNumberWithOrdinal(n) { return n + (s[(v - 20) % 10] || s[v] || s[0]); } +function isValidNetwork(network: string) { + return !!networks[network]; +} + +function isValidAddress(address: string) { + return isAddress(address) && address !== EMPTY_ADDRESS; +} + +function isValidSnapshot(snapshot: number | string, network: string) { + return ( + snapshot === 'latest' || + (typeof snapshot === 'number' && snapshot >= networks[network].start) + ); +} + export default { call, multicall,