Skip to content

fix: source-chain RPC transports + quote-review polish - #341

Merged
snissn merged 7 commits into
feat/explorer-squid-fundingfrom
feat/topup-transports-polish
Aug 24, 2026
Merged

fix: source-chain RPC transports + quote-review polish#341
snissn merged 7 commits into
feat/explorer-squid-fundingfrom
feat/topup-transports-polish

Conversation

@jennijuju

@jennijuju jennijuju commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Requires: FilOzone/squid-evm-funding#38

Issue: https://bafybeiddrcxncd3qj5hvkanrwk24gygi7tjuof62n4ae5uqoinccjjpgmq.ipfs.inbrowser.link

Problem

Guided top-up can fail to load source-token catalogs because default public RPC endpoints rate-limit shared IPs, while browser calls to the Squid token catalog also consume the direct client rate-limit budget. The quote review also grouped bridge value and network gas and showed only the planned bridge value even though execution permits bounded drift.

Changes

  • add fallback RPC transports for supported source chains;
  • load the Squid token catalog through a cached same-origin server route plus a short browser cache;
  • show estimated and maximum bridge fees separately from network gas and other Squid fees;
  • pass the same reviewed maximum into dependency execution as maxTotalNativeRouteFee;
  • consume PR Weekly Tracker Improvements #38 through an exact prebuilt release asset with SHA-512 lockfile integrity, avoiding raw Git dependency build hooks;
  • restore the public Squid application ID fallback;
  • add deep-link, transport, proxy, fee-cap, and execution integration coverage;
  • run build, tests, and the now-real Explorer type-check on stacked pull requests.

Validation

Local frozen install, production Next build, full monorepo tests, full type-check, lint, and format pass. The Explorer suite has 19 files and 134 passing tests.

@jennijuju
jennijuju requested a review from snissn August 22, 2026 11:13
@vercel

vercel Bot commented Aug 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
filecoin-pay-explorer Ready Ready Preview Aug 24, 2026 9:38pm

Request Review

Comment thread apps/explorer/.env.example Outdated
@jennijuju jennijuju changed the title fix(explorer): source-chain RPC transports + quote-review polish fix (explorer): source-chain RPC transports + quote-review polish Aug 22, 2026
@jennijuju jennijuju changed the title fix (explorer): source-chain RPC transports + quote-review polish fix: source-chain RPC transports + quote-review polish Aug 22, 2026
@jennijuju jennijuju changed the title fix: source-chain RPC transports + quote-review polish fix: source-chain RPC transports + quote-review polish Aug 22, 2026
@snissn

snissn commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Looks good @jennijuju ! Thanks for testing, finding the rate limit RPC bug and applying a good fix with public RPC urls.

I applied a commit to revert the squid integrator ID change, and am doing a deep AI review before merging.

@snissn

snissn commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

codex review below. two technical correctness concerns.

I'll own the synthesis and revisions.

Codex review

Reviewed at head 2c8f81b.

I found two issues that should be addressed before this stack is treated as fixed:

  1. The transport change does not cover the reported token-catalog request. In this head, SquidQuoteReview loads the catalog through fetchSourceTokens, whose implementation performs a direct fetch to Squid's /tokens endpoint. It does not use wagmi's usePublicClient or the new createChainTransport map. Consequently, the transport unit tests prove endpoint coverage but not that the reported browser catalog failure is resolved. Please add a failing-before/passing-after browser or integration test (or a network trace) identifying the RPC request that gates catalog rendering, or move the fix to the actual catalog-request path.

  2. The displayed fee is presented as the transaction value even though the required execution patch may send more. bridgeFeeLabel shows the planned native fee exactly, while FilOzone/squid-evm-funding#38 changes execution to accept a refreshed native fee up to 50% above the reviewed fee. Please label this as an estimate and expose the maximum permitted value, or otherwise make the review display and execution cap agree.

Integration gate: this branch still resolves @filecoin-project/squid-evm-funding to 0.3.1, and dependency PR #38 remains open. Merging #38 alone will not change this lockfile; the explorer must bump or pin the dependency to an artifact containing that fix before the integrated ERC-20 flow is considered repaired.

Verification note: the live head has successful Code quality/Biome and Vercel statuses, but the GitHub Actions job did not execute Vitest or a type-check.

— Codex

@snissn

snissn commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Implemented the bottom review findings in 9f608c262585b24716eff26ed6b4206f05e5a76c:

  • routed Squid token-catalog loading through the same-origin cached /api/squid/tokens endpoint while keeping /route direct;
  • separated bridge route fees from network gas and other Squid fees, labeled the reviewed bridge fee as estimated, and displayed the exact maximum allowed at execution;
  • passed that same reviewed maximum to maxTotalNativeRouteFee, using the helper exported by the dependency fix;
  • pinned the exact PR Weekly Tracker Improvements #38 implementation as a prebuilt release artifact with SHA-512 integrity, avoiding raw Git dependency build hooks;
  • enabled build and type-check on stacked PRs, added the missing test workflow, and made the Explorer type-check script execute tsc with the existing SVG imports declared.

Latest-head CI is green: build, test, type-check, code quality, title, Socket, and Vercel all pass. Local frozen install, production build, full tests, type-check, lint, and format also pass.

@snissn

snissn commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

codex review passing:

Codex follow-up review

Reviewed current head 9f608c262585b24716eff26ed6b4206f05e5a76c.

No blocking findings. The concerns from the earlier Codex review at 2c8f81b are resolved on this head:

  • Squid token-catalog reads now use the same-origin /api/squid/tokens proxy; successful responses receive bounded server/browser caching, while upstream and proxy failures are not cached.
  • The quote review separates estimated bridge fees, the maximum bridge fee permitted at execution, network gas, and other Squid fees.
  • The displayed maximum is computed with dependency PR Weekly Tracker Improvements #38's exported maximumNativeRouteFee helper, using the same source-chain native-fee predicate and per-route rounding as execution, and that identical summed ceiling is passed as maxTotalNativeRouteFee.
  • The exact dependency implementation is pinned to the PR Weekly Tracker Improvements #38 release asset with SHA-512 lockfile integrity rather than relying on an unbuilt Git ref.
  • Build, Test, Type Check, and Code quality all complete successfully at this head; the Vercel deployment is also successful. Existing review threads are resolved.

Non-blocking lifecycle note: once FilOzone/squid-evm-funding#38 is released normally, replace the temporary PR asset URL with the published package version while retaining the lockfile-integrity guarantee.

— Codex

@snissn snissn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

looks good now

@github-project-automation github-project-automation Bot moved this from 📌 Triage to ✔️ Approved by reviewer in FOC Aug 24, 2026
@snissn
snissn force-pushed the feat/topup-transports-polish branch from 9f608c2 to 3e43fe4 Compare August 24, 2026 21:37
@snissn

snissn commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Authored by Codex.

The reviewed stack has now been restacked onto PR #320 and pushed at 3e43fe4.

  • The integrated token proxy and separate bridge/gas cap work is in 6269979.
  • The temporary PR artifact was replaced by the published @filecoin-project/squid-evm-funding 0.3.2 release in 3e43fe4.
  • Two independent Codex review passes found no blocking issue in the conflict resolution or execution-cap wiring.
  • Frozen install, production build, all repository tests, type-check, lint, format, and diff checks pass locally on this exact head.

GitHub checks are now running against the restacked head. I will merge this PR into #320 only after those exact-head checks pass.

@socket-security

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Added@​filecoin-project/​squid-evm-funding@​0.3.27810010094100

View full report

@snissn
snissn merged commit 3e43fe4 into feat/explorer-squid-funding Aug 24, 2026
10 checks passed
@github-project-automation github-project-automation Bot moved this from ✔️ Approved by reviewer to 🎉 Done in FOC Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🎉 Done

Development

Successfully merging this pull request may close these issues.

3 participants