Repository navigation
feat: discord - #34
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR introduces Discord-specific support for the ZK email authentication system, including a Solidity deployment script, test fixtures, Noir circuits for regex-based email field extraction, and a Honk verifier contract for proof verification. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
benceharomi
force-pushed
the
generic-platform-handling
branch
from
February 20, 2026 19:54
053d3a5 to
53db751
Compare
benceharomi
marked this pull request as ready for review
February 24, 2026 13:43
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
script/DeployLinkHandleEntrypoint/DeployLinkHandleEntrypointDiscord.s.sol (1)
12-15: Hardcoded Sepolia DKIM registry address lacks chain ID safeguard.Both the Discord and Twitter implementations use the same hardcoded "sepolia always valid" address without assertion. While the base script's comments indicate this is intentionally network-specific, adding a chain ID check would prevent accidental deployment on other networks.
Optional chain guard
- function _getDkimRegistryAddress() internal pure override returns (address) { - // sepolia always valid dkim registry - return 0xc4f628496b8c474096650C8f9023954643cC614F; - } + function _getDkimRegistryAddress() internal view override returns (address) { + require(block.chainid == 11155111, "Sepolia only"); + return 0xc4f628496b8c474096650C8f9023954643cC614F; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@script/DeployLinkHandleEntrypoint/DeployLinkHandleEntrypointDiscord.s.sol` around lines 12 - 15, The hardcoded Sepolia DKIM registry in _getDkimRegistryAddress needs a chain guard: inside the function (or immediately before returning) assert that block.chainid == 11155111 (Sepolia) and revert or revert with a clear message if not, so the address cannot be used on other chains; keep returning 0xc4f628496b8c474096650C8f9023954643cC614F when the chain ID check passes and add a brief comment indicating this is Sepolia-only.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/fixtures/linkHandleCommand/discord/files/blueprint.json`:
- Around line 26-39: The regexDef "(?:\\r\\n|^)from:[^\\r\\n]*@" is
case‑sensitive and will miss headers like "From:"; update the pattern used in
the parts array (the regexDef entry) to match header names case‑insensitively —
for example add an inline case‑insensitive flag like
"(?i)(?:\\r\\n|^)from:[^\\r\\n]*@" or use a case‑insensitive class (e.g.
"(?:\\r\\n|^)(?i:from):[^\\r\\n]*@"); alternatively normalize header names to
lowercase before running the existing regex in the code that consumes this
blueprint (where the parts array is used).
In `@test/fixtures/linkHandleCommand/discord/Nargo.toml`:
- Around line 1-5: This Nargo.toml under test fixtures is a generated artifact
(package entry "zkemail/discord_2@1" / manifest content) and must not be edited
manually; revert any changes to this Nargo.toml and update the generator or
source-of-truth that produces fixtures (the tool or script that emits the
package/type/authors/compiler_version fields) so future regenerations reflect
the intended values—locate the fixture generator that outputs the Nargo.toml for
linkHandleCommand/discord and make the fix there rather than editing the
generated Nargo.toml directly.
In `@test/fixtures/linkHandleCommand/discord/src/handle_regex.nr`:
- Around line 1-6: handle_regex.nr is a generated fixture and should not be
edited directly; instead update the generator/template that emits files under
test/fixtures/linkHandleCommand/**/src (the fixture generator that produces
handle_regex.nr and its imports like zkregex::utils/select_subarray,
captures::capture_substring, sparse_array::SparseArray,
transitions::check_transition_with_captures) and re-run the generation step so
the corrected code is produced automatically.
In `@test/fixtures/linkHandleCommand/discord/src/main.nr`:
- Around line 1-37: This generated fixture (function main in the
test/fixtures/linkHandleCommand/*/src) was edited manually; revert any hand
changes and regenerate the file from the fixture generator instead of editing
directly. Locate the generated Noir file containing fn main(...) and restore it
by running the project’s fixture generation step (the generator that produces
test/fixtures/linkHandleCommand/*/src) so the file matches the canonical output;
if you need to change the behavior, update the generator that emits main rather
than editing this generated file.
In `@test/fixtures/linkHandleCommand/discord/src/sender_domain_regex.nr`:
- Around line 1-6: The file sender_domain_regex.nr is a generated fixture and
should not be edited directly; instead update the generator that produces files
under test/fixtures/linkHandleCommand/**/src so the import block (currently
referencing select_subarray, capture_substring, SparseArray,
check_transition_with_captures from zkregex::utils) is corrected there; modify
the generator code/template that emits sender_domain_regex.nr to change or
reorder those imports and then re-generate the fixtures so the repository
contains the updated generated artifact.
In `@test/fixtures/linkHandleCommand/discord/target/HonkVerifier.sol`:
- Around line 245-253: The exp(Fr base, Fr exponent) function currently only
repeatedly squares which yields base^(2^k) instead of base^exponent; replace it
with a square‑and‑multiply loop: unwrap the exponent to a uint (using
Fr.unwrap), initialize a result variable to Fr.wrap(1) and a working base
variable to base, then iterate while the exponent > 0—if (e & 1) multiply result
by working base, square the working base each iteration, and right‑shift the
exponent; finally return result (Fr) using Fr.wrap/unwrap as appropriate so
exp(base, exponent) computes base^exponent for any exponent bits.
- Around line 1837-1875: The batchMul function computes a cumulative "success"
flag across ecMul (precompile 7) and ecAdd (precompile 6) calls but never checks
it before returning, risking use of garbage results; update the assembly in
batchMul (function name: batchMul, local var: success) to revert on failure by
inserting an if iszero(success) { revert(0, 0) } check after the for loop
(mirroring the error handling used in invert and pow) so the function aborts
when any precompile call fails.
---
Nitpick comments:
In `@script/DeployLinkHandleEntrypoint/DeployLinkHandleEntrypointDiscord.s.sol`:
- Around line 12-15: The hardcoded Sepolia DKIM registry in
_getDkimRegistryAddress needs a chain guard: inside the function (or immediately
before returning) assert that block.chainid == 11155111 (Sepolia) and revert or
revert with a clear message if not, so the address cannot be used on other
chains; keep returning 0xc4f628496b8c474096650C8f9023954643cC614F when the chain
ID check passes and add a brief comment indicating this is Sepolia-only.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
script/DeployLinkHandleEntrypoint/DeployLinkHandleEntrypointDiscord.s.soltest/fixtures/linkHandleCommand/LinkHandleCommandTestFixture.soltest/fixtures/linkHandleCommand/discord/Nargo.tomltest/fixtures/linkHandleCommand/discord/files/blueprint.jsontest/fixtures/linkHandleCommand/discord/src/handle_regex.nrtest/fixtures/linkHandleCommand/discord/src/main.nrtest/fixtures/linkHandleCommand/discord/src/sender_domain_regex.nrtest/fixtures/linkHandleCommand/discord/target/HonkVerifier.sol
zkfriendly
approved these changes
Mar 9, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary by CodeRabbit
New Features
Tests