Skip to content

Make filter limit optional and harden filter and negentropy parsing - #142

Merged
kwsantiago merged 2 commits into
mainfrom
optional-filter-limit
Oct 6, 2026
Merged

kwsantiago merged 2 commits into
mainfrom
optional-filter-limit

Conversation

@kwsantiago

@kwsantiago kwsantiago commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

NIP-01 now requires a relay to return no stored events for a filter with "limit": 0, then send EOSE and keep the subscription live. limit_val defaulted to 0, so limit: 0 was indistinguishable from no limit. It is now ?u32, and Filter.limit() returns ?u32 (breaking).

Untrusted input hardening:

  • Filter parsing used @intCast from the JSON i64 into i32 for limit and kinds, so a REQ with "limit": 99999999999 panicked in Debug and ReleaseSafe and was undefined behavior in ReleaseFast. limit now saturates at u32 max, a negative limit is ignored, and a negative relay COUNT parses as 0.
  • kinds entries outside i32 or not integers are dropped. An ids, authors or kinds array that is non-empty but has no usable entry now yields an empty slice (matches nothing) instead of null (matches everything).
  • Negentropy reconcile cast the mode varint straight into the exhaustive Mode enum, so a mode byte of 3 or more panicked or was undefined behavior. A varint that decoded to zero length left the input unconsumed and looped forever. Both now return ParseError.

Tests cover each case, including a negentropy test that crashes on the previous code.

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 44 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c0761caa-eeab-4997-9cc7-0ad5379888bc
📥 Commits

Reviewing files that changed from the base of the PR and between 3430cd8 and 0d4bbc6.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • src/messages.zig
  • src/negentropy.zig

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9d988db3-31b4-47c0-9479-b0eb82a9c964
📥 Commits

Reviewing files that changed from the base of the PR and between 4ec8e85 and 3430cd8.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • src/filter.zig
  • src/messages.zig

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

Filter limits now use optional unsigned values, and serialization preserves an explicitly set zero. Message parsing clamps out-of-range filter integers and maps negative relay COUNT values to zero.

Changes

Filter Limits and Parsing

Layer / File(s) Summary
Represent and serialize optional limits
src/filter.zig, src/messages.zig
Filter.limit_val and Filter.limit() now use ?u32. Serialization includes any present limit, including zero. Tests cover cloning and serialization.
Bound parsed integer values
src/messages.zig, CHANGELOG.md
Regular and negentropy filters clamp kinds to the i32 range and limits to the u32 range. Negative limits become absent. Negative COUNT values become zero. Tests cover these cases and omitted limits.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 3430c

The change appears ready to merge: its intentional breaking API change is documented, no in-repository callers remain on the old signature, and no concrete user-facing failure is established.

Architecture Summary

Architecture risk: 🔵 Low · up to 3430c

The change affects 2 systems.

Changed systems: src, CHANGELOG.md

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 2 changed files map to changed impact.
  • observed — CHANGELOG.md (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in CHANGELOG.md: Adds Unreleased Changed and Fixed entries documenting limit representation, serialization and negative-limit handling, plus integer parsing outcomes: oversized limits clamp to u32 max, out-of-range kinds clamp to i32 bounds, and negative relay COUNT becomes zero.
  • observed — Modified behavior in src/filter.zig: Filter.limit_val changes from a signed integer defaulting to zero to an optional unsigned 32-bit value defaulting to null.
  • observed — Modified behavior in src/filter.zig: Filter.limit now returns an optional u32 rather than an i32.
  • observed — Modified behavior in src/filter.zig: Serialization now emits limit when limit_val is present, including zero; the previous positive-value check omitted zero and negative values.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: making filter limits optional and hardening filter and negentropy parsing.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

A rabbit checks the limit field,
A zero now can be revealed.
Kind values stay within their bounds,
While negative counts settle down.
The parser hops through numbers true,
And leaves absent limits null, not two.

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

@kwsantiago kwsantiago changed the title Make filter limit optional and clamp out-of-range filter integers Make filter limit optional and harden filter and negentropy parsing Oct 6, 2026
@kwsantiago
kwsantiago merged commit c1046f1 into main Oct 6, 2026
3 checks passed
@kwsantiago
kwsantiago deleted the optional-filter-limit branch October 6, 2026 03:42
@kwsantiago kwsantiago mentioned this pull request Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant