Skip to content

fix(explorer): respect settlement receipts and destination FIL balance - #490

Open
snissn wants to merge 11 commits into
stagingfrom
codex/issue-472-479
Open

snissn wants to merge 11 commits into
stagingfrom
codex/issue-472-479

Conversation

@snissn

@snissn snissn commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #472
Closes #479

Summary

Decision

#479 replaces #472's "funded starts unchecked" with "funded sees no option". That brings back #377's balance-aware intent and keeps #444's checked default for an unfunded wallet. One shared dialog serves MetaMask and Privy. Following review, the pre-Pay balance re-read was dropped: FIL arriving less than 30 s before Pay can still buy a top-up the user had just approved (about 0.06 USDFC).

Screenshot

Low-FIL recipient in the Squid deposit dialog: the 0.05 FIL top-up is selected by default.

FIL top-up selected for a low-FIL recipient

Validation

  • Mainnet wallet validation on this PR head: FIL option off and on both settled successfully, with source and destination transaction links and contract reads in the evidence comment.
  • Focused tests for the three affected areas: 108 passed, including stale receipt data on an RPC error, approval-read recovery, fresh dialog opens, cached quotes, and balance changes before and during execution.
  • pnpm lint && pnpm format && pnpm test: passed.
  • pnpm type-check: passed.
  • pnpm build: passed with process-only placeholder values for NEXT_PUBLIC_SUBGRAPH_URL_MAINNET and NEXT_PUBLIC_SUBGRAPH_URL_CALIBRATION.

The public subgraph URL values are supplied only for local build-time prerendering; no environment file or deployment configuration changed.

@vercel

vercel Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated
filecoin-pay-explorer Ready Ready Preview Sep 24, 2026 11:00pm UTC

Request Review

@snissn

snissn commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator Author

Review of #490

The review ran on becaffa1. abcb32eb changes only the copy, so all logic findings below apply to the current head. Line numbers refer to becaffa1. Four reviewers covered settlement and signing, the dialog's FIL state machine, the dialog's UX and notices, and spec and mutation testing. Findings marked verified were confirmed by a separate adversarial pass. Findings marked reproduced were shown with a repro test or a mutation run, but their verifier was stopped before it finished.

The three fixes do what #472 and #479 ask. One real issue should be fixed before merge. The rest are small UX gaps and test gaps.

Should fix

1. Back during the confirm-time FIL pre-check does not cancel the payment. Verified and reproduced; found independently by three reviewers.
confirm() awaits destinationClient.getBalance (DirectSquidDepositDialog.tsx:719-731) before it sets any busy state. stage stays null, so Pay and Back stay enabled and nothing shows progress. If the Glif read is slow and the user clicks Back, the form comes back. When the read then returns insufficient, the closure still holds the old reviewed. The dialog then jumps to the progress view, switches chain, and requests signatures for a deposit the user backed out of. In the repro, executeSquidDeposit ran once and switchChain was called with [8453] and then [314] after Back. The wallet still asks for approval, so funds do not move silently, but the flow is wrong.
Fix: enter a busy state before the await, either setStage("preparing") or a dedicated "Checking FIL balance…" state that disables Back, Pay and Close. After the await, you can also bail out if the review was dismissed, for example by comparing a ref captured at click time.

Minor, introduced here

2. No indicator while the dialog waits for the fresh FIL balance. Verified.
While isFilGasTopUpEnabled === null, the FIL option, the quote, "Fetching a quote…" and quote errors are all hidden. The user sees fields and a disabled Review button with no explanation. With viem http() defaults (10 s timeout, retries) plus retry: 1, a hanging Glif can hold this state for tens of seconds; that figure is traced, not measured. The test at DirectSquidDepositDialog.test.tsx:659-676 asserts this silent state. Suggest a "Checking your FIL balance…" line, and a shorter bound on this gating read, since an error already falls back to the safe checked default.

3. Notice handling. Verified.

  • The gas-maximum notice from reReview() is not cleared when the review is dropped by the context-mismatch effect (:480) or by the pending-marker refresh (:528). It can then render in the form under the new "Quote updated" title (:1002), for example "Network gas rose above the reviewed maximum… confirm to send the Squid transaction". Before this PR, notice rendered only in the review card.
  • The FIL notice appears before any new quote exists and survives amount, token and chain edits. If the balance drops below the reserve again, it shows next to "not enough FIL" with the box checked.
  • The review card renders every notice under "Review the updated gas maximum" (:946). The comment at :183 still says notice feeds only the review card.

Suggest typing the notice ({ kind: "gas" | "fil", message }) so each kind renders only where it belongs, with a fitting title. Also clear the FIL notice when hasRecipientFil turns false, and hoist the duplicated string into a constant.

4. Race: a funded poll during the pre-check is ignored. Reproduced; narrow window.
The reviewed-invalidation effect (:484-488) skips while isSubmitting.current is true. Suppose FIL arrives and the 30 s poll sees it during the pre-check, but the pre-check read was issued earlier and returns 0n, or it fails. Execution then continues with filGasTopUp and buys FIL the wallet no longer needs. Suggest treating preCheckFunded || hasRecipientFilRef.current as funded.

5. The signing-freshness fix (#472 §2) is not pinned by any test. Verified; reproduced by two reviewers.
Putting the base Promise.all back into assertFreshSigningState leaves all 108 tests green, and the base squid-deposit-execution.ts passes the PR's test file 55/55. The new account and network tests (squid-deposit-execution.test.ts:672) use headSequence: [41n, 42n], so the lag is spent on the read at execution.ts:705, and the base code catches the change at the unretried read at :724 anyway. Use fakeSource({ headSequence: [42n, 41n, 42n], receiptBlock: 42n }), with sleep flipping the owner or chain, so the lag hits the final pre-route check. That case fails on base and passes here.

6. The settlement receipt.status check cannot run with the wagmi version in this repo. Verified; info or nit.
@wagmi/core 3.6.5 waitForTransactionReceipt throws when receipt.status === "reverted": it replays the call and throws with the revert reason. So useWaitForTransactionReceipt never reports isSuccess with a reverted receipt, and reverts already took the error path on base. The new guard is fine as a defensive check, but the "reverted" test at useRailSettlements.test.tsx:72-79 models a state wagmi cannot produce. Suggest:

  • a comment saying the guard is defensive;
  • a test in the real shape: isError: true, data: undefined, and an error like "Execution reverted with reason: …";
  • expect(onSettlementSuccess).not.toHaveBeenCalled() in the failure cases. Right now a mutant that calls onSettlementSuccess in the error branch passes all 5 tests.

7. Other test gaps found by mutation. Reproduced. Each of these mutations leaves all 108 tests green:

The dialog tests stub useQuery and setQueryData, so query-key equality and cache timing are untested. A reviewer confirmed with a real QueryClient that the current key written at :723 does match the live key; only coverage is missing. One integration test with a real QueryClient would close most of these gaps.

Pre-existing (not blocking, FYI)

  • A cancelled or replaced settlement shows as "Rail #N Settled". Verified. viem's checkReplacement defaults to on, and the Filecoin chain definitions don't disable it. A MetaMask "Cancel" resolves with the replacement's status: "success" receipt, which runs the success toast, the invalidation and onSettlementSuccess. This PR is about receipt status, so it's a natural place to fix: pass onReplaced and route cancelled and replaced to the error path, or also require receipt.to to be the payments contract.
  • Misreported context changes after approval. Verified; nit. If the context, account or network changes during an approval retry that never recovers, the user sees "allowance does not match…" after about 8 s instead of the context or signer message. Calling assertCurrentContext inside readSourceTokenStateAfterApproval would fix it.
  • A failed quote refetch after reopen leaves the old quote reviewable next to the error. Reproduced with a real QueryClient. Adding !quoteQuery.isError to canReview would fix it.

Notes

  • abcb32eb rewrites the explanatory copy that fix(explorer): harden settlement receipts, signing freshness, and FIL top-up defaults #472 says to keep. It fixes the "unavailable" wording but drops the "about a month of typical activity" rationale. Worth recording in the PR description as an intentional change.
  • assertFreshSigningState now adds one round trip. sourceClient.getChainId() could stay parallel with the state read.
  • Checked and clean: the key written by setQueryData matches the live key, including checksum casing; the retry stays bounded; deterministic failures stay outside the retry; the scope and version reset on reopen and on recipient change; and the query-key prefix still matches invalidateTopUpQueries.

🤖 Generated with Claude Code

@snissn

snissn commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up to the review: "Should fix" #1 is fixed in e4cae33.

confirm() now enters the preparing stage before the live FIL balance read, instead of inside the acquisition lock. While the read is pending, Back and Pay are disabled, Pay shows "Processing…", and the progress view shows "Preparing the route…". When the read reports funded, the stage resets and the form comes back with the existing notice. Other outcomes continue as before, and failures still reset through fail().

The test "keeps the review visible while its live balance check is pending" is replaced by "shows progress and disables Back while the live balance check is pending". It checks that Back and Pay are disabled during the read, and that after a funded result nothing runs, the notice shows, and Close is enabled again. It failed on abcb32eb and passes now.

Verified locally: Biome on the changed files, tsc --noEmit, and the explorer suite (654 tests) all pass. The minor items in the review are still open.

🤖 Generated with Claude Code

@snissn

snissn commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator Author

Superseded: Both checkbox states have now been executed and verified on mainnet from the current PR head. See the completed validation and transaction links. The text below records the earlier preflight only.

Mainnet validation preflight (PR head e4cae332, 2026-09-24)

The current PR deployment is https://filecoin-pay-explorer-5x4cz92jj-filoz.vercel.app/mainnet. Its /console stays at “Loading wallet”. The production Privy app's public configuration currently allows localhost and the staging domain, but not this preview hostname, so the preview cannot yet be used for the wallet test.

Using the same PR code and Squid integrator locally, I requested live, read-only mainnet quotes for a 0.001 Base ETH deposit into Filecoin Pay:

  • FIL option off: quoted minimum 2.632698 USDFC to the destination; Squid also returned executable transaction data in a separate request.
  • FIL option on: quoted minimum 2.574817 USDFC, with 0.05 FIL for the wallet budgeted at 0.057881 USDFC. A later executable-route request was rate limited by Squid (HTTP 429) after repeated preflight quotes.

These are time-sensitive quotes, not completed deposits. No wallet signature or transaction was submitted, and there are no explorer links yet. MetaMask Account 2 currently shows 0 Base ETH, so the two transaction tests await a funded source wallet and preview hostname authorization in Privy. I will test the FIL option off first, then on, because the successful top-up changes whether the checkbox is offered.

@snissn

snissn commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator Author

Follow-up to the review: the minor items (2–7) are addressed in 7967aae.

  • 2. Waiting for the FIL balance: the form shows "Checking your FIL balance…" while the fresh balance read holds back the quote. I didn't shorten the read's timeout, because a timed-out read on a funded wallet would fall back to the checked FIL option.
  • 3. Notices: notice is typed as { kind: "gas" | "fil", message }. The gas-maximum notice renders only on the review card, and the FIL notice only on the form, now titled "FIL top-up removed". The FIL notice is hidden once the wallet drops below the reserve again. Its message is one constant, and the stale comment is updated. The gas notice no longer reappears on the form after another tab's deposit drops the review.
  • 4. Poll race: FIL that a background poll sees during the pre-pay check (tracked with hasRecipientFilRef) is treated as funded, including when the check itself returns 0 or fails.
  • 5. Signing freshness: added account and network tests where only the pre-route read lags (headSequence: [42n, 41n, 42n]). They pass here and fail on 458c9b7.
  • 6. Settlements: added a comment saying the status guard is defensive, and a wagmi-revert case in wagmi 3's real shape (error, no data). Success and error callbacks are now asserted to each fire exactly once, and never both, including after a re-render.
  • 7. Mutation gaps: added tests for a manual opt-in when the wallet then becomes funded, balance polls that don't change whether FIL is needed, and hiding a cached quote while the balance decides. The gcTime: 0 / refetchOnMount options are now pinned by an assertion; the suite mocks useQuery, so that is a config check, not a behavioral one. Each targeted mutant is now killed.

Verified locally: Biome on the changed files, tsc --noEmit, and the explorer suite (663 tests) all pass. The pre-existing FYI items are unchanged.

🤖 Generated with Claude Code

@snissn

snissn commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator Author

Mainnet validation (PR head 7967aae7, 2026-09-24)

FIL option off

I ran the local Explorer at http://localhost:3000/console from this PR head against mainnet, using MetaMask Account 1 (0x6d688df8762994ea5f67462f76e80b890e4cf065). The account held about 0.03 FIL, above the dialog's 0.02 FIL fee reserve, so the FIL checkbox was correctly hidden.

  • Paid 1 USDC on Base. The review promised at least 0.982336445773088871 USDFC to the same account's Filecoin Pay balance, with a 0.000017322757598247 ETH maximum native requirement for approval, Squid fees, and network gas.
  • Base source transaction: receipt status = 1; the USDC transfer was 1,000,000 base units (1 USDC).
  • Filecoin destination transaction: receipt status = 1 and DepositRecorded credited 0.992760430291145903 USDFC to Account 1, above the reviewed minimum. The Filecoin Pay accounts(USDFC, Account 1) contract read returned 3.408176374716668960 USDFC after settlement. Axelar route.

The console's overview initially showed its earlier rounded 2.415 USDFC after the on-chain receipt, then refreshed to 3.408 USDFC. The receipt and contract read establish settlement independent of that indexing delay.

FIL option on

I used MetaMask Account 2 (0xf8996665eda6362456C775d9Bd4B9DF7013e5C30), which had 0 FIL before the test. The local dialog showed the FIL checkbox checked by default. I paid 0.00025 ETH on Base with it checked. The review promised at least 0.599314056550764083 USDFC into Account 2's Filecoin Pay balance and at least 0.05 FIL to its wallet, using 0.057853324155191815 USDFC from the route for the FIL top-up. The reviewed maximum native requirement was 0.000271243812914728 ETH.

  • Base source transaction: receipt status = 1. Its value was 0.000256350290926191 ETH including Squid fees; actual Base gas including the receipt's L1 fee was 0.000003148004263802 ETH, so the total 0.000259498295189993 ETH stayed below the reviewed maximum.
  • Filecoin destination transaction: receipt status = 1; the DepositRecorded event credited 0.606723308640577420 USDFC to Account 2, above the reviewed minimum. The Filecoin Pay accounts(USDFC, Account 2) read returned the same balance, and the wallet's native FIL balance rose from 0 to 0.062504460981723053 FIL, above the 0.05 FIL minimum. Axelar route.

The console displayed 0.606 USDFC after settlement. Squid's earlier HTTP 429 did not recur during either real-wallet quote or execution. Both checkbox states are now validated on mainnet against this PR head.

@snissn

snissn commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up review at 7967aae

Verdict: no new correctness issues. The "should fix" item and minor items 2–7 are addressed, and CI is green.

Rechecked in e4cae33 and 7967aae

  • confirm() enters preparing before the FIL pre-check. Every exit path resets it: the funded early return, fail(), and a synchronous assertContext throw. close() stays blocked by isSubmitting, so Back, Pay and Close cannot drop a review that is still paying.
  • hasRecipientFilRef can only be true at Pay time if the balance flipped during the check, since canReview already blocks a FIL quote for a funded wallet. Treating that as funded costs at most a requote.
  • The typed notice compiles cleanly: tsc finds no remaining string consumers. The gas notice renders only on the review card, and the FIL notice only on the form, gated on the live balance.
  • The new tests fail on the code before each fix. The signing tests fail on 458c9b7, and each targeted dialog and settlement mutant is killed.

CI: the first Test run on 7967aae failed in the subgraph package, not the explorer: Failed to get matchstick binary: Invalid version. Must be a string. The failed job's re-run passed.

Still open (pre-existing, not blocking)

  • A cancelled or replaced settlement is reported as "Rail #N Settled". viem's checkReplacement resolves with the replacement's success receipt.
  • A context, account or network change during an approval retry that never recovers is reported as an allowance mismatch after about 8 s.
  • After reopening, a failed quote refetch leaves the previous quote reviewable next to the error. canReview has no !quoteQuery.isError.

Housekeeping

  • The PR description is stale. It says "Focused tests … 108 passed", doesn't mention e4cae33 or 7967aae, and doesn't record the abcb32e copy change, which departs from fix(explorer): harden settlement receipts, signing freshness, and FIL top-up defaults #472's "keep the existing copy".
  • A funded result from the pre-pay check now briefly shows "Preparing the route…" before the form returns with the notice. That is intended, and worth one line in the description.
  • The dialog suite still mocks useQuery and keys the quote mock on queryKey.at(-1). Cache behavior is only asserted as config (gcTime: 0, refetchOnMount: "always"), not exercised.

🤖 Generated with Claude Code

@FilOzzy FilOzzy moved this from 📌 Triage to ⌨️ In Progress in FOC Sep 24, 2026
@snissn
snissn marked this pull request as ready for review September 24, 2026 06:45
@snissn

snissn commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator Author

Code looks clean, the interface looks good. I think the copy is all aligned with our standards. I ran mainnet txs with and without the checkbox and it worked as intended.

@SgtPooki SgtPooki 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.

The change to the text contradicts the asks in #472 and #479. The rest is test cleanup and one possible cut.

// Busy from the click: Back during the FIL read below would show the form while this payment carries on.
setSignature(null);
setStage("preparing");
if (reviewed.quote.filGasTopUp) {

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.

the effect on 489 already drops a FIL review when a poll sees FIL. Drop this read, the ref, and setQueryData?

@snissn snissn Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Keeping the confirm-time FIL read for the gap after the last poll and before Pay: read and cache update (L727-L745), covered by the FIL-arrives-before-Pay test (L885-L904). The ref covers a poll that completes during the read, including a stale or failed read: poll/read race test (L945-L983).

Comment thread apps/explorer/src/hooks/useRailSettlements.test.tsx Outdated
snissn and others added 3 commits October 12, 2026 20:17
Enter the preparing stage before the live FIL read in confirm(), so Back
and Pay are disabled while it is pending. Previously Back returned to the
form and the payment still ran once the read reported an unfunded wallet.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
snissn and others added 6 commits October 12, 2026 20:17
- Show "Checking your FIL balance…" while the fresh FIL read gates the quote.
- Type the dialog notice so gas-maximum notices stay on the review card and
  the FIL notice stays on the form, hidden once the wallet is low again.
- Treat FIL a background poll saw during the pre-pay check as funded.
- Pin the pre-route signer recheck, wagmi-shaped reverts, settlement
  callback exclusivity, FIL opt-in, balance polls, and quote masking.
…ts/DirectSquidDepositDialog.tsx

Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>
…ts/DirectSquidDepositDialog.tsx

Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>
…id-deposit-execution.test.ts

Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>
…ts/DirectSquidDepositDialog.test.tsx

Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 12, 2026 16:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@vercel

vercel Bot commented Oct 16, 2026

Copy link
Copy Markdown

Deployment failed for project filecoin-pay-explorer with the following error:

Creating the Deployment Timed Out.

Drop the confirm-time FIL re-read, its ref and setQueryData, as asked
in review; the balance poll already drops a FIL review once FIL lands.
Derive "fresh balance not read yet" from the query's isPending instead
of mirroring it into state through an effect.

This branch was successfully deployed

1 active deployment
Preview — d3a65b85 Deployed Oct 28, 2026 by silent-cipher via Deploy #346
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ⌨️ In Progress

4 participants