Skip to content

Update LIFI Intent Data to more easily support destination swaps [LiFiIntentEscrowFacet v1.1.0]#1588

Open
reednaa wants to merge 12 commits intomainfrom
lifi-intent-destination-call
Open

Update LIFI Intent Data to more easily support destination swaps [LiFiIntentEscrowFacet v1.1.0]#1588
reednaa wants to merge 12 commits intomainfrom
lifi-intent-destination-call

Conversation

@reednaa
Copy link
Member

@reednaa reednaa commented Jan 16, 2026

Which Jira task belongs to this PR?

https://lifi-protocol.slack.com/archives/C08C4SQA5N3/p1768480592006519
Closes: API-200
Closes: https://linear.app/lifi-linear/issue/API-200/lifi-intent-facet-does-not-correctly-allow-destination-swaps
Closes: LII-270
Closes: https://lifi.atlassian.net/browse/LII-270?atlOrigin=eyJpIjoiZmMzMWRjZjg2ZTgwNDc5NmFiZmEzNTZhODcyMGZjMmIiLCJwIjoiaiJ9

Why did I implement it this way?

The previous implementation relied on the LI.FI backend to wrap and handle delivery of assets correctly. An unfortunate side effect was that implementation overhead associated with destination swaps was placed on the caller: including setting the recipient to the periphery contract ReceiverOIF.sol and encoding the "true" recipient in the calldata required by ReceiverOIF.sol.

This PR fixes this problem by generating appropriate calldata for ReceiverOIF.sol on-chain and setting the correct LI.FI intent recipient using now provided .dstCallSwapData and .dstCallReceiver.

This PR also streamlines some of the non_evm access functions, including optimising a single address 0 check.

Breaking Changes

The LiFiIntentEscrowData is modified:

  • New bytes32 field: .dstCallReceiver: Takes in the remote execution contract like Chainflip.
  • bytes outputCall -> LibSwap.SwapData[] dstCallSwapData
    Since LiFiIntentEscrowData is used in the facet entry functions, the entry functions' selectors have changed.

To migrate to the new contracts without destination calls:

  • set . dstCallSwapData to [] instead of 0x
  • set . dstCallReceiver to address(0).
  • Update ABI to new functions.

Checklist before requesting a review

Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)

  • I have checked that any arbitrary calls to external contracts are validated and or restricted
  • I have checked that any privileged calls (i.e. storage modifications) are validated and or restricted
  • I have ensured that any new contracts have had AT A MINIMUM 1 preliminary audit conducted on by <company/auditor>

@lifi-action-bot lifi-action-bot marked this pull request as draft January 16, 2026 09:07
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 16, 2026

Walkthrough

LiFiIntentEscrowFacet's public data and runtime flow were changed to support destination swaps: receiverAddress/outputCall replaced by dstCallReceiver, recipient, and dstCallSwapData; destination-call detection uses dstCallSwapData.length; callback/output is encoded as (transactionId, dstCallSwapData, recipient); non‑EVM bridges emit a bytes32 recipient event.

Changes

Cohort / File(s) Summary
Documentation
docs/LiFiIntentEscrowFacet.md
Updated docs to reflect destination-swap flow: receiverAddress/outputCall replaced by dstCallReceiver, recipient, and dstCallSwapData; parameter docs and mapping into StandardOrder.outputs updated based on dstCallSwapData.length.
Facet implementation
src/Facets/LiFiIntentEscrowFacet.sol
Version bumped to 1.1.0. LiFiIntentEscrowData redefined: removed receiverAddress and outputCall; added dstCallReceiver, recipient, and LibSwap.SwapData[] dstCallSwapData. Destination-call validation uses dstCallSwapData.length. Constructs callback outputCall by abi‑encoding (transactionId, dstCallSwapData, recipient) when swaps exist; external calldata can override recipient to dstCallReceiver. Emits BridgeToNonEVMChainBytes32(transactionId, destinationChainId, recipient) for non‑EVM bridges. Restores population of StandardOrder.inputs.
Tests & periphery scaffolding
test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
Tests adapted to new struct shape: added periphery imports/deployments (ReceiverOIF, Executor, ERC20Proxy, TokenWrapper, OutputSettler), introduced _validLIFIIntentData() helper, recorded allowed selectors for TokenWrapper, replaced receiverAddress/outputCall usages with recipient/dstCallReceiver/dstCallSwapData, and added test_SwapNativeToNativeWithSwaps covering callback encoding and ETH settlement.
Audit manifest
audit/auditLog.json
Added new audit entry audit20260130 and mapped LiFiIntentEscrowFacet version 1.1.0 to that audit record.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • lifinance/contracts#1361 — prior changes to LiFiIntentEscrowFacet and LiFiIntentEscrowData that overlap with the struct and destination-call handling updates.
  • lifinance/contracts#1275 — related non‑EVM receiver handling and BridgeToNonEVMChain logic adjustments.
  • lifinance/contracts#1518 — introduces ReceiverOIF/OutputSettler periphery wiring referenced by the new destination-swap callback flows.

Suggested labels

AuditCompleted

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: updating LiFiIntentEscrowData to better support destination swaps, with the version bump noted.
Description check ✅ Passed The description covers the Jira task references, implementation rationale, breaking changes, and migration guidance; most template sections are addressed with substantive content.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch lifi-intent-destination-call

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • JIRA integration encountered authorization issues. Please disconnect and reconnect the integration in the CodeRabbit UI.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@lifi-action-bot lifi-action-bot changed the title Update LIFI Intent Data to more easily support destination swaps Update LIFI Intent Data to more easily support destination swaps [LiFiIntentEscrowFacet v1.1.0] Jan 16, 2026
@lifi-action-bot
Copy link
Collaborator

lifi-action-bot commented Jan 16, 2026

Test Coverage Report

Line Coverage: 86.75% (2981 / 3436 lines)
Function Coverage: 90.30% ( 475 / 526 functions)
Branch Coverage: 65.45% ( 485 / 741 branches)
Test coverage (86.75%) is above min threshold (83%). Check passed.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/Facets/LiFiIntentEscrowFacet.sol (1)

146-195: Prevent destination swaps when receiver is NON_EVM_ADDRESS.

ReceiverOIF-based swaps can’t execute on non‑EVM chains. With NON_EVM_ADDRESS plus non‑empty dstCallSwapData, the order will encode ReceiverOIF calldata and set outputs.recipient to dstCallReceiver, which is invalid for non‑EVM destinations. Add an explicit guard to reject this combination.

🐛 Suggested guard
-        if (_bridgeData.receiver == NON_EVM_ADDRESS) {
+        if (_bridgeData.receiver == NON_EVM_ADDRESS) {
+            if (_lifiIntentData.dstCallSwapData.length != 0) {
+                revert InvalidReceiver();
+            }
             emit BridgeToNonEVMChain(
                 _bridgeData.transactionId,
                 _bridgeData.destinationChainId,
                 abi.encodePacked(recipient)
             );
         } else {
🤖 Fix all issues with AI agents
In `@docs/LiFiIntentEscrowFacet.md`:
- Line 5: Fix minor grammar and hyphenation in the LI.FI Intent Escrow
documentation: update the sentence about the facet so verbs agree and hyphenate
compound adjectives consistently (e.g., change "built in escrow" to "built-in
escrow", ensure "will release the deposited funds to the solver when the fill
has been proven" reads clearly with correct tense/subject agreement), and apply
the same edits to the repeated lines at 34-35; locate the paragraph referencing
"LI.FI Intent Escrow Facet" and "Escrow Input Settler" and adjust wording for
correct hyphenation and verb agreement across those instances.
🧹 Nitpick comments (1)
test/solidity/Facets/LiFiIntentEscrowFacet.t.sol (1)

517-605: Make the positive‑slippage test deterministic.

The assertion only triggers when actualUSDCOut > expectedUSDCOut, but actualUSDCOut reads the facet balance after _startBridge (often zero). This means the test often skips validation. Consider lowering bridgeData.minAmount to force positive slippage and assert the exact delta.

✅ Possible test adjustment
-        bridgeData.minAmount = expectedUSDCOut; // Minimum USDC expected from swap
+        bridgeData.minAmount = expectedUSDCOut - 1; // Force positive slippage by 1

 ...
-        uint256 actualUSDCOut = usdc.balanceOf(address(lifiIntentEscrowFacet));
-
         uint256 userUSDCBalanceAfter = usdc.balanceOf(USER_SENDER);
         uint256 positiveSlippage = userUSDCBalanceAfter -
             userUSDCBalanceBefore;

-        if (actualUSDCOut > expectedUSDCOut) {
-            assertEq(
-                positiveSlippage,
-                actualUSDCOut - expectedUSDCOut,
-                "Positive slippage not returned to user"
-            );
-        }
+        assertEq(
+            positiveSlippage,
+            expectedUSDCOut - bridgeData.minAmount,
+            "Positive slippage not returned to user"
+        );
📜 Review details

Configuration used: Repository 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 388d634 and c3e8569.

📒 Files selected for processing (3)
  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
🧰 Additional context used
📓 Path-based instructions (10)
test/solidity/**/*.sol

📄 CodeRabbit inference engine (.cursor/rules/001-project-structure.mdc)

Place test files in test/solidity/ mirroring the src/ directory structure for organization

Files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
**/*.sol

📄 CodeRabbit inference engine (.cursor/rules/001-project-structure.mdc)

Use remappings defined in remappings.txt for imports: lifi/src/, test/test/, and external libs like @openzeppelin/, solmate/, solady/, permit2/

**/*.sol: Single Diamond (EIP-2535) as main entrypoint for all protocol interactions
Delegate complex logic to libraries (LibAsset, LibSwap, LibAllowList, SwapperV2, Validatable) and helper contracts

After Solidity changes, run forge test (or note suites remaining)

**/*.sol: Own files must use // SPDX-License-Identifier: LGPL-3.0-only immediately followed by the pragma statement with no blank line in between
All contracts must use pragma solidity ^0.8.17;
Functions and variables use camelCase; constants and immutables are CONSTANT_CASE
Function parameters use leading underscore (e.g., _amount)
Contracts and interfaces must include NatSpec: @title (matching contract/interface name), @author LI.FI (https://li.fi), @notice describing purpose, and @custom:version X.Y.Z
Public and external functions require NatSpec including params and returns documentation
For pure test/script scaffolding keep NatSpec headers minimal but retain SPDX and pragma
Use single blank lines between logical sections and between function declarations
Follow in-function blank-line rules: blank lines before emits/returns; no stray gaps
Use custom errors instead of revert strings; prefer existing errors/helpers before adding new ones
Use generic errors from src/Errors/GenericErrors.sol; bump @custom:version when adding; facet-specific errors stay local
Adhere to rules in .solhint.json
Avoid assembly unless necessary and heavily commented with justification (why assembly is needed); prefer existing helpers over new implementations

Do not mix interface and implementation in the same file

Files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
**/test/**/*.sol

📄 CodeRabbit inference engine (.cursor/rules/103-solidity-interfaces.mdc)

Test interfaces may define inline interfaces as needed for testing purposes

Files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
{src,script,test}/**/*.{sol,ts}

📄 CodeRabbit inference engine (.cursor/rules/105-security.mdc)

Validate all external inputs and configuration (including script/env values) explicitly; prefer existing validation helpers (e.g., Validatable, config readers) over ad-hoc checks

Files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
test/**/*.t.sol

📄 CodeRabbit inference engine (.cursor/rules/400-solidity-tests.mdc)

test/**/*.t.sol: Tests under test/solidity/, mirroring src/ structure; require setUp() function; call initTestBase() when inheriting TestBase directly or indirectly; label actors with vm.label
For facet tests, inherit from TestBaseFacet rather than TestBase, since standard facet functions need to be overridden
Import ordering: system libraries first (e.g., forge-std, ds-test), then project files (e.g., lifi/, test/)
Test function names: test_ prefix for success tests, testRevert_ prefix for failure tests, testBase_ prefix for base tests
Test structure: setup → execute → assert; use vm.startPrank / vm.stopPrank, labels, and base inits
Always assert specific revert reasons; use vm.expectRevert with specific reason
Use vm.expectEmit(true, true, true, true, <addr>) for event assertions
Apply blank line conventions: add gap after vm.expectRevert before the call, and gap before assertions/events
For whitelist flows, inherit TestWhitelistManagerBase and use addToWhitelist / setFunctionWhitelistBySelector helpers
If a facet does not support native tokens, override tests such as testBase_CanSwapAndBridgeNativeTokens and testBase_CanBridgeNativeTokens with public override and include a comment explaining why the test is intentionally skipped

Files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
**/*.{test.ts,t.sol}

📄 CodeRabbit inference engine (.cursor/rules/401-testing-patterns.mdc)

**/*.{test.ts,t.sol}: Prefer adding or updating tests alongside logic changes
Keep tests structured as setup → execute → assert
Assert specific failure conditions (avoid overly-broad "catch-all" assertions)

Files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
src/Facets/**/*.sol

📄 CodeRabbit inference engine (.cursor/rules/001-project-structure.mdc)

src/Facets/**/*.sol: Place new facets in the src/Facets/ directory and use plop templates if available for code generation
Keep facets thin and delegate complex logic to libraries. Group imports in order: external libs → interfaces → libraries → contracts

src/Facets/**/*.sol: Facets provide modular functionality grouped by concern (bridges, swaps, receivers, admin, etc.)
Facets should contain thin, integration-specific logic only; do not move logic across layers without clear architectural reason

Files:

  • src/Facets/LiFiIntentEscrowFacet.sol
src/Facets/**/*Facet.sol

📄 CodeRabbit inference engine (.cursor/rules/102-facets.mdc)

src/Facets/**/*Facet.sol: Facets must be located in src/Facets/ directory with names containing Facet
Facets must implement required functions: _startBridge (internal), swapAndStartBridgeTokensVia{FacetName}, and startBridgeTokensVia{FacetName}
Facets must use modifiers: nonReentrant, refundExcessNative, validateBridgeData, doesNotContainSourceSwaps/doesContainSourceSwaps, doesNotContainDestinationCalls/doesContainDestinationCalls on applicable functions
In Solidity facets, receiverAddress must be the first parameter in {facetName}Data and must match bridgeData.receiver for EVM chains
In Solidity facets, validate targetChainId against bridgeData.destinationChain for EVM-to-EVM bridges
Facets must use LibAsset, LibSwap, and LibAllowList utilities along with Validatable and SwapperV2 interfaces; reserve native fees via _depositAndSwap variants when needed
In Solidity facets with minAmountOut or similar bridge parameters, update the bridge's minAmountOut in swapAndStartBridgeTokensVia{FacetName} to account for positive slippage from swaps after _depositAndSwap updates _bridgeData.minAmount, adjusting proportionally for decimal differences if applicable
For non-EVM receivers in Solidity facets, use bytes type; the value must be non-zero
For non-EVM flows in Solidity facets, bridgeData.receiver must equal NON_EVM_ADDRESS
In Solidity facets with {facetName}Data.receiverAddress field (e.g., _glacisData.receiverAddress), validate that it is not bytes32(0) for non-EVM chains and revert with InvalidNonEVMReceiver() if zero

Files:

  • src/Facets/LiFiIntentEscrowFacet.sol
{src,script}/**/*.sol

📄 CodeRabbit inference engine (.cursor/rules/105-security.mdc)

{src,script}/**/*.sol: Do not weaken existing access controls, timelock flows, or Safe multisig protections; any change that touches admin-only functionality must call out its governance impact
Avoid introducing new external call patterns in facets or scripts without checking existing libraries (LibAsset, LibSwap, LibAllowList) for prior art

Files:

  • src/Facets/LiFiIntentEscrowFacet.sol
src/**/*.sol

📄 CodeRabbit inference engine (.cursor/rules/106-gas.mdc)

src/**/*.sol: Follow gas-related best practices in existing facets before introducing new micro-optimizations in Solidity
Prefer using existing optimized libraries already in the repo (e.g., Solady/Solmate utilities) where they are in use, rather than hand-rolled low-level code
Do not sacrifice readability or safety for minor gas wins; explain any non-obvious optimization and justify it with expected impact
Avoid introducing inline assembly unless clearly needed; when used, document assumptions and invariants thoroughly

All contracts in src/ require audits except Interfaces in src/Interfaces/** (type definitions only) and external dependencies in lib/

src/**/*.sol: NatSpec required on contracts/interfaces in src/: @title, @author LI.FI (https://li.fi), @notice, @custom:version X.Y.Z; document all public/external functions with params/returns.
Bug bounty clarity: In contract NatSpec, explicitly document whether the contract is intended to hold/custody funds. If not designed to hold funds, state clearly (e.g., 'This contract is not intended to custody user funds / hold balances; any funds held are incidental'). If designed to hold funds, describe what funds it holds and under what conditions.
Apply Diamond patterns with existing libs (LibAsset/LibSwap/LibAllowList, Validatable/SwapperV2); prefer parameters over msg.sender for refund addresses.
In Solidity 0.8.17, you must not emit events using ContractName.EventName syntax (e.g., emit SomeContract.SomeEvent(...)). Events must be defined in the same contract where they're emitted, or defined in an interface that the contract uses, and then emitted using just the event name (e.g., emit SomeEvent(...)). This prevents compilation errors and ensures 0.8.17 compatibility.

Files:

  • src/Facets/LiFiIntentEscrowFacet.sol
🧠 Learnings (64)
📓 Common learnings
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must implement internal `_swapAndCompleteBridgeTokens()` function that handles native vs ERC20 tokens: use `safeApproveWithRetry()` or `safeIncreaseAllowance()` for ERC20 (reset to 0 after), pass `value: amount` for native tokens, call `executor.swapAndCompleteBridgeTokens()` in try-catch block, and on failure send raw tokens to receiver and emit `LiFiTransferRecovered`
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must only emit `LiFiTransferRecovered` when swaps fail and raw bridged tokens are sent to receiver, with event signature: `LiFiTransferRecovered(bytes32 indexed transactionId, address receivingAssetId, address receiver, uint256 amount, uint256 timestamp)`
Learnt from: reednaa
Repo: lifinance/contracts PR: 1361
File: script/demoScripts/demoLiFiIntentEscrow.ts:269-269
Timestamp: 2025-10-10T12:50:47.440Z
Learning: In `src/Interfaces/IOpenIntentFramework.sol`, the `StandardOrder` struct's `inputOracle` field is of type `address` (20 bytes), not `bytes32`. It should not be padded to 32 bytes when constructing `LiFiIntentEscrowData` in TypeScript/JavaScript code.
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must implement a bridge-specific external entry point (e.g., `handleV3AcrossMessage`, `lzCompose`, `cfReceive`) protected by authorization modifier that decodes bridge message to extract `bytes32 transactionId`, `LibSwap.SwapData[] memory swapData`, `address receiver`, and calls internal `_swapAndCompleteBridgeTokens()`
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must never emit `LiFiTransferStarted` (reserved for facets)
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/002-architecture.mdc:0-0
Timestamp: 2025-12-17T10:27:21.083Z
Learning: Applies to src/Periphery/Receiver*.sol : LiFiTransferRecovered event should only be emitted in Receiver contracts (src/Periphery/Receiver*.sol)
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : For non-EVM flows in Solidity facets, `bridgeData.receiver` must equal `NON_EVM_ADDRESS`
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1275
File: src/Facets/AllBridgeFacet.sol:186-188
Timestamp: 2025-07-16T01:04:55.857Z
Learning: In LiFi facet contracts, when public entry point functions have `nonReentrant` modifier protection, internal functions like `_startBridge` that they call benefit from this reentrancy protection, making event emission order after external calls acceptable from a security perspective.
Learnt from: mirooon
Repo: lifinance/contracts PR: 1413
File: src/Facets/EverclearV2Facet.sol:75-96
Timestamp: 2025-10-13T11:13:48.847Z
Learning: In EverclearV2Facet (and all LiFi facets), the team standard is to use msg.sender as the refund address with the refundExcessNative modifier, not requiring an explicit refund address parameter.
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/002-architecture.mdc:0-0
Timestamp: 2025-12-17T10:27:21.083Z
Learning: Applies to src/Facets/**/*Bridge*.sol : For non-EVM chains (bridgeData.receiver == NON_EVM_ADDRESS), emit BridgeToNonEVMChainBytes32 with transactionId, destinationChainId, and non-EVM receiver (bytes32)
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : In Solidity facets, `receiverAddress` must be the first parameter in `{facetName}Data` and must match `bridgeData.receiver` for EVM chains
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must implement internal `_swapAndCompleteBridgeTokens()` function that handles native vs ERC20 tokens: use `safeApproveWithRetry()` or `safeIncreaseAllowance()` for ERC20 (reset to 0 after), pass `value: amount` for native tokens, call `executor.swapAndCompleteBridgeTokens()` in try-catch block, and on failure send raw tokens to receiver and emit `LiFiTransferRecovered`

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must only emit `LiFiTransferRecovered` when swaps fail and raw bridged tokens are sent to receiver, with event signature: `LiFiTransferRecovered(bytes32 indexed transactionId, address receivingAssetId, address receiver, uint256 amount, uint256 timestamp)`

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must implement a bridge-specific external entry point (e.g., `handleV3AcrossMessage`, `lzCompose`, `cfReceive`) protected by authorization modifier that decodes bridge message to extract `bytes32 transactionId`, `LibSwap.SwapData[] memory swapData`, `address receiver`, and calls internal `_swapAndCompleteBridgeTokens()`

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : In Solidity facets with `minAmountOut` or similar bridge parameters, update the bridge's minAmountOut in `swapAndStartBridgeTokensVia{FacetName}` to account for positive slippage from swaps after `_depositAndSwap` updates `_bridgeData.minAmount`, adjusting proportionally for decimal differences if applicable

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:27:21.083Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/002-architecture.mdc:0-0
Timestamp: 2025-12-17T10:27:21.083Z
Learning: Applies to src/Facets/**/*Bridge*.sol : For non-EVM chains (bridgeData.receiver == NON_EVM_ADDRESS), emit BridgeToNonEVMChainBytes32 with transactionId, destinationChainId, and non-EVM receiver (bytes32)

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-08-27T13:07:58.254Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: src/Periphery/Lda/Facets/CurveFacet.sol:0-0
Timestamp: 2025-08-27T13:07:58.254Z
Learning: In the LiFi contracts codebase, ICurveLegacy interface is defined inline in src/Periphery/LiFiDEXAggregator.sol with a non-payable exchange function. The CurveFacet diff shows importing it from a separate file that doesn't exist, creating a compilation issue.

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must never emit `LiFiTransferStarted` (reserved for facets)

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-07-16T01:04:55.857Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1275
File: src/Facets/AllBridgeFacet.sol:186-188
Timestamp: 2025-07-16T01:04:55.857Z
Learning: In LiFi facet contracts, when public entry point functions have `nonReentrant` modifier protection, internal functions like `_startBridge` that they call benefit from this reentrancy protection, making event emission order after external calls acceptable from a security perspective.

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : For non-EVM flows in Solidity facets, `bridgeData.receiver` must equal `NON_EVM_ADDRESS`

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-10-10T12:50:47.440Z
Learnt from: reednaa
Repo: lifinance/contracts PR: 1361
File: script/demoScripts/demoLiFiIntentEscrow.ts:269-269
Timestamp: 2025-10-10T12:50:47.440Z
Learning: In `src/Interfaces/IOpenIntentFramework.sol`, the `StandardOrder` struct's `inputOracle` field is of type `address` (20 bytes), not `bytes32`. It should not be padded to 32 bytes when constructing `LiFiIntentEscrowData` in TypeScript/JavaScript code.

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-06-13T08:30:26.220Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1207
File: deployments/_deployments_log_file.json:34037-34080
Timestamp: 2025-06-13T08:30:26.220Z
Learning: LiFiDiamond contains generic withdrawal logic, so individual facet contracts (e.g., PioneerFacet) do not need their own Ether-withdraw functions.

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-10-13T11:13:48.847Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1413
File: src/Facets/EverclearV2Facet.sol:75-96
Timestamp: 2025-10-13T11:13:48.847Z
Learning: In EverclearV2Facet (and all LiFi facets), the team standard is to use msg.sender as the refund address with the refundExcessNative modifier, not requiring an explicit refund address parameter.

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-02-13T08:57:00.095Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 984
File: docs/ChainflipFacet.md:0-0
Timestamp: 2025-02-13T08:57:00.095Z
Learning: The ChainflipData struct in ChainflipFacet has three fields:
1. uint32 dstToken - The destination token identifier in Chainflip
2. bytes32 nonEvmAddress - The non-EVM destination address
3. bytes cfParameters - Additional parameters for Chainflip protocol

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-04-28T07:46:24.084Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 1116
File: src/Libraries/LibAsset.sol:102-108
Timestamp: 2025-04-28T07:46:24.084Z
Learning: In the LiFi contracts, the `LibAsset.transferFromERC20` function is intentionally designed to accept arbitrary `from` parameters, with security ensured through a whitelist mechanism that controls which contracts can call this function.

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : In Solidity facets, `receiverAddress` must be the first parameter in `{facetName}Data` and must match `bridgeData.receiver` for EVM chains

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-01-22T12:36:12.699Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 945
File: test/solidity/Facets/GlacisFacet.t.sol:214-262
Timestamp: 2025-01-22T12:36:12.699Z
Learning: In the LiFi contracts, token pair validation is handled by third-party contracts, and route discovery including liquidity checks are performed off-chain. The contract only receives pre-validated routes, making token pair and liquidity validation tests unnecessary at the contract level.

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : Facets must use LibAsset, LibSwap, and LibAllowList utilities along with Validatable and SwapperV2 interfaces; reserve native fees via `_depositAndSwap` variants when needed

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2024-12-04T02:05:41.355Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 832
File: src/Periphery/LiFiDEXAggregator.sol:578-697
Timestamp: 2024-12-04T02:05:41.355Z
Learning: Unit tests are not required for newly added swap callback functions in `LiFiDEXAggregator.sol`.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : Facets must use modifiers: `nonReentrant`, `refundExcessNative`, `validateBridgeData`, `doesNotContainSourceSwaps`/`doesContainSourceSwaps`, `doesNotContainDestinationCalls`/`doesContainDestinationCalls` on applicable functions

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-10-02T18:10:09.934Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1406
File: src/Facets/UnitFacet.sol:75-81
Timestamp: 2025-10-02T18:10:09.934Z
Learning: In UnitFacet.sol and similar facet contracts in src/Facets/, the LiFiTransferStarted event emission should ALWAYS be at the end of the _startBridge internal function, after all validations and asset transfers have been completed successfully.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must use `LibAsset` for asset operations, `LibSwap` for swap data structures, and `SafeTransferLib` from solady for safe transfers

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must never emit `LiFiTransferCompleted` (reserved for Executor contract)

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must use try-catch for executor calls, always have fallback to send raw tokens on failure, and reset approvals to 0 after operations

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must include immutable storage for `IExecutor public immutable executor` and bridge-specific authorized caller (e.g., `spokepool`, `chainflipVault`, `endpointV2`)

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must inherit from `ILiFi` and `WithdrawablePeriphery`

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : For native tokens with gas checks (e.g., StargateV2), Receiver contracts must check `gasleft()` before executing; if insufficient gas, send tokens directly to receiver and emit `LiFiTransferRecovered`

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-05-06T09:09:38.108Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1117
File: test/solidity/Periphery/LiFiDEXAggregator.t.sol:111-127
Timestamp: 2025-05-06T09:09:38.108Z
Learning: The `setupApechain()` function in LiFiDEXAggregator tests intentionally avoids calling `initTestBase()` to prevent needing to define standard contracts (like ADDRESS_USDC_PROXY and Uniswap) that aren't needed for Apechain-specific tests.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-07-11T09:43:22.393Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1193
File: test/solidity/Facets/AllowListMigratorFacet.t.sol:39-52
Timestamp: 2025-07-11T09:43:22.393Z
Learning: For fork-based tests like AllowListMigratorFacet.t.sol that work with existing deployed contracts (mainnet diamond), initTestBase() is intentionally omitted since the standard test initialization is not needed.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-02-24T09:35:34.908Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 984
File: src/Periphery/ReceiverChainflip.sol:48-62
Timestamp: 2025-02-24T09:35:34.908Z
Learning: Contract address validations for periphery contracts like ReceiverChainflip are handled in deployment scripts rather than in the constructor.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-11-25T02:57:58.015Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1466
File: config/whitelist.json:2546-2554
Timestamp: 2025-11-25T02:57:58.015Z
Learning: In lifinance/contracts config/whitelist.json, the SushiSwap Aggregator uses functions:
- snwap(address,uint256,address,address,uint256,address,bytes) -> selector 0x5f3bd1c8
- snwapMultiple((address,uint256,address)[],(address,address,uint256)[],(address,uint256,bytes)[]) -> selector 0xd33721a5
These selectors are canonical across networks and valid for the monad entry as well. Always compute with Keccak-256 (not SHA3-256) when verifying.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-08-29T11:12:02.184Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: src/Periphery/LDA/Facets/SyncSwapV2Facet.sol:33-38
Timestamp: 2025-08-29T11:12:02.184Z
Learning: In LDA facets (src/Periphery/Lda/Facets/**/*.sol), tokenIn parameter validation (checking for address(0)) is intentionally not performed as part of the design architecture.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-08-27T23:36:40.773Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 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:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-02-11T10:35:03.536Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 988
File: script/tasks/solidity/AddTokenApprovalsToCBridgeFacetPacked.s.sol:17-21
Timestamp: 2025-02-11T10:35:03.536Z
Learning: The CBridgeFacetPacked and cBridge addresses in AddTokenApprovalsToCBridgeFacetPacked.s.sol must not be zero addresses as they are required for token approvals to function properly.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : Facets must implement required functions: `_startBridge` (internal), `swapAndStartBridgeTokensVia{FacetName}`, and `startBridgeTokensVia{FacetName}`

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-10-10T10:56:04.861Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1413
File: src/Facets/EverclearFacet.sol:4-13
Timestamp: 2025-10-10T10:56:04.861Z
Learning: LibAllowList is only required for facets that make arbitrary external calls to DEX aggregators (e.g., GenericSwapFacetV3). Bridge facets that call specific protocol contracts (like EverclearFacet calling IEverclearFeeAdapter) do not need to import LibAllowList.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : In Solidity facets with `{facetName}Data.receiverAddress` field (e.g., `_glacisData.receiverAddress`), validate that it is not `bytes32(0)` for non-EVM chains and revert with `InvalidNonEVMReceiver()` if zero

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-02-21T09:00:28.226Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 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:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2024-10-22T03:16:28.754Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 807
File: test/solidity/Facets/GasZipFacet.t.sol:212-213
Timestamp: 2024-10-22T03:16:28.754Z
Learning: In the `GasZipFacetTest`, for the test case `testBase_Revert_SwapAndBridgeWithInvalidSwapData()`, a generic revert is expected, so `vm.expectRevert();` without specifying the expected error is appropriate.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-07-16T01:03:08.106Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 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:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:27:21.083Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/002-architecture.mdc:0-0
Timestamp: 2025-12-17T10:27:21.083Z
Learning: Applies to src/Facets/**/*Bridge*.sol : LiFiTransferStarted event must be emitted at the end of the internal _startBridge function in facets, after all validations and external bridge calls have completed successfully

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-02-21T09:00:28.226Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 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:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-01-22T12:36:12.699Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 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/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2024-10-22T03:24:24.705Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 807
File: test/solidity/Facets/GasZipFacet.t.sol:337-339
Timestamp: 2024-10-22T03:24:24.705Z
Learning: In `GasZipFacet` tests, when expecting a failure due to insufficient balance for sending value, the test should expect a generic revert without specifying a specific error.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:27:21.083Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/002-architecture.mdc:0-0
Timestamp: 2025-12-17T10:27:21.083Z
Learning: Applies to src/Periphery/Receiver*.sol : LiFiTransferRecovered event should only be emitted in Receiver contracts (src/Periphery/Receiver*.sol)

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-08-29T11:07:57.743Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: src/Periphery/LDA/Facets/CurveFacet.sol:87-91
Timestamp: 2025-08-29T11:07:57.743Z
Learning: In src/Periphery/LDA/Facets/CurveFacet.sol, modern Curve pools (isV2=true, representing V2/NG pools) should reject native tokenIn by adding an early revert check when LibAsset.isNativeAsset(tokenIn) is true, since ICurveV2 exchange functions are non-payable and cannot accept native ETH.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2024-10-10T03:26:23.793Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 782
File: test/solidity/Helpers/WithdrawablePeriphery.t.sol:79-80
Timestamp: 2024-10-10T03:26:23.793Z
Learning: In Solidity tests, it's acceptable to use `UnAuthorized.selector` in `vm.expectRevert()`; avoid suggesting changing it to string messages.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:29:29.239Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/400-solidity-tests.mdc:0-0
Timestamp: 2025-12-17T10:29:29.239Z
Learning: Applies to test/**/*.t.sol : If a facet does not support native tokens, override tests such as `testBase_CanSwapAndBridgeNativeTokens` and `testBase_CanBridgeNativeTokens` with `public override` and include a comment explaining why the test is intentionally skipped

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-01-28T11:29:09.566Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 945
File: test/solidity/Facets/GlacisFacet.t.sol:0-0
Timestamp: 2025-01-28T11:29:09.566Z
Learning: Empty test methods with explanatory comments are acceptable in test classes when the feature being tested (e.g., native token bridging) is not supported by the implementation.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-01-09T04:34:00.778Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 909
File: script/deploy/zksync/DeployTokenWrapper.s.sol:43-49
Timestamp: 2025-01-09T04:34:00.778Z
Learning: In deployment scripts for LiFi contracts, validation for refundWallet address is not required as it's handled elsewhere or through configuration validation.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-08-29T11:53:38.549Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: test/solidity/Periphery/LDA/BaseCoreRouteTest.t.sol:379-388
Timestamp: 2025-08-29T11:53:38.549Z
Learning: In test/solidity/Periphery/LDA/BaseCoreRouteTest.t.sol, for the revert-testing helper function _executeAndVerifySwap, only the aggregator branch (CommandType.DistributeSelfERC20) should use amountIn-1 to underfund and trigger insufficient balance errors, while user-funded branches should use the full amountIn to test other error conditions.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2024-12-02T06:33:33.309Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 827
File: src/Facets/DeBridgeDlnFacet.sol:0-0
Timestamp: 2024-12-02T06:33:33.309Z
Learning: In Solidity version 0.8.0 and above, arithmetic underflows and overflows automatically cause a revert; therefore, explicit checks for arithmetic underflows are not necessary in functions like `_startBridge` in `DeBridgeDlnFacet.sol`.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-09-16T07:56:45.093Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 1324
File: test/solidity/Facets/EcoFacet.t.sol:135-141
Timestamp: 2025-09-16T07:56:45.093Z
Learning: In EcoFacet test setup, the isNative parameter in swap-and-bridge flows refers to the bridge token type (output), not the swap input type. The ERC20 branch (isNative=false) can still swap from native ETH to ERC20 tokens, requiring msg.value = swapData[0].fromAmount for the ETH input to the swap.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : In Solidity facets, validate `targetChainId` against `bridgeData.destinationChain` for EVM-to-EVM bridges

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-08-27T22:29:00.940Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: test/solidity/Periphery/Lda/Facets/SyncSwapV2Facet.t.sol:312-323
Timestamp: 2025-08-27T22:29:00.940Z
Learning: In the LDA test suite, the `_buildRouteAndExecuteSwap` helper function already handles token approvals internally, so manual approval calls are not needed in individual test functions.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-08-27T22:28:47.277Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: test/solidity/Periphery/Lda/Facets/SyncSwapV2Facet.t.sol:278-304
Timestamp: 2025-08-27T22:28:47.277Z
Learning: The _buildRouteAndExecuteSwap helper function in LDA tests (like in test/solidity/Periphery/Lda/BaseCoreRouteTest.t.sol and similar) handles token approvals internally, so manual tokenIn.approve() calls are not needed in individual test functions when using this helper.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2024-10-31T09:09:38.568Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 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:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2024-10-09T03:47:21.269Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 819
File: deployments/bsc.json:48-48
Timestamp: 2024-10-09T03:47:21.269Z
Learning: The `LiFiDEXAggregator` contract does not include a `swapAndCompare` function.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-01-22T12:38:37.557Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 945
File: test/solidity/utils/TestBase.sol:446-470
Timestamp: 2025-01-22T12:38:37.557Z
Learning: Test utility functions in TestBase.sol don't require production-level validations like slippage checks and input validation since they operate in a controlled test environment.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-08-27T22:23:51.257Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: src/Periphery/Lda/Facets/NativeWrapperFacet.sol:0-0
Timestamp: 2025-08-27T22:23:51.257Z
Learning: In src/Periphery/Lda/Facets/NativeWrapperFacet.sol, the wrapNative function should not validate msg.value == amountIn because CoreRouteFacet's DistributeNative command (command type 3) overrides the amountIn parameter with address(this).balance, making such validation incorrect and causing false failures.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-15T09:02:31.300Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1530
File: test/solidity/Facets/CelerCircleBridgeFacet.t.sol:1-10
Timestamp: 2025-12-15T09:02:31.300Z
Learning: In Solidity test files under test/solidity/Facets (e.g., test/solidity/Facets/CelerCircleBridgeFacet.t.sol), ensure the SPDX license identifier line is immediately followed by the pragma statement with no blank line in between. Apply this formatting to all .sol files in that directory for consistency.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-01-28T14:30:06.911Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 924
File: deployments/abstract.json:2-4
Timestamp: 2025-01-28T14:30:06.911Z
Learning: In the LiFi contract architecture, "LiFiDiamond" is the diamond proxy contract that connects all facets into a cohesive system. The facets are individual contracts that provide specific functionality, and the diamond proxy delegates calls to these facets.

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-08-27T13:07:58.254Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: src/Periphery/Lda/Facets/CurveFacet.sol:0-0
Timestamp: 2025-08-27T13:07:58.254Z
Learning: The ICurveLegacy interface in src/Periphery/LiFiDEXAggregator.sol IS payable (can accept ETH), unlike ICurve and ICurveV2 which are non-payable. The current CurveFacet uses non-payable interfaces and cannot handle native assets.

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-12T11:16:09.236Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1530
File: src/Interfaces/ICircleBridgeProxy.sol:20-29
Timestamp: 2025-12-12T11:16:09.236Z
Learning: In src/Interfaces/ICircleBridgeProxy.sol, ensure the ICircleBridgeProxy interface corresponds to Celer's CircleBridgeProxy contract (not Circle's native TokenMessenger). The depositForBurn signature must be: depositForBurn(uint256 _amount, uint64 _dstChid, bytes32 _mintRecipient, address _burnToken, uint256 _maxFee, uint32 _minFinalityThreshold) external. This change should be scoped specifically to this file; pattern is not needed since it targets a single file. This improves correctness by aligning the interface with the correct contract and parameter types (dst chain id as uint64, no bytes32 destinationCaller parameter).

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
🪛 LanguageTool
docs/LiFiIntentEscrowFacet.md

[grammar] ~5-~5: Use a hyphen to join words.
Context: ... works LI.FI Intent Escrow uses a built in escrow as a deposit mechanism for its...

(QB_NEW_EN_HYPHEN)


[grammar] ~5-~5: Use a hyphen to join words.
Context: ...fill has been proven. The system is self serve, with the facet wrapping the depos...

(QB_NEW_EN_HYPHEN)

⏰ 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)
src/Facets/LiFiIntentEscrowFacet.sol (1)

37-66: Struct update aligns with the new destination‑swap flow.

Clear split between dstCallReceiver and recipient, and the docs in-line with the new callback encoding.

test/solidity/Facets/LiFiIntentEscrowFacet.t.sol (1)

674-759: Nice coverage for native→native with destination swaps.

The new test exercises both source wrap and destination unwrap paths with the ReceiverOIF callback encoding, which is exactly the scenario introduced by this PR.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@docs/LiFiIntentEscrowFacet.md`:
- Line 45: The doc comment for the parameter `nonce` contains a typo: change
"unqiue" to "unique" in the `@param` description for `nonce` in
LiFiIntentEscrowFacet documentation; locate the `@param nonce` line and correct
the spelling so it reads "unique orderIds" (ensure no other occurrences of
"unqiue" remain).

In `@src/Facets/LiFiIntentEscrowFacet.sol`:
- Around line 50-54: Fix the typos in the struct comment for
LiFiIntentEscrowData: add the missing space so the phrase reads "into
StandardOrder" (not "intoStandardOrder") and correct the misspelling "unqiue" to
"unique"; update the comments for the dstCallReceiver and recipient fields (and
any similar comment near the top of the struct around the other field at the
same location) to use the corrected wording.
📜 Review details

Configuration used: Repository 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.

📥 Commits

Reviewing files that changed from the base of the PR and between c3e8569 and 00b29c6.

📒 Files selected for processing (3)
  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
🧰 Additional context used
📓 Path-based instructions (10)
test/solidity/**/*.sol

📄 CodeRabbit inference engine (.cursor/rules/001-project-structure.mdc)

Place test files in test/solidity/ mirroring the src/ directory structure for organization

Files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
**/*.sol

📄 CodeRabbit inference engine (.cursor/rules/001-project-structure.mdc)

Use remappings defined in remappings.txt for imports: lifi/src/, test/test/, and external libs like @openzeppelin/, solmate/, solady/, permit2/

**/*.sol: Single Diamond (EIP-2535) as main entrypoint for all protocol interactions
Delegate complex logic to libraries (LibAsset, LibSwap, LibAllowList, SwapperV2, Validatable) and helper contracts

After Solidity changes, run forge test (or note suites remaining)

**/*.sol: Own files must use // SPDX-License-Identifier: LGPL-3.0-only immediately followed by the pragma statement with no blank line in between
All contracts must use pragma solidity ^0.8.17;
Functions and variables use camelCase; constants and immutables are CONSTANT_CASE
Function parameters use leading underscore (e.g., _amount)
Contracts and interfaces must include NatSpec: @title (matching contract/interface name), @author LI.FI (https://li.fi), @notice describing purpose, and @custom:version X.Y.Z
Public and external functions require NatSpec including params and returns documentation
For pure test/script scaffolding keep NatSpec headers minimal but retain SPDX and pragma
Use single blank lines between logical sections and between function declarations
Follow in-function blank-line rules: blank lines before emits/returns; no stray gaps
Use custom errors instead of revert strings; prefer existing errors/helpers before adding new ones
Use generic errors from src/Errors/GenericErrors.sol; bump @custom:version when adding; facet-specific errors stay local
Adhere to rules in .solhint.json
Avoid assembly unless necessary and heavily commented with justification (why assembly is needed); prefer existing helpers over new implementations

Do not mix interface and implementation in the same file

Files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
**/test/**/*.sol

📄 CodeRabbit inference engine (.cursor/rules/103-solidity-interfaces.mdc)

Test interfaces may define inline interfaces as needed for testing purposes

Files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
{src,script,test}/**/*.{sol,ts}

📄 CodeRabbit inference engine (.cursor/rules/105-security.mdc)

Validate all external inputs and configuration (including script/env values) explicitly; prefer existing validation helpers (e.g., Validatable, config readers) over ad-hoc checks

Files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
test/**/*.t.sol

📄 CodeRabbit inference engine (.cursor/rules/400-solidity-tests.mdc)

test/**/*.t.sol: Tests under test/solidity/, mirroring src/ structure; require setUp() function; call initTestBase() when inheriting TestBase directly or indirectly; label actors with vm.label
For facet tests, inherit from TestBaseFacet rather than TestBase, since standard facet functions need to be overridden
Import ordering: system libraries first (e.g., forge-std, ds-test), then project files (e.g., lifi/, test/)
Test function names: test_ prefix for success tests, testRevert_ prefix for failure tests, testBase_ prefix for base tests
Test structure: setup → execute → assert; use vm.startPrank / vm.stopPrank, labels, and base inits
Always assert specific revert reasons; use vm.expectRevert with specific reason
Use vm.expectEmit(true, true, true, true, <addr>) for event assertions
Apply blank line conventions: add gap after vm.expectRevert before the call, and gap before assertions/events
For whitelist flows, inherit TestWhitelistManagerBase and use addToWhitelist / setFunctionWhitelistBySelector helpers
If a facet does not support native tokens, override tests such as testBase_CanSwapAndBridgeNativeTokens and testBase_CanBridgeNativeTokens with public override and include a comment explaining why the test is intentionally skipped

Files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
**/*.{test.ts,t.sol}

📄 CodeRabbit inference engine (.cursor/rules/401-testing-patterns.mdc)

**/*.{test.ts,t.sol}: Prefer adding or updating tests alongside logic changes
Keep tests structured as setup → execute → assert
Assert specific failure conditions (avoid overly-broad "catch-all" assertions)

Files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
src/Facets/**/*.sol

📄 CodeRabbit inference engine (.cursor/rules/001-project-structure.mdc)

src/Facets/**/*.sol: Place new facets in the src/Facets/ directory and use plop templates if available for code generation
Keep facets thin and delegate complex logic to libraries. Group imports in order: external libs → interfaces → libraries → contracts

src/Facets/**/*.sol: Facets provide modular functionality grouped by concern (bridges, swaps, receivers, admin, etc.)
Facets should contain thin, integration-specific logic only; do not move logic across layers without clear architectural reason

Files:

  • src/Facets/LiFiIntentEscrowFacet.sol
src/Facets/**/*Facet.sol

📄 CodeRabbit inference engine (.cursor/rules/102-facets.mdc)

src/Facets/**/*Facet.sol: Facets must be located in src/Facets/ directory with names containing Facet
Facets must implement required functions: _startBridge (internal), swapAndStartBridgeTokensVia{FacetName}, and startBridgeTokensVia{FacetName}
Facets must use modifiers: nonReentrant, refundExcessNative, validateBridgeData, doesNotContainSourceSwaps/doesContainSourceSwaps, doesNotContainDestinationCalls/doesContainDestinationCalls on applicable functions
In Solidity facets, receiverAddress must be the first parameter in {facetName}Data and must match bridgeData.receiver for EVM chains
In Solidity facets, validate targetChainId against bridgeData.destinationChain for EVM-to-EVM bridges
Facets must use LibAsset, LibSwap, and LibAllowList utilities along with Validatable and SwapperV2 interfaces; reserve native fees via _depositAndSwap variants when needed
In Solidity facets with minAmountOut or similar bridge parameters, update the bridge's minAmountOut in swapAndStartBridgeTokensVia{FacetName} to account for positive slippage from swaps after _depositAndSwap updates _bridgeData.minAmount, adjusting proportionally for decimal differences if applicable
For non-EVM receivers in Solidity facets, use bytes type; the value must be non-zero
For non-EVM flows in Solidity facets, bridgeData.receiver must equal NON_EVM_ADDRESS
In Solidity facets with {facetName}Data.receiverAddress field (e.g., _glacisData.receiverAddress), validate that it is not bytes32(0) for non-EVM chains and revert with InvalidNonEVMReceiver() if zero

Files:

  • src/Facets/LiFiIntentEscrowFacet.sol
{src,script}/**/*.sol

📄 CodeRabbit inference engine (.cursor/rules/105-security.mdc)

{src,script}/**/*.sol: Do not weaken existing access controls, timelock flows, or Safe multisig protections; any change that touches admin-only functionality must call out its governance impact
Avoid introducing new external call patterns in facets or scripts without checking existing libraries (LibAsset, LibSwap, LibAllowList) for prior art

Files:

  • src/Facets/LiFiIntentEscrowFacet.sol
src/**/*.sol

📄 CodeRabbit inference engine (.cursor/rules/106-gas.mdc)

src/**/*.sol: Follow gas-related best practices in existing facets before introducing new micro-optimizations in Solidity
Prefer using existing optimized libraries already in the repo (e.g., Solady/Solmate utilities) where they are in use, rather than hand-rolled low-level code
Do not sacrifice readability or safety for minor gas wins; explain any non-obvious optimization and justify it with expected impact
Avoid introducing inline assembly unless clearly needed; when used, document assumptions and invariants thoroughly

All contracts in src/ require audits except Interfaces in src/Interfaces/** (type definitions only) and external dependencies in lib/

src/**/*.sol: NatSpec required on contracts/interfaces in src/: @title, @author LI.FI (https://li.fi), @notice, @custom:version X.Y.Z; document all public/external functions with params/returns.
Bug bounty clarity: In contract NatSpec, explicitly document whether the contract is intended to hold/custody funds. If not designed to hold funds, state clearly (e.g., 'This contract is not intended to custody user funds / hold balances; any funds held are incidental'). If designed to hold funds, describe what funds it holds and under what conditions.
Apply Diamond patterns with existing libs (LibAsset/LibSwap/LibAllowList, Validatable/SwapperV2); prefer parameters over msg.sender for refund addresses.
In Solidity 0.8.17, you must not emit events using ContractName.EventName syntax (e.g., emit SomeContract.SomeEvent(...)). Events must be defined in the same contract where they're emitted, or defined in an interface that the contract uses, and then emitted using just the event name (e.g., emit SomeEvent(...)). This prevents compilation errors and ensures 0.8.17 compatibility.

Files:

  • src/Facets/LiFiIntentEscrowFacet.sol
🧠 Learnings (64)
📓 Common learnings
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must implement internal `_swapAndCompleteBridgeTokens()` function that handles native vs ERC20 tokens: use `safeApproveWithRetry()` or `safeIncreaseAllowance()` for ERC20 (reset to 0 after), pass `value: amount` for native tokens, call `executor.swapAndCompleteBridgeTokens()` in try-catch block, and on failure send raw tokens to receiver and emit `LiFiTransferRecovered`
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must only emit `LiFiTransferRecovered` when swaps fail and raw bridged tokens are sent to receiver, with event signature: `LiFiTransferRecovered(bytes32 indexed transactionId, address receivingAssetId, address receiver, uint256 amount, uint256 timestamp)`
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must implement a bridge-specific external entry point (e.g., `handleV3AcrossMessage`, `lzCompose`, `cfReceive`) protected by authorization modifier that decodes bridge message to extract `bytes32 transactionId`, `LibSwap.SwapData[] memory swapData`, `address receiver`, and calls internal `_swapAndCompleteBridgeTokens()`
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : For non-EVM flows in Solidity facets, `bridgeData.receiver` must equal `NON_EVM_ADDRESS`
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must never emit `LiFiTransferStarted` (reserved for facets)
Learnt from: mirooon
Repo: lifinance/contracts PR: 1413
File: src/Facets/EverclearV2Facet.sol:75-96
Timestamp: 2025-10-13T11:13:48.847Z
Learning: In EverclearV2Facet (and all LiFi facets), the team standard is to use msg.sender as the refund address with the refundExcessNative modifier, not requiring an explicit refund address parameter.
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : In Solidity facets, `receiverAddress` must be the first parameter in `{facetName}Data` and must match `bridgeData.receiver` for EVM chains
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/002-architecture.mdc:0-0
Timestamp: 2025-12-17T10:27:21.083Z
Learning: Applies to src/Periphery/Receiver*.sol : LiFiTransferRecovered event should only be emitted in Receiver contracts (src/Periphery/Receiver*.sol)
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/002-architecture.mdc:0-0
Timestamp: 2025-12-17T10:27:21.083Z
Learning: Applies to src/Facets/**/*Bridge*.sol : For non-EVM chains (bridgeData.receiver == NON_EVM_ADDRESS), emit BridgeToNonEVMChainBytes32 with transactionId, destinationChainId, and non-EVM receiver (bytes32)
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1275
File: src/Facets/AllBridgeFacet.sol:186-188
Timestamp: 2025-07-16T01:04:55.857Z
Learning: In LiFi facet contracts, when public entry point functions have `nonReentrant` modifier protection, internal functions like `_startBridge` that they call benefit from this reentrancy protection, making event emission order after external calls acceptable from a security perspective.
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must implement internal `_swapAndCompleteBridgeTokens()` function that handles native vs ERC20 tokens: use `safeApproveWithRetry()` or `safeIncreaseAllowance()` for ERC20 (reset to 0 after), pass `value: amount` for native tokens, call `executor.swapAndCompleteBridgeTokens()` in try-catch block, and on failure send raw tokens to receiver and emit `LiFiTransferRecovered`

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : Facets must use LibAsset, LibSwap, and LibAllowList utilities along with Validatable and SwapperV2 interfaces; reserve native fees via `_depositAndSwap` variants when needed

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-08-27T13:07:58.254Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: src/Periphery/Lda/Facets/CurveFacet.sol:0-0
Timestamp: 2025-08-27T13:07:58.254Z
Learning: In the LiFi contracts codebase, ICurveLegacy interface is defined inline in src/Periphery/LiFiDEXAggregator.sol with a non-payable exchange function. The CurveFacet diff shows importing it from a separate file that doesn't exist, creating a compilation issue.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : In Solidity facets with `minAmountOut` or similar bridge parameters, update the bridge's minAmountOut in `swapAndStartBridgeTokensVia{FacetName}` to account for positive slippage from swaps after `_depositAndSwap` updates `_bridgeData.minAmount`, adjusting proportionally for decimal differences if applicable

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must only emit `LiFiTransferRecovered` when swaps fail and raw bridged tokens are sent to receiver, with event signature: `LiFiTransferRecovered(bytes32 indexed transactionId, address receivingAssetId, address receiver, uint256 amount, uint256 timestamp)`

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-10-02T18:10:09.934Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1406
File: src/Facets/UnitFacet.sol:75-81
Timestamp: 2025-10-02T18:10:09.934Z
Learning: In UnitFacet.sol and similar facet contracts in src/Facets/, the LiFiTransferStarted event emission should ALWAYS be at the end of the _startBridge internal function, after all validations and asset transfers have been completed successfully.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2024-12-04T02:05:41.355Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 832
File: src/Periphery/LiFiDEXAggregator.sol:578-697
Timestamp: 2024-12-04T02:05:41.355Z
Learning: Unit tests are not required for newly added swap callback functions in `LiFiDEXAggregator.sol`.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-08-28T02:41:07.505Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 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/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-10-13T11:13:48.847Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1413
File: src/Facets/EverclearV2Facet.sol:75-96
Timestamp: 2025-10-13T11:13:48.847Z
Learning: In EverclearV2Facet (and all LiFi facets), the team standard is to use msg.sender as the refund address with the refundExcessNative modifier, not requiring an explicit refund address parameter.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must never emit `LiFiTransferStarted` (reserved for facets)

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • docs/LiFiIntentEscrowFacet.md
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must use `LibAsset` for asset operations, `LibSwap` for swap data structures, and `SafeTransferLib` from solady for safe transfers

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • docs/LiFiIntentEscrowFacet.md
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must never emit `LiFiTransferCompleted` (reserved for Executor contract)

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must use try-catch for executor calls, always have fallback to send raw tokens on failure, and reset approvals to 0 after operations

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must include immutable storage for `IExecutor public immutable executor` and bridge-specific authorized caller (e.g., `spokepool`, `chainflipVault`, `endpointV2`)

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must inherit from `ILiFi` and `WithdrawablePeriphery`

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : For native tokens with gas checks (e.g., StargateV2), Receiver contracts must check `gasleft()` before executing; if insufficient gas, send tokens directly to receiver and emit `LiFiTransferRecovered`

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-10-10T12:50:47.440Z
Learnt from: reednaa
Repo: lifinance/contracts PR: 1361
File: script/demoScripts/demoLiFiIntentEscrow.ts:269-269
Timestamp: 2025-10-10T12:50:47.440Z
Learning: In `src/Interfaces/IOpenIntentFramework.sol`, the `StandardOrder` struct's `inputOracle` field is of type `address` (20 bytes), not `bytes32`. It should not be padded to 32 bytes when constructing `LiFiIntentEscrowData` in TypeScript/JavaScript code.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-05-06T09:09:38.108Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1117
File: test/solidity/Periphery/LiFiDEXAggregator.t.sol:111-127
Timestamp: 2025-05-06T09:09:38.108Z
Learning: The `setupApechain()` function in LiFiDEXAggregator tests intentionally avoids calling `initTestBase()` to prevent needing to define standard contracts (like ADDRESS_USDC_PROXY and Uniswap) that aren't needed for Apechain-specific tests.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-07-11T09:43:22.393Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1193
File: test/solidity/Facets/AllowListMigratorFacet.t.sol:39-52
Timestamp: 2025-07-11T09:43:22.393Z
Learning: For fork-based tests like AllowListMigratorFacet.t.sol that work with existing deployed contracts (mainnet diamond), initTestBase() is intentionally omitted since the standard test initialization is not needed.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-07-16T01:04:55.857Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1275
File: src/Facets/AllBridgeFacet.sol:186-188
Timestamp: 2025-07-16T01:04:55.857Z
Learning: In LiFi facet contracts, when public entry point functions have `nonReentrant` modifier protection, internal functions like `_startBridge` that they call benefit from this reentrancy protection, making event emission order after external calls acceptable from a security perspective.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-02-24T09:35:34.908Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 984
File: src/Periphery/ReceiverChainflip.sol:48-62
Timestamp: 2025-02-24T09:35:34.908Z
Learning: Contract address validations for periphery contracts like ReceiverChainflip are handled in deployment scripts rather than in the constructor.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must implement a bridge-specific external entry point (e.g., `handleV3AcrossMessage`, `lzCompose`, `cfReceive`) protected by authorization modifier that decodes bridge message to extract `bytes32 transactionId`, `LibSwap.SwapData[] memory swapData`, `address receiver`, and calls internal `_swapAndCompleteBridgeTokens()`

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-11-25T02:57:58.015Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1466
File: config/whitelist.json:2546-2554
Timestamp: 2025-11-25T02:57:58.015Z
Learning: In lifinance/contracts config/whitelist.json, the SushiSwap Aggregator uses functions:
- snwap(address,uint256,address,address,uint256,address,bytes) -> selector 0x5f3bd1c8
- snwapMultiple((address,uint256,address)[],(address,address,uint256)[],(address,uint256,bytes)[]) -> selector 0xd33721a5
These selectors are canonical across networks and valid for the monad entry as well. Always compute with Keccak-256 (not SHA3-256) when verifying.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-08-29T11:12:02.184Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: src/Periphery/LDA/Facets/SyncSwapV2Facet.sol:33-38
Timestamp: 2025-08-29T11:12:02.184Z
Learning: In LDA facets (src/Periphery/Lda/Facets/**/*.sol), tokenIn parameter validation (checking for address(0)) is intentionally not performed as part of the design architecture.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-08-27T23:36:40.773Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 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:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-02-11T10:35:03.536Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 988
File: script/tasks/solidity/AddTokenApprovalsToCBridgeFacetPacked.s.sol:17-21
Timestamp: 2025-02-11T10:35:03.536Z
Learning: The CBridgeFacetPacked and cBridge addresses in AddTokenApprovalsToCBridgeFacetPacked.s.sol must not be zero addresses as they are required for token approvals to function properly.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-06-13T08:30:26.220Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1207
File: deployments/_deployments_log_file.json:34037-34080
Timestamp: 2025-06-13T08:30:26.220Z
Learning: LiFiDiamond contains generic withdrawal logic, so individual facet contracts (e.g., PioneerFacet) do not need their own Ether-withdraw functions.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : Facets must implement required functions: `_startBridge` (internal), `swapAndStartBridgeTokensVia{FacetName}`, and `startBridgeTokensVia{FacetName}`

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-10-10T10:56:04.861Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1413
File: src/Facets/EverclearFacet.sol:4-13
Timestamp: 2025-10-10T10:56:04.861Z
Learning: LibAllowList is only required for facets that make arbitrary external calls to DEX aggregators (e.g., GenericSwapFacetV3). Bridge facets that call specific protocol contracts (like EverclearFacet calling IEverclearFeeAdapter) do not need to import LibAllowList.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : In Solidity facets with `{facetName}Data.receiverAddress` field (e.g., `_glacisData.receiverAddress`), validate that it is not `bytes32(0)` for non-EVM chains and revert with `InvalidNonEVMReceiver()` if zero

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:27:21.083Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/002-architecture.mdc:0-0
Timestamp: 2025-12-17T10:27:21.083Z
Learning: Applies to src/Facets/**/*Bridge*.sol : For non-EVM chains (bridgeData.receiver == NON_EVM_ADDRESS), emit BridgeToNonEVMChainBytes32 with transactionId, destinationChainId, and non-EVM receiver (bytes32)

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-02-21T09:00:28.226Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 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:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : For non-EVM flows in Solidity facets, `bridgeData.receiver` must equal `NON_EVM_ADDRESS`

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2024-10-22T03:16:28.754Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 807
File: test/solidity/Facets/GasZipFacet.t.sol:212-213
Timestamp: 2024-10-22T03:16:28.754Z
Learning: In the `GasZipFacetTest`, for the test case `testBase_Revert_SwapAndBridgeWithInvalidSwapData()`, a generic revert is expected, so `vm.expectRevert();` without specifying the expected error is appropriate.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-07-16T01:03:08.106Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 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:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:27:21.083Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/002-architecture.mdc:0-0
Timestamp: 2025-12-17T10:27:21.083Z
Learning: Applies to src/Facets/**/*Bridge*.sol : LiFiTransferStarted event must be emitted at the end of the internal _startBridge function in facets, after all validations and external bridge calls have completed successfully

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-02-21T09:00:28.226Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 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:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-01-22T12:36:12.699Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 945
File: test/solidity/Facets/GlacisFacet.t.sol:214-262
Timestamp: 2025-01-22T12:36:12.699Z
Learning: In the LiFi contracts, token pair validation is handled by third-party contracts, and route discovery including liquidity checks are performed off-chain. The contract only receives pre-validated routes, making token pair and liquidity validation tests unnecessary at the contract level.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • docs/LiFiIntentEscrowFacet.md
📚 Learning: 2025-01-22T12:36:12.699Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 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/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2024-10-22T03:24:24.705Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 807
File: test/solidity/Facets/GasZipFacet.t.sol:337-339
Timestamp: 2024-10-22T03:24:24.705Z
Learning: In `GasZipFacet` tests, when expecting a failure due to insufficient balance for sending value, the test should expect a generic revert without specifying a specific error.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : In Solidity facets, `receiverAddress` must be the first parameter in `{facetName}Data` and must match `bridgeData.receiver` for EVM chains

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:27:21.083Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/002-architecture.mdc:0-0
Timestamp: 2025-12-17T10:27:21.083Z
Learning: Applies to src/Periphery/Receiver*.sol : LiFiTransferRecovered event should only be emitted in Receiver contracts (src/Periphery/Receiver*.sol)

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-08-29T11:07:57.743Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: src/Periphery/LDA/Facets/CurveFacet.sol:87-91
Timestamp: 2025-08-29T11:07:57.743Z
Learning: In src/Periphery/LDA/Facets/CurveFacet.sol, modern Curve pools (isV2=true, representing V2/NG pools) should reject native tokenIn by adding an early revert check when LibAsset.isNativeAsset(tokenIn) is true, since ICurveV2 exchange functions are non-payable and cannot accept native ETH.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2024-10-10T03:26:23.793Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 782
File: test/solidity/Helpers/WithdrawablePeriphery.t.sol:79-80
Timestamp: 2024-10-10T03:26:23.793Z
Learning: In Solidity tests, it's acceptable to use `UnAuthorized.selector` in `vm.expectRevert()`; avoid suggesting changing it to string messages.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:29:29.239Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/400-solidity-tests.mdc:0-0
Timestamp: 2025-12-17T10:29:29.239Z
Learning: Applies to test/**/*.t.sol : If a facet does not support native tokens, override tests such as `testBase_CanSwapAndBridgeNativeTokens` and `testBase_CanBridgeNativeTokens` with `public override` and include a comment explaining why the test is intentionally skipped

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-01-28T11:29:09.566Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 945
File: test/solidity/Facets/GlacisFacet.t.sol:0-0
Timestamp: 2025-01-28T11:29:09.566Z
Learning: Empty test methods with explanatory comments are acceptable in test classes when the feature being tested (e.g., native token bridging) is not supported by the implementation.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-01-09T04:34:00.778Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 909
File: script/deploy/zksync/DeployTokenWrapper.s.sol:43-49
Timestamp: 2025-01-09T04:34:00.778Z
Learning: In deployment scripts for LiFi contracts, validation for refundWallet address is not required as it's handled elsewhere or through configuration validation.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-08-29T11:53:38.549Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: test/solidity/Periphery/LDA/BaseCoreRouteTest.t.sol:379-388
Timestamp: 2025-08-29T11:53:38.549Z
Learning: In test/solidity/Periphery/LDA/BaseCoreRouteTest.t.sol, for the revert-testing helper function _executeAndVerifySwap, only the aggregator branch (CommandType.DistributeSelfERC20) should use amountIn-1 to underfund and trigger insufficient balance errors, while user-funded branches should use the full amountIn to test other error conditions.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2024-12-02T06:33:33.309Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 827
File: src/Facets/DeBridgeDlnFacet.sol:0-0
Timestamp: 2024-12-02T06:33:33.309Z
Learning: In Solidity version 0.8.0 and above, arithmetic underflows and overflows automatically cause a revert; therefore, explicit checks for arithmetic underflows are not necessary in functions like `_startBridge` in `DeBridgeDlnFacet.sol`.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-09-16T07:56:45.093Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 1324
File: test/solidity/Facets/EcoFacet.t.sol:135-141
Timestamp: 2025-09-16T07:56:45.093Z
Learning: In EcoFacet test setup, the isNative parameter in swap-and-bridge flows refers to the bridge token type (output), not the swap input type. The ERC20 branch (isNative=false) can still swap from native ETH to ERC20 tokens, requiring msg.value = swapData[0].fromAmount for the ETH input to the swap.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : In Solidity facets, validate `targetChainId` against `bridgeData.destinationChain` for EVM-to-EVM bridges

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2024-10-31T09:09:38.568Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 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:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2024-10-09T03:47:21.269Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 819
File: deployments/bsc.json:48-48
Timestamp: 2024-10-09T03:47:21.269Z
Learning: The `LiFiDEXAggregator` contract does not include a `swapAndCompare` function.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-01-22T12:38:37.557Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 945
File: test/solidity/utils/TestBase.sol:446-470
Timestamp: 2025-01-22T12:38:37.557Z
Learning: Test utility functions in TestBase.sol don't require production-level validations like slippage checks and input validation since they operate in a controlled test environment.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-08-27T22:28:47.277Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: test/solidity/Periphery/Lda/Facets/SyncSwapV2Facet.t.sol:278-304
Timestamp: 2025-08-27T22:28:47.277Z
Learning: The _buildRouteAndExecuteSwap helper function in LDA tests (like in test/solidity/Periphery/Lda/BaseCoreRouteTest.t.sol and similar) handles token approvals internally, so manual tokenIn.approve() calls are not needed in individual test functions when using this helper.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-08-27T22:29:00.940Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: test/solidity/Periphery/Lda/Facets/SyncSwapV2Facet.t.sol:312-323
Timestamp: 2025-08-27T22:29:00.940Z
Learning: In the LDA test suite, the `_buildRouteAndExecuteSwap` helper function already handles token approvals internally, so manual approval calls are not needed in individual test functions.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-08-27T22:23:51.257Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: src/Periphery/Lda/Facets/NativeWrapperFacet.sol:0-0
Timestamp: 2025-08-27T22:23:51.257Z
Learning: In src/Periphery/Lda/Facets/NativeWrapperFacet.sol, the wrapNative function should not validate msg.value == amountIn because CoreRouteFacet's DistributeNative command (command type 3) overrides the amountIn parameter with address(this).balance, making such validation incorrect and causing false failures.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-12-15T09:02:31.300Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1530
File: test/solidity/Facets/CelerCircleBridgeFacet.t.sol:1-10
Timestamp: 2025-12-15T09:02:31.300Z
Learning: In Solidity test files under test/solidity/Facets (e.g., test/solidity/Facets/CelerCircleBridgeFacet.t.sol), ensure the SPDX license identifier line is immediately followed by the pragma statement with no blank line in between. Apply this formatting to all .sol files in that directory for consistency.

Applied to files:

  • test/solidity/Facets/LiFiIntentEscrowFacet.t.sol
📚 Learning: 2025-02-13T08:57:00.095Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 984
File: docs/ChainflipFacet.md:0-0
Timestamp: 2025-02-13T08:57:00.095Z
Learning: The ChainflipData struct in ChainflipFacet has three fields:
1. uint32 dstToken - The destination token identifier in Chainflip
2. bytes32 nonEvmAddress - The non-EVM destination address
3. bytes cfParameters - Additional parameters for Chainflip protocol

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-04-28T07:46:24.084Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 1116
File: src/Libraries/LibAsset.sol:102-108
Timestamp: 2025-04-28T07:46:24.084Z
Learning: In the LiFi contracts, the `LibAsset.transferFromERC20` function is intentionally designed to accept arbitrary `from` parameters, with security ensured through a whitelist mechanism that controls which contracts can call this function.

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-01-28T14:30:06.911Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 924
File: deployments/abstract.json:2-4
Timestamp: 2025-01-28T14:30:06.911Z
Learning: In the LiFi contract architecture, "LiFiDiamond" is the diamond proxy contract that connects all facets into a cohesive system. The facets are individual contracts that provide specific functionality, and the diamond proxy delegates calls to these facets.

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-08-27T13:07:58.254Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: src/Periphery/Lda/Facets/CurveFacet.sol:0-0
Timestamp: 2025-08-27T13:07:58.254Z
Learning: The ICurveLegacy interface in src/Periphery/LiFiDEXAggregator.sol IS payable (can accept ETH), unlike ICurve and ICurveV2 which are non-payable. The current CurveFacet uses non-payable interfaces and cannot handle native assets.

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-12T11:16:09.236Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1530
File: src/Interfaces/ICircleBridgeProxy.sol:20-29
Timestamp: 2025-12-12T11:16:09.236Z
Learning: In src/Interfaces/ICircleBridgeProxy.sol, ensure the ICircleBridgeProxy interface corresponds to Celer's CircleBridgeProxy contract (not Circle's native TokenMessenger). The depositForBurn signature must be: depositForBurn(uint256 _amount, uint64 _dstChid, bytes32 _mintRecipient, address _burnToken, uint256 _maxFee, uint32 _minFinalityThreshold) external. This change should be scoped specifically to this file; pattern is not needed since it targets a single file. This improves correctness by aligning the interface with the correct contract and parameter types (dst chain id as uint64, no bytes32 destinationCaller parameter).

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
🔇 Additional comments (9)
src/Facets/LiFiIntentEscrowFacet.sol (3)

152-170: Recipient validation and non-EVM handling look correct.

The logic properly:

  1. Validates recipient != bytes32(0) first (line 154)
  2. Handles non-EVM case by emitting BridgeToNonEVMChain with the encoded recipient (lines 155-161)
  3. For EVM, validates that recipient matches bridgeData.receiver converted to bytes32 (line 165)
  4. Validates depositAndRefundAddress is non-zero (lines 169-170)

The comment on line 164 ("Note: We already know 0 <= _bridgeData.receiver < recipient != 0 thus _bridgeData.receiver != 0") is mathematically awkward but the intent is clear: since recipient != 0 was verified and the check is recipient != bytes32(receiver), if they're equal, receiver is also non-zero.


183-195: Destination call encoding and recipient override logic is sound.

When dstCallSwapData.length != 0:

  1. outputCall is ABI-encoded with transactionId, dstCallSwapData, and the original recipient (preserving the final destination for ReceiverOIF)
  2. recipient is then overridden to dstCallReceiver for the MandateOutput
  3. dstCallReceiver is validated to be non-zero (line 194)

This correctly routes the bridged funds to the ReceiverOIF contract which will decode and execute the swaps, forwarding to the original recipient.


197-227: MandateOutput and StandardOrder construction is correct.

The output correctly uses:

  • recipient (which is either the original recipient or dstCallReceiver depending on swap data presence)
  • callbackData: outputCall (empty or encoded swap data)

The inputs array construction at lines 209-211 properly formats the sending asset and amount for the StandardOrder.

Event emission follows the pattern of emitting LiFiTransferStarted at the end of _startBridge after all validations and external calls, consistent with learnings.

test/solidity/Facets/LiFiIntentEscrowFacet.t.sol (5)

84-105: Clean helper function for test data construction.

The _validLIFIIntentData() helper centralizes test data creation with the new struct shape, making tests more maintainable. The use of view modifier is appropriate since it accesses state variables.


112-125: Good approach deploying periphery components in-test.

Deploying periphery contracts locally rather than accessing mainnet deployments reduces RPC calls and speeds up tests, as the comment notes. The wiring of ERC20ProxyExecutorReceiverOIF follows the expected pattern from learnings.


460-479: Good coverage for the dstCallReceiver zero-address edge case.

This test correctly validates that when dstCallSwapData is provided, dstCallReceiver must be non-zero. This directly tests the validation at lines 192-194 in the facet.


695-780: Comprehensive end-to-end test for native-to-native with destination swaps.

This test exercises the full flow:

  1. Source swap: Native ETH → WETH via TokenWrapper
  2. Intent creation with destination swap data (WETH → ETH on destination)
  3. Fill execution via OutputSettler
  4. Verification that final ETH reaches the receiver

The MandateOutput construction at lines 748-761 correctly mirrors the facet's encoding logic. The assertion at line 779 validates the complete flow.

One consideration: The test sets chainId: block.chainid (line 751) for the output, which works for this local test but differs from typical cross-chain scenarios where bridgeData.destinationChainId would be used. This is acceptable for testing the encoding/decoding logic.


413-419: Correctly overridden base tests for unsupported native bridging.

These empty overrides follow the pattern from coding guidelines: "If a facet does not support native tokens, override tests such as testBase_CanSwapAndBridgeNativeTokens and testBase_CanBridgeNativeTokens with public override and include a comment explaining why the test is intentionally skipped."

The comments explain the reason. As per coding guidelines, this is the correct approach.

docs/LiFiIntentEscrowFacet.md (1)

53-53: Minor typo in documentation.

There's a missing space: "Goes intoStandardOrder" should be "Goes into StandardOrder".

-    // Goes intoStandardOrder.outputs.recipient if .dstCallSwapData.length == 0
+    // Goes into StandardOrder.outputs.recipient if .dstCallSwapData.length == 0
⛔ Skipped due to learnings
Learnt from: reednaa
Repo: lifinance/contracts PR: 1361
File: script/demoScripts/demoLiFiIntentEscrow.ts:269-269
Timestamp: 2025-10-10T12:50:47.440Z
Learning: In `src/Interfaces/IOpenIntentFramework.sol`, the `StandardOrder` struct's `inputOracle` field is of type `address` (20 bytes), not `bytes32`. It should not be padded to 32 bytes when constructing `LiFiIntentEscrowData` in TypeScript/JavaScript code.
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/002-architecture.mdc:0-0
Timestamp: 2025-12-17T10:27:21.083Z
Learning: Applies to **/*Swap*.sol : Emit GenericSwapCompleted event for same-chain swaps

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@docs/LiFiIntentEscrowFacet.md`:
- Line 53: Update the doc comment for the dstCallSwapData parameter to replace
the outdated reference to receiverAddress with the current field name
dstCallReceiver: edit the line containing "@param dstCallSwapData" so it reads
that swaps are executed on dstCallReceiver (not receiverAddress) and that if
empty no call is made; ensure the comment now references dstCallReceiver
consistently with the struct used by LiFiIntentEscrowFacet.
♻️ Duplicate comments (1)
src/Facets/LiFiIntentEscrowFacet.sol (1)

50-67: Struct field ordering and documentation look good.

The new LiFiIntentEscrowData struct correctly places dstCallReceiver and recipient at the top with clear comments explaining their usage based on dstCallSwapData.length. The design cleanly separates the two recipient scenarios.

Note: Per past review comments, there was a typo "intoStandardOrder" (missing space) in line 53 comment that should be "into StandardOrder".

🧹 Nitpick comments (2)
src/Facets/LiFiIntentEscrowFacet.sol (1)

152-170: Recipient validation logic is sound, but clarify the inline comment.

The validation correctly handles both EVM and non-EVM cases. However, the comment on line 164 is confusing:

// Note: We already know 0 <= _bridgeData.receiver < recipient != 0 thus _bridgeData.receiver != 0.

The mathematical notation is unclear. Consider rephrasing to something like:

// Note: We've verified recipient != 0 above. If this branch executes, 
// _bridgeData.receiver != NON_EVM_ADDRESS, so comparing them is valid.
docs/LiFiIntentEscrowFacet.md (1)

55-72: Inconsistent indentation in struct definition.

The struct comments have mixed indentation (2 spaces vs 4 spaces). Consider standardizing for consistency.

✍️ Suggested fix
 struct LiFiIntentEscrowData {
-  // Goes into StandardOrder.outputs.recipient if .dstCallSwapData.length > 0
-  bytes32 dstCallReceiver;
-    // Goes into StandardOrder.outputs.recipient if .dstCallSwapData.length == 0
-  bytes32 recipient;
+    // Goes into StandardOrder.outputs.recipient if .dstCallSwapData.length > 0
+    bytes32 dstCallReceiver;
+    // Goes into StandardOrder.outputs.recipient if .dstCallSwapData.length == 0
+    bytes32 recipient;
📜 Review details

Configuration used: Repository 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 00b29c6 and 60c7614.

📒 Files selected for processing (2)
  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
🧰 Additional context used
📓 Path-based instructions (6)
src/Facets/**/*.sol

📄 CodeRabbit inference engine (.cursor/rules/001-project-structure.mdc)

src/Facets/**/*.sol: Place new facets in the src/Facets/ directory and use plop templates if available for code generation
Keep facets thin and delegate complex logic to libraries. Group imports in order: external libs → interfaces → libraries → contracts

src/Facets/**/*.sol: Facets provide modular functionality grouped by concern (bridges, swaps, receivers, admin, etc.)
Facets should contain thin, integration-specific logic only; do not move logic across layers without clear architectural reason

Files:

  • src/Facets/LiFiIntentEscrowFacet.sol
**/*.sol

📄 CodeRabbit inference engine (.cursor/rules/001-project-structure.mdc)

Use remappings defined in remappings.txt for imports: lifi/src/, test/test/, and external libs like @openzeppelin/, solmate/, solady/, permit2/

**/*.sol: Single Diamond (EIP-2535) as main entrypoint for all protocol interactions
Delegate complex logic to libraries (LibAsset, LibSwap, LibAllowList, SwapperV2, Validatable) and helper contracts

After Solidity changes, run forge test (or note suites remaining)

**/*.sol: Own files must use // SPDX-License-Identifier: LGPL-3.0-only immediately followed by the pragma statement with no blank line in between
All contracts must use pragma solidity ^0.8.17;
Functions and variables use camelCase; constants and immutables are CONSTANT_CASE
Function parameters use leading underscore (e.g., _amount)
Contracts and interfaces must include NatSpec: @title (matching contract/interface name), @author LI.FI (https://li.fi), @notice describing purpose, and @custom:version X.Y.Z
Public and external functions require NatSpec including params and returns documentation
For pure test/script scaffolding keep NatSpec headers minimal but retain SPDX and pragma
Use single blank lines between logical sections and between function declarations
Follow in-function blank-line rules: blank lines before emits/returns; no stray gaps
Use custom errors instead of revert strings; prefer existing errors/helpers before adding new ones
Use generic errors from src/Errors/GenericErrors.sol; bump @custom:version when adding; facet-specific errors stay local
Adhere to rules in .solhint.json
Avoid assembly unless necessary and heavily commented with justification (why assembly is needed); prefer existing helpers over new implementations

Do not mix interface and implementation in the same file

Files:

  • src/Facets/LiFiIntentEscrowFacet.sol
src/Facets/**/*Facet.sol

📄 CodeRabbit inference engine (.cursor/rules/102-facets.mdc)

src/Facets/**/*Facet.sol: Facets must be located in src/Facets/ directory with names containing Facet
Facets must implement required functions: _startBridge (internal), swapAndStartBridgeTokensVia{FacetName}, and startBridgeTokensVia{FacetName}
Facets must use modifiers: nonReentrant, refundExcessNative, validateBridgeData, doesNotContainSourceSwaps/doesContainSourceSwaps, doesNotContainDestinationCalls/doesContainDestinationCalls on applicable functions
In Solidity facets, receiverAddress must be the first parameter in {facetName}Data and must match bridgeData.receiver for EVM chains
In Solidity facets, validate targetChainId against bridgeData.destinationChain for EVM-to-EVM bridges
Facets must use LibAsset, LibSwap, and LibAllowList utilities along with Validatable and SwapperV2 interfaces; reserve native fees via _depositAndSwap variants when needed
In Solidity facets with minAmountOut or similar bridge parameters, update the bridge's minAmountOut in swapAndStartBridgeTokensVia{FacetName} to account for positive slippage from swaps after _depositAndSwap updates _bridgeData.minAmount, adjusting proportionally for decimal differences if applicable
For non-EVM receivers in Solidity facets, use bytes type; the value must be non-zero
For non-EVM flows in Solidity facets, bridgeData.receiver must equal NON_EVM_ADDRESS
In Solidity facets with {facetName}Data.receiverAddress field (e.g., _glacisData.receiverAddress), validate that it is not bytes32(0) for non-EVM chains and revert with InvalidNonEVMReceiver() if zero

Files:

  • src/Facets/LiFiIntentEscrowFacet.sol
{src,script,test}/**/*.{sol,ts}

📄 CodeRabbit inference engine (.cursor/rules/105-security.mdc)

Validate all external inputs and configuration (including script/env values) explicitly; prefer existing validation helpers (e.g., Validatable, config readers) over ad-hoc checks

Files:

  • src/Facets/LiFiIntentEscrowFacet.sol
{src,script}/**/*.sol

📄 CodeRabbit inference engine (.cursor/rules/105-security.mdc)

{src,script}/**/*.sol: Do not weaken existing access controls, timelock flows, or Safe multisig protections; any change that touches admin-only functionality must call out its governance impact
Avoid introducing new external call patterns in facets or scripts without checking existing libraries (LibAsset, LibSwap, LibAllowList) for prior art

Files:

  • src/Facets/LiFiIntentEscrowFacet.sol
src/**/*.sol

📄 CodeRabbit inference engine (.cursor/rules/106-gas.mdc)

src/**/*.sol: Follow gas-related best practices in existing facets before introducing new micro-optimizations in Solidity
Prefer using existing optimized libraries already in the repo (e.g., Solady/Solmate utilities) where they are in use, rather than hand-rolled low-level code
Do not sacrifice readability or safety for minor gas wins; explain any non-obvious optimization and justify it with expected impact
Avoid introducing inline assembly unless clearly needed; when used, document assumptions and invariants thoroughly

All contracts in src/ require audits except Interfaces in src/Interfaces/** (type definitions only) and external dependencies in lib/

src/**/*.sol: NatSpec required on contracts/interfaces in src/: @title, @author LI.FI (https://li.fi), @notice, @custom:version X.Y.Z; document all public/external functions with params/returns.
Bug bounty clarity: In contract NatSpec, explicitly document whether the contract is intended to hold/custody funds. If not designed to hold funds, state clearly (e.g., 'This contract is not intended to custody user funds / hold balances; any funds held are incidental'). If designed to hold funds, describe what funds it holds and under what conditions.
Apply Diamond patterns with existing libs (LibAsset/LibSwap/LibAllowList, Validatable/SwapperV2); prefer parameters over msg.sender for refund addresses.
In Solidity 0.8.17, you must not emit events using ContractName.EventName syntax (e.g., emit SomeContract.SomeEvent(...)). Events must be defined in the same contract where they're emitted, or defined in an interface that the contract uses, and then emitted using just the event name (e.g., emit SomeEvent(...)). This prevents compilation errors and ensures 0.8.17 compatibility.

Files:

  • src/Facets/LiFiIntentEscrowFacet.sol
🧠 Learnings (35)
📓 Common learnings
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : For non-EVM flows in Solidity facets, `bridgeData.receiver` must equal `NON_EVM_ADDRESS`
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must implement internal `_swapAndCompleteBridgeTokens()` function that handles native vs ERC20 tokens: use `safeApproveWithRetry()` or `safeIncreaseAllowance()` for ERC20 (reset to 0 after), pass `value: amount` for native tokens, call `executor.swapAndCompleteBridgeTokens()` in try-catch block, and on failure send raw tokens to receiver and emit `LiFiTransferRecovered`
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/002-architecture.mdc:0-0
Timestamp: 2025-12-17T10:27:21.083Z
Learning: Applies to src/Facets/**/*Bridge*.sol : For non-EVM chains (bridgeData.receiver == NON_EVM_ADDRESS), emit BridgeToNonEVMChainBytes32 with transactionId, destinationChainId, and non-EVM receiver (bytes32)
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must implement a bridge-specific external entry point (e.g., `handleV3AcrossMessage`, `lzCompose`, `cfReceive`) protected by authorization modifier that decodes bridge message to extract `bytes32 transactionId`, `LibSwap.SwapData[] memory swapData`, `address receiver`, and calls internal `_swapAndCompleteBridgeTokens()`
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must only emit `LiFiTransferRecovered` when swaps fail and raw bridged tokens are sent to receiver, with event signature: `LiFiTransferRecovered(bytes32 indexed transactionId, address receivingAssetId, address receiver, uint256 amount, uint256 timestamp)`
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : In Solidity facets, `receiverAddress` must be the first parameter in `{facetName}Data` and must match `bridgeData.receiver` for EVM chains
Learnt from: mirooon
Repo: lifinance/contracts PR: 1413
File: src/Facets/EverclearV2Facet.sol:75-96
Timestamp: 2025-10-13T11:13:48.847Z
Learning: In EverclearV2Facet (and all LiFi facets), the team standard is to use msg.sender as the refund address with the refundExcessNative modifier, not requiring an explicit refund address parameter.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1275
File: src/Facets/AllBridgeFacet.sol:186-188
Timestamp: 2025-07-16T01:04:55.857Z
Learning: In LiFi facet contracts, when public entry point functions have `nonReentrant` modifier protection, internal functions like `_startBridge` that they call benefit from this reentrancy protection, making event emission order after external calls acceptable from a security perspective.
Learnt from: ezynda3
Repo: lifinance/contracts PR: 1116
File: src/Libraries/LibAsset.sol:102-108
Timestamp: 2025-04-28T07:46:24.084Z
Learning: In the LiFi contracts, the `LibAsset.transferFromERC20` function is intentionally designed to accept arbitrary `from` parameters, with security ensured through a whitelist mechanism that controls which contracts can call this function.
Learnt from: ezynda3
Repo: lifinance/contracts PR: 984
File: docs/ChainflipFacet.md:0-0
Timestamp: 2025-02-13T08:57:00.095Z
Learning: The ChainflipData struct in ChainflipFacet has three fields:
1. uint32 dstToken - The destination token identifier in Chainflip
2. bytes32 nonEvmAddress - The non-EVM destination address
3. bytes cfParameters - Additional parameters for Chainflip protocol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must only emit `LiFiTransferRecovered` when swaps fail and raw bridged tokens are sent to receiver, with event signature: `LiFiTransferRecovered(bytes32 indexed transactionId, address receivingAssetId, address receiver, uint256 amount, uint256 timestamp)`

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must implement internal `_swapAndCompleteBridgeTokens()` function that handles native vs ERC20 tokens: use `safeApproveWithRetry()` or `safeIncreaseAllowance()` for ERC20 (reset to 0 after), pass `value: amount` for native tokens, call `executor.swapAndCompleteBridgeTokens()` in try-catch block, and on failure send raw tokens to receiver and emit `LiFiTransferRecovered`

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-10-10T12:50:47.440Z
Learnt from: reednaa
Repo: lifinance/contracts PR: 1361
File: script/demoScripts/demoLiFiIntentEscrow.ts:269-269
Timestamp: 2025-10-10T12:50:47.440Z
Learning: In `src/Interfaces/IOpenIntentFramework.sol`, the `StandardOrder` struct's `inputOracle` field is of type `address` (20 bytes), not `bytes32`. It should not be padded to 32 bytes when constructing `LiFiIntentEscrowData` in TypeScript/JavaScript code.

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : Receiver contracts must implement a bridge-specific external entry point (e.g., `handleV3AcrossMessage`, `lzCompose`, `cfReceive`) protected by authorization modifier that decodes bridge message to extract `bytes32 transactionId`, `LibSwap.SwapData[] memory swapData`, `address receiver`, and calls internal `_swapAndCompleteBridgeTokens()`

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-02-13T08:57:00.095Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 984
File: docs/ChainflipFacet.md:0-0
Timestamp: 2025-02-13T08:57:00.095Z
Learning: The ChainflipData struct in ChainflipFacet has three fields:
1. uint32 dstToken - The destination token identifier in Chainflip
2. bytes32 nonEvmAddress - The non-EVM destination address
3. bytes cfParameters - Additional parameters for Chainflip protocol

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
📚 Learning: 2025-10-13T11:13:48.847Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1413
File: src/Facets/EverclearV2Facet.sol:75-96
Timestamp: 2025-10-13T11:13:48.847Z
Learning: In EverclearV2Facet (and all LiFi facets), the team standard is to use msg.sender as the refund address with the refundExcessNative modifier, not requiring an explicit refund address parameter.

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:27:21.083Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/002-architecture.mdc:0-0
Timestamp: 2025-12-17T10:27:21.083Z
Learning: Applies to src/Facets/**/*Bridge*.sol : For non-EVM chains (bridgeData.receiver == NON_EVM_ADDRESS), emit BridgeToNonEVMChainBytes32 with transactionId, destinationChainId, and non-EVM receiver (bytes32)

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : In Solidity facets with `minAmountOut` or similar bridge parameters, update the bridge's minAmountOut in `swapAndStartBridgeTokensVia{FacetName}` to account for positive slippage from swaps after `_depositAndSwap` updates `_bridgeData.minAmount`, adjusting proportionally for decimal differences if applicable

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2024-11-05T17:16:19.946Z
Learnt from: maxklenk
Repo: lifinance/contracts PR: 782
File: script/demoScripts/demoPermit2.ts:0-0
Timestamp: 2024-11-05T17:16:19.946Z
Learning: In `script/demoScripts/demoPermit2.ts`, the `nextNonce` function should be called on the `PERMIT2_PROXY_ADDRESS` because the nonce is stored in the proxy contract, not in the `Permit2` contract (`PERMIT2_ADDRESS`).

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
📚 Learning: 2025-06-13T08:30:26.220Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1207
File: deployments/_deployments_log_file.json:34037-34080
Timestamp: 2025-06-13T08:30:26.220Z
Learning: LiFiDiamond contains generic withdrawal logic, so individual facet contracts (e.g., PioneerFacet) do not need their own Ether-withdraw functions.

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-04-28T07:46:24.084Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 1116
File: src/Libraries/LibAsset.sol:102-108
Timestamp: 2025-04-28T07:46:24.084Z
Learning: In the LiFi contracts, the `LibAsset.transferFromERC20` function is intentionally designed to accept arbitrary `from` parameters, with security ensured through a whitelist mechanism that controls which contracts can call this function.

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : In Solidity facets, `receiverAddress` must be the first parameter in `{facetName}Data` and must match `bridgeData.receiver` for EVM chains

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-01-22T12:36:12.699Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 945
File: test/solidity/Facets/GlacisFacet.t.sol:214-262
Timestamp: 2025-01-22T12:36:12.699Z
Learning: In the LiFi contracts, token pair validation is handled by third-party contracts, and route discovery including liquidity checks are performed off-chain. The contract only receives pre-validated routes, making token pair and liquidity validation tests unnecessary at the contract level.

Applied to files:

  • docs/LiFiIntentEscrowFacet.md
📚 Learning: 2025-10-02T18:10:09.934Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1406
File: src/Facets/UnitFacet.sol:75-81
Timestamp: 2025-10-02T18:10:09.934Z
Learning: In UnitFacet.sol and similar facet contracts in src/Facets/, the LiFiTransferStarted event emission should ALWAYS be at the end of the _startBridge internal function, after all validations and asset transfers have been completed successfully.

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:27:21.083Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/002-architecture.mdc:0-0
Timestamp: 2025-12-17T10:27:21.083Z
Learning: Applies to src/Facets/**/*Bridge*.sol : LiFiTransferStarted event must be emitted at the end of the internal _startBridge function in facets, after all validations and external bridge calls have completed successfully

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-07-16T01:04:55.857Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1275
File: src/Facets/AllBridgeFacet.sol:186-188
Timestamp: 2025-07-16T01:04:55.857Z
Learning: In LiFi facet contracts, when public entry point functions have `nonReentrant` modifier protection, internal functions like `_startBridge` that they call benefit from this reentrancy protection, making event emission order after external calls acceptable from a security perspective.

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : In Solidity facets, validate `targetChainId` against `bridgeData.destinationChain` for EVM-to-EVM bridges

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : For non-EVM flows in Solidity facets, `bridgeData.receiver` must equal `NON_EVM_ADDRESS`

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-24T08:52:48.554Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/101-solidity-contracts.mdc:0-0
Timestamp: 2025-12-24T08:52:48.554Z
Learning: Applies to src/**/*.sol : NatSpec required on contracts/interfaces in `src/`: `title`, `author LI.FI (https://li.fi)`, `notice`, `custom:version X.Y.Z`; document all public/external functions with params/returns.

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-08-29T11:18:56.656Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: src/Periphery/LDA/Facets/NativeWrapperFacet.sol:1-2
Timestamp: 2025-08-29T11:18:56.656Z
Learning: For src/Periphery/LDA/Facets/NativeWrapperFacet.sol, the SPDX license identifier should be immediately followed by the pragma statement without a blank line in between, as confirmed by mirooon.

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-03-05T14:41:05.791Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 953
File: script/deploy/zksync/utils/UpdateScriptBase.sol:32-50
Timestamp: 2025-03-05T14:41:05.791Z
Learning: In the LiFi contracts repository, explicit error handling for file operations and address validation in deployment scripts like UpdateScriptBase.sol is not considered necessary by the maintainer.

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-08-29T11:13:18.076Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: src/Periphery/LDA/Facets/VelodromeV2Facet.sol:5-11
Timestamp: 2025-08-29T11:13:18.076Z
Learning: The correct directory structure in the LiFi contracts codebase is src/Periphery/LDA/ (uppercase "LDA"), not src/Periphery/Lda/ (mixed case). Import paths should use "lifi/Periphery/LDA/..." to match the actual filesystem structure. The directory src/Periphery/LDA/ exists and contains all the LDA facets, while src/Periphery/Lda/ does not exist.

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-08-27T13:07:58.254Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: src/Periphery/Lda/Facets/CurveFacet.sol:0-0
Timestamp: 2025-08-27T13:07:58.254Z
Learning: In the LiFi contracts codebase, ICurveLegacy interface is defined inline in src/Periphery/LiFiDEXAggregator.sol with a non-payable exchange function. The CurveFacet diff shows importing it from a separate file that doesn't exist, creating a compilation issue.

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2024-10-09T03:47:21.269Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 806
File: deployments/_deployments_log_file.json:5780-5793
Timestamp: 2024-10-09T03:47:21.269Z
Learning: The owner address `0x11f11121df7256c40339393b0fb045321022ce44` and the `DiamondCutFacet` address `0xd5cf40a2a18b633cfd6a1ae16d1771596498cf83` in the `LiFiDiamond` deployment on `xlayer` are correct and should not be flagged as issues, even if they are not referenced in the Solidity files.

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-08-29T11:13:11.137Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: src/Periphery/LDA/Facets/UniV3StyleFacet.sol:4-12
Timestamp: 2025-08-29T11:13:11.137Z
Learning: The correct directory structure for LDA imports in the LiFi contracts codebase is "src/Periphery/LDA/" (uppercase). Import paths should use "lifi/Periphery/LDA/..." to match the actual filesystem structure. The files exist at paths like src/Periphery/LDA/PoolCallbackAuthenticated.sol, src/Periphery/LDA/Errors/Errors.sol, and src/Periphery/LDA/BaseRouteConstants.sol.

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-29T12:28:41.835Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1555
File: script/playgroundHelpers.sh:60-68
Timestamp: 2025-12-29T12:28:41.835Z
Learning: In script/playgroundHelpers.sh at line 63 within the getContractVerified() function, the field `.CONSTRUCTOR_ARGS` should be updated to `.constructorArgs` (camelCase) to match the MongoDB IDeploymentRecord interface and be consistent with other field names in the same block (address, optimizerRuns, timestamp, salt, verified, solcVersion, evmVersion, zkSolcVersion). This inconsistency causes the field to return null. User mirooon requested this be flagged when playgroundHelpers.sh is modified in future PRs.

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : Facets must use LibAsset, LibSwap, and LibAllowList utilities along with Validatable and SwapperV2 interfaces; reserve native fees via `_depositAndSwap` variants when needed

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-01-28T14:30:06.911Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 924
File: deployments/abstract.json:2-4
Timestamp: 2025-01-28T14:30:06.911Z
Learning: In the LiFi contract architecture, "LiFiDiamond" is the diamond proxy contract that connects all facets into a cohesive system. The facets are individual contracts that provide specific functionality, and the diamond proxy delegates calls to these facets.

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-08-27T13:07:58.254Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1328
File: src/Periphery/Lda/Facets/CurveFacet.sol:0-0
Timestamp: 2025-08-27T13:07:58.254Z
Learning: The ICurveLegacy interface in src/Periphery/LiFiDEXAggregator.sol IS payable (can accept ETH), unlike ICurve and ICurveV2 which are non-payable. The current CurveFacet uses non-payable interfaces and cannot handle native assets.

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:34.379Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/104-receiver-contracts.mdc:0-0
Timestamp: 2025-12-17T10:28:34.379Z
Learning: Applies to src/Periphery/Receiver*.sol : For native tokens with gas checks (e.g., StargateV2), Receiver contracts must check `gasleft()` before executing; if insufficient gas, send tokens directly to receiver and emit `LiFiTransferRecovered`

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-17T10:28:13.904Z
Learnt from: CR
Repo: lifinance/contracts PR: 0
File: .cursor/rules/102-facets.mdc:0-0
Timestamp: 2025-12-17T10:28:13.904Z
Learning: Applies to src/Facets/**/*Facet.sol : In Solidity facets with `{facetName}Data.receiverAddress` field (e.g., `_glacisData.receiverAddress`), validate that it is not `bytes32(0)` for non-EVM chains and revert with `InvalidNonEVMReceiver()` if zero

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2024-10-31T09:09:38.568Z
Learnt from: ezynda3
Repo: lifinance/contracts PR: 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:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-12-12T11:16:09.236Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1530
File: src/Interfaces/ICircleBridgeProxy.sol:20-29
Timestamp: 2025-12-12T11:16:09.236Z
Learning: In src/Interfaces/ICircleBridgeProxy.sol, ensure the ICircleBridgeProxy interface corresponds to Celer's CircleBridgeProxy contract (not Circle's native TokenMessenger). The depositForBurn signature must be: depositForBurn(uint256 _amount, uint64 _dstChid, bytes32 _mintRecipient, address _burnToken, uint256 _maxFee, uint32 _minFinalityThreshold) external. This change should be scoped specifically to this file; pattern is not needed since it targets a single file. This improves correctness by aligning the interface with the correct contract and parameter types (dst chain id as uint64, no bytes32 destinationCaller parameter).

Applied to files:

  • src/Facets/LiFiIntentEscrowFacet.sol
📚 Learning: 2025-08-27T23:36:40.773Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 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:

  • src/Facets/LiFiIntentEscrowFacet.sol
🔇 Additional comments (6)
src/Facets/LiFiIntentEscrowFacet.sol (4)

82-97: Entry point looks correct.

The startBridgeTokensViaLiFiIntentEscrow function has appropriate modifiers for a bridge-only path. The internal validation in _startBridge handles the hasDestinationCall flag check.


103-133: Positive slippage returned to user is appropriate for intent-based bridging.

The design returns positive slippage to the user (lines 124-130) rather than adjusting outputAmount. This is correct for an intent system where outputAmount represents a fixed expectation that the solver must fulfill, independent of input-side slippage.


183-195: Destination call encoding is correct.

The logic properly encodes transactionId, dstCallSwapData, and the original recipient into outputCall when destination swaps are present, then correctly swaps the output recipient to dstCallReceiver. The zero-check on dstCallReceiver ensures valid configuration.


197-228: Output construction and event emission are correct.

The MandateOutput is properly constructed with the conditional recipient and callback data. The LiFiTransferStarted event is correctly emitted at the end of _startBridge after all validations and external calls, following the facet pattern.

docs/LiFiIntentEscrowFacet.md (2)

5-5: Grammar fixes applied correctly.

The "built-in" and "self-serve" hyphenation issues from previous reviews have been addressed.


34-35: Destination calls documentation is accurate and grammar is correct.

The description correctly explains how dstCallReceiver and dstCallSwapData work together, and the grammar issues from previous reviews have been addressed.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

@mirooon mirooon marked this pull request as ready for review January 16, 2026 14:01
@lifi-action-bot
Copy link
Collaborator

lifi-action-bot commented Jan 16, 2026

🤖 GitHub Action: Security Alerts Review 🔍

🟢 Dismissed Security Alerts with Comments
The following alerts were dismissed with proper comments:

🟢 View Alert - File: src/Facets/LiFiIntentEscrowFacet.sol
🔹 Contracts that can receive ether but cannot send it may lock value permanently. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/locked-ether
🔹 Dismiss Reason: False positive
🔹 Dismiss Comment: The contract is intended to be used in a Diamond Facet scheme. As a result, the Diamond will have to implement "unlocking" functionality. The contract is intended to be delegate called.

🟢 View Alert - File: src/Facets/LiFiIntentEscrowFacet.sol
🔹 Reentrant functions which emit events after making an external call may lead to out-of-order events. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/reentrancy-events
🔹 Dismiss Reason: Won't fix
🔹 Dismiss Comment: This is intended

No unresolved security alerts! 🎉

@lifi-action-bot lifi-action-bot marked this pull request as draft January 16, 2026 14:01
@reednaa reednaa marked this pull request as ready for review January 19, 2026 09:41
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/Facets/LiFiIntentEscrowFacet.sol`:
- Around line 144-169: The code must explicitly block destination calls when the
bridge receiver is NON_EVM_ADDRESS: add an early guard in the validation block
(using dstCallSwapDataLength or _lifiIntentData.dstCallSwapData,
_bridgeData.receiver and NON_EVM_ADDRESS) that reverts (e.g.,
InformationMismatch or InvalidReceiver) if dstCallSwapDataLength > 0 (or
_lifiIntentData.dstCallSwapData is non-empty) while _bridgeData.receiver ==
NON_EVM_ADDRESS, to prevent overwriting the recipient with an EVM
dstCallReceiver and creating an unfillable order; place this check before
emitting BridgeToNonEVMChainBytes32 or comparing recipient
bytes32(uint256(uint160(_bridgeData.receiver))).

mirooon
mirooon previously approved these changes Jan 19, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/Facets/LiFiIntentEscrowFacet.sol (1)

116-125: ⚠️ Potential issue | 🟠 Major

Excess native refunds bypass depositAndRefundAddress.

refundExcessNative(payable(msg.sender)) returns any overpaid native to the caller, but this flow now treats depositAndRefundAddress as the canonical refund target (including positive slippage). If callers are relayers, excess ETH can be sent to the wrong party. Consider routing the modifier refund to depositAndRefundAddress (or explicitly document that excess msg.value is always refunded to the caller).

💡 Proposed fix
-        refundExcessNative(payable(msg.sender))
+        refundExcessNative(payable(_lifiIntentData.depositAndRefundAddress))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants