forked from Consensys/linea-monorepo
-
Notifications
You must be signed in to change notification settings - Fork 2
Rln fixes #84
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
+74
−75
Merged
Rln fixes #84
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
64de266
feat(RLN)!: make the slash function internal to avoid front-running
gravityblast 0c46c78
feat(RLN)!: use the slasher address to compute the slash commitment id
gravityblast 680d85f
fix(RLN): fix doc comment
gravityblast 0541e68
fix(RLN): check that the slashed account is the same of the one used …
gravityblast 6512da4
test(RLN): add test to ensure users cannot slash an account using the…
gravityblast File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -117,9 +117,19 @@ contract RLN is Initializable, UUPSUpgradeable, AccessControlUpgradeable { | |
| } | ||
| } | ||
|
|
||
| /// @dev Computes the slash commitment key with the slasher address and hash. | ||
| /// The slasher address is included to ensure uniqueness per slasher, not allowing anyone else to override the same | ||
| /// hash without even knowing the private key. | ||
| /// @param sender: address of the slasher; | ||
| /// @param hash: keccak256 hash of abi.encodePacked(privateKey, rewardRecipient); | ||
| /// @return bytes32: the computed commitment key. | ||
| function _slashCommitmentKey(address sender, bytes32 hash) internal pure returns (bytes32) { | ||
| return keccak256(abi.encodePacked(sender, hash)); | ||
| } | ||
|
|
||
| /// @dev Sets the slash reveal window time. | ||
| /// @param _slashRevealWindowTime: new reveal window time in seconds. | ||
| /// @notice The window time must be at least 1 second and no more than 365 days. | ||
| /// @notice The window time must be at least 1 second and no more than 1 day. | ||
| /// A non-zero value is required to ensure the queuing mechanism functions correctly. | ||
| /// An excessively large value could lock commitments indefinitely. | ||
| function setSlashRevealWindowTime(uint256 _slashRevealWindowTime) external onlyRole(DEFAULT_ADMIN_ROLE) { | ||
|
|
@@ -154,23 +164,6 @@ contract RLN is Initializable, UUPSUpgradeable, AccessControlUpgradeable { | |
| } | ||
| } | ||
|
|
||
| /// @dev Slashes identity with privateKey. | ||
| /// @param privateKey: RLN private key as bytes32; | ||
| /// @param rewardRecipient: Address that will receive the slash reward; | ||
| function slash(bytes32 privateKey, address rewardRecipient) public onlyRole(SLASHER_ROLE) { | ||
| // Hash the private key using Poseidon to get identityCommitment | ||
| uint256 identityCommitment = poseidonHasher.hash(uint256(privateKey)); | ||
|
|
||
| User memory member = members[identityCommitment]; | ||
| if (member.userAddress == address(0)) { | ||
| revert RLN__MemberNotFound(); | ||
| } | ||
| karma.slash(member.userAddress, rewardRecipient); | ||
| delete members[identityCommitment]; | ||
|
|
||
| emit MemberSlashed(member.index, msg.sender); | ||
| } | ||
|
|
||
| /// @dev Commits to a future slash operation using a hash. | ||
| /// @notice This is the first step of the commit-reveal scheme for slashing. | ||
| /// The slasher must first commit to a hash of (privateKey, rewardRecipient) before revealing | ||
|
|
@@ -190,7 +183,8 @@ contract RLN is Initializable, UUPSUpgradeable, AccessControlUpgradeable { | |
| revealStartTime = lastReveal + slashRevealWindowTime; | ||
| } | ||
|
|
||
| slashCommitments[account][hash] = revealStartTime; | ||
| bytes32 key = _slashCommitmentKey(msg.sender, hash); | ||
| slashCommitments[account][key] = revealStartTime; | ||
| lastRevealStartTime[account] = revealStartTime; | ||
| } | ||
|
|
||
|
|
@@ -214,7 +208,8 @@ contract RLN is Initializable, UUPSUpgradeable, AccessControlUpgradeable { | |
| { | ||
| /// forge-lint: disable-next-line(asm-keccak256) | ||
| bytes32 hash = keccak256(abi.encodePacked(privateKey, rewardRecipient)); | ||
| uint256 revealStartTime = slashCommitments[account][hash]; | ||
| bytes32 key = _slashCommitmentKey(msg.sender, hash); | ||
| uint256 revealStartTime = slashCommitments[account][key]; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this is a fix, we should make it a |
||
|
|
||
| if (revealStartTime == 0) { | ||
| revert RLN__InvalidCommitment(); | ||
|
|
@@ -224,7 +219,24 @@ contract RLN is Initializable, UUPSUpgradeable, AccessControlUpgradeable { | |
| revert RLN__RevealWindowNotStarted(); | ||
| } | ||
|
|
||
| delete slashCommitments[account][hash]; | ||
| slash(privateKey, rewardRecipient); | ||
| delete slashCommitments[account][key]; | ||
|
|
||
| uint256 identityCommitment = poseidonHasher.hash(uint256(privateKey)); | ||
| User memory member = members[identityCommitment]; | ||
| if (member.userAddress == address(0)) { | ||
| revert RLN__MemberNotFound(); | ||
| } | ||
|
|
||
| // We make sure that the account slashed matches the account used during the commit phase | ||
| // otherwise someone could front-run the slasher by committing to slash a different account | ||
| // to skip the queuing mechanism. | ||
| if (account != member.userAddress) { | ||
| revert RLN__InvalidCommitment(); | ||
| } | ||
|
|
||
| karma.slash(member.userAddress, rewardRecipient); | ||
| delete members[identityCommitment]; | ||
|
|
||
| emit MemberSlashed(member.index, msg.sender); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add a test for this please? |
||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we reference the issue that it will close?