feat: layerzero #719
feat: layerzero #719
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds LayerZero integration: new ABIs, LayerZero client and gateway client with quote/execute flows, tests and a small typing change, and bumps sdk package metadata and devDependencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant LZGC as LayerZeroGatewayClient
participant LZC as LayerZeroClient
participant OFT as LayerZero OFT
participant Quoter as Uniswap V3 QuoterV2
participant GAPI as GatewayApiClient (base)
rect rgba(230,245,255,0.6)
note over User,LZGC: getQuote flow
User->>LZGC: getQuote(params)
LZGC->>LZC: getEidForChain(dst)
LZC-->>LZGC: dstEid / oftAddress
alt simple btc↔bob
LZGC->>GAPI: getQuote(params)
GAPI-->>LZGC: quote
else cross-chain via LayerZero
LZGC->>OFT: quoteSend(sendParam,isNative)
OFT-->>LZGC: MessagingFee
opt Swap required
LZGC->>Quoter: quoteExactOutputSingle(params)
Quoter-->>LZGC: amountIn, gasEstimate
end
LZGC->>GAPI: getQuote(adjusted with LZ fees)
GAPI-->>LZGC: quote
end
LZGC-->>User: quote
end
sequenceDiagram
autonumber
actor User
participant LZGC as LayerZeroGatewayClient
participant Public as EVM PublicClient
participant Wallet as EVM WalletClient
participant OFT as LayerZero OFT
rect rgba(240,255,240,0.6)
note over User,LZGC: executeQuote flow
User->>LZGC: executeQuote(quote, clients, btcSigner)
LZGC->>OFT: quoteSend(sendParam,isNative)
OFT-->>LZGC: MessagingFee
LZGC->>Public: simulate OFT.send(...) / estimate calldata
Public-->>LZGC: calldata / estimate
LZGC->>Wallet: sendTransaction(to=OFT, data, value=fee)
Wallet-->>LZGC: txHash
LZGC-->>User: txHash
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Signed-off-by: Gregory Hill <[email protected]>
Signed-off-by: Gregory Hill <[email protected]>
Signed-off-by: Gregory Hill <[email protected]>
gregdhill
force-pushed
the
feat/layerzero
branch
from
e374be8 to
8ef07fc
Compare
feat: working layer zero client and test
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
sdk/src/gateway/client.ts (3)
244-253: Bug: double-reading response body.You call
response.json()twice; the second read will fail. Use the first parsed JSON for error details and also checkresponse.ok.Apply:
- const jsonResponse = await response.json(); - - if (!jsonResponse.orderDetails) { - const errorData = await response.json().catch(() => null); - const apiMessage = errorData?.message; + const jsonResponse = await response.json(); + if (!response.ok || !jsonResponse.orderDetails) { + const apiMessage = (jsonResponse && jsonResponse.message) as string | undefined; const errorMessage = apiMessage ? `Failed to get onramp quote: ${apiMessage}` : 'Failed to get onramp quote'; throw new Error(errorMessage); }
569-575: Unsafe numeric pow and misleading error; enforce decimals ≥ 8 and use BigInt math.
10 ** (decimals - 8)uses floating math; also the error triggers only whendecimals === undefinedbut message mentions “less than 8”. Fix both.- const decimals = getTokenDecimals(token); - if (decimals === undefined) { - throw new Error('Tokens with less than 8 decimals are not supported'); - } - - const amountInToken = satAmountLocked * BigInt(10 ** (decimals - 8)); + const decimals = getTokenDecimals(token); + if (decimals === undefined || decimals < 8) { + throw new Error('Tokens with less than 8 decimals are not supported'); + } + const scale = 10n ** BigInt(decimals - 8); + const amountInToken = satAmountLocked * scale;
794-796: Same pow issue here; compute required allowance in BigInt.Avoid float math and handle tokens with decimals < 8.
- const multiplier = 10 ** (decimals - 8); - if (BigInt(quote.amountLockInSat * multiplier) > allowance) { + const decimalsNum = Number(decimals); + if (decimalsNum < 8) { + throw new Error('Tokens with less than 8 decimals are not supported'); + } + const required = BigInt(quote.amountLockInSat) * (10n ** BigInt(decimalsNum - 8)); + if (required > allowance) {
🧹 Nitpick comments (11)
sdk/src/gateway/utils.ts (1)
86-86: Return Hex from toHexScriptPubKey to remove downstream casts.Since client.ts now expects Hex, expose that here to avoid
as Hexlater.You can change the signature (outside this hunk):
// sdk/src/gateway/utils.ts import type { Hex } from 'viem'; export function toHexScriptPubKey(userAddress: string, network: bitcoin.Network): Hex { const address = bitcoin.address.toOutputScript(userAddress, network); const buffer = Buffer.concat([Buffer.from([address.length]), address]); return ('0x' + buffer.toString('hex')) as Hex; }sdk/src/gateway/client.ts (3)
15-16: Hex import is fine; pair it with a Hex-returning helper to avoid casts.Use a Hex-typed
toHexScriptPubKey(utils.ts) so you don’t needas Hexlater.
415-416: Drop the cast by returning Hex from the helper.Once
toHexScriptPubKeyreturnsHex, you can avoidas Hex.- outputScript: receiverAddress as Hex, + outputScript: receiverAddress,
901-904: Minor: stray parenthesis in error message.Tidy the fallback error string.
- const errorMessage = apiMessage ? `Failed to update order: ${apiMessage}` : `Failed to update order)`; + const errorMessage = apiMessage ? `Failed to update order: ${apiMessage}` : 'Failed to update order';sdk/src/gateway/abis/OFT.abi.ts (1)
3-721: ABI looks comprehensive.No issues spotted in structure. Consider reusing this single source for OFT-related fragments elsewhere to avoid duplication.
sdk/test/layerzero.test.ts (1)
11-46: Test hardcodes chain EIDs that may become outdatedThe test asserts specific EID values for each chain (e.g., '30101' for ethereum), which will fail if LayerZero updates these values. Consider either fetching expected values from a configuration source or making the test more resilient to EID changes.
sdk/src/gateway/layerzero.ts (5)
88-89: Missing BOB Sepolia support as noted in TODOThe TODO comment indicates that BOB Sepolia support is missing, which limits testnet functionality.
Would you like me to help implement BOB Sepolia support? This would involve updating the chain resolution logic and ensuring proper testnet addresses are used throughout the LayerZero integration.
129-129: Fix typo in comment- minAmountLD: BigInt(0), // will be added inside the strategyzz + minAmountLD: BigInt(0), // will be added inside the strategy
146-149: Buffer calculation uses incorrect basis pointsThe comment says "5% buffer" but the calculation uses 500 basis points which is actually 5% (500/10000 = 0.05). However, using a raw BigInt(500) without clear documentation could be confusing.
Define the buffer as a constant with clear documentation:
+ const BUFFER_BASIS_POINTS = 500n; // 5% buffer (500 / 10000) - const buffer = BigInt(500); // 5% buffer + const buffer = BUFFER_BASIS_POINTS; // Add buffer to the layer zero fee to account for changes from now until the order is executed const layerZeroFeeWithBuffer = (layerZeroFee.nativeFee * (10000n + buffer)) / 10000n; // 5% buffer
241-241: Missing gas estimation for extraOptionsThe TODO comment indicates that gas estimation is not implemented for the extraOptions field, which could lead to failed transactions due to insufficient gas.
Would you like me to help implement the gas estimation logic for the extraOptions field? This would involve calculating appropriate gas limits based on the destination chain and transaction complexity.
55-61: Generic error handling loses response detailsThe error handling only uses
response.statusTextwhich may not provide sufficient debugging information.Improve error handling to include more details:
private async getJson<T>(url: string): Promise<T> { const response = await fetch(url); if (!response.ok) { - throw new Error(response.statusText); + const errorBody = await response.text().catch(() => ''); + throw new Error(`LayerZero API error: ${response.status} ${response.statusText}${errorBody ? ` - ${errorBody}` : ''}`); } return (await response.json()) as Promise<T>; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
sdk/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
sdk/package.json(1 hunks)sdk/src/gateway/abi.ts(1 hunks)sdk/src/gateway/abis/OFT.abi.ts(1 hunks)sdk/src/gateway/client.ts(3 hunks)sdk/src/gateway/layerzero.ts(1 hunks)sdk/src/gateway/utils.ts(1 hunks)sdk/test/layerzero.test.ts(1 hunks)
🧰 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/abi.ts
🧬 Code graph analysis (2)
sdk/test/layerzero.test.ts (3)
crates/utils/src/bitcoin_core.rs (1)
client(46-51)sdk/src/gateway/layerzero.ts (2)
LayerZeroClient(21-62)LayerZeroGatewayClient(89-282)sdk/src/gateway/types.ts (1)
BitcoinSigner(551-560)
sdk/src/gateway/layerzero.ts (4)
sdk/src/gateway/client.ts (2)
GatewayApiClient(121-1159)AllWalletClientParams(91-96)sdk/src/gateway/types.ts (5)
GetQuoteParams(537-537)GatewayTokensInfo(362-367)OfframpQuote(399-410)ExecuteQuoteParams(549-549)OfframpExecuteQuoteParams(544-547)sdk/src/gateway/utils.ts (2)
viemClient(62-64)toHexScriptPubKey(33-37)sdk/src/gateway/abi.ts (3)
layerZeroOftAbi(164-202)quoterV2Abi(204-230)offrampCaller(41-153)
🪛 Biome (2.1.2)
sdk/test/layerzero.test.ts
[error] 89-147: Do not export from a test file.
(lint/suspicious/noExportsInTest)
⏰ 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/utils.ts (1)
86-86: No-op formatting change.No functional impact. OK.
sdk/src/gateway/abi.ts (1)
204-230: Confirmed stateMutability of quoteExactOutputSingle is nonpayable
Matches the official QuoterV2 ABI; no changes needed.sdk/src/gateway/layerzero.ts (1)
29-38: LGTM! Clean API client implementationThe method properly fetches supported chains from the LayerZero API with appropriate error handling.
Signed-off-by: Gregory Hill <[email protected]>
Signed-off-by: Gregory Hill <[email protected]>
Signed-off-by: Gregory Hill <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (6)
sdk/src/gateway/layerzero.ts (6)
198-205: Avoid mutating params.fromChain; copy insteadThis mutation can surprise callers if they reuse the object.
Apply:
- params.fromChain = bob.id; - // Handle l0 -> bitcoin: estimate bob -> bitcoin - const response = await super.getQuote(params); + const modifiedParams = { ...params, fromChain: bob.id }; + const response = await super.getQuote(modifiedParams); // revert fromChain for handling in executeQuote response.params.fromChain = fromChain; return response;
145-146: Public client is hardcoded to BOB; derive from this.chainThis breaks on BOB Sepolia or other chains. Use the configured chain.
Apply:
- const publicClient = viemClient(bob); + const publicClient = viemClient(this.chain);
161-175: Quoter and WETH addresses should be chain-configuredHardcoded Uniswap quoter and WETH addresses will fail on other chains and upgrades.
Example:
+ const QUOTER_BY_CHAIN: Record<number, Address> = { + [bob.id]: '0x6Aa54a43d7eEF5b239a18eed3Af4877f46522BCA', + [bobSepolia.id]: '0x...', // TODO + }; + const WETH_BY_CHAIN: Record<number, Address> = { + [bob.id]: '0x4200000000000000000000000000000000000006', + [bobSepolia.id]: '0x4200000000000000000000000000000000000006', // verify + }; const quote = await publicClient.readContract({ - address: '0x6Aa54a43d7eEF5b239a18eed3Af4877f46522BCA', + address: QUOTER_BY_CHAIN[this.chain.id], abi: quoterV2Abi, functionName: 'quoteExactOutputSingle', args: [ { tokenIn: params.toToken as Hex, - tokenOut: '0x4200000000000000000000000000000000000006' as Hex, + tokenOut: WETH_BY_CHAIN[this.chain.id] as Hex,
191-197: Don’t mutate params; avoid hardcoded strategy addressMutating params leaks side effects to callers; hardcoded strategyAddress blocks multi-env support.
Apply:
- params.strategyAddress = '0x5Fd9B934c219663C7f4f432f39682be2dC42eDC7'; - params.message = encodedParameters; - // change to BOB chain for bridging - params.toChain = bob.id; - - // Handle bitcoin -> l0 chain: need to add calldata - return super.getQuote(params); + const STRATEGY_BY_CHAIN: Record<number, Address> = { + [bob.id]: '0x5Fd9B934c219663C7f4f432f39682be2dC42eDC7', + // [bobSepolia.id]: '0x...', // TODO + }; + const modifiedParams = { + ...params, + strategyAddress: STRATEGY_BY_CHAIN[this.chain.id], + message: encodedParameters, + toChain: bob.id, + }; + return super.getQuote(modifiedParams);
241-241: Select the correct Bitcoin network (mainnet/testnet)The scriptPubKey conversion always uses mainnet. Pick network from this.chain.
Apply:
- const receiverAddress = toHexScriptPubKey(params.toUserAddress, bitcoin.networks.bitcoin); + const bitcoinNetwork = this.chain.id === bob.id ? bitcoin.networks.bitcoin : bitcoin.networks.testnet; + const receiverAddress = toHexScriptPubKey(params.toUserAddress, bitcoinNetwork);
266-277: OFT address must be configurable per token/chainWBTC_OFT_ADDRESS is hardcoded. This will break across chains/tokens.
Apply:
- const sendFees = await publicClient.readContract({ - abi: oftAbi, - address: WBTC_OFT_ADDRESS, // TODO: may be different for other chains + const oftAddress = params.toToken as Address; // or resolve via a per-chain map + const sendFees = await publicClient.readContract({ + abi: oftAbi, + address: oftAddress, functionName: 'quoteSend', args: [sendParam, false], }); @@ - const { request } = await publicClient.simulateContract({ + const { request } = await publicClient.simulateContract({ account: walletClient.account, abi: oftAbi, - address: WBTC_OFT_ADDRESS, + address: oftAddress, functionName: 'send', args: [sendParam, sendFees, recipient], value: sendFees.nativeFee, });If multiple tokens are supported, introduce a resolver: getOftAddress(token, chainId).
🧹 Nitpick comments (6)
sdk/src/gateway/layerzero.ts (6)
38-48: Harden LayerZero metadata parsing and errorsgetSupportedChains indexes data.WBTC[0] without checks. A schema change or empty results will throw.
Apply:
- return Object.keys(data.WBTC[0].deployments); + const deployments = data?.WBTC?.[0]?.deployments; + if (!deployments) throw new Error('LayerZero metadata did not return WBTC deployments'); + return Object.keys(deployments);Also consider normalizing/validating chain keys (casing, spaces) returned by the API.
65-71: Add timeout and better error context to fetchNetwork calls should be bounded and errors actionable.
Apply:
- private async getJson<T>(url: string): Promise<T> { - const response = await fetch(url); + private async getJson<T>(url: string): Promise<T> { + const controller = new AbortController(); + const t = setTimeout(() => controller.abort(), 10_000); + const response = await fetch(url, { signal: controller.signal }); + clearTimeout(t); if (!response.ok) { - throw new Error(response.statusText); + const body = await response.text().catch(() => ''); + throw new Error(`LayerZero metadata request failed: ${response.status} ${response.statusText} ${body}`); } return (await response.json()) as Promise<T>; }
156-160: Nit: avoid redundant BigInt castbuffer is already a bigint; no need to BigInt() it again in encodeAbiParameters.
Apply:
- [sendParam, BigInt(buffer), BigInt(tokensToSwapForLayerZeroFeesWithBuffer)] + [sendParam, buffer, BigInt(tokensToSwapForLayerZeroFeesWithBuffer)]
210-219: Require publicClient/walletClient for EVM-side L0 callsexecuteQuote uses publicClient and walletClient in the L0 branch; add a guard for clearer errors.
Apply:
async executeQuote({ quote: executeQuoteParams, walletClient, publicClient, btcSigner, }: { quote: ExecuteQuoteParams } & AllWalletClientParams): Promise<string> { + if (!publicClient || !walletClient) { + throw new Error('publicClient and walletClient are required to execute LayerZero sends'); + }
121-133: Confirm OptionsBuilder/ExecutorOptions encoding stays in syncManual encodePacked with magic numbers (TYPE_3, WORKER_ID, OPTION_TYPE_LZRECEIVE) is brittle.
- Extract these constants into named variables with references.
- Add a unit test asserting the encoded bytes for given inputs.
Would you like me to draft a test vector?
139-139: Typo in comment“strategyzz” should be “strategy”.
Apply:
- minAmountLD: BigInt(0), // will be added inside the strategyzz + minAmountLD: BigInt(0), // will be added inside the strategy
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
sdk/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
sdk/package.json(2 hunks)sdk/src/gateway/index.ts(1 hunks)sdk/src/gateway/layerzero.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk/package.json
🧰 Additional context used
🧬 Code graph analysis (1)
sdk/src/gateway/layerzero.ts (5)
sdk/src/gateway/index.ts (3)
LayerZeroGatewayClient(6-6)GatewayApiClient(1-1)GatewayQuote(2-2)sdk/src/gateway/client.ts (2)
GatewayApiClient(121-1159)AllWalletClientParams(91-96)sdk/src/gateway/types.ts (5)
GetQuoteParams(537-537)GatewayTokensInfo(362-367)OfframpQuote(399-410)ExecuteQuoteParams(549-549)OfframpExecuteQuoteParams(544-547)sdk/src/gateway/utils.ts (2)
viemClient(62-64)toHexScriptPubKey(33-37)sdk/src/gateway/abi.ts (3)
layerZeroOftAbi(164-202)quoterV2Abi(204-230)offrampCaller(41-153)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (1)
sdk/src/gateway/index.ts (1)
6-6: Public export looks goodAdding LayerZeroGatewayClient to the public surface is consistent with the new client in this PR.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
sdk/test/layerzero.test.ts (1)
48-84: Incomplete execute path test — skip or implement with mocks.Body has no assertions and key paths are commented out. Mark as skipped until proper mocking is added.
Apply this diff:
- it('should get a quote and execute it', async () => { + it.skip('should get a quote and execute it — TODO: implement with proper mocking', async () => {Suggested TODO: mock LayerZeroGatewayClient, viem Public/Wallet clients, and a BitcoinSigner; assert executeQuote calls/writeContract flow and returned txHash.
🧹 Nitpick comments (5)
sdk/test/layerzero.test.ts (5)
12-46: Make the “should get chains” test deterministic (mock or gate as E2E).This hits live LayerZero endpoints and will be flaky/offline in CI. Either mock fetch or gate behind an env flag.
Apply this diff to gate it as an integration test:
-it('should get chains', async () => { +it.runIf(process.env.L0_E2E === 'true')('should get chains (integration)', async () => {
32-46: Avoid hard-coding EIDs (they can change) — assert presence/shape instead.Replace literal EID equality checks with a stable loop validating non-null numeric strings.
Apply this diff:
- assert.equal(await client.getEidForChain('ethereum'), '30101'); - assert.equal(await client.getEidForChain('bob'), '30279'); - assert.equal(await client.getEidForChain('base'), '30184'); - assert.equal(await client.getEidForChain('bera'), '30362'); - assert.equal(await client.getEidForChain('unichain'), '30320'); - assert.equal(await client.getEidForChain('avalanche'), '30106'); - assert.equal(await client.getEidForChain('sonic'), '30332'); - assert.equal(await client.getEidForChain('aptos'), '30108'); - assert.equal(await client.getEidForChain('bsc'), '30102'); - assert.equal(await client.getEidForChain('soneium'), '30340'); - assert.equal(await client.getEidForChain('telos'), '30199'); - assert.equal(await client.getEidForChain('swell'), '30335'); - assert.equal(await client.getEidForChain('optimism'), '30111'); - assert.equal(await client.getEidForChain('sei'), '30280'); + const expected = [ + 'ethereum','bob','base','bera','unichain','avalanche','sonic','aptos','bsc','soneium','telos','swell','optimism','sei', + ]; + for (const chain of expected) { + const eid = await client.getEidForChain(chain); + assert.ok(eid, `Missing EID for ${chain}`); + assert.ok(/^\d+$/.test(eid!), `EID for ${chain} must be numeric`); + }
91-97: Prefer hex.decode over Buffer; add key zeroization.Avoid Node Buffer in tests/libs and add a disposer to clear key material.
Apply this diff (imports + constructor + disposer):
- import { base64 } from '@scure/base'; + import { base64, hex } from '@scure/base'; @@ constructor(privateKeyHex: string) { const cleanPrivateKey = privateKeyHex.startsWith('0x') ? privateKeyHex.slice(2) : privateKeyHex; - this.privateKey = new Uint8Array(Buffer.from(cleanPrivateKey, 'hex')); + this.privateKey = hex.decode(cleanPrivateKey); + if (this.privateKey.length !== 32) throw new Error('Invalid private key length'); } + + /** + * Best-effort zeroization of private key bytes. + */ + dispose(): void { + this.privateKey.fill(0); + this.privateKey = new Uint8Array(0); + }
124-139: Consider validating all inputs are signed before finalize.Optionally verify signatures per input (or rely on a library verify utility) and throw if any input remains unsigned to avoid finalizing/broadcasting incomplete TXs.
87-90: Keeping ScureBitcoinSigner local to the test is fine.No exports from the test file anymore — resolves the earlier lint concern. If you intend to reuse it, consider moving it under sdk/src/gateway/signers/.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
sdk/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
sdk/package.json(2 hunks)sdk/src/gateway/layerzero.ts(1 hunks)sdk/test/layerzero.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- sdk/src/gateway/layerzero.ts
- sdk/package.json
🧰 Additional context used
🧬 Code graph analysis (1)
sdk/test/layerzero.test.ts (2)
sdk/src/gateway/layerzero.ts (2)
LayerZeroClient(30-71)LayerZeroGatewayClient(88-292)sdk/src/gateway/types.ts (1)
BitcoinSigner(551-560)
🪛 Gitleaks (8.27.2)
sdk/test/layerzero.test.ts
55-55: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (1)
sdk/test/layerzero.test.ts (1)
61-71: Wallet client isn’t a signer — zeroAddress won’t be able to write.If you unskip the test,
walletClient.writeContractwill fail without a signing account. Use a private key or an injected account for E2E only.Example:
import { privateKeyToAccount } from 'viem/accounts'; // DO NOT commit real keys. Use CI secrets for E2E. const account = privateKeyToAccount(process.env.PRIVATE_KEY as `0x${string}`); const walletClient = createWalletClient({ chain: bob, transport: http(), account });
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
sdk/src/gateway/abi.ts (1)
164-266: Avoid duplicating OFT ABI; import the single source of truth.This redefines IOFT locally. Reuse the canonical ABI to prevent drift and ease upgrades.
Apply this diff to replace the local const with a re-export:
- export const layerZeroOftAbi = [ - /* … IOFT ABI literal … */ - ] as const; +// Re-export the canonical OFT ABI (adjust path if needed) +export { oftAbi as layerZeroOftAbi } from '../abis/OFT.abi';sdk/test/layerzero.test.ts (3)
155-162: Key material persists in memory; add a cleanup method.Even in tests, zeroizing the private key is a good habit.
class ScureBitcoinSigner implements BitcoinSigner { private privateKey: Uint8Array; @@ constructor(privateKeyHex: string) { const cleanPrivateKey = privateKeyHex.startsWith('0x') ? privateKeyHex.slice(2) : privateKeyHex; this.privateKey = new Uint8Array(Buffer.from(cleanPrivateKey, 'hex')); } + + dispose(): void { + if (this.privateKey) this.privateKey.fill(0); + }
189-204: Validate all inputs are signed before finalizing the PSBT.Current code may finalize with unsigned inputs.
async signAllInputs(psbtBase64: string): Promise<string> { try { const tx = btc.Transaction.fromPSBT(base64.decode(psbtBase64)); @@ for (let i = 0; i < tx.inputsLength; i++) { tx.signIdx(this.privateKey, i); } - - tx.finalize(); + // Ensure all inputs are signed before finalizing + for (let i = 0; i < tx.inputsLength; i++) { + const input = tx.getInput(i); + const hasWitness = !!input.finalScriptWitness && input.finalScriptWitness.length > 0; + const hasScriptSig = !!input.finalScriptSig && input.finalScriptSig.length > 0; + if (!hasWitness && !hasScriptSig) { + throw new Error(`Failed to sign input ${i}`); + } + } + tx.finalize();
68-106: Integration test without assertions; uses a zero-address wallet.
- No assertions => no coverage.
account: zeroAddresswill fail if any on-chain call is attempted.- Hits live networks, making CI flaky.
Either fully mock and assert, or skip with a TODO:
- it('should get an onramp quote and execute it', async () => { + it.skip('should get an onramp quote and execute it - TODO: mock clients and assert behavior', async () => {If you want to run it, require env keys and use a real account:
- const walletClient = createWalletClient({ + const walletClient = createWalletClient({ chain: bob, transport: http(), - account: zeroAddress, + account: privateKeyToAccount(process.env.PRIVATE_KEY as Hex), });
🧹 Nitpick comments (4)
sdk/src/gateway/abi.ts (1)
268-294: Tighten ABI typing for safer contract calls.Use Viem’s Abi type-check to catch shape mistakes at compile time. Optional but recommended.
Apply:
-export const quoterV2Abi = [ +export const quoterV2Abi = [ { /* … */ } -] as const; +] as const satisfies import('viem').Abi;Alternatively,
parseAbiwith human-readable fragments also works for consistency with nearby ABIs.sdk/test/layerzero.test.ts (3)
34-66: Brittle assertions against external metadata.Hard-coding EIDs/OFT addresses makes tests flaky when LayerZero metadata changes. Prefer mocking
LayerZeroClientor asserting structural properties (non-null, hex format) instead of exact values.If you keep these as integration checks, guard them behind an env flag and reduce the set to a few critical chains.
138-139: Unprotected secret usage in tests.
process.env.MNEMONICis required but not validated. Add a guard or skip the test when missing.- const btcSignerFromSeed = await ScureBitcoinSigner.fromSeedPhrase(process.env.MNEMONIC!, "m/84'/0'/0'/0/0"); + if (!process.env.MNEMONIC) throw new Error('MNEMONIC not set'); + const btcSignerFromSeed = await ScureBitcoinSigner.fromSeedPhrase(process.env.MNEMONIC, "m/84'/0'/0'/0/0");
71-82: Remove console logs from tests.They add noise to CI output; prefer assertions. If needed, guard behind
DEBUG.Also applies to: 121-122, 104-105, 148-149
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
sdk/package.json(2 hunks)sdk/src/gateway/abi.ts(1 hunks)sdk/src/gateway/layerzero.ts(1 hunks)sdk/test/layerzero.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- sdk/package.json
- sdk/src/gateway/layerzero.ts
🧰 Additional context used
🧬 Code graph analysis (1)
sdk/test/layerzero.test.ts (2)
sdk/src/gateway/layerzero.ts (2)
LayerZeroClient(28-81)LayerZeroGatewayClient(98-327)sdk/src/gateway/types.ts (1)
BitcoinSigner(551-560)
🪛 Gitleaks (8.27.2)
sdk/test/layerzero.test.ts
75-75: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (1)
sdk/test/layerzero.test.ts (1)
1-12: Sanity-check: potential false-positive from Gitleaks.A “Generic API Key” alert was flagged; looks like regular addresses/constants. Verify no secrets are committed.
| assert.containSubset(chains, [ | ||
| 'ethereum', | ||
| 'bob', | ||
| 'base', | ||
| 'bera', | ||
| 'unichain', | ||
| 'avalanche', | ||
| 'sonic', | ||
| 'aptos', | ||
| 'bsc', | ||
| 'soneium', | ||
| 'telos', | ||
| 'swell', | ||
| 'optimism', | ||
| 'sei', | ||
| ]); | ||
|
|
There was a problem hiding this comment.
assert.containSubset isn’t part of core Chai/Vitest; test will fail.
Use assert.includeMembers (Chai) or expect(...).toEqual(expect.arrayContaining(...)).
- assert.containSubset(chains, [
+ assert.includeMembers(chains, [
'ethereum',
'bob',
'base',
'bera',
'unichain',
'avalanche',
'sonic',
'aptos',
'bsc',
'soneium',
'telos',
'swell',
'optimism',
'sei',
]);📝 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.
| assert.containSubset(chains, [ | |
| 'ethereum', | |
| 'bob', | |
| 'base', | |
| 'bera', | |
| 'unichain', | |
| 'avalanche', | |
| 'sonic', | |
| 'aptos', | |
| 'bsc', | |
| 'soneium', | |
| 'telos', | |
| 'swell', | |
| 'optimism', | |
| 'sei', | |
| ]); | |
| assert.includeMembers(chains, [ | |
| 'ethereum', | |
| 'bob', | |
| 'base', | |
| 'bera', | |
| 'unichain', | |
| 'avalanche', | |
| 'sonic', | |
| 'aptos', | |
| 'bsc', | |
| 'soneium', | |
| 'telos', | |
| 'swell', | |
| 'optimism', | |
| 'sei', | |
| ]); |
🤖 Prompt for AI Agents
In sdk/test/layerzero.test.ts around lines 17 to 33, the test uses
assert.containSubset which is not part of core Chai/Vitest and will fail;
replace that call with a supported assertion such as
assert.includeMembers(chains, [...]) if using Chai, or use Vitest/Jest style
expect(chains).toEqual(expect.arrayContaining([...])) — update the assertion
line accordingly and ensure the test imports/uses the matching assertion
library.
| it('should get an offramp quote and execute it', async () => { | ||
| const client = new LayerZeroGatewayClient(bob.id); | ||
| const layerZeroClient = new LayerZeroClient(); | ||
|
|
||
| const quote = await client.getQuote({ | ||
| fromChain: 'base', | ||
| fromToken: (await layerZeroClient.getOftAddressForChain('base')) as string, // WBTC on base | ||
| toChain: 'bitcoin', | ||
| toToken: 'bitcoin', | ||
| fromUserAddress: '0x2A7f5295ac6e24b6D2ca78d82E3cbf01dDA52745', // EVM address (sender) | ||
| toUserAddress: 'bc1q6tgkjx4pgc5qda52fsgeuvjrhml5nuawwplejq', // Bitcoin address (recipient) | ||
| amount: 17000, | ||
| }); | ||
|
|
||
| console.log('quote', quote); | ||
|
|
||
| // Verify we got an offramp quote | ||
| assert.ok(quote.offrampQuote, 'Should have offramp quote'); | ||
| assert.ok(quote.params, 'Should have quote params'); | ||
|
|
||
| const publicClient = createPublicClient({ | ||
| chain: base, | ||
| transport: http(), | ||
| }); | ||
|
|
||
| const walletClient = createWalletClient({ | ||
| chain: base, | ||
| transport: http(), | ||
| account: privateKeyToAccount(process.env.PRIVATE_KEY as Hex), | ||
| }); | ||
|
|
||
| const btcSignerFromSeed = await ScureBitcoinSigner.fromSeedPhrase(process.env.MNEMONIC!, "m/84'/0'/0'/0/0"); | ||
| console.log('P2WPKH Address: ', await btcSignerFromSeed.getP2WPKHAddress()); | ||
|
|
||
| const txHash = await client.executeQuote({ | ||
| quote, | ||
| walletClient, | ||
| publicClient: publicClient as PublicClient<Transport>, | ||
| btcSigner: btcSignerFromSeed, | ||
| }); | ||
|
|
||
| console.log(txHash); | ||
| }, 120000); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Gate the offramp E2E test and remove console noise.
This sends real transactions. Guard with an env flag and avoid console logs in CI.
- it('should get an offramp quote and execute it', async () => {
+ it.runIf(process.env.RUN_E2E === '1')('should get an offramp quote and execute it', async () => {
@@
- console.log('quote', quote);
+ // E2E: requires funded accounts and RPC endpoints
@@
- console.log('P2WPKH Address: ', await btcSignerFromSeed.getP2WPKHAddress());
+ // Optionally log only when DEBUG is set
+ if (process.env.DEBUG) console.log('P2WPKH Address:', await btcSignerFromSeed.getP2WPKHAddress());
@@
- console.log(txHash);
+ if (process.env.DEBUG) console.log(txHash);Also, assert something about txHash (shape/non-empty) after execution.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In sdk/test/layerzero.test.ts around lines 107 to 149, gate the real-offramp E2E
test behind an env flag (e.g. RUN_OFFRAMP_E2E) so it only runs when explicitly
enabled, remove the console.log calls (quote, P2WPKH address, txHash) to
eliminate CI noise, and add an assertion after executeQuote to validate txHash
is non-empty and matches expected shape (e.g. string length/pattern or at least
truthy). Implement the guard at the start of the test to skip/return immediately
if the env flag is not set, delete the console logs, and insert a concise
assertion like assert.ok(typeof txHash === 'string' && txHash.length > 0) after
execution.
| async getP2WPKHAddress(): Promise<string> { | ||
| return btc.getAddress('wpkh', this.privateKey) as string; | ||
| } |
There was a problem hiding this comment.
Bug: deriving P2WPKH address from a private key (should use public key).
This returns an invalid address. Derive the compressed pubkey first.
- async getP2WPKHAddress(): Promise<string> {
- return btc.getAddress('wpkh', this.privateKey) as string;
- }
+ async getP2WPKHAddress(): Promise<string> {
+ const pub = (await import('@noble/curves/secp256k1')).secp256k1.getPublicKey(this.privateKey, true);
+ return btc.getAddress('wpkh', pub) as string;
+ }Add the import once at the top if you prefer static import:
import { secp256k1 } from '@noble/curves/secp256k1';🤖 Prompt for AI Agents
In sdk/test/layerzero.test.ts around lines 209 to 211, the code derives a P2WPKH
address directly from the private key which produces an invalid address; instead
derive the compressed public key from the private key (using secp256k1 to
compute the public key and compress it) and pass that public key into
btc.getAddress('wpkh', <compressedPubKey>) to generate the correct address; also
add an import for secp256k1 from '@noble/curves/secp256k1' at the top if you
prefer a static import.
Adds L0 methods to get supported chains and lookup EIDs for send txs.
Once the new L0 strategy has been developed we can include an extension to the client to automatically route Gateway calls through L0.
Summary by CodeRabbit
New Features
Public API
Tests
Chores