feat: fill solana to evm intents#46
feat: fill solana to evm intents#46Asem-Abdelhady wants to merge 4 commits intofeature/v2-112-issue-solana-to-evm-ordersfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
reednaa
left a comment
reednaa
left a comment
There was a problem hiding this comment.
The code is AI slop. There is a lot of improvements that needs to be performed. See review. Once done, I will have to re-review everything.
| const orderContainer = reviveOrderBigInts({ | ||
| ...order, | ||
| allocatorSignature, | ||
| sponsorSignature | ||
| } as OrderContainer); |
There was a problem hiding this comment.
We dealing with orders and orders ids via 2 fronts, Websocket and HTTP. After integrating solana i faced some problems with parsing that because of the Websocket front. Claude figured out the problem with how it parses bigint and offered a standarized way to parse. Doing that solved the problem
| const orderContainer = reviveOrderBigInts({ | ||
| ...order, | ||
| allocatorSignature, | ||
| sponsorSignature | ||
| } as OrderContainer); |
There was a problem hiding this comment.
We dealing with orders and orders ids via 2 fronts, Websocket and HTTP. After integrating solana i faced some problems with parsing that because of the Websocket front. Claude figured out the problem with how it parses bigint and offered a standarized way to parse. Doing that solved the problem
reednaa
left a comment
reednaa
left a comment
There was a problem hiding this comment.
See AI report below along with my own findings as comments.
PR #46 Code Review — feat: fill solana to evm intents
Overview
Adds the complete Solana→EVM solver pipeline: fill outputs on EVM, submit a Polymer proof to Solana's oracle_polymer, then finalise the escrow. Three new libs (evmFillLib, solanaValidateLib, solanaFinaliseLib), three updated screens, and a parseOrderBigInts utility.
Duplicates / Hacky Paths
1. anchorWallet boilerplate repeated 3×
Identical anchorWallet object literal (with generic signTransaction/signAllTransactions wrappers) appears independently in:
solanaEscrowLib.tssolanaFinaliseLib.tssolanaValidateLib.ts
Extract a makeAnchorWallet(pubkey, walletAdapter) helper in a shared module (e.g. solanaAnchorLib.ts).
2. hexToBytes32 / normalizeBytes32Hex are the same operation
solanaFinaliseLib.ts defines hexToBytes32 returning number[]; solanaValidateLib.ts defines normalizeBytes32Hex returning Buffer. The call sites convert between them with Array.from(...). One shared helper covers both.
3. orderId derivation duplicated within solanaFinaliseLib.ts
Both deriveOrderContextPda and finaliseSolanaEscrow independently execute:
const encoded = borshEncodeSolanaOrder(order);
const orderIdHex = keccak256(encoded);
const orderId = Buffer.from(orderIdHex.slice(2), "hex");finaliseSolanaEscrow already has the orderId; it could call deriveOrderContextPda instead of re-deriving it.
4. "Is Solana order finalised?" logic duplicated between Finalise.svelte and flowProgress.ts
Finalise.svelte:isClaimed (Solana branch) and flowProgress.ts:isInputChainFinalised (Solana branch) contain identical logic:
const orderContextPda = await deriveOrderContextPda(order as StandardSolana);
const info = await conn.getAccountInfo(new PublicKey(orderContextPda));
return info === null;Extract to isSolanaEscrowFinalised(order, connection): Promise<boolean> in solanaFinaliseLib.ts.
5. outputKey helper bypassed in FillIntent.svelte:fillWrapper
The component defines const outputKey = (output) => hashStruct(...) but fillWrapper (line 121) hashes inline again instead of calling outputKey(output).
6. Fill hash validation inlined in 3 places
ReceiveMessage.svelte:isValidateduses!=(not!==)ReceiveMessage.svelte:solanaValidateButtonFnuses!==Finalise.sveltehas a typedisValidFillTxHashguard
The typed guard in Finalise.svelte is the right pattern — export and share it.
Code Quality & Readability
7. parseOrderBigInts silently falls through for StandardSolana
parseOrderBigInts dispatches on "originChainId" in order, which is true for both StandardOrder and StandardSolana. StandardSolana is silently routed into parseStandardOrderBigInts with no explicit branch or comment. Works today due to matching field shapes, but fragile.
8. solanaClaimFn double-invocation pattern
buttonFunction={async () => {
await solanaClaimFn(orderContainer)();
await postHookRefreshValidate();
}}The ()() is visually odd. Since there's only one call site, either inline it or have solanaClaimFn accept a post-callback.
9. Dynamic await import("@solana/web3.js") scattered across functions
deriveAttestationPda, deriveOrderContextPda, finaliseSolanaEscrow, and the Solana branch in flowProgress.ts each independently call await import("@solana/web3.js"). A shared lazy loader would be cleaner.
10. as any cast on IDL undocumented in solanaValidateLib.ts
solanaFinaliseLib.ts has a good comment explaining the idl as any and anchorWallet as any casts. The equivalent casts in solanaValidateLib.ts (submitProofToSolanaOracle) have only an eslint-disable comment, not an explanation.
11. Removal of try/catch in flowProgress.ts:getOrderProgressChecks
The PR removes the top-level try/catch that previously swallowed errors and returned {allFilled: false, allValidated: false, allFinalised: false}. Confirm call sites handle thrown exceptions — previously any error was silently swallowed, so this is a behaviour change.
Simplifications Available Higher in the Tree
12. Anchor setup boilerplate — highest-value extraction
solanaEscrowLib, solanaValidateLib, and solanaFinaliseLib each independently import and instantiate AnchorProvider, Program, PublicKey, build anchorWallet, cast the IDL, etc. A createSolanaAnchorProgram(idl, connection, walletAdapter, publicKey) factory would remove ~25 lines of identical setup per file and expose the actual business logic more clearly.
13. Attestation PDA derivation + fill log lookup always go together
The sequence:
getTransactionReceipt(fillHash)findOutputFilledLog(receipt, output)deriveAttestationPda({...match...})
Appears in ReceiveMessage.svelte:isValidatedSolana, ReceiveMessage.svelte:solanaValidateButtonFn, Finalise.svelte:solanaClaimFn, and flowProgress.ts:isOutputValidatedOnChain. Extract a resolveAttestationPda(output, fillHash, orderId) helper to halve the surface area.
Minor Nits
Finalise.sveltelines 335–338 casttokenRaw/amountRawtobigint | string | numberin the template. This defensive casting should be handled upstream byparseOrderBigIntsso the template can trust they'rebigint.ReceiveMessage.svelte:isValidatedSolanafetches a fresh receipt from RPC on every call, bypassingstore.getTransactionReceipt. Not a hot path (button-click only), but inconsistent with howflowProgress.tscaches the same data.
| if (!db) return; | ||
| const rows = await db!.select().from(intents); | ||
| this.orders = rows.map((r: any) => JSON.parse(r.data) as OrderContainer); | ||
| this.orders = rows.map((r: any) => parseOrderBigInts(JSON.parse(r.data) as OrderContainer)); |
There was a problem hiding this comment.
I hate how parseOrderBigInts is being used.
Can we expose some alternative function for JSON.parse with parseOrderBigInts instead?
| * orderId = keccak256(borsh(order)); seeds = [b"order_context", orderId] | ||
| * Used to check if the order has been finalised — PDA closed means finalised. | ||
| */ | ||
| export async function deriveOrderContextPda(order: StandardSolana): Promise<string> { |
There was a problem hiding this comment.
Why is this not part of the lower level intent.ts library?
There was a problem hiding this comment.
If we are doing that then i believe we should generalize and add every PDA derivation to intent.ts
| // Compute orderId = keccak256(borsh(order)) — uses the canonical @lifi/intent encoder | ||
| const encoded = borshEncodeSolanaOrder(order); | ||
| const orderIdHex = keccak256(encoded); | ||
| const orderId = Buffer.from(orderIdHex.slice(2), "hex"); |
There was a problem hiding this comment.
This is repeated in deriveOrderContextPda. Why is there no orderId function in the library or elsewhere.
|
Once fixes are in, request my review. |
|
@codex Review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da7a5a269e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| connection: getSolanaConnection( | ||
| store.mainnet ? chainMap.solanaMainnet.id : chainMap.solanaDevnet.id | ||
| ) |
There was a problem hiding this comment.
This proof submission path picks the Solana connection from store.mainnet instead of the order’s actual Solana input chain. If a user has a devnet Solana intent but later flips the UI to mainnet, submitProofToSolanaOracle is sent to mainnet while the rest of the flow (attestation derivation/status checks) still targets the order’s chain, causing deterministic proof-submission failures for that order.
Useful? React with 👍 / 👎.
| connection: getSolanaConnection( | ||
| store.mainnet ? chainMap.solanaMainnet.id : chainMap.solanaDevnet.id | ||
| ) |
There was a problem hiding this comment.
Finalisation also derives the Solana connection from store.mainnet rather than the selected order’s Solana chain. In mixed sessions (e.g., devnet order loaded, then network toggle changed), this sends finalise to the wrong cluster, so claims fail even though chain-status checks for the same order are performed against the correct input chain.
Useful? React with 👍 / 👎.
| nextStatuses[validationKey(inputChain, output)] = false; | ||
| } | ||
| } | ||
| validationStatuses = nextStatuses; | ||
| return; |
There was a problem hiding this comment.
For Solana intents, each refresh unconditionally rewrites every validation entry to false. Because a successful solanaValidateButtonFn call increments refreshValidation, previously validated outputs are immediately cleared, so multi-output orders cannot retain accumulated “validated” state in this screen and may never auto-advance based on local status.
Useful? React with 👍 / 👎.
2 participants
Summary
oracle_polymerprogram, then callsinput_settler_escrow.finalise()to release escrowed SPL tokenssolanaValidateLib.ts(Polymer proof submission + attestation PDA derivation),solanaFinaliseLib.ts(order_context PDA + finalise instruction), andreviveOrderBigInts.ts(bigint revival after DB round-trip)FillIntent,ReceiveMessage, andFinalisescreens with Solana-aware branchesFlow
fillOrderOutputs()so the on-chainOutputFilledevent records the Solana address/polymerAPI (4 attempts, exponential back-off), submits it tooracle_polymer.receive()on Solana devnet via Phantom walletOutputFilledevents from the fill receipt, derives attestation PDAs, callsinput_settler_escrow.finalise()to transfer escrowed SPL tokens to the solverKey constants
SOLANA_OUTPUT_SETTLER_PDA0xabb04f05...POLYMER_ORACLE.solanaDevnet0xe48a6f95...1151111081099712How to test
IssueIntentscreen, select a Solana devnet input token)Linked ticket
Closes V2-114
Expected Output
Screen.Recording.2026-04-09.at.2.29.13.PM.mov