Repository navigation
Conversation
`^` (Look::Start) was compiled as an unconditional epsilon transition. Because matchStart is a prover input, a circuit for a `^`-anchored pattern accepted a match at any haystack offset. For example, `(?:\r\n|^)from:...` accepted `from:` inside `subject:from:x\r\n`. Start anchors are now a separate kind of epsilon edge. Ordinary epsilon closures never cross them, since a `^` reached after consuming a byte can never hold. Each start state whose closure reaches a `^` gets an anchored twin that carries the transitions of the anchor-crossing closure. Nothing transitions into the twin, and both backends require matchStart == 0 when the path begins there. NFAGraph records these states in `anchored_start_states`, and witness generation only uses them for matches at offset 0. Other look-around assertions are still compiled as unconditional epsilons. Until they are supported, they are rejected with E1002 instead of being silently dropped. Also assert `isValidStartState === 1` in Circom circuits with a single start state (e.g. mixed_capture_length). Before, it was computed but never constrained. Tests: regressions for the reported witness, and a differential test that checks the circuit's path constraints against regex-automata for every (matchStart, matchLength) window over enumerated haystacks. Regenerated the bundled Circom and Noir templates. The Noir tests are regenerated only for anchored patterns; elsewhere the circuit bodies are unchanged.
Noir transition lookup keys were packed as `state + byte * 257 + next * 257^2`. That only works while state ids stay below 257, and the compiler had to scan for aliasing keys and reject larger automata. A single Unicode `\w` already needs over 300 states. The prover-supplied states were also never range-checked in-circuit, so a witness could claim a state beyond the radix (e.g. `s + 257` reading `b - 1`) whose key collides with a real transition. Keys are now `state + byte * 2^16 + next * 2^24`. `check_transition` asserts both states fit in 16 bits, which makes the packing injective for any witness. The compiler rejects automata with more than 2^16 states, replacing the alias scan. States stay `Field`s, so the generated `regex_match` signature is unchanged. The sparse array now range-checks keys to 63 bits instead of 32, since keys use up to 40 bits. Tables are sized from the largest real key instead of an estimate that assumed at most 200 states. Tests: Noir regressions for the `^` fix (the reported witness and its anchored variant, plus an honest control), a state-aliasing test that fails without the range check, and Rust tests for key injectivity and >257-state automata. Noir templates regenerated (only the lookup tables change).
|
Important Review skippedToo many files! This PR contains 107 files, which is 7 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
📒 Files selected for processing (107)
You can disable this status message by setting the
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 |
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
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.
Fixes #119.
I also found that without the epsilon transitions, the states can be around 300, so sadly we need a u16 for that... I needed to make space for the novel packing format. Happy for suggestions to better packing.
The code was vibed by claude 5.5, but I checked it didn't do anything stupid. For example it tried to replace all the Field to u16 and I told it not to.