Conversation
Make exclusive for optional
# Conflicts: # src/lib/libraries/intent.ts # src/lib/libraries/orderServer.ts
* Add deploy worker workflow (#28) * chore: add the workers deploy workflow + wrangler.toml file * delete wranger.jsonc to avoid pipeline conflict * Fix view on mobile and enhance uuid generation * Allow directly adding tokens * Use spaces instead of tabs. * update the workflow inline because the repository is public and cannot access central repository (#30) * update the workflow inline because the repository is public and cannot access central repository * add the env vriable to the pipeline * add the env variables to the Deploy to Cloudflare Workers steps * Throw bad DBs for adding new tokens. * Add deps in an attempt to get worker build to pass * Update wrangler for crypto * Fix db crash * Support selecting intent api env * arcTestnet * new testnet polymer oracles * Fix USDC address --------- Co-authored-by: azzahamdani <azzahamdani@gmail.com>
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 Open source vulnerabilities detected - medium severity
Aikido detected 2 vulnerabilities across 1 package, it includes 2 medium vulnerabilities.
Details
Remediation Aikido suggests bumping the vulnerable packages to a safe version.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
📝 WalkthroughWalkthroughThis PR migrates the application from a local OrderServer to LiFi's public Intent API while restructuring the token and chain data models to use chainId-based identifiers instead of chain names. Database migrations are consolidated, infrastructure is modernized with new CI/CD workflows, and contract ABIs are expanded with complete definitions. ChangesIntent API Migration & Type System Refactor
Database Schema Consolidation
Development Infrastructure & Configuration
Contract ABI Definitions & Interfaces
Component Markup & Styling Refinements
Sequence DiagramssequenceDiagram
participant User
participant App
participant OrderServer as OrderServer<br/>(Legacy)
participant Settlement
User->>App: Select tokens & create intent
App->>OrderServer: POST /quote (TokenContext[])
OrderServer-->>App: Quote response
App->>User: Show quote
User->>App: Confirm
App->>Settlement: Sign & submit intent
Settlement-->>App: Order ID
App->>User: Order created
sequenceDiagram
participant User
participant App
participant IntentApi as LiFi Intent API
participant WalletClient as Wallet Client
participant SmartContract as Settlement Contract
User->>App: Select tokens & create intent
App->>App: Convert AppCreateIntentOptions<br/>to LiFi CreateIntentOptions
App->>IntentApi: POST /quote (chainId-based)
IntentApi-->>App: Quote response
App->>User: Show quote
User->>App: Confirm
App->>WalletClient: signIntentCompact()
WalletClient-->>App: Signature
App->>SmartContract: openEscrowIntent()
SmartContract-->>App: Escrow hash
App->>SmartContract: finaliseIntent()
SmartContract-->>App: Finalization receipt
App->>User: Order completed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Rationale: This PR spans multiple independent concerns (Intent API migration, database consolidation, infrastructure updates, ABI expansions) with significant logic density in configuration rewriting and component refactoring. The variety of changes across different layers (types, libraries, components, ABIs, workflows) demands separate reasoning for each area. ABI updates are largely mechanical but require verification of completeness. The Intent API integration involves non-trivial type conversions and function signature changes that interact across multiple modules. While there are repetitive formatting/indentation changes, the core functional changes are heterogeneous and require careful tracing through the new data flow. Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (12)
src/lib/components/ui/ScreenFrame.svelte-11-12 (1)
11-12:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
titleanddescriptiontyped as requiredstringbut guarded with{#if}— consider marking them optionalBoth props are declared as
title: stringanddescription: string(non-optional), meaning TypeScript will error if a caller omits them. However, the template wraps them in{#iftitle}/{#ifdescription}, which implies callers may legitimately omit or pass an empty value. If the intent is to allow omission, mark them optional:🔧 Proposed fix
}: { - title: string; - description: string; + title?: string; + description?: string; contentClass?: string; bodyClass?: string; children?: Snippet; } = $props();Also applies to: 20-25
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/components/ui/ScreenFrame.svelte` around lines 11 - 12, The props title and description in ScreenFrame.svelte are declared as required (title: string; description: string;) but are conditionally used in the template, so update their typings to be optional (title?: string; description?: string;) and apply the same optional change to the other prop declarations around the 20–25 region that mirror these lines; ensure any related methods or consumers handle possibly undefined values (e.g., keep the existing {`#if` title} / {`#if` description} guards).src/lib/components/ui/FlowStepTracker.svelte-59-73 (1)
59-73:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale-check doesn't guard against post-unmount state updates
The
flowChecksRuncounter correctly drops results from superseded runs, but if the component unmounts while agetOrderProgressCheckspromise is in flight,flowChecksRunis never incremented and the resolved callback will still write toflowChecks. In Svelte 5 runes mode this can trigger a dev-modestate_unsafe_mutationwarning (and at minimum wastes the update).Increment
flowChecksRunin the effect's cleanup so any in-flight request is treated as stale on unmount:🛡️ Proposed fix
$effect(() => { progressRefreshTick; store.connectedAccount; store.walletClient; selectedOrder; selectedOutputFillHashSignature; if (!store.connectedAccount || !store.walletClient || !selectedOrder) { flowChecks = { allFilled: false, allValidated: false, allFinalised: false }; return; } const currentRun = ++flowChecksRun; getOrderProgressChecks(selectedOrder, store.fillTransactions) .then((checks) => { if (currentRun !== flowChecksRun) return; flowChecks = checks; }) .catch((error) => { console.warn("flow progress update failed", error); if (currentRun !== flowChecksRun) return; flowChecks = { allFilled: false, allValidated: false, allFinalised: false }; }); + + return () => { + flowChecksRun++; + }; });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/components/ui/FlowStepTracker.svelte` around lines 59 - 73, The promise from getOrderProgressChecks can still resolve after the component unmounts and mutate flowChecks because flowChecksRun isn't advanced on cleanup; update the effect that calls getOrderProgressChecks (the block referencing flowChecksRun, getOrderProgressChecks, selectedOrder, and store.fillTransactions) to increment flowChecksRun in the effect's cleanup/unmount handler so any in-flight callbacks see a stale currentRun and early-return instead of writing to flowChecks.src/lib/components/ui/InlineMetaField.svelte-16-22 (1)
16-22:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winVerify the combination of
defaultValuewithbind:valueand null handling.In Svelte,
defaultValueis typically used for uncontrolled inputs, whilebind:valuecreates a controlled two-way binding. When both are present,defaultValuemay be ignored or cause unexpected behavior. Additionally, whenvalueisnull, the number input's display behavior is unclear.Consider removing
defaultValueand explicitly handling the null case in the bound variable, or using a placeholder if a visual cue is needed when the field is empty.🔧 Proposed fix to remove defaultValue and handle null explicitly
<input type="number" class="min-w-0 flex-1 border-0 bg-transparent p-0 text-sm outline-none disabled:cursor-not-allowed disabled:text-gray-400" - defaultValue="0" + placeholder="0" bind:value {disabled} />Alternatively, if you want the input to always show a number, initialize
valueto0instead of allowingnull, or coerce null to 0 in the binding.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/components/ui/InlineMetaField.svelte` around lines 16 - 22, Remove the uncontrolled defaultValue from the <input> (it conflicts with bind:value) and explicitly handle null/undefined in the bound variable used by bind:value inside InlineMetaField.svelte: delete the defaultValue attribute and either initialize or coerce the bound prop/variable (the one referenced by bind:value) to a safe display value (e.g. '' for empty or 0 if you want a numeric default) via an initializer or a small reactive statement (e.g. $: safeValue = value ?? '' and bind to that or coerce on input), or use a placeholder for visual hinting instead of defaultValue so the number input behavior is deterministic.src/lib/components/ui/FlowProgressList.svelte-85-106 (1)
85-106:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCenter the connector fill overlay to match the track position.
The fill bar is anchored at the container corner while the track is centered, which can render the fill visibly offset in both orientations.
Suggested fix
.connector-fill { + top: 50%; + left: 0; + transform: translateY(-50%); width: var(--fill-pct); height: 2px; transition: width 75ms linear; } `@media` (min-width: 768px) { .connector-fill { + top: 0; + left: 50%; + transform: translateX(-50%); width: 2px; height: var(--fill-pct); transition: height 75ms linear; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/components/ui/FlowProgressList.svelte` around lines 85 - 106, The connector fill is anchored to the container corner so it doesn’t align with the centered track; update the .connector-fill positioning so it is centered along the orthogonal axis for each orientation: in the base rule (horizontal track) keep width: var(--fill-pct) and height: 2px but replace top:0 left:0 with top:50% and transform: translateY(-50%) so the 2px bar is vertically centered; in the `@media` (min-width: 768px) override (vertical track) keep width: 2px and height: var(--fill-pct) but replace top/left with left:50% and transform: translateX(-50%) so the 2px column is horizontally centered; this will ensure the fill rendered by connectorFillPercent(i) lines up with the centered track.src/lib/libraries/flowProgress.ts-157-168 (1)
157-168:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing guard for empty inner input array before
flattenedInputs[0][0]access.Line 158 guards
flattenedInputs.length === 0, but not thatflattenedInputs[0]itself is non-empty. If the first input group has no entries,flattenedInputs[0][0]isundefined, which will be forwarded to thegetLockDetailscontract call and produce a confusing RPC error instead of a cleanfalse.🛡️ Proposed fix
- if (!flattenedInputs || flattenedInputs.length === 0) return false; + if (!flattenedInputs || flattenedInputs.length === 0 || !flattenedInputs[0]?.length) return false;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/libraries/flowProgress.ts` around lines 157 - 168, The code calls inputChainClient.readContract(... args: [flattenedInputs[0][0]]) without checking that flattenedInputs[0] is a non-empty array; add a guard before the getOrFetchRpc call (return false) when flattenedInputs[0] is missing or has length 0 so getLockDetails never receives undefined. Locate the usage of flattenedInputs in the function (the flattenedInputs assignment and the getOrFetchRpc block) and ensure you check flattenedInputs[0] && flattenedInputs[0].length > 0 (or equivalent) before proceeding to call inputChainClient.readContract/getLockDetails.src/lib/components/Introduction.svelte-7-77 (1)
7-77:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix user-facing copy typos/grammar in intro text.
There are several textual issues (e.g., Line 7 “support” → “supports”, Line 20 “work in progress” → “a work in progress”, Line 38 “transaction are” → “transactions are”, Line 76 “configured different” → “configured differently”, Line 77 “Learm” → “Learn”). Please clean these up before release.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/components/Introduction.svelte` around lines 7 - 77, Update the user-facing copy in Introduction.svelte: change "support" to "supports" in the sentence containing "It currently support a seamless resource lock flow", change "work in progress" to "a work in progress" where "Multichain intents are currently work in progress", change "transaction are" to "transactions are" in the list item "No upfront deposit or initiation transaction are required.", change "configured different" to "configured differently" in the Same Chain paragraph, and fix "Learm" to "Learn" in the trailing sentence; locate these phrases in the component's paragraph and heading nodes and apply the exact word replacements to correct grammar/typos.playwright.config.ts-16-16 (1)
16-16:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse explicit env parsing for
PW_VIDEO_ALL.At Line 16,
process.env.PW_VIDEO_ALL ? "on" : ...treats"0"/"false"as truthy. Prefer explicit checks (e.g.,=== "1"or=== "true") to avoid accidental always-on video in CI/local runs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@playwright.config.ts` at line 16, The video setting currently uses a truthy check on process.env.PW_VIDEO_ALL which treats values like "0" or "false" as true; update the video assignment in the Playwright config (the video property in playwright.config.ts) to parse the env explicitly—e.g., check process.env.PW_VIDEO_ALL === "1" or === "true" (or use a small helper like isEnvTrue) so only explicit truthy strings enable "on", otherwise use "retain-on-failure".src/lib/abi/escrow.ts-1231-1262 (1)
1231-1262:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDuplicate
FillDeadlineAfterExpiryerror entry.The
FillDeadlineAfterExpiryerror is declared twice with identical inputs (lines 1231–1246 and 1247–1262). This looks like a copy/paste artifact from ABI regeneration. Most tooling will tolerate it, but it is dead noise and can confuse static analyzers or downstream ABI consumers. Drop the second occurrence.♻️ Proposed cleanup
{ type: "error", name: "FillDeadlineAfterExpiry", inputs: [ { name: "fillDeadline", type: "uint32", internalType: "uint32" }, { name: "expires", type: "uint32", internalType: "uint32" } ] }, - { - type: "error", - name: "FillDeadlineAfterExpiry", - inputs: [ - { - name: "fillDeadline", - type: "uint32", - internalType: "uint32" - }, - { - name: "expires", - type: "uint32", - internalType: "uint32" - } - ] - }, { type: "error", name: "GovernanceFeeChangeNotReady",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/abi/escrow.ts` around lines 1231 - 1262, The ABI contains a duplicated error definition for FillDeadlineAfterExpiry; remove the second identical error object so only one declaration of the error named "FillDeadlineAfterExpiry" with the two uint32 inputs remains in the exported ABI (search for the error object with name "FillDeadlineAfterExpiry" in the escrow ABI array and delete the duplicate entry, ensuring no other duplicates are present).src/lib/components/AwaitButton.svelte-45-48 (1)
45-48:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
defaultLazyshould be wrapped in$derivedto maintain reactivity with thevariantprop.Unlike
defaultBaseanddefaultHover,defaultLazyis a plainconst, so it captures the initial value ofvariantand will not update if the prop changes after mount. Ifvariantever transitions to/from"muted", the awaiting button will display the wrong text color. Wrap it in$derivedfor consistency and correctness.♻️ Proposed fix
- const defaultLazy = [ - "cursor-not-allowed", - variant === "muted" ? "text-gray-500" : "text-gray-400" - ]; + const defaultLazy = $derived([ + "cursor-not-allowed", + variant === "muted" ? "text-gray-500" : "text-gray-400" + ]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/components/AwaitButton.svelte` around lines 45 - 48, defaultLazy is currently a plain const and captures variant's initial value; make it reactive like defaultBase/defaultHover by wrapping it in a Svelte $: or using derived reactive declaration so it recomputes when the variant prop changes. Update the declaration of defaultLazy (referencing defaultLazy and variant) to a reactive derived value that returns ["cursor-not-allowed", variant === "muted" ? "text-gray-500" : "text-gray-400"] so the text color updates when variant changes.src/lib/libraries/intentList.ts-77-96 (1)
77-96:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winVerify behavior when
getCoinreturns its unknown-token fallback.When the token isn't in
coinList,getCoin(insrc/lib/config.ts) silently returns{ name: "Unknown", decimals: 1, ... }.formatTokenAmount(amount, 1)then divides by10rather than the real decimals, displaying severely wrong values for any token not enumerated incoinList. Consider explicit handling here (e.g., short-circuit to the raw-address branch whencoin.name === "Unknown").🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/libraries/intentList.ts` around lines 77 - 96, summarizeInput and summarizeOutput call getCoin and always format amounts, but getCoin's unknown-token fallback ({name: "Unknown", decimals: 1}) causes wrong scaling; update both functions to detect the unknown fallback (e.g., if coin.name === "Unknown" or otherwise indicates fallback) and short-circuit to the raw-address branch (use shortAddress(tokenAddress) in summarizeInput and shortAddress(token) in summarizeOutput and return the `${amount.toString()} ... on chain-${chainId.toString()}` style string) instead of calling formatTokenAmount; keep existing chainName handling and only format when a real coin is returned.src/lib/config.ts-348-391 (1)
348-391:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
getCoinsilently fabricates an "Unknown" token withdecimals: 1, yet throws viaisChainIdTestnetfor unknown chainIds.Two related concerns:
- The fallback at lines 371–377 returns
decimals: 1, which causes any downstreamformatTokenAmount(e.g.summarizeInput/summarizeOutputinintentList.ts) to display amounts off by many orders of magnitude rather than producing a visible error.getCoincallsisChainIdTestnet(chainId)(line 362) which throws for unknown chains — so unknown-chain inputs throw, but unknown-token-with-known-chain inputs silently return the wrong fallback. Pick one consistent behavior (either throw in both cases or return a sentinel that callers must handle).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/config.ts` around lines 348 - 391, getCoin currently validates the chain via isChainIdTestnet but then silently returns a fabricated token with decimals: 1 when a token is not found, causing downstream formatting errors; make the behavior consistent by first normalizing/validating the chain (reuse normalizeChainId and call isChainIdTestnet to surface unknown-chain errors) and then, if no matching token from coinList(...) is found, throw a clear Error (e.g. "Token not found for name/address on chain <chainId>") instead of returning a bogus token with decimals: 1 so callers of formatTokenAmount/summarizeInput cannot silently mis-format values. Ensure you reference getCoin, normalizeChainId, isChainIdTestnet and coinList when making these changes.src/lib/components/InputTokenModal.svelte-13-14 (1)
13-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPrecision loss in
formatBalancefor large balances.
Number(value)lossily convertsbigintonce the balance exceedsNumber.MAX_SAFE_INTEGER(2^53−1). For 18-decimal tokens this happens at ~9 tokens; the displayedmetaTextwill then be inaccurate. Consider formatting via integer/fractional bigint split.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/components/InputTokenModal.svelte` around lines 13 - 14, formatBalance currently calls Number(value) which loses precision for bigints > Number.MAX_SAFE_INTEGER; change formatBalance to do integer/fractional bigint math: compute integerPart = value / 10n**BigInt(decimals) and fracPart = value % 10n**BigInt(decimals), build fracStr by left-padding fracPart.toString() to decimals and trimming trailing zeros, format integerPart as a thousands-separated string (operate on integerPart.toString() and insert separators) and return combined "integer.frac" (or just integer if fracStr is empty); update any uses such as metaText to rely on the new function.
🧹 Nitpick comments (12)
src/lib/components/ui/FormControl.svelte (1)
4-24: 💤 Low value
[key: string]: unknownindex signature weakens prop type safety.The index signature
[key: string]: unknownis necessary to type...rest, but it silently makes all declared prop types compatible withunknown, so typos in prop names won't produce a TS error at the call site. This is a known Svelte 5 limitation with rest-prop typing and is an accepted trade-off for passthrough components — no immediate action required, but worth noting for future consideration if the Svelte team improvesHTMLAttributes-based typing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/components/ui/FormControl.svelte` around lines 4 - 24, The props type currently contains a broad index signature `[key: string]: unknown` which weakens type checking; remove that index signature and instead type the rest-payload explicitly (e.g., add a `rest?: Record<string, unknown>` entry in the props type) while keeping the destructuring `...rest` in FormControl.svelte; specifically, edit the prop type for the let-block (the destructured object containing as, value, type, size, density, state, className, children) to drop `[key: string]: unknown` and add a named `rest?: Record<string, unknown>` (or another narrow type) so callers get stricter prop checks while `...rest` still captures passthrough attributes.src/lib/components/ui/FlowProgressList.svelte (1)
71-73: ⚡ Quick winMark the active step with
aria-currentfor screen readers.The active step is visually clear, but assistive tech doesn’t get an explicit current-step indicator yet.
Suggested fix
title={`${step.label} (${statusText[step.status]})`} aria-label={`${step.label} (${statusText[step.status]})`} + aria-current={step.status === "active" ? "step" : undefined} onclick={() => step.clickable && onStepClick?.(step)}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/components/ui/FlowProgressList.svelte` around lines 71 - 73, Add an explicit aria-current indicator for the active step so screen readers know which is current: in FlowProgressList.svelte update the element that renders each step (the same element that sets title, aria-label and onclick for the local variable step) to include an aria-current attribute when step represents the active state (e.g., aria-current="step" only when step.status equals the active status value used in this component); ensure the attribute is omitted or falsy for non-active steps so it isn't present erroneously.drizzle/0000_brief_corsair.sql (1)
1-8: 🏗️ Heavy liftDROP TABLE statements in initial migration are safe but affect local user data.
The migration uses
DROP TABLE IF EXISTSbecause the migration set was regenerated from the schema definition (drizzle-kit consolidating 0000-0004 into a single 0000 entry). However, this is a PGlite IndexedDB database running in the browser (idb://orders), not a multi-user PostgreSQL server with shared state across environments.This means:
- No risk of losing production/staging/shared database content
- Users will lose their local IndexedDB cache on next app deployment
- Data loss is per-user and non-critical since it's ephemeral client-side storage
If you want to preserve existing user data across deployments, either:
- Keep the incremental migration set (0000-0004) without the
DROP TABLE IF EXISTSstatements, or- Alter the migration to check if data exists before dropping and perform schema-only changes
Otherwise, the current approach is acceptable for a client-side cache, though users will see a blank state on their first load after update.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drizzle/0000_brief_corsair.sql` around lines 1 - 8, This migration currently unconditionally drops local IndexedDB tables transaction_receipts, tokens, intents, and fill_transactions which will wipe each user's client-side cache; either remove the DROP TABLE IF EXISTS statements so the migration becomes additive (keep the incremental migrations instead of consolidating 0000-0004) or replace each DROP with a safe-preserve strategy: check for existing data in the respective table and only perform schema-only changes (or create tables with CREATE TABLE IF NOT EXISTS) so existing IndexedDB data isn't removed; update the 0000_brief_corsair.sql migration to implement one of these two approaches and ensure the statements referencing "transaction_receipts", "tokens", "intents", and "fill_transactions" are adjusted accordingly.src/lib/components/ui/SegmentedControl.svelte (1)
26-41: ⚡ Quick winExpose selected state for assistive tech.
At Line 26-41, each segment is a button but lacks
aria-pressed(or equivalent state semantics). Addaria-pressed={value === option.value}so keyboard/screen-reader users can identify the active option.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/components/ui/SegmentedControl.svelte` around lines 26 - 41, The SegmentedControl buttons lack accessible state semantics: update the button in SegmentedControl.svelte (the element using value, option.value, testIdPrefix, and onclick/onChange) to include an aria-pressed attribute set to the active check (aria-pressed={value === option.value}) so assistive tech can identify the selected segment; ensure the expression uses the same reactive/value variables and does not alter existing onclick/onChange behavior or testId usage.src/lib/components/AwaitButton.svelte (1)
50-55: 💤 Low valueRe-throwing in
.catchworks, but consider awaiting the rejection visibly.The promise chain assigns the rejected promise back to
buttonPromise, so{:catch}will render. However, swallowing then re-throwing only to log is a minor smell — the{:catch}block already gets the error and you log it before re-throwing. This is fine as-is; consider exposing the error to the caller via anonErrorprop or surfacing it in UI rather than onlyconsole.error, since users will see no feedback when the action fails (the catch arm renders the same active button as success).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/components/AwaitButton.svelte` around lines 50 - 55, The run function currently logs and re-throws errors from buttonFunction but doesn't surface them to the component consumer or UI; add an optional onError prop (e.g., export let onError: (err) => void) and call it inside the catch handler for buttonPromise (reference run, buttonPromise, buttonFunction) so callers can react; also consider storing the error in a local exported/state variable that the {:catch} block can display instead of only logging, and remove the needless re-throw if you want the component to handle the error locally.src/lib/abi/wormholeoracle.ts (1)
1-1: ⚡ Quick winTypo in exported identifier:
WROMHOLE_ORACLE_ABI.The export name is misspelled —
WROMHOLEshould beWORMHOLE. Since the ABI was reworked in this PR, this is a good opportunity to rename the export. The identifier appears only at its declaration, so the rename is straightforward.♻️ Proposed rename
-export const WROMHOLE_ORACLE_ABI = [ +export const WORMHOLE_ORACLE_ABI = [🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/abi/wormholeoracle.ts` at line 1, The exported identifier is misspelled as WROMHOLE_ORACLE_ABI; rename it to WORMHOLE_ORACLE_ABI in the declaration (export const WROMHOLE_ORACLE_ABI -> export const WORMHOLE_ORACLE_ABI) and update any local references or imports/usages to the new name to keep the module API consistent (search for WROMHOLE_ORACLE_ABI and replace with WORMHOLE_ORACLE_ABI).src/lib/libraries/intentFactory.ts (2)
254-258: 💤 Low valueRemove commented-out code.
Dead code adds noise; if the receipt-wait is intentionally deferred, replace with a brief comment explaining why.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/libraries/intentFactory.ts` around lines 254 - 258, Remove the commented-out receipt-wait loop that references transactionHashes and clients[inputChain].waitForTransactionReceipt in intentFactory.ts; either delete those lines entirely to eliminate dead code, or replace them with a one-line comment explaining why waiting for receipts is intentionally skipped (e.g., "Receipt wait deferred to caller" or similar) so future readers know this behavior is intentional; ensure references to transactionHashes, clients, waitForTransactionReceipt, and inputChain remain clear in the comment if kept.
210-214: ⚡ Quick winUnused
receipt.The result of
waitForTransactionReceiptis awaited but never read or status-checked. If the goal is to block until inclusion, preferawait ... .waitForTransactionReceipt(...)without the binding; if the goal is to verify success, branch onreceipt.status.♻️ Drop the unused binding (or assert success)
- const receipt = await getClient(inputTokens[0].token.chainId).waitForTransactionReceipt({ - hash: transactionHash - }); + const receipt = await getClient(inputTokens[0].token.chainId).waitForTransactionReceipt({ + hash: transactionHash + }); + if (receipt.status !== "success") { + throw new Error(`compactDepositAndRegister tx ${transactionHash} reverted`); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/libraries/intentFactory.ts` around lines 210 - 214, The await on getClient(...).waitForTransactionReceipt(...) assigns an unused variable receipt; either drop the binding to simply await completion or explicitly check receipt.status to assert success. Locate the call after depositAndRegisterCompact in intentFactory (the lines using transactionHash and inputTokens[0].token.chainId), and replace the unused "const receipt = await getClient(...).waitForTransactionReceipt(...)" with either "await getClient(...).waitForTransactionReceipt(...)" if only waiting is needed, or keep the binding and add a status check on receipt.status (throw or handle non-success) to verify the transaction succeeded.src/lib/libraries/intentList.ts (1)
1-14: 💤 Low valueOptional: consolidate
@lifi/intentimports.The four separate
@lifi/intentimport statements can be combined into a single grouped import for readability.♻️ Proposed consolidation
-import { orderToIntent } from "@lifi/intent"; -import { bytes32ToAddress, idToToken } from "@lifi/intent"; -import type { OrderContainer, StandardOrder, MultichainOrder } from "@lifi/intent"; -import { validateOrderContainerWithReason } from "@lifi/intent"; +import { + orderToIntent, + bytes32ToAddress, + idToToken, + validateOrderContainerWithReason, + type OrderContainer, + type StandardOrder, + type MultichainOrder +} from "@lifi/intent";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/libraries/intentList.ts` around lines 1 - 14, Consolidate the multiple imports from "@lifi/intent" into a single grouped import to improve readability: replace the separate import lines that bring in orderToIntent, bytes32ToAddress, idToToken, OrderContainer, StandardOrder, MultichainOrder, and validateOrderContainerWithReason with one import statement from "@lifi/intent" that lists all those symbols together; leave the unrelated import (orderValidationDeps) from "./coreDeps" and the other config imports unchanged.src/lib/components/GetQuote.svelte (1)
28-28: 💤 Low value
$derived(new IntentApi(...))re-instantiates on every reactive read.
$derivedrecomputes when its dependencies change, so any update touseProductionApi/mainnetwill allocate a freshIntentApi. If the class holds caches/HTTP keep-alives this is wasteful. If the inputs are effectively static after mount, hoist into alet intentApi = new IntentApi(...)assigned once in an$effectkeyed on the env flags.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/components/GetQuote.svelte` at line 28, The current use of $derived(new IntentApi(useProductionApi ?? mainnet)) recreates an IntentApi on every reactive read; instead, stop constructing IntentApi inside $derived and instantiate it once when the env flags change: create a let intentApi variable and set it inside an $: or $effect reactive block keyed on useProductionApi and mainnet (e.g., $: if (useProductionApi !== undefined || mainnet !== undefined) intentApi = new IntentApi(useProductionApi ?? mainnet)), so IntentApi (class name IntentApi) is only re-created when those inputs change and not on every reactive read.src/lib/libraries/compactLib.ts (2)
35-58: 💤 Low valueRedundant
awaitand inconsistentwriteContracthandling.Inside
compactDeposit,transactionHashis already awaited (lines 37/47), sohash: await transactionHashon line 57 is a no-op. Compare withcompactWithdraw(line 102) andcompactApprove(line 141), wherewriteContractis intentionally not awaited and the resultingPromise<0x...>is consumed bywaitForTransactionReceiptviaawait transactionHash. Pick one pattern across the three methods to avoid future confusion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/libraries/compactLib.ts` around lines 35 - 58, In compactDeposit the code currently awaits walletClient.writeContract but then does await transactionHash again when calling getClient(...).waitForTransactionReceipt, causing inconsistency with compactWithdraw/compactApprove; make the pattern consistent by not awaiting walletClient.writeContract in compactDeposit (assign the Promise returned by walletClient.writeContract to transactionHash for both branches: depositNative and depositERC20) and keep the await when passing it into getClient(token.chainId).waitForTransactionReceipt({ hash: await transactionHash }); ensure you update the transactionHash handling around walletClient.writeContract, transactionHash variable usage, and leave depositNative/depositERC20, ADDRESS_ZERO, compactDeposit identifiers intact.
89-99: TODOs in withdrawal struct.
allocatorData: "0x"andexpires: maxInt32are placeholder values. WithmaxInt32(year 2038) the expiry effectively never times out for the foreseeable future, but the missing allocator data may produce invalid claims if a real allocator is configured. Please track these explicitly.Want me to open a follow-up issue tracking the allocator-data + expires wiring?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/libraries/compactLib.ts` around lines 89 - 99, The withdrawal struct currently uses placeholders (allocatorData: "0x", expires: maxInt32) which can produce invalid claims; update the code that constructs the withdrawal (refer to allocatorData, expires, maxInt32, nonce, assetId, recipients) to accept/derive real allocatorData and a proper expiry: either pass allocatorData and expires into the function that builds this withdrawal, read them from the configured allocator/contract config, validate they are non-empty and within allowed ranges, and throw or return an error if missing; also replace the TODOs with a short comment linking to a created follow-up issue tracking wiring if you cannot fully resolve it now.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/deploy.yml:
- Around line 52-59: The deploy-preview job should skip runs from forked PRs
because forks don't have access to repository secrets; update the job's if
expression on the deploy-preview job (and the other PR-only jobs around lines
118-129) to require the PR comes from the same repository by adding a check like
github.event.pull_request.head.repo.full_name == github.repository (e.g., change
the if to: github.event_name == 'pull_request' && github.event.action !=
'closed' && github.event.pull_request.head.repo.full_name == github.repository)
so preview/cleanup jobs only run for non-fork PRs.
In `@drizzle/meta/_journal.json`:
- Around line 4-12: The migration journal was rewritten to a single entry
"0000_brief_corsair" which will cause existing databases (with
__drizzle_migrations populated) to treat drizzle/0000_brief_corsair.sql as
unapplied and run SQL that starts with DROP TABLE IF EXISTS on
transaction_receipts, tokens, intents, and fill_transactions; to fix, either
restore the original journal entries into drizzle/meta/_journal.json so prior
migrations remain recorded, or mark this change as fresh-DB-only by reverting
the journal edit and instead adding a new forward-only migration that does not
drop existing tables; alternatively, modify drizzle/0000_brief_corsair.sql to
remove destructive DROP TABLE statements and make it idempotent (use ALTER TABLE
/ CREATE TABLE IF NOT EXISTS or safe checks) and ensure __drizzle_migrations
reflects the correct history before deploying to non-fresh environments.
In `@src/lib/components/BalanceField.svelte`:
- Around line 21-26: The input currently uses Number(value) / 10 ** decimals
which loses precision for large bigint balances; change the display logic in
BalanceField.svelte to treat value as a BigInt (e.g., valueBig = BigInt(value)),
compute integerPart = valueBig / (10n ** BigInt(decimals)) and fracPart =
valueBig % (10n ** BigInt(decimals)), pad and trim the fractional string to the
correct length, compose a single display string (handle zero/empty and sign if
needed), and set that string as the input's value instead of using
Number(value); make sure to also guard for null/undefined inputs and for
decimals being a number.
In `@src/lib/components/GetQuote.svelte`:
- Around line 94-119: The first $effect schedules a setTimeout that calls
updateQuote() but never clears it on teardown—capture the timeout id and return
a cleanup that calls clearTimeout(id); the online listener effect already
returns a remover so keep that pattern; for the rxjs interval logic around
counter/updater/quoteExpires, ensure each $effect that creates or reassigns
counter returns a teardown that calls counter.unsubscribe(), and move the
initial counter = interval(1000).subscribe(updater) into an effect (or at least
store and unsubscribe it on component teardown) so the interval is always
unsubscribed when the component unmounts; reference updateQuote, quoteExpires,
counter, interval, and updater when implementing these cleanups.
In `@src/lib/components/InputTokenModal.svelte`:
- Around line 86-99: Guard against empty currentInputTokens before dereferencing
it: initialize selectedTokenName using a safe fallback (e.g., empty string or a
first-available token if currentInputTokens.length > 0) instead of directly
using currentInputTokens[0].token.name, and update the tokenSet/$effect logic to
no-op or reset when currentInputTokens.length === 0; specifically modify the
selectedTokenName initialization and the $effect block that checks
currentInputTokens[0].token.name so they first check currentInputTokens.length
and only access currentInputTokens[0].token when non-empty, otherwise set inputs
and enabledByToken to empty objects (or appropriate defaults) and avoid using
iaddrFor on undefined tokens.
In `@src/lib/components/OutputTokenModal.svelte`:
- Around line 51-57: The add() function currently hard-codes the new output's
name to "usdc", which breaks when the selected chain doesn't expose that exact
token; change add() to look up tokens for the selected chain and seed the new
row with getTokensForChain(chainId)[0].name (or fallback to an empty string or
the first available property) when constructing the pushed object so new outputs
use a valid token for outputs[outputs.length - 1].chainId instead of "usdc".
- Around line 17-25: The outputs initialization in OutputTokenModal.svelte is
converting amounts to Number (outputs / currentOutputTokens mapping uses
Number(amount) / 10 ** token.decimals) causing precision loss; change it to
store amount as a decimal string instead (format the bigint base-unit amount
into a string using token.decimals) and remove the Number(...) division. Also
update the input handling that converts the edited HTML number back to base
units (the code that parses the edited amount and recreates the bigint) to
explicitly parse the decimal string and shift the decimal by token.decimals to
produce a bigint, rather than using floating-point math; reference
variables/functions: outputs, currentOutputTokens, token.decimals, and the code
path that parses the edited amount back into base units. Ensure all places
expecting amount adjust from number to string types.
In `@src/lib/components/ui/FormControl.svelte`:
- Around line 28-32: The component currently only sets visual classes via
stateClass when state === "disabled" but doesn't set the actual disabled HTML
attribute; update the component so the underlying form controls receive
disabled={state === "disabled"} by default (but allow callers to override via
{...rest} if desired). Concretely, locate the rendering of the input and select
elements in FormControl.svelte and add the disabled prop derived from state (use
state === "disabled") when spreading {...rest} — ensure you place disabled
before {...rest} if you want callers to be able to override, and keep stateClass
logic unchanged.
In `@src/lib/config.ts`:
- Around line 31-50: WORMHOLE_ORACLE currently maps ethereum/arbitrum/base to
the zero address which makes getOracle("wormhole", chainId) return a
false-positive configured address; either remove those zero-address entries from
WORMHOLE_ORACLE (delete the ethereum.arbitrum.base keys) or change getOracle to
treat the zero address as unconfigured by checking the value from
WORMHOLE_ORACLE (and comparing !== "0x0000000000000000000000000000000000000000")
and returning undefined for zero addresses so callers won't treat wormhole as
configured.
- Around line 467-473: The polygon public client is incorrectly initialized with
chain: base; update the createPublicClient call for the polygon entry to use the
correct Polygon chain object (replace chain: base with the Polygon chain
variable used elsewhere in the file) so that clients.polygon and the value
lifted into clientsById use the proper chain context; ensure the unique symbols
involved are the polygon entry passed to createPublicClient, the chain: base
setting, and the clients.polygon / clientsById mapping so the RPC URLs are
validated/encoded against Polygon (chainId 137) instead of Base.
In `@src/lib/libraries/coreDeps.ts`:
- Around line 28-38: The allowedInputOracles function drops sameChainFill when
no bridge oracle exists because it returns before appending COIN_FILLER; update
allowedInputOracles so that if sameChainFill is true you still return the
same-chain filler even when POLYMER_ORACLE[key] and WORMHOLE_ORACLE[key] are
missing—e.g. compute allowed from POLYMER_ORACLE and WORMHOLE_ORACLE (using
isNonZeroAddress), then if allowed is empty and sameChainFill return
[COIN_FILLER], or alternatively append COIN_FILLER before the empty-length early
return; reference function allowedInputOracles and symbols POLYMER_ORACLE,
WORMHOLE_ORACLE, COIN_FILLER, isNonZeroAddress.
- Around line 15-23: getOracle currently returns the oracle value from
POLYMER_ORACLE or WORMHOLE_ORACLE even when it is the wormhole zero-address
sentinel; update getOracle (the getOracle(verifier, chainId) function) to check
the resolved oracle string and return undefined if it equals the zero-address
sentinel (e.g. "0x0000000000000000000000000000000000000000", case-insensitive)
so it matches the behavior of allowedInputOracles/allowedOutputOracles and
prevents leaking a bogus oracle into intent creation.
In `@src/lib/libraries/intentExecution.ts`:
- Around line 167-192: The for-loop over components in finalise logic should be
replaced with direct handling of the first component since the loop always
returns on the first iteration; retrieve the first element (e.g., const {
orderComponent } = components[0]) and then branch on
intent.inputSettler.toLowerCase() against MULTICHAIN_INPUT_SETTLER_ESCROW and
MULTICHAIN_INPUT_SETTLER_COMPACT, calling walletClient.writeContract exactly as
currently done (use combineSignatures(signatures) for the COMPACT branch) and
throw a single informative Error if the settler type is unknown; also remove the
unreachable trailing throw that reports Failed to finalise multichain order on
chain.
In `@src/lib/libraries/intentFactory.ts`:
- Around line 30-55: The bug is that applySameChainTimings sets expiry and
fillDeadline to a duration value rather than an absolute Unix timestamp; change
applySameChainTimings to set both expiry and fillDeadline to
Math.floor(Date.now()/1000) + SAME_CHAIN_DURATION_SECONDS (i.e., current time
plus the duration) instead of SAME_CHAIN_DURATION_SECONDS. Also remove or
minimize unsafe (intent as any) writes: prefer using the Intent API
setters/factory methods or proper typed fields on intent (or expose a small
helper on Intent to set expiry/fillDeadline) so the code doesn’t rely on as any
and will fail loudly if the library adds readonly/relocated fields. Ensure
parity with applyExclusivityOverride’s use of currentTime + duration.
---
Minor comments:
In `@playwright.config.ts`:
- Line 16: The video setting currently uses a truthy check on
process.env.PW_VIDEO_ALL which treats values like "0" or "false" as true; update
the video assignment in the Playwright config (the video property in
playwright.config.ts) to parse the env explicitly—e.g., check
process.env.PW_VIDEO_ALL === "1" or === "true" (or use a small helper like
isEnvTrue) so only explicit truthy strings enable "on", otherwise use
"retain-on-failure".
In `@src/lib/abi/escrow.ts`:
- Around line 1231-1262: The ABI contains a duplicated error definition for
FillDeadlineAfterExpiry; remove the second identical error object so only one
declaration of the error named "FillDeadlineAfterExpiry" with the two uint32
inputs remains in the exported ABI (search for the error object with name
"FillDeadlineAfterExpiry" in the escrow ABI array and delete the duplicate
entry, ensuring no other duplicates are present).
In `@src/lib/components/AwaitButton.svelte`:
- Around line 45-48: defaultLazy is currently a plain const and captures
variant's initial value; make it reactive like defaultBase/defaultHover by
wrapping it in a Svelte $: or using derived reactive declaration so it
recomputes when the variant prop changes. Update the declaration of defaultLazy
(referencing defaultLazy and variant) to a reactive derived value that returns
["cursor-not-allowed", variant === "muted" ? "text-gray-500" : "text-gray-400"]
so the text color updates when variant changes.
In `@src/lib/components/InputTokenModal.svelte`:
- Around line 13-14: formatBalance currently calls Number(value) which loses
precision for bigints > Number.MAX_SAFE_INTEGER; change formatBalance to do
integer/fractional bigint math: compute integerPart = value /
10n**BigInt(decimals) and fracPart = value % 10n**BigInt(decimals), build
fracStr by left-padding fracPart.toString() to decimals and trimming trailing
zeros, format integerPart as a thousands-separated string (operate on
integerPart.toString() and insert separators) and return combined "integer.frac"
(or just integer if fracStr is empty); update any uses such as metaText to rely
on the new function.
In `@src/lib/components/Introduction.svelte`:
- Around line 7-77: Update the user-facing copy in Introduction.svelte: change
"support" to "supports" in the sentence containing "It currently support a
seamless resource lock flow", change "work in progress" to "a work in progress"
where "Multichain intents are currently work in progress", change "transaction
are" to "transactions are" in the list item "No upfront deposit or initiation
transaction are required.", change "configured different" to "configured
differently" in the Same Chain paragraph, and fix "Learm" to "Learn" in the
trailing sentence; locate these phrases in the component's paragraph and heading
nodes and apply the exact word replacements to correct grammar/typos.
In `@src/lib/components/ui/FlowProgressList.svelte`:
- Around line 85-106: The connector fill is anchored to the container corner so
it doesn’t align with the centered track; update the .connector-fill positioning
so it is centered along the orthogonal axis for each orientation: in the base
rule (horizontal track) keep width: var(--fill-pct) and height: 2px but replace
top:0 left:0 with top:50% and transform: translateY(-50%) so the 2px bar is
vertically centered; in the `@media` (min-width: 768px) override (vertical track)
keep width: 2px and height: var(--fill-pct) but replace top/left with left:50%
and transform: translateX(-50%) so the 2px column is horizontally centered; this
will ensure the fill rendered by connectorFillPercent(i) lines up with the
centered track.
In `@src/lib/components/ui/FlowStepTracker.svelte`:
- Around line 59-73: The promise from getOrderProgressChecks can still resolve
after the component unmounts and mutate flowChecks because flowChecksRun isn't
advanced on cleanup; update the effect that calls getOrderProgressChecks (the
block referencing flowChecksRun, getOrderProgressChecks, selectedOrder, and
store.fillTransactions) to increment flowChecksRun in the effect's
cleanup/unmount handler so any in-flight callbacks see a stale currentRun and
early-return instead of writing to flowChecks.
In `@src/lib/components/ui/InlineMetaField.svelte`:
- Around line 16-22: Remove the uncontrolled defaultValue from the <input> (it
conflicts with bind:value) and explicitly handle null/undefined in the bound
variable used by bind:value inside InlineMetaField.svelte: delete the
defaultValue attribute and either initialize or coerce the bound prop/variable
(the one referenced by bind:value) to a safe display value (e.g. '' for empty or
0 if you want a numeric default) via an initializer or a small reactive
statement (e.g. $: safeValue = value ?? '' and bind to that or coerce on input),
or use a placeholder for visual hinting instead of defaultValue so the number
input behavior is deterministic.
In `@src/lib/components/ui/ScreenFrame.svelte`:
- Around line 11-12: The props title and description in ScreenFrame.svelte are
declared as required (title: string; description: string;) but are conditionally
used in the template, so update their typings to be optional (title?: string;
description?: string;) and apply the same optional change to the other prop
declarations around the 20–25 region that mirror these lines; ensure any related
methods or consumers handle possibly undefined values (e.g., keep the existing
{`#if` title} / {`#if` description} guards).
In `@src/lib/config.ts`:
- Around line 348-391: getCoin currently validates the chain via
isChainIdTestnet but then silently returns a fabricated token with decimals: 1
when a token is not found, causing downstream formatting errors; make the
behavior consistent by first normalizing/validating the chain (reuse
normalizeChainId and call isChainIdTestnet to surface unknown-chain errors) and
then, if no matching token from coinList(...) is found, throw a clear Error
(e.g. "Token not found for name/address on chain <chainId>") instead of
returning a bogus token with decimals: 1 so callers of
formatTokenAmount/summarizeInput cannot silently mis-format values. Ensure you
reference getCoin, normalizeChainId, isChainIdTestnet and coinList when making
these changes.
In `@src/lib/libraries/flowProgress.ts`:
- Around line 157-168: The code calls inputChainClient.readContract(... args:
[flattenedInputs[0][0]]) without checking that flattenedInputs[0] is a non-empty
array; add a guard before the getOrFetchRpc call (return false) when
flattenedInputs[0] is missing or has length 0 so getLockDetails never receives
undefined. Locate the usage of flattenedInputs in the function (the
flattenedInputs assignment and the getOrFetchRpc block) and ensure you check
flattenedInputs[0] && flattenedInputs[0].length > 0 (or equivalent) before
proceeding to call inputChainClient.readContract/getLockDetails.
In `@src/lib/libraries/intentList.ts`:
- Around line 77-96: summarizeInput and summarizeOutput call getCoin and always
format amounts, but getCoin's unknown-token fallback ({name: "Unknown",
decimals: 1}) causes wrong scaling; update both functions to detect the unknown
fallback (e.g., if coin.name === "Unknown" or otherwise indicates fallback) and
short-circuit to the raw-address branch (use shortAddress(tokenAddress) in
summarizeInput and shortAddress(token) in summarizeOutput and return the
`${amount.toString()} ... on chain-${chainId.toString()}` style string) instead
of calling formatTokenAmount; keep existing chainName handling and only format
when a real coin is returned.
---
Nitpick comments:
In `@drizzle/0000_brief_corsair.sql`:
- Around line 1-8: This migration currently unconditionally drops local
IndexedDB tables transaction_receipts, tokens, intents, and fill_transactions
which will wipe each user's client-side cache; either remove the DROP TABLE IF
EXISTS statements so the migration becomes additive (keep the incremental
migrations instead of consolidating 0000-0004) or replace each DROP with a
safe-preserve strategy: check for existing data in the respective table and only
perform schema-only changes (or create tables with CREATE TABLE IF NOT EXISTS)
so existing IndexedDB data isn't removed; update the 0000_brief_corsair.sql
migration to implement one of these two approaches and ensure the statements
referencing "transaction_receipts", "tokens", "intents", and "fill_transactions"
are adjusted accordingly.
In `@src/lib/abi/wormholeoracle.ts`:
- Line 1: The exported identifier is misspelled as WROMHOLE_ORACLE_ABI; rename
it to WORMHOLE_ORACLE_ABI in the declaration (export const WROMHOLE_ORACLE_ABI
-> export const WORMHOLE_ORACLE_ABI) and update any local references or
imports/usages to the new name to keep the module API consistent (search for
WROMHOLE_ORACLE_ABI and replace with WORMHOLE_ORACLE_ABI).
In `@src/lib/components/AwaitButton.svelte`:
- Around line 50-55: The run function currently logs and re-throws errors from
buttonFunction but doesn't surface them to the component consumer or UI; add an
optional onError prop (e.g., export let onError: (err) => void) and call it
inside the catch handler for buttonPromise (reference run, buttonPromise,
buttonFunction) so callers can react; also consider storing the error in a local
exported/state variable that the {:catch} block can display instead of only
logging, and remove the needless re-throw if you want the component to handle
the error locally.
In `@src/lib/components/GetQuote.svelte`:
- Line 28: The current use of $derived(new IntentApi(useProductionApi ??
mainnet)) recreates an IntentApi on every reactive read; instead, stop
constructing IntentApi inside $derived and instantiate it once when the env
flags change: create a let intentApi variable and set it inside an $: or $effect
reactive block keyed on useProductionApi and mainnet (e.g., $: if
(useProductionApi !== undefined || mainnet !== undefined) intentApi = new
IntentApi(useProductionApi ?? mainnet)), so IntentApi (class name IntentApi) is
only re-created when those inputs change and not on every reactive read.
In `@src/lib/components/ui/FlowProgressList.svelte`:
- Around line 71-73: Add an explicit aria-current indicator for the active step
so screen readers know which is current: in FlowProgressList.svelte update the
element that renders each step (the same element that sets title, aria-label and
onclick for the local variable step) to include an aria-current attribute when
step represents the active state (e.g., aria-current="step" only when
step.status equals the active status value used in this component); ensure the
attribute is omitted or falsy for non-active steps so it isn't present
erroneously.
In `@src/lib/components/ui/FormControl.svelte`:
- Around line 4-24: The props type currently contains a broad index signature
`[key: string]: unknown` which weakens type checking; remove that index
signature and instead type the rest-payload explicitly (e.g., add a `rest?:
Record<string, unknown>` entry in the props type) while keeping the
destructuring `...rest` in FormControl.svelte; specifically, edit the prop type
for the let-block (the destructured object containing as, value, type, size,
density, state, className, children) to drop `[key: string]: unknown` and add a
named `rest?: Record<string, unknown>` (or another narrow type) so callers get
stricter prop checks while `...rest` still captures passthrough attributes.
In `@src/lib/components/ui/SegmentedControl.svelte`:
- Around line 26-41: The SegmentedControl buttons lack accessible state
semantics: update the button in SegmentedControl.svelte (the element using
value, option.value, testIdPrefix, and onclick/onChange) to include an
aria-pressed attribute set to the active check (aria-pressed={value ===
option.value}) so assistive tech can identify the selected segment; ensure the
expression uses the same reactive/value variables and does not alter existing
onclick/onChange behavior or testId usage.
In `@src/lib/libraries/compactLib.ts`:
- Around line 35-58: In compactDeposit the code currently awaits
walletClient.writeContract but then does await transactionHash again when
calling getClient(...).waitForTransactionReceipt, causing inconsistency with
compactWithdraw/compactApprove; make the pattern consistent by not awaiting
walletClient.writeContract in compactDeposit (assign the Promise returned by
walletClient.writeContract to transactionHash for both branches: depositNative
and depositERC20) and keep the await when passing it into
getClient(token.chainId).waitForTransactionReceipt({ hash: await transactionHash
}); ensure you update the transactionHash handling around
walletClient.writeContract, transactionHash variable usage, and leave
depositNative/depositERC20, ADDRESS_ZERO, compactDeposit identifiers intact.
- Around line 89-99: The withdrawal struct currently uses placeholders
(allocatorData: "0x", expires: maxInt32) which can produce invalid claims;
update the code that constructs the withdrawal (refer to allocatorData, expires,
maxInt32, nonce, assetId, recipients) to accept/derive real allocatorData and a
proper expiry: either pass allocatorData and expires into the function that
builds this withdrawal, read them from the configured allocator/contract config,
validate they are non-empty and within allowed ranges, and throw or return an
error if missing; also replace the TODOs with a short comment linking to a
created follow-up issue tracking wiring if you cannot fully resolve it now.
In `@src/lib/libraries/intentFactory.ts`:
- Around line 254-258: Remove the commented-out receipt-wait loop that
references transactionHashes and clients[inputChain].waitForTransactionReceipt
in intentFactory.ts; either delete those lines entirely to eliminate dead code,
or replace them with a one-line comment explaining why waiting for receipts is
intentionally skipped (e.g., "Receipt wait deferred to caller" or similar) so
future readers know this behavior is intentional; ensure references to
transactionHashes, clients, waitForTransactionReceipt, and inputChain remain
clear in the comment if kept.
- Around line 210-214: The await on
getClient(...).waitForTransactionReceipt(...) assigns an unused variable
receipt; either drop the binding to simply await completion or explicitly check
receipt.status to assert success. Locate the call after
depositAndRegisterCompact in intentFactory (the lines using transactionHash and
inputTokens[0].token.chainId), and replace the unused "const receipt = await
getClient(...).waitForTransactionReceipt(...)" with either "await
getClient(...).waitForTransactionReceipt(...)" if only waiting is needed, or
keep the binding and add a status check on receipt.status (throw or handle
non-success) to verify the transaction succeeded.
In `@src/lib/libraries/intentList.ts`:
- Around line 1-14: Consolidate the multiple imports from "@lifi/intent" into a
single grouped import to improve readability: replace the separate import lines
that bring in orderToIntent, bytes32ToAddress, idToToken, OrderContainer,
StandardOrder, MultichainOrder, and validateOrderContainerWithReason with one
import statement from "@lifi/intent" that lists all those symbols together;
leave the unrelated import (orderValidationDeps) from "./coreDeps" and the other
config imports unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+4
to
+12
| "entries": [ | ||
| { | ||
| "idx": 0, | ||
| "version": "7", | ||
| "when": 1773159031946, | ||
| "tag": "0000_brief_corsair", | ||
| "breakpoints": true | ||
| } | ||
| ] |
There was a problem hiding this comment.
Migration history was rewritten — confirm fresh-DB-only deployment.
The entries array has been reduced to a single new entry (0000_brief_corsair, ts 1773159031946), discarding all prior migration records. Any environment whose __drizzle_migrations table was populated from the old journal will now see this as an unapplied migration and execute drizzle/0000_brief_corsair.sql, which begins with DROP TABLE IF EXISTS on transaction_receipts, tokens, intents, and fill_transactions (data loss). See the related comment on drizzle/0000_brief_corsair.sql for the root cause and remediation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@drizzle/meta/_journal.json` around lines 4 - 12, The migration journal was
rewritten to a single entry "0000_brief_corsair" which will cause existing
databases (with __drizzle_migrations populated) to treat
drizzle/0000_brief_corsair.sql as unapplied and run SQL that starts with DROP
TABLE IF EXISTS on transaction_receipts, tokens, intents, and fill_transactions;
to fix, either restore the original journal entries into
drizzle/meta/_journal.json so prior migrations remain recorded, or mark this
change as fresh-DB-only by reverting the journal edit and instead adding a new
forward-only migration that does not drop existing tables; alternatively, modify
drizzle/0000_brief_corsair.sql to remove destructive DROP TABLE statements and
make it idempotent (use ALTER TABLE / CREATE TABLE IF NOT EXISTS or safe checks)
and ensure __drizzle_migrations reflects the correct history before deploying to
non-fresh environments.
Comment on lines
+21
to
+26
| <input | ||
| type="text" | ||
| class="w-20 cursor-not-allowed rounded border border-gray-700 bg-gray-50 px-2 py-1 text-gray-600" | ||
| disabled | ||
| value={Number(value) / 10 ** decimals} | ||
| /> |
There was a problem hiding this comment.
Avoid precision loss when formatting bigint balances (Line 25).
Number(value) can truncate large bigint values, so displayed balances may be incorrect for high balances/tokens with many decimals.
💡 BigInt-safe formatting diff
<script lang="ts">
import { toBigIntWithDecimals } from "@lifi/intent";
@@
let {
value,
decimals
}: {
value: Promise<bigint>;
decimals: number;
} = $props();
+
+ function formatUnitsSafe(amount: bigint, decimals: number): string {
+ if (decimals <= 0) return amount.toString();
+ const negative = amount < 0n;
+ const abs = negative ? -amount : amount;
+ const base = 10n ** BigInt(decimals);
+ const whole = abs / base;
+ const fraction = (abs % base).toString().padStart(decimals, "0").replace(/0+$/, "");
+ return `${negative ? "-" : ""}${whole}${fraction ? `.${fraction}` : ""}`;
+ }
</script>
@@
{:then value}
<input
@@
- value={Number(value) / 10 ** decimals}
+ value={formatUnitsSafe(value, decimals)}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <input | |
| type="text" | |
| class="w-20 cursor-not-allowed rounded border border-gray-700 bg-gray-50 px-2 py-1 text-gray-600" | |
| disabled | |
| value={Number(value) / 10 ** decimals} | |
| /> | |
| <script lang="ts"> | |
| import { toBigIntWithDecimals } from "@lifi/intent"; | |
| let { | |
| value, | |
| decimals | |
| }: { | |
| value: Promise<bigint>; | |
| decimals: number; | |
| } = $props(); | |
| function formatUnitsSafe(amount: bigint, decimals: number): string { | |
| if (decimals <= 0) return amount.toString(); | |
| const negative = amount < 0n; | |
| const abs = negative ? -amount : amount; | |
| const base = 10n ** BigInt(decimals); | |
| const whole = abs / base; | |
| const fraction = (abs % base).toString().padStart(decimals, "0").replace(/0+$/, ""); | |
| return `${negative ? "-" : ""}${whole}${fraction ? `.${fraction}` : ""}`; | |
| } | |
| </script> | |
| {`#await` value} | |
| <!-- Loading state --> | |
| {:then value} | |
| <input | |
| type="text" | |
| class="w-20 cursor-not-allowed rounded border border-gray-700 bg-gray-50 px-2 py-1 text-gray-600" | |
| disabled | |
| value={formatUnitsSafe(value, decimals)} | |
| /> | |
| {/await} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/components/BalanceField.svelte` around lines 21 - 26, The input
currently uses Number(value) / 10 ** decimals which loses precision for large
bigint balances; change the display logic in BalanceField.svelte to treat value
as a BigInt (e.g., valueBig = BigInt(value)), compute integerPart = valueBig /
(10n ** BigInt(decimals)) and fracPart = valueBig % (10n ** BigInt(decimals)),
pad and trim the fractional string to the correct length, compose a single
display string (handle zero/empty and sign if needed), and set that string as
the input's value instead of using Number(value); make sure to also guard for
null/undefined inputs and for decimals being a number.
Comment on lines
+94
to
+119
| $effect(() => { | ||
| mainnet; | ||
| setTimeout(() => { | ||
| updateQuote(); | ||
| }, 1000); | ||
| }); | ||
|
|
||
| $effect(() => { | ||
| if (typeof window === "undefined") return; | ||
| const onOnline = () => updateQuote(); | ||
| window.addEventListener("online", onOnline); | ||
| return () => window.removeEventListener("online", onOnline); | ||
| }); | ||
| $effect(() => { | ||
| if (typeof window === "undefined") return; | ||
| const onOnline = () => updateQuote(); | ||
| window.addEventListener("online", onOnline); | ||
| return () => window.removeEventListener("online", onOnline); | ||
| }); | ||
|
|
||
| $effect(() => { | ||
| quoteExpires; | ||
| if (quoteExpires === 0) { | ||
| width = 0; | ||
| counter.unsubscribe(); | ||
| return; | ||
| } | ||
| counter.unsubscribe(); | ||
| counter = interval(1000).subscribe(updater); | ||
| }); | ||
| let quoteDuration = 30 * 1000; | ||
| let counter = interval(1000).subscribe(updater); | ||
| $effect(() => { | ||
| quoteExpires; | ||
| if (quoteExpires === 0) { | ||
| width = 0; | ||
| counter.unsubscribe(); | ||
| return; | ||
| } | ||
| counter.unsubscribe(); | ||
| counter = interval(1000).subscribe(updater); | ||
| }); | ||
| let quoteDuration = 30 * 1000; | ||
| let counter = interval(1000).subscribe(updater); |
There was a problem hiding this comment.
Missing cleanup for setTimeout and the rxjs interval subscription.
- The
setTimeoutat line 96 is never cleared; if the component unmounts within the 1s window it still firesupdateQuote()against stale state. counter = interval(1000).subscribe(updater)is created at module init (line 119) and reassigned by thequoteExpireseffect, but neither path returns a teardown. After unmount the subscription keeps polling and re-triggersupdateQuote()indefinitely, leaking timers and (intermittently) network calls.
Return cleanup functions from the $effects and remember to clear the setTimeout.
🧹 Suggested cleanup
$effect(() => {
mainnet;
- setTimeout(() => {
+ const t = setTimeout(() => {
updateQuote();
}, 1000);
+ return () => clearTimeout(t);
});
@@
$effect(() => {
quoteExpires;
if (quoteExpires === 0) {
width = 0;
counter.unsubscribe();
return;
}
counter.unsubscribe();
counter = interval(1000).subscribe(updater);
+ return () => counter.unsubscribe();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $effect(() => { | |
| mainnet; | |
| setTimeout(() => { | |
| updateQuote(); | |
| }, 1000); | |
| }); | |
| $effect(() => { | |
| if (typeof window === "undefined") return; | |
| const onOnline = () => updateQuote(); | |
| window.addEventListener("online", onOnline); | |
| return () => window.removeEventListener("online", onOnline); | |
| }); | |
| $effect(() => { | |
| if (typeof window === "undefined") return; | |
| const onOnline = () => updateQuote(); | |
| window.addEventListener("online", onOnline); | |
| return () => window.removeEventListener("online", onOnline); | |
| }); | |
| $effect(() => { | |
| quoteExpires; | |
| if (quoteExpires === 0) { | |
| width = 0; | |
| counter.unsubscribe(); | |
| return; | |
| } | |
| counter.unsubscribe(); | |
| counter = interval(1000).subscribe(updater); | |
| }); | |
| let quoteDuration = 30 * 1000; | |
| let counter = interval(1000).subscribe(updater); | |
| $effect(() => { | |
| quoteExpires; | |
| if (quoteExpires === 0) { | |
| width = 0; | |
| counter.unsubscribe(); | |
| return; | |
| } | |
| counter.unsubscribe(); | |
| counter = interval(1000).subscribe(updater); | |
| }); | |
| let quoteDuration = 30 * 1000; | |
| let counter = interval(1000).subscribe(updater); | |
| $effect(() => { | |
| mainnet; | |
| const t = setTimeout(() => { | |
| updateQuote(); | |
| }, 1000); | |
| return () => clearTimeout(t); | |
| }); | |
| $effect(() => { | |
| if (typeof window === "undefined") return; | |
| const onOnline = () => updateQuote(); | |
| window.addEventListener("online", onOnline); | |
| return () => window.removeEventListener("online", onOnline); | |
| }); | |
| $effect(() => { | |
| quoteExpires; | |
| if (quoteExpires === 0) { | |
| width = 0; | |
| counter.unsubscribe(); | |
| return; | |
| } | |
| counter.unsubscribe(); | |
| counter = interval(1000).subscribe(updater); | |
| return () => counter.unsubscribe(); | |
| }); | |
| let quoteDuration = 30 * 1000; | |
| let counter = interval(1000).subscribe(updater); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/components/GetQuote.svelte` around lines 94 - 119, The first $effect
schedules a setTimeout that calls updateQuote() but never clears it on
teardown—capture the timeout id and return a cleanup that calls
clearTimeout(id); the online listener effect already returns a remover so keep
that pattern; for the rxjs interval logic around counter/updater/quoteExpires,
ensure each $effect that creates or reassigns counter returns a teardown that
calls counter.unsubscribe(), and move the initial counter =
interval(1000).subscribe(updater) into an effect (or at least store and
unsubscribe it on component teardown) so the interval is always unsubscribed
when the component unmounts; reference updateQuote, quoteExpires, counter,
interval, and updater when implementing these cleanups.
Comment on lines
+86
to
+99
| let selectedTokenName = $state<string>(currentInputTokens[0].token.name); | ||
| const tokenSet = $derived( | ||
| store.availableTokens.filter((v) => v.name.toLowerCase() === selectedTokenName.toLowerCase()) | ||
| ); | ||
|
|
||
| let circuitBreaker = false; | ||
| $effect(() => { | ||
| selectedTokenName; | ||
| if (circuitBreaker || currentInputTokens[0].token.name !== selectedTokenName) { | ||
| circuitBreaker = true; | ||
| inputs = Object.fromEntries(tokenSet.map((token) => [iaddrFor(token), 0])); | ||
| enabledByToken = Object.fromEntries(tokenSet.map((token) => [iaddrFor(token), true])); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Crash on empty currentInputTokens.
Lines 86 and 94 dereference currentInputTokens[0].token without a length check, even though the surrounding initializers (lines 24–39) defensively coalesce to []. If the parent ever passes (or briefly transitions through) an empty array, both the initial selectedTokenName initialization and the $effect will throw.
🛡️ Suggested guard
- // svelte-ignore state_referenced_locally
- let selectedTokenName = $state<string>(currentInputTokens[0].token.name);
+ // svelte-ignore state_referenced_locally
+ let selectedTokenName = $state<string>(currentInputTokens[0]?.token.name ?? "");
const tokenSet = $derived(
store.availableTokens.filter((v) => v.name.toLowerCase() === selectedTokenName.toLowerCase())
);
let circuitBreaker = false;
$effect(() => {
selectedTokenName;
- if (circuitBreaker || currentInputTokens[0].token.name !== selectedTokenName) {
+ if (circuitBreaker || currentInputTokens[0]?.token.name !== selectedTokenName) {
circuitBreaker = true;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let selectedTokenName = $state<string>(currentInputTokens[0].token.name); | |
| const tokenSet = $derived( | |
| store.availableTokens.filter((v) => v.name.toLowerCase() === selectedTokenName.toLowerCase()) | |
| ); | |
| let circuitBreaker = false; | |
| $effect(() => { | |
| selectedTokenName; | |
| if (circuitBreaker || currentInputTokens[0].token.name !== selectedTokenName) { | |
| circuitBreaker = true; | |
| inputs = Object.fromEntries(tokenSet.map((token) => [iaddrFor(token), 0])); | |
| enabledByToken = Object.fromEntries(tokenSet.map((token) => [iaddrFor(token), true])); | |
| } | |
| }); | |
| let selectedTokenName = $state<string>(currentInputTokens[0]?.token.name ?? ""); | |
| const tokenSet = $derived( | |
| store.availableTokens.filter((v) => v.name.toLowerCase() === selectedTokenName.toLowerCase()) | |
| ); | |
| let circuitBreaker = false; | |
| $effect(() => { | |
| selectedTokenName; | |
| if (circuitBreaker || currentInputTokens[0]?.token.name !== selectedTokenName) { | |
| circuitBreaker = true; | |
| inputs = Object.fromEntries(tokenSet.map((token) => [iaddrFor(token), 0])); | |
| enabledByToken = Object.fromEntries(tokenSet.map((token) => [iaddrFor(token), true])); | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/components/InputTokenModal.svelte` around lines 86 - 99, Guard
against empty currentInputTokens before dereferencing it: initialize
selectedTokenName using a safe fallback (e.g., empty string or a first-available
token if currentInputTokens.length > 0) instead of directly using
currentInputTokens[0].token.name, and update the tokenSet/$effect logic to no-op
or reset when currentInputTokens.length === 0; specifically modify the
selectedTokenName initialization and the $effect block that checks
currentInputTokens[0].token.name so they first check currentInputTokens.length
and only access currentInputTokens[0].token when non-empty, otherwise set inputs
and enabledByToken to empty objects (or appropriate defaults) and avoid using
iaddrFor on undefined tokens.
Comment on lines
+467
to
+473
| polygon: createPublicClient({ | ||
| chain: base, | ||
| transport: fallback([ | ||
| http("https://polygon-bor-rpc.publicnode.com"), | ||
| ...polygon.rpcUrls.default.http.map((v) => http(v)) | ||
| ]) | ||
| }), |
There was a problem hiding this comment.
Critical: polygon public client is wired to the base chain.
createPublicClient({ chain: base, ... }) for the polygon entry is almost certainly a copy-paste mistake — RPC URLs target Polygon but viem will validate/encode against base (chainId 8453 vs 137). Any contract read/write through clients.polygon (and through clientsById[polygon.id] since you lift the same value into the id-keyed map at line 525) will use the wrong chain context. This breaks polygon support entirely.
🐛 Fix
polygon: createPublicClient({
- chain: base,
+ chain: polygon,
transport: fallback([
http("https://polygon-bor-rpc.publicnode.com"),
...polygon.rpcUrls.default.http.map((v) => http(v))
])
}),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| polygon: createPublicClient({ | |
| chain: base, | |
| transport: fallback([ | |
| http("https://polygon-bor-rpc.publicnode.com"), | |
| ...polygon.rpcUrls.default.http.map((v) => http(v)) | |
| ]) | |
| }), | |
| polygon: createPublicClient({ | |
| chain: polygon, | |
| transport: fallback([ | |
| http("https://polygon-bor-rpc.publicnode.com"), | |
| ...polygon.rpcUrls.default.http.map((v) => http(v)) | |
| ]) | |
| }), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/config.ts` around lines 467 - 473, The polygon public client is
incorrectly initialized with chain: base; update the createPublicClient call for
the polygon entry to use the correct Polygon chain object (replace chain: base
with the Polygon chain variable used elsewhere in the file) so that
clients.polygon and the value lifted into clientsById use the proper chain
context; ensure the unique symbols involved are the polygon entry passed to
createPublicClient, the chain: base setting, and the clients.polygon /
clientsById mapping so the RPC URLs are validated/encoded against Polygon
(chainId 137) instead of Base.
Comment on lines
+15
to
+23
| getOracle(verifier, chainId) { | ||
| const key = Number(chainId); | ||
| if (!Number.isFinite(key)) return undefined; | ||
| if (verifier === "polymer") return POLYMER_ORACLE[key]; | ||
| if (verifier === "wormhole") { | ||
| return WORMHOLE_ORACLE[key]; | ||
| } | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
Filter out the wormhole zero-address sentinel here too.
allowedInputOracles and allowedOutputOracles already treat 0x000…000 as unsupported, but getOracle() returns that value unchanged. On unsupported chains this can leak a bogus oracle address into intent creation and fail later.
Suggested fix
getOracle(verifier, chainId) {
const key = Number(chainId);
if (!Number.isFinite(key)) return undefined;
if (verifier === "polymer") return POLYMER_ORACLE[key];
if (verifier === "wormhole") {
- return WORMHOLE_ORACLE[key];
+ const wormhole = WORMHOLE_ORACLE[key];
+ return isNonZeroAddress(wormhole) ? wormhole : undefined;
}
return undefined;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| getOracle(verifier, chainId) { | |
| const key = Number(chainId); | |
| if (!Number.isFinite(key)) return undefined; | |
| if (verifier === "polymer") return POLYMER_ORACLE[key]; | |
| if (verifier === "wormhole") { | |
| return WORMHOLE_ORACLE[key]; | |
| } | |
| return undefined; | |
| } | |
| getOracle(verifier, chainId) { | |
| const key = Number(chainId); | |
| if (!Number.isFinite(key)) return undefined; | |
| if (verifier === "polymer") return POLYMER_ORACLE[key]; | |
| if (verifier === "wormhole") { | |
| const wormhole = WORMHOLE_ORACLE[key]; | |
| return isNonZeroAddress(wormhole) ? wormhole : undefined; | |
| } | |
| return undefined; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/libraries/coreDeps.ts` around lines 15 - 23, getOracle currently
returns the oracle value from POLYMER_ORACLE or WORMHOLE_ORACLE even when it is
the wormhole zero-address sentinel; update getOracle (the getOracle(verifier,
chainId) function) to check the resolved oracle string and return undefined if
it equals the zero-address sentinel (e.g.
"0x0000000000000000000000000000000000000000", case-insensitive) so it matches
the behavior of allowedInputOracles/allowedOutputOracles and prevents leaking a
bogus oracle into intent creation.
Comment on lines
+28
to
+38
| allowedInputOracles({ chainId, sameChainFill }) { | ||
| const key = Number(chainId); | ||
| if (!Number.isFinite(key)) return undefined; | ||
| const polymer = POLYMER_ORACLE[key]; | ||
| const wormhole = WORMHOLE_ORACLE[key]; | ||
| const allowed: `0x${string}`[] = []; | ||
| if (polymer) allowed.push(polymer); | ||
| if (isNonZeroAddress(wormhole)) allowed.push(wormhole); | ||
| if (allowed.length === 0) return undefined; | ||
| if (sameChainFill) allowed.push(COIN_FILLER); | ||
| return allowed; |
There was a problem hiding this comment.
sameChainFill currently gets dropped when no bridge oracle is configured.
Line 36 returns before COIN_FILLER is appended, so a same-chain order on a chain without Polymer/Wormhole config is rejected instead of falling back to the same-chain filler.
Suggested fix
allowedInputOracles({ chainId, sameChainFill }) {
const key = Number(chainId);
if (!Number.isFinite(key)) return undefined;
const polymer = POLYMER_ORACLE[key];
const wormhole = WORMHOLE_ORACLE[key];
const allowed: `0x${string}`[] = [];
if (polymer) allowed.push(polymer);
if (isNonZeroAddress(wormhole)) allowed.push(wormhole);
- if (allowed.length === 0) return undefined;
if (sameChainFill) allowed.push(COIN_FILLER);
+ if (allowed.length === 0) return undefined;
return allowed;
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| allowedInputOracles({ chainId, sameChainFill }) { | |
| const key = Number(chainId); | |
| if (!Number.isFinite(key)) return undefined; | |
| const polymer = POLYMER_ORACLE[key]; | |
| const wormhole = WORMHOLE_ORACLE[key]; | |
| const allowed: `0x${string}`[] = []; | |
| if (polymer) allowed.push(polymer); | |
| if (isNonZeroAddress(wormhole)) allowed.push(wormhole); | |
| if (allowed.length === 0) return undefined; | |
| if (sameChainFill) allowed.push(COIN_FILLER); | |
| return allowed; | |
| allowedInputOracles({ chainId, sameChainFill }) { | |
| const key = Number(chainId); | |
| if (!Number.isFinite(key)) return undefined; | |
| const polymer = POLYMER_ORACLE[key]; | |
| const wormhole = WORMHOLE_ORACLE[key]; | |
| const allowed: `0x${string}`[] = []; | |
| if (polymer) allowed.push(polymer); | |
| if (isNonZeroAddress(wormhole)) allowed.push(wormhole); | |
| if (sameChainFill) allowed.push(COIN_FILLER); | |
| if (allowed.length === 0) return undefined; | |
| return allowed; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/libraries/coreDeps.ts` around lines 28 - 38, The allowedInputOracles
function drops sameChainFill when no bridge oracle exists because it returns
before appending COIN_FILLER; update allowedInputOracles so that if
sameChainFill is true you still return the same-chain filler even when
POLYMER_ORACLE[key] and WORMHOLE_ORACLE[key] are missing—e.g. compute allowed
from POLYMER_ORACLE and WORMHOLE_ORACLE (using isNonZeroAddress), then if
allowed is empty and sameChainFill return [COIN_FILLER], or alternatively append
COIN_FILLER before the empty-length early return; reference function
allowedInputOracles and symbols POLYMER_ORACLE, WORMHOLE_ORACLE, COIN_FILLER,
isNonZeroAddress.
Comment on lines
+167
to
+192
| for (const { orderComponent } of components) { | ||
| if (intent.inputSettler.toLowerCase() === MULTICHAIN_INPUT_SETTLER_ESCROW.toLowerCase()) { | ||
| return walletClient.writeContract({ | ||
| chain: actionChain, | ||
| account, | ||
| address: intent.inputSettler, | ||
| abi: MULTICHAIN_SETTLER_ESCROW_ABI, | ||
| functionName: "finalise", | ||
| args: [orderComponent, solveParams, addressToBytes32(account), "0x"] | ||
| }); | ||
| } | ||
| if (intent.inputSettler.toLowerCase() === MULTICHAIN_INPUT_SETTLER_COMPACT.toLowerCase()) { | ||
| const combinedSignatures = combineSignatures(signatures); | ||
| return walletClient.writeContract({ | ||
| chain: actionChain, | ||
| account, | ||
| address: intent.inputSettler, | ||
| abi: MULTICHAIN_SETTLER_COMPACT_ABI, | ||
| functionName: "finalise", | ||
| args: [orderComponent, combinedSignatures, solveParams, addressToBytes32(account), "0x"] | ||
| }); | ||
| } | ||
| throw new Error(`Could not detect settler type ${intent.inputSettler}`); | ||
| } | ||
|
|
||
| throw new Error(`Failed to finalise multichain order on chain ${actionChain.id}.`); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm no caller relies on multiple finalise tx hashes per call (the loop never could return them).
rg -nP -C3 '\bfinaliseIntent\s*\('Repository: lifinance/lintent
Length of output: 901
🏁 Script executed:
# Get the full finaliseIntent function signature and return type
sed -n '111,130p' src/lib/libraries/intentExecution.tsRepository: lifinance/lintent
Length of output: 841
🏁 Script executed:
# Check how the result is used in solver.ts after line 358
sed -n '355,370p' src/lib/libraries/solver.tsRepository: lifinance/lintent
Length of output: 463
🏁 Script executed:
# Verify component filtering - search for where components are created/passed
rg -nP 'components\s*=' src/lib/libraries/intentExecution.ts | head -20Repository: lifinance/lintent
Length of output: 208
🏁 Script executed:
# Search for other potential calls to finaliseIntent
rg -n 'finaliseIntent' --type ts --type jsRepository: lifinance/lintent
Length of output: 318
🏁 Script executed:
# Read the complete finaliseIntent function including the loop and return statements (lines 160-192)
sed -n '160,192p' src/lib/libraries/intentExecution.tsRepository: lifinance/lintent
Length of output: 1387
🏁 Script executed:
# Check what comes after line 192 - is there a return statement elsewhere?
sed -n '190,200p' src/lib/libraries/intentExecution.tsRepository: lifinance/lintent
Length of output: 154
Remove misleading loop and unreachable throw — only the first component is ever finalised.
The for loop at line 167 always exits on the first iteration: both conditional branches return, and the inner throw on line 187 also exits. Since intent.inputSettler is invariant across components and components is pre-filtered to actionChain, the loop semantically reduces to components[0]. The trailing throw on line 191 is therefore unreachable dead code.
Replace the loop with direct access to the first component:
Suggested refactor
- for (const { orderComponent } of components) {
- if (intent.inputSettler.toLowerCase() === MULTICHAIN_INPUT_SETTLER_ESCROW.toLowerCase()) {
- return walletClient.writeContract({
- chain: actionChain,
- account,
- address: intent.inputSettler,
- abi: MULTICHAIN_SETTLER_ESCROW_ABI,
- functionName: "finalise",
- args: [orderComponent, solveParams, addressToBytes32(account), "0x"]
- });
- }
- if (intent.inputSettler.toLowerCase() === MULTICHAIN_INPUT_SETTLER_COMPACT.toLowerCase()) {
- const combinedSignatures = combineSignatures(signatures);
- return walletClient.writeContract({
- chain: actionChain,
- account,
- address: intent.inputSettler,
- abi: MULTICHAIN_SETTLER_COMPACT_ABI,
- functionName: "finalise",
- args: [orderComponent, combinedSignatures, solveParams, addressToBytes32(account), "0x"]
- });
- }
- throw new Error(`Could not detect settler type ${intent.inputSettler}`);
- }
-
- throw new Error(`Failed to finalise multichain order on chain ${actionChain.id}.`);
+ const { orderComponent } = components[0];
+ if (intent.inputSettler.toLowerCase() === MULTICHAIN_INPUT_SETTLER_ESCROW.toLowerCase()) {
+ return walletClient.writeContract({
+ chain: actionChain,
+ account,
+ address: intent.inputSettler,
+ abi: MULTICHAIN_SETTLER_ESCROW_ABI,
+ functionName: "finalise",
+ args: [orderComponent, solveParams, addressToBytes32(account), "0x"]
+ });
+ }
+ if (intent.inputSettler.toLowerCase() === MULTICHAIN_INPUT_SETTLER_COMPACT.toLowerCase()) {
+ const combinedSignatures = combineSignatures(signatures);
+ return walletClient.writeContract({
+ chain: actionChain,
+ account,
+ address: intent.inputSettler,
+ abi: MULTICHAIN_SETTLER_COMPACT_ABI,
+ functionName: "finalise",
+ args: [orderComponent, combinedSignatures, solveParams, addressToBytes32(account), "0x"]
+ });
+ }
+ throw new Error(`Could not detect settler type ${intent.inputSettler}`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/libraries/intentExecution.ts` around lines 167 - 192, The for-loop
over components in finalise logic should be replaced with direct handling of the
first component since the loop always returns on the first iteration; retrieve
the first element (e.g., const { orderComponent } = components[0]) and then
branch on intent.inputSettler.toLowerCase() against
MULTICHAIN_INPUT_SETTLER_ESCROW and MULTICHAIN_INPUT_SETTLER_COMPACT, calling
walletClient.writeContract exactly as currently done (use
combineSignatures(signatures) for the COMPACT branch) and throw a single
informative Error if the settler type is unknown; also remove the unreachable
trailing throw that reports Failed to finalise multichain order on chain.
Comment on lines
+30
to
+55
| function applySameChainTimings(intent: Intent): void { | ||
| if (!intent.isSameChain()) return; | ||
| (intent as any).expiry = SAME_CHAIN_DURATION_SECONDS; | ||
| (intent as any).fillDeadline = SAME_CHAIN_DURATION_SECONDS; | ||
| } | ||
|
|
||
| function applyExclusivityOverride( | ||
| orderIntent: ReturnType<Intent["order"]>, | ||
| exclusiveFor: string | undefined, | ||
| isSameChain: boolean | ||
| ): void { | ||
| if (!isSameChain || !exclusiveFor) return; | ||
| const order = orderIntent.asOrder() as StandardOrder; | ||
| const currentTime = Math.floor(Date.now() / 1000); | ||
| const paddedExclusiveFor = | ||
| `0x${exclusiveFor.replace("0x", "").padStart(64, "0")}` as `0x${string}`; | ||
| const newContext = encodePacked( | ||
| ["bytes1", "bytes32", "uint32"], | ||
| ["0xe0", paddedExclusiveFor, currentTime + SAME_CHAIN_EXCLUSIVITY_SECONDS] | ||
| ); | ||
| for (const output of order.outputs) { | ||
| if (output.context !== "0x") { | ||
| (output as any).context = newContext; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether expiry/fillDeadline are seconds-from-now or absolute timestamps in `@lifi/intent`.
ast-grep --pattern 'class Intent { $$$ }'
rg -nP -C3 '\b(expiry|fillDeadline)\b' --type=ts -g '!**/test/**' -g '!**/node_modules/**'Repository: lifinance/lintent
Length of output: 17090
🏁 Script executed:
# Find SAME_CHAIN_DURATION_SECONDS definition and examine full intentFactory.ts
rg -n 'SAME_CHAIN_DURATION_SECONDS' --type=ts -B2 -A2
# Get full intentFactory.ts content
wc -l src/lib/libraries/intentFactory.tsRepository: lifinance/lintent
Length of output: 894
🏁 Script executed:
# Search for tests of applySameChainTimings and usage of the function
rg -n 'applySameChainTimings' --type=ts
# Search for Intent class definition and its properties
rg -n 'class Intent' --type=ts -A10
# Check if Intent exposes setters
rg -n '(expiry|fillDeadline).*set' --type=tsRepository: lifinance/lintent
Length of output: 1047
🏁 Script executed:
# Check imports and Intent source
head -30 src/lib/libraries/intentFactory.ts | grep -E '(import|from)'
# Check if Intent is from external library
rg -n '@lifi/intent' --type=ts | head -20
# Look for any Intent constructor or factory calls in intentFactory.ts
rg -n 'new Intent|Intent\(' src/lib/libraries/intentFactory.ts -A2 | head -30Repository: lifinance/lintent
Length of output: 3362
🏁 Script executed:
# Search for test files related to intentFactory
fd -e test.ts -e test.js intentFactory
# Check if there are tests that verify same-chain timing behavior
rg -n 'applySameChainTimings|SAME_CHAIN_DURATION' tests/ --type=ts
# Look at the entire applySameChainTimings and applyExclusivityOverride functions
sed -n '30,55p' src/lib/libraries/intentFactory.tsRepository: lifinance/lintent
Length of output: 970
Critical bug: applySameChainTimings assigns duration instead of absolute timestamp, expiring all same-chain intents immediately.
Setting expiry and fillDeadline to SAME_CHAIN_DURATION_SECONDS (600) creates a Unix timestamp of "Jan 1, 1970 + 10 minutes", permanently in the past. Based on the codebase:
- Test fixtures set
fillDeadline: Math.floor(Date.now() / 1000) + offset - Expiration logic (intentList.ts) computes
fillDeadline - nowSeconds, confirming fillDeadline must be an absolute timestamp applyExclusivityOverridein the same file correctly usescurrentTime + duration(line 47)
Should be:
(intent as any).expiry = Math.floor(Date.now() / 1000) + SAME_CHAIN_DURATION_SECONDS;
(intent as any).fillDeadline = Math.floor(Date.now() / 1000) + SAME_CHAIN_DURATION_SECONDS;Additionally, both helpers bypass type safety via as any writes. If @lifi/intent adds readonly modifiers or relocates these fields, the casts will silently fail. Prefer setters or factory methods if exposed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/libraries/intentFactory.ts` around lines 30 - 55, The bug is that
applySameChainTimings sets expiry and fillDeadline to a duration value rather
than an absolute Unix timestamp; change applySameChainTimings to set both expiry
and fillDeadline to Math.floor(Date.now()/1000) + SAME_CHAIN_DURATION_SECONDS
(i.e., current time plus the duration) instead of SAME_CHAIN_DURATION_SECONDS.
Also remove or minimize unsafe (intent as any) writes: prefer using the Intent
API setters/factory methods or proper typed fields on intent (or expose a small
helper on Intent to set expiry/fillDeadline) so the code doesn’t rely on as any
and will fail loudly if the library adds readonly/relocated fields. Ensure
parity with applyExclusivityOverride’s use of currentTime + duration.
* accommodate intent.ts standardise-order-classes API changes - add containerToIntent() helper in utils/intent.ts that wraps orderToIntent with automatic namespace detection (solana vs eip155) from OrderContainer - replace all orderToIntent(container) call sites with containerToIntent() - alias StandardOrderIntent as StandardEVMIntent in intentExecution.ts - add local idToToken() in intentList.ts (removed from @lifi/intent exports) * Update @lifi/intent to 0.0.4 and fix breaking API changes - Bump @lifi/intent from 0.0.3-alpha.1 to 0.0.4 - Add chainNamespace field to CoreToken in toCoreTokenContext - Fix containerToIntent: use 'in' narrowing for proper TypeScript overload resolution - Add StandardSolanaIntent guards in IntentFactory compact/escrow methods - Add StandardSolanaIntent guard in Solver.claim before finaliseIntent * fix: CI/CD tests to use secret variables * Merge pull request #41 from lifinance/feature/v2-116-add-recipient-field-on-lintentorg Add optional recipient field for intent outputs --------- Co-authored-by: Alexander <git@lindgren.xyz> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…nt name Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
reednaa
changed the title
Pin third-party GitHub Actions to SHA hashes, add fork-PR guards to deploy workflow, bump axios to fix follow-redirects vulnerability, add input validation and error handling to all server routes, await DB writes in intentFactory, and remove unsafe type casts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
src/lib/libraries/intentFactory.ts (1)
42-47:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical:
applySameChainTimingsassigns a duration instead of an absolute timestamp.Setting
expiry/fillDeadlinetoSAME_CHAIN_DURATION_SECONDS(600) yields a Unix timestamp of1970-01-01 00:10:00, which is permanently in the past — every same-chain intent will be marked expired immediately.applyExclusivityOverridein this same file already follows the correct pattern (currentTime + duration). UseMath.floor(Date.now() / 1000) + SAME_CHAIN_DURATION_SECONDSfor both fields, and prefer a typed setter onIntentover theas unknown as { … }write to avoid silent breakage if@lifi/intentadds readonly modifiers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/libraries/intentFactory.ts` around lines 42 - 47, applySameChainTimings incorrectly writes a duration constant into expiry/fillDeadline, causing immediate expiry; change it to set absolute Unix timestamps by computing Math.floor(Date.now() / 1000) + SAME_CHAIN_DURATION_SECONDS for both expiry and fillDeadline (mirror the pattern used in applyExclusivityOverride). Replace the unsafe cast "as unknown as { expiry: number; fillDeadline: number }" with the proper typed setter on the Intent object (use Intent's setters or explicit properties on the Intent type) so the fields are assigned with correct types and respect possible readonly modifiers in the `@lifi/intent` definition.src/lib/libraries/intentExecution.ts (1)
167-192:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMisleading loop and unreachable throw — only the first component is ever finalised.
The
forloop on line 167 always exits on the first iteration: both branchesreturn, and the innerthrowon line 189 also exits. Sinceintent.inputSettleris invariant across components andcomponentsis already filtered toactionChain, the loop reduces tocomponents[0], making the trailingthrowon line 192 dead code. Replace the loop with direct access tocomponents[0].🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/libraries/intentExecution.ts` around lines 167 - 192, The for-loop over components in finalise logic is misleading and effectively only processes components[0]; change it to directly read the first element (const component = components[0]) and handle the empty-case by throwing the existing error (`Failed to finalise multichain order on chain ${actionChain.id}.`); then run the same settler dispatch using intent.inputSettler comparisons to MULTICHAIN_INPUT_SETTLER_ESCROW and MULTICHAIN_INPUT_SETTLER_COMPACT, calling walletClient.writeContract with the same args (using combineSignatures(signatures) for the COMPACT path) and preserving the inner throw for unknown settler types (`Could not detect settler type ${intent.inputSettler}`).
🧹 Nitpick comments (4)
.github/workflows/test.yml (1)
17-17: ⚡ Quick winPin
bun-versionto an explicit version instead oflatest.Using
latestmakes CI non-deterministic and can cause flaky builds across runs. Pin both occurrences to the project's intended Bun version for reproducible checks/tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/test.yml at line 17, Replace the two occurrences of the GitHub Actions key "bun-version: latest" in the workflow (test.yml) with the project's explicit Bun version string (e.g., "bun-version: 1.x.y") so CI is deterministic; locate both "bun-version" entries in the file and set them to the pinned version used by the project..github/workflows/deploy.yml (1)
28-28: ⚡ Quick winPin Bun version in deploy jobs instead of using
latestfor reproducible releases.While the
setup-bunaction itself is pinned to a specific commit, thebun-version: latestparameter causes the Bun runtime to float to the latest release. For production and preview deployments, this introduces reproducibility risk—the same action version may pull different Bun versions between runs, potentially causing unexpected behavior changes. Specify a fixed Bun version (e.g.,1.0.30orlatest:^1.0) on both lines 28 and 74.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/deploy.yml at line 28, Change the floating bun-version: latest used with the setup-bun action to a pinned version to ensure reproducible deployments; locate both occurrences of the bun-version: latest setting (used in the deploy jobs for production/preview) and replace with a fixed version string such as bun-version: 1.0.30 or a semver pin like bun-version: latest:^1.0 so both places use the same explicit version.src/lib/libraries/intentFactory.ts (1)
124-134: 💤 Low valueReplace
Promise<any>hooks with concrete return types.ESLint flags four
Promise<any>occurrences (lines 124, 125, 133, 134, 298, 299). Since neitherpreHooknorpostHookreturn values are consumed at the call sites (if (this.preHook) await this.preHook(inputChain)),Promise<unknown>orPromise<void>is a strictly better contract and removes theno-explicit-anyviolations.♻️ Proposed fix
- preHook?: (chainId: number) => Promise<any>; - postHook?: () => Promise<any>; + preHook?: (chainId: number) => Promise<unknown>; + postHook?: () => Promise<unknown>;Apply the same change inside the constructor options type and in
escrowApprove'soptstype.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/libraries/intentFactory.ts` around lines 124 - 134, Change the hook return types from Promise<any> to a concrete type (use Promise<void> since the return values are not consumed) for the preHook and postHook declarations, update the constructor options type (the options parameter with walletClient, preHook, postHook) to use Promise<void>, and update the escrowApprove function's opts type to replace Promise<any> with Promise<void>; ensure you modify all occurrences of preHook and postHook type annotations (including the class properties, constructor options and escrowApprove's opts) to remove the no-explicit-any warnings while keeping call sites (e.g., if (this.preHook) await this.preHook(inputChain)) unchanged.src/lib/libraries/intentExecution.ts (1)
24-33: 💤 Low valueAsymmetric
?? "0x"fallback incombineSignatures.
sponsorSignature.payload ?? "0x"defends against a missing payload, butallocatorSignature.payloadis passed through without the same guard. Either both need the fallback or neither does (theNoSignatureshape used byintentFactory.tsalways populatespayload: "0x", so the existing??is also redundant). Pick one style for consistency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/libraries/intentExecution.ts` around lines 24 - 33, The combineSignatures function uses an asymmetric fallback (sponsorSignature.payload ?? "0x" while allocatorSignature.payload is used directly); update combineSignatures to be consistent with the NoSignature shape used in intentFactory.ts by removing the unnecessary "?? '0x'" fallback and passing sponsorSignature.payload and allocatorSignature.payload directly into encodeAbiParameters (locate the combineSignatures function and the encodeAbiParameters/parseAbiParameters call to make the change).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 53-66: Package dependency versions conflict: wagmi (^3.5.0) can
resolve to 3.6.x which requires `@wagmi/connectors` 8.x but package.json currently
uses `@wagmi/connectors` ^7.2.1; fix by either pinning wagmi to the 3.5.x line
(change "wagmi" to "~3.5.0" or similar) to keep compatibility with
`@wagmi/connectors` 7.x, or upgrade connectors to "^8.0.0" and update related
MetaMask packages by removing "@metamask/sdk" and adding "@metamask/connect-evm"
(and run a fresh install to update the lockfile); update package.json entries
for "wagmi", "@wagmi/connectors", "@metamask/sdk" / "@metamask/connect-evm"
accordingly and verify installs resolve without peer dependency warnings.
In `@src/lib/libraries/flowProgress.ts`:
- Around line 153-178: The compact branch selects the wrong per-chain inputs for
multichain orders causing getLockDetails to query the wrong chain; update the
flattenedInputs selection in isInputChainFinalised so for multichain/compact
cases you locate the inputs object whose chainId matches the current chainId
(e.g. find entry in order.inputs with .chainId === chainId and use its .inputs)
instead of always using order.inputs[0]?.inputs, then proceed to call
COMPACT.getLockDetails and hasConsumedAllocatorNonce with that chain-matched
lockId; ensure the fallback still handles missing matches by returning false.
In `@src/lib/libraries/intentFactory.ts`:
- Around line 192-211: The code saves the order via saveOrder (which also writes
to this.orders and store.saveOrderToDb) before checking if the order is a
supported compact order; move the validation to run immediately after creating
the order with intent.asOrder() and before calling saveOrder so that if
(!("originChainId" in order)) throws, nothing is persisted; then call
saveOrder(...) and afterwards call intentApi.submitOrder(...) with
INPUT_SETTLER_COMPACT_LIFI and signatures as before.
- Around line 285-291: The call to saveOrder is using the global reactive
store.inputSettler which can differ from the actual settler used by the intent;
update the save to pass intent.inputSettler so the persisted order matches
execution. Specifically, in openEscorwIntent (where you call this.saveOrder({
order: intent.asOrder(), inputSettler: store.inputSettler })), replace
store.inputSettler with intent.inputSettler to mirror the compact() pattern and
ensure saveOrder records the settler that was actually executed by the intent.
---
Duplicate comments:
In `@src/lib/libraries/intentExecution.ts`:
- Around line 167-192: The for-loop over components in finalise logic is
misleading and effectively only processes components[0]; change it to directly
read the first element (const component = components[0]) and handle the
empty-case by throwing the existing error (`Failed to finalise multichain order
on chain ${actionChain.id}.`); then run the same settler dispatch using
intent.inputSettler comparisons to MULTICHAIN_INPUT_SETTLER_ESCROW and
MULTICHAIN_INPUT_SETTLER_COMPACT, calling walletClient.writeContract with the
same args (using combineSignatures(signatures) for the COMPACT path) and
preserving the inner throw for unknown settler types (`Could not detect settler
type ${intent.inputSettler}`).
In `@src/lib/libraries/intentFactory.ts`:
- Around line 42-47: applySameChainTimings incorrectly writes a duration
constant into expiry/fillDeadline, causing immediate expiry; change it to set
absolute Unix timestamps by computing Math.floor(Date.now() / 1000) +
SAME_CHAIN_DURATION_SECONDS for both expiry and fillDeadline (mirror the pattern
used in applyExclusivityOverride). Replace the unsafe cast "as unknown as {
expiry: number; fillDeadline: number }" with the proper typed setter on the
Intent object (use Intent's setters or explicit properties on the Intent type)
so the fields are assigned with correct types and respect possible readonly
modifiers in the `@lifi/intent` definition.
---
Nitpick comments:
In @.github/workflows/deploy.yml:
- Line 28: Change the floating bun-version: latest used with the setup-bun
action to a pinned version to ensure reproducible deployments; locate both
occurrences of the bun-version: latest setting (used in the deploy jobs for
production/preview) and replace with a fixed version string such as bun-version:
1.0.30 or a semver pin like bun-version: latest:^1.0 so both places use the same
explicit version.
In @.github/workflows/test.yml:
- Line 17: Replace the two occurrences of the GitHub Actions key "bun-version:
latest" in the workflow (test.yml) with the project's explicit Bun version
string (e.g., "bun-version: 1.x.y") so CI is deterministic; locate both
"bun-version" entries in the file and set them to the pinned version used by the
project.
In `@src/lib/libraries/intentExecution.ts`:
- Around line 24-33: The combineSignatures function uses an asymmetric fallback
(sponsorSignature.payload ?? "0x" while allocatorSignature.payload is used
directly); update combineSignatures to be consistent with the NoSignature shape
used in intentFactory.ts by removing the unnecessary "?? '0x'" fallback and
passing sponsorSignature.payload and allocatorSignature.payload directly into
encodeAbiParameters (locate the combineSignatures function and the
encodeAbiParameters/parseAbiParameters call to make the change).
In `@src/lib/libraries/intentFactory.ts`:
- Around line 124-134: Change the hook return types from Promise<any> to a
concrete type (use Promise<void> since the return values are not consumed) for
the preHook and postHook declarations, update the constructor options type (the
options parameter with walletClient, preHook, postHook) to use Promise<void>,
and update the escrowApprove function's opts type to replace Promise<any> with
Promise<void>; ensure you modify all occurrences of preHook and postHook type
annotations (including the class properties, constructor options and
escrowApprove's opts) to remove the no-explicit-any warnings while keeping call
sites (e.g., if (this.preHook) await this.preHook(inputChain)) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 41128794-bc32-4a66-bf4c-b012e8460805
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
.env.e2e.example.github/workflows/deploy.yml.github/workflows/test.ymlpackage.jsonsrc/lib/components/ui/ScreenFrame.sveltesrc/lib/libraries/flowProgress.tssrc/lib/libraries/intentExecution.tssrc/lib/libraries/intentFactory.tssrc/lib/libraries/intentList.tssrc/lib/libraries/solver.tssrc/lib/screens/FillIntent.sveltesrc/lib/screens/Finalise.sveltesrc/lib/screens/IntentList.sveltesrc/lib/screens/IssueIntent.sveltesrc/lib/screens/ReceiveMessage.sveltesrc/lib/state.svelte.tssrc/lib/utils/intent.tssrc/routes/+page.sveltesrc/routes/allocator/+server.tssrc/routes/polymer/+server.tssrc/routes/polymer/health/+server.tstests/unit/recipientField.test.ts
Comment on lines
+53
to
+66
| "@metamask/sdk": "^0.34.0", | ||
| "@safe-global/safe-apps-provider": "~0.18.6", | ||
| "@safe-global/safe-apps-sdk": "^9.1.0", | ||
| "@sveltejs/adapter-cloudflare": "^7.0.3", | ||
| "@wagmi/connectors": "^7.2.1", | ||
| "@wagmi/core": "^3.4.0", | ||
| "@walletconnect/ethereum-provider": "^2.21.1", | ||
| "axios": "^1.13.6", | ||
| "base64-js": "^1.5.1", | ||
| "drizzle-orm": "^0.45.1", | ||
| "porto": "~0.2.35", | ||
| "rxjs": "^7.8.2", | ||
| "viem": "~2.45.1", | ||
| "wagmi": "^3.5.0" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect resolved versions and peer constraints
fd -t f 'bun.lockb|bun.lock|package-lock.json|pnpm-lock.yaml' -d 2 | head
rg -nP '"(wagmi|@wagmi/(core|connectors)|@metamask/(sdk|connect-evm))"' --type=json -g '!**/node_modules/**'Repository: lifinance/lintent
Length of output: 252
🏁 Script executed:
# Check the bun.lock file for resolved versions
# First, let's see what the lock file format is
head -50 bun.lockRepository: lifinance/lintent
Length of output: 1720
🏁 Script executed:
# Search for the resolved versions of key packages in bun.lock
rg -A 5 'wagmi@' bun.lock | head -60Repository: lifinance/lintent
Length of output: 794
🏁 Script executed:
# Search for `@wagmi/connectors` resolved version
rg -A 5 '@wagmi/connectors@' bun.lock | head -40Repository: lifinance/lintent
Length of output: 1345
🏁 Script executed:
# Check if `@metamask/sdk` or `@metamask/connect-evm` are used in the codebase
rg '@metamask/(sdk|connect-evm)' --type=ts --type=js --type=svelte src/ -lRepository: lifinance/lintent
Length of output: 92
🌐 Web query:
wagmi 3.6.0 release notes @wagmi/connectors 8.0.0 dependency
💡 Result:
wagmi 3.6.0 release notes highlight a minor change: Update MetaMask connector from @metamask/sdk to the new @metamask/connect-evm (#4960), requiring npm install @metamask/connect-evm and `npm uninstall `@metamask/sdk [1][2][3]. Patch changes include updated dependencies: @wagmi/core@3.4.1 and @wagmi/connectors@8.0.0 [1][2][3]. This confirms @wagmi/connectors@8.0.0 is a dependency (peer dependency) of wagmi 3.6.0, part of the monorepo release [1][2][4][5]. The connectors update migrates the MetaMask implementation [6].
Citations:
- 1: https://github.com/wevm/wagmi/releases
- 2: wevm/wagmi@434ddf9
- 3: chore(deps): bump wagmi from 2.14.9 to 3.6.0 enioxt/egos-lab#43
- 4: https://github.com/wevm/wagmi/releases/tag/%40wagmi%2Fconnectors%408.0.4
- 5: https://npmx.dev/package/@wagmi/connectors
- 6: wevm/wagmi@2a566ec
🏁 Script executed:
# Try searching for `@metamask` imports in the codebase (adjusted syntax)
rg '@metamask/(sdk|connect-evm)' src/ --type=ts --type=js 2>/dev/null || rg '@metamask/(sdk|connect-evm)' src/Repository: lifinance/lintent
Length of output: 43
Resolve wagmi / @wagmi/connectors version incompatibility.
wagmi: ^3.5.0 will resolve to 3.6.x, which requires @wagmi/connectors@8.0.0. However, @wagmi/connectors: ^7.2.1 excludes 8.0.0, creating a peer dependency conflict on fresh installs or lockfile updates. The current lockfile pins to compatible versions (3.5.0 + 7.2.1), but this incompatibility will surface once dependencies are updated.
Additionally, @metamask/sdk: ^0.34.0 is unused in the codebase and will become orphaned when migrating to connectors 8.0.0, which uses @metamask/connect-evm instead.
Either:
- pin
wagmito~3.5.xto maintain connectors 7.x compatibility, or - bump
@wagmi/connectorsto^8.0.0, remove@metamask/sdk, and add@metamask/connect-evm.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` around lines 53 - 66, Package dependency versions conflict:
wagmi (^3.5.0) can resolve to 3.6.x which requires `@wagmi/connectors` 8.x but
package.json currently uses `@wagmi/connectors` ^7.2.1; fix by either pinning
wagmi to the 3.5.x line (change "wagmi" to "~3.5.0" or similar) to keep
compatibility with `@wagmi/connectors` 7.x, or upgrade connectors to "^8.0.0" and
update related MetaMask packages by removing "@metamask/sdk" and adding
"@metamask/connect-evm" (and run a fresh install to update the lockfile); update
package.json entries for "wagmi", "@wagmi/connectors", "@metamask/sdk" /
"@metamask/connect-evm" accordingly and verify installs resolve without peer
dependency warnings.
Comment on lines
+153
to
+178
| if ( | ||
| inputSettler === INPUT_SETTLER_COMPACT_LIFI || | ||
| inputSettler === MULTICHAIN_INPUT_SETTLER_COMPACT | ||
| ) { | ||
| const flattenedInputs = "originChainId" in order ? order.inputs : order.inputs[0]?.inputs; | ||
| if (!flattenedInputs || flattenedInputs.length === 0) return false; | ||
|
|
||
| return getOrFetchRpc( | ||
| `progress:finalised:compact:${orderId}:${chainId.toString()}`, | ||
| async () => { | ||
| const [, allocator] = await inputChainClient.readContract({ | ||
| address: COMPACT, | ||
| abi: COMPACT_ABI, | ||
| functionName: "getLockDetails", | ||
| args: [flattenedInputs[0][0]] | ||
| }); | ||
| return inputChainClient.readContract({ | ||
| address: COMPACT, | ||
| abi: COMPACT_ABI, | ||
| functionName: "hasConsumedAllocatorNonce", | ||
| args: [order.nonce, allocator] | ||
| }); | ||
| }, | ||
| { ttlMs: PROGRESS_TTL_MS } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Multichain compact finalisation queries the wrong chain's lock.
isInputChainFinalised is invoked once per inputChain in getOrderProgressChecks (line 215), but the compact branch always picks order.inputs[0]?.inputs for the multichain case. When the call is made for a chainId other than order.inputs[0].chainId, the code calls COMPACT.getLockDetails on the current chain's compact contract using a lockId that belongs to a different chain — almost certainly returning a wrong/empty allocator and producing a meaningless hasConsumedAllocatorNonce result. This will cause allFinalised to be flaky/false-negative for multi-input-chain compact orders.
Select the inputs that match the chain being checked.
🐛 Proposed fix
if (
inputSettler === INPUT_SETTLER_COMPACT_LIFI ||
inputSettler === MULTICHAIN_INPUT_SETTLER_COMPACT
) {
- const flattenedInputs = "originChainId" in order ? order.inputs : order.inputs[0]?.inputs;
+ const flattenedInputs =
+ "originChainId" in order
+ ? order.inputs
+ : order.inputs.find((i) => i.chainId === chainId)?.inputs;
if (!flattenedInputs || flattenedInputs.length === 0) return false;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ( | |
| inputSettler === INPUT_SETTLER_COMPACT_LIFI || | |
| inputSettler === MULTICHAIN_INPUT_SETTLER_COMPACT | |
| ) { | |
| const flattenedInputs = "originChainId" in order ? order.inputs : order.inputs[0]?.inputs; | |
| if (!flattenedInputs || flattenedInputs.length === 0) return false; | |
| return getOrFetchRpc( | |
| `progress:finalised:compact:${orderId}:${chainId.toString()}`, | |
| async () => { | |
| const [, allocator] = await inputChainClient.readContract({ | |
| address: COMPACT, | |
| abi: COMPACT_ABI, | |
| functionName: "getLockDetails", | |
| args: [flattenedInputs[0][0]] | |
| }); | |
| return inputChainClient.readContract({ | |
| address: COMPACT, | |
| abi: COMPACT_ABI, | |
| functionName: "hasConsumedAllocatorNonce", | |
| args: [order.nonce, allocator] | |
| }); | |
| }, | |
| { ttlMs: PROGRESS_TTL_MS } | |
| ); | |
| } | |
| if ( | |
| inputSettler === INPUT_SETTLER_COMPACT_LIFI || | |
| inputSettler === MULTICHAIN_INPUT_SETTLER_COMPACT | |
| ) { | |
| const flattenedInputs = | |
| "originChainId" in order | |
| ? order.inputs | |
| : order.inputs.find((i) => i.chainId === chainId)?.inputs; | |
| if (!flattenedInputs || flattenedInputs.length === 0) return false; | |
| return getOrFetchRpc( | |
| `progress:finalised:compact:${orderId}:${chainId.toString()}`, | |
| async () => { | |
| const [, allocator] = await inputChainClient.readContract({ | |
| address: COMPACT, | |
| abi: COMPACT_ABI, | |
| functionName: "getLockDetails", | |
| args: [flattenedInputs[0][0]] | |
| }); | |
| return inputChainClient.readContract({ | |
| address: COMPACT, | |
| abi: COMPACT_ABI, | |
| functionName: "hasConsumedAllocatorNonce", | |
| args: [order.nonce, allocator] | |
| }); | |
| }, | |
| { ttlMs: PROGRESS_TTL_MS } | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/libraries/flowProgress.ts` around lines 153 - 178, The compact branch
selects the wrong per-chain inputs for multichain orders causing getLockDetails
to query the wrong chain; update the flattenedInputs selection in
isInputChainFinalised so for multichain/compact cases you locate the inputs
object whose chainId matches the current chainId (e.g. find entry in
order.inputs with .chainId === chainId and use its .inputs) instead of always
using order.inputs[0]?.inputs, then proceed to call COMPACT.getLockDetails and
hasConsumedAllocatorNonce with that chain-matched lockId; ensure the fallback
still handles missing matches by returning false.
Comment on lines
+192
to
+211
| await this.saveOrder({ | ||
| order: intent.asOrder(), | ||
| inputSettler: intent.inputSettler, | ||
| sponsorSignature: { | ||
| type: "ECDSA", | ||
| payload: sponsorSignature | ||
| } | ||
| }); | ||
|
|
||
| const order = intent.asOrder(); | ||
| if (!("originChainId" in order)) { | ||
| throw new Error("CatalystCompactOrder submission currently supports standard orders."); | ||
| } | ||
| const signedOrder = await this.intentApi.submitOrder({ | ||
| orderType: "CatalystCompactOrder", | ||
| order, | ||
| inputSettler: INPUT_SETTLER_COMPACT_LIFI, | ||
| sponsorSignature, | ||
| allocatorSignature: "0x" | ||
| }); |
There was a problem hiding this comment.
Order is persisted before the validity check — failed multichain compact submissions still leave a row in the DB.
saveOrder writes to store and the local orders[] array on lines 192–199, but the if (!("originChainId" in order)) guard that rejects multichain compact orders is on line 202. When the guard throws, the order has already been pushed to this.orders and saved via store.saveOrderToDb, so the UI will show an orphaned, unsubmittable row. Validate first, then save.
🐛 Proposed fix
- await this.saveOrder({
- order: intent.asOrder(),
- inputSettler: intent.inputSettler,
- sponsorSignature: {
- type: "ECDSA",
- payload: sponsorSignature
- }
- });
-
const order = intent.asOrder();
if (!("originChainId" in order)) {
throw new Error("CatalystCompactOrder submission currently supports standard orders.");
}
+ await this.saveOrder({
+ order,
+ inputSettler: intent.inputSettler,
+ sponsorSignature: {
+ type: "ECDSA",
+ payload: sponsorSignature
+ }
+ });
const signedOrder = await this.intentApi.submitOrder({
orderType: "CatalystCompactOrder",
order,
inputSettler: INPUT_SETTLER_COMPACT_LIFI,
sponsorSignature,
allocatorSignature: "0x"
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| await this.saveOrder({ | |
| order: intent.asOrder(), | |
| inputSettler: intent.inputSettler, | |
| sponsorSignature: { | |
| type: "ECDSA", | |
| payload: sponsorSignature | |
| } | |
| }); | |
| const order = intent.asOrder(); | |
| if (!("originChainId" in order)) { | |
| throw new Error("CatalystCompactOrder submission currently supports standard orders."); | |
| } | |
| const signedOrder = await this.intentApi.submitOrder({ | |
| orderType: "CatalystCompactOrder", | |
| order, | |
| inputSettler: INPUT_SETTLER_COMPACT_LIFI, | |
| sponsorSignature, | |
| allocatorSignature: "0x" | |
| }); | |
| const order = intent.asOrder(); | |
| if (!("originChainId" in order)) { | |
| throw new Error("CatalystCompactOrder submission currently supports standard orders."); | |
| } | |
| await this.saveOrder({ | |
| order, | |
| inputSettler: intent.inputSettler, | |
| sponsorSignature: { | |
| type: "ECDSA", | |
| payload: sponsorSignature | |
| } | |
| }); | |
| const signedOrder = await this.intentApi.submitOrder({ | |
| orderType: "CatalystCompactOrder", | |
| order, | |
| inputSettler: INPUT_SETTLER_COMPACT_LIFI, | |
| sponsorSignature, | |
| allocatorSignature: "0x" | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/libraries/intentFactory.ts` around lines 192 - 211, The code saves
the order via saveOrder (which also writes to this.orders and
store.saveOrderToDb) before checking if the order is a supported compact order;
move the validation to run immediately after creating the order with
intent.asOrder() and before calling saveOrder so that if (!("originChainId" in
order)) throws, nothing is persisted; then call saveOrder(...) and afterwards
call intentApi.submitOrder(...) with INPUT_SETTLER_COMPACT_LIFI and signatures
as before.
Comment on lines
+285
to
+291
| await this.saveOrder({ | ||
| order: intent.asOrder(), | ||
| inputSettler: store.inputSettler | ||
| }); | ||
|
|
||
| return transactionHashes; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# How is store.inputSettler set/maintained relative to intent flows?
rg -nP -C3 '\binputSettler\b' --type=ts -g '!**/node_modules/**'Repository: lifinance/lintent
Length of output: 18283
🏁 Script executed:
# Get context around the saveOrder call in openEscrow (lines 260-295)
sed -n '260,295p' src/lib/libraries/intentFactory.tsRepository: lifinance/lintent
Length of output: 1258
🏁 Script executed:
# Find the openEscrowIntent function definition
rg -n 'export.*function.*openEscrowIntent|export const openEscrowIntent' --type=tsRepository: lifinance/lintent
Length of output: 141
🏁 Script executed:
# Check the openEscrowIntent implementation to see what inputSettler it uses
rg -n -A 30 'export.*function.*openEscrowIntent|export const openEscrowIntent' src/lib/libraries/intentExecution.ts --type=tsRepository: lifinance/lintent
Length of output: 1122
🏁 Script executed:
# Find Intent class definition and inputSettler property
rg -n 'class Intent|inputSettler' src/lib/utils/intent.ts -A 2 -B 2 | head -60Repository: lifinance/lintent
Length of output: 549
🏁 Script executed:
# Check how intent.inputSettler is set in Intent constructor or as property
ast-grep --pattern 'class Intent {
$$$
inputSettler
$$$
}'Repository: lifinance/lintent
Length of output: 43
🏁 Script executed:
# Get the full Intent class definition focusing on inputSettler
rg -n -A 100 'export class Intent' src/lib/utils/intent.ts | grep -A 5 -B 5 'inputSettler'Repository: lifinance/lintent
Length of output: 43
🏁 Script executed:
# Search for Intent class and its properties
rg -n 'class Intent|export class Intent' --type=ts -A 30Repository: lifinance/lintent
Length of output: 2101
🏁 Script executed:
# Check the toCoreCreateIntentOptions function to understand inputSettler flow
rg -n 'toCoreCreateIntentOptions' src/lib/libraries/intentFactory.ts -A 10Repository: lifinance/lintent
Length of output: 2405
🏁 Script executed:
# Find where Intent is instantiated with options in openEscrow context
sed -n '260,280p' src/lib/libraries/intentFactory.tsRepository: lifinance/lintent
Length of output: 1006
🏁 Script executed:
# Find where Intent class comes from and check if it has inputSettler
rg -n 'import.*Intent|from.*intent' src/lib/libraries/intentFactory.ts | head -20Repository: lifinance/lintent
Length of output: 297
🏁 Script executed:
# Check what the Intent.order() method returns and if it has inputSettler
rg -n 'StandardEVMIntent|MultichainOrderIntent' src/lib/libraries/intentFactory.ts -B 2 -A 2 | grep -A 5 -B 5 'inputSettler'Repository: lifinance/lintent
Length of output: 43
🏁 Script executed:
# Check the StandardEVMIntent and MultichainOrderIntent classes for inputSettler property
rg -n 'class StandardEVMIntent|class MultichainOrderIntent' --type=ts -A 20Repository: lifinance/lintent
Length of output: 43
🏁 Script executed:
# Search for inputSettler as a property/field in intent classes
rg -n 'inputSettler:' --type=ts -B 3 -A 1Repository: lifinance/lintent
Length of output: 3958
🏁 Script executed:
# Check if intent.inputSettler can be determined from the Intent object after construction
rg -n 'intent\.inputSettler' src/lib/libraries/intentFactory.tsRepository: lifinance/lintent
Length of output: 108
🏁 Script executed:
# Verify how compact and compactDepositAndRegister flows use inputSettler
sed -n '175,200p' src/lib/libraries/intentFactory.tsRepository: lifinance/lintent
Length of output: 1003
🏁 Script executed:
# Check compactDepositAndRegister context to see full picture
sed -n '220,245p' src/lib/libraries/intentFactory.tsRepository: lifinance/lintent
Length of output: 1274
🏁 Script executed:
# Verify that intent.inputSettler is indeed a property on StandardEVMIntent and MultichainOrderIntent
rg -n '@lifi/intent' src/lib/libraries/intentFactory.tsRepository: lifinance/lintent
Length of output: 113
🏁 Script executed:
# Check if there's any documentation or type definition for intent.inputSettler
rg -n 'inputSettler' node_modules/@lifi/intent --type=ts 2>/dev/null | head -20 || echo "Checking local type refs instead"Repository: lifinance/lintent
Length of output: 43
🏁 Script executed:
# Look for how intent objects are used to understand inputSettler property
rg -n 'intent\.inputSettler' src/lib/libraries --type=tsRepository: lifinance/lintent
Length of output: 1308
Use intent.inputSettler instead of store.inputSettler to ensure the persisted order matches what was actually executed.
The openEscorwIntent function writes to different settler contracts depending on intent type (INPUT_SETTLER_ESCROW_LIFI for StandardEVMIntent, intent.inputSettler for multichain), but saveOrder records store.inputSettler, a global reactive value that may not reflect the current execution. The intent object already has the correct settler value; use intent.inputSettler to align with the compact() pattern and guarantee the persisted record matches the on-chain transaction.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/libraries/intentFactory.ts` around lines 285 - 291, The call to
saveOrder is using the global reactive store.inputSettler which can differ from
the actual settler used by the intent; update the save to pass
intent.inputSettler so the persisted order matches execution. Specifically, in
openEscorwIntent (where you call this.saveOrder({ order: intent.asOrder(),
inputSettler: store.inputSettler })), replace store.inputSettler with
intent.inputSettler to mirror the compact() pattern and ensure saveOrder records
the settler that was actually executed by the intent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes since main
@lifi/intentpackage integration, standardised order classesTest plan
bun run check— 0 errors, 0 warnings)bun run test:unit)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Refactor
Chores