HackMDScope
- RLN.sol at commit 2dd6719387620e4693c0ce6daf6c203512b8cfb1.
Overview
Rate limiting nullifier uses Fiat-Shamir secret sharing and token staking to prevent spamming on a network. A participant should lock a stake with RLN contract which can only be withdrawn or slashed if a secret is known. That secret is split using Fiat-Shamir secret sharing scheme and if a user sends more messages than a limit, any observer can extract the secret and slash the participant. For more details, head to the documentation.
Issues
1. Low - Careful when setting FEE_RECEIVER
FEE_RECEIVER is transferred some tokens during slashing:
If this statement reverts, it becomes impossible to slash. Hence, care should be taken to ensure FEE_RECEIVER can always receiver tokens.
Recommendation
Add a require statement in constructor to ensure it's not set to 0x0 as some popular ERC20 tokens revert on transfer to 0x0. Also prepare an advisory for RLN deployers to perform a check that their FEE_RECEIVER address can receive tokens (for example, it's not blocked by the token contract).
2. Gas - Cache identityCommitmentIndex
register() reads the storage variable 4 times. Since storage reads are expensive, its value can be stored on the stack.
Recommendation
Load identityCommitmentIndex in a new index variable at the beginning of the function, and replace all further uses with index.
3. Gas - DEPTH not used
DEPTH variable is not used in the contract. Since it's a public variable, a getter function is created increasing the deploy time gas.
Recommendation
Remove the variable if not required.
4. Informational - Document why self-slashing is prohibited
Self-slashing is prohibited:
As clarified by the RLN team, withdraw() transaction can be frontrun by an attacker by calling slash() and providing member.userAddress as receiver. To prevent, this attack, it's important to revert when member.userAddress == receiver.
Recommendation
Document this attack as the reason to prevent self-slashing.
