Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Allow shutter in proposal privacy if space is using shutter privacy #475

Merged
merged 22 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
bada402
fix: update privacy check logic in proposal
ChaituVR Dec 5, 2024
cfe4691
remove unwanted log
ChaituVR Dec 5, 2024
ca5306e
fix
ChaituVR Dec 5, 2024
0fa0bc0
Merge branch 'master' into fix-privacy-check
ChaituVR Dec 6, 2024
d4587b7
Merge branch 'master' into fix-privacy-check
ChaituVR Dec 7, 2024
a9ef94e
test: add tests for proposal privacy
wa0x6e Dec 8, 2024
6647ceb
Merge branch 'fix-privacy-check' of https://github.com/snapshot-labs/…
wa0x6e Dec 8, 2024
ff07b9f
Merge branch 'master' into fix-privacy-check
ChaituVR Dec 8, 2024
a5faf82
Merge branch 'master' of https://github.com/snapshot-labs/snapshot-se…
ChaituVR Dec 9, 2024
71b1fa4
Merge branch 'master' into fix-privacy-check
ChaituVR Jan 4, 2025
f828941
Merge branch 'fix-privacy-check' of https://github.com/snapshot-labs/…
ChaituVR Jan 4, 2025
2b4c67f
fix tests
ChaituVR Jan 4, 2025
b848c06
Update test/schema.sql
ChaituVR Jan 7, 2025
7220696
Remove duplicate test
ChaituVR Jan 7, 2025
534877e
Merge branch 'master' of https://github.com/snapshot-labs/snapshot-se…
ChaituVR Jan 7, 2025
ffafd19
test: remove superfluous assertions
wa0x6e Jan 7, 2025
129bf30
Merge branch 'fix-privacy-check' of https://github.com/snapshot-labs/…
ChaituVR Jan 7, 2025
6cf8c8c
Merge branch 'master' into fix-privacy-check
ChaituVR Jan 7, 2025
98c1f40
Merge branch 'fix-privacy-check' of https://github.com/snapshot-labs/…
ChaituVR Jan 7, 2025
6ed7a53
fix handle both undefined and empty string cases in proposal
ChaituVR Jan 7, 2025
66585e0
test: update test for new privacy behavior when missing
wa0x6e Jan 7, 2025
adf757c
Merge branch 'fix-privacy-check' of https://github.com/snapshot-labs/…
wa0x6e Jan 7, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/writer/proposal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,11 @@ export async function verify(body): Promise<any> {
if (msg.payload.type !== space.voting.type) return Promise.reject('invalid voting type');
}

if (space.voting?.privacy !== 'any' && msg.payload.privacy) {
if (
space.voting?.privacy !== 'any' &&
msg.payload.privacy &&
!(space.voting?.privacy === 'shutter' && msg.payload.privacy === 'shutter')
) {
return Promise.reject('not allowed to set privacy');
}

Expand Down
6 changes: 5 additions & 1 deletion src/writer/update-proposal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ export async function verify(body): Promise<any> {

if (proposal.author !== body.address) return Promise.reject('Not the author');

if (space.voting?.privacy !== 'any' && msg.payload.privacy) {
if (
space.voting?.privacy !== 'any' &&
msg.payload.privacy &&
ChaituVR marked this conversation as resolved.
Show resolved Hide resolved
!(space.voting.privacy === 'shutter' && msg.payload.privacy === 'shutter')
) {
return Promise.reject('not allowed to set privacy');
}

Expand Down
1 change: 1 addition & 0 deletions test/schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ CREATE TABLE proposals (
scores_state VARCHAR(24) NOT NULL,
scores_total DECIMAL(64,30) NOT NULL,
scores_updated INT(11) NOT NULL,
vp_value_by_strategy JSON DEFAULT NULL,
ChaituVR marked this conversation as resolved.
Show resolved Hide resolved
votes INT(12) NOT NULL,
flagged INT NOT NULL DEFAULT 0,
PRIMARY KEY (id),
Expand Down
159 changes: 149 additions & 10 deletions test/unit/writer/proposal.test.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
import omit from 'lodash/omit';
import * as writer from '../../../src/writer/proposal';
import input from '../../fixtures/writer-payload/proposal.json';
import { spacesGetSpaceFixtures } from '../../fixtures/space';
import {
ACTIVE_PROPOSAL_BY_AUTHOR_LIMIT,
ECOSYSTEM_SPACE_PROPOSAL_DAY_LIMIT,
FLAGGED_SPACE_PROPOSAL_DAY_LIMIT,
SPACE_PROPOSAL_DAY_LIMIT,
VERIFIED_SPACE_PROPOSAL_DAY_LIMIT,
ECOSYSTEM_SPACE_PROPOSAL_MONTH_LIMIT,
FLAGGED_SPACE_PROPOSAL_DAY_LIMIT,
FLAGGED_SPACE_PROPOSAL_MONTH_LIMIT,
SPACE_PROPOSAL_MONTH_LIMIT,
VERIFIED_SPACE_PROPOSAL_MONTH_LIMIT,
MAINNET_ECOSYSTEM_SPACES,
ACTIVE_PROPOSAL_BY_AUTHOR_LIMIT,
SPACE_PROPOSAL_DAY_LIMIT,
SPACE_PROPOSAL_MONTH_LIMIT,
TURBO_SPACE_PROPOSAL_DAY_LIMIT,
TURBO_SPACE_PROPOSAL_MONTH_LIMIT
TURBO_SPACE_PROPOSAL_MONTH_LIMIT,
VERIFIED_SPACE_PROPOSAL_DAY_LIMIT,
VERIFIED_SPACE_PROPOSAL_MONTH_LIMIT
} from '../../../src/helpers/limits';
import * as writer from '../../../src/writer/proposal';
import { spacesGetSpaceFixtures } from '../../fixtures/space';
import input from '../../fixtures/writer-payload/proposal.json';

const FLAGGED_ADDRESSES = ['0x0'];

Expand Down Expand Up @@ -69,6 +69,12 @@ mockGetProposalsCount.mockResolvedValue([
}
]);

function updateInputPayload(input: any, payload: any) {
const msg = JSON.parse(input.msg);

return { ...input, msg: JSON.stringify({ ...msg, payload: { ...msg.payload, ...payload } }) };
}

describe('writer/proposal', () => {
afterEach(jest.clearAllMocks);

Expand Down Expand Up @@ -294,6 +300,139 @@ describe('writer/proposal', () => {
expect(mockSnapshotUtilsValidate).toHaveBeenCalledTimes(0);
});
});

describe('when the space is using ANY privacy', () => {
beforeEach(() => {
mockGetSpace.mockResolvedValueOnce({
...spacesGetSpaceFixtures,
voting: { privacy: 'any' }
});
});

it('accepts a proposal with shutter privacy', async () => {
expect.assertions(2);
await expect(
writer.verify(updateInputPayload(input, { privacy: 'shutter' }))
).resolves.toBeUndefined();
expect(mockGetSpace).toHaveBeenCalledTimes(1);
});

it('accepts a proposal with undefined privacy', async () => {
expect.assertions(2);
await expect(
writer.verify(updateInputPayload(input, { privacy: undefined }))
).resolves.toBeUndefined();
expect(mockGetSpace).toHaveBeenCalledTimes(1);
});

it('accepts a proposal with no privacy', async () => {
expect.assertions(2);
await expect(
writer.verify(updateInputPayload(input, { privacy: '' }))
).resolves.toBeUndefined();
expect(mockGetSpace).toHaveBeenCalledTimes(1);
});
});

// Fallback as if { privacy: '' } (privacy disabled)
describe('when the space is missing the privacy settings', () => {
beforeEach(() => {
mockGetSpace.mockResolvedValueOnce({
...spacesGetSpaceFixtures,
voting: undefined
});
});

it('rejects a proposal with shutter privacy', async () => {
expect.assertions(2);
await expect(
writer.verify(updateInputPayload(input, { privacy: 'shutter' }))
).rejects.toMatch('not allowed to set privacy');
expect(mockGetSpace).toHaveBeenCalledTimes(1);
});

it('accepts a proposal with undefined privacy', async () => {
expect.assertions(2);
await expect(
writer.verify(updateInputPayload(input, { privacy: undefined }))
).resolves.toBeUndefined();
expect(mockGetSpace).toHaveBeenCalledTimes(1);
});

it('accepts a proposal with no privacy', async () => {
expect.assertions(2);
await expect(
writer.verify(updateInputPayload(input, { privacy: '' }))
).resolves.toBeUndefined();
expect(mockGetSpace).toHaveBeenCalledTimes(1);
});
});

describe('when the space is using NO privacy', () => {
beforeEach(() => {
mockGetSpace.mockResolvedValueOnce({
...spacesGetSpaceFixtures,
voting: { privacy: '' }
});
});

it('rejects a proposal with shutter privacy', async () => {
expect.assertions(2);
await expect(
writer.verify(updateInputPayload(input, { privacy: 'shutter' }))
).rejects.toMatch('not allowed to set privacy');
expect(mockGetSpace).toHaveBeenCalledTimes(1);
});

it('accepts a proposal with undefined privacy', async () => {
expect.assertions(2);
await expect(
writer.verify(updateInputPayload(input, { privacy: undefined }))
).resolves.toBeUndefined();
expect(mockGetSpace).toHaveBeenCalledTimes(1);
});

it('accepts a proposal with no privacy', async () => {
expect.assertions(2);
await expect(
writer.verify(updateInputPayload(input, { privacy: '' }))
).resolves.toBeUndefined();
expect(mockGetSpace).toHaveBeenCalledTimes(1);
});
});

describe('when the space is using SHUTTER privacy', () => {
beforeEach(() => {
mockGetSpace.mockResolvedValueOnce({
...spacesGetSpaceFixtures,
voting: { privacy: 'shutter' }
});
});

it('accepts a proposal with shutter privacy', async () => {
expect.assertions(2);
await expect(
writer.verify(updateInputPayload(input, { privacy: 'shutter' }))
).resolves.toBeUndefined();
expect(mockGetSpace).toHaveBeenCalledTimes(1);
});

it('accepts a proposal with undefined privacy', async () => {
expect.assertions(2);
await expect(
writer.verify(updateInputPayload(input, { privacy: undefined }))
).resolves.toBeUndefined();
expect(mockGetSpace).toHaveBeenCalledTimes(1);
});

it('accepts a proposal with empty string in privacy', async () => {
ChaituVR marked this conversation as resolved.
Show resolved Hide resolved
expect.assertions(2);
await expect(
writer.verify(updateInputPayload(input, { privacy: '' }))
).resolves.toBeUndefined();
expect(mockGetSpace).toHaveBeenCalledTimes(1);
});
});
});

it('rejects if the snapshot is in the future', async () => {
Expand Down
Loading