test(e2e): Arbitrum USDT0 chain swaps#1499
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds complete end-to-end test infrastructure for Arbitrum stablecoin swaps. It includes CI workflow updates to inject RPC URLs, Playwright environment loading, fixtures for Arbitrum chain setup with RPC polling and TBTC liquidity management, regtest configuration for TBTC/USDT0 assets, and a comprehensive E2E test validating L-BTC→USDT0 swaps through UI and onchain claim. ChangesArbitrum Stablecoin E2E Test Suite
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
kiocos
force-pushed
the
harden-usdc-rescue
branch
5 times, most recently
from
175c73c to
75e31e5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@e2e/arbitrum/lbtcUsdt0.spec.ts`:
- Around line 19-24: The suite gating currently uses shouldRunArbitrumE2e()
which only checks CI=true, but the arbitrumWorker fixture still requires RPC env
vars; update the skip condition in describeArbitrumE2e to also check for the
presence of the Arbitrum configuration (e.g. process.env.ARBITRUM_RPC_URL or
whatever secret env(s) your arbitrumWorker expects) or replace
shouldRunArbitrumE2e() with a new predicate like hasArbitrumConfig() that
returns true only when the required RPC envs/secrets exist; ensure
describeArbitrumE2e calls that predicate before deciding between test.describe
and test.describe.skip so the spec self-skips when RPC vars are missing (or
alternatively remove this spec from CI shards that don't inject the RPC vars).
- Around line 32-35: The outer test timeout (swapClaimTestTimeout) is too small
relative to nested wait budgets (quoteReadinessTimeout, quoteRequestTimeout,
swapClaimTimeout) and can cause Playwright to kill slow-but-valid runs; update
swapClaimTestTimeout to exceed the sum of the nested waits (or otherwise set it
to a safe margin above the largest combined waits used in the test) so the outer
timeout cannot fire before the condition-specific timeouts; change the
constant(s) referenced (quoteRequestTimeout, quoteReadinessTimeout,
swapClaimTimeout, swapClaimTestTimeout) where they are declared and also adjust
the other occurrences mentioned (lines around 117-128, 156, 192-194) to use the
new larger test timeout value.
- Around line 179-187: After clicking the "address" button (locator:
page.locator("div[data-testid='pay-onchain-buttons']").getByText("address").click()),
wait until the clipboard actually contains a non-empty lockup address before
reading into lockupAddress; replace the immediate navigator.clipboard.readText()
with a wait/poll (e.g. page.waitForFunction or a short loop with retries) that
calls navigator.clipboard.readText() and resolves only when the returned string
is non-empty (and optionally trimmed) so that the subsequent
expect(lockupAddress) assertion validates the real copied value rather than a
stale/empty string.
In `@e2e/fixtures/arbitrum.ts`:
- Around line 36-37: The function arbitrumRpcUrl currently only reads
ARBITRUM_E2E_PORT and builds a localhost URL, ignoring the CI-provided full RPC
URL; change arbitrumRpcUrl to first return process.env.ARBITRUM_E2E_RPC_URL if
present, otherwise fall back to constructing
`http://127.0.0.1:${process.env.ARBITRUM_E2E_PORT ?? "18545"}` so CI can supply
a full URL via ARBITRUM_E2E_RPC_URL while local runs still use the port-based
default.
- Around line 66-71: shouldRunArbitrumE2e() currently returns true when
process.env.CI === "true" even if ARBITRUM_E2E_RPC_URL or ETHEREUM_E2E_RPC_URL
are missing, causing arbitrumWorker to throw; change the guard so CI does not
bypass configuration checks by making shouldRunArbitrumE2e() return
hasArbitrumE2eConfig() only (i.e., require envValue("ARBITRUM_E2E_RPC_URL") and
envValue("ETHEREUM_E2E_RPC_URL") to be set), or alternatively modify
arbitrumWorker to check hasArbitrumE2eConfig() and skip/throw with a clear
message; update references to shouldRunArbitrumE2e, hasArbitrumE2eConfig,
envValue and arbitrumWorker to reflect the chosen behavior.
In `@src/configs/regtest.ts`:
- Around line 87-90: The regtest config sets gasSponsor.normal and
gasSponsor.tor to an unreachable URL (http://localhost:18547/alchemy); update
these values in the gasSponsor config (gasSponsor.normal, gasSponsor.tor) to a
reachable gas-sponsor endpoint or wire them to a configurable environment
variable (e.g., process.env.GAS_SPONSOR_URL) so E2E swaps don't fail when the
local service isn't running — ensure both keys point to the correct
host:port/path or fallback to a known-working default.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9db34ecb-1068-4554-95bc-5c77b75d2db8
📒 Files selected for processing (6)
.github/workflows/CI.ymle2e/arbitrum/lbtcUsdt0.spec.tse2e/fixtures/arbitrum.tsplaywright.config.tsregtestsrc/configs/regtest.ts
| const describeArbitrumE2e = (title: string, callback: () => void) => { | ||
| if (shouldRunArbitrumE2e()) { | ||
| test.describe(title, callback); | ||
| } else { | ||
| test.describe.skip(title, callback); | ||
| } |
There was a problem hiding this comment.
Gate this suite on actual Arbitrum config, not just CI=true.
shouldRunArbitrumE2e() currently enables the suite on any CI worker, but the arbitrumWorker fixture still throws when the RPC env is absent. That is the current CI failure path, so this spec is not self-skipping in shards/jobs that lack the Arbitrum secrets. Please make this suite’s skip condition config-aware or keep it out of CI jobs that do not inject the required RPC vars.
🤖 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 `@e2e/arbitrum/lbtcUsdt0.spec.ts` around lines 19 - 24, The suite gating
currently uses shouldRunArbitrumE2e() which only checks CI=true, but the
arbitrumWorker fixture still requires RPC env vars; update the skip condition in
describeArbitrumE2e to also check for the presence of the Arbitrum configuration
(e.g. process.env.ARBITRUM_RPC_URL or whatever secret env(s) your arbitrumWorker
expects) or replace shouldRunArbitrumE2e() with a new predicate like
hasArbitrumConfig() that returns true only when the required RPC envs/secrets
exist; ensure describeArbitrumE2e calls that predicate before deciding between
test.describe and test.describe.skip so the spec self-skips when RPC vars are
missing (or alternatively remove this spec from CI shards that don't inject the
RPC vars).
Source: Pipeline failures
| await page | ||
| .locator("div[data-testid='pay-onchain-buttons']") | ||
| .getByText("address") | ||
| .click(); | ||
|
|
||
| const lockupAddress = await page.evaluate(() => | ||
| navigator.clipboard.readText(), | ||
| ); | ||
| expect(lockupAddress).toBeDefined(); |
There was a problem hiding this comment.
Wait for a non-empty copied lockup address before funding.
The clipboard is read immediately after the click, and toBeDefined() still passes for an empty or stale string. That makes this step race the copy handler and shifts the failure to sendtoaddress instead of the actual clipboard handoff.
Suggested fix
await page
.locator("div[data-testid='pay-onchain-buttons']")
.getByText("address")
.click();
- const lockupAddress = await page.evaluate(() =>
- navigator.clipboard.readText(),
- );
- expect(lockupAddress).toBeDefined();
+ await expect
+ .poll(
+ () => page.evaluate(() => navigator.clipboard.readText()),
+ { timeout: 5_000 },
+ )
+ .not.toBe("");
+
+ const lockupAddress = await page.evaluate(() =>
+ navigator.clipboard.readText(),
+ );🤖 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 `@e2e/arbitrum/lbtcUsdt0.spec.ts` around lines 179 - 187, After clicking the
"address" button (locator:
page.locator("div[data-testid='pay-onchain-buttons']").getByText("address").click()),
wait until the clipboard actually contains a non-empty lockup address before
reading into lockupAddress; replace the immediate navigator.clipboard.readText()
with a wait/poll (e.g. page.waitForFunction or a short loop with retries) that
calls navigator.clipboard.readText() and resolves only when the returned string
is non-empty (and optionally trimmed) so that the subsequent
expect(lockupAddress) assertion validates the real copied value rather than a
stale/empty string.
| const arbitrumRpcUrl = () => | ||
| `http://127.0.0.1:${process.env.ARBITRUM_E2E_PORT ?? "18545"}`; |
There was a problem hiding this comment.
Environment variable mismatch: CI provides full URL but code expects port.
The CI workflow (.github/workflows/CI.yml lines 77, 96) sets ARBITRUM_E2E_RPC_URL containing a full RPC URL, but this function only reads ARBITRUM_E2E_PORT and constructs http://127.0.0.1:${port}. The full RPC URL from the environment is never used.
Either:
- Parse the port from
ARBITRUM_E2E_RPC_URLwhen available, or - Use
ARBITRUM_E2E_RPC_URLdirectly if set, falling back to the constructed localhost URL
🔧 Proposed fix to use the full RPC URL from environment
const arbitrumRpcUrl = () =>
- `http://127.0.0.1:${process.env.ARBITRUM_E2E_PORT ?? "18545"}`;
+ process.env.ARBITRUM_E2E_RPC_URL ??
+ `http://127.0.0.1:${process.env.ARBITRUM_E2E_PORT ?? "18545"}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const arbitrumRpcUrl = () => | |
| `http://127.0.0.1:${process.env.ARBITRUM_E2E_PORT ?? "18545"}`; | |
| const arbitrumRpcUrl = () => | |
| process.env.ARBITRUM_E2E_RPC_URL ?? | |
| `http://127.0.0.1:${process.env.ARBITRUM_E2E_PORT ?? "18545"}`; |
🤖 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 `@e2e/fixtures/arbitrum.ts` around lines 36 - 37, The function arbitrumRpcUrl
currently only reads ARBITRUM_E2E_PORT and builds a localhost URL, ignoring the
CI-provided full RPC URL; change arbitrumRpcUrl to first return
process.env.ARBITRUM_E2E_RPC_URL if present, otherwise fall back to constructing
`http://127.0.0.1:${process.env.ARBITRUM_E2E_PORT ?? "18545"}` so CI can supply
a full URL via ARBITRUM_E2E_RPC_URL while local runs still use the port-based
default.
| export const hasArbitrumE2eConfig = () => | ||
| envValue("ARBITRUM_E2E_RPC_URL") !== undefined && | ||
| envValue("ETHEREUM_E2E_RPC_URL") !== undefined; | ||
|
|
||
| export const shouldRunArbitrumE2e = () => | ||
| process.env.CI === "true" || hasArbitrumE2eConfig(); |
There was a problem hiding this comment.
Fixture execution guard is inconsistent with CI gating logic.
shouldRunArbitrumE2e() returns true whenever CI=true, regardless of whether ARBITRUM_E2E_RPC_URL and ETHEREUM_E2E_RPC_URL are actually set. This causes the arbitrumWorker fixture (line 191-192) to throw "missing Arbitrum e2e RPC config" when these variables are unset in CI, leading to the pipeline failures shown in the logs.
The guard should require valid config even in CI mode, or tests must be conditionally skipped at the suite level when config is absent.
🔧 Proposed fix to enforce config presence in CI
export const shouldRunArbitrumE2e = () =>
- process.env.CI === "true" || hasArbitrumE2eConfig();
+ hasArbitrumE2eConfig();If CI runs should always attempt Arbitrum E2E (and fail when config is missing), then remove the hasArbitrumE2eConfig() check from the fixture:
async ({}, use) => {
- if (!hasArbitrumE2eConfig()) {
- throw new Error("missing Arbitrum e2e RPC config");
- }
-
const arbitrum = createArbitrum();And gate at the test level instead (the test already calls shouldRunArbitrumE2e() at line 20 in e2e/arbitrum/lbtcUsdt0.spec.ts).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const hasArbitrumE2eConfig = () => | |
| envValue("ARBITRUM_E2E_RPC_URL") !== undefined && | |
| envValue("ETHEREUM_E2E_RPC_URL") !== undefined; | |
| export const shouldRunArbitrumE2e = () => | |
| process.env.CI === "true" || hasArbitrumE2eConfig(); | |
| export const hasArbitrumE2eConfig = () => | |
| envValue("ARBITRUM_E2E_RPC_URL") !== undefined && | |
| envValue("ETHEREUM_E2E_RPC_URL") !== undefined; | |
| export const shouldRunArbitrumE2e = () => | |
| hasArbitrumE2eConfig(); |
🤖 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 `@e2e/fixtures/arbitrum.ts` around lines 66 - 71, shouldRunArbitrumE2e()
currently returns true when process.env.CI === "true" even if
ARBITRUM_E2E_RPC_URL or ETHEREUM_E2E_RPC_URL are missing, causing arbitrumWorker
to throw; change the guard so CI does not bypass configuration checks by making
shouldRunArbitrumE2e() return hasArbitrumE2eConfig() only (i.e., require
envValue("ARBITRUM_E2E_RPC_URL") and envValue("ETHEREUM_E2E_RPC_URL") to be
set), or alternatively modify arbitrumWorker to check hasArbitrumE2eConfig() and
skip/throw with a clear message; update references to shouldRunArbitrumE2e,
hasArbitrumE2eConfig, envValue and arbitrumWorker to reflect the chosen
behavior.
| gasSponsor: { | ||
| normal: "http://localhost:18547/alchemy", | ||
| tor: "http://localhost:18547/alchemy", | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if the gas sponsor service is running and responsive
GAS_SPONSOR_URL="http://localhost:18547/alchemy"
echo "Testing gas sponsor endpoint: $GAS_SPONSOR_URL"
HTTP_CODE=$(curl -s -o /dev/null -w "%{http_code}" "$GAS_SPONSOR_URL" || echo "Connection failed")
if [ "$HTTP_CODE" = "Connection failed" ]; then
echo "ERROR: Could not connect to gas sponsor service"
exit 1
elif [ "$HTTP_CODE" -ge 200 ] && [ "$HTTP_CODE" -lt 300 ]; then
echo "SUCCESS: Gas sponsor service is reachable (HTTP $HTTP_CODE)"
elif [ "$HTTP_CODE" -ge 400 ]; then
echo "WARNING: Gas sponsor service returned HTTP $HTTP_CODE"
fiRepository: BoltzExchange/boltz-web-app
Length of output: 296
🏁 Script executed:
#!/bin/bash
set -euo pipefail
GAS_SPONSOR_URL="http://localhost:18547/alchemy"
echo "Testing gas sponsor endpoint: $GAS_SPONSOR_URL"
HTTP_CODE="$(curl -sS -o /dev/null -w "%{http_code}" --max-time 5 "$GAS_SPONSOR_URL" 2>/dev/null || true)"
CURL_EXIT=$?
if [ $CURL_EXIT -ne 0 ]; then
echo "ERROR: Could not connect to gas sponsor service (curl exit code: $CURL_EXIT)"
exit 1
fi
if [ "$HTTP_CODE" -ge 200 ] && [ "$HTTP_CODE" -lt 300 ]; then
echo "SUCCESS: Gas sponsor service is reachable (HTTP $HTTP_CODE)"
else
echo "WARNING: Gas sponsor service returned HTTP $HTTP_CODE"
fiRepository: BoltzExchange/boltz-web-app
Length of output: 179
Fix/align regtest gasSponsor endpoint (http://localhost:18547/alchemy)
src/configs/regtest.ts sets both gasSponsor.normal and gasSponsor.tor to http://localhost:18547/alchemy, but the endpoint is not responding locally (curl reports HTTP 000), so gas sponsorship for swaps during E2E will fail until the gas sponsor service is running at that exact host/port/path or the config URL is updated.
🤖 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/configs/regtest.ts` around lines 87 - 90, The regtest config sets
gasSponsor.normal and gasSponsor.tor to an unreachable URL
(http://localhost:18547/alchemy); update these values in the gasSponsor config
(gasSponsor.normal, gasSponsor.tor) to a reachable gas-sponsor endpoint or wire
them to a configurable environment variable (e.g., process.env.GAS_SPONSOR_URL)
so E2E swaps don't fail when the local service isn't running — ensure both keys
point to the correct host:port/path or fallback to a known-working default.
|
converting to draft til dependency pr is merged |
2 participants
Depends on BoltzExchange/regtest#105
Summary by CodeRabbit
Tests
Chores