Skip to content

test(walletkit-e2e): USDT-on-Polygon Permit2 pay test#533

Draft
ignaciosantise wants to merge 4 commits into
mainfrom
feat/maestro-usdt-arbitrum-test
Draft

test(walletkit-e2e): USDT-on-Polygon Permit2 pay test#533
ignaciosantise wants to merge 4 commits into
mainfrom
feat/maestro-usdt-arbitrum-test

Conversation

@ignaciosantise

@ignaciosantise ignaciosantise commented Jun 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

What

Wires up the new USDT-on-Polygon (Permit2) Maestro pay test for wallets/rn_cli_wallet. The test flow itself lives in the shared actions repo — see WalletConnect/actions#97 — and this PR is the consumer side that runs it and keeps it repeatable.

⚠️ Draft / depends on WalletConnect/actions#97. maestro/pay-tests is temporarily pinned to that PR's head SHA (e78552d) so CI can exercise the new flow before it merges. Re-pin to the squash-merge commit once #97 lands.

Why Polygon (not Arbitrum)

The wallet just executes whatever actions the Pay backend returns; it does no local allowance check. USDT on Arbitrum (0xFd086…) is EIP-3009 (signature-based / gasless), so the backend never returns an on-chain approve action — no gas, no approval step, and the approve path we want to test would never run. USDT on Polygon (0xc2132D…) is a plain ERC-20, so WC Pay uses the Permit2 path (one-time approve(PERMIT2) + signature) — exactly the two-tx approve + pay flow under test.

Changes

  • LoadingView.tsx — add testID="pay-loading-setup-note" to the token-setup note. It renders only during first-time token setup (allowance == 0), so the test can observe the Permit2 approve step by id instead of matching a copy string.
  • walletkit-build-and-maestro/action.yml — after the Maestro run (if: always(), gated to the pay suite), reset the USDT-on-Polygon Permit2 allowance back to 0 via revoke-permit2-approval.js (--chainId eip155:137 --rpcUrl https://polygon-rpc.com), so every run re-exercises approve. Renames the input arbitrum-rpc-url → polygon-rpc-url. It's a Node step, not a Maestro runScript — the sandbox can't sign transactions. Pins pay-tests to the [example] Improve W3W Tutorial #97 head SHA.
  • revoke-permit2-approval.js — --walletAddress is now optional: derived from the private key when omitted, verified only when explicitly passed. So the teardown needs just the key + RPC (no extra var).
  • e2e-balance-check.yml — monitor USDT and POL (gas) on Polygon against the test wallet, Slack-alert when below threshold (USDT_POLYGON_THRESHOLD_UNITS, POLYGON_POL_THRESHOLD_WEI).
  • AGENTS.md — document the flow, the new testID, the allowance reset, and the Polygon rationale.

How to test

Trigger CI E2E WalletKit (workflow_dispatch) with tag pay-usdt-polygon (or default pay) on iOS + Android. Confirm the success screen is reached and the "Reset USDT Permit2 allowance (Polygon)" step ran. Requires the test wallet to hold USDT + a little POL (gas) on Polygon.

Open items

🤖 Generated with Claude Code

@ignaciosantise @claude
test(walletkit-e2e): wire up USDT-on-Arbitrum Permit2 pay test
cd3407e 
Consumer side of WalletConnect/actions#97 (adds pay_usdt_arbitrum). USDT is a
Permit2 token, so the wallet sends approve (allowance) then payment.

- LoadingView: add pay-loading-setup-note testID on the token-setup note so the
  test can observe the approve step by id instead of a copy string.
- walletkit-build-and-maestro action: after the Maestro run (if: always(), gated
  to the pay suite), reset the USDT-on-Arbitrum Permit2 allowance to 0 via
  revoke-permit2-approval.js so each run re-exercises approve. Add arbitrum-rpc-url
  input. Runs as a Node step (Maestro's runScript sandbox can't sign).
- revoke-permit2-approval.js: make --walletAddress optional (derive from the
  private key; verify only when explicitly passed).
- e2e-balance-check: monitor USDT + ETH (gas) on Arbitrum, Slack-alert when low.
- Pin maestro/pay-tests to the #97 head SHA for testing (re-pin on merge).
- AGENTS.md: document the flow, testID, and allowance reset.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jun 11, 2026 •
edited
Loading

Copy link
Copy Markdown

Claude finished @ignaciosantise's task in 2m 12s —— View job

PR Review

Found 3 issue(s)

Issue 1: pay-tests action pinned to unmerged PR head SHA

ID: brk-action-pay-tests-sha-unmerged-pr-c3f2
File: .github/actions/walletkit-build-and-maestro/action.yml:412
Severity: MEDIUM
Category: breaking_change

Context:

  • Pattern: pay-tests step uses WalletConnect/actions/maestro/pay-tests@3ccda39148c71e8d85ee8ae324d23a0b8b02f02f, a commit on the unmerged WalletConnect/actions#97 PR branch, not a master commit.
  • Risk: If the upstream PR branch is force-pushed, rebased, or the commit is GC'd before the PR merges, the SHA becomes unresolvable and all CI jobs using this action will break immediately.
  • Impact: All workflows calling .github/actions/walletkit-build-and-maestro — including the 6-hour scheduled pay tests and any cross-repo callers — will fail at the Copy shared Pay test flows step.
  • Trigger: Any CI run while WalletConnect/actions#97 is still unmerged.

Recommendation: Don't merge this PR until WalletConnect/actions#97 lands. Once it does, re-pin to the stable squash-merge commit on master before merging here. The PR description already tracks this; treat it as a hard merge blocker.

Issue 2: Allowance-reset step condition too broad — fires on all pay tests, not just pay-usdt-arbitrum

ID: action-permit2-reset-condition-too-broad-a7c1
File: .github/actions/walletkit-build-and-maestro/action.yml:579
Severity: LOW
Category: performance

Context:

  • Pattern: if: always() && contains(inputs.maestro-tags, 'pay') — contains() is a substring match, so pay-single-nokyc, pay-multiple-options-nokyc, and pay all trigger the reset.
  • Risk: For every non-USDT pay test run the script submits a real on-chain approve(Permit2, 0) transaction even when the allowance is already 0 (an idempotent but non-free operation). On Arbitrum L2 fees are cheap, but this runs on the 6-hour scheduler for every tag that includes pay.
  • Impact: Faster drain of the monitored ETH gas balance on Arbitrum than expected, and noise in the tx history.
  • Trigger: Any scheduled or dispatched run with a maestro-tags value other than pay-usdt-arbitrum (e.g. the default pay-single-nokyc).

Recommendation: Narrow the condition to the specific tag:

if: always() && contains(inputs.maestro-tags, 'pay-usdt-arbitrum')

Or make it configurable via a separate input (reset-permit2: true/false) to keep callers in control.

Issue 3: External domain URLs in workflow/action files

ID: workflow-external-rpc-domains-e3b0
File: .github/workflows/e2e-balance-check.yml:16-19, .github/actions/walletkit-build-and-maestro/action.yml:49
Severity: LOW (Non-blocking)
Category: security

Context:

  • Pattern: Public RPC endpoints from third-party domains used directly: https://mainnet.base.org, https://mainnet.optimism.io, https://arb1.arbitrum.io/rpc.
  • Risk: These are the canonical public endpoints for Base, Optimism, and Arbitrum, but they are rate-limited and unauthenticated. Under sustained CI load (6-hour schedule + test runs) they could throttle or return stale data, causing false alerts or flaky resets.
  • Impact: Non-critical for the balance-check workflow (read-only cast call/cast balance). For the reset step in action.yml, the description already recommends a custom RPC and the arbitrum-rpc-url input allows callers to override it.

Recommendation: Verify these domains are intentional and consider documenting in AGENTS.md that callers should supply arbitrum-rpc-url with a dedicated RPC key to avoid throttling on the allowance-reset step.

Data classification: No issues found.

@vercel

vercel Bot commented Jun 11, 2026 •
edited
Loading

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
pos-demo Ready Ready Preview, Comment Jun 11, 2026 7:03pm

Request Review

@ignaciosantise ignaciosantise mentioned this pull request Jun 11, 2026
Draft
@ignaciosantise @claude
refactor(walletkit-e2e): target USDT on Polygon instead of Arbitrum
04a43e4 
USDT on Arbitrum (0xFd086…) is EIP-3009 (signature-based, gasless), so WC Pay
never returns an on-chain approve action for it — the approve step the test is
meant to cover would never run. USDT on Polygon (0xc2132D…) is a plain ERC-20,
so WC Pay uses the Permit2 approve + pay path.

- Reset teardown now targets --chainId eip155:137 --rpcUrl https://polygon-rpc.com;
  rename the action input arbitrum-rpc-url -> polygon-rpc-url.
- balance-check: monitor USDT + POL (gas) on Polygon instead of USDC/ETH on Arbitrum.
- Bump maestro/pay-tests pin to the updated PR #97 head (pay_usdt_polygon).
- AGENTS.md: document the Polygon rationale.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ignaciosantise ignaciosantise changed the title test(walletkit-e2e): USDT-on-Arbitrum Permit2 pay test test(walletkit-e2e): USDT-on-Polygon Permit2 pay test Jun 11, 2026
@ignaciosantise @claude
fix(walletkit-e2e): use publicnode Polygon RPC (polygon-rpc.com is ga…
2a31fdc 
…ted in CI)

polygon-rpc.com returns HTTP 401 "tenant disabled" from CI runners, which made
the Permit2 allowance-reset step fail (eth_estimateGas) and would also break the
balance-check cast calls. Switch the reset default and the balance-check RPC to
https://polygon-bor-rpc.publicnode.com (keyless, returns 200).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ignaciosantise @claude
feat(pay): add stable network+token testID to payment options
555a267 
The payment-options screen keyed rows only by order-dependent index, with the
network in the accessibilityLabel — so a test couldn't deterministically pick a
specific asset+network (e.g. USDT on Polygon vs the same token on Arbitrum).

Add an additive `pay-option-${assetSymbol}-${networkName}` testID (e.g.
`pay-option-usdt-polygon`) via a thin wrapper, leaving `pay-option-${index}`
intact for the existing multi-options flow and other platforms. Bump the
maestro/pay-tests pin to the actions PR head that selects USDT-on-Polygon by
this id.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ignaciosantise