fix: correct RevealSubstring LessThan bit widths

[leo1987820]

Description

Fixes #289.

LessThan(n) requires both inputs to fit in [0, 2^n - 1]. RevealSubstring compared dynamic signals against exclusive upper-bound constants such as maxLength, maxSubstringLength + 1, and maxLength + 1, but computed n from the constant itself. When one of those constants is a power of two, the constant cannot fit in the selected bit width.

This PR updates the three LessThan bit-width calculations so each exclusive upper-bound constant has one representable value of headroom:

• substringStartIndex < maxLength uses log2Ceil(maxLength + 1)
• substringLength < maxSubstringLength + 1 uses log2Ceil(maxSubstringLength + 2)
• substringStartIndex + substringLength < maxLength + 1 uses log2Ceil(maxLength + 2)

It also adds a focused regression test that guards those formulas.

Validation

• npx.cmd jest tests/reveal-substring-bit-width.test.ts --runInBand
• npx.cmd prettier --check tests/reveal-substring-bit-width.test.ts
• git diff --check

I also ran npx.cmd jest tests/reveal-substring.test.ts --runInBand --detectOpenHandles --forceExit --verbose, but this local Windows environment does not have circom in PATH, so the existing circuit execution tests stop at circom --version before running assertions.

Summary by CodeRabbit

• Bug Fixes

  • Tightened boundary checks for substring validation to handle edge cases more accurately when start positions, lengths, and combined ranges are near their limits.
  • Kept the substring extraction and optional uniqueness behavior unchanged.

• Tests

  • Added coverage to verify the updated boundary-check logic for substring-related constraints.

[coderabbitai]

📝 Walkthrough

Walkthrough

RevealSubstring updates its range-check bit widths for substring start, length, and total span. A new Jest test reads the circuit helper source and checks the revised LessThan(log2Ceil(...)) expressions.

Changes

RevealSubstring bit-width validation

Layer / File(s)	Summary
Constraint bounds and coverage packages/circuits/helpers/reveal-substring.circom, packages/circuits/tests/reveal-substring-bit-width.test.ts	The circuit now uses revised LessThan(log2Ceil(...)) bounds for substring start, substring length, and their sum, and the added test asserts those updated expressions in the helper source.

Review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I hopped through bits with a careful grin,
The bounds got snug from end to end within.
A nibble here, a nibble there,
My whiskers twitch at numbers fair.
Hop hop—now the substring fits just right!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main change: fixing RevealSubstring LessThan bit widths.
Description check	✅ Passed	The description includes the issue link, motivation, fix summary, and validation, though the template checklist/type sections are incomplete.
Linked Issues check	✅ Passed	The circuit and test changes directly address #289 by widening the three LessThan bit-widths as required.
Out of Scope Changes check	✅ Passed	The PR stays focused on the RevealSubstring bit-width fix and its regression test with no unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/circuits/tests/reveal-substring-bit-width.test.ts (1)

10-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Text-matching assertions guard formatting, not behavior.

This test verifies the circuit source text contains the expected LessThan(log2Ceil(...)) expressions, so it will break on harmless reformatting/renames and won't catch an actual range/overflow regression (e.g., a power-of-two maxLength). A behavioral test that compiles the circuit and feeds a power-of-two bound (mirroring the harness in reveal-substring.test.ts) would more directly protect against #289 recurring. Reasonable to defer if circom isn't available in CI.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/circuits/tests/reveal-substring-bit-width.test.ts` around lines 10 -
20, The current assertion in reveal-substring-bit-width.test.ts is checking
source text in the circuit rather than behavior, so it is brittle and misses
real range/overflow regressions. Replace the string-matching expectations around
the LessThan(log2Ceil(...)) expressions with a behavioral test that compiles and
exercises the reveal-substring circuit using a power-of-two maxLength, following
the same harness pattern used in reveal-substring.test.ts. Keep the test focused
on the relevant circuit entry points and bounds so it validates actual runtime
behavior instead of formatting.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/circuits/tests/reveal-substring-bit-width.test.ts`:
- Around line 10-20: The current assertion in reveal-substring-bit-width.test.ts
is checking source text in the circuit rather than behavior, so it is brittle
and misses real range/overflow regressions. Replace the string-matching
expectations around the LessThan(log2Ceil(...)) expressions with a behavioral
test that compiles and exercises the reveal-substring circuit using a
power-of-two maxLength, following the same harness pattern used in
reveal-substring.test.ts. Keep the test focused on the relevant circuit entry
points and bounds so it validates actual runtime behavior instead of formatting.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6ad7e831-0270-4e12-8bdf-442f8dfef8c2

📥 Commits

Reviewing files that changed from the base of the PR and between 9f01979 and 9856d30.

📒 Files selected for processing (2)

• packages/circuits/helpers/reveal-substring.circom
• packages/circuits/tests/reveal-substring-bit-width.test.ts