fix(ensindexer): reject ENSRainbow heals that don't hash back to the requested labelHash#2181
Conversation
…requested labelHash A poisoned rainbow record (a label whose bytes were mangled while its labelHash key was preserved — e.g. CSV processing stripping the quotes of `"007"` to `007`) heals to a label whose labelhash differs from the request. `ensureUnknownLabel`/`ensureLabel` re-key on the healed label, so the row was written under the wrong primary key and the requested labelHash was never persisted — tripping the canonical-name materialization invariant in `ensureDomainInRegistry` and terminating the indexer. `labelByLabelHash` now verifies `labelhashLiteralLabel(label) === labelHash` (mirroring graph-node's `ens.nameByHash`) and treats a mismatch as unhealable, logging the bad record. The label then falls back to an Encoded LabelHash keyed by the requested labelHash. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: b963025 The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds validation to the ENS rainbow SDK's ChangesMalformed rainbow heal validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Greptile SummaryGuards
Confidence Score: 5/5Safe to merge; the integrity check is a narrow, well-tested addition that only changes behavior for records the server was already returning incorrectly. The fix is tightly scoped: a single guard in client.ts comparing two canonical lowercase hex strings, gated behind an existing success status check. The comparison uses normalizedLabelHash (output of parseLabelHashOrEncodedLabelHash) so casing and padding edge cases are already handled. The rewritten HealNotFoundError is cacheable, matching existing NotFound caching behavior. Tests are correctly updated to supply labels whose keccak256 matches the requested hash. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["heal(labelHash)"] --> B["parseLabelHashOrEncodedLabelHash\nnormalizedLabelHash"]
B --> C{Cache hit?}
C -- Yes --> D[Return cached response]
C -- No --> E[fetch /v1/heal/normalizedLabelHash]
E --> F{status == Success?}
F -- No --> G[Return error as-is]
F -- Yes --> H{"labelhash(label)\n== normalizedLabelHash?"}
H -- Yes --> I[Return HealSuccess]
H -- No --> J["Rewrite as HealNotFoundError\n(malformed record)"]
I --> K{isCacheable?}
J --> K
G --> K
K -- Yes --> L[cache.set]
K -- No --> M[Return uncached]
L --> N[Return response]
Reviews (12): Last reviewed commit: "fix: changeset clarity" | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
This PR hardens ENSIndexer’s ENSRainbow label healing path by rejecting “poisoned” heals where the returned label does not hash back to the requested labelHash, preventing downstream invariants from crashing the indexer and ensuring the fallback path stores an Encoded LabelHash under the correct primary key.
Changes:
- Add a post-heal verification in
labelByLabelHashto ensurekeccak(label) === requested labelHash; mismatches are logged and treated as unhealable (null). - Update
labelByLabelHashtests to cover the poisoned-record scenario and adjust retry success fixtures to satisfy the new hash-back requirement. - Add a Changeset documenting the patch-level behavior change for
ensindexer.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/ensindexer/src/lib/graphnode-helpers.ts | Add hash-back validation for healed labels and log+return null on mismatch. |
| apps/ensindexer/src/lib/graphnode-helpers.test.ts | Update tests to cover poisoned heal rejection and align retry success cases with new validation. |
| .changeset/reject-poisoned-rainbow-heals.md | Publish a patch Changeset describing the robustness fix and its motivation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…shes, "malformed" wording - retry tests use ordinary labels (nick/alice) instead of misleading "networkretry"/"servererror" - tests derive the labelHash via labelhashLiteralLabel instead of hardcoding hex - rename "poisoned" → "malformed"/"inconsistent" in docs/comments/log + changeset - concise changeset wording Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per review, the validation that a healed label hashes back to the requested labelHash now lives in EnsRainbowApiClient.heal() instead of labelByLabelHash. The client compares against its own normalized labelHash (parseLabelHashOrEncodedLabelHash), so non-canonical-but-valid inputs are no longer falsely rejected — addressing the Greptile/Copilot/Vercel notes. A malformed record is returned as NotFound (404) so consumers fall back gracefully via the existing unhealable path. - graphnode-helpers.ts/.test.ts reverted to base; 3 retry/normalization test fixtures made hash-consistent (they exercise the real client, which now rejects inconsistent heals) - new malformed-record test in ensrainbow-sdk client.test.ts - changeset retargeted to @ensnode/ensrainbow-sdk Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptile review |
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… (loop) Converts all success/null result assertions to the await expect(...).resolves form (matching the file's existing .rejects throw-tests), per CodeRabbit and repo convention. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptile review |
what
EnsRainbowApiClient.heal()now verifies a healed label hashes back to the requested labelHash (compared against the client's normalized labelHash). A mismatch — a malformed rainbow record — is returned asNotFound, so consumers fall back gracefully via the existing unhealable path instead of keying data under the wrong labelHash.why
A v1.14.0-alpha reindex crashed at ~78.6% backfill on
unigraph/ENSv1Registry:NewOwner(chain 1, block 15546441):Root cause — a malformed ENSRainbow record:
"007".eth— the on-chain label is the literal 5-char string"007"(quotes included), andkeccak("007"-with-quotes) == 0x00677002….0x00677002…to007(quotes stripped —"is the CSV quote char, so a CSV/SQL ingest step mangled the label while preserving the original labelHash key). ENSRainbow re-validates neither at ingest nor at heal time.ensureUnknownLabel→ensureLabelre-keys on the healed label, writing the row underkeccak("007") = 0xadafce…instead of0x00677002…. The requested labelHash is never persisted.ensureDomainInRegistrysees the.ethvirtual registry is canonical, doesfind(label, { labelHash: 0x00677002… }), getsnull, and throws — a fail-fast that kills the indexer.notes
"007"and similar quoted/special-char labels are recoverable rather than silently dropped to Encoded LabelHashes.🤖 Generated with Claude Code