feat(gateway-v4): expose layerzero #770
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![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 @mastilver, 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 Gateway SDK by providing explicit support for orders that utilize LayerZero for cross-chain routing. It equips the OnrampOrder object with new methods to identify LayerZero-enabled orders and to extract crucial details such as the destination Endpoint ID and its corresponding blockchain chain ID. This is achieved through a new internal service that queries and caches LayerZero's public metadata API, ensuring efficient and accurate retrieval of cross-chain routing information.
Highlights
- LayerZero Integration: Introduced new functionality to detect if an order is routed via LayerZero and to retrieve its specific LayerZero details.
- New API Methods: Added
isLayerZero()andgetLayerZeroDetails()methods to theOnrampOrderobject, allowing programmatic access to LayerZero routing information. - LayerZero Metadata API: Implemented a mechanism to fetch LayerZero deployment metadata from a public API, mapping LayerZero Endpoint IDs (EIDs) to native chain IDs.
- Caching for Efficiency: Incorporated an internal caching layer for LayerZero EID to chain ID mappings to optimize performance and reduce redundant API calls.
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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| 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 pull request 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. ↩
|
Caution Review failedThe pull request is closed. 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. WalkthroughAdds an optional Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant Gateway as LayerZeroGatewayClient
participant Client as LayerZeroClient
participant Deploy as Deployments (chain configs)
Caller->>Gateway: getChainIdForEid(eid)
Gateway->>Client: getChainId(eid)
Client->>Deploy: scan deployments for matching eid
alt match found
Deploy-->>Client: nativeChainId
Client-->>Gateway: nativeChainId
Gateway-->>Caller: nativeChainId
else no match
Deploy-->>Client: not found
Client-->>Gateway: null
Gateway-->>Caller: null
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
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 |
mastilver
force-pushed
the
feat/gateway-v4-layerzero
branch
from
09bf8d9 to
f4563ad
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces functionality to expose LayerZero details for onramp orders. It adds two new methods, isLayerZero and getLayerZeroDetails, to the OnrampOrder object. The implementation includes fetching and caching LayerZero chain metadata.
My review focuses on improving code clarity, correctness, and testing practices. I've identified a typo, a suggestion for more modern syntax, a hardcoded URL, and some confusing logic in the data transformation. Additionally, a new test case is making a live network request, which should be mocked to ensure test stability and determinism. Addressing these points will enhance the maintainability and robustness of the new functionality.
sdk/test/gateway.test.ts
Outdated
| assert.strictEqual(orders[7].isLayerZero(), true); | ||
| assert.deepEqual(await orders[7].getLayerZeroDetails(), { dstEid: 30111, chainId: optimism.id }); |
There was a problem hiding this comment.
This test makes a live network request to https://metadata.layerzero-api.com to fetch LayerZero details. Unit and integration tests should be deterministic and not rely on external services. Please use nock to mock this API call and provide a stable, predictable response for the test.
For example:
nock('https://metadata.layerzero-api.com')
.get('/v1/metadata/deployments')
.reply(200, {
'optimism': {
deployments: [{ eid: '30111' }],
chainDetails: { nativeChainId: optimism.id }
}
});
sdk/src/gateway/client.ts
Outdated
| `Failed to get layerzero deployments` | ||
| ); | ||
|
|
||
| if (!response.ok) { |
There was a problem hiding this comment.
It's a good practice to define URLs as constants rather than hardcoding them. This improves maintainability and makes it easier to manage for different environments. Please extract this URL to a constant at the top of the file.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
sdk/src/gateway/client.ts (2)
99-111: Fix type alias typo: Deplotments → Deployments.Pure rename for clarity and future grepping.
-// subset of: https://github.com/LayerZero-Labs/devtools/blob/aa43d67ec0ca5a1668b9174a3f3a032e6ce8693d/packages/metadata-tools/src/types.ts#L41 -type LayerZeroDeplotmentsMetadata = Record< +// subset of: https://github.com/LayerZero-Labs/devtools/blob/aa43d67ec0ca5a1668b9174a3f3a032e6ce8693d/packages/metadata-tools/src/types.ts#L41 +type LayerZeroDeploymentsMetadata = Record< string, { deployments?: { eid: string; }[]; chainDetails?: { nativeChainId?: number; }; } >;
1328-1369: Tighten mapping build and avoid hardcoded URL.
- Replace ineffective
.filter(Boolean)with a predicate on the tuple’s second element.- Use a constant for the metadata URL (cleaner config).
- Avoid truthiness check that would reject a valid 0 chainId; check for null/undefined instead.
- private async getLayerZeroChainId(eid: number): Promise<number | null> { + private async getLayerZeroChainId(eid: number): Promise<number | null> { // Only calls the API once and cache it for future calls if (!this.layerZeroEidToChainIdMapP) { this.layerZeroEidToChainIdMapP = (async () => { const response = await this.safeFetch( - 'https://metadata.layerzero-api.com/v1/metadata/deployments', + GatewayApiClient.LAYERZERO_DEPLOYMENTS_URL, { headers: { 'Content-Type': 'application/json', Accept: 'application/json', }, }, `Failed to get layerzero deployments` ); if (!response.ok) { return new Map(); } - const data: LayerZeroDeplotmentsMetadata = await response.json(); + const data: LayerZeroDeploymentsMetadata = await response.json(); - return new Map( - Object.values(data) - .map((x) => x.deployments?.map((d) => [d.eid, x.chainDetails?.nativeChainId] as const) ?? []) - .flat() - .filter(Boolean) - ); + const entries = Object.values(data).flatMap((x) => + (x.deployments ?? []).map((d) => [d.eid, x.chainDetails?.nativeChainId] as const) + ); + // Keep only entries that have a defined chainId + const filtered = entries.filter( + (pair): pair is readonly [string, number] => pair[1] !== undefined && pair[1] !== null + ); + return new Map(filtered); })(); } const map = await this.layerZeroEidToChainIdMapP; - const chainId = map.get(eid.toString()); + const chainId = map.get(eid.toString()); - // make sure it's not null, undefined, or 0 - if (chainId) { - return chainId; - } + // Return null if missing; allow 0 if ever used + if (chainId !== undefined && chainId !== null) return chainId; return null; }Add this constant (or similar) near the other base URLs or as a static in the class:
// Place near other API base URLs export const LAYERZERO_DEPLOYMENTS_URL = 'https://metadata.layerzero-api.com/v1/metadata/deployments'; // ...then reference it as LAYERZERO_DEPLOYMENTS_URL inside getLayerZeroChainIdor
// Inside GatewayApiClient private static readonly LAYERZERO_DEPLOYMENTS_URL = 'https://metadata.layerzero-api.com/v1/metadata/deployments';sdk/test/gateway.test.ts (1)
436-438: Mock the LayerZero metadata endpoint; avoid live network in tests.This test currently hits https://metadata.layerzero-api.com at runtime. Please stub it with nock to keep tests deterministic.
const gatewaySDK = new GatewaySDK(bob.id); + // Stub LayerZero deployments metadata used by getLayerZeroDetails() + nock('https://metadata.layerzero-api.com') + .get('/v1/metadata/deployments') + .reply(200, { + optimism: { + deployments: [{ eid: '30111' }], + chainDetails: { nativeChainId: optimism.id }, + }, + }); const orders = await gatewaySDK.getOnrampOrders(zeroAddress);
🧹 Nitpick comments (3)
sdk/src/gateway/types.ts (2)
349-352: Type addition looks good; add brief field docs for consumers.Optional JSDoc to make API self‑describing.
export interface LayerZeroDetails { - dstEid: number; - chainId: number | null; + /** @description Destination Endpoint ID (LayerZero EID) */ + dstEid: number; + /** @description Native EVM chainId for the destination chain, or null if unknown */ + chainId: number | null; }
371-375: Normalize “LayerZero” capitalization and clarify null case.- /** @description Check if the order is routed through layerzero */ + /** @description Check if the order is routed through LayerZero */ isLayerZero(): boolean; - /** @description Get the layerzero details */ - getLayerZeroDetails(): Promise<LayerZeroDetails | null>; + /** @description Get the LayerZero details (returns null when not routed via LayerZero) */ + getLayerZeroDetails(): Promise<LayerZeroDetails | null>;sdk/src/gateway/client.ts (1)
1099-1101: Use strict comparison.Make the intent explicit; only undefined is possible from typing.
- isLayerZero() { - return order.layerzeroDstEid != undefined; - }, + isLayerZero() { + return order.layerzeroDstEid !== undefined; + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
sdk/src/gateway/client.ts(5 hunks)sdk/src/gateway/types.ts(3 hunks)sdk/test/gateway.test.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
sdk/test/gateway.test.ts (1)
sdk/src/gateway/tokens.ts (1)
SYMBOL_LOOKUP(376-376)
🪛 GitHub Check: Tests
sdk/src/gateway/client.ts
[failure] 1067-1067:
Unexpected aliasing of 'this' to local variable
🪛 GitHub Actions: SDK
sdk/src/gateway/client.ts
[warning] 1-1: ESLint: 'AddressType' is defined but never used. (no-unused-vars)
[warning] 20-20: ESLint: 'getAddressInfo' is defined but never used. (no-unused-vars)
[error] 1067-1067: ESLint: Unexpected aliasing of 'this' to local variable. (no-this-alias). Command: npm run lint
🔇 Additional comments (4)
sdk/src/gateway/types.ts (1)
321-322: Capitalize “LayerZero” in JSDoc and clarify wording.Small polish to match brand capitalization and be clearer.
[ suggest_nitpick_refactor ]
- /** layerzero dst eid if the order being routed through layerzero */ + /** @description LayerZero destination EID if the order is routed through LayerZero */sdk/test/gateway.test.ts (1)
410-424: LGTM: adds LayerZero order fixture and length assertion.The added mock order and count change look consistent with the new API surface.
sdk/src/gateway/client.ts (2)
140-142: Good: single-flight cache for LayerZero mapping.This Promise cache avoids redundant fetches across calls.
1066-1115: Fix lint error: no-this-alias (pipeline blocker)Replace
const that = this;with a bound function reference and update its usage in getLayerZeroDetails; re-run the verification.- const that = this; + const getLayerZeroChainId = this.getLayerZeroChainId.bind(this); @@ isLayerZero() { return order.layerzeroDstEid != undefined; }, async getLayerZeroDetails() { const eid = order.layerzeroDstEid; if (!eid) { return null; } - const chainId = await that.getLayerZeroChainId(eid); + const chainId = await getLayerZeroChainId(eid); return { dstEid: eid, chainId, }; },rg results you ran:
- sdk/src/gateway/client.ts:1066 —
const that = this;(actual this-alias to fix)- other matches are
const x = this.yproperty copies (fine): sdk/src/inscription.ts:72, sdk/src/gateway/client.ts:1029, 1133, 1134Run the verification script again after applying the patch.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
sdk/src/gateway/client.ts (1)
1334-1342: Extract the LayerZero metadata URL to a constant.This mirrors the earlier feedback about avoiding hardcoded URLs and improves maintainability across environments.
- const response = await this.safeFetch( - 'https://metadata.layerzero-api.com/v1/metadata/deployments', + const response = await this.safeFetch( + LAYERZERO_DEPLOYMENTS_METADATA_URL, {Add this near the other base URLs:
export const LAYERZERO_DEPLOYMENTS_METADATA_URL = 'https://metadata.layerzero-api.com/v1/metadata/deployments';
🧹 Nitpick comments (3)
sdk/src/gateway/client.ts (3)
140-142: Narrow the cache type; store only valid chainIds.Storing null/undefined in the Map complicates usage. Build the Map with only valid numbers and type it accordingly.
- private layerZeroEidToChainIdMapP: Promise<Map<string, number | null | undefined>> | null = null; + private layerZeroEidToChainIdMapP: Promise<ReadonlyMap<string, number>> | null = null;
1066-1068: Avoid this-alias; use arrow functions and add resilient error handling.Use arrow functions to capture the class this without disabling the lint rule. Also make getLayerZeroDetails robust if the metadata fetch fails.
- // eslint-disable-next-line @typescript-eslint/no-this-alias - const that = this; + // Capture class `this` via arrow functions below; no alias needed. ... - isLayerZero() { - return order.layerzeroDstEid != undefined; - }, - async getLayerZeroDetails() { - const eid = order.layerzeroDstEid; - - if (!eid) { - return null; - } - - const chainId = await that.getLayerZeroChainId(eid); - - return { - dstEid: eid, - chainId, - }; - }, + isLayerZero: () => order.layerzeroDstEid != undefined, + getLayerZeroDetails: async () => { + const eid = order.layerzeroDstEid; + if (!eid) return null; + try { + const chainId = await this.getLayerZeroChainId(eid); + return { dstEid: eid, chainId }; + } catch { + // Gracefully degrade if metadata lookup fails + return { dstEid: eid, chainId: null }; + } + },Also applies to: 1100-1116
1350-1367: Tighten mapping and checks; filter invalid entries and simplify consumers.Filter out entries lacking a nativeChainId and keep only positive numbers. This lets you drop nullable types and avoid truthiness checks on 0.
- return new Map( - Object.values(data) - .map((x) => x.deployments?.map((d) => [d.eid, x.chainDetails?.nativeChainId] as const) ?? []) - .flat() - ); + const pairs = Object.values(data).flatMap((x) => + (x.deployments ?? []).map((d) => [String(d.eid), x.chainDetails?.nativeChainId] as const) + ); + const filtered = pairs.filter(([, chainId]) => typeof chainId === 'number' && chainId > 0); + return new Map(filtered); })(); } const map = await this.layerZeroEidToChainIdMapP; - const chainId = map.get(eid.toString()); - - // make sure it's not null, undefined, or 0 - if (chainId) { - return chainId; - } - - return null; + const chainId = map.get(String(eid)); + return typeof chainId === 'number' && chainId > 0 ? chainId : null;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sdk/src/gateway/client.ts(5 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
🔇 Additional comments (1)
sdk/src/gateway/client.ts (1)
99-111: Typo fixed and type alias looks good.Name is corrected to LayerZeroDeploymentsMetadata and the shape matches the referenced subset.
sdk/src/gateway/layerzero-client.ts
Outdated
| }); | ||
| } | ||
|
|
||
| async getChainId(eid: string): Promise<number | null> { |
mastilver
force-pushed
the
feat/gateway-v4-layerzero
branch
from
562f83a to
2b41035
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
sdk/src/gateway/layerzero-client.ts (9)
1-6: Consider makingeidrequired for better type safety.The
LayerZeroChainInfotype has optionaleidandnativeChainIdfields, but based on the LayerZero v2 documentation, every supported chain should have a valid endpoint ID. Makingeidrequired would prevent potential null pointer errors downstream.export type LayerZeroChainInfo = { name: string; - eid?: string; + eid: string; oftAddress: string; nativeChainId?: number; };
32-35: Add input validation for chainKey parameter.The function should validate that
chainKeyis not empty to prevent unnecessary API calls and provide better error handling.async getEidForChain(chainKey: string) { + if (!chainKey?.trim()) { + return null; + } const data = await this.getChainDeployments(); return data[chainKey]?.deployments?.find((item) => item.version === 2)?.eid || null; }
54-57: Add input validation for chainKey parameter.Similar to
getEidForChain, this function should validate the input parameter.async getOftAddressForChain(chainKey: string): Promise<string | null> { + if (!chainKey?.trim()) { + return null; + } const deployments = await this.getWbtcDeployments(); return deployments[chainKey]?.address || null; }
59-81: Handle potential missing chain information gracefully.Based on LayerZero documentation, endpoint IDs are unique identifiers for each chain endpoint. The current implementation may create entries with undefined
eidvalues, which could cause issues downstream.async getSupportedChainsInfo(): Promise<Array<LayerZeroChainInfo>> { const chains = await this.getChainDeployments(); const chainLookup = Object.fromEntries( Object.entries(chains).map(([_, chainData]) => [ chainData.chainKey, { eid: chainData.deployments?.find((item) => item.version === 2)?.eid, nativeChainId: chainData.chainDetails?.nativeChainId, }, ]) ); const deployments = await this.getWbtcDeployments(); - return Object.entries(deployments).map(([chainKey, deployment]) => { + return Object.entries(deployments).map(([chainKey, deployment]) => { const chainInfo = chainLookup[chainKey]; + const eid = chainInfo?.eid; + if (!eid) { + throw new Error(`Missing EID for chain: ${chainKey}`); + } return { name: chainKey, - eid: chainInfo?.eid, + eid, oftAddress: deployment.address, nativeChainId: chainInfo?.nativeChainId, }; - }); + }).filter(chain => chain.eid); // Additional safety filter }
83-93: Validate EID parameter format.LayerZero uses numeric IDs (e.g., 30101 for Ethereum, 30168 for Solana). The function should validate that the EID parameter is a valid numeric string.
async getChainId(eid: string): Promise<number | null> { + if (!eid?.trim() || !/^\d+$/.test(eid)) { + return null; + } + const chains = await this.getChainDeployments(); const chainId = Object.values(chains).find((chain) => chain.deployments?.some((deployment) => deployment.eid === eid))?.chainDetails?.nativeChainId; if (chainId) { return chainId; } return null; }
95-102: Improve error handling and type safety.The current error handling only throws generic status text, which may not be helpful for debugging. Also, the return type casting is unnecessary and potentially unsafe.
private async getJson<T>(url: string): Promise<T> { const response = await fetch(url); if (!response.ok) { - throw new Error(response.statusText); + throw new Error(`Failed to fetch ${url}: ${response.status} ${response.statusText}`); } - return (await response.json()) as Promise<T>; + return await response.json() as T; }
11-13: Consider making the base path configurable.The hardcoded LayerZero API endpoint should be configurable to support different environments (mainnet, testnet) or allow for API endpoint changes.
export class LayerZeroClient { private basePath: string; - constructor() { - this.basePath = 'https://metadata.layerzero-api.com/v1/metadata'; + constructor(basePath?: string) { + this.basePath = basePath ?? 'https://metadata.layerzero-api.com/v1/metadata'; }
37-47: Add error handling for malformed API response.The WBTC deployments endpoint structure could change, and the current code assumes a specific nested structure. Adding validation would prevent runtime errors.
private async getWbtcDeployments() { const params = new URLSearchParams({ symbols: 'WBTC', }); const data = await this.getJson<{ WBTC: [{ deployments: { [chainKey: string]: { address: string; }; }; }]; }>(`${this.basePath}/experiment/ofts/list?${params.toString()}`); + if (!data.WBTC?.[0]?.deployments) { + throw new Error('Invalid WBTC deployments response structure'); + } + return data.WBTC[0].deployments; }
8-102: Consider adding caching to reduce API calls.The LayerZero metadata is relatively static and doesn't change frequently. Adding a simple caching mechanism would improve performance and reduce API load, especially since multiple methods call the same endpoints.
Add a simple cache with TTL:
export class LayerZeroClient { private basePath: string; private cache = new Map<string, { data: any; expires: number }>(); private cacheTtl = 5 * 60 * 1000; // 5 minutes private isCacheValid(key: string): boolean { const cached = this.cache.get(key); return cached !== undefined && cached.expires > Date.now(); } private getCached<T>(key: string): T | null { if (this.isCacheValid(key)) { return this.cache.get(key)!.data as T; } return null; } private setCached(key: string, data: any): void { this.cache.set(key, { data, expires: Date.now() + this.cacheTtl }); } private async getJson<T>(url: string): Promise<T> { const cached = this.getCached<T>(url); if (cached) { return cached; } const response = await fetch(url); if (!response.ok) { throw new Error(`Failed to fetch ${url}: ${response.status} ${response.statusText}`); } const data = await response.json() as T; this.setCached(url, data); return data; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
sdk/src/gateway/client.ts(4 hunks)sdk/src/gateway/layerzero-client.ts(1 hunks)sdk/src/gateway/layerzero.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk/src/gateway/client.ts
🔇 Additional comments (1)
sdk/src/gateway/layerzero.ts (1)
16-16: LGTM! Clean separation of concerns.Moving LayerZero types and client logic to a separate module improves maintainability and reduces code duplication. The import clearly indicates the external dependency.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
sdk/src/gateway/layerzero.ts (1)
44-44: Consolidate LayerZeroClient usage (single shared instance or add caching)LayerZeroClient makes HTTP metadata calls (getChainDeployments / getWbtcDeployments / getEidForChain / getOftAddressForChain). The code currently creates both a class member and a local instance, causing redundant network requests — replace the local
new LayerZeroClient()with the existingthis.l0Clientor add memoization/singleton behavior in LayerZeroClient.
Locations: sdk/src/gateway/layerzero.ts —private l0Client(line 44), constructorthis.l0Client = new LayerZeroClient()(line 51), localconst layerZeroClient = new LayerZeroClient()(≈line 218).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
sdk/src/gateway/client.ts(4 hunks)sdk/src/gateway/layerzero-client.ts(1 hunks)sdk/src/gateway/layerzero.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- sdk/src/gateway/layerzero-client.ts
- sdk/src/gateway/client.ts
🔇 Additional comments (1)
sdk/src/gateway/layerzero.ts (1)
16-16: Good refactoring to prevent circular import dependencies.The import statement correctly brings in
LayerZeroChainInfoandLayerZeroClientfrom the new dedicated module, which helps maintain better code organization and prevents the circular import issues mentioned in the previous review comments.
mastilver
force-pushed
the
feat/gateway-v4-layerzero
branch
from
2b41035 to
d7d35d1
Compare
mastilver
force-pushed
the
feat/gateway-v4-layerzero
branch
from
d7d35d1 to
dcb76b1
Compare
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/layerzero.ts (1)
235-243: bytes32 bug: pad ‘to’ to 32 bytes or ABI encoding will fail.
quoteSend/sendexpectbytes32forto.padHex(addr)doesn’t set size; Viem will error for non‑32‑byte hex. Pad to 32 bytes.Apply this diff:
- const sendParam: SendParam = { + const sendParam: SendParam = { dstEid: parseInt(dstEid!, 10), - to: padHex(params.toUserAddress as Hex) as Hex, + to: padHex(params.toUserAddress as Hex, { size: 32 }) as Hex, amountLD: BigInt(0), // will be added inside the strategy minAmountLD: BigInt(0), // will be added inside the strategyzz extraOptions: extraOptions, composeMsg: '0x' as Hex, oftCmd: '0x' as Hex, };
🧹 Nitpick comments (4)
sdk/test/gateway.test.ts (3)
410-418: Good addition: covers L0 EID propagation on onramp order.As a small hardening, also assert the type to prevent regressions where EIDs become strings.
Apply this diff to extend the assertion:
{ ...mockOrder, baseTokenAddress: SYMBOL_LOOKUP[bob.id]['wbtc'], satoshis: 1000, fee: 0, status: true, layerzeroDstEid: 30111, },Add near the existing checks:
+ assert.strictEqual(typeof orders[7].layerzeroDstEid, 'number');
423-423: Assert the 8th entry is the L0 case for clarity.Length change is correct. Consider also asserting a distinguishing field (e.g., baseTokenAddress is wBTC) to make the intent explicit.
Example:
assert.lengthOf(orders, 8); +assert.strictEqual(orders[7].baseTokenAddress, SYMBOL_LOOKUP[bob.id]['wbtc']);
436-437: Also validate L0 details end-to-end (optional).If
getLayerZeroDetails/isLayerZerowere added in this PR, add assertions here to cover them too.I can wire a small test that, given
layerzeroDstEid: 30111, assertsisLayerZero === trueandgetLayerZeroDetails().chainId === optimism.id(mocking the L0 API).sdk/src/gateway/layerzero.ts (1)
112-124: Avoid truthy check; return explicitly when chainId is 0/undefined-safe.Use an explicit type check to future‑proof.
Apply this diff:
- if (chainId) { - return chainId; - } - - return null; + return typeof chainId === 'number' ? chainId : null;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
sdk/src/gateway/layerzero.ts(2 hunks)sdk/src/gateway/types.ts(1 hunks)sdk/test/gateway.test.ts(2 hunks)sdk/test/layerzero.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk/src/gateway/types.ts
🧰 Additional context used
🧬 Code graph analysis (3)
sdk/src/gateway/layerzero.ts (1)
sdk/src/gateway/client.ts (1)
chainId(169-171)
sdk/test/gateway.test.ts (1)
sdk/src/gateway/tokens.ts (1)
SYMBOL_LOOKUP(376-376)
sdk/test/layerzero.test.ts (1)
sdk/src/gateway/layerzero.ts (1)
LayerZeroClient(37-133)
⏰ 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 (2)
sdk/test/layerzero.test.ts (1)
14-70: Tests rely on live LayerZero metadata; mock these endpoints.This suite hits
https://metadata.layerzero-api.comand asserts exact EIDs/OFT addresses, which is brittle and can flake when metadata changes or the service blips. Please stub withnockand use fixed fixtures.Proposed setup (add once per suite):
import nock from 'nock'; import { afterEach, beforeAll } from 'vitest'; beforeAll(() => { nock.disableNetConnect(); }); afterEach(() => { nock.cleanAll(); nock.enableNetConnect(); }); // Example stubs used by "should get chains" nock('https://metadata.layerzero-api.com') .get('/v1/metadata') .reply(200, { ethereum: { chainKey: 'ethereum', deployments: [{ version: 2, eid: '30101' }], chainDetails: { nativeChainId: 1 } }, bob: { chainKey: 'bob', deployments: [{ version: 2, eid: '30279' }], chainDetails: { nativeChainId: 60808 } }, base: { chainKey: 'base', deployments: [{ version: 2, eid: '30184' }], chainDetails: { nativeChainId: 8453 } }, optimism: { chainKey: 'optimism', deployments: [{ version: 2, eid: '30111' }], chainDetails: { nativeChainId: 10 } }, // ...include only chains you assert on }); nock('https://metadata.layerzero-api.com') .get('/v1/metadata/experiment/ofts/list') .query({ symbols: 'WBTC' }) .reply(200, { WBTC: [{ deployments: { ethereum: { address: '0x0555e30da8f98308edb960aa94c0db47230d2b9c' }, bob: { address: '0x0555e30da8f98308edb960aa94c0db47230d2b9c' }, base: { address: '0x0555e30da8f98308edb960aa94c0db47230d2b9c' }, optimism: { address: '0xc3f854b2970f8727d28527ece33176fac67fef48' }, } }] });Optionally relax assertions to structure/containment instead of exact values if you don’t want to maintain fixtures.
sdk/src/gateway/layerzero.ts (1)
179-187: Thin delegator looks fine.
getChainIdForEidcorrectly proxies toLayerZeroClient. No issues.
| it('should get chain id for eid', async () => { | ||
| const client = new LayerZeroClient(); | ||
|
|
||
| const optimismEid = '30111'; | ||
|
|
||
| assert.equal(await client.getChainId(optimismEid), optimism.id); | ||
| }, 120000); |
There was a problem hiding this comment.
Mock the metadata call in this test; don’t hit the network.
getChainId('30111') currently fetches live data. Stub /v1/metadata with just the optimism entry.
Apply this diff inside the test:
it('should get chain id for eid', async () => {
- const client = new LayerZeroClient();
-
- const optimismEid = '30111';
-
- assert.equal(await client.getChainId(optimismEid), optimism.id);
+ const client = new LayerZeroClient();
+ const optimismEid = '30111';
+ nock('https://metadata.layerzero-api.com')
+ .get('/v1/metadata')
+ .reply(200, {
+ optimism: {
+ chainKey: 'optimism',
+ deployments: [{ version: 2, eid: optimismEid }],
+ chainDetails: { nativeChainId: optimism.id },
+ },
+ });
+ assert.equal(await client.getChainId(optimismEid), optimism.id);
}, 120000);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should get chain id for eid', async () => { | |
| const client = new LayerZeroClient(); | |
| const optimismEid = '30111'; | |
| assert.equal(await client.getChainId(optimismEid), optimism.id); | |
| }, 120000); | |
| it('should get chain id for eid', async () => { | |
| const client = new LayerZeroClient(); | |
| const optimismEid = '30111'; | |
| nock('https://metadata.layerzero-api.com') | |
| .get('/v1/metadata') | |
| .reply(200, { | |
| optimism: { | |
| chainKey: 'optimism', | |
| deployments: [{ version: 2, eid: optimismEid }], | |
| chainDetails: { nativeChainId: optimism.id }, | |
| }, | |
| }); | |
| assert.equal(await client.getChainId(optimismEid), optimism.id); | |
| }, 120000); |
🤖 Prompt for AI Agents
In sdk/test/layerzero.test.ts around lines 155 to 161, the test currently calls
client.getChainId('30111') which performs a live /v1/metadata network request;
replace that with a mocked response: before invoking getChainId, stub the GET
/v1/metadata endpoint to return a minimal JSON payload containing only the
optimism entry mapping eid '30111' to the expected chain id, then call
getChainId and assert the result; ensure the stub is scoped to this test (and
cleaned up/restored after) so no real network calls occur.
Add two new function
isLayerZeroandgetLayerZeroDetails.getLayerZeroDetailsexpose eid and if possible the chain id.Summary by CodeRabbit