chore: expose layerzero mothods, fix token logos #734
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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. WalkthroughUpdates token logo URIs for solvBTC variants, adds a 60-second gate to unlocking Active offramp orders, removes unused imports, and introduces two LayerZero lookup methods delegating to the underlying client. Also adds a trailing newline to sdk/package.json. No public API changes except the two new LayerZero methods. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant SDK as GatewayClient
participant Clock as Time Source
User->>SDK: canOrderBeUnlocked(order)
alt status == "Accepted"
Note over SDK: Enforce on-chain CLAIM_DELAY
SDK-->>User: true/false based on CLAIM_DELAY
else status == "Active"
SDK->>Clock: now()
Clock-->>SDK: currentTime
Note over SDK: Allow only if now - orderTimestamp >= 60s
SDK-->>User: true/false
else
SDK-->>User: false
end
sequenceDiagram
autonumber
actor Dev
participant LZClient as LayerZeroGatewayClient
participant L0 as underlying l0Client
Dev->>LZClient: getEidForChain(chainKey)
LZClient->>L0: getEidForChain(chainKey)
L0-->>LZClient: eid | null
LZClient-->>Dev: eid | null
Dev->>LZClient: getOftAddressForChain(chainKey)
LZClient->>L0: getOftAddressForChain(chainKey)
L0-->>LZClient: address | null
LZClient-->>Dev: address | null
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @slavastartsev, 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 streamlines the display names for Wrapped BTC by removing an extraneous suffix. The change aims to improve clarity and consistency in how the token is presented to users, without altering its underlying functionality or properties.
Highlights
- Token Naming Convention: The "(OFT)" suffix has been removed from the name and symbol of the Wrapped BTC token within the
bobTokensarray, simplifying its display.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request aims to remove the '(OFT)' suffix from a WBTC token definition. However, this change introduces a critical issue by creating a duplicate token symbol 'WBTC'. This will cause one of the WBTC tokens to be overwritten in the symbol-based lookup map, potentially leading to incorrect token resolution. I've added a comment detailing the issue and suggesting a fix.
sdk/src/gateway/tokens.ts
Outdated
| name: 'Wrapped BTC', | ||
| symbol: 'WBTC', |
There was a problem hiding this comment.
This change introduces a duplicate token symbol 'WBTC'. There is another token with the same symbol defined on lines 19-20.
The SYMBOL_LOOKUP map is populated by iterating through the TOKENS array, and it uses the lower-cased token symbol as the key. With this change, the token at lines 29-39 will overwrite the token at lines 18-28 in the SYMBOL_LOOKUP map because they will both use the key 'wbtc'.
This will make the first WBTC token (0x03C7...) inaccessible via symbol lookup, which can lead to bugs where the wrong token address is resolved.
Token symbols must be unique for a given chain. Please either restore a unique symbol for this token or remove one of the duplicate entries if it's no longer needed. If the goal is to rename it, consider a different unique symbol, for example WBTC.l0 if this is the LayerZero version.
There was a problem hiding this comment.
@slavastartsev can you remove the old wBTC token?
There was a problem hiding this comment.
Will do,
need this change as a quick fix to remove WBTC (OFT) entries from UI.
I'll migrate SDK to use optimism tokenlist next
There was a problem hiding this comment.
We should probably retain legacy WBTC support due to this line:
Line 937 in 911355d
Otherwise, we won't be able to display gateway transactions related to the legacy WBTC token.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk/src/gateway/tokens.ts (1)
369-382: Detect duplicate symbols per chain to avoid silent overwritesAdd a guard in addToken so collisions are surfaced during init rather than silently changing behavior.
Apply:
SYMBOL_LOOKUP[chainId][lowerToken.symbol.toLowerCase()] = lowerToken; ADDRESS_LOOKUP[chainId][lowerAddress] = lowerToken;→
- SYMBOL_LOOKUP[chainId][lowerToken.symbol.toLowerCase()] = lowerToken; + const sym = lowerToken.symbol.toLowerCase(); + const existing = SYMBOL_LOOKUP[chainId][sym]; + if (existing && existing.address !== lowerToken.address) { + console.warn( + `Duplicate token symbol on chain ${chainId}: ${lowerToken.symbol} ` + + `(${existing.address}) vs (${lowerToken.address}). Using the latter.`, + ); + } + SYMBOL_LOOKUP[chainId][sym] = lowerToken; ADDRESS_LOOKUP[chainId][lowerAddress] = lowerToken;
🧹 Nitpick comments (1)
sdk/src/gateway/tokens.ts (1)
417-425: Normalize symbol lookups to lowercase to match how keys are storedCurrently keys are stored lowercased, but getTokenAddress reads with the raw token string, making lookups case-sensitive.
Apply:
export function getTokenAddress(chainId: number, token: string): Address { if (isAddress(token)) { return token; - } else if (SYMBOL_LOOKUP[chainId][token]) { - return SYMBOL_LOOKUP[chainId][token].address; + } else { + const key = token.toLowerCase(); + const bySymbol = SYMBOL_LOOKUP[chainId]?.[key]; + if (bySymbol) return bySymbol.address; + throw new Error(`Unknown output token: ${token}`); } - } else { - throw new Error('Unknown output token'); - } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
sdk/src/gateway/tokens.ts(1 hunks)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: Bindings
…hore/gateway-v4-improvements
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk/src/gateway/tokens.ts (1)
406-414: Fix case sensitivity and typing in getTokenAddress()Lookups are stored with lower-cased keys, but the function indexes with the raw token string. This makes symbol resolution case-sensitive and brittle. Also cast the address on the “isAddress” branch.
export function getTokenAddress(chainId: number, token: string): Address { if (isAddress(token)) { - return token; - } else if (SYMBOL_LOOKUP[chainId][token]) { - return SYMBOL_LOOKUP[chainId][token].address; - } else { - throw new Error('Unknown output token'); - } + return token as Address; + } + const key = token.toLowerCase(); + const entry = SYMBOL_LOOKUP[chainId][key]; + if (entry) { + return entry.address; + } + throw new Error(`Unknown output token: ${token}`); }
🧹 Nitpick comments (3)
sdk/src/gateway/tokens.ts (3)
347-356: Prefer Record types for lookup mapsMapped types over primitive “number/string” are unusual here. Record improves readability and tooling.
-export const SYMBOL_LOOKUP: { [key in number]: { [key in string]: Token } } = {}; -export const ADDRESS_LOOKUP: { [key in number]: { [key in string]: Token } } = {}; +export const SYMBOL_LOOKUP: Record<number, Record<string, Token>> = {}; +export const ADDRESS_LOOKUP: Record<number, Record<string, Token>> = {};
358-371: Guard against silent symbol collisions when populating SYMBOL_LOOKUPA lightweight check here would have surfaced the prior WBTC collision immediately.
function addToken(address: string, token: (typeof TOKENS)[number], chainId: number) { const lowerAddress = address.toLowerCase(); const lowerToken: Token = { chainId, address: lowerAddress as Address, name: token.name, symbol: token.symbol, decimals: token.decimals, logoURI: token.logoURI, }; - SYMBOL_LOOKUP[chainId][lowerToken.symbol.toLowerCase()] = lowerToken; + const symbolKey = lowerToken.symbol.toLowerCase(); + const prev = SYMBOL_LOOKUP[chainId][symbolKey]; + if (prev && prev.address !== lowerAddress) { + // Consider `throw` in CI or retain as warn in runtime + // to avoid breaking consumers. + // eslint-disable-next-line no-console + console.warn( + `[gateway/tokens] Duplicate symbol "${lowerToken.symbol}" on chain ${chainId}: ${prev.address} -> ${lowerAddress}` + ); + } + SYMBOL_LOOKUP[chainId][symbolKey] = lowerToken; ADDRESS_LOOKUP[chainId][lowerAddress] = lowerToken; }
396-404: Make getTokenDecimals chain-agnostic using ADDRESS_LOOKUPCurrent implementation ignores Optimism and future chains. Use the already-built address index.
export function getTokenDecimals(tokenAddress: Address): number | undefined { - const normalizedAddress = tokenAddress.toString().toLowerCase(); - - const token = - bobTokens.find((t) => t.tokens.bob.address.toLowerCase() === normalizedAddress) || - bobSepoliaTokens.find((t) => t.tokens['bob-sepolia'].address.toLowerCase() === normalizedAddress); - - return token?.decimals; + const normalizedAddress = tokenAddress.toString().toLowerCase(); + for (const chainMap of Object.values(ADDRESS_LOOKUP)) { + const t = chainMap[normalizedAddress]; + if (t) return t.decimals; + } + return undefined; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (4)
assets/BTC_plus.svgis excluded by!**/*.svgassets/solvBTC.JUP.svgis excluded by!**/*.svgassets/solvBTC.plus.svgis excluded by!**/*.svgassets/solvBTC_Jupiter.svgis excluded by!**/*.svg
📒 Files selected for processing (1)
sdk/src/gateway/tokens.ts(2 hunks)
⏰ 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
…hore/gateway-v4-improvements
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk/src/gateway/client.ts (1)
591-606: Fix: 60s bypass lets Accepted orders ignore on-chain CLAIM_DELAYEarly-return at Line 591 returns true for any status (including Accepted) once 60s passes, skipping the CLAIM_DELAY check below. That violates the intended policy and allows premature refunds.
Apply this diff to gate the 60s window only for Active orders and always enforce CLAIM_DELAY for Accepted:
- if (Math.floor(Date.now() / 1000) - orderTimestamp >= 60) { - return true; - } - if (status !== 'Accepted') { - return false; - } - // check if Accepted order has passed claim delay - const nowInSec = Math.floor(Date.now() / 1000); + const nowInSec = Math.floor(Date.now() / 1000); + // Active: unlockable only after a short grace period + if (status === 'Active') { + return nowInSec - orderTimestamp >= ACTIVE_UNLOCK_GRACE_PERIOD_SECONDS; + } + // Only Accepted orders can unlock after on-chain claim delay + if (status !== 'Accepted') { + return false; + } + // Accepted: enforce on-chain claim delay const publicClient = viemClient(this.chain); const claimDelay = await publicClient.readContract({ address: offrampRegistryAddress, abi: claimDelayAbi, functionName: 'CLAIM_DELAY', }); return orderTimestamp + Number(claimDelay) <= nowInSec;And define a named constant near ORDER_DEADLINE_IN_SECONDS:
export const ORDER_DEADLINE_IN_SECONDS = 30 * 60; // 30 minutes +export const ACTIVE_UNLOCK_GRACE_PERIOD_SECONDS = 60; // 1 minute
🧹 Nitpick comments (1)
sdk/src/gateway/client.ts (1)
498-504: Align error message with new unlock rulesMessage hardcodes “7-day claim delay,” which is inaccurate for Active orders (now 60s grace). Make it status-aware or generic to avoid confusing users.
- throw new Error(`Offramp order is still within the 7-day claim delay and cannot be unlocked yet.`); + throw new Error( + orderDetails.status === 'Active' + ? `Offramp order cannot be unlocked yet: must be at least ${ACTIVE_UNLOCK_GRACE_PERIOD_SECONDS}s old.` + : `Offramp order cannot be unlocked yet: still within the on-chain claim delay.` + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
sdk/src/gateway/client.ts(1 hunks)
⏰ 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
slavastartsev
force-pushed
the
chore/gateway-v4-improvements
branch
from
b817f24 to
9d44368
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
sdk/test/gateway.test.ts (1)
507-507: Confirm unlockability for 'Processed' orders or switch status to 'Active'.If canOrderBeUnlocked is intended to be true only for Active (>=60s) and Accepted (post-CLAIM_DELAY), asserting true for a 'Processed' order may be incorrect. Either:
- Change the mocked status to 'Active' to reflect the 60s rule, or
- Keep 'Processed' and assert false.
Suggested tweak (if intent is Active + 60s):
- status: 'Processed', + status: 'Active',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
sdk/package.json(1 hunks)sdk/src/gateway/index.ts(1 hunks)sdk/test/gateway.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- sdk/package.json
⏰ 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 (1)
sdk/src/gateway/index.ts (1)
6-6: Public API surface change: re-exporting LayerZeroClient.Looks good. Please confirm no circular imports and update any user-facing docs/CHANGELOG since this is a new named export.
slavastartsev
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk/src/gateway/layerzero.ts (1)
150-158: PadSendParam.toto 32 bytes
Replaceto: padHex(params.toUserAddress as Hex) as Hex,with
to: padHex(params.toUserAddress as Hex, { size: 32 }) as Hex,to satisfy the
bytes32ABI requirement.
🧹 Nitpick comments (6)
sdk/src/gateway/layerzero.ts (6)
112-114: New public method is fine; prefer routing internal callers through it for consistency.You added LayerZeroGatewayClient.getEidForChain(), but internal sites still call l0Client directly/new up clients. Using the new method improves testability/mocking and centralizes behavior.
248-255: Reuse the existing l0Client instead of instantiating a new one.Avoids duplicated configuration and eases testing/mocking.
Apply:
- const layerZeroClient = new LayerZeroClient(); + const layerZeroClient = this.l0Client;
73-79: Type fix:response.json()returnsT, notPromise<T>.Minor typing correction; avoids misleading types in callers.
- return (await response.json()) as Promise<T>; + return (await response.json()) as T;
73-79: Add a fetch timeout to harden metadata calls.Network hangs will stall quotes/execution. Use AbortController with a sensible default (e.g., 8–10s) and surface a descriptive error.
Example:
private async getJson<T>(url: string, timeoutMs = 10_000): Promise<T> { const c = new AbortController(); const id = setTimeout(() => c.abort(), timeoutMs); try { const response = await fetch(url, { signal: c.signal }); if (!response.ok) throw new Error(`GET ${url} failed: ${response.status} ${response.statusText}`); return (await response.json()) as T; } finally { clearTimeout(id); } }
171-175: Name clarity: avoidbufferambiguity; use basis points terminology.Renaming clarifies intent (bps) and avoids confusion with Node’s Buffer/global terms.
- const buffer = BigInt(500); // 5% buffer + const feeBufferBps = 500n; // 5% buffer - const layerZeroFeeWithBuffer = (layerZeroFee.nativeFee * (10000n + buffer)) / 10000n; // 5% buffer + const layerZeroFeeWithBuffer = (layerZeroFee.nativeFee * (10000n + feeBufferBps)) / 10000n; // 5% buffer - const tokensToSwapForLayerZeroFeesWithBuffer = (tokensToSwapForLayerZeroFees * (10000n + buffer)) / 10000n; // 5% buffer + const tokensToSwapForLayerZeroFeesWithBuffer = (tokensToSwapForLayerZeroFees * (10000n + feeBufferBps)) / 10000n; // 5% buffer - [sendParam, BigInt(buffer), BigInt(tokensToSwapForLayerZeroFeesWithBuffer)] + [sendParam, feeBufferBps, BigInt(tokensToSwapForLayerZeroFeesWithBuffer)]Also applies to: 193-194, 202-202
176-190: Hard-coded addresses: consider centralizing as named constants/config.QuoterV2 and WETH addresses are literals here; placing them in a constants module (per-chain) avoids drift and eases future upgrades.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
sdk/src/gateway/client.ts(1 hunks)sdk/src/gateway/layerzero.ts(2 hunks)sdk/src/gateway/tokens.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- sdk/src/gateway/tokens.ts
- sdk/src/gateway/client.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-17T11:18:53.195Z
Learnt from: slavastartsev
PR: bob-collective/bob#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/layerzero.ts
⏰ 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 (1)
sdk/src/gateway/layerzero.ts (1)
1-1: Imports are compatible with viem ^2.33.2. encodeAbiParameters, parseAbiParameters, encodePacked, and padHex are all exported in v2.33.2, so no further changes are required. (npmjs.com, viem.sh, app.unpkg.com)
Summary by CodeRabbit