feat: l0 - l0 swaps #808
feat: l0 - l0 swaps #808
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @Orland0x, 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 SDK's cross-chain capabilities by introducing a new client for direct LayerZero to LayerZero (L0-L0) token transfers. This functionality allows for seamless movement of OFT-compatible assets between various EVM chains, providing methods to both quote and execute these complex transactions. The update also includes necessary type definitions to support the new transaction flows. 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
|
|
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. WalkthroughReplaces the monolithic gateway types with modular type modules; adds LayerZero numeric-EID wiring and LayerZero-specific types; extends getQuote/executeQuote to support Onramp/Offramp/CrossChainSwap; introduces BaseClient, token slot refactoring, LayerZero gas-cost estimation, new utilities, and expanded tests/exports. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Consumer
participant Gateway as LayerZeroGatewayClient
participant Registry as Deployments API
participant OFT as OFT Contract
participant SrcRPC as Source Chain RPC
Note over App,Gateway: Request cross‑chain quote
App->>Gateway: getQuote(params + LayerZeroQuoteParamsExt)
Gateway->>Registry: fetch deployments & token data
Registry-->>Gateway: deployments (numeric EIDs, oftAddress)
alt deployment & oftAddress present
Gateway->>OFT: simulate composeMsg / estimate gas (getL0CreateOrderGasCost)
OFT-->>Gateway: fee & gas estimate
Gateway-->>App: CrossChainSwapQuote (feeBreakdown, oftAddress)
else missing eid/OFT or invalid params
Gateway-->>App: error (unsupported chain / invalid to)
end
Note over App,Gateway: Execute quote
App->>Gateway: executeQuote(ExecuteQuoteParams, signer/clients)
Gateway->>SrcRPC: writeContract / submit tx (build LayerZeroSendParam)
SrcRPC-->>Gateway: txHash / receipt
Gateway-->>App: execution result (txHash, fees, status)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a LayerZeroSendClient to handle LayerZero to LayerZero swaps. The implementation has some areas for improvement regarding correctness, efficiency, and code clarity. Key feedback points include addressing unused constructor parameters, fixing a bug where fromToken is ignored, adding slippage handling, and refactoring to remove duplicated logic between getQuote and executeQuote. Additionally, a minor optimization for the new test case is suggested.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
sdk/src/gateway/types.ts (1)
618-621: Clarify units for fees in docsDocument that nativeFee is in wei (origin chain) and lzTokenFee is LZ token wei to avoid ambiguity.
Apply this doc tweak:
-export interface LayerZeroSendQuote { +/** @description LayerZero messaging fee quote for a send. Units: nativeFee in wei (origin chain), lzTokenFee in LZ token wei (if applicable). */ +export interface LayerZeroSendQuote { nativeFee: bigint; lzTokenFee: bigint; }sdk/test/layerzero.test.ts (1)
156-191: Minor test cleanups and stronger assertions
- Reuse getSupportedChainsInfo result instead of calling twice.
- Assert quote.type === 'layerzero-send' to validate flow.
Apply:
- const quote = await client.getQuote({ + const chains = await client.getSupportedChainsInfo(); + const quote = await client.getQuote({ fromChain: 'base', - fromToken: (await client.getSupportedChainsInfo()).find((chain) => chain.name === 'base') - ?.oftAddress as string, + fromToken: chains.find((chain) => chain.name === 'base')?.oftAddress as string, toChain: 'optimism', - toToken: (await client.getSupportedChainsInfo()).find((chain) => chain.name === 'optimism') - ?.oftAddress as string, + toToken: chains.find((chain) => chain.name === 'optimism')?.oftAddress as string, fromUserAddress: '0xEf7Ff7Fb24797656DF41616e807AB4016AE9dCD5', toUserAddress: '0xEf7Ff7Fb24797656DF41616e807AB4016AE9dCD5', amount: 100, }); console.log('quote', quote); + assert.equal(quote.type, 'layerzero-send');sdk/src/gateway/layerzero.ts (5)
469-471: Unused constructor params (chainId, options)Either use these to configure clients or mark them unused to silence warnings.
Apply:
- constructor(chainId: number, options?: { rpcUrl?: string }) { + constructor(_chainId?: number, _options?: { rpcUrl?: string }) { this.l0Client = new LayerZeroClient(); }Alternatively, wire options.rpcUrl into the public client construction.
518-521: Remove unused srcEid fetchsrcEid is retrieved but never used.
Apply:
- const srcEid = await this.l0Client.getEidForChain(fromChain); - if (!srcEid) { - throw new Error(`Source EID not found for chain: ${fromChain}`); - }
542-559: Consider validating token params and documenting units
- getQuote ignores fromToken/toToken; optionally validate they match the WBTC OFT to prevent misuse.
- finalOutputSats uses Number(params.amount); ensure amounts stay within safe range (WBTC 8d is typically fine). Add a note if needed.
Would you like me to add lightweight validation and a comment on units?
564-568: Sanity-check publicClient is on origin chainAvoid sending from the wrong network; mirror the check used in LayerZeroGatewayClient.
Add before simulate/write:
+ const maybeFromChainId = quote.params.fromChain; + if (typeof maybeFromChainId === 'number' && publicClient.chain?.id !== maybeFromChainId) { + throw new Error('Public client must be origin chain'); + }
572-579: Minor: fromChain/toChain resolved but unusedIf not needed, remove to keep function concise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
sdk/src/gateway/layerzero.ts(2 hunks)sdk/src/gateway/types.ts(1 hunks)sdk/test/layerzero.test.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
sdk/src/gateway/layerzero.ts (4)
sdk/src/gateway/client.ts (2)
chainId(177-179)AllWalletClientParams(100-105)sdk/src/gateway/types.ts (2)
GetQuoteParams(591-591)ExecuteQuoteParams(624-627)sdk/src/gateway/utils.ts (2)
getChainConfig(181-188)viemClient(70-72)sdk/src/gateway/abi.ts (1)
layerZeroOftAbi(164-266)
sdk/test/layerzero.test.ts (1)
sdk/src/gateway/layerzero.ts (1)
LayerZeroSendClient(467-617)
🪛 GitHub Check: Tests
sdk/src/gateway/layerzero.ts
[warning] 480-480:
'options' is defined but never used
[warning] 480-480:
'chainId' is defined but never used
⏰ 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 (4)
sdk/src/gateway/types.ts (2)
613-616: Additive quote type looks goodNew LayerZeroSendQuoteParams extends BaseExecuteQuoteParams cleanly.
623-627: ExecuteQuoteParams union extension is correctUnion now includes LayerZero send; no breaking changes.
sdk/test/layerzero.test.ts (1)
2-2: Import addition LGTMImporting LayerZeroSendClient alongside others is fine.
sdk/src/gateway/layerzero.ts (1)
8-8: Import update LGTMUsing getChainConfig with viemClient is appropriate here.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sdk/src/gateway/index.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: Tests
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
sdk/src/gateway/client.ts (1)
1415-1415: Remove underscore prefix from parameter name.The
_userAddressparameter is actively used in the URL construction on line 1416, so the underscore prefix (typically indicating an intentionally unused parameter) is misleading.Apply this diff:
- async getLayerZeroSendOrders(_userAddress: Address): Promise<LayerZeroSendOrder[]> { + async getLayerZeroSendOrders(userAddress: Address): Promise<LayerZeroSendOrder[]> { - const url = new URL(`https://scan.layerzero-api.com/v1/messages/wallet/${_userAddress}`); + const url = new URL(`https://scan.layerzero-api.com/v1/messages/wallet/${userAddress}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
sdk/src/gateway/client.ts(2 hunks)sdk/src/gateway/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk/src/gateway/types.ts
🧰 Additional context used
🧬 Code graph analysis (1)
sdk/src/gateway/client.ts (1)
sdk/src/gateway/types.ts (3)
OnrampOrder(378-404)OfframpOrder(495-520)LayerZeroSendOrder(612-621)
⏰ 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/src/gateway/client.ts (2)
1389-1413: LGTM! Well-structured union type for orders.The method correctly extends the order types to include LayerZero sends and fetches all order types concurrently. The discriminated union pattern with type literals ('onramp', 'offramp', 'layerzero-send') enables proper type narrowing.
1440-1440: Confirm validlzCompose.statusvalues. The test API query returned no data, so please consult the LayerZero API docs or sample responses to ensure filtering by'N/A'accurately identifies the intended send orders.
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 (5)
sdk/src/gateway/layerzero.ts (3)
165-177: resolveChainName throws for most numeric chain IDsOnly BOB/BOB Sepolia numeric IDs work; others throw. Derive name from chain config to accept generic numeric IDs.
Apply:
function resolveChainName(chain: number | string): string { - if (typeof chain === 'number') { - switch (chain) { - case bob.id: - return bob.name.toLowerCase(); - case bobSepolia.id: - return bobSepolia.name.toLowerCase(); - default: - throw new Error(`Unsupported chain ID: ${chain}`); - } - } - return chain.toLowerCase(); + if (typeof chain === 'number') { + try { + const cfg = getChainConfig(chain); + return cfg.name.toLowerCase(); + } catch { + throw new Error(`Unsupported chain ID: ${chain}`); + } + } + return String(chain).toLowerCase(); }
261-269: bytes32 recipient must be 32 bytes: padHex needs sizeSendParam.to is bytes32. padHex without size can produce invalid length.
Apply:
to: padHex(params.toUserAddress as Hex) as Hex, + // ensure bytes32 + to: padHex(params.toUserAddress as Hex, { size: 32 }) as Hex,- to: padHex(params.toUserAddress as Hex) as Hex, + to: padHex(params.toUserAddress as Hex, { size: 32 }) as Hex,- to: padHex(params.toUserAddress as Hex) as Hex, + to: padHex(params.toUserAddress as Hex, { size: 32 }) as Hex,Optionally add a runtime guard to assert length === 66 including 0x.
Also applies to: 370-378, 524-532
241-246: Don’t hardcode WBTC; use the provided fromToken (OFT) when availableHardcoding WBTC restricts usage and ignores params.fromToken. Prefer the token specified by caller, fallback to WBTC only if absent.
Example:
- const wbtcOftAddress = await this.l0Client.getOftAddressForChain('bob'); + const oftAddress = (params.fromToken as Hex) ?? (await this.l0Client.getOftAddressForChain('bob')); - if (!wbtcOftAddress) { + if (!oftAddress) { throw new Error(`WBTC OFT not found for chain: ${fromChain}`); } ... - address: wbtcOftAddress as Hex, + address: oftAddress as Hex,Repeat similarly in the non-bitcoin → non-bitcoin quote and both execute paths. If you adopt this, also propagate oftAddress in the quote data so executeQuote doesn’t re-fetch it.
Also applies to: 361-364, 436-439, 519-522
sdk/src/gateway/client.ts (2)
204-233: Discriminant mismatch: compare against GatewayOrderType, not string literals in executeQuote.getQuote now returns
type: GatewayOrderType.*butexecuteQuotestill checks'onramp'/'offramp', causing the else/throw path at runtime. SwitchexecuteQuotecomparisons toGatewayOrderType.Onramp/GatewayOrderType.Offramp.Apply outside-range change (for reference):
// in executeQuote(...) if (quote.type === GatewayOrderType.Onramp) { // ... } else if (quote.type === GatewayOrderType.Offramp) { // ... }
414-421: Potential undefined fromUserAddress passed to fetchOfframpQuote.
params.fromUserAddress as Addressmay beundefined, resulting in"userAddress=undefined"in the query. Validate or default it (like in gas-cost method) before calling.- const quote = await this.fetchOfframpQuote( - tokenAddress, - BigInt(params.amount || 0), - params.fromUserAddress as Address, - params.toUserAddress - ); + const fromUser = params.fromUserAddress; + if (!fromUser) throw new Error('fromUserAddress is required for offramp quotes'); + const quote = await this.fetchOfframpQuote( + tokenAddress, + BigInt(params.amount || 0), + fromUser as Address, + params.toUserAddress + );
♻️ Duplicate comments (4)
sdk/src/gateway/layerzero.ts (2)
356-359: Remove unused srcEidFetched but not used; drop to reduce noise.
- const srcEid = await this.l0Client.getEidForChain(fromChain); - if (!srcEid) { - throw new Error(`Source EID not found for chain: ${fromChain}`); - }
387-398: Include oftAddress/dstEid in the returned quote to avoid re-fetch in executeQuoteexecuteQuote recomputes these; pass them in data to keep flows consistent.
return { type: GatewayOrderType.CrossChainSwap, finalOutputSats: Number(params.amount), finalFeeSats: 0, params, data: { + dstEid: parseInt(dstEid!, 10), + oftAddress: wbtcOftAddress as Hex, feeBreakdown: { nativeFee: sendFees.nativeFee, lzTokenFee: sendFees.lzTokenFee, }, }, };sdk/src/gateway/client.ts (2)
1430-1441: Guard payload access and document parsing; current code can throw on undefined payload.Accessing
payload.startsWithwill throw if payload is null/undefined; also the “last 16 hex chars” assumption is undocumented and fragile. Add guards, minimal validation (even length), and a brief comment.- let amount = 0n; - - const hex = payload.startsWith('0x') ? payload.slice(2) : payload; - if (hex.length >= 16) { - const last16 = hex.slice(-16); - try { - amount = BigInt('0x' + last16); - } catch { - amount = 0n; - } - } + let amount = 0n; + // LayerZero payload format: amount encoded in the last 8 bytes (16 hex chars). + // Validate minimally to avoid mis-parsing on format changes. + const payloadStr = typeof payload === 'string' ? payload : ''; + const hex = payloadStr.startsWith('0x') ? payloadStr.slice(2) : payloadStr; + if (hex.length >= 16 && hex.length % 2 === 0) { + const last16 = hex.slice(-16); + try { + amount = BigInt('0x' + last16); + } catch (e) { + console.warn('Failed to parse amount from LayerZero payload tail:', last16, e); + amount = 0n; + } + } else if (hex.length > 0) { + console.warn('Unexpected LayerZero payload length:', hex.length); + }This mirrors prior feedback to document/validate the parsing. [Based on learnings]
1447-1457: Handle nullable/variant EIDs; avoid unsafe assignment to number fields.
srcEid/dstEidmay benullor string per upstream API;CrossChainOrder.source/destination.eidexpectsnumber. Normalize and drop invalid items.- source: { - eid: item.pathway.srcEid, + source: { + eid: typeof item.pathway.srcEid === 'string' + ? parseInt(item.pathway.srcEid, 10) + : (item.pathway.srcEid as number), txHash: sourceTxHash, token: item.pathway.sender.address as Address, }, - destination: { - eid: item.pathway.dstEid, + destination: { + eid: typeof item.pathway.dstEid === 'string' + ? parseInt(item.pathway.dstEid, 10) + : (item.pathway.dstEid as number), txHash: destinationTxHash, token: item.pathway.receiver.address as Address, },Optionally skip items with missing EIDs:
// after building array }).filter(o => Number.isFinite(o.source.eid) && Number.isFinite(o.destination.eid));Also consider coercing the timestamp explicitly:
timestamp: Number(blockTimestamp).
🧹 Nitpick comments (10)
sdk/src/gateway/types/quote.ts (1)
28-28: Consider documenting expected amount formats.The
amountfield acceptsnumber | string | bigint, which provides flexibility but may lead to inconsistent handling across consumers. Consider adding JSDoc examples showing the expected format for each type (e.g., whether string should be decimal or integer representation).sdk/src/gateway/types/strategy.ts (1)
112-120: Consider using viem's Address type for consistency.The internal
GatewayStrategyinterface usesstringfor address fields (lines 113, 118, 119), while the publicGatewayStrategyContractandStrategyParamsuse viem'sAddresstype. This inconsistency could lead to type confusion.Consider standardizing on
Addressfor all address fields throughout the module, or document why internal interfaces use plain strings.sdk/src/gateway/utils/layerzero.ts (1)
3-15: Refactor nested ternary for readability.The deeply nested ternary expression is difficult to read and maintain. Consider refactoring to if/else statements or extracting status mapping logic.
Apply this diff to improve readability:
-export const getCrossChainStatus = (item: LayerZeroMessageWallet) => { - return item.source.status === 'WAITING' - ? 'source-pending' - : item.source.status === 'SIMULATION_REVERTED' - ? 'source-failed' - : item.source.status === 'SUCCEEDED' && item.destination.status === 'WAITING' - ? 'destination-pending' - : item.source.status === 'SUCCEEDED' && item.destination.status === 'SUCCEEDED' - ? 'destination-confirmed' - : item.source.status === 'SUCCEEDED' && item.destination.status === 'SIMULATION_REVERTED' - ? 'destination-failed' - : 'unknown'; -}; +export const getCrossChainStatus = (item: LayerZeroMessageWallet) => { + const { source, destination } = item; + + if (source.status === 'WAITING') { + return 'source-pending'; + } + + if (source.status === 'SIMULATION_REVERTED') { + return 'source-failed'; + } + + if (source.status === 'SUCCEEDED') { + if (destination.status === 'WAITING') { + return 'destination-pending'; + } + if (destination.status === 'SUCCEEDED') { + return 'destination-confirmed'; + } + if (destination.status === 'SIMULATION_REVERTED') { + return 'destination-failed'; + } + } + + return 'unknown'; +};sdk/src/gateway/types/bitcoin.ts (1)
1-10: Consider using Hex type for opReturn field.The
opReturnfield insendBitcoinparams is typed asstring, but OP_RETURN data is typically hex-encoded. Consider using viem'sHextype for consistency with other hex data in the codebase (e.g.,messagefield inGatewayQuoteParams).Apply this diff:
+import { Hex } from 'viem'; + export interface BitcoinSigner { signAllInputs?: (psbtBase64: string) => Promise<string>; sendBitcoin?: (params: { from: string; to: string; value: string; - opReturn: string; + opReturn: Hex; isSignet: boolean; }) => Promise<string>; }sdk/src/gateway/types/crosschain-swap.ts (1)
36-43: Consider carrying oftAddress/dstEid in the quote dataexecuteQuote re-fetches these; include in CrossChainSwapQuote to avoid duplication and ensure consistency.
Apply:
export interface CrossChainSwapQuote { - feeBreakdown: CrossChainFeeBreakdown; + feeBreakdown: CrossChainFeeBreakdown; + /** @description Destination LayerZero EID used for the send */ + dstEid?: number; + /** @description Origin OFT address used for the send */ + oftAddress?: Address; }sdk/src/gateway/layerzero.ts (3)
370-375: Set a realistic minAmountLD (slippage tolerance)minAmountLD equals amountLD (zero slippage) risks reverts. Honor an optional slippage input, default to e.g. 1%.
Example:
- minAmountLD: BigInt(params.amount), + // default 1% slippage if not provided + minAmountLD: BigInt( + Math.floor( + Number(params.amount) * (1 - (Number((params as any).maxSlippage ?? 0.01))) + ) + ),Adjust to your actual param naming (e.g., maxSlippage or slippageBps).
Also applies to: 524-529
423-426: Reuse the class client instead of instantiating a new oneUse this.l0Client to leverage caching and reduce overhead.
- const layerZeroClient = new LayerZeroClient(); + const layerZeroClient = this.l0Client;
288-309: Quoter/addresses: lift to constants/config and add a brief docHardcoded Uniswap Quoter and WETH addresses are fine for OP-stack but should be centralized and documented for clarity and future changes.
Move:
- QuoterV2 address
- WETH address (0x4200...06)
- Strategy and Offramp composer addresses
to a constants module with comments and network notes.sdk/src/gateway/client.ts (1)
248-322: Gas estimate stateOverride: RPC support and param mutation risks.
- stateOverride in estimateContractGas may not be supported on all RPCs; confirm BOB RPCs accept it, otherwise wrap with fallback or feature-detect.
- Avoid mutating
params(lines 252-263). Copy defaults to locals to prevent side effects for callers.Suggested localizing defaults:
- if (!params.toUserAddress) { - params.toUserAddress = this.isSignet - ? 'tb1q0c2qnya702wrna5hqjp83jqqhx8zh5p9au2rqt' - : '14EvE4gm1yiYSzN8dYBtgYDppsaa1VVfud'; - } + const toUserAddress = params.toUserAddress ?? + (this.isSignet + ? 'tb1q0c2qnya702wrna5hqjp83jqqhx8zh5p9au2rqt' + : '14EvE4gm1yiYSzN8dYBtgYDppsaa1VVfud'); - if ( - !params.fromUserAddress || - (isAddress(params.fromUserAddress) && isAddressEqual(params.fromUserAddress, zeroAddress)) - ) { - params.fromUserAddress = '0x1111111111111111111111111111111111111111'; - } + const fromUserAddress = + !params.fromUserAddress || + (isAddress(params.fromUserAddress) && isAddressEqual(params.fromUserAddress, zeroAddress)) + ? ('0x1111111111111111111111111111111111111111' as Address) + : (params.fromUserAddress as Address);And pass
fromUserAddress/toUserAddresslocals where used.Please also verify
stateOverridesupport on your RPCs; if unsupported, estimate without overrides and document assumptions. [Based on learnings]sdk/src/gateway/types/offramp.ts (1)
31-45: Consider bigint for satoshi amounts to avoid precision loss.
amountLockInSat/amountReceiveInSatarenumber. For large orders, JS number precision can truncate; considerbigint(as used elsewhere) for sat values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
sdk/src/gateway/client.ts(8 hunks)sdk/src/gateway/index.ts(1 hunks)sdk/src/gateway/layerzero.ts(4 hunks)sdk/src/gateway/types.ts(0 hunks)sdk/src/gateway/types/bitcoin.ts(1 hunks)sdk/src/gateway/types/crosschain-swap.ts(1 hunks)sdk/src/gateway/types/defiLlama.ts(1 hunks)sdk/src/gateway/types/index.ts(1 hunks)sdk/src/gateway/types/layerzero.ts(1 hunks)sdk/src/gateway/types/offramp.ts(1 hunks)sdk/src/gateway/types/onramp.ts(1 hunks)sdk/src/gateway/types/order.ts(1 hunks)sdk/src/gateway/types/quote.ts(1 hunks)sdk/src/gateway/types/strategy.ts(1 hunks)sdk/src/gateway/types/token.ts(1 hunks)sdk/src/gateway/types/utils.ts(1 hunks)sdk/src/gateway/utils/common.ts(1 hunks)sdk/src/gateway/utils/index.ts(1 hunks)sdk/src/gateway/utils/layerzero.ts(1 hunks)sdk/test/gateway.test.ts(11 hunks)
💤 Files with no reviewable changes (1)
- sdk/src/gateway/types.ts
✅ Files skipped from review due to trivial changes (1)
- sdk/test/gateway.test.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/types/quote.ts
🧬 Code graph analysis (10)
sdk/src/gateway/types/defiLlama.ts (1)
sdk/src/gateway/types.ts (1)
DefiLlamaPool(578-588)
sdk/src/gateway/types/crosschain-swap.ts (1)
sdk/src/gateway/types/quote.ts (1)
BaseExecuteQuoteParams(60-64)
sdk/src/gateway/utils/layerzero.ts (1)
sdk/src/gateway/types/layerzero.ts (1)
LayerZeroMessageWallet(49-53)
sdk/src/gateway/types/strategy.ts (1)
sdk/src/gateway/index.ts (1)
GatewayStrategyContract(12-12)
sdk/src/gateway/types/order.ts (3)
sdk/src/gateway/types/onramp.ts (1)
OnrampOrder(124-150)sdk/src/gateway/types/offramp.ts (1)
OfframpOrder(82-107)sdk/src/gateway/types/crosschain-swap.ts (1)
CrossChainOrder(14-28)
sdk/src/gateway/client.ts (5)
sdk/src/gateway/types/onramp.ts (1)
OnrampOrderStatus(103-121)sdk/src/gateway/types/order.ts (1)
GatewayOrder(89-92)sdk/src/gateway/types/crosschain-swap.ts (1)
CrossChainOrder(14-28)sdk/src/gateway/types/layerzero.ts (1)
LayerZeroMessagesWalletResponse(55-57)sdk/src/gateway/utils/layerzero.ts (1)
getCrossChainStatus(3-15)
sdk/src/gateway/types/offramp.ts (2)
sdk/src/gateway/abi.ts (1)
offrampCaller(41-153)sdk/src/gateway/types/quote.ts (1)
BaseExecuteQuoteParams(60-64)
sdk/src/gateway/types/onramp.ts (4)
sdk/src/gateway/types/order.ts (2)
OrderDetails(21-44)OrderDetailsRaw(6-19)sdk/src/gateway/types/token.ts (2)
Token(7-14)GatewayTokensInfo(29-34)sdk/src/esplora.ts (1)
EsploraClient(154-517)sdk/src/gateway/types/quote.ts (1)
BaseExecuteQuoteParams(60-64)
sdk/src/gateway/layerzero.ts (2)
sdk/src/gateway/utils/common.ts (3)
getChainConfig(192-199)viemClient(81-83)toHexScriptPubKey(52-56)sdk/src/gateway/abi.ts (1)
layerZeroOftAbi(164-266)
sdk/src/gateway/types/quote.ts (4)
sdk/src/gateway/types/utils.ts (1)
Optional(1-1)sdk/src/gateway/types/onramp.ts (1)
OnrampExecuteQuoteParams(153-156)sdk/src/gateway/types/offramp.ts (1)
OfframpExecuteQuoteParams(148-151)sdk/src/gateway/types/crosschain-swap.ts (1)
CrossChainSwapQuoteParams(31-34)
🔇 Additional comments (18)
sdk/src/gateway/types/strategy.ts (2)
73-91: LGTM! Well-structured Swing SDK compatible interface.The
GatewayStrategyContractinterface is well-documented with comprehensive JSDoc comments and appropriate typing. The use of viem'sAddresstype ensures type safety for Ethereum addresses.
96-109: LGTM! Appropriate types for strategy parameters.The
StrategyParamstype correctly usesbigintfor token amounts, which avoids precision issues with large values. Good use of viem'sAddresstype for type safety.sdk/src/gateway/types/utils.ts (1)
1-1: LGTM! Clean utility type.The
Optional<T, K>utility type is a well-known TypeScript pattern for making specific keys optional while preserving the rest of the type. This is appropriately used inGetQuoteParamsto makefromUserAddressoptional.sdk/src/gateway/utils/index.ts (1)
1-2: LGTM! Standard barrel export pattern.This index module provides a clean, consolidated entry point for gateway utilities from both common and layerzero modules.
sdk/src/gateway/utils/common.ts (5)
29-50: LGTM! Proper encoding for OP_RETURN hash.The
calculateOpReturnHashfunction correctly uses viem's ABI encoding utilities with appropriate fallbacks for optional fields (zeroAddress,'0x'). The comment indicating compatibility with Gateway API and smart contracts is helpful.
73-79: LGTM! Status parsing with appropriate error handling.The
parseOrderStatusfunction correctly maps numeric values to status strings with proper error handling for invalid indices.
89-139: LGTM! Correct order and fee conversion utilities.The conversion utilities properly handle string ↔ bigint transformations, with appropriate null handling and correct Bitcoin decimal places (8).
141-199: LGTM! Robust chain configuration utilities.The chain configuration functions provide clear error messages with allowed values when a chain is not found. The dual support for string names and numeric IDs in
getChainConfigis well-implemented.
201-239: LGTM! Correct ERC20 storage slot computation.The
computeAllowanceSlotandcomputeBalanceSlotfunctions correctly implement Solidity's mapping storage layout using nested keccak256 hashing with proper ABI encoding.sdk/src/gateway/types/defiLlama.ts (1)
1-12: LGTM! Clean interface for DeFi Llama pool data.The
DefiLlamaPoolinterface appropriately types all fields with proper nullable markers for optional data (apy, apyBase, apyReward, underlyingTokens, rewardTokens). This appears to be a refactoring move from the monolithic types.ts file.sdk/src/gateway/types/quote.ts (1)
61-62: No precision risk for finalOutputSats/finalFeeSats number types JavaScript’snumbersafely represents up to ~9×10^15, covering all possible satoshi values (<2.1×10^15).sdk/src/gateway/index.ts (1)
2-18: Public exports look consistentRe-exports align with new types surface and keep the SDK entry stable. LGTM.
sdk/src/gateway/client.ts (3)
1212-1223: Onramp getStatus return shape aligns with new OnrampOrderStatus.Logic is consistent and concise.
1398-1410: Unified getOrders looks good.Straightforward aggregation with correct discriminants.
1424-1425: Filter criterion ‘lzCompose.status === "N/A"’ needs validation.Hard-coding
'N/A'as the compose status filter is brittle. Confirm this is the intended condition for “send-only” or pre-compose messages and document the rationale.sdk/src/gateway/types/offramp.ts (1)
147-151: OfframpExecuteQuoteParams discriminant looks correct.Matches GatewayOrderType.Offramp union usage.
sdk/src/gateway/types/onramp.ts (2)
20-25: Bigint fee fields are good; ensure conversion utilities exist.Types promote wei fields to bigint; confirm callers convert from raw strings consistently (e.g., via BigInt or viem parsers).
153-156: OnrampExecuteQuoteParams discriminant aligns with GatewayOrderType.Onramp.Looks consistent with client changes.
danielsimao
force-pushed
the
feat/l0-l0-swaps
branch
from
a5b5b3e to
79f76de
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (8)
sdk/src/gateway/index.ts (1)
1-18: Verify test coverage for LayerZeroSendClient.The export additions look correct. However, a past review comment noted that LayerZero integration tests are skipped. Please ensure adequate test coverage is in place before merging.
Based on learnings
Run the following script to check the current test status:#!/bin/bash # Description: Check for skipped LayerZero tests # Search for skipped tests in layerzero test files rg -n "\.skip|describe\.skip|it\.skip" --type ts -g "*layerzero*test*" -C 2sdk/src/gateway/types/quote.ts (1)
67-70: Discriminated union conflicts with codebase requirement.
ExecuteQuoteParamsis a discriminated union type (using thetypefield as a discriminator), which contradicts a documented codebase requirement from PR #634: "The codebase has a requirement not to introduce discriminated unions in the Gateway SDK client methods, including the getQuote method return types."Based on learnings
If this requirement is still valid, refactor to use a non-discriminated approach or document why this exception is necessary.
sdk/src/gateway/layerzero.ts (5)
334-337: Use params.fromToken instead of hardcoding WBTC OFT lookup.The code ignores
params.fromTokenand always fetches the WBTC OFT address for the origin chain. This limits the function to WBTC-only transfers and breaks the contract offromTokenbeing honored.Apply this diff:
- const wbtcOftAddress = await this.l0Client.getOftAddressForChain(fromChain); + const oftAddress = params.fromToken as string; + if (!oftAddress) { + throw new Error(`fromToken (OFT address) must be provided for a LayerZero send.`); + } - if (!wbtcOftAddress) { + if (!oftAddress) { throw new Error(`WBTC OFT not found for chain: ${fromChain}`); }
345-345: Add size parameter to padHex for bytes32 recipient.
SendParam.toexpects abytes32value, butpadHexwithout a size parameter won't guarantee 32 bytes. This will cause contract call failures.Apply this diff:
- to: padHex(params.toUserAddress as Hex) as Hex, + to: padHex(params.toUserAddress as Hex, { size: 32 }) as Hex,
347-347: Add slippage tolerance to minAmountLD.Setting
minAmountLDequal toamountLDmeans zero slippage tolerance, which will cause transactions to revert in volatile market conditions. Useparams.maxSlippageto calculate an acceptable minimum.Apply this diff:
- minAmountLD: BigInt(params.amount), + minAmountLD: BigInt(Math.floor(Number(params.amount) * (1 - (params.maxSlippage ?? 0.01)))), // Default 1% slippage
499-499: Add size parameter to padHex for bytes32 recipient (execute path).Same critical issue as in
getQuote:SendParam.toexpects abytes32value. Without the size parameter,padHexwon't guarantee 32 bytes.Apply this diff:
- to: padHex(params.toUserAddress as Hex) as Hex, + to: padHex(params.toUserAddress as Hex, { size: 32 }) as Hex,
329-332: Remove unused srcEid variable.
srcEidis fetched and validated but never used in the subsequent logic. This is dead code that should be removed for clarity.Apply this diff:
- const srcEid = await this.l0Client.getEidForChain(fromChain); - if (!srcEid) { - throw new Error(`Source EID not found for chain: ${fromChain}`); - } -sdk/src/gateway/client.ts (1)
1433-1443: Document and validate the payload parsing logic.The code assumes the order size is encoded in the last 16 hex characters (8 bytes) of the payload without documentation or structural validation. If the LayerZero payload format changes, this could silently return incorrect values.
Apply this diff to add documentation and validation:
let amount = 0n; + // LayerZero payload format: the order size is encoded in the last 8 bytes (16 hex chars) const hex = payload.startsWith('0x') ? payload.slice(2) : payload; - if (hex.length >= 16) { + // Validate minimum expected payload length and even number of hex chars + if (hex.length >= 16 && hex.length % 2 === 0) { const last16 = hex.slice(-16); try { amount = BigInt('0x' + last16); - } catch { + } catch (err) { + console.warn('Failed to parse order size from LayerZero payload:', err); amount = 0n; } }
🧹 Nitpick comments (1)
sdk/src/gateway/utils/layerzero.ts (1)
3-15: Refactor nested ternary for better readability.The deeply nested ternary operators reduce code readability and make maintenance harder. Consider refactoring to use if-else statements or an early-return pattern.
Apply this diff to improve readability:
export const getCrossChainStatus = (item: LayerZeroMessageWallet) => { - return item.source.status === 'WAITING' - ? 'source-pending' - : item.source.status === 'SIMULATION_REVERTED' - ? 'source-failed' - : item.source.status === 'SUCCEEDED' && item.destination.status === 'WAITING' - ? 'destination-pending' - : item.source.status === 'SUCCEEDED' && item.destination.status === 'SUCCEEDED' - ? 'destination-confirmed' - : item.source.status === 'SUCCEEDED' && item.destination.status === 'SIMULATION_REVERTED' - ? 'destination-failed' - : 'unknown'; + if (item.source.status === 'WAITING') { + return 'source-pending'; + } + if (item.source.status === 'SIMULATION_REVERTED') { + return 'source-failed'; + } + if (item.source.status === 'SUCCEEDED') { + if (item.destination.status === 'WAITING') { + return 'destination-pending'; + } + if (item.destination.status === 'SUCCEEDED') { + return 'destination-confirmed'; + } + if (item.destination.status === 'SIMULATION_REVERTED') { + return 'destination-failed'; + } + } + return 'unknown'; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
sdk/src/gateway/client.ts(8 hunks)sdk/src/gateway/index.ts(1 hunks)sdk/src/gateway/layerzero.ts(6 hunks)sdk/src/gateway/types.ts(0 hunks)sdk/src/gateway/types/bitcoin.ts(1 hunks)sdk/src/gateway/types/crosschain-swap.ts(1 hunks)sdk/src/gateway/types/defiLlama.ts(1 hunks)sdk/src/gateway/types/index.ts(1 hunks)sdk/src/gateway/types/layerzero.ts(1 hunks)sdk/src/gateway/types/offramp.ts(1 hunks)sdk/src/gateway/types/onramp.ts(1 hunks)sdk/src/gateway/types/order.ts(1 hunks)sdk/src/gateway/types/quote.ts(1 hunks)sdk/src/gateway/types/strategy.ts(1 hunks)sdk/src/gateway/types/token.ts(1 hunks)sdk/src/gateway/types/utils.ts(1 hunks)sdk/src/gateway/utils/common.ts(1 hunks)sdk/src/gateway/utils/index.ts(1 hunks)sdk/src/gateway/utils/layerzero.ts(1 hunks)sdk/test/gateway.test.ts(11 hunks)sdk/test/layerzero.test.ts(2 hunks)
💤 Files with no reviewable changes (1)
- sdk/src/gateway/types.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- sdk/test/layerzero.test.ts
- sdk/src/gateway/types/crosschain-swap.ts
- sdk/src/gateway/utils/index.ts
- sdk/src/gateway/types/bitcoin.ts
- sdk/src/gateway/types/defiLlama.ts
- sdk/src/gateway/types/order.ts
- sdk/src/gateway/types/index.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/types/quote.ts
🧬 Code graph analysis (7)
sdk/src/gateway/types/offramp.ts (2)
sdk/src/gateway/abi.ts (1)
offrampCaller(41-153)sdk/src/gateway/types/quote.ts (1)
BaseExecuteQuoteParams(60-64)
sdk/src/gateway/utils/layerzero.ts (1)
sdk/src/gateway/types/layerzero.ts (1)
LayerZeroMessageWallet(51-55)
sdk/src/gateway/types/strategy.ts (1)
sdk/src/gateway/index.ts (1)
GatewayStrategyContract(12-12)
sdk/src/gateway/types/quote.ts (4)
sdk/src/gateway/types/utils.ts (1)
Optional(1-1)sdk/src/gateway/types/onramp.ts (1)
OnrampExecuteQuoteParams(153-156)sdk/src/gateway/types/offramp.ts (1)
OfframpExecuteQuoteParams(148-151)sdk/src/gateway/types/crosschain-swap.ts (1)
CrossChainSwapQuoteParams(31-34)
sdk/src/gateway/layerzero.ts (3)
sdk/src/gateway/types/layerzero.ts (1)
LayerZeroSendParam(61-69)sdk/src/gateway/utils/common.ts (3)
getChainConfig(192-199)viemClient(81-83)toHexScriptPubKey(52-56)sdk/src/gateway/abi.ts (1)
layerZeroOftAbi(164-266)
sdk/src/gateway/types/onramp.ts (4)
sdk/src/gateway/types/order.ts (2)
OrderDetails(21-44)OrderDetailsRaw(6-19)sdk/src/gateway/types/token.ts (2)
Token(7-14)GatewayTokensInfo(29-34)sdk/src/esplora.ts (1)
EsploraClient(154-517)sdk/src/gateway/types/quote.ts (1)
BaseExecuteQuoteParams(60-64)
sdk/src/gateway/client.ts (5)
sdk/src/gateway/types/onramp.ts (1)
OnrampOrderStatus(103-121)sdk/src/gateway/types/order.ts (1)
GatewayOrder(89-92)sdk/src/gateway/types/crosschain-swap.ts (1)
CrossChainOrder(14-28)sdk/src/gateway/types/layerzero.ts (1)
LayerZeroMessagesWalletResponse(57-59)sdk/src/gateway/utils/layerzero.ts (1)
getCrossChainStatus(3-15)
⏰ 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 (11)
sdk/src/gateway/utils/common.ts (1)
1-23: LGTM! Import surface expanded appropriately.The expanded viem imports, additional chain presets, and bitcoinjs-lib integration align well with the cross-chain swap functionality being introduced. The path change from
./typesto../typescorrectly reflects the file structure reorganization.sdk/src/gateway/types/strategy.ts (1)
1-126: LGTM! Well-structured strategy types.The type definitions are comprehensive and well-documented. The Swing SDK compatibility note is helpful context. The use of viem's
Addresstype ensures type safety for blockchain addresses.sdk/src/gateway/types/token.ts (1)
1-34: LGTM! Token types are well-defined.The type definitions are comprehensive and clearly documented. The Superchain token list compatibility note provides useful context for integration.
sdk/test/gateway.test.ts (2)
16-16: LGTM! Improved type safety with enum.Using
GatewayOrderTypeenum instead of string literals improves type safety and makes the test expectations more maintainable.
954-1423: LGTM! Comprehensive cross-chain order testing.The test suite thoroughly covers cross-chain swap orders with:
- Multiple status scenarios (destination-confirmed, destination-pending, source-pending, source-failed)
- Filtering logic (lzCompose.status != 'N/A')
- Edge cases (empty payload, invalid hex parsing)
- Error handling (API errors, network errors)
- Empty result handling
This comprehensive coverage ensures the cross-chain functionality is well-tested.
sdk/src/gateway/types/quote.ts (1)
14-54: LGTM! Comprehensive quote parameters.The
GatewayQuoteParamsinterface is well-documented and provides flexibility with optional fields while maintaining required core identifiers. The Swing SDK compatibility note is helpful for integration.sdk/src/gateway/types/utils.ts (1)
1-1: Fix the Optional type definition.The current implementation
Omit<T, K> & Partial<T>makes all properties ofToptional in the intersection, not just the keysK. This can lead to incorrect type inference.Apply this diff to fix the type:
-export type Optional<T, K extends keyof T> = Omit<T, K> & Partial<T>; +export type Optional<T, K extends keyof T> = Omit<T, K> & Partial<Pick<T, K>>;This ensures only the keys
Kare made optional, while preserving the rest ofTas required.Likely an incorrect or invalid review comment.
sdk/src/gateway/client.ts (1)
204-204: LGTM: Type-safe order discriminants.Using
GatewayOrderTypeenum values instead of string literals provides better type safety and aligns with the unifiedGatewayOrdermodel.Also applies to: 221-221
sdk/src/gateway/types/offramp.ts (1)
1-160: LGTM: Clean and well-documented type definitions.The offramp type definitions are comprehensive, well-structured, and properly documented. The clear separation between raw/processed types (e.g.,
OfframpRawOrdervsOfframpOrder) and the detailed JSDoc comments make the codebase more maintainable.sdk/src/gateway/types/layerzero.ts (1)
1-106: LGTM: Well-structured LayerZero type definitions.The type definitions provide a clean abstraction for LayerZero messaging with appropriate separation of internal vs public types. The metadata structures (
LayerZeroDeploymentsMetadata,LayerZeroTokenDeployments) align well with the LayerZero API data model.sdk/src/gateway/types/onramp.ts (1)
1-156: LGTM: Comprehensive onramp type definitions.The onramp types are well-structured with clear transformation patterns (raw string values to bigint), type-safe discriminated unions for status, and useful helper methods on
OnrampOrder. The deprecation notices on legacy methods guide users toward the updated API.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
sdk/src/gateway/tokens.ts (1)
469-476: RenamechainIdparameter tonetworkKey. All call sites already pass string network keys (e.g. literals,resolveChainName, ternary), so updating the signature and its usages clarifies intent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sdk/src/gateway/tokens.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
sdk/src/gateway/tokens.ts (1)
sdk/src/gateway/client.ts (1)
chainId(181-183)
⏰ 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/tokens.ts (1)
185-197: LGTM! WBTC entry correctly structured.The Wrapped BTC entry follows the existing pattern and includes the required
allowanceSlotandbalanceSlotfields for storage slot calculations.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
sdk/src/gateway/layerzero.ts (7)
372-372: Add explicit size for bytes32 recipient field.The
tofield inSendParamexpects a bytes32 value (32 bytes). Without specifying the size,padHexmay not produce the correct length.Apply this diff:
- to: padHex(params.toUserAddress as Hex), + to: padHex(params.toUserAddress as Hex, { size: 32 }),
522-522: Add explicit size for bytes32 recipient field.The
tofield inSendParamexpects a bytes32 value. Without specifying the size,padHexmay not produce the correct 32-byte length.Apply this diff:
- to: padHex(params.toUserAddress as Hex) as Hex, + to: padHex(params.toUserAddress as Hex, { size: 32 }) as Hex,
361-364: Useparams.fromTokeninstead of hardcoding WBTC lookup.The current implementation ignores
params.fromTokenand hardcodes the WBTC OFT address lookup by chain. This limits the function to WBTC only and makes thefromTokenparameter misleading. The implementation should useparams.fromTokenas the OFT address.Apply this diff:
- const wbtcOftAddress = await this.l0Client.getOftAddressForChain(fromChain); - if (!wbtcOftAddress) { - throw new Error(`WBTC OFT not found for chain: ${fromChain}`); - } + const oftAddress = params.fromToken as Hex; + if (!oftAddress || !isAddress(oftAddress)) { + throw new Error(`fromToken (OFT address) must be a valid address for LayerZero swaps.`); + }Then update all references from
wbtcOftAddresstooftAddressin this code block.
374-374: Zero slippage tolerance is risky.Setting
minAmountLDequal toamountLDmeans zero tolerance for slippage, which can cause transaction failures in volatile conditions. TheGetQuoteParamstype includes an optionalmaxSlippageproperty that should be used here.Apply this diff to use a default 1% slippage if not specified:
- minAmountLD: BigInt(params.amount), + minAmountLD: BigInt(Math.floor(Number(params.amount) * (1 - (params.maxSlippage ?? 0.01)))),
161-162: Usemainnet.name.toLowerCase()for consistency.The current code returns the hardcoded string
'mainnet', but LayerZero expects'ethereum'as the metadata key. For consistency with the existing pattern used forbobandbobSepolia, usemainnet.name.toLowerCase()which returns'ethereum'.Apply this diff:
case mainnet.id: - return 'mainnet'; + return mainnet.name.toLowerCase();
608-613: Use correct spender address incomputeAllowanceSlot.The allowance slot computation uses
wbtcMainnetAddressas the spender, but the actual spender is thewbtcOftAddress(the OFT contract that pulls the underlying token). This will result in an incorrect allowance override.Apply this diff:
const allowanceSlot = computeAllowanceSlot( user as Address, - wbtcMainnetAddress as Address, + wbtcOftAddress as Address, wbtcMainnetSlots.allowanceSlot );
603-603: Pass chain ID instead of chain key togetTokenSlots.
getTokenSlotsexpects a numericchainIdparameter, butfromChainis a lowercase string like'bob'or'base'. This will cause a runtime error.You need to resolve the chain key to a numeric chain ID. Apply this approach:
- const wbtcOftSlots = getTokenSlots(wbtcOftAddress as Address, fromChain); + const chainConfig = getChainConfig(params.l0ChainId ?? params.fromChain); + const wbtcOftSlots = getTokenSlots(wbtcOftAddress as Address, chainConfig.chainId.toString());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sdk/src/gateway/layerzero.ts(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
sdk/src/gateway/layerzero.ts (7)
sdk/src/gateway/types/layerzero.ts (5)
LayerZeroDeploymentsMetadataResponse(78-91)LayerZeroTokenDeploymentsResponse(93-97)LayerZeroSendParam(61-69)LayerZeroQuoteParamsExt(99-106)LayerZeroMessagesWalletResponse(57-59)sdk/src/gateway/utils/common.ts (5)
getChainConfig(192-199)viemClient(81-83)toHexScriptPubKey(52-56)computeBalanceSlot(226-239)computeAllowanceSlot(202-223)sdk/src/gateway/abi.ts (1)
layerZeroOftAbi(164-266)sdk/src/gateway/tokens.ts (2)
getTokenAddress(460-468)getTokenSlots(469-491)sdk/src/gateway/types/crosschain-swap.ts (1)
CrossChainOrder(14-28)sdk/src/gateway/utils/layerzero.ts (1)
getCrossChainStatus(4-32)sdk/src/gateway/types/order.ts (1)
GatewayOrder(89-92)
⏰ 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 (3)
sdk/src/gateway/layerzero.ts (3)
662-713: Implementation looks solid.The
getCrossChainSwapOrdersmethod correctly:
- Uses
safeFetchfor robust error handling- Filters out compose messages to isolate cross-chain swaps
- Validates payload format before parsing
- Handles parsing errors gracefully with fallback to 0
- Maps LayerZero message status to
CrossChainOrderStatus
721-731: Correctly combines multiple order types.The method properly:
- Uses
super.getOrdersto avoid infinite recursion (fixed from previous review)- Fetches both order types in parallel with
Promise.all- Maps cross-chain orders to the correct union type
- Returns a combined array of all order types
3-41: Import updates align with refactored architecture.The imports correctly reflect the modular type structure and include all necessary utilities for:
- Address validation and padding (
isAddress,isAddressEqual,padHex)- Storage slot computation (
computeAllowanceSlot,computeBalanceSlot)- Chain configuration and status mapping (
getChainConfig,getCrossChainStatus)- New LayerZero types for cross-chain swaps and quotes
danielsimao
force-pushed
the
feat/l0-l0-swaps
branch
from
827a8b1 to
a7faf69
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
sdk/src/gateway/layerzero.ts (4)
374-374: Major: Zero slippage tolerance is risky.Setting
minAmountLDequal toamountLDmeans zero tolerance for slippage, which can cause transaction failures in volatile conditions. TheGetQuoteParamstype includes an optionalmaxSlippageproperty that should be used here.Consider applying this diff:
- minAmountLD: BigInt(params.amount), + minAmountLD: BigInt(Math.floor(Number(params.amount) * (1 - (params.maxSlippage ?? 0.01)))),Based on past review comments.
608-613: Critical:allowanceSlotspender must be the OFT contract.The spender in the allowance slot should be
wbtcOftAddress(the contract that pulls the underlying tokens), notwbtcMainnetAddress(the token itself).Apply this diff:
const allowanceSlot = computeAllowanceSlot( user as Address, - wbtcMainnetAddress as Address, + wbtcOftAddress as Address, wbtcMainnetSlots.allowanceSlot );Based on past review comments.
522-522: Critical:padHexmust specify{ size: 32 }for bytes32 recipient.Same issue - the
tofield inSendParamrequires a 32-byte value.Apply this diff:
- to: padHex(params.toUserAddress as Hex) as Hex, + to: padHex(params.toUserAddress as Hex, { size: 32 }) as Hex,Based on past review comments.
162-163: Usemainnet.name.toLowerCase()for consistency.The cases for
bobandbobSepoliause.name.toLowerCase(), butmainnetreturns a hardcoded string. For consistency and to match LayerZero's expected metadata key ('ethereum'), use the same pattern:Apply this diff:
case mainnet.id: - return 'mainnet'; + return mainnet.name.toLowerCase();Based on past review comments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
sdk/src/gateway/layerzero.ts(11 hunks)sdk/src/gateway/types/crosschain-swap.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk/src/gateway/types/crosschain-swap.ts
🧰 Additional context used
🧬 Code graph analysis (1)
sdk/src/gateway/layerzero.ts (7)
sdk/src/gateway/types/layerzero.ts (4)
LayerZeroDeploymentsMetadataResponse(78-91)LayerZeroTokenDeploymentsResponse(93-97)LayerZeroSendParam(61-69)LayerZeroQuoteParamsExt(99-106)sdk/src/gateway/utils/common.ts (5)
getChainConfig(192-199)viemClient(81-83)toHexScriptPubKey(52-56)computeAllowanceSlot(202-223)computeBalanceSlot(226-239)sdk/src/gateway/abi.ts (1)
layerZeroOftAbi(164-266)sdk/src/gateway/tokens.ts (2)
getTokenAddress(460-468)getTokenSlots(469-494)sdk/src/gateway/types/crosschain-swap.ts (1)
CrossChainOrder(14-28)sdk/src/gateway/utils/layerzero.ts (1)
getCrossChainStatus(4-32)sdk/src/gateway/types/order.ts (1)
GatewayOrder(89-92)
🔇 Additional comments (6)
sdk/src/gateway/layerzero.ts (6)
666-717: LGTM! Solid implementation of cross-chain order retrieval.The payload parsing includes proper validation (length checks, error handling), and the status mapping uses the
getCrossChainStatusutility correctly.
725-735: LGTM! Infinite recursion issue correctly resolved.Using
super.getOrders()properly delegates to the base implementation, avoiding the stack overflow that would occur withthis.getOrders().
566-586: LGTM! Sensible default address handling for gas estimation.Setting a default address when none is provided (or when it's zero) is appropriate for gas estimation purposes.
420-515: LGTM! Offramp execution logic is well-structured.The compose message encoding, chain validation, and padHex usage (line 466 correctly specifies size) are all properly implemented.
631-631: Major: Add fallback forgetTokenSlotson unsupported chains.
getTokenSlotsonly supports BOB networks (per the implementation in tokens.ts). For other origin chains like Base or Optimism, this will throw and cause gas estimation to fail.Wrap the slot lookup in a try-catch and conditionally apply the state override:
+ let oftBalanceSlot: Hex | null = null; + try { const wbtcOftSlots = getTokenSlots(wbtcOftAddress as Address, fromChain); - const oftBalanceSlot = computeBalanceSlot(user as Address, wbtcOftSlots.balanceSlot); + oftBalanceSlot = computeBalanceSlot(user as Address, wbtcOftSlots.balanceSlot); + } catch { + // Fallback: proceed without OFT balance override for non-BOB chains + oftBalanceSlot = null; + }Then update the state override to conditionally include it:
- } else { + } else if (oftBalanceSlot) { // WBTC OFT - const wbtcOftSlots = getTokenSlots(wbtcOftAddress as Address, fromChain); - - const oftBalanceSlot = computeBalanceSlot(user as Address, wbtcOftSlots.balanceSlot); - overrides.push({ address: wbtcOftAddress as Address, stateDiff: [Based on past review comments.
616-628: Major: State override should target token contract.The state override should target
wbtcMainnetAddress(the token contract), not be part of a conditional that might usezeroAddresselsewhere.Ensure the override targets the correct address:
overrides.push({ - address: wbtcMainnetAddress, + address: wbtcMainnetAddress as Address, stateDiff: [Note: Verify this override is correctly applied and not targeting
zeroAddressanywhere.Based on past review comments.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
sdk/src/gateway/layerzero.ts (5)
374-374: Major: Add slippage tolerance for cross-chain swaps.Setting
minAmountLDequal toamountLDprovides zero slippage tolerance, which can cause transaction failures in volatile market conditions. Consider using the optionalmaxSlippageparameter fromGetQuoteParamsto calculate an acceptable minimum.Apply this diff:
- minAmountLD: BigInt(params.amount), + minAmountLD: BigInt(Math.floor(Number(params.amount) * (1 - (params.maxSlippage ?? 0.01)))), // Default 1% slippageBased on past review comments.
162-163: Critical: Use consistent chain key for mainnet.Based on past review discussion and the pattern used for
bobandbobSepolia, this should usemainnet.name.toLowerCase()to return'ethereum', which matches LayerZero's metadata key. Later code at line 608 uses'ethereum'as the chain key, confirming this is the expected value.Apply this diff for consistency:
case mainnet.id: - return 'mainnet'; + return mainnet.name.toLowerCase(); // returns 'ethereum'Based on past review comments.
609-613: Critical: Use OFT contract as spender in allowance slot.The
computeAllowanceSlotshould usewbtcOftAddressas the spender (the OFT contract that pulls underlying tokens), notwbtcMainnetAddress. The current code computes the allowance for the wrong spender, which will cause the state override to fail.Apply this diff:
const allowanceSlot = computeAllowanceSlot( user as Address, - wbtcMainnetAddress as Address, + wbtcOftAddress as Address, // spender is the OFT contract wbtcMainnetSlots.allowanceSlot );Based on past review comments.
372-372: Critical: Specify{ size: 32 }for bytes32 recipient.The
SendParam.tofield requires a 32-byte value. Without an explicit size,padHexmay not produce the required bytes32, causing transaction failures.Apply this diff:
- to: padHex(params.toUserAddress as Hex), + to: padHex(params.toUserAddress as Hex, { size: 32 }),Based on past review comments.
522-522: Critical: Specify{ size: 32 }for bytes32 recipient.The
tofield inSendParamrequires a 32-byte value. Without the size parameter, this may produce incorrect-length values.Apply this diff:
- to: padHex(params.toUserAddress as Hex) as Hex, + to: padHex(params.toUserAddress as Hex, { size: 32 }) as Hex,Based on past review comments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sdk/src/gateway/layerzero.ts(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
sdk/src/gateway/layerzero.ts (7)
sdk/src/gateway/types/layerzero.ts (5)
LayerZeroDeploymentsMetadataResponse(78-91)LayerZeroTokenDeploymentsResponse(93-97)LayerZeroSendParam(61-69)LayerZeroQuoteParamsExt(99-106)LayerZeroMessagesWalletResponse(57-59)sdk/src/gateway/utils/common.ts (5)
getChainConfig(192-199)viemClient(81-83)toHexScriptPubKey(52-56)computeAllowanceSlot(202-223)computeBalanceSlot(226-239)sdk/src/gateway/abi.ts (1)
layerZeroOftAbi(164-266)sdk/src/gateway/tokens.ts (2)
getTokenAddress(460-468)getTokenSlots(469-494)sdk/src/gateway/types/crosschain-swap.ts (1)
CrossChainOrder(14-28)sdk/src/gateway/utils/layerzero.ts (1)
getCrossChainStatus(4-32)sdk/src/gateway/types/order.ts (1)
GatewayOrder(89-92)
⏰ 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 (3)
sdk/src/gateway/layerzero.ts (3)
3-42: LGTM: Imports expanded appropriately.The expanded imports from viem and local modules support the new LayerZero cross-chain swap functionality. All additions are necessary for state overrides, gas estimation, and order management.
666-717: LGTM: Cross-chain order retrieval implemented well.The
getCrossChainSwapOrdersmethod properly:
- Fetches data from LayerZero API with error handling
- Filters for relevant orders (excluding lzCompose operations)
- Safely parses the amount from the payload with validation
- Maps to the
CrossChainOrdertype with proper status derivation
725-735: LGTM: getOrders correctly calls parent implementation.The method properly uses
super.getOrders(userAddress)to avoid infinite recursion (as flagged in past comments) and correctly combines results from both onramp/offramp orders and cross-chain swap orders into a unified array.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
sdk/src/gateway/layerzero.ts (4)
255-255: Critical:padHexmust specify{ size: 32 }for bytes32 recipient.The
SendParam.tofield expects a bytes32 value. Without an explicit size,padHexmay not produce the required 32-byte value, causing transaction failures.Apply this diff:
- to: padHex(params.toUserAddress as Hex), + to: padHex(params.toUserAddress as Hex, { size: 32 }),Based on past review comments.
372-372: Critical:padHexmust specify{ size: 32 }for bytes32 recipient.Same issue as line 255 - the
tofield requires a 32-byte value.Apply this diff:
- to: padHex(params.toUserAddress as Hex), + to: padHex(params.toUserAddress as Hex, { size: 32 }),Based on past review comments.
631-643: Major: Add fallback forgetTokenSlotson unsupported chains.
getTokenSlotscurrently only supports BOB networks (persdk/src/gateway/tokens.ts). For other origin chains (like Base, Optimism, etc.), this will throw and gas estimation will fail.Add a try-catch wrapper to gracefully handle unsupported chains:
+ let oftBalanceSlot: Hex | null = null; + try { const wbtcOftSlots = getTokenSlots(wbtcOftAddress as Address, fromChain); - - const oftBalanceSlot = computeBalanceSlot(user as Address, wbtcOftSlots.balanceSlot); - - overrides.push({ + oftBalanceSlot = computeBalanceSlot(user as Address, wbtcOftSlots.balanceSlot); + } catch { + // Skip OFT balance override for chains not in TOKENS metadata + oftBalanceSlot = null; + } + + if (oftBalanceSlot !== null) { + overrides.push({ address: wbtcOftAddress as Address, stateDiff: [ { slot: oftBalanceSlot, value: toHex(maxUint256), }, ], }); + }Based on past review comments.
162-163: Fix LayerZero chain key for Ethereum mainnet on line 163.LayerZero uses "ethereum" as the chain key for mainnet, not "mainnet". The current code is inconsistent with the pattern used for
bobandbobSepolia(lines 159, 161) which use.name.toLowerCase(). Update line 163:case mainnet.id: - return 'mainnet'; + return mainnet.name.toLowerCase(); // returns 'ethereum'This aligns with LayerZero's metadata API expectations and maintains pattern consistency.
🧹 Nitpick comments (1)
sdk/src/gateway/layerzero.ts (1)
370-378: Consider adding slippage tolerance for cross-chain swaps.Line 374 sets
minAmountLDequal toamountLD, which means zero tolerance for slippage. While this may be intentional for direct LayerZero transfers, it could cause transaction failures if there's any amount variance during transit.Consider using the
maxSlippageparameter fromGetQuoteParamsto allow some tolerance:const sendParam: LayerZeroSendParam = { dstEid, to: padHex(params.toUserAddress as Hex, { size: 32 }), amountLD: BigInt(params.amount), - minAmountLD: BigInt(params.amount), + minAmountLD: BigInt(Math.floor(Number(params.amount) * (1 - (params.maxSlippage ?? 0.01)))), // Default 1% slippage extraOptions: '0x', composeMsg: '0x', oftCmd: '0x', };Based on past review comments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
sdk/src/gateway/layerzero.ts(11 hunks)sdk/src/gateway/types/crosschain-swap.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
sdk/src/gateway/types/crosschain-swap.ts (1)
sdk/src/gateway/types/quote.ts (1)
BaseExecuteQuoteParams(60-64)
sdk/src/gateway/layerzero.ts (7)
sdk/src/gateway/types/layerzero.ts (5)
LayerZeroDeploymentsMetadataResponse(78-91)LayerZeroTokenDeploymentsResponse(93-97)LayerZeroSendParam(61-69)LayerZeroQuoteParamsExt(99-106)LayerZeroMessagesWalletResponse(57-59)sdk/src/gateway/utils/common.ts (5)
getChainConfig(192-199)viemClient(81-83)toHexScriptPubKey(52-56)computeAllowanceSlot(202-223)computeBalanceSlot(226-239)sdk/src/gateway/abi.ts (1)
layerZeroOftAbi(164-266)sdk/src/gateway/tokens.ts (2)
getTokenAddress(460-468)getTokenSlots(469-494)sdk/src/gateway/types/crosschain-swap.ts (1)
CrossChainOrder(14-28)sdk/src/gateway/utils/layerzero.ts (1)
getCrossChainStatus(4-32)sdk/src/gateway/types/order.ts (1)
GatewayOrder(89-92)
⏰ 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 sendParam: LayerZeroSendParam = { | ||
| dstEid: destinationEid, | ||
| to: padHex(params.toUserAddress as Hex) as Hex, |
There was a problem hiding this comment.
Critical: padHex must specify { size: 32 } for bytes32 recipient.
The to field in SendParam requires a 32-byte value.
Apply this diff:
- to: padHex(params.toUserAddress as Hex) as Hex,
+ to: padHex(params.toUserAddress as Hex, { size: 32 }) as Hex,Based on past review comments.
📝 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.
| to: padHex(params.toUserAddress as Hex) as Hex, | |
| to: padHex(params.toUserAddress as Hex, { size: 32 }) as Hex, |
🤖 Prompt for AI Agents
In sdk/src/gateway/layerzero.ts around line 522, the SendParam `to` field must
be a bytes32 value but currently calls padHex without size, producing incorrect
length; update the call to padHex so it specifies a 32-byte size (e.g.,
padHex(params.toUserAddress as Hex, { size: 32 }) as Hex) to ensure the
recipient is padded to bytes32.
| const allowanceSlot = computeAllowanceSlot( | ||
| user as Address, | ||
| wbtcMainnetAddress as Address, | ||
| wbtcMainnetSlots.allowanceSlot | ||
| ); |
There was a problem hiding this comment.
Major: Allowance slot uses wrong spender address.
The allowance slot should use wbtcOftAddress as the spender (the OFT contract that will pull the underlying token), not wbtcMainnetAddress. The current logic incorrectly computes approval for the token to spend itself.
Apply this diff:
const allowanceSlot = computeAllowanceSlot(
user as Address,
- wbtcMainnetAddress as Address,
+ wbtcOftAddress as Address, // spender is the OFT contract
wbtcMainnetSlots.allowanceSlot
);Based on past review comments.
🤖 Prompt for AI Agents
In sdk/src/gateway/layerzero.ts around lines 609 to 613, the
computeAllowanceSlot call is using wbtcMainnetAddress as the spender which
causes the allowance slot to be computed for the token approving itself; change
the spender argument to wbtcOftAddress so the slot reflects approval for the OFT
contract that will pull the underlying token, i.e. pass wbtcOftAddress (not
wbtcMainnetAddress) into computeAllowanceSlot and keep other parameters the
same.
Summary by CodeRabbit
New Features
Improvements
Tests
Chore