fix: `isRegistrationFullyExpired` helper by tk-o · Pull Request #2348 · namehash/ensnode

tk-o · 2026-07-09T07:23:20Z

Lite PR

Tip: Review docs on the ENSNode PR process

Summary

What changed (1-3 bullets, no essays).

Why

Why this change exists. Link to related GitHub issues where relevant.

Testing

How this was tested.
If you didn't test it, say why.

Notes for Reviewer (Optional)

Anything non-obvious or worth a heads-up.

Pre-Review Checklist (Blocking)

This PR does not introduce significant changes and is low-risk to review quickly.
Relevant changesets are included (or are not required)

changeset-bot · 2026-07-09T07:23:24Z

⚠️ No Changeset found

Latest commit: 448d5b0

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

vercel · 2026-07-09T07:23:26Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
admin.ensnode.io	Ready	Preview, Comment	Jul 9, 2026 7:23am
enskit-react-example.ensnode.io	Ready	Preview, Comment	Jul 9, 2026 7:23am
ensnode.io	Ready	Preview, Comment	Jul 9, 2026 7:23am
ensrainbow.io	Ready	Preview, Comment	Jul 9, 2026 7:23am

coderabbitai · 2026-07-09T07:24:55Z

📝 Walkthrough

Walkthrough

The isRegistrationFullyExpired boundary check changed from >= to > when comparing current time against expiry plus grace period. A new Vitest test file was added covering isRegistrationExpired, isRegistrationFullyExpired, and isRegistrationInGracePeriod, including boundary and null-input cases.

Changes

Registration expiration boundary logic and tests

Layer / File(s)	Summary
Fully-expired boundary comparison `packages/ensnode-sdk/src/registrars/registration-expiration.ts`	Boundary check changed from `now >= expiry + grace` to `now > expiry + grace`, so a registration exactly at that instant is no longer treated as fully expired.
Expiration helper test suite `packages/ensnode-sdk/src/registrars/registration-expiration.test.ts`	New Vitest suite with parameterized cases for `isRegistrationExpired`, `isRegistrationFullyExpired`, and `isRegistrationInGracePeriod`, including boundary conditions and null `expiry`/`gracePeriod` handling.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

A hop, a nibble, a boundary tweak,
>= gave way to >, so sleek!
Tests now guard each edge in sight,
Expiry, grace, and null just right.
This bunny thumps approval — day made bright! 🐇✅

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is only the template and lacks concrete summary, rationale, and testing details.	Replace the placeholders with real Summary, Why, and Testing details, and note any reviewer points or checklist exceptions.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
Title check	✅ Passed	The title names the main fix and matches the helper logic change in this PR.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/is-registration-in-grace-period-helper

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

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

vercel · 2026-07-09T07:26:33Z


-  // otherwise it is expired if now >= expiry + grace
-  return now >= info.expiry + (info.gracePeriod ?? 0n);
+  // otherwise it is expired if now > expiry + grace


A one-instant gap at now == expiry + gracePeriod where a registration is reported as NEITHER in grace period NOR fully expired.

Copilot

Pull request overview

Adjusts ENSNode SDK registration-expiration boundary logic so “fully expired” aligns with strict post-grace semantics, and adds Vitest coverage to lock in the expected behavior.

Changes:

Change isRegistrationFullyExpired to use a strict now > expiry + gracePeriod comparison.
Add a new registration-expiration.test.ts suite covering isRegistrationExpired, isRegistrationFullyExpired, and isRegistrationInGracePeriod, including boundary cases.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
packages/ensnode-sdk/src/registrars/registration-expiration.ts	Tweaks fully-expired boundary condition to be strict after grace period.
packages/ensnode-sdk/src/registrars/registration-expiration.test.ts	Adds unit tests validating expiration/grace behavior and boundary conditions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+  // otherwise it is expired if now > expiry + grace
+  return now > info.expiry + (info.gracePeriod ?? 0n);


greptile-apps · 2026-07-09T07:27:18Z

Greptile Summary

This PR fixes a perceived off-by-one in isRegistrationFullyExpired (changing >= to >) and adds the first test suite covering all three registration-expiration helpers. However, the change introduces a logical gap rather than fixing one.

Implementation change: isRegistrationFullyExpired now uses strict >, making now == expiry + gracePeriod return false.
Test coverage: New tests cover isRegistrationExpired, isRegistrationFullyExpired, and isRegistrationInGracePeriod across boundary cases, but the assertion at now = expiry + gracePeriod for isRegistrationFullyExpired encodes the wrong expected value (false instead of true).
Logic gap: After the fix, at now = expiry + gracePeriod, isRegistrationExpired is true while both isRegistrationInGracePeriod and isRegistrationFullyExpired are false — a state that should be impossible. The original >= bound maintained the correct exhaustive partition: grace period = [expiry, expiry+gracePeriod), fully expired = [expiry+gracePeriod, ∞).

Confidence Score: 3/5

Not safe to merge as-is — the isRegistrationFullyExpired change creates a silent gap in state coverage that could cause callers to misclassify a registration at the exact expiry+grace boundary.

The core change introduces a one-second window where a registration is logically expired but classified as neither in grace period nor fully expired, breaking the mutual-exclusivity invariant between the three helpers. The added tests encode the wrong boundary expectation, meaning the bug passes CI undetected.

Both changed files need attention: registration-expiration.ts for the >= vs > boundary decision, and registration-expiration.test.ts for the incorrect expected value at now = expiry + gracePeriod.

Important Files Changed

Filename	Overview
packages/ensnode-sdk/src/registrars/registration-expiration.ts	Changes `isRegistrationFullyExpired` boundary from `>=` to `>`, introducing a logical gap where `expiry + gracePeriod` is neither in grace period nor fully expired.
packages/ensnode-sdk/src/registrars/registration-expiration.test.ts	New test file covering all three helpers; the `isRegistrationFullyExpired` expectation at `now = expiry + gracePeriod` is incorrect (expects false, should be true).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([now]) --> B{expiry == null?}
    B -- yes --> C([false: never expired])
    B -- no --> D{now >= expiry?}
    D -- no --> E([isRegistrationExpired: false
isRegistrationInGracePeriod: false
isRegistrationFullyExpired: false])
    D -- yes --> F{gracePeriod != null
AND expiry + gracePeriod > now?}
    F -- yes --> G([isRegistrationExpired: true
isRegistrationInGracePeriod: true
isRegistrationFullyExpired: false])
    F -- no --> H{now > expiry + gracePeriod
PR fix: strict greater-than}
    H -- yes --> I([isRegistrationExpired: true
isRegistrationInGracePeriod: false
isRegistrationFullyExpired: true])
    H -- no --> J([GAP: isRegistrationExpired: true
isRegistrationInGracePeriod: false
isRegistrationFullyExpired: false
now == expiry + gracePeriod])

    style J fill:#f66,color:#fff
    style G fill:#6a6,color:#fff
    style I fill:#66a,color:#fff

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A([now]) --> B{expiry == null?}
    B -- yes --> C([false: never expired])
    B -- no --> D{now >= expiry?}
    D -- no --> E([isRegistrationExpired: false
isRegistrationInGracePeriod: false
isRegistrationFullyExpired: false])
    D -- yes --> F{gracePeriod != null
AND expiry + gracePeriod > now?}
    F -- yes --> G([isRegistrationExpired: true
isRegistrationInGracePeriod: true
isRegistrationFullyExpired: false])
    F -- no --> H{now > expiry + gracePeriod
PR fix: strict greater-than}
    H -- yes --> I([isRegistrationExpired: true
isRegistrationInGracePeriod: false
isRegistrationFullyExpired: true])
    H -- no --> J([GAP: isRegistrationExpired: true
isRegistrationInGracePeriod: false
isRegistrationFullyExpired: false
now == expiry + gracePeriod])

    style J fill:#f66,color:#fff
    style G fill:#6a6,color:#fff
    style I fill:#66a,color:#fff

_{Reviews (1): Last reviewed commit: "fix: `isRegistrationInGracePeriod` helpe..." | Re-trigger Greptile}

greptile-apps · 2026-07-09T07:27:21Z

-  // otherwise it is expired if now >= expiry + grace
-  return now >= info.expiry + (info.gracePeriod ?? 0n);
+  // otherwise it is expired if now > expiry + grace
+  return now > info.expiry + (info.gracePeriod ?? 0n);


Boundary gap breaks expired-state invariant

After the fix, at exactly now == expiry + gracePeriod all three helpers are inconsistent: isRegistrationExpired returns true (because expiry <= now), but isRegistrationInGracePeriod returns false (its upper bound is strictly expiry + gracePeriod > now) and isRegistrationFullyExpired now also returns false (the new strict now > expiry + gracePeriod). A registration is therefore expired, not in grace period, and not fully expired simultaneously — a state that should be impossible.

The original >= made isRegistrationFullyExpired true at the boundary and kept the functions mutually exclusive and exhaustive. The correct complementary partition is: grace period = [expiry, expiry + gracePeriod), fully expired = [expiry + gracePeriod, ∞), which requires >= in isRegistrationFullyExpired, not >.

greptile-apps · 2026-07-09T07:27:22Z

+
+    it("returns false when expiry is null", () => {


Wrong expected value for boundary case

The test asserts that isRegistrationFullyExpired returns false when now == expiry + gracePeriod (1100n). That assertion is incorrect: a registration whose grace period has just concluded is fully expired at that exact timestamp, not in some intermediate state. Keeping expected: true here (matching the original >= semantics) would correctly document that the boundary is the first instant of full expiry, consistent with how isRegistrationInGracePeriod already uses a strict upper-bound > to exclude that same instant.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

fix: isRegistrationInGracePeriod helper

448d5b0

tk-o requested a review from a team as a code owner July 9, 2026 07:23

Copilot AI review requested due to automatic review settings July 9, 2026 07:23

Copilot started reviewing on behalf of tk-o July 9, 2026 07:23 View session

vercel Bot reviewed Jul 9, 2026

View reviewed changes

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Comment thread packages/ensnode-sdk/src/registrars/registration-expiration.ts

Comment on lines +26 to +27

// otherwise it is expired if now > expiry + grace

return now > info.expiry + (info.gracePeriod ?? 0n);

tk-o changed the title ~~fix: isRegistrationInGracePeriod helper~~ fix: isRegistrationFullyExpired helper Jul 9, 2026

greptile-apps Bot reviewed Jul 9, 2026

View reviewed changes

tk-o mentioned this pull request Jul 9, 2026

Hotfix/22f5cb7 #2349

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: `isRegistrationFullyExpired` helper#2348

fix: `isRegistrationFullyExpired` helper#2348
tk-o wants to merge 1 commit into
mainfrom
fix/is-registration-in-grace-period-helper

tk-o commented Jul 9, 2026 •

edited

Loading

Uh oh!

changeset-bot Bot commented Jul 9, 2026

Uh oh!

vercel Bot commented Jul 9, 2026

Uh oh!

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

vercel Bot Jul 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

greptile-apps Bot commented Jul 9, 2026

Uh oh!

greptile-apps Bot Jul 9, 2026

Uh oh!

greptile-apps Bot Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// otherwise it is expired if now > expiry + grace
		return now > info.expiry + (info.gracePeriod ?? 0n);

Uh oh!

Conversation

tk-o commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Lite PR

Summary

Why

Testing

Notes for Reviewer (Optional)

Pre-Review Checklist (Blocking)

Uh oh!

changeset-bot Bot commented Jul 9, 2026

⚠️ No Changeset found

Uh oh!

vercel Bot commented Jul 9, 2026

Uh oh!

coderabbitai Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

vercel Bot Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

greptile-apps Bot commented Jul 9, 2026

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Flowchart

Uh oh!

greptile-apps Bot Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tk-o commented Jul 9, 2026 •

edited

Loading

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading