refactor: simplify Squid funding API - #10
Conversation
📝 WalkthroughWalkthroughThe PR updates the library to Squid v2. Planning resolves source tokens and returns a ChangesSquid v2 funding workflow
Packaging validation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
|
@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: 831c922c7e
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/execution.ts (1)
392-421: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAttach completed route hashes to the error when a later leg fails.
routesis a local array. When a leg throws after an earlier leg already broadcast and settled, the error propagates androutesis discarded. The host then knows that funding partially completed, but it does not receive the transaction hashes or requirement IDs of the settled legs. Manual chain search becomes the only recovery path. Expose the completed routes on the thrown error so the host can reconcile.This applies to every throw site after the first successful
send, including "Squid route failed" at line 410 and "did not complete within the poll limit" at line 419.♻️ Proposed approach
export class SquidPartialExecutionError extends Error { constructor( message: string, readonly routes: readonly { requirementId: string; transactionHash: Hash }[], options?: ErrorOptions, ) { super(message, options) this.name = "SquidPartialExecutionError" } }Wrap the per-leg body in
try/catch, and rethrow withnew SquidPartialExecutionError(message, routes, { cause: error })whenroutes.length > 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/execution.ts` around lines 392 - 421, Preserve completed route metadata when a later leg fails: add the proposed SquidPartialExecutionError type, then wrap each per-leg execution flow after the first successful send in try/catch and rethrow this error with the current routes and original error as cause when routes is non-empty. Update all relevant failure paths, including “Squid route failed” and the poll-limit failure, while leaving initial-leg errors unchanged.
🤖 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 `@src/planner.ts`:
- Around line 32-38: Extend the validation in planSquidFunding around the
existing sourceChainId and requirements checks to reject duplicate requirement
IDs and requirements targeting more than one destination chain. Match the
corresponding validation rules used by executeSquidFunding, ensuring invalid
plans fail before any provider or route requests.
In `@src/squid.ts`:
- Around line 81-92: Add optional timeoutMs to SquidClientOptions and have the
shared client helper create an AbortSignal.timeout(timeoutMs) for requests. In
src/squid.ts lines 81-92, update fetchSourceTokens’ /tokens request to pass that
signal; likewise update the /status request at lines 225-228. Ensure both
requests use the configured deadline while preserving existing headers and
behavior.
In `@src/types.ts`:
- Around line 61-68: Remove prepareTransactionRequest from the SquidPublicClient
Pick in the SquidPublicClient type. Update any callers that require this wallet
action to use the appropriate wallet-client abstraction instead, while
preserving the remaining PublicClient methods.
---
Outside diff comments:
In `@src/execution.ts`:
- Around line 392-421: Preserve completed route metadata when a later leg fails:
add the proposed SquidPartialExecutionError type, then wrap each per-leg
execution flow after the first successful send in try/catch and rethrow this
error with the current routes and original error as cause when routes is
non-empty. Update all relevant failure paths, including “Squid route failed” and
the poll-limit failure, while leaving initial-leg errors 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c1d6a719-fbca-4c08-a1ca-3d88cff63191
📒 Files selected for processing (11)
README.mdpackage.jsonscripts/pack-check.mjssrc/catalog.tssrc/execution.test.tssrc/execution.tssrc/index.test.tssrc/index.tssrc/planner.tssrc/squid.tssrc/types.ts
💤 Files with no reviewable changes (1)
- scripts/pack-check.mjs
|
Review follow-up at
Local validation passes: lint, typecheck, 18 tests, build, package dry-run, and |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba3c5bba20
ℹ️ 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".
What changed
This reduces
squid-evm-fundingto the API needed by Filecoin Pin:planSquidFundingresolves a source token from Squid's current token catalog and plans exact FIL and USDFC shortfalls.executeSquidFundingvalidates refreshed routes, allowances, balances, transaction fees, receipts, provider status, and destination balances.The retained checks cover arbitrary catalog tokens, native tokens, route identity, trusted targets and spenders, exact allowances, standard and OP Stack native-fee ceilings, wallet and chain identity, nonce drift, partial failure, and sensitive-value redaction.
This is consumed by filecoin-project/filecoin-pin#637.
Validation
pnpm lintpnpm typecheckpnpm test— 18 passing testspnpm buildpnpm pack:checkThe change is not performance-oriented. It removes internal abstractions without changing the network transaction contract, so no performance benchmark is required.
Size