LF-15494 - Relay added support for SUI [RelayFacet v1.0.2] #1378
Conversation
WalkthroughReplaced literal non‑EVM chain ID checks in RelayFacet with named constants, added a SUI mapping returning 103665049, updated Relay API doc URL, bumped facet version to 1.0.2, extended tests to cover SUI mapping, added deployment and diamond staging entries for the new facet address, and refocused demoRelay to a SUI USDC relay flow. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (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 |
Test Coverage ReportLine Coverage: 85.84% (2547 / 2967 lines) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Facets/RelayFacet.sol (1)
18-18: Fix CI: bump custom:version to unblock pipeline.Update the NatSpec version tag to reflect this change set.
Apply:
-/// @custom:version 1.0.1 +/// @custom:version 1.0.2test/solidity/Facets/RelayFacet.t.sol (1)
774-786: Keep test signing helper in sync: include Sui mapping to avoid future signature mismatches.signData uses this helper; without Sui here, any future Sui bridging tests will produce invalid signatures.
function _getMappedChainId( uint256 chainId ) internal pure returns (uint256) { if (chainId == 20000000000001) { return 8253038; } if (chainId == 1151111081099710) { return 792703809; } + // Sui + if (chainId == 9270000000000000) { + return 103665049; + } + return chainId; }
🧹 Nitpick comments (5)
src/Facets/RelayFacet.sol (4)
222-222: Nit: tidy the doc comment and make the link reference explicit.- /// IDs found here https://api.relay.link/chains + /// Non-EVM chain IDs reference: https://api.relay.link/chains
228-235: Replace magic numbers on the right-hand side with named constants for readability.Left-hand side already uses LIFI constants; consider adding Relay-side constants to avoid magic numbers (8253038, 792703809).
+ uint256 internal constant RELAY_CHAIN_ID_BTC = 8_253_038; + uint256 internal constant RELAY_CHAIN_ID_SOLANA = 792_703_809; @@ - if (chainId == LIFI_CHAIN_ID_BTC) { - return 8253038; + if (chainId == LIFI_CHAIN_ID_BTC) { + return RELAY_CHAIN_ID_BTC; } @@ - if (chainId == LIFI_CHAIN_ID_SOLANA) { - return 792703809; + if (chainId == LIFI_CHAIN_ID_SOLANA) { + return RELAY_CHAIN_ID_SOLANA; }
237-241: Sui mapping addition looks good; align tests/helpers and consider constantizing the RHS.
- Please mirror this Sui mapping in the test helper used by signData to prevent future mismatches when adding Sui flow tests.
- Optional: introduce RELAY_CHAIN_ID_SUI for consistency.
+ uint256 internal constant RELAY_CHAIN_ID_SUI = 103_665_049; @@ - if (chainId == LIFI_CHAIN_ID_SUI) { - return 103665049; + if (chainId == LIFI_CHAIN_ID_SUI) { + return RELAY_CHAIN_ID_SUI; }
218-219: Event ordering: emit LiFiTransferStarted before external calls.Repo conventions require emitting at the start of the transaction, before external calls. Consider adjusting in a follow-up PR to avoid behavioral churn here.
test/solidity/Facets/RelayFacet.t.sol (1)
1-1: License header deviates from repo standard.Tests are first-party; please standardize SPDX to LGPL-3.0-only to match guidelines.
-// SPDX-License-Identifier: Unlicense +// SPDX-License-Identifier: LGPL-3.0-only
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/Facets/RelayFacet.sol(1 hunks)test/solidity/Facets/RelayFacet.t.sol(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
{src,test/solidity,script}/**/*.sol
📄 CodeRabbit inference engine (conventions.md)
{src,test/solidity,script}/**/*.sol: All Solidity files must start with a license identifier, followed by a blank line, then the pragma statement
Use SPDX license identifier // SPDX-License-Identifier: LGPL-3.0-only for all first-party Solidity files
Error names must be descriptive PascalCase, use custom errors instead of revert() strings, and omit error messages for gas optimization
Solidity variable/function naming: state variables camelCase; function params camelCase with leading underscore; constants and immutables in CONSTANT_CASE; private/internal variables camelCase; function names camelCase
Files:
src/Facets/RelayFacet.soltest/solidity/Facets/RelayFacet.t.sol
src/**/*.sol
📄 CodeRabbit inference engine (conventions.md)
src/**/*.sol: Every contract must include NatSpec header in this order: @title, @author LI.FI (https://li.fi), @notice, @Custom:version X.Y.Z
All public/external functions must have NatSpec comments documenting purpose, params, and returns
Add inline comments for complex logic, optimizations, gas-saving techniques, and math steps
Emit GenericSwapCompleted after successful same-chain swap completion
Files:
src/Facets/RelayFacet.sol
src/Facets/**/*Facet.sol
📄 CodeRabbit inference engine (conventions.md)
src/Facets/**/*Facet.sol: Facet contracts must be located in src/Facets and include "Facet" in the contract name
Facet contracts must implement: internal _startBridge, swapAndStartBridgeTokensVia{FacetName}, and startBridgeTokensVia{FacetName}
Facet contracts must import/use: ILiFi, LibAsset, LibSwap, LibAllowList; and use security mixins ReentrancyGuard, SwapperV2, Validatable; ECDSA optional
Do not use msg.sender as refund address; require an explicit parameter for refund address
In {facetName}Data, receiverAddress must be the first field and be validated against bridgeData.receiver; verify targetChainId equals bridgeData.destinationChain for EVM-to-EVM
Apply required modifiers where appropriate: nonReentrant, refundExcessNative, validateBridgeData, doesNotContainSourceSwaps/doesContainSourceSwaps, doesNotContainDestinationCalls/doesContainDestinationCalls
Emit LiFiTransferStarted at the start of a transaction before external calls; do not emit LiFiTransferCompleted or LiFiTransferRecovered in facets
For native fees in facets, use the _depositAndSwap variant and reserve required fees before execution
For non-EVM chains: receiverAddress must be bytes (not bytes32), must not be zero, and bridgeData.receiver must equal NON_EVM_ADDRESS
Files:
src/Facets/RelayFacet.sol
**/*.sol
📄 CodeRabbit inference engine (conventions.md)
Follow solhint rules as configured in .solhint.json
Files:
src/Facets/RelayFacet.soltest/solidity/Facets/RelayFacet.t.sol
test/solidity/**/*.t.sol
📄 CodeRabbit inference engine (conventions.md)
test/solidity/**/*.t.sol: Solidity test files must use .t.sol extension; organize imports with system libraries first, then project files
Test naming: successful tests prefixed test_, failure tests prefixed testRevert_, base/inherited tests prefixed testBase_; negative tests must check specific revert reasons
Each test contract must implement setUp() that configures block numbers, initializes base contracts, sets up facets, and uses vm.label; contracts inheriting TestBase.sol must call initTestBase() in setUp()
Use vm.startPrank()/vm.stopPrank() for user simulation; maintain blank-line formatting rules for tests (expectRevert separation, assertion grouping, spacing)
Use assertEq for equality, custom assertion modifiers for balance changes, vm.expectRevert with precise reasons, and vm.expectEmit(true,true,true,true,) to verify events and parameters
Files:
test/solidity/Facets/RelayFacet.t.sol
🧠 Learnings (12)
📓 Common learnings
Learnt from: mirooon
PR: lifinance/contracts#975
File: test/solidity/Facets/DeBridgeDlnFacet.t.sol:0-0
Timestamp: 2025-02-17T07:59:54.979Z
Learning: In DeBridgeDlnFacet, chain ID mappings are hardcoded and not designed to be dynamically updated, so tests for mapping updates are not needed.
📚 Learning: 2024-11-22T08:49:35.205Z
Learnt from: ezynda3
PR: lifinance/contracts#827
File: src/Facets/DeBridgeDlnFacet.sol:0-0
Timestamp: 2024-11-22T08:49:35.205Z
Learning: In `DeBridgeDlnFacet.sol`, direct access to the `deBridgeChainId` mapping is acceptable, and using the `getDeBridgeChainId` function is not necessary in these instances.
Applied to files:
src/Facets/RelayFacet.soltest/solidity/Facets/RelayFacet.t.sol
📚 Learning: 2025-02-17T07:59:54.979Z
Learnt from: mirooon
PR: lifinance/contracts#975
File: test/solidity/Facets/DeBridgeDlnFacet.t.sol:0-0
Timestamp: 2025-02-17T07:59:54.979Z
Learning: In DeBridgeDlnFacet, chain ID mappings are hardcoded and not designed to be dynamically updated, so tests for mapping updates are not needed.
Applied to files:
src/Facets/RelayFacet.soltest/solidity/Facets/RelayFacet.t.sol
📚 Learning: 2025-09-12T10:17:51.646Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.646Z
Learning: Applies to src/Facets/**/*Facet.sol : Emit LiFiTransferStarted at the start of a transaction before external calls; do not emit LiFiTransferCompleted or LiFiTransferRecovered in facets
Applied to files:
src/Facets/RelayFacet.sol
📚 Learning: 2025-02-13T08:54:06.846Z
Learnt from: ezynda3
PR: lifinance/contracts#984
File: src/Facets/ChainflipFacet.sol:119-131
Timestamp: 2025-02-13T08:54:06.846Z
Learning: In Solidity, when mapping between fixed values known at compile time (like chain IDs), using if-else statements with constants is preferred over mappings since mappings cannot be declared as constant or immutable.
Applied to files:
src/Facets/RelayFacet.sol
📚 Learning: 2025-09-12T10:17:51.646Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.646Z
Learning: Applies to src/Facets/**/*Facet.sol : In {facetName}Data, receiverAddress must be the first field and be validated against bridgeData.receiver; verify targetChainId equals bridgeData.destinationChain for EVM-to-EVM
Applied to files:
src/Facets/RelayFacet.soltest/solidity/Facets/RelayFacet.t.sol
📚 Learning: 2025-04-21T03:17:53.443Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs involving address updates like the RelayFacet to Worldchain, verify the actual presence of entries in files before reporting issues. The RelayFacet exists in the diamond log file and the PR diff already contains the necessary address change.
Applied to files:
src/Facets/RelayFacet.soltest/solidity/Facets/RelayFacet.t.sol
📚 Learning: 2025-05-14T16:28:30.376Z
Learnt from: mirooon
PR: lifinance/contracts#1137
File: src/Periphery/GasZipPeriphery.sol:21-23
Timestamp: 2025-05-14T16:28:30.376Z
Learning: When shifting by 16 bits per chain ID in the GasZip contracts, the MAX_CHAINID_LENGTH_ALLOWED constant should be set to 16 (since a uint256 can hold at most 16 chain IDs when using 16 bits per chain). This constant should be consistent between related contracts like GasZipPeriphery and GasZipFacet.
Applied to files:
src/Facets/RelayFacet.sol
📚 Learning: 2025-08-28T02:41:07.505Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1321
File: docs/RelayDepositoryFacet.md:9-10
Timestamp: 2025-08-28T02:41:07.505Z
Learning: Unit tests for RelayDepositoryFacet cannot verify fund forwarding behavior after deposits because the facet delegates to external IRelayDepository contracts. The forwarding logic is implemented in the Relay Protocol V2 Depository contracts, not in the facet itself.
Applied to files:
test/solidity/Facets/RelayFacet.t.sol
📚 Learning: 2024-10-31T09:10:16.115Z
Learnt from: ezynda3
PR: lifinance/contracts#843
File: test/solidity/Facets/RelayFacet.t.sol:0-0
Timestamp: 2024-10-31T09:10:16.115Z
Learning: In the `signData` function within `RelayFacetTest` in `test/solidity/Facets/RelayFacet.t.sol`, the use of the EIP-712 standard for hashing and signing is not required.
Applied to files:
test/solidity/Facets/RelayFacet.t.sol
📚 Learning: 2025-01-22T12:36:12.699Z
Learnt from: mirooon
PR: lifinance/contracts#945
File: test/solidity/Facets/GlacisFacet.t.sol:214-262
Timestamp: 2025-01-22T12:36:12.699Z
Learning: The GlacisFacet test suite inherits from TestBaseFacet which already covers various failure scenarios including invalid receiver address, invalid amounts, same chain bridging, and insufficient funds, making additional failure scenario tests redundant.
Applied to files:
test/solidity/Facets/RelayFacet.t.sol
📚 Learning: 2025-04-21T03:17:53.443Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs updating contract addresses (like RelayFacet on Worldchain), verify the presence of entries in all relevant files (worldchain.json, worldchain.diamond.json, _deployments_log_file.json) before reporting inconsistencies. The RelayFacet entry exists in all required deployment files with the correct address.
Applied to files:
test/solidity/Facets/RelayFacet.t.sol
🪛 GitHub Actions: VersionControlAndAuditVerification
src/Facets/RelayFacet.sol
[error] 1-1: Version audit failed: the file was modified but the custom:version tag was not updated (current version v1.0.1). This CI step cannot complete until the version tag is updated.
⏰ 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: enforce-min-test-coverage
- GitHub Check: run-unit-tests
🔇 Additional comments (1)
test/solidity/Facets/RelayFacet.t.sol (1)
737-741: LGTM: Sui mapping covered in fuzz helper.
🤖 GitHub Action: Security Alerts Review 🔍🟢 Dismissed Security Alerts with Comments 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: ✅ No unresolved security alerts! 🎉 |
lifi-action-bot
changed the title
| if (chainId == LIFI_CHAIN_ID_SUI) { | ||
| return 103665049; | ||
| } | ||
|
|
There was a problem hiding this comment.
checked also other non-EVM chains for Relay and looking at list of currently existing from LiFiData we can only add SUI at this point
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
script/demoScripts/demoRelay.ts (1)
23-31: Use repo helpers and validate envs (citty + consola + getEnvVar/getProvider/getWalletFromPrivateKeyInDotEnv)Replace the direct process.env usage and manual ethers provider/wallet in script/demoScripts/demoRelay.ts (lines 23–31) with the existing helpers in script/demoScripts/utils/demoScriptHelpers.ts — use getEnvVar('ETH_NODE_URI_ARBITRUM') and the provider/wallet helpers (getProvider / getWalletFromPrivateKeyInDotEnv), parse CLI args with citty, and log/validate with consola so ETH RPC and PRIVATE_KEY are checked at startup.
🧹 Nitpick comments (6)
script/demoScripts/demoRelay.ts (6)
334-341: Handle non‑2xx HTTP responses from Relay API.Currently assumes success; add resp.ok checks to fail fast with diagnostics.
- const resp = await fetch('https://api.relay.link/quote', { + const resp = await fetch('https://api.relay.link/quote', { method: 'POST', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify(params), }) - const quote = await resp.json() + if (!resp.ok) { + throw new Error(`Relay quote failed: ${resp.status} ${await resp.text()}`) + } + const quote = await resp.json()
343-351: Defensive guards for quote shape and signature fetch.Avoid runtime TypeErrors if steps is empty or request fails.
- const requestId = quote.steps[0].requestId + const step0 = quote?.steps?.[0] + if (!step0?.requestId) throw new Error('Missing requestId in quote.steps[0]') + const requestId = step0.requestId ... - const sigResp = await fetch( + const sigResp = await fetch( `https://api.relay.link/requests/${requestId}/signature/v2`, { headers: { 'Content-Type': 'application/json' } } ) - const sigData = await sigResp.json() + if (!sigResp.ok) { + throw new Error(`Signature fetch failed: ${sigResp.status} ${await sigResp.text()}`) + } + const sigData = await sigResp.json()
342-346: Avoid logging sensitive payloads (quotes/signatures) by default.These may contain request identifiers and signatures. Gate behind DEBUG.
- console.log(quote) + if (process.env.DEBUG?.toLowerCase() === 'true') console.log(quote) ... - console.log(sigData) + if (process.env.DEBUG?.toLowerCase() === 'true') console.log(sigData)Also applies to: 351-351
1-2: Prune unused imports.PublicKey and Uniswap helpers/constants are unused in the active path; remove to reduce bundle and lint noise.
-import { PublicKey } from '@solana/web3.js' -import { _100 } from '@uniswap/sdk/dist/constants' ... -import { - ADDRESS_UNISWAP_ARB, - ADDRESS_USDC_ARB, - ADDRESS_WETH_ARB, - getUniswapSwapDataERC20ToETH, -} from './utils/demoScriptHelpers'Also applies to: 14-19
353-364: Strongly type structs with TypeChain to catch field/type mistakes at compile time.You’ve dropped ILiFi.BridgeDataStruct/RelayDataStruct; re‑introduce them for safety.
- const bridgeData = { + const bridgeData: ILiFi.BridgeDataStruct = { transactionId: utils.randomBytes(32), bridge: 'Relay', integrator: 'ACME Devs', referrer: '0x0000000000000000000000000000000000000000', sendingAssetId: '0xaf88d065e77c8cC2239327C5EDb3A432268e5831', - receiver: suiRandomReceiver, + receiver: address, minAmount: '500000', - destinationChainId: 9270000000000000, // SUI + destinationChainId: '9270000000000000', // SUI hasSourceSwaps: false, hasDestinationCall: false, } - const relayData = { + const relayData: RelayFacet.RelayDataStruct = { requestId, - nonEVMReceiver: `0x${new PublicKey(suiRandomReceiver) - .toBuffer() - .toString('hex')}`, - receivingAssetId: `0x${new PublicKey(suiUSDC) - .toBuffer() - .toString('hex')}`, + nonEVMReceiver: utils.hexZeroPad(suiRandomReceiver, 32), + receivingAssetId: utils.hexZeroPad(suiUSDC.split('::')[0], 32), signature: sigData.signature, }Also applies to: 366-375
318-332: Parameterize SUI receiver/asset via CLI flags or env.Hard‑coding a “random” recipient is risky in a demo that can execute real transfers.
- Expose --suiReceiver and --suiUSDC via citty; default to env vars.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
script/demoScripts/demoRelay.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
script/**/*.ts
📄 CodeRabbit inference engine (conventions.md)
script/**/*.ts: All TypeScript scripts must follow .eslintrc.cjs, use async/await, try/catch error handling, proper logging, environment variables, correct typings, use citty for CLI parsing, consola for logging, validate env vars via getEnvVar(), and exit with appropriate codes
Prefer existing helper functions over reimplementation (e.g., getDeployments, getProvider, getWalletFromPrivateKeyInDotEnv, sendTransaction, ensureBalanceAndAllowanceToDiamond, getUniswapData*)
Always use proper TypeChain types (e.g., ILiFi.BridgeDataStruct) and never use any for bridge data or contract structures
Before/after changes: verify imports and types exist, ensure typechain is generated, keep changes comprehensive and consistent, run TS compilation, remove unused imports, and ensure function signatures match usage
Scripts must execute with bunx tsx
Files:
script/demoScripts/demoRelay.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: 0xDEnYO
PR: lifinance/contracts#782
File: script/deploy/_targetState.json:49-49
Timestamp: 2024-11-25T09:05:03.917Z
Learning: The `RelayFacet` contract, when missing from the source code but referenced in deployment configurations, should be treated the same way as `OpBNBBridgeFacet` and can be ignored in code reviews.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs involving address updates like the RelayFacet to Worldchain, verify the actual presence of entries in files before reporting issues. The RelayFacet exists in the diamond log file and the PR diff already contains the necessary address change.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs updating contract addresses (like RelayFacet on Worldchain), verify the presence of entries in all relevant files (worldchain.json, worldchain.diamond.json, _deployments_log_file.json) before reporting inconsistencies. The RelayFacet entry exists in all required deployment files with the correct address.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1381
File: deployments/arbitrum.json:65-69
Timestamp: 2025-09-16T01:39:54.086Z
Learning: When verifying facet deployments across .json and .diamond.json files, search by facet name rather than trying to cross-reference addresses between the files, as the same contract can have different addresses in different deployment files.
Learnt from: mirooon
PR: lifinance/contracts#975
File: test/solidity/Facets/DeBridgeDlnFacet.t.sol:0-0
Timestamp: 2025-02-17T07:59:54.979Z
Learning: In DeBridgeDlnFacet, chain ID mappings are hardcoded and not designed to be dynamically updated, so tests for mapping updates are not needed.
Learnt from: ezynda3
PR: lifinance/contracts#843
File: script/deploy/facets/DeployRelayFacet.s.sol:0-0
Timestamp: 2024-10-31T09:07:36.301Z
Learning: In `script/deploy/facets/DeployRelayFacet.s.sol`, additional validation for the configuration file existence and address validity is unnecessary because the code will already fail appropriately if the configuration file is missing or the addresses are invalid.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1321
File: docs/RelayDepositoryFacet.md:9-10
Timestamp: 2025-08-28T02:41:07.505Z
Learning: Unit tests for RelayDepositoryFacet cannot verify fund forwarding behavior after deposits because the facet delegates to external IRelayDepository contracts. The forwarding logic is implemented in the Relay Protocol V2 Depository contracts, not in the facet itself.
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.646Z
Learning: Applies to src/Facets/**/*Facet.sol : In {facetName}Data, receiverAddress must be the first field and be validated against bridgeData.receiver; verify targetChainId equals bridgeData.destinationChain for EVM-to-EVM
📚 Learning: 2025-08-27T23:36:40.773Z
Learnt from: mirooon
PR: lifinance/contracts#1328
File: test/solidity/Periphery/GasZipPeriphery.t.sol:223-233
Timestamp: 2025-08-27T23:36:40.773Z
Learning: In bridge facet swap flows with requiresDeposit=false, tokens are already held by the diamond contract from previous swaps. When subsequent contracts like GasZipPeriphery call LibAsset.depositAsset, they pull tokens from msg.sender (the diamond) to themselves via transferFrom, requiring no additional approvals since the diamond already owns the tokens.
Applied to files:
script/demoScripts/demoRelay.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: run-unit-tests
- GitHub Check: enforce-min-test-coverage
🔇 Additional comments (2)
script/demoScripts/demoRelay.ts (2)
379-379: Approve the exact amount you intend to spend.Stay consistent with params.amount to prevent mismatches.
[ suggest_nitpick_refactor ]- tx = await token.connect(signer).approve(LIFI_ADDRESS, '500000') + tx = await token.connect(signer).approve(LIFI_ADDRESS, params.amount)
368-374: Replace Solana PublicKey() conversions with 32‑byte hex for SUI (nonEVMReceiver & receivingAssetId)Solana PublicKey is not valid for SUI. Relay v2 expects 32‑byte hex values: nonEVMReceiver = hex zero‑padded 0x SUI address (32 bytes); receivingAssetId = the 32‑byte SUI object id (substring before '::') zero‑padded to 32 bytes.
- nonEVMReceiver: `0x${new PublicKey(suiRandomReceiver) - .toBuffer() - .toString('hex')}`, - receivingAssetId: `0x${new PublicKey(suiUSDC) - .toBuffer() - .toString('hex')}`, + nonEVMReceiver: utils.hexZeroPad(suiRandomReceiver, 32), + receivingAssetId: utils.hexZeroPad(suiUSDC.split('::')[0], 32),
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
script/demoScripts/demoRelay.ts (1)
366-377: SUI on‑chain structs: fix receiver, chainId, and non‑EVM fields before enabling.
- BridgeData.receiver must be an EVM address; using a SUI address will revert ABI encoding.
- Use the SUI chain id 103665049 (number is JS‑safe). Do not use 9270000000000000 (exceeds MAX_SAFE_INTEGER).
- Do not use Solana PublicKey for SUI values. For relayData, prefer API‑provided fields over local transforms.
Apply this diff:
- // const bridgeData = { - // transactionId: utils.randomBytes(32), - // bridge: 'Relay', - // integrator: 'ACME Devs', - // referrer: '0x0000000000000000000000000000000000000000', - // sendingAssetId: '0xaf88d065e77c8cC2239327C5EDb3A432268e5831', - // receiver: suiRandomReceiver, - // minAmount: '500000', - // destinationChainId: 9270000000000000, // SUI - // hasSourceSwaps: false, - // hasDestinationCall: false, - // } + // const bridgeData: ILiFi.BridgeDataStruct = { + // transactionId: utils.randomBytes(32), + // bridge: 'Relay', + // integrator: 'ACME Devs', + // referrer: ethers.constants.AddressZero, + // sendingAssetId: ADDRESS_USDC_ARB, + // receiver: address, // EVM address only + // minAmount: '500000', + // destinationChainId: 103665049, // SUI + // hasSourceSwaps: false, + // hasDestinationCall: false, + // } @@ - // const relayData = { - // requestId, - // nonEVMReceiver: `0x${new PublicKey(suiRandomReceiver) - // .toBuffer() - // .toString('hex')}`, - // receivingAssetId: `0x${new PublicKey(suiUSDC) - // .toBuffer() - // .toString('hex')}`, - // signature: sigData.signature, - // } + // const relayData: RelayFacet.RelayDataStruct = { + // requestId, + // // Prefer using values returned by the Relay signature endpoint: + // nonEVMReceiver: sigData.nonEVMReceiver ?? utils.hexZeroPad(suiRandomReceiver, 32), + // receivingAssetId: sigData.receivingAssetId, + // signature: sigData.signature, + // }Additionally, don’t use Solana’s
PublicKeyfor SUI. If you must compute bytes locally, zero‑pad the 0x‑hex SUI address to 32 bytes withutils.hexZeroPad.#!/bin/bash # Find unsafe SUI chainId literals and non‑EVM receivers in BridgeData across the repo rg -nP -C2 '(destinationChainId:\s*9270000000000000)|(receiver:\s*suiRandomReceiver)'Also applies to: 379-389
🧹 Nitpick comments (7)
script/demoScripts/demoRelay.ts (7)
38-96: Reduce large commented demo blocks to avoid drift/noise.These ETH/USDC/swap sections are all commented out and make the script harder to read. Consider deleting or moving them to dedicated example files, or guard them behind a flag (e.g., RUN_EXAMPLE=eth/usdc/swap) to keep the main SUI flow focused.
Also applies to: 97-126, 133-167, 168-244
245-317: Solana example has inconsistent/mistyped params; fix before re‑enabling.
- Mixed destinationChainId values: earlier uses 792703809 (correct), later uses 1151111081099710 (typo).
- BridgeData.receiver must be an EVM address (not Solana), and non‑EVM receiver goes into relayData.nonEVMReceiver.
- Prefer using receivingAssetId from the API response instead of recomputing.
If you plan to re‑enable, align these with the SUI fixes suggested below.
336-344: Guard external call: add timeout and handle non‑2xx responses.Fetch without a timeout can hang; also check resp.ok before parsing.
Apply this diff:
- const resp = await fetch('https://api.relay.link/quote', { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - }, - body: JSON.stringify(params), - }) - const quote = await resp.json() + const ac = new AbortController() + const timer = setTimeout(() => ac.abort(), 20_000) + const resp = await fetch('https://api.relay.link/quote', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(params), + signal: ac.signal, + }) + clearTimeout(timer) + if (!resp.ok) { + throw new Error(`Relay quote failed: ${resp.status} ${await resp.text()}`) + } + const quote = await resp.json()
345-356: Deduplicate noisy logging.Collapse repeated logs into one concise line.
Apply this diff:
- console.log("requestId") - console.log("requestId") - console.log("requestId") - console.log("requestId") - console.log("requestId") - console.log(requestId) - console.log(requestId) - console.log(requestId) - console.log(requestId) - console.log(requestId) + console.log('requestId:', requestId)
357-365: Handle signature API failures and validate payload.Add timeout, resp.ok check, and presence of signature fields.
Apply this diff:
- const sigResp = await fetch( - `https://api.relay.link/requests/${requestId}/signature/v2`, - { headers: { 'Content-Type': 'application/json' } } - ) - console.log("sigResp") - console.log(sigResp) - const sigData = await sigResp.json() + const ac2 = new AbortController() + const timer2 = setTimeout(() => ac2.abort(), 20_000) + const sigResp = await fetch( + `https://api.relay.link/requests/${requestId}/signature/v2`, + { headers: { 'Content-Type': 'application/json' }, signal: ac2.signal } + ) + clearTimeout(timer2) + if (!sigResp.ok) { + throw new Error(`Relay signature failed: ${sigResp.status} ${await sigResp.text()}`) + } + const sigData = await sigResp.json() + if (!sigData?.signature) { + throw new Error('Relay signature response missing `signature`') + } console.log(sigData)
321-333: Script hygiene: env validation and logging.
- Validate ETH_NODE_URI_ARBITRUM and PRIVATE_KEY via getEnvVar().
- Prefer consola over raw console for structured logs; keep consistent with repo scripts.
- Consider citty for simple CLI flags (e.g., amount, dest coin).
I can patch this script to use getEnvVar/citty/consola if you want.
318-320: Remove or gate unused imports and helpers.With non‑SUI flows commented out, imports like PublicKey and _100 are unused. Drop them or guard the example blocks behind a flag to avoid churn when linting/building.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
script/demoScripts/demoRelay.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
script/**/*.ts
📄 CodeRabbit inference engine (conventions.md)
script/**/*.ts: All TypeScript scripts must follow .eslintrc.cjs, use async/await, try/catch error handling, proper logging, environment variables, correct typings, use citty for CLI parsing, consola for logging, validate env vars via getEnvVar(), and exit with appropriate codes
Prefer existing helper functions over reimplementation (e.g., getDeployments, getProvider, getWalletFromPrivateKeyInDotEnv, sendTransaction, ensureBalanceAndAllowanceToDiamond, getUniswapData*)
Always use proper TypeChain types (e.g., ILiFi.BridgeDataStruct) and never use any for bridge data or contract structures
Before/after changes: verify imports and types exist, ensure typechain is generated, keep changes comprehensive and consistent, run TS compilation, remove unused imports, and ensure function signatures match usage
Scripts must execute with bunx tsx
Files:
script/demoScripts/demoRelay.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: 0xDEnYO
PR: lifinance/contracts#782
File: script/deploy/_targetState.json:49-49
Timestamp: 2024-11-25T09:05:03.917Z
Learning: The `RelayFacet` contract, when missing from the source code but referenced in deployment configurations, should be treated the same way as `OpBNBBridgeFacet` and can be ignored in code reviews.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs involving address updates like the RelayFacet to Worldchain, verify the actual presence of entries in files before reporting issues. The RelayFacet exists in the diamond log file and the PR diff already contains the necessary address change.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs updating contract addresses (like RelayFacet on Worldchain), verify the presence of entries in all relevant files (worldchain.json, worldchain.diamond.json, _deployments_log_file.json) before reporting inconsistencies. The RelayFacet entry exists in all required deployment files with the correct address.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1381
File: deployments/arbitrum.json:65-69
Timestamp: 2025-09-16T01:39:54.086Z
Learning: When verifying facet deployments across .json and .diamond.json files, search by facet name rather than trying to cross-reference addresses between the files, as the same contract can have different addresses in different deployment files.
Learnt from: mirooon
PR: lifinance/contracts#975
File: test/solidity/Facets/DeBridgeDlnFacet.t.sol:0-0
Timestamp: 2025-02-17T07:59:54.979Z
Learning: In DeBridgeDlnFacet, chain ID mappings are hardcoded and not designed to be dynamically updated, so tests for mapping updates are not needed.
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.646Z
Learning: Applies to src/Facets/**/*Facet.sol : In {facetName}Data, receiverAddress must be the first field and be validated against bridgeData.receiver; verify targetChainId equals bridgeData.destinationChain for EVM-to-EVM
Learnt from: ezynda3
PR: lifinance/contracts#843
File: script/deploy/facets/DeployRelayFacet.s.sol:0-0
Timestamp: 2024-10-31T09:07:36.301Z
Learning: In `script/deploy/facets/DeployRelayFacet.s.sol`, additional validation for the configuration file existence and address validity is unnecessary because the code will already fail appropriately if the configuration file is missing or the addresses are invalid.
Learnt from: ezynda3
PR: lifinance/contracts#843
File: test/solidity/Facets/RelayFacet.t.sol:0-0
Timestamp: 2024-10-31T09:10:16.115Z
Learning: In the `signData` function within `RelayFacetTest` in `test/solidity/Facets/RelayFacet.t.sol`, the use of the EIP-712 standard for hashing and signing is not required.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1321
File: docs/RelayDepositoryFacet.md:9-10
Timestamp: 2025-08-28T02:41:07.505Z
Learning: Unit tests for RelayDepositoryFacet cannot verify fund forwarding behavior after deposits because the facet delegates to external IRelayDepository contracts. The forwarding logic is implemented in the Relay Protocol V2 Depository contracts, not in the facet itself.
📚 Learning: 2025-05-14T16:28:30.376Z
Learnt from: mirooon
PR: lifinance/contracts#1137
File: src/Periphery/GasZipPeriphery.sol:21-23
Timestamp: 2025-05-14T16:28:30.376Z
Learning: When shifting by 16 bits per chain ID in the GasZip contracts, the MAX_CHAINID_LENGTH_ALLOWED constant should be set to 16 (since a uint256 can hold at most 16 chain IDs when using 16 bits per chain). This constant should be consistent between related contracts like GasZipPeriphery and GasZipFacet.
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2025-09-12T10:17:51.646Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.646Z
Learning: Applies to src/Facets/**/*Facet.sol : For non-EVM chains: receiverAddress must be bytes (not bytes32), must not be zero, and bridgeData.receiver must equal NON_EVM_ADDRESS
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2025-09-12T10:17:51.646Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.646Z
Learning: Applies to src/Facets/**/*Facet.sol : In {facetName}Data, receiverAddress must be the first field and be validated against bridgeData.receiver; verify targetChainId equals bridgeData.destinationChain for EVM-to-EVM
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2025-02-21T09:00:28.226Z
Learnt from: ezynda3
PR: lifinance/contracts#984
File: src/Facets/ChainflipFacet.sol:127-146
Timestamp: 2025-02-21T09:00:28.226Z
Learning: The `validateBridgeData` modifier in `Validatable.sol` already validates that the receiver address is not zero, making additional zero-address checks redundant in functions using this modifier.
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2025-02-21T09:00:28.226Z
Learnt from: ezynda3
PR: lifinance/contracts#984
File: src/Facets/ChainflipFacet.sol:127-146
Timestamp: 2025-02-21T09:00:28.226Z
Learning: The `validateBridgeData` modifier in `Validatable.sol` validates that the receiver address is not zero using `LibUtil.isZeroAddress`, making additional zero-address checks redundant in functions using this modifier.
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2025-07-16T01:03:08.106Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1275
File: src/Facets/AllBridgeFacet.sol:164-164
Timestamp: 2025-07-16T01:03:08.106Z
Learning: In src/Facets/AllBridgeFacet.sol, the team has decided that explicit validation for address downcasting from `_bridgeData.sendingAssetId` to `bytes32(uint256(uint160(_bridgeData.sendingAssetId)))` is not required, accepting the potential risk of silent overflow from unsafe downcasting.
Applied to files:
script/demoScripts/demoRelay.ts
🧬 Code graph analysis (1)
script/demoScripts/demoRelay.ts (1)
script/demoScripts/utils/demoScriptHelpers.ts (1)
ADDRESS_USDC_ARB(63-63)
⏰ 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: run-unit-tests
- GitHub Check: enforce-min-test-coverage
🔇 Additional comments (1)
script/demoScripts/demoRelay.ts (1)
318-333: SUI params look good; keep non‑EVM receiver confined to the API layer.
- destinationChainId: 103665049 (SUI) is fine here.
- Using the SUI address as params.recipient is OK for the Relay API. Don’t propagate it into BridgeData.receiver when you enable the on‑chain call.
Optional: lift 103665049 into a CHAIN_ID_SUI constant to avoid “magic numbers.”
Can you confirm this matches the RelayFacet mapping added in this PR’s tests?
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
script/demoScripts/demoRelay.ts (4)
336-346: Harden network calls: check HTTP status and shape.Add
resp.ok/sigResp.okchecks and validatequote.steps[0]?.requestIdto avoid undefined access.Apply this diff:
- const resp = await fetch('https://api.relay.link/quote', { + const resp = await fetch('https://api.relay.link/quote', { method: 'POST', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify(params), }) - const quote = await resp.json() + if (!resp.ok) { + throw new Error(`quote failed: ${resp.status} ${resp.statusText}`) + } + const quote = await resp.json() + if (!quote?.steps?.[0]?.requestId) { + throw new Error('quote missing steps[0].requestId') + } console.log(quote) const requestId = quote.steps[0].requestId @@ - const sigResp = await fetch( + const sigResp = await fetch( `https://api.relay.link/requests/${requestId}/signature/v2`, { headers: { 'Content-Type': 'application/json' } } ) - console.log("sigResp") - console.log(sigResp) - const sigData = await sigResp.json() + if (!sigResp.ok) { + throw new Error(`signature fetch failed: ${sigResp.status} ${sigResp.statusText}`) + } + const sigData = await sigResp.json()Also applies to: 357-366
347-356: Remove duplicated requestId logs.These six identical lines look like leftover debugging.
Apply this diff:
- console.log("requestId") - console.log("requestId") - console.log("requestId") - console.log("requestId") - console.log("requestId") - console.log(requestId) console.log(requestId) - console.log(requestId) - console.log(requestId) - console.log(requestId) + // requestId logged once above
361-366: Prefer actionable logging and avoid dumping Response objects.Log
status/urland the parsed body as needed. Raw Response logs are noisy.Apply this diff:
- console.log("sigResp") - console.log(sigResp) + console.log(`sigResp: ${sigResp.status} ${sigResp.url}`)
24-31: Use shared helpers for env/provider/wallet and structured logging.Follow repo script guidelines: validate env with
getEnvVar(), usegetProvider/getWalletFromPrivateKeyInDotEnv, andconsolainstead ofconsole.*. Also considercittyif you plan to pass runtime args (amount, token).I can refactor this script to the standard helpers and consola in a follow-up commit if you want.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
script/demoScripts/demoRelay.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
script/**/*.ts
📄 CodeRabbit inference engine (conventions.md)
script/**/*.ts: All TypeScript scripts must follow .eslintrc.cjs, use async/await, try/catch error handling, proper logging, environment variables, correct typings, use citty for CLI parsing, consola for logging, validate env vars via getEnvVar(), and exit with appropriate codes
Prefer existing helper functions over reimplementation (e.g., getDeployments, getProvider, getWalletFromPrivateKeyInDotEnv, sendTransaction, ensureBalanceAndAllowanceToDiamond, getUniswapData*)
Always use proper TypeChain types (e.g., ILiFi.BridgeDataStruct) and never use any for bridge data or contract structures
Before/after changes: verify imports and types exist, ensure typechain is generated, keep changes comprehensive and consistent, run TS compilation, remove unused imports, and ensure function signatures match usage
Scripts must execute with bunx tsx
Files:
script/demoScripts/demoRelay.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: 0xDEnYO
PR: lifinance/contracts#782
File: script/deploy/_targetState.json:49-49
Timestamp: 2024-11-25T09:05:03.917Z
Learning: The `RelayFacet` contract, when missing from the source code but referenced in deployment configurations, should be treated the same way as `OpBNBBridgeFacet` and can be ignored in code reviews.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs involving address updates like the RelayFacet to Worldchain, verify the actual presence of entries in files before reporting issues. The RelayFacet exists in the diamond log file and the PR diff already contains the necessary address change.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs updating contract addresses (like RelayFacet on Worldchain), verify the presence of entries in all relevant files (worldchain.json, worldchain.diamond.json, _deployments_log_file.json) before reporting inconsistencies. The RelayFacet entry exists in all required deployment files with the correct address.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1381
File: deployments/arbitrum.json:65-69
Timestamp: 2025-09-16T01:39:54.099Z
Learning: When verifying facet deployments across .json and .diamond.json files, search by facet name rather than trying to cross-reference addresses between the files, as the same contract can have different addresses in different deployment files.
Learnt from: mirooon
PR: lifinance/contracts#975
File: test/solidity/Facets/DeBridgeDlnFacet.t.sol:0-0
Timestamp: 2025-02-17T07:59:54.979Z
Learning: In DeBridgeDlnFacet, chain ID mappings are hardcoded and not designed to be dynamically updated, so tests for mapping updates are not needed.
Learnt from: ezynda3
PR: lifinance/contracts#843
File: script/deploy/facets/DeployRelayFacet.s.sol:0-0
Timestamp: 2024-10-31T09:07:36.301Z
Learning: In `script/deploy/facets/DeployRelayFacet.s.sol`, additional validation for the configuration file existence and address validity is unnecessary because the code will already fail appropriately if the configuration file is missing or the addresses are invalid.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1321
File: docs/RelayDepositoryFacet.md:9-10
Timestamp: 2025-08-28T02:41:07.505Z
Learning: Unit tests for RelayDepositoryFacet cannot verify fund forwarding behavior after deposits because the facet delegates to external IRelayDepository contracts. The forwarding logic is implemented in the Relay Protocol V2 Depository contracts, not in the facet itself.
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.686Z
Learning: Applies to src/Facets/**/*Facet.sol : In {facetName}Data, receiverAddress must be the first field and be validated against bridgeData.receiver; verify targetChainId equals bridgeData.destinationChain for EVM-to-EVM
Learnt from: mirooon
PR: lifinance/contracts#985
File: script/deploy/_targetState.json:0-0
Timestamp: 2025-02-12T09:44:12.961Z
Learning: The bsca network intentionally maintains different facet versions between staging and production environments, specifically:
1. CalldataVerificationFacet: v1.1.1 in staging vs v1.1.2 in production
2. EmergencyPauseFacet: present only in production
3. Permit2Proxy: present only in production
Learnt from: ezynda3
PR: lifinance/contracts#843
File: test/solidity/Facets/RelayFacet.t.sol:0-0
Timestamp: 2024-10-31T09:10:16.115Z
Learning: In the `signData` function within `RelayFacetTest` in `test/solidity/Facets/RelayFacet.t.sol`, the use of the EIP-712 standard for hashing and signing is not required.
📚 Learning: 2025-05-14T16:28:30.376Z
Learnt from: mirooon
PR: lifinance/contracts#1137
File: src/Periphery/GasZipPeriphery.sol:21-23
Timestamp: 2025-05-14T16:28:30.376Z
Learning: When shifting by 16 bits per chain ID in the GasZip contracts, the MAX_CHAINID_LENGTH_ALLOWED constant should be set to 16 (since a uint256 can hold at most 16 chain IDs when using 16 bits per chain). This constant should be consistent between related contracts like GasZipPeriphery and GasZipFacet.
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2025-09-12T10:17:51.686Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.686Z
Learning: Applies to src/Facets/**/*Facet.sol : For non-EVM chains: receiverAddress must be bytes (not bytes32), must not be zero, and bridgeData.receiver must equal NON_EVM_ADDRESS
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2025-09-12T10:17:51.686Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.686Z
Learning: Applies to src/Facets/**/*Facet.sol : In {facetName}Data, receiverAddress must be the first field and be validated against bridgeData.receiver; verify targetChainId equals bridgeData.destinationChain for EVM-to-EVM
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2025-02-21T09:00:28.226Z
Learnt from: ezynda3
PR: lifinance/contracts#984
File: src/Facets/ChainflipFacet.sol:127-146
Timestamp: 2025-02-21T09:00:28.226Z
Learning: The `validateBridgeData` modifier in `Validatable.sol` already validates that the receiver address is not zero, making additional zero-address checks redundant in functions using this modifier.
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2025-02-21T09:00:28.226Z
Learnt from: ezynda3
PR: lifinance/contracts#984
File: src/Facets/ChainflipFacet.sol:127-146
Timestamp: 2025-02-21T09:00:28.226Z
Learning: The `validateBridgeData` modifier in `Validatable.sol` validates that the receiver address is not zero using `LibUtil.isZeroAddress`, making additional zero-address checks redundant in functions using this modifier.
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2025-07-16T01:03:08.106Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1275
File: src/Facets/AllBridgeFacet.sol:164-164
Timestamp: 2025-07-16T01:03:08.106Z
Learning: In src/Facets/AllBridgeFacet.sol, the team has decided that explicit validation for address downcasting from `_bridgeData.sendingAssetId` to `bytes32(uint256(uint160(_bridgeData.sendingAssetId)))` is not required, accepting the potential risk of silent overflow from unsafe downcasting.
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2025-08-27T23:36:40.773Z
Learnt from: mirooon
PR: lifinance/contracts#1328
File: test/solidity/Periphery/GasZipPeriphery.t.sol:223-233
Timestamp: 2025-08-27T23:36:40.773Z
Learning: In bridge facet swap flows with requiresDeposit=false, tokens are already held by the diamond contract from previous swaps. When subsequent contracts like GasZipPeriphery call LibAsset.depositAsset, they pull tokens from msg.sender (the diamond) to themselves via transferFrom, requiring no additional approvals since the diamond already owns the tokens.
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2024-10-31T09:09:38.568Z
Learnt from: ezynda3
PR: lifinance/contracts#843
File: src/Facets/RelayFacet.sol:165-170
Timestamp: 2024-10-31T09:09:38.568Z
Learning: In the `RelayFacet` contract (`src/Facets/RelayFacet.sol`), within the `_startBridge` function, low-level `call` is intentionally used to transfer tokens so that extra bytes can be added to the calldata, as required for integrating with Relay Protocol.
Applied to files:
script/demoScripts/demoRelay.ts
🧬 Code graph analysis (1)
script/demoScripts/demoRelay.ts (1)
script/demoScripts/utils/demoScriptHelpers.ts (1)
ADDRESS_USDC_ARB(63-63)
⏰ 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: enforce-min-test-coverage
- GitHub Check: run-unit-tests
🔇 Additional comments (2)
script/demoScripts/demoRelay.ts (2)
321-332: Chain ID usage looks consistent with SUI mapping.Quote uses 103665049 and the on-chain call uses '9270000000000000' (string) which matches the new mapping approach. Good.
If available, please confirm the Relay API expects 103665049 for SUI in quotes while the facet mapping handles 9270000000000000 on-chain. If docs differ, align both sides accordingly.
Also applies to: 375-375
367-379: Emit a hex transactionId, use the NON_EVM_ADDRESS sentinel, and align minAmount; confirm TypeChain types before typing the object
- transactionId: use utils.hexlify(utils.randomBytes(32)) so the value is a hex string compatible with Solidity bytes32.
- TypeChain: add ILiFi.BridgeDataStruct only if generated TypeChain artifacts exist — my search could not find a typechain directory (no generated types). Confirm the generated type name/path before adding the annotation.
- receiver: use the NON_EVM_ADDRESS sentinel defined at src/Helpers/LiFiData.sol (0x11f111f111f111F111f111f111F111f111f111F1).
- minAmount: set to params.amount or derive from quote/sig to respect slippage; do not hardcode "500000".
| const relayData = { | ||
| requestId, | ||
| nonEVMReceiver: `0x${new PublicKey(solanaReceiver) | ||
| .toBuffer() | ||
| .toString('hex')}`, | ||
| receivingAssetId: `0x${new PublicKey(solanaUSDC) | ||
| .toBuffer() | ||
| .toString('hex')}`, | ||
| nonEVMReceiver: ethers.utils.hexZeroPad( | ||
| suiRandomReceiver, | ||
| 32 | ||
| ), | ||
| receivingAssetId: suiUSDC, | ||
| signature: sigData.signature, | ||
| } |
There was a problem hiding this comment.
receivingAssetId must be raw bytes, not a SUI type string.
RelayData.receivingAssetId is a bytes field on-chain. Passing '0xdba...::usdc::USDC' will fail ABI encoding at runtime. Encode the SUI asset identifier as bytes (e.g., UTF‑8) before sending.
Apply this diff:
- const relayData = {
+ const relayData: RelayFacet.RelayDataStruct = {
requestId,
nonEVMReceiver: ethers.utils.hexZeroPad(
suiRandomReceiver,
32
),
- receivingAssetId: suiUSDC,
+ // Encode the SUI asset type string as bytes
+ receivingAssetId: utils.hexlify(utils.toUtf8Bytes(suiUSDC)),
signature: sigData.signature,
}🤖 Prompt for AI Agents
In script/demoScripts/demoRelay.ts around lines 383 to 391,
RelayData.receivingAssetId is currently set to a SUI type string (e.g.
"0xdba...::usdc::USDC") but the on‑chain field expects raw bytes; convert the
SUI asset identifier to raw bytes (UTF‑8) and pass it as a hex byte string
(0x...) or Uint8Array instead of the human‑readable type string so ABI encoding
succeeds.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
script/demoScripts/demoRelay.ts (2)
352-361: Remove excessive debug logging.The repeated
console.log('requestId')statements appear to be debug code that should be cleaned up before committing.- console.log('requestId') - console.log('requestId') - console.log('requestId') - console.log('requestId') - console.log('requestId') console.log(requestId) - console.log(requestId) - console.log(requestId) - console.log(requestId) - console.log(requestId)
412-413: Improve transaction hash logging format.The current logging format could be more descriptive for debugging purposes.
- console.log('tx') - console.log(tx.hash) + console.info('Transaction hash:', tx.hash)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
script/demoScripts/demoRelay.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
script/**/*.ts
📄 CodeRabbit inference engine (conventions.md)
script/**/*.ts: All TypeScript scripts must follow .eslintrc.cjs, use async/await, try/catch error handling, proper logging, environment variables, correct typings, use citty for CLI parsing, consola for logging, validate env vars via getEnvVar(), and exit with appropriate codes
Prefer existing helper functions over reimplementation (e.g., getDeployments, getProvider, getWalletFromPrivateKeyInDotEnv, sendTransaction, ensureBalanceAndAllowanceToDiamond, getUniswapData*)
Always use proper TypeChain types (e.g., ILiFi.BridgeDataStruct) and never use any for bridge data or contract structures
Before/after changes: verify imports and types exist, ensure typechain is generated, keep changes comprehensive and consistent, run TS compilation, remove unused imports, and ensure function signatures match usage
Scripts must execute with bunx tsx
Files:
script/demoScripts/demoRelay.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: 0xDEnYO
PR: lifinance/contracts#782
File: script/deploy/_targetState.json:49-49
Timestamp: 2024-11-25T09:05:03.917Z
Learning: The `RelayFacet` contract, when missing from the source code but referenced in deployment configurations, should be treated the same way as `OpBNBBridgeFacet` and can be ignored in code reviews.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs involving address updates like the RelayFacet to Worldchain, verify the actual presence of entries in files before reporting issues. The RelayFacet exists in the diamond log file and the PR diff already contains the necessary address change.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1109
File: deployments/worldchain.json:28-28
Timestamp: 2025-04-21T03:17:53.443Z
Learning: For deployment PRs updating contract addresses (like RelayFacet on Worldchain), verify the presence of entries in all relevant files (worldchain.json, worldchain.diamond.json, _deployments_log_file.json) before reporting inconsistencies. The RelayFacet entry exists in all required deployment files with the correct address.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1381
File: deployments/arbitrum.json:65-69
Timestamp: 2025-09-16T01:39:54.099Z
Learning: When verifying facet deployments across .json and .diamond.json files, search by facet name rather than trying to cross-reference addresses between the files, as the same contract can have different addresses in different deployment files.
Learnt from: mirooon
PR: lifinance/contracts#975
File: test/solidity/Facets/DeBridgeDlnFacet.t.sol:0-0
Timestamp: 2025-02-17T07:59:54.979Z
Learning: In DeBridgeDlnFacet, chain ID mappings are hardcoded and not designed to be dynamically updated, so tests for mapping updates are not needed.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1321
File: docs/RelayDepositoryFacet.md:9-10
Timestamp: 2025-08-28T02:41:07.505Z
Learning: Unit tests for RelayDepositoryFacet cannot verify fund forwarding behavior after deposits because the facet delegates to external IRelayDepository contracts. The forwarding logic is implemented in the Relay Protocol V2 Depository contracts, not in the facet itself.
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.686Z
Learning: Applies to src/Facets/**/*Facet.sol : In {facetName}Data, receiverAddress must be the first field and be validated against bridgeData.receiver; verify targetChainId equals bridgeData.destinationChain for EVM-to-EVM
Learnt from: mirooon
PR: lifinance/contracts#985
File: script/deploy/_targetState.json:0-0
Timestamp: 2025-02-12T09:44:12.961Z
Learning: The bsca network intentionally maintains different facet versions between staging and production environments, specifically:
1. CalldataVerificationFacet: v1.1.1 in staging vs v1.1.2 in production
2. EmergencyPauseFacet: present only in production
3. Permit2Proxy: present only in production
Learnt from: ezynda3
PR: lifinance/contracts#843
File: script/deploy/facets/DeployRelayFacet.s.sol:0-0
Timestamp: 2024-10-31T09:07:36.301Z
Learning: In `script/deploy/facets/DeployRelayFacet.s.sol`, additional validation for the configuration file existence and address validity is unnecessary because the code will already fail appropriately if the configuration file is missing or the addresses are invalid.
Learnt from: ezynda3
PR: lifinance/contracts#843
File: src/Facets/RelayFacet.sol:165-170
Timestamp: 2024-10-31T09:09:38.568Z
Learning: In the `RelayFacet` contract (`src/Facets/RelayFacet.sol`), within the `_startBridge` function, low-level `call` is intentionally used to transfer tokens so that extra bytes can be added to the calldata, as required for integrating with Relay Protocol.
📚 Learning: 2025-05-14T16:28:30.376Z
Learnt from: mirooon
PR: lifinance/contracts#1137
File: src/Periphery/GasZipPeriphery.sol:21-23
Timestamp: 2025-05-14T16:28:30.376Z
Learning: When shifting by 16 bits per chain ID in the GasZip contracts, the MAX_CHAINID_LENGTH_ALLOWED constant should be set to 16 (since a uint256 can hold at most 16 chain IDs when using 16 bits per chain). This constant should be consistent between related contracts like GasZipPeriphery and GasZipFacet.
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2025-07-16T01:03:08.106Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1275
File: src/Facets/AllBridgeFacet.sol:164-164
Timestamp: 2025-07-16T01:03:08.106Z
Learning: In src/Facets/AllBridgeFacet.sol, the team has decided that explicit validation for address downcasting from `_bridgeData.sendingAssetId` to `bytes32(uint256(uint160(_bridgeData.sendingAssetId)))` is not required, accepting the potential risk of silent overflow from unsafe downcasting.
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2025-09-12T10:17:51.686Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.686Z
Learning: Applies to src/Facets/**/*Facet.sol : For non-EVM chains: receiverAddress must be bytes (not bytes32), must not be zero, and bridgeData.receiver must equal NON_EVM_ADDRESS
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2025-09-12T10:17:51.686Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-09-12T10:17:51.686Z
Learning: Applies to src/Facets/**/*Facet.sol : In {facetName}Data, receiverAddress must be the first field and be validated against bridgeData.receiver; verify targetChainId equals bridgeData.destinationChain for EVM-to-EVM
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2025-02-21T09:00:28.226Z
Learnt from: ezynda3
PR: lifinance/contracts#984
File: src/Facets/ChainflipFacet.sol:127-146
Timestamp: 2025-02-21T09:00:28.226Z
Learning: The `validateBridgeData` modifier in `Validatable.sol` already validates that the receiver address is not zero, making additional zero-address checks redundant in functions using this modifier.
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2025-02-21T09:00:28.226Z
Learnt from: ezynda3
PR: lifinance/contracts#984
File: src/Facets/ChainflipFacet.sol:127-146
Timestamp: 2025-02-21T09:00:28.226Z
Learning: The `validateBridgeData` modifier in `Validatable.sol` validates that the receiver address is not zero using `LibUtil.isZeroAddress`, making additional zero-address checks redundant in functions using this modifier.
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2025-08-27T23:36:40.773Z
Learnt from: mirooon
PR: lifinance/contracts#1328
File: test/solidity/Periphery/GasZipPeriphery.t.sol:223-233
Timestamp: 2025-08-27T23:36:40.773Z
Learning: In bridge facet swap flows with requiresDeposit=false, tokens are already held by the diamond contract from previous swaps. When subsequent contracts like GasZipPeriphery call LibAsset.depositAsset, they pull tokens from msg.sender (the diamond) to themselves via transferFrom, requiring no additional approvals since the diamond already owns the tokens.
Applied to files:
script/demoScripts/demoRelay.ts
📚 Learning: 2024-10-31T09:09:38.568Z
Learnt from: ezynda3
PR: lifinance/contracts#843
File: src/Facets/RelayFacet.sol:165-170
Timestamp: 2024-10-31T09:09:38.568Z
Learning: In the `RelayFacet` contract (`src/Facets/RelayFacet.sol`), within the `_startBridge` function, low-level `call` is intentionally used to transfer tokens so that extra bytes can be added to the calldata, as required for integrating with Relay Protocol.
Applied to files:
script/demoScripts/demoRelay.ts
🧬 Code graph analysis (1)
script/demoScripts/demoRelay.ts (1)
script/demoScripts/utils/demoScriptHelpers.ts (1)
ADDRESS_USDC_ARB(63-63)
⏰ 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: enforce-min-test-coverage
🔇 Additional comments (5)
script/demoScripts/demoRelay.ts (5)
378-378: Use proper NON_EVM_ADDRESS constant.The hardcoded address
'0x11f111f111f111F111f111f111F111f111f111F1'appears to be a placeholder for non-EVM bridging. Consider using the establishedNON_EVM_ADDRESSconstant if available in your codebase.- receiver: '0x11f111f111f111F111f111f111F111f111f111F1', // Non-EVM address + receiver: address, // Use sender's address as EVM receiver for non-EVM bridges
395-401: Good defensive programming with balance check.The balance validation before attempting the bridge transaction is a solid practice that prevents unnecessary transaction failures.
330-330: Chain ID type inconsistency and unsafe numeric representation.The chain ID
103665049is used here as a number, but later at line 380 the same chain ID is represented as a string'9270000000000000'. Both values appear to represent SUI, but they don't match numerically. Additionally,9270000000000000exceeds JavaScript'sMAX_SAFE_INTEGERand should be handled as a string to prevent precision loss.Verify which chain ID value is correct for SUI and ensure consistent representation:
// If 103665049 is correct: destinationChainId: 103665049, // SUI // OR if 9270000000000000 is correct: destinationChainId: '9270000000000000', // SUI (as string to prevent precision loss)
380-380: Chain ID type inconsistency with params object.This string representation
'9270000000000000'doesn't match the numeric103665049used in the params object at line 330. The inconsistency will cause confusion and potentially incorrect bridging behavior.Ensure consistent chain ID representation throughout:
- destinationChainId: '9270000000000000', // SUI + destinationChainId: 103665049, // SUIOr verify which value is correct and update both locations accordingly.
390-391: Incorrect handling of non-EVM receiver and asset ID for SUI.Two issues here:
nonEVMReceivershould be properly padded to 32 bytes for ABI encoding- The asset ID truncation logic is undocumented and fragile - taking "first 66 characters (32 bytes)" from a SUI type string may not produce valid bytes
Apply this fix:
- nonEVMReceiver: suiRandomReceiver, - receivingAssetId: suiUSDC.substring(0, 66), // take first 66 characters (32 bytes) - there is no documentation on this, Relay team said to do this "We take the first 32 bytes of the currency when we generate the signature" + nonEVMReceiver: ethers.utils.hexZeroPad(suiRandomReceiver, 32), + receivingAssetId: ethers.utils.hexlify(ethers.utils.toUtf8Bytes(suiUSDC)),
Which Jira task belongs to this PR?
LF-15494
Why did I implement it this way?
Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)