feat: Benchmark exchanges on deposit-addresses best practice, batch 2 - #242
Conversation
…d example fails for ATs 6 & 7
|
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:
✨ 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 |
There was a problem hiding this comment.
Pull request overview
This PR adds deposit-addresses best-practice benchmark results for three exchange apps (Coinbase Exchange, Crypto.com Exchange, Robinhood Exchange) and updates the shared technical details page to replace placeholder “example fail” media for Acceptance Tests 6 & 7 with real examples.
Changes:
- Added
deposit-addressesbenchmark implementations for Coinbase Exchange, Crypto.com Exchange, and Robinhood Exchange (including proof assets and standardized notes). - Switched each of those apps’ benchmark registries to import
./resolution/deposit-addresses(and removed the old placeholderdeposit-addressbenchmark files). - Replaced AT6/AT7 example-fail placeholders in
technicalDetails.tsxwith real images and improvedalttext.
Reviewed changes
Copilot reviewed 10 out of 22 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| ensawards.org/data/ens-best-practices/resolution/deposit-addresses/technicalDetails.tsx | Replaces AT6/AT7 placeholder example-fail media with real assets + updated alt text and contributions. |
| ensawards.org/data/apps/robinhood-exchange/benchmarks/resolution/deposit-addresses/index.tsx | Adds Robinhood Exchange deposit-addresses benchmark results + proof references. |
| ensawards.org/data/apps/robinhood-exchange/benchmarks/resolution/deposit-address/index.tsx | Removes the old placeholder benchmark file (all undefined). |
| ensawards.org/data/apps/robinhood-exchange/benchmarks/index.tsx | Updates registry import to ./resolution/deposit-addresses and registers under "deposit-addresses". |
| ensawards.org/data/apps/cryptocom-exchange/benchmarks/resolution/deposit-addresses/index.tsx | Adds Crypto.com Exchange deposit-addresses benchmark results + proof references. |
| ensawards.org/data/apps/cryptocom-exchange/benchmarks/resolution/deposit-address/index.tsx | Removes the old placeholder benchmark file (all undefined). |
| ensawards.org/data/apps/cryptocom-exchange/benchmarks/index.tsx | Updates registry import to ./resolution/deposit-addresses and registers under "deposit-addresses". |
| ensawards.org/data/apps/coinbase-exchange/benchmarks/resolution/deposit-addresses/index.tsx | Adds Coinbase Exchange deposit-addresses benchmark results (with AT6 intentionally left undefined / pending) + proof references. |
| ensawards.org/data/apps/coinbase-exchange/benchmarks/resolution/deposit-address/index.tsx | Removes the old placeholder benchmark file (all undefined). |
| ensawards.org/data/apps/coinbase-exchange/benchmarks/index.tsx | Updates registry import to ./resolution/deposit-addresses and registers under "deposit-addresses". |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryThis PR fills in the
Confidence Score: 4/5Safe to merge; all benchmark logic follows established patterns and the only open item (Coinbase AT6) is explicitly acknowledged in the PR description. The changes are data/content additions that closely follow patterns already established across a dozen other benchmark files. The AT8 Not Applicable cascade for Coinbase Exchange matches identical usage in Coinbase Wallet, Gemini, and Phantom. The one gap — at06-resolve-bitcoin-address: undefined — is intentional and called out in the PR description, but there is no inline comment in the code to signal future contributors, which is a minor documentation concern. ensawards.org/data/apps/coinbase-exchange/benchmarks/resolution/deposit-addresses/index.tsx — the undefined AT6 entry has no inline TODO comment explaining why it was left pending. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
AT1["AT1: Resolve onchain name"] -->|Pass| AT2["AT2: Normalization"]
AT1 -->|Fail - ENS not supported| NA_ALL["AT2–AT8: Not Applicable\n(ENS not supported at all)"]
AT2 --> AT3["AT3: Offchain ETH subname"]
AT3 --> AT4["AT4: Offchain DNS name"]
AT4 --> AT5["AT5: Other EVM chain (Base)"]
AT5 -->|Pass| AT6["AT6: Bitcoin address"]
AT5 -->|Fail - wrong chain| NA_BASE["AT8: Not Applicable\n(no Base support)"]
AT6 -->|Pending - undefined| PENDING["AT6: undefined\n(Coinbase Exchange only)"]
AT6 --> AT7["AT7: Solana address"]
AT7 --> AT8["AT8: Invalid EVM address format (Base)"]
style NA_ALL fill:#f9c,stroke:#c99
style NA_BASE fill:#f9c,stroke:#c99
style PENDING fill:#ffc,stroke:#cc9
%%{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
AT1["AT1: Resolve onchain name"] -->|Pass| AT2["AT2: Normalization"]
AT1 -->|Fail - ENS not supported| NA_ALL["AT2–AT8: Not Applicable\n(ENS not supported at all)"]
AT2 --> AT3["AT3: Offchain ETH subname"]
AT3 --> AT4["AT4: Offchain DNS name"]
AT4 --> AT5["AT5: Other EVM chain (Base)"]
AT5 -->|Pass| AT6["AT6: Bitcoin address"]
AT5 -->|Fail - wrong chain| NA_BASE["AT8: Not Applicable\n(no Base support)"]
AT6 -->|Pending - undefined| PENDING["AT6: undefined\n(Coinbase Exchange only)"]
AT6 --> AT7["AT7: Solana address"]
AT7 --> AT8["AT8: Invalid EVM address format (Base)"]
style NA_ALL fill:#f9c,stroke:#c99
style NA_BASE fill:#f9c,stroke:#c99
style PENDING fill:#ffc,stroke:#cc9
Reviews (1): Last reviewed commit: "Add benchmarks for exchanges: coinbase, ..." | Re-trigger Greptile |
Greptile SummaryThis PR adds
Confidence Score: 5/5Safe to merge — all changes are benchmark data entries and image swaps with no runtime logic. The changes are purely additive data: three exchange benchmark modules, six proof images, and a straightforward placeholder-image swap in technicalDetails.tsx. All three new benchmark files consistently follow the patterns already established by Coinbase Wallet, Gemini Wallet, and other exchanges. The acceptance-test-example-placeholder.png import has been fully cleaned up with no remaining references. The Coinbase AT6 undefined (pending) is an intentional and acknowledged editorial hold, not a code defect. No files require special attention. The Coinbase Exchange AT6 entry is intentionally undefined and will need a follow-up PR once the minimum send threshold can be met. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph CoinbaseExchange["Coinbase Exchange — deposit-addresses"]
CE_AT1["AT1: Pass ✅\nonchain .eth resolves correctly"]
CE_AT2["AT2: Pass ✅\nnormalization works"]
CE_AT3["AT3: Pass ✅\noffchain .eth subname (CCIP-Read)"]
CE_AT4["AT4: Fail ❌\noffchain DNS name"]
CE_AT5["AT5: Fail ❌\nother EVM chain (Base)"]
CE_AT6["AT6: Pending ⏳\nBitcoin (untestable — min threshold)"]
CE_AT7["AT7: Fail ❌\nSolana address"]
CE_AT8["AT8: N/A 🚫\n(prereq AT5 failed on Base)"]
end
subgraph CryptoCom["Crypto.com Exchange — deposit-addresses"]
CC_AT1["AT1: Fail ❌\nENS not supported at all"]
CC_REST["AT2–AT8: NotApplicable 🚫\n(prereq AT1 failed)"]
CC_AT1 --> CC_REST
end
subgraph Robinhood["Robinhood Exchange — deposit-addresses"]
RH_AT1["AT1: Fail ❌\nENS not supported at all"]
RH_REST["AT2–AT8: NotApplicable 🚫\n(prereq AT1 failed)"]
RH_AT1 --> RH_REST
end
subgraph TechDetails["technicalDetails.tsx"]
TD1["AT6 example-fail image\nPlaceholder → Phantom/Bitcoin screenshot"]
TD2["AT7 example-fail image\nPlaceholder → Coinbase Wallet/Solana screenshot"]
end
%%{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
subgraph CoinbaseExchange["Coinbase Exchange — deposit-addresses"]
CE_AT1["AT1: Pass ✅\nonchain .eth resolves correctly"]
CE_AT2["AT2: Pass ✅\nnormalization works"]
CE_AT3["AT3: Pass ✅\noffchain .eth subname (CCIP-Read)"]
CE_AT4["AT4: Fail ❌\noffchain DNS name"]
CE_AT5["AT5: Fail ❌\nother EVM chain (Base)"]
CE_AT6["AT6: Pending ⏳\nBitcoin (untestable — min threshold)"]
CE_AT7["AT7: Fail ❌\nSolana address"]
CE_AT8["AT8: N/A 🚫\n(prereq AT5 failed on Base)"]
end
subgraph CryptoCom["Crypto.com Exchange — deposit-addresses"]
CC_AT1["AT1: Fail ❌\nENS not supported at all"]
CC_REST["AT2–AT8: NotApplicable 🚫\n(prereq AT1 failed)"]
CC_AT1 --> CC_REST
end
subgraph Robinhood["Robinhood Exchange — deposit-addresses"]
RH_AT1["AT1: Fail ❌\nENS not supported at all"]
RH_REST["AT2–AT8: NotApplicable 🚫\n(prereq AT1 failed)"]
RH_AT1 --> RH_REST
end
subgraph TechDetails["technicalDetails.tsx"]
TD1["AT6 example-fail image\nPlaceholder → Phantom/Bitcoin screenshot"]
TD2["AT7 example-fail image\nPlaceholder → Coinbase Wallet/Solana screenshot"]
end
Reviews (2): Last reviewed commit: "Add benchmarks for exchanges: coinbase, ..." | Re-trigger Greptile |
lightwalker-eth
left a comment
There was a problem hiding this comment.
@Y3drk Looks great! 🫡
* 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>
Lite PR → Benchmark exchanges on
deposit-addressesbest practice, batch 2Summary
exchangeondeposit-addressesbest practice.NOTE: Had trouble with benchmarking AT6 (Bitcoin, Coinbase's minimal sending threshold was too high for me to test). Left it as pending to not block myselfensawards.org/data/ens-best-practices/resolution/deposit-addresses/technicalDetails.tsxand removed the placeholder image from the related directoryWhy
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)