test: cross chain e2e tests - #420
Open
0xLaruku wants to merge 5 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
PR Summary by QodoCross-chain Playwright E2E tests against real testnets (no Anvil fork) Description
Diagram
High-Level Assessment
Files changed (7)
|
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
Code Review by Qodo
1. Unmocked OIF stalls E2E
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| const e2eTestProvider = createE2EProvider({ | ||
| chains: ALL_CHAINS, | ||
| rpcUrls, | ||
| account: (process.env.NEXT_PUBLIC_E2E_PRIVATE_KEY as `0x${string}`) || '', |
There was a problem hiding this comment.
1. Client-bundled private key 🐞 Bug ⛨ Security
The cross-chain E2E connector reads process.env.NEXT_PUBLIC_E2E_PRIVATE_KEY in client-side code, so any build produced with that env set will ship the private key to the browser runtime where it can be extracted. CI workflows also inject this secret into build/test steps, increasing the chance of accidental exposure via build artifacts/logging/misdeployment.
Agent Prompt
## Issue description
`NEXT_PUBLIC_E2E_PRIVATE_KEY` is consumed in client-side code to create the walletless E2E provider. If that env var is present during a build that is deployed/shared, the resulting client bundle/runtime contains the private key and it can be extracted.
## Issue Context
This PR also passes `NEXT_PUBLIC_E2E_PRIVATE_KEY` into CI build/test workflows, making it more likely the key ends up embedded in built assets.
## Fix
Refactor so the private key is not part of the Next.js client bundle:
- Do **not** use a `NEXT_PUBLIC_*` env var for a private key.
- Prefer passing the key only to the Playwright runner and injecting it at runtime (e.g., `page.addInitScript(...)` to set a global like `globalThis.__E2E_PRIVATE_KEY`, then have the E2E connector read that global only when `NEXT_PUBLIC_E2E === 'true'`).
- Remove passing the secret to non-E2E build steps unless strictly required.
## Fix Focus Areas
- examples/ui/app/cross-chain/config/e2eConnector.ts[1-26]
- .github/workflows/build-lint.yml[34-39]
- .github/workflows/test-e2e.yml[43-49]
- examples/ui/playwright.config.ts[1-36]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="examples/ui/app/cross-chain/config/e2eConnector.ts">
<violation number="1" location="examples/ui/app/cross-chain/config/e2eConnector.ts:10">
P1: Using `NEXT_PUBLIC_` prefix for a private key causes Next.js to inline it into the client-side JavaScript bundle at build time. Any visitor to a deployed build can extract it from the static JS assets. Use a non-prefixed env var and inject it only at test runtime (e.g., via Playwright's `page.addInitScript` or a server-only route) to keep the key out of client bundles.</violation>
<violation number="2" location="examples/ui/app/cross-chain/config/e2eConnector.ts:10">
P2: Custom agent: **TypeScript & React Standards**
Missing runtime validation for NEXT_PUBLIC_E2E_PRIVATE_KEY env variable; a compile-time `as` assertion provides no runtime guarantees. Use Zod (or equivalent) to validate the env var and fail fast with a clear error.</violation>
</file>
<file name=".github/workflows/build-lint.yml">
<violation number="1" location=".github/workflows/build-lint.yml:38">
P1: Private key is passed as a `NEXT_PUBLIC_*` build env, which exposes it to client bundles. Use a server-only env name and keep signing key access out of browser-facing code/build-time public envs.</violation>
</file>
<file name="examples/ui/tests/cross-chain.spec.ts">
<violation number="1" location="examples/ui/tests/cross-chain.spec.ts:33">
P2: Tests hard-code a specific wallet address even though the E2E signer is env-configured and rotatable, making CI brittle on key rotation.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| const e2eTestProvider = createE2EProvider({ | ||
| chains: ALL_CHAINS, | ||
| rpcUrls, | ||
| account: (process.env.NEXT_PUBLIC_E2E_PRIVATE_KEY as `0x${string}`) || '', |
Contributor
There was a problem hiding this comment.
P1: Using NEXT_PUBLIC_ prefix for a private key causes Next.js to inline it into the client-side JavaScript bundle at build time. Any visitor to a deployed build can extract it from the static JS assets. Use a non-prefixed env var and inject it only at test runtime (e.g., via Playwright's page.addInitScript or a server-only route) to keep the key out of client bundles.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/ui/app/cross-chain/config/e2eConnector.ts, line 10:
<comment>Using `NEXT_PUBLIC_` prefix for a private key causes Next.js to inline it into the client-side JavaScript bundle at build time. Any visitor to a deployed build can extract it from the static JS assets. Use a non-prefixed env var and inject it only at test runtime (e.g., via Playwright's `page.addInitScript` or a server-only route) to keep the key out of client bundles.</comment>
<file context>
@@ -7,6 +7,7 @@ import type { Chain } from 'viem/chains';
const e2eTestProvider = createE2EProvider({
chains: ALL_CHAINS,
rpcUrls,
+ account: (process.env.NEXT_PUBLIC_E2E_PRIVATE_KEY as `0x${string}`) || '',
});
</file context>
| run: pnpm build | ||
| env: | ||
| MAINNET_RPC_URL: ${{ secrets.MAINNET_RPC_URL }} | ||
| NEXT_PUBLIC_E2E_PRIVATE_KEY: ${{ secrets.NEXT_PUBLIC_E2E_PRIVATE_KEY }} |
Contributor
There was a problem hiding this comment.
P1: Private key is passed as a NEXT_PUBLIC_* build env, which exposes it to client bundles. Use a server-only env name and keep signing key access out of browser-facing code/build-time public envs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/build-lint.yml, line 38:
<comment>Private key is passed as a `NEXT_PUBLIC_*` build env, which exposes it to client bundles. Use a server-only env name and keep signing key access out of browser-facing code/build-time public envs.</comment>
<file context>
@@ -35,6 +35,7 @@ jobs:
run: pnpm build
env:
MAINNET_RPC_URL: ${{ secrets.MAINNET_RPC_URL }}
+ NEXT_PUBLIC_E2E_PRIVATE_KEY: ${{ secrets.NEXT_PUBLIC_E2E_PRIVATE_KEY }}
- name: Check types
</file context>
| const e2eTestProvider = createE2EProvider({ | ||
| chains: ALL_CHAINS, | ||
| rpcUrls, | ||
| account: (process.env.NEXT_PUBLIC_E2E_PRIVATE_KEY as `0x${string}`) || '', |
Contributor
There was a problem hiding this comment.
P2: Custom agent: TypeScript & React Standards
Missing runtime validation for NEXT_PUBLIC_E2E_PRIVATE_KEY env variable; a compile-time as assertion provides no runtime guarantees. Use Zod (or equivalent) to validate the env var and fail fast with a clear error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/ui/app/cross-chain/config/e2eConnector.ts, line 10:
<comment>Missing runtime validation for NEXT_PUBLIC_E2E_PRIVATE_KEY env variable; a compile-time `as` assertion provides no runtime guarantees. Use Zod (or equivalent) to validate the env var and fail fast with a clear error.</comment>
<file context>
@@ -7,6 +7,7 @@ import type { Chain } from 'viem/chains';
const e2eTestProvider = createE2EProvider({
chains: ALL_CHAINS,
rpcUrls,
+ account: (process.env.NEXT_PUBLIC_E2E_PRIVATE_KEY as `0x${string}`) || '',
});
</file context>
| const recipientInput = page.getByRole('textbox', { name: 'Recipient Address' }); | ||
| await expect(recipientInput).toHaveValue('0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266'); | ||
|
|
||
| await expect(recipientInput).toHaveValue('0xc59c92D9d6064464280B621C42A6ECDa0EA2D29b'); |
Contributor
There was a problem hiding this comment.
P2: Tests hard-code a specific wallet address even though the E2E signer is env-configured and rotatable, making CI brittle on key rotation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/ui/tests/cross-chain.spec.ts, line 33:
<comment>Tests hard-code a specific wallet address even though the E2E signer is env-configured and rotatable, making CI brittle on key rotation.</comment>
<file context>
@@ -123,36 +24,13 @@ test.describe('Asset Discovery', () => {
const recipientInput = page.getByRole('textbox', { name: 'Recipient Address' });
- await expect(recipientInput).toHaveValue('0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266');
+
+ await expect(recipientInput).toHaveValue('0xc59c92D9d6064464280B621C42A6ECDa0EA2D29b');
});
</file context>
Comment on lines
+3
to
5
| test.beforeEach(async ({ page }) => { | ||
| await page.goto('/cross-chain?testnet=true'); | ||
| }); |
There was a problem hiding this comment.
1. Unmocked oif stalls e2e 🐞 Bug ☼ Reliability
cross-chain.spec.ts no longer aborts OIF network traffic, but the app still instantiates the OIF provider and eagerly prefetches discoverAssets(), making page readiness dependent on https://oif-api.openzeppelin.com/api latency/availability and causing Playwright timeouts/flakes. This can impact multiple tests because the warm-cache prefetch happens as soon as an executor is created, not only when OIF is explicitly selected.
Agent Prompt
## Issue description
The E2E suite removed the request interception that previously aborted OIF API calls, but the app still constructs an OIF provider and prefetches discovery. This makes E2E runs sensitive to OIF endpoint slowness/outages, leading to long hangs and flaky timeouts.
## Issue Context
Even if tests primarily exercise Across/Relay/LI.FI, `getExecutor()` prefetches discovery for both executor instances, which can trigger OIF network traffic.
## Fix Focus Areas
- examples/ui/tests/cross-chain.spec.ts[1-6]
- examples/ui/app/cross-chain/services/sdk.ts[17-97]
## Suggested fix
Reintroduce a fast-fail route abort for OIF in Playwright (e.g., `context.route('**/oif-api.openzeppelin.com/**', route => route.abort('blockedbyclient'))`), or add an E2E-only switch in `buildExecutor()` to skip adding the OIF provider during E2E runs.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 23b58af |
|
Code review by qodo was updated up to the latest commit 56ecea2 |
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="examples/ui/app/cross-chain/config/e2eConnector.ts">
<violation number="1" location="examples/ui/app/cross-chain/config/e2eConnector.ts:10">
P1: Using a `NEXT_PUBLIC_` prefix for a private key causes Next.js to inline the value into the client-side JavaScript bundle at build time. Anyone inspecting the browser bundle can extract the key. Use a non-prefixed env var and inject the key at runtime (e.g., via Playwright's `page.addInitScript`) instead of baking it into the build output.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| const e2eTestProvider = createE2EProvider({ | ||
| chains: ALL_CHAINS, | ||
| rpcUrls, | ||
| account: (process.env.NEXT_PUBLIC_E2E_PRIVATE_KEY as `0x${string}`) || undefined, |
Contributor
There was a problem hiding this comment.
P1: Using a NEXT_PUBLIC_ prefix for a private key causes Next.js to inline the value into the client-side JavaScript bundle at build time. Anyone inspecting the browser bundle can extract the key. Use a non-prefixed env var and inject the key at runtime (e.g., via Playwright's page.addInitScript) instead of baking it into the build output.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/ui/app/cross-chain/config/e2eConnector.ts, line 10:
<comment>Using a `NEXT_PUBLIC_` prefix for a private key causes Next.js to inline the value into the client-side JavaScript bundle at build time. Anyone inspecting the browser bundle can extract the key. Use a non-prefixed env var and inject the key at runtime (e.g., via Playwright's `page.addInitScript`) instead of baking it into the build output.</comment>
<file context>
@@ -7,7 +7,7 @@ import type { Chain } from 'viem/chains';
chains: ALL_CHAINS,
rpcUrls,
- account: (process.env.NEXT_PUBLIC_E2E_PRIVATE_KEY as `0x${string}`) || '',
+ account: (process.env.NEXT_PUBLIC_E2E_PRIVATE_KEY as `0x${string}`) || undefined,
});
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test suite reorganization
demo-limits.spec.tsandmint.spec.tsintocross-chain.spec.tsas theDemo limitsandMint mockUSDCdescribe blocks; deleted the standalone files.addresses-build.spec.ts: select theEthereumchain option via.last()instead of.first()to target the intended dropdown entry.Walletless config
e2eConnector.ts: the E2E signing account is now sourced fromNEXT_PUBLIC_E2E_PRIVATE_KEY, so the headless connector signs as a real, funded testnet wallet.CI
test-e2e.yml: removed the Foundry install step and theNEXT_PUBLIC_ANVIL_URL/NEXT_PUBLIC_ANVIL_CHAIN_IDenv;build-lint.yml: passNEXT_PUBLIC_E2E_PRIVATE_KEYto the build step.playwright.config.ts/.env.e2e: removed the Anvil forkwebServerand the Anvil env vars.Required setup (before merge)
These repo secrets must exist for CI to pass:
NEXT_PUBLIC_E2E_PRIVATE_KEY— a dedicated, low-balance, rotatable testnet account, funded with testnet ETH on every chain the flow touches (gas for mint + intent execution).