fix(ensindexer): make bridged-resolver subregistry write order-independent#2175
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
|
|
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 (3)
📝 WalkthroughWalkthroughThis PR fixes bridged-resolver canonicality by removing handler-ordering dependence. ChangesBridged resolver canonicality fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
…ndent handleBridgedResolverChange read the previous resolver from the Domain-Resolver Relation, which Protocol Acceleration overwrites for the same NewResolver event. Two handlers on one log get identical Ponder checkpoints with no deterministic tie-break, so the relation could already hold the new resolver, the equality guard would early-return, and the origin Domain's subregistryId was never set (base.eth/linea.eth subtree left non-canonical). Derive the previous bridged target from the Domain's own subregistryId instead, so the helper no longer depends on cross-plugin handler ordering. Correct the now-false ordering comments in register-handlers.ts and node-migration.ts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
shrugs
force-pushed
the
fix/bridged-resolver-subregistry
branch
from
44cd86a to
39660fe
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes a canonicality edge case in ensindexer where bridged-resolver origin domains (e.g. base.eth, linea.eth) could fail to get their Domain.subregistryId set due to same-log cross-plugin handler write ordering. The fix makes handleBridgedResolverChange derive the “previous bridged target” from the Domain’s own subregistryId (state it owns), removing dependence on the Protocol Acceleration Domain-Resolver Relation write timing.
Changes:
- Update
handleBridgedResolverChangeto compute the previous bridged target fromDomain.subregistryIdviaisBridgedTargetRegistry, instead of reading the Domain-Resolver Relation. - Correct ordering-related comments in handler registration and node-migration docs to reflect that same-log handler ordering can’t be relied on.
- Add a Changeset entry documenting the patch fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/ensindexer/src/plugins/protocol-acceleration/handlers/node-migration.ts | Updates documentation to clarify correctness relies on cross-log checkpoint ordering, not attach order. |
| apps/ensindexer/src/lib/ensv2/canonicality-db-helpers.ts | Makes bridged-resolver subregistry reconciliation order-independent by reading prior state from Domain.subregistryId. |
| apps/ensindexer/ponder/src/register-handlers.ts | Updates comments to warn against relying on same-log handler ordering and explains the new independence model. |
| .changeset/bridged-resolver-subregistry-ordering.md | Adds a patch Changeset describing the canonicality fix and why it’s needed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryThis PR fixes a handler-ordering bug where
Confidence Score: 5/5The change is safe to merge; the logic fix is minimal, well-reasoned, and verified against a live trace. The fix replaces a fragile DRR read with a read of the field the helper itself writes, a narrowly scoped change with a clear correctness argument. The two other files are comment-only. The invariant the fix relies on — that Domain.subregistryId on a bridge origin is exclusively owned by handleBridgedResolverChange — is explicitly documented in the SDK and enforced by the existing isBridgedOriginDomain guard used elsewhere in the unigraph plugin. No files require special attention; all changes are either the targeted one-function fix or documentation corrections. Important Files Changed
Sequence DiagramsequenceDiagram
participant Ponder
participant Unigraph as Unigraph Handler<br/>(handleBridgedResolverChange)
participant PA as Protocol Acceleration<br/>(NewResolver handler)
participant DB as Database
Note over Ponder: Same NewResolver log — identical checkpoints<br/>Tie-break order is non-deterministic
rect rgb(255, 220, 220)
Note over Ponder,DB: BEFORE fix (PA wins tie-break — bug)
Ponder->>PA: dispatch NewResolver (base.eth — BasenamesL1Resolver)
PA->>DB: "write DRR: resolver = BasenamesL1Resolver"
Ponder->>Unigraph: dispatch NewResolver
Unigraph->>DB: "read DRR — prevResolver = BasenamesL1Resolver (PA already wrote it)"
Note over Unigraph: prevBridged.targetRegistryId == nextBridged.targetRegistryId — early return, subregistryId never written
end
rect rgb(220, 255, 220)
Note over Ponder,DB: AFTER fix (order-independent)
Ponder->>PA: dispatch NewResolver
PA->>DB: "write DRR: resolver = BasenamesL1Resolver"
Ponder->>Unigraph: dispatch NewResolver
Unigraph->>DB: read Domain.subregistryId — null (no prior bridged target)
Note over Unigraph: prevBridged = null, nextBridged = Basenames target — handleSubregistryUpdated called
Unigraph->>DB: "write Domain.subregistryId = Basenames target registry"
Note over DB: base.eth canonical edge established
end
Reviews (1): Last reviewed commit: "fix(ensindexer): make bridged-resolver s..." | Re-trigger Greptile |
Summary
Completes the bridged-resolver canonicality fix (#2164) for the origin side.
base.eth/linea.ethsubregistryIdwas being leftNULL, leaving the entire Basenames/Lineanames subtree non-canonical despite the target-sidecanonicalDomainIdbeing set correctly.Root cause — a cross-plugin handler-ordering hazard:
handleBridgedResolverChangeread the "previous" resolver from the Domain-Resolver Relation (DRR).NewResolverlog and overwrites the DRR with the new resolver.block, txIndex, eventType, logIndex— no source discriminator) and sorts with an inconsistent comparator (a.cp < b.cp ? -1 : 1), so the tie order is implementation-defined. In practice Protocol Acceleration's write landed first.prevResolver === nextResolver ===the bridged resolver → the equality guard early-returned →subregistryIdwas never written.Fix — derive the previous bridged target from the Domain's own
subregistryId(viaisBridgedTargetRegistry) instead of the PA-owned DRR. The helper now reads only state it owns, so it's independent of handler ordering. Verified against a captured live trace atbase.eth'sBasenamesL1Resolverevent.Also corrects the now-false ordering comments in
register-handlers.tsandnode-migration.ts: attach-order does not control Ponder dispatch order; same-log handlers can't be ordered deterministically, while cross-log ordering (NodeMigration) is safe by checkpoint.