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.

Sign up for GitHub

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

Jump to bottom

feat(contracts): implement semaphore gatekeeper #1579

Merged
merged 1 commit into from
Jun 21, 2024
Merged

feat(contracts): implement semaphore gatekeeper #1579

merged 1 commit into from
Jun 21, 2024

Conversation

ctrlc03
Copy link
Collaborator

@ctrlc03 ctrlc03 commented Jun 20, 2024 •
edited
Loading

Description

Implement a MACI gatekeeper that uses semaphore to gatekeep access to group members only

Confirmation

@vercel Vercel
Copy link

vercel bot commented Jun 20, 2024 •
edited
Loading

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

Name Status Preview Updated (UTC)
maci-website ✅ Ready (Inspect) Visit Preview Jun 21, 2024 8:25am

@ctrlc03 ctrlc03 requested review from cedoor and 0xjei June 20, 2024 11:13
github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 20, 2024
View reviewed changes
contracts/contracts/gatekeepers/SemaphoreGatekeeper.sol Dismissed Show dismissed Hide dismissed
contracts/contracts/interfaces/ISemaphore.sol
@@ -0,0 +1,20 @@
//SPDX-License-Identifier: MIT
pragma solidity 0.8.20;

Check warning

Code scanning / Slither

Incorrect versions of Solidity Warning

Version constraint 0.8.20 contains known severe issues (https://solidity.readthedocs.io/en/latest/bugs.html)
- VerbatimInvalidDeduplication
- FullInlinerNonExpressionSplitArgumentEvaluationOrder
- MissingSideEffectsOnSelectorAccess.
It is used by:
- 0.8.20
contracts/contracts/mocks/MockSemaphore.sol Fixed Show fixed Hide fixed
contracts/contracts/gatekeepers/SemaphoreGatekeeper.sol Dismissed Show dismissed Hide dismissed
contracts/contracts/gatekeepers/SemaphoreGatekeeper.sol Fixed Show fixed Hide fixed
contracts/contracts/mocks/MockSemaphore.sol Fixed Show fixed Hide fixed
@ctrlc03 ctrlc03 self-assigned this Jun 20, 2024
cedoor
cedoor suggested changes Jun 20, 2024
View reviewed changes
Copy link
Member

@cedoor cedoor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall, I just have a question. Shouldn't there be a mapping to save the id commitments? I can only see one where you're storing the nullifiers.

contracts/contracts/gatekeepers/SemaphoreGatekeeper.sol Outdated Show resolved Hide resolved
0xmad
0xmad reviewed Jun 20, 2024
View reviewed changes
Copy link
Collaborator

@0xmad 0xmad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ctrlc03 thanks, just few suggestions

contracts/contracts/mocks/MockSemaphore.sol Outdated Show resolved Hide resolved
contracts/contracts/mocks/MockSemaphore.sol Outdated Show resolved Hide resolved
@ctrlc03 ctrlc03 force-pushed the feat/semaphore-gatekeeper branch from 9fb4cd8 to f9dd369 Compare June 20, 2024 13:53
@ctrlc03 ctrlc03 force-pushed the feat/semaphore-gatekeeper branch from f9dd369 to 0ce904f Compare June 20, 2024 14:01
@ctrlc03 ctrlc03 requested review from cedoor and 0xmad June 20, 2024 14:31
0xmad
0xmad approved these changes Jun 20, 2024
View reviewed changes
Copy link
Collaborator

@0xmad 0xmad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ctrlc03 thanks!

kittybest
kittybest approved these changes Jun 20, 2024
View reviewed changes
Copy link
Collaborator

@kittybest kittybest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@0xmad 0xmad force-pushed the feat/semaphore-gatekeeper branch from 0ce904f to 3eb331e Compare June 20, 2024 15:02
cedoor
cedoor approved these changes Jun 20, 2024
View reviewed changes
Copy link
Member

@cedoor cedoor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@0xmad 0xmad force-pushed the feat/semaphore-gatekeeper branch from 3eb331e to d0e795b Compare June 20, 2024 16:07
@ctrlc03 ctrlc03 force-pushed the feat/semaphore-gatekeeper branch from d0e795b to dac8bb8 Compare June 21, 2024 07:54
@ctrlc03 ctrlc03 force-pushed the feat/semaphore-gatekeeper branch from dac8bb8 to 5990e3f Compare June 21, 2024 08:01
@ctrlc03 ctrlc03 enabled auto-merge (squash) June 21, 2024 08:01
@ctrlc03 ctrlc03 force-pushed the feat/semaphore-gatekeeper branch from 5990e3f to 2ceae71 Compare June 21, 2024 08:22
@ctrlc03 ctrlc03 merged commit d1f744e into dev Jun 21, 2024
22 checks passed
@ctrlc03 ctrlc03 deleted the feat/semaphore-gatekeeper branch June 21, 2024 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xmad 0xmad 0xmad approved these changes

@cedoor cedoor cedoor approved these changes

@kittybest kittybest kittybest approved these changes

@crisgarner crisgarner crisgarner approved these changes

@samajammin samajammin Awaiting requested review from samajammin

@0xjei 0xjei Awaiting requested review from 0xjei

Assignees

@ctrlc03 ctrlc03

Labels
None yet
Projects
MACI
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ctrlc03 @crisgarner @kittybest @cedoor @0xmad