Conversation
Y3drk
added
ensawards
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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 (4)
📝 WalkthroughWalkthroughThe PR migrates ENS name/address types and values across the codebase: ChangesInterpretedName & NormalizedAddress Migration
Sequence Diagram(s)(omitted — changes are primarily type/data migrations and UI typing updates without a new multi-component sequential control flow) Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
…rk/chore/apply-remaining-freedback-from-pr195
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR applies remaining follow-ups from PR #195 by aligning name/address typing with enssdk (InterpretedName / NormalizedAddress), updating mocks/tests to use SDK parsers (parseUsdc, parseTimestamp), and making a small mobile layout fix to the referral link form.
Changes:
- Migrate various
Name-typed fields and literals toInterpretedNameusingasInterpretedName(...)across UI + data files. - Update mocks/tests to use
parseUsdc(...)andparseTimestamp(...)for consistent currency/timestamp modeling. - Fix referral link form mobile sizing and do small naming cleanups for EnsNode provider options.
Reviewed changes
Copilot reviewed 53 out of 53 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ensawards.org/src/utils/resolution.ts | Updates resolveEthAddress typing/docs to NormalizedAddress. |
| ensawards.org/src/pages/advocate/index.astro | Switches example advocate names to InterpretedName via asInterpretedName. |
| ensawards.org/src/contract-pipelines/contractsTestData.ts | Converts contract test ENS names to asInterpretedName. |
| ensawards.org/src/components/referral-awards-program/referrals/utils.test.ts | Uses parseTimestamp and asInterpretedName in referral qualification tests. |
| ensawards.org/src/components/referral-awards-program/ReferralLinkForm.tsx | Refactors name normalization variables + adjusts max width for mobile layout. |
| ensawards.org/src/components/mocks/referrers-leaderboard/data.tsx | Uses parseUsdc for award pool/money fields + renames mock admin action vars. |
| ensawards.org/src/components/mocks/referrers-leaderboard/MockDisplayReferrerLeaderboardPage.tsx | Renames provider options variable for clarity/consistency. |
| ensawards.org/src/components/mocks/referral-program-editions/data.ts | Uses parseUsdc for mock edition rule amounts. |
| ensawards.org/src/components/mocks/referral-live-feed/data.ts | Uses asInterpretedName for mock registrar action names. |
| ensawards.org/src/components/ens-advocates/details-page-components/advocate-profile/FetchAndDisplayAdvocateProfileWithName.tsx | Updates prop type from Name to InterpretedName. |
| ensawards.org/src/components/ens-advocates/details-page-components/FetchAndDisplayAdvocateReferrals.tsx | Aligns Address type import to enssdk (not viem). |
| ensawards.org/src/components/ens-advocates/details-page-components/FetchAndDisplayAdvocateProfile.tsx | Wraps usePrimaryName result with asInterpretedName for downstream typing. |
| ensawards.org/src/components/ens-advocates/AdvocateSearchForm.tsx | Aligns Address type import to enssdk (keeps viem validation). |
| ensawards.org/src/components/atoms/cards/quoteCard/types.ts | Updates quote author name type to InterpretedName. |
| ensawards.org/src/components/atoms/cards/quoteCard/data.ts | Converts static quote author names to asInterpretedName. |
| ensawards.org/src/components/atoms/cards/ContributorsCard.tsx | Renames provider options variable for consistency. |
| ensawards.org/src/components/atoms/cards/ContractCard.astro | Avoids passing name when unnamed; removes unused name prop from metadata component usage. |
| ensawards.org/src/components/atoms/badges/ContractBadge.tsx | Updates name prop type to InterpretedName and adjusts helper signature formatting. |
| ensawards.org/src/components/atoms/SmartContractMetadata.tsx | Removes unused name prop and DRYs repeated “Learn more” link into a component. |
| ensawards.org/data/protocols/uniswap-defi/index.ts | Converts protocol socials.ens to asInterpretedName. |
| ensawards.org/data/protocols/uniswap-dao/index.ts | Converts protocol socials.ens to asInterpretedName. |
| ensawards.org/data/protocols/uniswap-dao/contracts.ts | Converts cached contract name values to asInterpretedName. |
| ensawards.org/data/protocols/types.ts | Updates protocol socials.ens type to InterpretedName. |
| ensawards.org/data/protocols/taiko-defi/index.ts | Converts protocol socials.ens to asInterpretedName. |
| ensawards.org/data/protocols/taiko-defi/contracts.ts | Converts cached contract name values to asInterpretedName. |
| ensawards.org/data/protocols/taiko-dao/index.ts | Converts protocol socials.ens to asInterpretedName. |
| ensawards.org/data/protocols/taiko-dao/contracts.ts | Converts cached contract name values to asInterpretedName. |
| ensawards.org/data/protocols/ssvnetwork-dao/index.ts | Converts protocol socials.ens to asInterpretedName. |
| ensawards.org/data/protocols/ssvnetwork-dao/contracts.ts | Converts cached contract name values to asInterpretedName. |
| ensawards.org/data/protocols/nouns-dao/index.ts | Converts protocol socials.ens to asInterpretedName. |
| ensawards.org/data/protocols/nouns-dao/contracts.ts | Converts cached contract name values to asInterpretedName. |
| ensawards.org/data/protocols/liquity-defi/index.ts | Converts protocol socials.ens to asInterpretedName. |
| ensawards.org/data/protocols/liquity-defi/contracts.ts | Converts cached contract name values to asInterpretedName. |
| ensawards.org/data/protocols/index.test.ts | Improves failure message for invalid interpreted names. |
| ensawards.org/data/protocols/giveth-defi/index.ts | Converts protocol socials.ens to asInterpretedName. |
| ensawards.org/data/protocols/giveth-defi/contracts.ts | Converts cached contract name values to asInterpretedName. |
| ensawards.org/data/protocols/ens-dao/index.ts | Converts protocol socials.ens to asInterpretedName. |
| ensawards.org/data/protocols/ens-dao/contracts.ts | Converts cached contract name values to asInterpretedName. |
| ensawards.org/data/protocols/cork-defi/index.ts | Converts protocol socials.ens to asInterpretedName. |
| ensawards.org/data/protocols/cork-defi/contracts.ts | Converts cached contract name values to asInterpretedName. |
| ensawards.org/data/protocols/contracts.test.ts | Adds clarifying comments + improves invalid name error message. |
| ensawards.org/data/protocols/contracts-types.ts | Updates contract identity name fields to InterpretedName. |
| ensawards.org/data/protocols/arbitrum-dao/index.ts | Converts protocol socials.ens to asInterpretedName. |
| ensawards.org/data/protocols/aave-dao/index.ts | Converts protocol socials.ens to asInterpretedName. |
| ensawards.org/data/apps/walletchan-wallet/index.ts | Converts app socials.ens to asInterpretedName. |
| ensawards.org/data/apps/types.ts | Updates app socials.ens type to InterpretedName. |
| ensawards.org/data/apps/rainbow-wallet/index.ts | Converts app socials.ens to asInterpretedName. |
| ensawards.org/data/apps/metamask-wallet/index.ts | Converts app socials.ens to asInterpretedName. |
| ensawards.org/data/apps/index.test.ts | Improves failure message for invalid interpreted names. |
| ensawards.org/data/apps/etherscan-explorer/index.ts | Converts app socials.ens to asInterpretedName. |
| ensawards.org/data/apps/coinbase-wallet/index.ts | Converts app socials.ens to asInterpretedName. |
| ensawards.org/data/apps/blockscout-explorer/index.ts | Converts app socials.ens to asInterpretedName. |
| CONTRIBUTING.md | Updates documentation snippets to reflect InterpretedName in socials.ens. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ensawards.org/src/components/ens-advocates/AdvocateSearchForm.tsx (1)
52-64:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a real disabled state for the empty form.
aria-disabledalone doesn't stop interaction here, so the submit button can still be focused and activated when the field is empty. Add the nativedisabledstate so the affordance matches the behavior.♿ Proposed fix
<button type="submit" + disabled={rawInputAddress.length === 0} aria-disabled={rawInputAddress.length === 0} className={cn(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ensawards.org/src/components/ens-advocates/AdvocateSearchForm.tsx` around lines 52 - 64, The submit button in AdvocateSearchForm.tsx uses aria-disabled but remains interactive; change the button to use the native disabled attribute when rawInputAddress.length === 0 (e.g., add disabled={rawInputAddress.length === 0}), keep aria-disabled if you want the ARIA state but ensure the CSS/variants reflect the disabled state (adjust shadcnButtonVariants or className to show disabled cursor/opacity when disabled) so the button is truly non-interactive and visually disabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@ensawards.org/src/components/ens-advocates/AdvocateSearchForm.tsx`:
- Around line 52-64: The submit button in AdvocateSearchForm.tsx uses
aria-disabled but remains interactive; change the button to use the native
disabled attribute when rawInputAddress.length === 0 (e.g., add
disabled={rawInputAddress.length === 0}), keep aria-disabled if you want the
ARIA state but ensure the CSS/variants reflect the disabled state (adjust
shadcnButtonVariants or className to show disabled cursor/opacity when disabled)
so the button is truly non-interactive and visually disabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c610166c-0d0f-44b2-86af-4ae37162f808
📒 Files selected for processing (53)
CONTRIBUTING.mdensawards.org/data/apps/blockscout-explorer/index.tsensawards.org/data/apps/coinbase-wallet/index.tsensawards.org/data/apps/etherscan-explorer/index.tsensawards.org/data/apps/index.test.tsensawards.org/data/apps/metamask-wallet/index.tsensawards.org/data/apps/rainbow-wallet/index.tsensawards.org/data/apps/types.tsensawards.org/data/apps/walletchan-wallet/index.tsensawards.org/data/protocols/aave-dao/index.tsensawards.org/data/protocols/arbitrum-dao/index.tsensawards.org/data/protocols/contracts-types.tsensawards.org/data/protocols/contracts.test.tsensawards.org/data/protocols/cork-defi/contracts.tsensawards.org/data/protocols/cork-defi/index.tsensawards.org/data/protocols/ens-dao/contracts.tsensawards.org/data/protocols/ens-dao/index.tsensawards.org/data/protocols/giveth-defi/contracts.tsensawards.org/data/protocols/giveth-defi/index.tsensawards.org/data/protocols/index.test.tsensawards.org/data/protocols/liquity-defi/contracts.tsensawards.org/data/protocols/liquity-defi/index.tsensawards.org/data/protocols/nouns-dao/contracts.tsensawards.org/data/protocols/nouns-dao/index.tsensawards.org/data/protocols/ssvnetwork-dao/contracts.tsensawards.org/data/protocols/ssvnetwork-dao/index.tsensawards.org/data/protocols/taiko-dao/contracts.tsensawards.org/data/protocols/taiko-dao/index.tsensawards.org/data/protocols/taiko-defi/contracts.tsensawards.org/data/protocols/taiko-defi/index.tsensawards.org/data/protocols/types.tsensawards.org/data/protocols/uniswap-dao/contracts.tsensawards.org/data/protocols/uniswap-dao/index.tsensawards.org/data/protocols/uniswap-defi/index.tsensawards.org/src/components/atoms/SmartContractMetadata.tsxensawards.org/src/components/atoms/badges/ContractBadge.tsxensawards.org/src/components/atoms/cards/ContractCard.astroensawards.org/src/components/atoms/cards/ContributorsCard.tsxensawards.org/src/components/atoms/cards/quoteCard/data.tsensawards.org/src/components/atoms/cards/quoteCard/types.tsensawards.org/src/components/ens-advocates/AdvocateSearchForm.tsxensawards.org/src/components/ens-advocates/details-page-components/FetchAndDisplayAdvocateProfile.tsxensawards.org/src/components/ens-advocates/details-page-components/FetchAndDisplayAdvocateReferrals.tsxensawards.org/src/components/ens-advocates/details-page-components/advocate-profile/FetchAndDisplayAdvocateProfileWithName.tsxensawards.org/src/components/mocks/referral-live-feed/data.tsensawards.org/src/components/mocks/referral-program-editions/data.tsensawards.org/src/components/mocks/referrers-leaderboard/MockDisplayReferrerLeaderboardPage.tsxensawards.org/src/components/mocks/referrers-leaderboard/data.tsxensawards.org/src/components/referral-awards-program/ReferralLinkForm.tsxensawards.org/src/components/referral-awards-program/referrals/utils.test.tsensawards.org/src/contract-pipelines/contractsTestData.tsensawards.org/src/pages/advocate/index.astroensawards.org/src/utils/resolution.ts
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
Greptile SummaryThis PR applies the remaining feedback from PR#195, systematically migrating
Confidence Score: 5/5This PR is safe to merge — all changes are mechanical type tightening, utility-constructor adoption, and a well-bounded mobile layout fix with no new logic paths that could regress. Every changed file applies one of three well-understood transformations: replacing bare literals with validated constructors, narrowing type aliases, or extracting duplicated JSX. No new async paths, state machines, or data-flow branches are introduced, and CI passes. No files require special attention — the changes are uniform and low-risk across all 60 files. Important Files Changed
Reviews (5): Last reviewed commit: "Apply ai assistant's suggestions, pt.4" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@Y3drk Thanks for this! Great to see the updates. There's a few more opportunities for refinement here. Thanks 👍
| ) => { | ||
| switch (resolutionStatus) { | ||
| case ContractResolutionStatusIds.ForwardNamed: | ||
| return ( |
There was a problem hiding this comment.
I note the following line of code below:
{ensName} successfully resolves to this contract, but this contract does not have an{" "}
We should avoid ever directly taking ENS names and displaying them in UIs as strings without processing that converts them into "beautified" display form. The best practice for this is implemented inside one of our namehash-ui components if I remember correctly? I think we have a UI component such as "DisplayName" or something like that?
Please apply this idea to all the relevant places 👍
There was a problem hiding this comment.
Applied the idea in:
- ContractBadge
- QuoteCard (
@/components/atoms/cards/quoteCard/index.astro) - ContractCard
- Advocate Details search page (currently not publicly linked/advertised)
| @@ -1,6 +1,7 @@ | |||
| import type { ReferralProgramEditionSummary } from "@namehash/ens-referrals"; | |||
| import { type Address } from "enssdk"; | |||
There was a problem hiding this comment.
Please review any use of Address anywhere across the code in ENSAwards. Unless there's a special situation all of these should become NormalizedAddress. Please review each case and ensure our logic is correct. Thanks.
Note that all addresses returned by ENSNode APIs are guaranteed to always be NormalizedAddress.
There was a problem hiding this comment.
Changed from using Address to NormalizedAddress in:
- AdvocateSearchForm
- Shared sub-components of
ReferrerCard - Components related to fetching & displaying the Advocate details page
- RegistrarActionsList
- getEnsAdvocateDetailsRelativePath helper function
Plus, adjusted the ReferralLinkForm component to always use normalized addresses when building the referral link.
| { | ||
| name: "vitalik.eth", | ||
| name: asInterpretedName("vitalik.eth"), | ||
| ethAddress: "0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045", |
There was a problem hiding this comment.
I expect data contributors are going to mess up making these addresses to be NormalizedAddress in many cases and it's difficult for our eyes to manually catch the case that some hex digits are uppercase.
I note here how some of the hex digits here are uppercase and therefore it's not appropriate to call this a NormalizedAddress.
I would also note how there's technical reasons in Ponder for why Matt didn't make NormalizedAddress as strictly implemented as InterpretedName is from a type-system perspective (with type branding, etc..).
Perhaps we should revisit this? Matt is very busy now and we shouldn't distract him with these questions for now, but I suggest sometime next week to raise this question with him and see what he thinks. I believe we need to implement the same formalities around NormalizedAddress as we implemented for InterpretedName and that we need utility functions such as asNormalizedAddress which would then guarantee the value returned is a NormalizedAddress.
This would then also allow us to remove the unit tests in ENSAwards that complain if a user's raw input address is not all in lowercase. I think maybe you defined some tests like this? But note how it's easy to miss the need to do this because for example the hardcoded address here is in mixed case, not all lowercase (normalized form).
There was a problem hiding this comment.
- For now, in this specific component, I simply use the
toNormalizedAddresshelper function. - And yes, we have unit tests in place that check whether all addresses in
dataare normalized. At the same time, as you said, it's easy to miss any potential mistakes in user-facing examples like here.
Also created a follow-up issue to log the need to discuss that → namehash/ensnode#2053
| // or the name is not a valid ENS interpreted name, | ||
| // display a UI based just on the address | ||
| if (data.name === null) return <AdvocateProfileWithoutName address={address} />; | ||
| if (data.name === null || !isInterpretedName(data.name)) |
There was a problem hiding this comment.
We should change our strategy here.
ENSNode guarantees that all names it returns in its APIs are a valid InterpretedName. However it should be noted that it's not proper for an ENSNode client such as ENSAwards to then just assume it's correct to type the name values it receives as an InterpretedName and be done with it. No, the problem with this is that InterpretedName depends on ens-normalize, and ens-normalize can change across time. The version of ens-normalize on the server can be different than the version of ens-normalize on the server and this can cause errors to be thrown on clients if they mistakingly make this assumption.
The complete solution to this is that whenever an ENSNode client receives an InterpretedName in an API response from ENSNode, it should call reinterpretName (I think this is what the utility function is called, please check in enssdk) on that value. This will solve for the edge case that the client and server have different versions of ens-normalize.
If any questions please let me know 👍
There was a problem hiding this comment.
Refactored this and other similar use case in ensawards.org/data/protocols/contracts.test.ts according to the new strategy guidelines.
I am completely sure that this approach is necessary in those two places (direct use of our api/resolve/primary-name endpoint).
I am not sure about the places where we use our other API endpoints (useRegistrarActions or useResolvedIdentity) to simply display the name on the UI and not perform any additional actions on it. Appreciate your advice regarding such cases, and please let me know if I misunderstood something.
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 `@ensawards.org/src/components/atoms/badges/ContractBadge.tsx`:
- Around line 36-38: The current early return checks name === undefined before
resolutionStatus, causing unresolved identities without a name to show the
"unnamed" tooltip; update the logic in ContractBadge (where name,
resolutionStatus, unnamedContractContent, and unresolvedContractContent are
used) to check resolutionStatus === 'Unresolved' (or the equivalent unresolved
enum/value) first and return unresolvedContractContent for that case, and only
if not unresolved then fall back to the name === undefined check to return
unnamedContractContent.
🪄 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: 9d3c53ec-d89f-4f3f-af71-bd8eff888407
📒 Files selected for processing (20)
ensawards.org/data/protocols/contracts.test.tsensawards.org/data/shared/test-utils.tsensawards.org/src/components/atoms/badges/ContractBadge.tsxensawards.org/src/components/atoms/cards/ContractCard.astroensawards.org/src/components/atoms/cards/quoteCard/index.astroensawards.org/src/components/atoms/cards/referrerCard/shared.tsxensawards.org/src/components/ens-advocates/AdvocateSearchForm.tsxensawards.org/src/components/ens-advocates/EnsAdvocateDetailsPageProvider.tsxensawards.org/src/components/ens-advocates/details-page-components/FetchAndDisplayAdvocateProfile.tsxensawards.org/src/components/ens-advocates/details-page-components/FetchAndDisplayAdvocateReferrals.tsxensawards.org/src/components/ens-advocates/details-page-components/advocate-profile/AdvocateProfileWithoutName.tsxensawards.org/src/components/ens-advocates/details-page-components/advocate-profile/FetchAndDisplayAdvocateProfileWithName.tsxensawards.org/src/components/mocks/referral-live-feed/data.tsensawards.org/src/components/mocks/referral-program-editions/data.tsensawards.org/src/components/mocks/referrers-leaderboard/data.tsxensawards.org/src/components/referral-awards-program/ReferralLinkForm.tsxensawards.org/src/components/referral-awards-program/referrals/RegistrarActionsList.tsxensawards.org/src/contract-pipelines/contractsTestData.tsensawards.org/src/pages/advocate/index.astroensawards.org/src/utils/index.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 60 out of 60 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Lite PR → Apply remaining freedback from PR#195
Summary
ensawards.org\src\components\referral-awards-program\ReferralLinkForm.tsxWhy
Testing
typecheck,lint, andtestcommands locally to ensure that the migration didn't break anything, and later confirmed that in our CI workflowNotes for Reviewer (Optional)
NamewithInterpretedNameacross the entire appPre-Review Checklist (Blocking)