diff --git a/src/helpers/strategies.ts b/src/helpers/strategies.ts new file mode 100644 index 00000000..d10002d7 --- /dev/null +++ b/src/helpers/strategies.ts @@ -0,0 +1,68 @@ +import { URL } from 'url'; +import { capture } from '@snapshot-labs/snapshot-sentry'; +import snapshot from '@snapshot-labs/snapshot.js'; +import log from './log'; + +type Strategy = { + id: string; + override: boolean; + disabled: boolean; +}; + +const RUN_INTERVAL = 60e3; +const MAX_CONSECUTIVE_FAILS = 3; +const URI = new URL( + '/api/strategies', + process.env.SCORE_API_URL ?? 'https://score.snapshot.org' +).toString(); + +let consecutiveFailsCount = 0; +let shouldStop = false; +export let strategies: Record = {}; + +async function loadStrategies() { + const res = await snapshot.utils.getJSON(URI); + + if (res.hasOwnProperty('error')) { + capture(new Error('Failed to load strategies'), { + contexts: { input: { uri: URI }, res } + }); + return true; + } + + const strat = Object.values(res).map((strategy: any) => { + strategy.id = strategy.key; + strategy.override = strategy.dependOnOtherAddress || false; + strategy.disabled = strategy.disabled || false; + return strategy; + }); + + strategies = Object.fromEntries(strat.map(strategy => [strategy.id, strategy])); +} + +export async function run() { + while (!shouldStop) { + try { + log.info('[strategies] Start strategies refresh'); + await loadStrategies(); + consecutiveFailsCount = 0; + log.info('[strategies] End strategies refresh'); + } catch (e: any) { + consecutiveFailsCount++; + + if (consecutiveFailsCount >= MAX_CONSECUTIVE_FAILS) { + capture(e); + } + log.error(`[strategies] failed to load ${JSON.stringify(e)}`); + } + + // if stop() has been called after sleep started, + // the loop will exit only after the sleep has completed + await snapshot.utils.sleep(RUN_INTERVAL); + } +} + +export function stop() { + log.info('[strategies] Stopping strategies refresh'); + shouldStop = true; +} diff --git a/src/helpers/validation.ts b/src/helpers/validation.ts new file mode 100644 index 00000000..33de4004 --- /dev/null +++ b/src/helpers/validation.ts @@ -0,0 +1,84 @@ +import snapshot from '@snapshot-labs/snapshot.js'; +import log from './log'; +import { strategies } from './strategies'; + +const DEFAULT_SNAPSHOT_ENV: string = 'testnet'; + +export async function validateSpaceSettings( + originalSpace: any, + snapshotEnv = DEFAULT_SNAPSHOT_ENV +): Promise { + const spaceType = originalSpace.turbo ? 'turbo' : 'default'; + const space = snapshot.utils.clone(originalSpace); + + if (space?.deleted) return Promise.reject('space deleted, contact admin'); + + delete space.deleted; + delete space.flagged; + delete space.verified; + delete space.turbo; + delete space.hibernated; + delete space.id; + + if (space.parent && space.parent === originalSpace.id) { + return Promise.reject('space cannot be its own parent'); + } + + if ( + space.children && + Array.isArray(space.children) && + space.children.includes(originalSpace.id) + ) { + return Promise.reject('space cannot be its own child'); + } + + const schemaIsValid: any = snapshot.utils.validateSchema(snapshot.schemas.space, space, { + spaceType, + snapshotEnv + }); + + if (schemaIsValid !== true) { + log.warn('[writer] Wrong space format', schemaIsValid); + const firstErrorObject: any = Object.values(schemaIsValid)[0]; + if (firstErrorObject.message === 'network not allowed') { + return Promise.reject(firstErrorObject.message); + } + return Promise.reject('wrong space format'); + } + + const strategiesIds: string[] = space.strategies.map((strategy: any) => strategy.name); + if (snapshotEnv !== 'testnet') { + const hasTicket = strategiesIds.includes('ticket'); + const hasVotingValidation = + space.voteValidation?.name && !['any'].includes(space.voteValidation.name); + + if (hasTicket && !hasVotingValidation) { + return Promise.reject('space with ticket requires voting validation'); + } + + const hasProposalValidation = + (space.validation?.name && space.validation.name !== 'any') || + space.filters?.minScore || + space.filters?.onlyMembers; + + if (!hasProposalValidation) { + return Promise.reject('space missing proposal validation'); + } + } + + for (const id of strategiesIds) { + const strategy = strategies[id]; + + if (!strategy) { + return Promise.reject(`strategy "${id}" is not a valid strategy`); + } + + if (strategy.disabled) { + return Promise.reject(`strategy "${id}" is not available anymore`); + } + + if (strategy.override && spaceType !== 'turbo') { + return Promise.reject(`strategy "${id}" is only available for pro spaces`); + } + } +} diff --git a/src/index.ts b/src/index.ts index a0f591d7..5d5b711b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -8,12 +8,14 @@ import initMetrics from './helpers/metrics'; import refreshModeration from './helpers/moderation'; import rateLimit from './helpers/rateLimit'; import shutter from './helpers/shutter'; +import { run as refreshStrategies, stop as stopStrategies } from './helpers/strategies'; import { trackTurboStatuses } from './helpers/turbo'; const app = express(); initLogger(app); refreshModeration(); +refreshStrategies(); initMetrics(app); trackTurboStatuses(); @@ -29,4 +31,23 @@ app.use('/shutter', shutter); fallbackLogger(app); const PORT = process.env.PORT || 3001; -app.listen(PORT, () => log.info(`Started on: http://localhost:${PORT}`)); +const server = app.listen(PORT, () => log.info(`Started on: http://localhost:${PORT}`)); + +const gracefulShutdown = (signal: string) => { + log.info(`Received ${signal}, shutting down gracefully...`); + + stopStrategies(); + + server.close(() => { + log.info('Server closed'); + process.exit(0); + }); + + setTimeout(() => { + log.error('Could not close connections in time, forcefully shutting down'); + process.exit(1); + }, 10000); +}; + +process.on('SIGTERM', () => gracefulShutdown('SIGTERM')); +process.on('SIGINT', () => gracefulShutdown('SIGINT')); diff --git a/src/writer/proposal.ts b/src/writer/proposal.ts index 9cec1e74..a6af3b0d 100644 --- a/src/writer/proposal.ts +++ b/src/writer/proposal.ts @@ -2,7 +2,6 @@ import { capture } from '@snapshot-labs/snapshot-sentry'; import snapshot from '@snapshot-labs/snapshot.js'; import networks from '@snapshot-labs/snapshot.js/src/networks.json'; import { uniq } from 'lodash'; -import { validateSpaceSettings } from './settings'; import { getPremiumNetworkIds, getSpace } from '../helpers/actions'; import log from '../helpers/log'; import { containsFlaggedLinks, flaggedAddresses } from '../helpers/moderation'; @@ -10,6 +9,7 @@ import { isMalicious } from '../helpers/monitoring'; import db from '../helpers/mysql'; import { getLimits, getSpaceType } from '../helpers/options'; import { captureError, getQuorum, jsonParse, validateChoices } from '../helpers/utils'; +import { validateSpaceSettings } from '../helpers/validation'; const scoreAPIUrl = process.env.SCORE_API_URL || 'https://score.snapshot.org'; const broviderUrl = process.env.BROVIDER_URL || 'https://rpc.snapshot.org'; @@ -65,11 +65,7 @@ async function validateSpace(space: any) { return Promise.reject('space hibernated'); } - try { - await validateSpaceSettings(space); - } catch (e) { - return Promise.reject(e); - } + await validateSpaceSettings(space, process.env.NETWORK); } export async function verify(body): Promise { diff --git a/src/writer/settings.ts b/src/writer/settings.ts index b1bed57a..6b70f8d5 100644 --- a/src/writer/settings.ts +++ b/src/writer/settings.ts @@ -1,5 +1,4 @@ import { capture } from '@snapshot-labs/snapshot-sentry'; -import snapshot from '@snapshot-labs/snapshot.js'; import isEqual from 'lodash/isEqual'; import { addOrUpdateSpace, getSpace } from '../helpers/actions'; import log from '../helpers/log'; @@ -12,68 +11,10 @@ import { jsonParse, removeFromWalletConnectWhitelist } from '../helpers/utils'; +import { validateSpaceSettings } from '../helpers/validation'; const SNAPSHOT_ENV = process.env.NETWORK || 'testnet'; -export async function validateSpaceSettings(originalSpace: any) { - const spaceType = originalSpace.turbo ? 'turbo' : 'default'; - const space = snapshot.utils.clone(originalSpace); - - if (space?.deleted) return Promise.reject('space deleted, contact admin'); - - delete space.deleted; - delete space.flagged; - delete space.verified; - delete space.turbo; - delete space.hibernated; - delete space.id; - - if (space.parent && space.parent === originalSpace.id) { - return Promise.reject('space cannot be its own parent'); - } - - if ( - space.children && - Array.isArray(space.children) && - space.children.includes(originalSpace.id) - ) { - return Promise.reject('space cannot be its own child'); - } - - const schemaIsValid: any = snapshot.utils.validateSchema(snapshot.schemas.space, space, { - spaceType, - snapshotEnv: SNAPSHOT_ENV - }); - - if (schemaIsValid !== true) { - log.warn('[writer] Wrong space format', schemaIsValid); - const firstErrorObject: any = Object.values(schemaIsValid)[0]; - if (firstErrorObject.message === 'network not allowed') { - return Promise.reject(firstErrorObject.message); - } - return Promise.reject('wrong space format'); - } - - if (SNAPSHOT_ENV !== 'testnet') { - const hasTicket = space.strategies.some(strategy => strategy.name === 'ticket'); - const hasVotingValidation = - space.voteValidation?.name && !['any'].includes(space.voteValidation.name); - - if (hasTicket && !hasVotingValidation) { - return Promise.reject('space with ticket requires voting validation'); - } - - const hasProposalValidation = - (space.validation?.name && space.validation.name !== 'any') || - space.filters?.minScore || - space.filters?.onlyMembers; - - if (!hasProposalValidation) { - return Promise.reject('space missing proposal validation'); - } - } -} - export async function verify(body): Promise { const msg = jsonParse(body.msg); if (msg.space.length > 64) { @@ -82,12 +23,15 @@ export async function verify(body): Promise { const space = await getSpace(msg.space, true); try { - await validateSpaceSettings({ - ...msg.payload, - id: msg.space, - deleted: space?.deleted, - turbo: space?.turbo - }); + await validateSpaceSettings( + { + ...msg.payload, + id: msg.space, + deleted: space?.deleted, + turbo: space?.turbo + }, + process.env.NETWORK + ); } catch (e) { return Promise.reject(e); } diff --git a/test/integration/ingestor.test.ts b/test/integration/ingestor.test.ts index bc2483b0..a3b9a063 100644 --- a/test/integration/ingestor.test.ts +++ b/test/integration/ingestor.test.ts @@ -2,6 +2,7 @@ import cloneDeep from 'lodash/cloneDeep'; import omit from 'lodash/omit'; import db, { sequencerDB } from '../../src/helpers/mysql'; import relayer from '../../src/helpers/relayer'; +import { run, stop } from '../../src/helpers/strategies'; import ingestor from '../../src/ingestor'; import proposalInput from '../fixtures/ingestor-payload/proposal.json'; import voteInput from '../fixtures/ingestor-payload/vote.json'; @@ -37,6 +38,7 @@ const LIMITS = { 'user.default.follow.limit': 25 }; const ECOSYSTEM_LIST = ['test.eth', 'snapshot.eth']; + jest.mock('../../src/helpers/options', () => { const originalModule = jest.requireActual('../../src/helpers/options'); @@ -129,7 +131,13 @@ function cloneWithNewMessage(data: Record) { } describe('ingestor', () => { - beforeAll(() => { + beforeAll(async () => { + // Start the strategies loader (runs in background) + run(); + + // Wait a bit for the first load to complete + await new Promise(resolve => setTimeout(resolve, 2000)); + proposalInput.data.message.timestamp = Math.floor(Date.now() / 1e3) - 60; proposalInput.data.message.end = Math.floor(Date.now() / 1e3) + 60; voteInput.data.message.timestamp = Math.floor(Date.now() / 1e3) - 60; @@ -142,6 +150,8 @@ describe('ingestor', () => { }); afterAll(async () => { + // Stop any running strategies loader + stop(); await db.queryAsync('DELETE FROM snapshot_sequencer_test.proposals;'); await db.queryAsync('DELETE FROM snapshot_sequencer_test.messages;'); await db.endAsync(); diff --git a/test/unit/helpers/validation.test.ts b/test/unit/helpers/validation.test.ts new file mode 100644 index 00000000..3d98a460 --- /dev/null +++ b/test/unit/helpers/validation.test.ts @@ -0,0 +1,229 @@ +const mockStrategies: Record = { + 'erc20-balance-of': { + id: 'erc20-balance-of', + disabled: false, + override: false + }, + whitelist: { id: 'whitelist', disabled: false, override: false }, + ticket: { id: 'ticket', disabled: false, override: false } +}; + +jest.mock('../../../src/helpers/strategies', () => ({ + strategies: mockStrategies +})); + +// Mock the log module to avoid any issues +jest.mock('../../../src/helpers/log', () => ({ + warn: jest.fn() +})); + +import { validateSpaceSettings } from '../../../src/helpers/validation'; + +describe('helpers/validation', () => { + describe('validateSpaceSettings()', () => { + beforeEach(() => { + jest.clearAllMocks(); + + // Reset mockStrategies to original state + Object.keys(mockStrategies).forEach(key => delete mockStrategies[key]); + Object.assign(mockStrategies, { + 'erc20-balance-of': { + id: 'erc20-balance-of', + disabled: false, + override: false + }, + whitelist: { id: 'whitelist', disabled: false, override: false }, + ticket: { id: 'ticket', disabled: false, override: false } + }); + }); + + const createMockSpace = (overrides = {}) => ({ + id: 'test-space.eth', + name: 'Test Space', + network: '1', + symbol: 'TEST', + strategies: [ + { name: 'erc20-balance-of', params: {} }, + { name: 'whitelist', params: {} } + ], + validation: { name: 'basic' }, + voteValidation: { name: 'basic', params: { minScore: 1 } }, + ...overrides + }); + + describe('strategy validation', () => { + it('should pass when all strategies are valid and enabled', async () => { + const space = createMockSpace(); + await expect(validateSpaceSettings(space, 'mainnet')).resolves.toBeUndefined(); + }); + + it('should reject when strategy does not exist', async () => { + // Remove whitelist strategy from mock + delete mockStrategies['whitelist']; + + const space = createMockSpace(); + await expect(validateSpaceSettings(space, 'mainnet')).rejects.toBe( + 'strategy "whitelist" is not a valid strategy' + ); + }); + + it('should reject when strategy is disabled', async () => { + mockStrategies['whitelist'] = { id: 'whitelist', disabled: true, override: false }; + + const space = createMockSpace(); + await expect(validateSpaceSettings(space, 'mainnet')).rejects.toBe( + 'strategy "whitelist" is not available anymore' + ); + }); + + it('should reject first invalid strategy when multiple are invalid', async () => { + // Keep only erc20-balance-of strategy + Object.keys(mockStrategies).forEach(key => { + if (key !== 'erc20-balance-of') delete mockStrategies[key]; + }); + + const space = createMockSpace({ + strategies: [ + { name: 'invalid-strategy-1', params: {} }, + { name: 'invalid-strategy-2', params: {} } + ] + }); + + await expect(validateSpaceSettings(space, 'mainnet')).rejects.toBe( + 'strategy "invalid-strategy-1" is not a valid strategy' + ); + }); + + it('should reject first disabled strategy when multiple are disabled', async () => { + mockStrategies['strategy1'] = { id: 'strategy1', disabled: true, override: false }; + mockStrategies['strategy2'] = { id: 'strategy2', disabled: true, override: false }; + + const space = createMockSpace({ + strategies: [ + { name: 'strategy1', params: {} }, + { name: 'strategy2', params: {} } + ] + }); + + await expect(validateSpaceSettings(space, 'mainnet')).rejects.toBe( + 'strategy "strategy1" is not available anymore' + ); + }); + + it('should reject override strategy for non-turbo space', async () => { + mockStrategies['override-strategy'] = { + id: 'override-strategy', + disabled: false, + override: true + }; + + const space = createMockSpace({ + strategies: [{ name: 'override-strategy', params: {} }] + }); + + await expect(validateSpaceSettings(space, 'mainnet')).rejects.toBe( + 'strategy "override-strategy" is only available for pro spaces' + ); + }); + + it('should allow override strategy for turbo space', async () => { + mockStrategies['override-strategy'] = { + id: 'override-strategy', + disabled: false, + override: true + }; + + const space = createMockSpace({ + turbo: true, + strategies: [{ name: 'override-strategy', params: {} }] + }); + + await expect(validateSpaceSettings(space, 'mainnet')).resolves.toBeUndefined(); + }); + }); + + describe('other validations', () => { + it('should reject deleted spaces', async () => { + const space = createMockSpace({ deleted: true }); + await expect(validateSpaceSettings(space, 'mainnet')).rejects.toBe( + 'space deleted, contact admin' + ); + }); + + it('should reject space that is its own parent', async () => { + const space = createMockSpace({ + id: 'test-space.eth', + parent: 'test-space.eth' + }); + + await expect(validateSpaceSettings(space, 'mainnet')).rejects.toBe( + 'space cannot be its own parent' + ); + }); + + it('should reject space that includes itself in children', async () => { + const space = createMockSpace({ + id: 'test-space.eth', + children: ['other-space.eth', 'test-space.eth'] + }); + + await expect(validateSpaceSettings(space, 'mainnet')).rejects.toBe( + 'space cannot be its own child' + ); + }); + }); + + describe('mainnet environment', () => { + it('should reject ticket strategy without vote validation on mainnet', async () => { + const space = createMockSpace({ + strategies: [{ name: 'ticket', params: {} }], + voteValidation: undefined + }); + + await expect(validateSpaceSettings(space, 'mainnet')).rejects.toBe( + 'space with ticket requires voting validation' + ); + }); + + it('should reject ticket strategy with "any" vote validation on mainnet', async () => { + const space = createMockSpace({ + strategies: [{ name: 'ticket', params: {} }], + voteValidation: { name: 'any' } + }); + + await expect(validateSpaceSettings(space, 'mainnet')).rejects.toBe( + 'space with ticket requires voting validation' + ); + }); + + it('should allow ticket strategy with proper vote validation on mainnet', async () => { + const space = createMockSpace({ + strategies: [{ name: 'ticket', params: {} }], + voteValidation: { name: 'basic', params: { minScore: 1 } } + }); + + await expect(validateSpaceSettings(space, 'mainnet')).resolves.toBeUndefined(); + }); + }); + + describe('testnet environment', () => { + it('should allow spaces without proposal validation in testnet', async () => { + const space = createMockSpace({ + validation: { name: 'any' }, + strategies: [{ name: 'erc20-balance-of', params: {} }] + }); + + await expect(validateSpaceSettings(space, 'testnet')).resolves.toBeUndefined(); + }); + + it('should allow ticket strategy without vote validation in testnet', async () => { + const space = createMockSpace({ + strategies: [{ name: 'ticket', params: {} }], + voteValidation: undefined + }); + + await expect(validateSpaceSettings(space, 'testnet')).resolves.toBeUndefined(); + }); + }); + }); +}); diff --git a/test/unit/writer/proposal.test.ts b/test/unit/writer/proposal.test.ts index 90a87af5..4f6447f1 100644 --- a/test/unit/writer/proposal.test.ts +++ b/test/unit/writer/proposal.test.ts @@ -1,5 +1,4 @@ import omit from 'lodash/omit'; -import * as writer from '../../../src/writer/proposal'; import { spacesGetSpaceFixtures } from '../../fixtures/space'; import input from '../../fixtures/writer-payload/proposal.json'; @@ -93,6 +92,17 @@ jest.mock('../../../src/helpers/moderation', () => { }; }); +// Mock validateSpaceSettings function +jest.mock('../../../src/helpers/validation', () => ({ + validateSpaceSettings: jest.fn() +})); + +// Get the mocked function after the mock is created +const { validateSpaceSettings: mockValidateSpaceSettings } = jest.requireMock('../../../src/helpers/validation'); + +// Import after mocks are set up +import * as writer from '../../../src/writer/proposal'; + const mockGetProposalsCount = jest.spyOn(writer, 'getProposalsCount'); mockGetProposalsCount.mockResolvedValue([ { @@ -109,6 +119,11 @@ function updateInputPayload(input: any, payload: any) { } describe('writer/proposal', () => { + beforeEach(() => { + // Default validateSpaceSettings to resolve (success) + mockValidateSpaceSettings.mockResolvedValue(undefined); + }); + afterEach(jest.clearAllMocks); const msg = JSON.parse(input.msg); @@ -160,44 +175,6 @@ describe('writer/proposal', () => { expect(mockGetProposalsCount).toHaveBeenCalledTimes(1); }); - describe('when the space has enabled the ticket validation strategy', () => { - it('rejects if the space does not have voting validation', async () => { - expect.assertions(2); - mockGetSpace.mockResolvedValueOnce({ - ...spacesGetSpaceFixtures, - strategies: [{ name: 'ticket' }], - voteValidation: undefined - }); - - await expect(writer.verify(input)).rejects.toMatch('ticket'); - expect(mockGetSpace).toHaveBeenCalledTimes(1); - }); - - it('rejects if the space voting validation is ', async () => { - expect.assertions(2); - mockGetSpace.mockResolvedValueOnce({ - ...spacesGetSpaceFixtures, - strategies: [{ name: 'ticket' }], - voteValidation: { name: 'any' } - }); - - await expect(writer.verify(input)).rejects.toMatch('ticket'); - expect(mockGetSpace).toHaveBeenCalledTimes(1); - }); - - it('does not reject if the space voting validation is anything else valid than ', async () => { - expect.assertions(3); - mockGetSpace.mockResolvedValueOnce({ - ...spacesGetSpaceFixtures, - strategies: [{ name: 'ticket' }], - voteValidation: { name: 'gitcoin' } - }); - - await expect(writer.verify(input)).resolves.toBeUndefined(); - expect(mockGetSpace).toHaveBeenCalledTimes(1); - expect(mockGetProposalsCount).toHaveBeenCalledTimes(1); - }); - }); describe('when the space has set a voting period', () => { const VOTING_PERIOD = 120; @@ -559,108 +536,17 @@ describe('writer/proposal', () => { expect(mockGetSpace).toHaveBeenCalledTimes(1); }); - describe('on invalid space settings', () => { - it('rejects if using testnet on production', async () => { - expect.assertions(2); - mockGetSpace.mockResolvedValueOnce({ - ...spacesGetSpaceFixtures, - network: '5' - }); - - await expect(writer.verify(input)).rejects.toMatch( - 'invalid space settings: network not allowed' - ); - expect(mockGetSpace).toHaveBeenCalledTimes(1); - }); - - it('rejects if the network does not exist', async () => { - expect.assertions(2); - mockGetSpace.mockResolvedValueOnce({ - ...spacesGetSpaceFixtures, - network: '123abc' - }); - - await expect(writer.verify(input)).rejects.toMatch( - 'invalid space settings: network not allowed' - ); - expect(mockGetSpace).toHaveBeenCalledTimes(1); - }); - - it('rejects if using a non-premium network', async () => { - expect.assertions(2); - mockGetSpace.mockResolvedValueOnce({ - ...spacesGetSpaceFixtures, - network: '56' // Using BSC network, which is not in the premium list - }); - - await expect(writer.verify(input)).rejects.toMatch('space is using a non-premium network'); - expect(mockGetSpace).toHaveBeenCalledTimes(1); - }); - - it('should not reject if using a premium network for turbo space', async () => { - expect.assertions(3); - mockGetSpace.mockResolvedValueOnce({ - ...spacesGetSpaceFixtures, - network: '56', // Using Ethereum mainnet, which is in the premium list - turbo: '1' - }); - - await expect(writer.verify(input)).resolves.toBeUndefined(); - expect(mockGetSpace).toHaveBeenCalledTimes(1); - expect(mockGetProposalsCount).toHaveBeenCalledTimes(1); - }); - - it('rejects if strategies use a non-premium network', async () => { - expect.assertions(2); - mockGetSpace.mockResolvedValueOnce({ - ...spacesGetSpaceFixtures, - strategies: [{ name: 'erc20-balance-of', network: '56' }] // Non-premium network - }); - - await expect(writer.verify(input)).rejects.toMatch('space is using a non-premium network'); - expect(mockGetSpace).toHaveBeenCalledTimes(1); - }); - - it('rejects if missing proposal validation', async () => { - expect.assertions(2); - mockGetSpace.mockResolvedValueOnce({ - ...spacesGetSpaceFixtures, - validation: { name: 'any' } - }); - - await expect(writer.verify(input)).rejects.toMatch( - 'invalid space settings: space missing proposal validation' - ); - expect(mockGetSpace).toHaveBeenCalledTimes(1); - }); - - it('rejects if missing vote validation with ticket strategy', async () => { - expect.assertions(2); - mockGetSpace.mockResolvedValueOnce({ - ...spacesGetSpaceFixtures, - validation: { name: 'any' }, - strategies: [{ name: 'ticket' }] - }); - - await expect(writer.verify(input)).rejects.toMatch( - 'invalid space settings: space with ticket requires voting validation' - ); - expect(mockGetSpace).toHaveBeenCalledTimes(1); + it('rejects if space validation fails', async () => { + expect.assertions(2); + mockValidateSpaceSettings.mockRejectedValueOnce('space validation failed'); + mockGetSpace.mockResolvedValueOnce({ + ...spacesGetSpaceFixtures }); - it('rejects if the space was deleted', async () => { - expect.assertions(2); - mockGetSpace.mockResolvedValueOnce({ - ...spacesGetSpaceFixtures, - filters: { onlyMembers: true }, - deleted: true - }); - - await expect(writer.verify(input)).rejects.toMatch( - 'invalid space settings: space deleted, contact admin' - ); - expect(mockGetSpace).toHaveBeenCalledTimes(1); - }); + await expect(writer.verify(input)).rejects.toMatch( + 'invalid space settings: space validation failed' + ); + expect(mockGetSpace).toHaveBeenCalledTimes(1); }); describe('when only members can propose', () => { diff --git a/test/unit/writer/settings.test.ts b/test/unit/writer/settings.test.ts index f78a5db2..25f4b9ab 100644 --- a/test/unit/writer/settings.test.ts +++ b/test/unit/writer/settings.test.ts @@ -1,4 +1,3 @@ -import { verify } from '../../../src/writer/settings'; import { spacesGetSpaceFixtures } from '../../fixtures/space'; import input from '../../fixtures/writer-payload/space.json'; @@ -13,7 +12,10 @@ function randomStrategies(count = 1) { return Array(count) .fill(0) .map(() => ({ - name: `strategy-${Math.floor(Math.random() * 1000)}` + name: 'whitelist', + params: { + addresses: [`0x${Math.floor(Math.random() * 1000)}`] + } })); } @@ -48,7 +50,7 @@ const LIMITS = { }; const ECOSYSTEM_LIST = ['test.eth', 'snapshot.eth']; -const mockGetSpaceType = jest.fn((): any => { +const mockGetSpaceType = jest.fn(async (space: any): Promise => { return 'default'; }); jest.mock('../../../src/helpers/options', () => { @@ -63,8 +65,8 @@ jest.mock('../../../src/helpers/options', () => { getLimit: async (key: string) => { return LIMITS[key]; }, - getSpaceType: () => { - return mockGetSpaceType(); + getSpaceType: (space: any) => { + return mockGetSpaceType(space); } }; }); @@ -99,63 +101,43 @@ jest.mock('@snapshot-labs/snapshot.js', () => { }; }); -describe('writer/settings', () => { - describe('verify()', () => { - describe('on invalid input', () => { - it.todo('rejects if the schema is invalid'); - it('rejects if the space was deleted', async () => { - mockGetSpace.mockResolvedValueOnce({ ...spacesGetSpaceFixtures, deleted: true }); - return expect(verify(input)).rejects.toContain('space deleted'); - }); +// Mock validateSpaceSettings function +jest.mock('../../../src/helpers/validation', () => ({ + validateSpaceSettings: jest.fn() +})); - it('rejects if the network does not exist', async () => { - return expect(verify(editedInput({ network: '1919191919' }))).rejects.toContain( - 'network not allowed' - ); - }); - - it('rejects if using testnet on production', async () => { - return expect(verify(editedInput({ network: '5' }))).rejects.toContain( - 'network not allowed' - ); - }); +// Get the mocked function after the mock is created +const { validateSpaceSettings: mockValidateSpaceSettings } = jest.requireMock('../../../src/helpers/validation'); - it('rejects if missing proposal validation', () => { - return expect(verify(editedInput({ validation: { name: 'any' } }))).rejects.toContain( - 'space missing proposal validation' - ); - }); +// Import after mocks are set up +import { verify } from '../../../src/writer/settings'; - it('rejects if missing vote validation with ticket strategy', async () => { - return expect( - verify(editedInput({ validation: { name: 'any' }, strategies: [{ name: 'ticket' }] })) - ).rejects.toContain('space with ticket requires voting validation'); - }); +describe('writer/settings', () => { + describe('verify()', () => { + beforeEach(() => { + // Reset all mocks + jest.clearAllMocks(); + // Default validateSpaceSettings to resolve (success) + mockValidateSpaceSettings.mockResolvedValue(undefined); + }); - it('rejects if space tries to set itself as parent', async () => { - return expect(verify(editedInput({ parent: 'fabien.eth' }))).rejects.toContain( - 'space cannot be its own parent' - ); - }); + describe('on invalid input', () => { + it.todo('rejects if the schema is invalid'); - it('rejects if space tries to include itself in children array', async () => { - return expect( - verify(editedInput({ children: ['other-space.eth', 'fabien.eth', 'another-space.eth'] })) - ).rejects.toContain('space cannot be its own child'); + it('rejects if validateSpaceSettings fails', async () => { + mockValidateSpaceSettings.mockRejectedValue('validation error'); + return expect(verify(input)).rejects.toBe('validation error'); }); - it('accepts valid parent that is not the space itself', async () => { - return expect(verify(editedInput({ parent: 'parent-space.eth' }))).resolves.toBeUndefined(); - }); + it('rejects if space id is too long', async () => { + const longId = 'a'.repeat(65); + const longInput = { ...input, msg: JSON.parse(input.msg) }; + longInput.msg.space = longId; + const inputWithLongId = { ...input, msg: JSON.stringify(longInput.msg) }; - it('accepts valid children array that does not include the space itself', async () => { - return expect( - verify(editedInput({ children: ['child1.eth', 'child2.eth'] })) - ).resolves.toBeUndefined(); + return expect(verify(inputWithLongId)).rejects.toBe('id too long'); }); - it.todo('rejects if the submitter does not have permission'); - it.todo('rejects if the submitter does not have permission to change admin'); const maxStrategiesForNormalSpace = LIMITS['space.default.strategies_limit']; const maxStrategiesForTurboSpace = LIMITS['space.turbo.strategies_limit']; it(`rejects if passing more than ${maxStrategiesForNormalSpace} strategies for normal space`, async () => { @@ -180,6 +162,9 @@ describe('writer/settings', () => { ).rejects.toContain(`max number of strategies is ${maxStrategiesForTurboSpace}`); }); + it.todo('rejects if the submitter does not have permission'); + it.todo('rejects if the submitter does not have permission to change admin'); + describe('when the space has an existing custom domain', () => { it('accepts a new domain for non-turbo spaces', () => { mockGetSpace.mockResolvedValueOnce({ @@ -246,38 +231,26 @@ describe('writer/settings', () => { }); describe('on valid data', () => { - describe('with ticket strategy but with voting validation', () => { - it('returns a Promise resolve', async () => { - return expect( - verify( - editedInput({ strategies: [{ name: 'ticket' }], voteValidation: { name: 'basic' } }) - ) - ).resolves.toBe(undefined); - }); + it('calls validateSpaceSettings with correct parameters', async () => { + await verify(input); + + expect(mockValidateSpaceSettings).toHaveBeenCalledWith({ + ...JSON.parse(input.msg).payload, + id: JSON.parse(input.msg).space, + deleted: spacesGetSpaceFixtures.deleted, + turbo: spacesGetSpaceFixtures.turbo + }, 'mainnet'); }); - describe('with not ANY validation', () => { - it('returns a Promise resolve', async () => { - return expect(verify(editedInput({ validation: { name: 'basic' } }))).resolves.toBe( - undefined - ); - }); - }); - - describe('with ANY validation but with minScores filters', () => { - it('returns a Promise resolve', async () => { - return expect( - verify(editedInput({ validation: { name: 'any' }, filters: { minScore: 1 } })) - ).resolves.toBe(undefined); - }); - }); + it('passes when validateSpaceSettings succeeds and strategy count is valid', async () => { + const result = await verify( + editedInput({ + strategies: randomStrategies(5) + }) + ); - describe('with ANY validation but with onlyMembers filters', () => { - it('returns a Promise resolve', async () => { - return expect( - verify(editedInput({ validation: { name: 'any' }, filters: { onlyMembers: true } })) - ).resolves.toBe(undefined); - }); + expect(result).toBeUndefined(); + expect(mockValidateSpaceSettings).toHaveBeenCalled(); }); describe('with correct number of strategies for normal spaces', () => {