dev: Add deposit-address best practice#233
Merged
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughRenames the ChangesDeposit-Addresses Best Practice + ENS Resolution Category Rename
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 |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR introduces the new deposit-addresses ENS resolution best practice (including technical details + acceptance tests), while also refactoring the “overview” field across the codebase from useCaseSummary to ensBestPracticeOverview and renaming the ENSv2 readiness category to the broader resolution category (with redirects).
Changes:
- Add the new
deposit-addressesbest practice under the ENS Resolution category, including 8 acceptance tests and UI-ready technical detail content. - Rename
useCaseSummary→ensBestPracticeOverviewacross types, UI components, test utils, and existing best practices. - Rename the ENSv2 readiness category slug/id to
resolutionand add Astro redirects for legacy URLs.
Reviewed changes
Copilot reviewed 48 out of 52 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ensawards.org/src/components/molecules/technicalDetails/shared.tsx | Renames the rendered “overview” component to use ensBestPracticeOverview. |
| ensawards.org/src/components/molecules/technicalDetails/bestPractice/index.tsx | Updates best-practice details UI to render EnsBestPracticeOverview. |
| ensawards.org/src/components/molecules/technicalDetails/benchmark/index.astro | Updates benchmark details UI to render EnsBestPracticeOverview. |
| ensawards.org/data/shared/test-utils.tsx | Updates mock best practice technical details to use ensBestPracticeOverview. |
| ensawards.org/data/ens-best-practices/types.ts | Renames the technical-details field in the core type definition. |
| ensawards.org/data/ens-best-practices/styles.ts | Adds shared Tailwind styles for inline “code-like” spans in technical details content. |
| ensawards.org/data/ens-best-practices/ens-resolution/index.ts | Renames category from ENSv2 readiness to “ENS Resolution” (id/slug/name/description). |
| ensawards.org/data/ens-best-practices/ens-resolution/ensv2-ready-resolution/technicalDetails.tsx | Updates technical details to new field name and shared code-span styling. |
| ensawards.org/data/ens-best-practices/ens-resolution/ensv2-ready-resolution/index.ts | Switches best practice category reference to ENSResolutionCategory. |
| ensawards.org/data/ens-best-practices/ens-resolution/ensv2-ready-resolution/images/correctly-resolve-ensv2-test-name-address-not-applicable-example.gif | Adds/updates benchmark example media. |
| ensawards.org/data/ens-best-practices/ens-resolution/ensv2-ready-resolution/images/correctly-resolve-ensv2-test-name-address-fail-example.png | Adds/updates benchmark example media. |
| ensawards.org/data/ens-best-practices/ens-resolution/deposit-addresses/technicalDetails.tsx | Adds the new best practice’s overview, recommendations, and 8 acceptance tests (with placeholders). |
| ensawards.org/data/ens-best-practices/ens-resolution/deposit-addresses/index.ts | Registers the new deposit-addresses best practice under ENS Resolution. |
| ensawards.org/data/ens-best-practices/contract-naming/name-your-smart-contracts/technicalDetails.tsx | Renames the technical-details overview field to ensBestPracticeOverview. |
| ensawards.org/data/ens-best-practices/contract-naming/display-named-smart-contracts-mainnet/technicalDetails.tsx | Renames the technical-details overview field to ensBestPracticeOverview. |
| ensawards.org/data/ens-best-practices/contract-naming/display-named-smart-contracts-l2-chains/technicalDetails.tsx | Renames the technical-details overview field to ensBestPracticeOverview. |
| ensawards.org/data/apps/zerion-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/worldapp-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/walletchan-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/uniswap-defi/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/trust-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/status-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/safe-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/robinhood-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/robinhood-exchange/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/readyx-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/rainbow-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/rabby-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/phantom-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/okx-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/okx-exchange/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/metamask-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/lido-defi/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/kraken-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/kraken-exchange/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/gemini-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/frame-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/etherscan-explorer/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/cryptocom-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/cryptocom-exchange/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/coinbase-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/coinbase-exchange/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/blockscout-explorer/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/binance-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/binance-exchange/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/ambire-wallet/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/aave-defi/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/data/apps/1inch-defi/benchmarks/index.tsx | Adds deposit-addresses benchmark keys (currently all undefined). |
| ensawards.org/astro.config.mjs | Adds redirects from legacy ensv2-readiness URLs to the new resolution paths. |
| CONTRIBUTING.md | Updates contributor documentation to reflect ensBestPracticeOverview field name. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+58
to
+67
| "deposit-addresses": { | ||
| "correctly-resolve-direct-onchain-subname-address": undefined, | ||
| "correctly-resolve-names-requiring-normalization": undefined, | ||
| "correctly-implement-ccip-read-for-eth-subnames": undefined, | ||
| "correctly-implement-ccip-read-for-offchain-dns-names": undefined, | ||
| "correctly-resolve-names-for-different-evm-chains": undefined, | ||
| "correctly-resolve-names-for-bitcoin": undefined, | ||
| "correctly-resolve-names-for-solana": undefined, | ||
| "correctly-handle-resolution-for-chains-with-invalid-address-formatting": undefined, | ||
| }, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/data/apps/ambire-wallet/benchmarks/index.tsx`:
- Around line 70-79: The deposit-addresses benchmark object contains multiple
test entries (correctly-resolve-direct-onchain-subname-address,
correctly-resolve-names-requiring-normalization,
correctly-implement-ccip-read-for-eth-subnames,
correctly-implement-ccip-read-for-offchain-dns-names,
correctly-resolve-names-for-different-evm-chains,
correctly-resolve-names-for-bitcoin, correctly-resolve-names-for-solana,
correctly-handle-resolution-for-chains-with-invalid-address-formatting) that are
all set to undefined. Replace each undefined value with the actual benchmark
test implementation or function that performs the respective test, ensuring each
benchmark has executable logic rather than placeholder values.
In
`@ensawards.org/data/ens-best-practices/ens-resolution/deposit-addresses/technicalDetails.tsx`:
- Line 200: Replace all placeholder TODO comments with actual, concrete examples
of applications that pass the acceptance tests being documented. The TODO
comments appear at multiple locations throughout the technicalDetails.tsx file
(lines 200, 220, 272, 293, 360, 380, 446, 466, 534, 554, 619, 639, 703, 723,
792, 813) and each contains the phrase "Insert true example of an app passing
this acceptance test when performing the benchmarks". For each occurrence,
replace the TODO comment with a real-world example or case study that
demonstrates an actual application successfully passing the corresponding
acceptance test, ensuring the benchmark evidence is complete and user-facing
before merge.
🪄 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: f32c59f7-c6e6-42f6-b7db-efeeef7a4514
⛔ Files ignored due to path filters (4)
ensawards.org/data/ens-best-practices/ens-resolution/deposit-addresses/images/acceptance-test-example-placeholder.pngis excluded by!**/*.pngensawards.org/data/ens-best-practices/ens-resolution/ensv2-ready-resolution/images/correctly-resolve-ensv2-test-name-address-fail-example.pngis excluded by!**/*.pngensawards.org/data/ens-best-practices/ens-resolution/ensv2-ready-resolution/images/correctly-resolve-ensv2-test-name-address-not-applicable-example.gifis excluded by!**/*.gifensawards.org/data/ens-best-practices/ens-resolution/ensv2-ready-resolution/images/correctly-resolve-ensv2-test-name-address-pass-example.gifis excluded by!**/*.gif
📒 Files selected for processing (48)
CONTRIBUTING.mdensawards.org/astro.config.mjsensawards.org/data/apps/1inch-defi/benchmarks/index.tsxensawards.org/data/apps/aave-defi/benchmarks/index.tsxensawards.org/data/apps/ambire-wallet/benchmarks/index.tsxensawards.org/data/apps/binance-exchange/benchmarks/index.tsxensawards.org/data/apps/binance-wallet/benchmarks/index.tsxensawards.org/data/apps/blockscout-explorer/benchmarks/index.tsxensawards.org/data/apps/coinbase-exchange/benchmarks/index.tsxensawards.org/data/apps/coinbase-wallet/benchmarks/index.tsxensawards.org/data/apps/cryptocom-exchange/benchmarks/index.tsxensawards.org/data/apps/cryptocom-wallet/benchmarks/index.tsxensawards.org/data/apps/etherscan-explorer/benchmarks/index.tsxensawards.org/data/apps/frame-wallet/benchmarks/index.tsxensawards.org/data/apps/gemini-wallet/benchmarks/index.tsxensawards.org/data/apps/kraken-exchange/benchmarks/index.tsxensawards.org/data/apps/kraken-wallet/benchmarks/index.tsxensawards.org/data/apps/lido-defi/benchmarks/index.tsxensawards.org/data/apps/metamask-wallet/benchmarks/index.tsxensawards.org/data/apps/okx-exchange/benchmarks/index.tsxensawards.org/data/apps/okx-wallet/benchmarks/index.tsxensawards.org/data/apps/phantom-wallet/benchmarks/index.tsxensawards.org/data/apps/rabby-wallet/benchmarks/index.tsxensawards.org/data/apps/rainbow-wallet/benchmarks/index.tsxensawards.org/data/apps/readyx-wallet/benchmarks/index.tsxensawards.org/data/apps/robinhood-exchange/benchmarks/index.tsxensawards.org/data/apps/robinhood-wallet/benchmarks/index.tsxensawards.org/data/apps/safe-wallet/benchmarks/index.tsxensawards.org/data/apps/status-wallet/benchmarks/index.tsxensawards.org/data/apps/trust-wallet/benchmarks/index.tsxensawards.org/data/apps/uniswap-defi/benchmarks/index.tsxensawards.org/data/apps/walletchan-wallet/benchmarks/index.tsxensawards.org/data/apps/worldapp-wallet/benchmarks/index.tsxensawards.org/data/apps/zerion-wallet/benchmarks/index.tsxensawards.org/data/ens-best-practices/contract-naming/display-named-smart-contracts-l2-chains/technicalDetails.tsxensawards.org/data/ens-best-practices/contract-naming/display-named-smart-contracts-mainnet/technicalDetails.tsxensawards.org/data/ens-best-practices/contract-naming/name-your-smart-contracts/technicalDetails.tsxensawards.org/data/ens-best-practices/ens-resolution/deposit-addresses/index.tsensawards.org/data/ens-best-practices/ens-resolution/deposit-addresses/technicalDetails.tsxensawards.org/data/ens-best-practices/ens-resolution/ensv2-ready-resolution/index.tsensawards.org/data/ens-best-practices/ens-resolution/ensv2-ready-resolution/technicalDetails.tsxensawards.org/data/ens-best-practices/ens-resolution/index.tsensawards.org/data/ens-best-practices/styles.tsensawards.org/data/ens-best-practices/types.tsensawards.org/data/shared/test-utils.tsxensawards.org/src/components/molecules/technicalDetails/benchmark/index.astroensawards.org/src/components/molecules/technicalDetails/bestPractice/index.tsxensawards.org/src/components/molecules/technicalDetails/shared.tsx
Comment on lines
+70
to
+79
| "deposit-addresses": { | ||
| "correctly-resolve-direct-onchain-subname-address": undefined, | ||
| "correctly-resolve-names-requiring-normalization": undefined, | ||
| "correctly-implement-ccip-read-for-eth-subnames": undefined, | ||
| "correctly-implement-ccip-read-for-offchain-dns-names": undefined, | ||
| "correctly-resolve-names-for-different-evm-chains": undefined, | ||
| "correctly-resolve-names-for-bitcoin": undefined, | ||
| "correctly-resolve-names-for-solana": undefined, | ||
| "correctly-handle-resolution-for-chains-with-invalid-address-formatting": undefined, | ||
| }, |
There was a problem hiding this comment.
Benchmark stubs are still incomplete for merge readiness.
All deposit-addresses tests are undefined, so this app has no executable benchmark outcomes for the new best practice yet. That conflicts with the stated pre-merge requirement to include all necessary benchmark components.
🤖 Prompt for 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.
In `@ensawards.org/data/apps/ambire-wallet/benchmarks/index.tsx` around lines 70 -
79, The deposit-addresses benchmark object contains multiple test entries
(correctly-resolve-direct-onchain-subname-address,
correctly-resolve-names-requiring-normalization,
correctly-implement-ccip-read-for-eth-subnames,
correctly-implement-ccip-read-for-offchain-dns-names,
correctly-resolve-names-for-different-evm-chains,
correctly-resolve-names-for-bitcoin, correctly-resolve-names-for-solana,
correctly-handle-resolution-for-chains-with-invalid-address-formatting) that are
all set to undefined. Replace each undefined value with the actual benchmark
test implementation or function that performs the respective test, ensuring each
benchmark has executable logic rather than placeholder values.
Member
Author
There was a problem hiding this comment.
These benchmarks will be filled/performed in the few following days.
There was a problem hiding this comment.
@Y3drk Understood, thanks for the heads-up! Since this is a draft PR and the merge pre-condition explicitly requires all benchmarks to be included before merging, the undefined placeholders are fine for now. Just make sure to flip the PR out of draft once all eight acceptance test benchmarks for deposit-addresses are filled in across all tracked apps. I'll keep this comment open until then.
🧠 Learnings used
Learnt from: Y3drk
Repo: namehash/ensawards PR: 117
File: ensawards.org/src/components/atoms/icons/ens-integrating-entities/apps/CoinbaseWalletIcon.tsx:11-12
Timestamp: 2026-02-04T14:50:19.321Z
Learning: In TSX components, when a component applies inline styles (style={{...}}) and later spreads props ({...props}) after, allow that spread to override the inline styles if this ordering is intentional and consistent with the project’s styling approach (e.g., Tailwind via className). Document and enforce this convention in code reviews to prevent accidental overrides and ensure the behavior is intentional across components.
Learnt from: Y3drk
Repo: namehash/ensawards PR: 204
File: ensawards.org/data/apps/metamask-wallet/benchmarks/index.tsx:25-25
Timestamp: 2026-05-12T06:57:59.413Z
Learning: In benchmark files like `ensawards.org/data/apps/<app>/benchmarks/index.tsx`, don’t flag identical `parseTimestamp(...)` results as copy-paste errors when they appear across multiple benchmark entries for the same contributor. For `lastUpdated` in these benchmarks, only the calendar date is meaningful; exact hour/minute/second precision is not significant. It’s acceptable for entries sharing the same contributor/date to have the same normalized timestamp—only flag if the timestamps differ in a way that indicates an unintended date mismatch (e.g., wrong day for the contributor).
Learnt from: Y3drk
Repo: namehash/ensawards PR: 204
File: ensawards.org/data/apps/rainbow-wallet/benchmarks/index.tsx:19-24
Timestamp: 2026-05-12T06:59:04.010Z
Learning: In `ensawards.org/data/apps/*/benchmarks/index.tsx`, do not treat intentionally preserved, commented-out benchmark blocks (e.g., `
Learnt from: Y3drk
Repo: namehash/ensawards PR: 204
File: ensawards.org/src/components/molecules/technicalDetails/benchmark/acceptanceTestResultDetails.tsx:19-24
Timestamp: 2026-05-12T16:07:54.248Z
Learning: In this TypeScript/React codebase, if a contributions-like field is typed as a non-empty tuple (e.g., `[Contribution, ...Contribution[]]`), it is guaranteed at compile time to have at least one element. In code reviews, do not flag potential empty-array risks for that field, and rely on the non-empty tuple type (and any existing unit tests that assert the runtime invariant) instead.
lightwalker-eth
left a comment
lightwalker-eth
left a comment
Member
There was a problem hiding this comment.
Great to see the updates here! This is a great foundation 👍 @Y3drk @sevenzing Please see some suggestions for next steps 👍
| benefitFromUsingEns, | ||
| implementationRecommendations, | ||
| acceptanceTests: [ | ||
| acceptanceTest1, |
Member
There was a problem hiding this comment.
Something is causing the acceptance tests to render in the UI in a different order then they are defined here. Suggest to prioritize updating this as soon as possible as otherwise it may complicate the next steps to populate this content.
Member
Author
There was a problem hiding this comment.
This happens due to the fact that we're currently sorting acceptance tests and their benchmarks based on the alphabetical order of their slugs to ensure that, no matter how the user defines the order of benchmarks in the apps/x/benchmarks/index file the order will always be the same.
I'll remove the current sorting and try to replace it with something better. If it doesn't work, we can still ensure the order manually, which isn't great, but it's not terrible either.
Member
Author
There was a problem hiding this comment.
UPDATE: Added an optional order field to the AcceptanceTest type. This guides the sorting process and will ensure the correct order
sevenzing
approved these changes
Jun 19, 2026
Greptile SummaryThis PR introduces the
Confidence Score: 4/5Safe to merge once the expected address for The structural changes (category rename, field rename, sort logic, redirects, stub benchmarks) are all consistent and well-tested. The one concern is that ensawards.org/data/ens-best-practices/resolution/deposit-addresses/technicalDetails.tsx — confirm the expected address for Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[AcceptanceTest definition\noptional order field] --> B{Has order?}
B -- Yes --> C[Sort by order ascending]
B -- No --> D[Sort last\norder = +Infinity]
C --> E{Tie on order?}
D --> E
E -- Yes --> F[Tiebreak: localeCompare slug]
E -- No --> G[Use numeric order]
F --> H[Sorted acceptance tests list]
G --> H
subgraph deposit-addresses [deposit-addresses best practice]
T1[test 1: direct onchain\norder=0]
T2[test 2: normalization\norder=1]
T3[test 3: CCIP-read .eth\norder=2]
T4[test 4: CCIP-read DNS\norder=3]
T5[test 5: EVM multi-chain\norder=4]
T6[test 6: Bitcoin\norder=5]
T7[test 7: Solana\norder=6]
T8[test 8: invalid address\norder=7]
end
deposit-addresses --> A
%%{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[AcceptanceTest definition\noptional order field] --> B{Has order?}
B -- Yes --> C[Sort by order ascending]
B -- No --> D[Sort last\norder = +Infinity]
C --> E{Tie on order?}
D --> E
E -- Yes --> F[Tiebreak: localeCompare slug]
E -- No --> G[Use numeric order]
F --> H[Sorted acceptance tests list]
G --> H
subgraph deposit-addresses [deposit-addresses best practice]
T1[test 1: direct onchain\norder=0]
T2[test 2: normalization\norder=1]
T3[test 3: CCIP-read .eth\norder=2]
T4[test 4: CCIP-read DNS\norder=3]
T5[test 5: EVM multi-chain\norder=4]
T6[test 6: Bitcoin\norder=5]
T7[test 7: Solana\norder=6]
T8[test 8: invalid address\norder=7]
end
deposit-addresses --> A
Reviews (3): Last reviewed commit: "Apply AI agents' suggestions, pt.2" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
sevenzing
reviewed
Jun 19, 2026
| // Sort acceptance tests by order (ascending) to ensure consistent display order | ||
| // across best practice details and benchmark details pages. | ||
| // The tiebreaker is the alphabetical order of the acceptance test slug. | ||
| const sortedAcceptanceTestBenchmarks = [...Object.entries(acceptanceTestBenchmarks)].sort( |
Member
There was a problem hiding this comment.
do we really need to sort it here? maybe we just keep the same order as defined during creation of BestPracticeTechnicalDetails ? looks like we always either have 1 test or the order field is same as order in array acceptanceTests
Member
Author
There was a problem hiding this comment.
@sevenzing In my opinion, this is necessary because when external contributors contribute benchmarks, we have no guarantee that they will keep the same order as we do, and we want to keep the display consistent between the best practice details page and the benchmark report page
Member
There was a problem hiding this comment.
let's just follow order from like
const technicalDetails = {
useCaseSummary,
benefitFromUsingEns,
implementationRecommendations,
acceptanceTests: [acceptanceTest1],
} as const satisfies BestPracticeTechnicalDetails;
Member
Author
sevenzing
added a commit
that referenced
this pull request
Jun 22, 2026
* dev: Add `deposit-address` best practice (#233) * Action Item 1 * Action Item 2 * Apply 06/18/26 GitHub review feedback & AI agents' suggestions, pt.1 * Apply AI agents' suggestions, pt.2 * feat: etherscan acceptance test (#234) * Action Item 1 * Action Item 2 * Apply 06/18/26 GitHub review feedback & AI agents' suggestions, pt.1 * Add etherscan acceptance test for deposit address * revert mockedExampleBenchmark * delete file * oops * add not-applicable --------- Co-authored-by: y3drk <ziebura.jedrzej@gmail.com> * feat: ATs for blockscout and 1inch (#235) * add skill to rearrange app benchmark * rearrange blockscout-explorer to new structure * add blockscout-explorer case * p lint * rearrange 1inch-defi benchmark to new structure * update skill to use values from technicalDetails * generate benchmark for 1inch * p lint * update benchmark of blockscout and etherscan accordingly * apply fixes for AI review * feat: Benchmark wallets on `deposit-addresses` best practice (#236) * Action Item 1 * Action Item 2 * Apply 06/18/26 GitHub review feedback & AI agents' suggestions, pt.1 * Add benchmarks for ambire + example pass for AT1 and example fail for AT4 * Add benchmarks for binance wallet + example fail for AT1 * Add benchmarks for coinbase wallet * Add benchmarks for readyx * Add benchmarks for world app wallet * Add benchmarks for robinhood wallet * Adjust benchmarked apps to the newly established structure * Add benchmarks for zerion * Add benchmarks for walletchan * unify code with my PR * p lint * add more images or at-2 in zerion --------- Co-authored-by: sevenzing <lymarenko.lev@gmail.com> * feat: add builders for notes to decrease boilerplate (#237) * rename ATs and use notes.tsx to remove boilerplate * rename images * add indexed key * apply greptile feedback * feat: rearrange benchmarks folders for all apps (#238) * rearrange to new structure * remove useless skill * p lint * feat: add benchmarks for aave, uniswap and lido (#239) * add aave and uniswap * fix uniswap case * add lido * yeah i forgot at7 and 8 * just rename deposit-address -> deposit-addresses * feat: Benchmark wallets on `deposit-addresses` best practice, batch 2 (#241) * Add benchmarks for okx wallet & crypto.com wallet + add bitcoin benchmark for coinbase wallet + add example passes for ATs 5-8 * Add benchmarks for phantom & rabby + add example pass for AT2 and example fails for ATs 3 & 5 * Update method context notes for AT5 pass/fail template * Add benchmarks for safe{wallet} + add extra explanations for failed benchmarks + add example pass for AT3 * Add benchmarks for gemini wallet & metamask + update CONTRIBUTING.md + add example fails for ATs 2 & 8 * Apply AI agents' suggestions, pt.1 * feat: add benchmarks for `binance`, `bybit` and `okx` (#240) * benchmark binance,bybit,okx * add ensv2 for bybit * reexport icon for bybit * add benchmarks for raibow-wallet,status-wallet,trust-wallet (#243) * add benchmarks for raibow-wallet,status-wallet,trust-wallet * rename to deposit-addresses * Add benchmarks for exchanges: coinbase, crypto.com and robinhood + add example fails for ATs 6 & 7 (#242) * add explain why AT8 is failed in example (#245) * small fix --------- Co-authored-by: Jędrzej Ziebura <ziebura.jedrzej@gmail.com>
This was referenced Jun 23, 2026
This was referenced Jul 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add
deposit-addressbest practiceSummary
Why
Testing
typecheck,lint, andtestcommands locally to ensure that the migration didn't break anything, and later confirmed that in our CI workflowPre-Review Checklist (Blocking)