-
Notifications
You must be signed in to change notification settings - Fork 66
feat: add tokenlist and generate docs #838
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@gregdhill has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 35 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (1)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughToken registry docs moved from a static Markdown page to a dynamic MDX component sourcing a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant MDX as TokenTable (MDX)
participant TokenList as token-list.json
participant Verify as verify-tokenlist.ts
participant SDK as SDK Gateway
participant RPC as On-Chain RPC
User->>MDX: Open Token Registry page
MDX->>TokenList: import tokens
TokenList-->>MDX: token array
MDX->>MDX: sort, shorten addresses, build links
MDX-->>User: render token table (Name, Symbol, L1/L2, Bridges)
Note over Verify,RPC: Offline verification flow
Verify->>TokenList: read tokens & validate schema
Verify->>RPC: query ERC-20 name/symbol/decimals per token
RPC-->>Verify: on-chain metadata
Verify->>Verify: compare fields, detect duplicates
Verify-->>User: report & exit code
Note over SDK,TokenList: Runtime SDK usage
SDK->>TokenList: load token-list for TOKENS
SDK->>SDK: attach STORAGE_SLOTS_MAP where available
SDK->>SDK: getTokenSlots(tokenAddress) -- (originChain removed)
SDK-->>User: use token lookups for gateway ops
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
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 |
Summary of ChangesHello @gregdhill, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the management and presentation of token information by introducing a standardized, machine-readable token list and automating its integration into the documentation. It also adds a robust validation mechanism to ensure the accuracy and consistency of token data, streamlining future updates and reducing manual errors. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request is a great improvement, replacing a static Markdown table of tokens with a dynamically generated one from a token-list.json file. The addition of a validation script (verify-tokenlist.ts) is an excellent step towards ensuring data integrity. My review includes suggestions to improve the React component's implementation, fix a discrepancy between the documentation and the data, address potentially incomplete token information, and make the validation script stricter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
sdk/src/scripts/verify-tokenlist.ts (1)
40-53: Consider validating bridgeInfo addresses for checksum compliance.The script validates checksums for main token addresses (line 48-49) but doesn't validate addresses within
extensions.bridgeInfo(e.g.,tokenAddress,originBridgeAddress,destBridgeAddress). Since token-list.json contains lowercase addresses in these fields, they won't be caught by this validation.Extend validation to check bridgeInfo addresses:
// After line 136, add: if (token.extensions?.bridgeInfo) { for (const [chainId, info] of Object.entries(token.extensions.bridgeInfo)) { if (info.tokenAddress && !(await validateTokenAddress(info.tokenAddress))) { issues.push(`Invalid or non-checksummed bridgeInfo.${chainId}.tokenAddress`); } if (info.originBridgeAddress && !(await validateTokenAddress(info.originBridgeAddress))) { issues.push(`Invalid or non-checksummed bridgeInfo.${chainId}.originBridgeAddress`); } if (info.destBridgeAddress && !(await validateTokenAddress(info.destBridgeAddress))) { issues.push(`Invalid or non-checksummed bridgeInfo.${chainId}.destBridgeAddress`); } } }Also applies to: 91-139
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/docs/docs/reference/token-registry/index.md(0 hunks)docs/docs/docs/reference/token-registry/index.mdx(1 hunks)sdk/src/scripts/verify-tokenlist.ts(1 hunks)token-list.json(1 hunks)
💤 Files with no reviewable changes (1)
- docs/docs/docs/reference/token-registry/index.md
🧰 Additional context used
📓 Path-based instructions (1)
docs/docs/docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (docs/CLAUDE.md)
docs/docs/docs/**/*.{md,mdx}: Use Docusaurus admonition syntax for notes::::infofor informational notes,:::warningfor warnings,:::dangerfor critical warnings,:::tipfor helpful tips
Do NOT use markdown blockquotes (>) for notes
Contract addresses should be sorted alphabetically within their sections
Keep all contracts under a single hierarchy (no sub-sections for different deployment types)
Include both implementation and proxy contracts when available
Format contract entries as:- ContractName: [address](explorer-link)
Always maintain alphabetical ordering when adding new contracts
Distinguish between L1 (Ethereum) and L2 (BOB) contracts clearly
Files:
docs/docs/docs/reference/token-registry/index.mdx
🧠 Learnings (1)
📚 Learning: 2025-07-31T14:44:59.656Z
Learnt from: CR
Repo: bob-collective/bob PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2025-07-31T14:44:59.656Z
Learning: Applies to docs/docs/docs/**/*.{md,mdx} : Distinguish between L1 (Ethereum) and L2 (BOB) contracts clearly
Applied to files:
docs/docs/docs/reference/token-registry/index.mdx
🧬 Code graph analysis (1)
sdk/src/scripts/verify-tokenlist.ts (1)
sdk/src/gateway/types/token.ts (1)
Token(7-14)
🪛 GitHub Actions: SDK
sdk/src/scripts/verify-tokenlist.ts
[error] 1-1: Prettier formatting check failed. Code style issues found in 'src/scripts/verify-tokenlist.ts'; run 'npx prettier --write src/scripts/verify-tokenlist.ts' to fix. The CI step 'npx prettier --check {examples,src,test}' exited with code 1.
🪛 Gitleaks (8.28.0)
docs/docs/docs/reference/token-registry/index.mdx
[high] 144-144: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
token-list.json
[high] 20-20: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 37-37: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 69-69: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 86-86: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 103-103: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 160-160: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 177-177: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 194-194: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 211-211: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 228-228: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 245-245: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 262-262: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 279-279: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 296-296: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 313-313: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 330-330: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 347-347: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 364-364: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 381-381: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 398-398: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (7)
token-list.json (2)
1-429: Gitleaks false positives can be safely ignored.The static analysis tool flagged multiple lines as containing "Generic API Keys," but these are actually Ethereum contract addresses (e.g.,
0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2), not secrets. These warnings are false positives and can be dismissed.
414-428: Duplicate symbol "satUSD" confirmed — verify if intentional.The duplicate symbol "satUSD" is verified: both tokens exist on chainId 60808 with different addresses (V1 and V2). While this may be intentional for versioning, confirm whether your token resolution logic can handle symbol collisions on the same chain. If symbol-based lookups are used elsewhere, consider renaming to distinct symbols like
satUSDandsatUSDv2.sdk/src/scripts/verify-tokenlist.ts (1)
8-11: Solid validation approach with on-chain verification.The script uses viem to fetch on-chain token data and compare it against the token list, which effectively catches mismatches in name, symbol, and decimals. The special handling for ETH at the zero address (lines 58-60) is appropriate.
Also applies to: 55-89
docs/docs/docs/reference/token-registry/index.mdx (4)
11-16: Info admonition follows Docusaurus guidelines.The use of
:::infofor the disclaimer correctly follows the Docusaurus admonition syntax as specified in the coding guidelines.Based on coding guidelines.
20-102: TokenTable component implementation is well-structured.The component properly handles edge cases:
- Sorts tokens alphabetically (line 22)
- Formats addresses consistently (lines 24-26)
- Distinguishes between native assets, L2-native tokens, and bridged tokens (lines 45-64)
- Gracefully handles missing bridge information with "N/A" fallbacks (lines 67-68)
- Links to appropriate block explorers (Etherscan for L1, BOB explorer for L2)
106-121: Bridge information clearly distinguishes L1 and L2 contracts.The Bridge Information section properly separates L1 (Ethereum) and L2 (BOB) contract addresses for each bridge type, making it easy for users to identify the correct contracts for their needs.
Based on learnings.
144-144: Gitleaks false positive can be ignored.The flagged "Generic API Key" on line 144 is actually an Ethereum address in example JSON documentation, not a secret. This is a false positive.
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
sdk/src/gateway/client.ts(6 hunks)sdk/src/gateway/layerzero.ts(2 hunks)sdk/src/gateway/tokens.ts(3 hunks)token-list.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-17T11:18:53.195Z
Learnt from: slavastartsev
Repo: bob-collective/bob PR: 634
File: sdk/src/gateway/client.ts:136-140
Timestamp: 2025-06-17T11:18:53.195Z
Learning: The codebase has a requirement not to introduce discriminated unions in the Gateway SDK client methods, including the getQuote method return types in sdk/src/gateway/client.ts.
Applied to files:
sdk/src/gateway/client.ts
🧬 Code graph analysis (3)
sdk/src/gateway/tokens.ts (1)
sdk/src/gateway/types/token.ts (1)
Token(7-14)
sdk/src/gateway/client.ts (1)
sdk/src/gateway/tokens.ts (1)
ADDRESS_LOOKUP(134-134)
sdk/src/gateway/layerzero.ts (1)
sdk/src/gateway/tokens.ts (1)
getTokenSlots(197-214)
🪛 GitHub Actions: SDK
sdk/src/gateway/tokens.ts
[warning] 1-1: Code style issues found by Prettier. Run 'npx prettier --write' to fix.
sdk/src/gateway/client.ts
[warning] 1-1: Code style issues found by Prettier. Run 'npx prettier --write' to fix.
🪛 Gitleaks (8.28.0)
token-list.json
[high] 20-20: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 37-37: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 52-52: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 69-69: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 86-86: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 103-103: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 160-160: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 177-177: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 194-194: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 211-211: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 228-228: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 245-245: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 262-262: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 279-279: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 296-296: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 313-313: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 330-330: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 347-347: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 364-364: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 381-381: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 398-398: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 439-439: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 456-456: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 481-481: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 498-498: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 515-515: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 532-532: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
| const slots = STORAGE_SLOTS_MAP[lowerAddress]; | ||
| if (!slots) { | ||
| throw new Error(`Slots not found for address ${tokenAddress}`); | ||
| } | ||
|
|
||
| // Check if slots are defined | ||
| if (token.allowanceSlot === undefined || token.balanceSlot === undefined) { | ||
| throw new Error(`Slots not defined for token ${token.symbol} at address ${tokenAddress}`); | ||
| if (slots.allowanceSlot === undefined || slots.balanceSlot === undefined) { | ||
| throw new Error(`Slots not defined at address ${tokenAddress}`); | ||
| } | ||
|
|
||
| return { | ||
| allowanceSlot: token.allowanceSlot, | ||
| balanceSlot: token.balanceSlot, | ||
| }; | ||
| return slots; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not throw when slot metadata is missing
STORAGE_SLOTS_MAP only contains the three WBTC addresses that happen to be listed above. As soon as a user requests an offramp quote for any other supported asset (for example tBTC at 0xBBa2eF945D523C4e2608C9E1214C2Cc64D4fc2e2), getTokenSlots() raises Slots not found…. That exception bubbles out of GatewayApiClient.getOfframpCreateOrderGasCost, so the surrounding try/catch in getQuote defaults gasFee to 0 and logs a warning. We lose the allowance/balance overrides and the quote no longer reflects the real cost. The same helper is also used in getL0CreateOrderGasCost; if the OFT address for a LayerZero chain isn’t in this hard-coded map the entire quote fails.
Previously, tokens that followed the standard ERC‑20 layout still worked because we fell back to the default slot indices. Please keep that behaviour (e.g. default to { balanceSlot: 0n, allowanceSlot: 1n }) or extend the token metadata so every supported asset supplies its slots before throwing.
🤖 Prompt for AI Agents
In sdk/src/gateway/tokens.ts around lines 203 to 214, the current implementation
throws when STORAGE_SLOTS_MAP lacks an entry or when allowanceSlot/balanceSlot
are undefined; instead, return a sensible default of { balanceSlot: 0n,
allowanceSlot: 1n } (or merge with any partial metadata) so standard ERC‑20
tokens fall back to default slots rather than causing an exception; ensure you
only throw for truly invalid inputs (if needed) and optionally emit a
warning/log when falling back to the default slots.
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (3)
sdk/src/gateway/tokens.ts (1)
198-213: Critical: Still throwing when storage slots are missing.This issue was flagged in a previous review and remains unresolved. The function throws when
STORAGE_SLOTS_MAPlacks an entry for a token address. For example, tBTC at0xBBa2eF945D523C4e2608C9E1214C2Cc64D4fc2e2(line 52 in token-list.json) will causegetTokenSlots()to throw, breakinggetOfframpCreateOrderGasCostandgetL0CreateOrderGasCost.Standard ERC-20 tokens typically use
balanceSlot: 0nandallowanceSlot: 1n. Please return these defaults when a token is not in the map instead of throwing.Apply this diff to fall back to standard ERC-20 slots:
export function getTokenSlots(tokenAddress: Address): { allowanceSlot: bigint; balanceSlot: bigint } { const lowerAddress = tokenAddress.toLowerCase(); - // Look up the token in the master TOKENS array const slots = STORAGE_SLOTS_MAP[lowerAddress]; - if (!slots) { - throw new Error(`Slots not found for address ${tokenAddress}`); - } - - // Check if slots are defined - if (slots.allowanceSlot === undefined || slots.balanceSlot === undefined) { - throw new Error(`Slots not defined at address ${tokenAddress}`); - } - - return slots; + + // Return custom slots if available, otherwise fall back to standard ERC-20 layout + return slots && slots.allowanceSlot !== undefined && slots.balanceSlot !== undefined + ? slots + : { balanceSlot: 0n, allowanceSlot: 1n }; }token-list.json (2)
36-49: WBTC bridgeInfo is incomplete.The WBTC entry at lines 42-48 is missing
originBridgeAddressanddestBridgeAddressfields in its bridgeInfo, which other bridged tokens include (e.g., WETH at lines 25-33). This incomplete metadata may cause issues for applications relying on complete bridge information.Please add the missing bridge addresses:
"bridgeInfo": { "1": { - "tokenAddress": "0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599" + "tokenAddress": "0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599", + "originBridgeAddress": "<bridge_contract_address>", + "destBridgeAddress": "<bridge_contract_address>" } }
403-417: Duplicate symbol: Two tokens use "satUSD".Both "Satoshi Stablecoin" (line 405) and "Satoshi Stablecoin V2" (line 413) use the symbol
satUSD. While the token list standard doesn't prohibit duplicate symbols, this creates ambiguity for applications that use symbols as identifiers and may confuse users.Consider differentiating the symbols:
{ "name": "Satoshi Stablecoin", "address": "0x78Fea795cBFcC5fFD6Fb5B845a4f53d25C283bDB", "symbol": "satUSD", ... }, { "name": "Satoshi Stablecoin V2", "address": "0xecf21b335B41f9d5A89f6186A99c19a3c467871f", - "symbol": "satUSD", + "symbol": "satUSDv2", ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
sdk/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
sdk/package.json(2 hunks)sdk/src/gateway/client.ts(4 hunks)sdk/src/gateway/tokens.ts(3 hunks)sdk/src/scripts/verify-tokenlist.ts(1 hunks)token-list.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- sdk/src/gateway/client.ts
- sdk/src/scripts/verify-tokenlist.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-17T11:18:53.195Z
Learnt from: slavastartsev
Repo: bob-collective/bob PR: 634
File: sdk/src/gateway/client.ts:136-140
Timestamp: 2025-06-17T11:18:53.195Z
Learning: The codebase has a requirement not to introduce discriminated unions in the Gateway SDK client methods, including the getQuote method return types in sdk/src/gateway/client.ts.
Applied to files:
sdk/src/gateway/tokens.ts
🧬 Code graph analysis (1)
sdk/src/gateway/tokens.ts (1)
sdk/src/gateway/types/token.ts (1)
Token(7-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (6)
sdk/src/gateway/tokens.ts (4)
6-23: LGTM - Storage slot map is well-structured.The centralized
STORAGE_SLOTS_MAPcleanly separates slot metadata from token definitions. The three addresses cover the critical WBTC variants and Bob Sepolia BTC that require non-standard storage layouts.
25-46: LGTM - Token derivation logic is correct.The filtering correctly excludes the deprecated WBTC address while preserving all other Bob mainnet tokens from the token list. The conditional slot enrichment cleanly handles tokens with and without custom storage layouts.
48-134: Mixed approach: token-list for Bob mainnet, hard-coded for other networks.The migration to token-list-driven tokens is applied selectively - Bob mainnet tokens come from the JSON file while Bob Sepolia, Optimism, and Ethereum tokens remain statically defined. This is a reasonable incremental migration strategy.
150-167: LGTM - Simplified initialization logic.The refactored
addTokensignature readingchainIdfrom the token object itself is cleaner and reduces parameter redundancy. The initialization loop is now straightforward.sdk/package.json (1)
29-30: LGTM - Dependencies support token list validation.The
ajvandajv-formatspackages enable JSON schema validation for the token list, aligning with the verification script mentioned in the PR summary. Version ranges are appropriate.token-list.json (1)
1-599: Well-structured token list with comprehensive metadata.The token list follows a consistent format with proper EIP-55 checksummed addresses (the previous checksum issue appears resolved), appropriate decimal values for each token type, and rich bridge metadata for cross-chain tokens. The inclusion of logoURIs and extensions provides good UX support.
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (3)
docs/docs/docs/reference/token-registry/index.mdx (2)
90-90: Use token.address as React key instead of array index.Using
indexas the key is an anti-pattern in React that can lead to unpredictable behavior and performance issues when the list is modified. Sincetoken.addressis unique and stable for each token, use it as the key.- <tr key={index}> + <tr key={token.address}>
135-149: JSON example is incomplete; missing bridge address fields.The Wrapped BTC example shows only
tokenAddress, but the component code (lines 72, 80–81) accessesdestBridgeAddressandoriginBridgeAddress. Either:
- Update the Wrapped BTC entry in token-list.json with the correct bridge addresses
- Use a different token example that has a complete
bridgeInfoobjectsdk/src/gateway/tokens.ts (1)
198-213: Critical issue: getTokenSlots throws for unsupported tokens (duplicate).This issue was flagged in a previous review. The function throws when
STORAGE_SLOTS_MAPlacks an entry, breaking offramp quotes for tokens like tBTC and any LayerZero OFT not in the hardcoded map. Standard ERC-20 tokens that followed the default layout previously worked via fallback behavior.Please apply the fix suggested in the previous review: return default slots
{ balanceSlot: 0n, allowanceSlot: 1n }for standard ERC-20 tokens instead of throwing, or ensure all supported assets have entries inSTORAGE_SLOTS_MAP.
🧹 Nitpick comments (1)
sdk/src/gateway/tokens.ts (1)
48-123: Consider consolidating storage slot definitions.Storage slots are defined in three different ways: inline for some tokens here (lines 56-57, 120-121), via
STORAGE_SLOTS_MAPfor others (lines 6-23), and not at all for most tokens. While this works, consolidating all slot definitions intoSTORAGE_SLOTS_MAPwould improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/docs/docs/reference/token-registry/index.mdx(1 hunks)sdk/src/gateway/tokens.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/docs/docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (docs/CLAUDE.md)
docs/docs/docs/**/*.{md,mdx}: Use Docusaurus admonition syntax for notes::::infofor informational notes,:::warningfor warnings,:::dangerfor critical warnings,:::tipfor helpful tips
Do NOT use markdown blockquotes (>) for notes
Contract addresses should be sorted alphabetically within their sections
Keep all contracts under a single hierarchy (no sub-sections for different deployment types)
Include both implementation and proxy contracts when available
Format contract entries as:- ContractName: [address](explorer-link)
Always maintain alphabetical ordering when adding new contracts
Distinguish between L1 (Ethereum) and L2 (BOB) contracts clearly
Files:
docs/docs/docs/reference/token-registry/index.mdx
🧠 Learnings (2)
📚 Learning: 2025-06-17T11:18:53.195Z
Learnt from: slavastartsev
Repo: bob-collective/bob PR: 634
File: sdk/src/gateway/client.ts:136-140
Timestamp: 2025-06-17T11:18:53.195Z
Learning: The codebase has a requirement not to introduce discriminated unions in the Gateway SDK client methods, including the getQuote method return types in sdk/src/gateway/client.ts.
Applied to files:
sdk/src/gateway/tokens.ts
📚 Learning: 2025-07-31T14:44:59.656Z
Learnt from: CR
Repo: bob-collective/bob PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2025-07-31T14:44:59.656Z
Learning: Applies to docs/docs/docs/**/*.{md,mdx} : Distinguish between L1 (Ethereum) and L2 (BOB) contracts clearly
Applied to files:
docs/docs/docs/reference/token-registry/index.mdx
🧬 Code graph analysis (1)
sdk/src/gateway/tokens.ts (1)
sdk/src/gateway/types/token.ts (1)
Token(7-14)
🪛 GitHub Actions: SDK
sdk/src/gateway/tokens.ts
[warning] 1-1: Prettier formatting issues found. Run 'npx prettier --write' to fix.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (5)
docs/docs/docs/reference/token-registry/index.mdx (1)
5-5: Import path is correct.The import path
'../../../../../token-list.json'correctly resolves to the repository root wheretoken-list.jsonis located. No changes needed.sdk/src/gateway/tokens.ts (4)
25-46: LGTM: Token augmentation logic is sound.The filtering and conditional slot augmentation is implemented correctly. Tokens from the list are properly filtered by chain ID, and storage slots are added when available in
STORAGE_SLOTS_MAP.
125-148: LGTM: Token array and lookup structure.The combined token array and dual lookup indexes (by symbol and address per chain) provide efficient access patterns.
150-163: LGTM: Simplified signature using chainId from token object.The refactored signature is cleaner, and the implementation correctly uses
token.chainIdfor indexing.
169-187: LGTM: Strategy mapping is comprehensive.The address-to-strategy mapping correctly covers tokens and includes explicit mappings for specialized strategies.
| - **L1:** [0x3F6c...1f7](https://etherscan.io/address/0x3F6cE1b36e5120BBc59D0cFe8A5aC8b6464ac1f7) | ||
| - **L2:** [0x4200...0010](https://explorer.gobob.xyz/address/0x4200000000000000000000000000000000000010) | ||
|
|
||
| ### USDC Bridge | ||
| - **L1:** [0x450D...aCBb](https://etherscan.io/address/0x450D55a4B4136805B0e5A6BB59377c71FC4FaCBb) | ||
| - **L2:** [0xe497...cE90](https://explorer.gobob.xyz/address/0xe497788F8Fcc30B773C9A181a0FFE2e60645cE90) | ||
|
|
||
| ### wstETH Bridge | ||
| - **L1:** [0x091d...5B72](https://etherscan.io/address/0x091dF5E1284E49fA682407096aD34cfD42B95B72) | ||
| - **L2:** [0xd155...c4d1](https://explorer.gobob.xyz/address/0xd1559523374D93972E0F7fE1AA98642754f5c4d1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Address formatting is inconsistent with formatAddress function.
The manual bridge address entries (e.g., 0x3F6c...1f7) use a different shortening scheme than the formatAddress() function (lines 25–27), which produces 0x3F6c...c1f7 (first 6, last 4 chars). Either:
- Update these manual entries to match formatAddress output
- Update formatAddress to match the manual format (first 4, last 3)
Choose one approach consistently.
🤖 Prompt for AI Agents
docs/docs/docs/reference/token-registry/index.mdx lines 110-119: the manual
shortened bridge addresses use a different truncation than the existing
formatAddress() (lines 25–27) which returns first 6 and last 4 characters;
update the manual entries to match that output (first 6 chars, ellipse, last 4
chars) for all listed addresses in this block (and scan nearby entries for other
mismatches) so the doc uses a single consistent shortening scheme.
| ```javascript | ||
| // Fetch the tokenlist | ||
| const response = await fetch('https://raw.githubusercontent.com/bob-collective/bob/master/token-list.json'); | ||
| const tokens = await response.json(); | ||
|
|
||
| // Find WBTC token info | ||
| const wbtc = tokens.find(token => token.symbol === 'WBTC'); | ||
| console.log(wbtc.address); // L2 WBTC address | ||
|
|
||
| // Get bridge information | ||
| const bridgeInfo = wbtc.extensions?.bridgeInfo?.["1"]; | ||
| console.log(bridgeInfo.tokenAddress); // L1 WBTC address | ||
| console.log(bridgeInfo.destBridgeAddress); // L1 bridge address | ||
| console.log(bridgeInfo.originBridgeAddress); // L2 bridge address | ||
| ``` No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usage example has incorrect data access pattern.
Line 160 calls tokens.find(), but tokens is the entire JSON object, not an array. It should be tokens.tokens.find() to access the tokens array.
-// Find WBTC token info
-const wbtc = tokens.find(token => token.symbol === 'WBTC');
+// Find WBTC token info
+const wbtc = tokens.tokens.find(token => token.symbol === 'WBTC');🤖 Prompt for AI Agents
In docs/docs/docs/reference/token-registry/index.mdx around lines 154 to 168,
the example treats the fetched JSON as an array but the payload structure nests
the array under a tokens property; change the access from tokens.find(...) to
tokens.tokens.find(...), and add a defensive null/length check (e.g., ensure
tokens && Array.isArray(tokens.tokens)) before calling find to avoid runtime
errors when the response shape differs.
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Summary by CodeRabbit
Documentation
New Features
Refactor
Chores