feat: add contenthash interpretation - #2285
Conversation
🦋 Changeset detectedLatest commit: fdd3bda The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
|
Warning Review limit reached
More reviews will be available in 21 minutes and 44 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds end-to-end ENSIP-7 contenthash interpretation and exposes it as ChangesENS Domain Contenthash Profile Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 SummaryThis PR adds
Confidence Score: 5/5Safe to merge — the interpreter is well-bounded by a try/catch, all null/empty cases are explicitly handled, and the schema change is purely additive. The core decoder is straightforward: strip 0x, call getCodec/decode, assemble the output object, catch everything else and return null. Error paths are covered. The GraphQL additions are additive and non-breaking. The two comments are style observations about dead fallback code and sentinel fixtures in the devnet, neither of which affects correctness in production. No files require special attention. fixtures.ts is worth a glance to confirm that seeding the empty/invalid/zero sentinel subnames on-chain is intentional. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as GraphQL Client
participant Schema as profile.ts (Schema)
participant Builder as build-profile-selection.ts
participant Resolver as ENS Resolver (on-chain)
participant Interpreter as ProfileContenthashInterpreter
participant Lib as @ensdomains/content-hash
Client->>Schema: "query { domain { resolve { profile { contenthash { ... } } } } }"
Schema->>Builder: buildProfileSelectionFromResolveContainerInfo(info)
Builder->>Builder: topLevelFields.has(contenthash) → merge selection
Builder-->>Schema: "{ contenthash: true }"
Schema->>Resolver: fetch records.contenthash (raw hex)
Resolver-->>Schema: "0xe30101... | 0x | null"
Schema->>Interpreter: interpret(result)
Interpreter->>Interpreter: "raw == null || raw == 0x → return null"
Interpreter->>Lib: "getCodec(hex) → ipfs | swarm | ..."
Interpreter->>Lib: decode(hex) → CID / hash string
Interpreter->>Interpreter: build uri, httpUrl from PROTOCOL_CONFIG
Interpreter-->>Schema: "{ protocolType, decoded, uri, httpUrl } | null"
Schema-->>Client: "ProfileContenthash | null"
Reviews (6): Last reviewed commit: "add UrlString" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@packages/integration-test-env/src/devnet/fixtures.ts`:
- Around line 97-99: The inline comment for the contenthash value (the property
named contenthash in the fixture object) is inconsistent with the actual decoded
CID — decode the hex string "0xe30101..." assigned to contenthash (or re-derive
the correct hex for the expected CID) and update either the comment to reflect
the true CID (e.g., bafybei... if that decodes correctly) or replace the hex
value so it encodes the commented CID (QmRAQ...). Locate the contenthash
property in fixtures.ts, validate the multicodec/multihash decoding result, and
make the comment and assigned hex consistent with each other. Ensure the chosen
fix preserves test expectations that rely on the decoded CID.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 94d69902-f372-4323-9abf-6fb60b9b98e8
⛔ Files ignored due to path filters (3)
packages/enssdk/src/omnigraph/generated/introspection.tsis excluded by!**/generated/**packages/enssdk/src/omnigraph/generated/schema.graphqlis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.changeset/clean-insects-accept.mdapps/ensapi/package.jsonapps/ensapi/src/omnigraph-api/lib/resolution/profile/README.mdapps/ensapi/src/omnigraph-api/lib/resolution/profile/build-profile-selection.tsapps/ensapi/src/omnigraph-api/lib/resolution/profile/interpreters/contenthash.tsapps/ensapi/src/omnigraph-api/lib/resolution/profile/interpreters/index.tsapps/ensapi/src/omnigraph-api/lib/resolution/profile/profile-descriptions.tsapps/ensapi/src/omnigraph-api/schema/domain.integration.test.tsapps/ensapi/src/omnigraph-api/schema/profile.tspackages/ensskills/skills/omnigraph/SKILL.mdpackages/integration-test-env/src/devnet/fixtures.ts
|
i could see a schema improvement with either: a) protocolType should become enum, or |
sevenzing
changed the title
|
agree, I will do option A. |
|
@greptile review please ❤️ |
2 participants
Summary
Domain.resolve.profile.contenthashinterpretationdecided to use simple structure for that:
Pre-Review Checklist (Blocking)