feat(e2e): add account balance monitoring and pre-test checks#4276
Open
feat(e2e): add account balance monitoring and pre-test checks#4276
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
4 Skipped Deployments
|
Contributor
WalkthroughReworks e2e balance tooling: adds address derivation and wallet utilities, centralized per-account balance requirements, live-price USD→token conversion, a balance-check CLI, and refactors balance-checker to use address/privateKey inputs; updates tests to derive addresses and perform address-based balance verification with min/warn thresholds. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Suite
participant Checker as balance-checker (utils)
participant Wallet as wallet-utils
participant REST as REST API
participant SQS as SQS API
Test->>Checker: ensureBalances(privateKeyOrAddress, requirements)
Checker->>Wallet: resolveAddress(privateKeyOrAddress)
Wallet-->>Checker: address
alt requirements include USD units
Checker->>SQS: fetchTokenPrices(denoms)
SQS-->>Checker: price map (USDC)
Checker->>Checker: convert USD requirements → token amounts
end
Checker->>REST: GET /bank/balances/{address}
REST-->>Checker: balances
Checker->>Checker: compare balances vs minAmount/warnAmount
alt all >= warnAmount
Checker-->>Test: pass (exit 0)
else any < minAmount
Checker-->>Test: critical (exit 1)
else any < warnAmount
Checker-->>Test: warning (exit 2)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Member
Author
|
R4R @JohnnyWyles or @JoseRFelix ... nothing other than optimizations to test workflows, need to merge so that i can test them properly |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/e2e/utils/balance-checker.ts (2)
148-201: Duplicated fetch logic and over-fetching ingetBalanceTwo issues in this block:
Code duplication: The URL construction,
fetch,response.okguard, andresponse.json()cast are copied verbatim betweengetBalance(lines 152–161) andgetAllBalances(lines 179–188). Extracting a sharedfetchAllBalances(address)helper would keep the two public functions thin.Over-fetching in
getBalance: The function retrieves up to 1,000 denominations to find one. The Cosmos Bank REST API exposes a targeted single-denom endpoint:GET /cosmos/bank/v1beta1/balances/{address}/{denom}IBC denoms (e.g.
ibc/498A0751...) contain a/, so the denom must be percent-encoded (encodeURIComponent(denom)) before interpolation.♻️ Proposed refactor — shared helper + targeted single-denom endpoint
+async function fetchRawBalances( + address: string +): Promise<Array<{ denom: string; amount: string }>> { + const url = `${REST_ENDPOINT}/cosmos/bank/v1beta1/balances/${address}?pagination.limit=1000`; + const response = await fetch(url, { signal: AbortSignal.timeout(30_000) }); + if (!response.ok) { + throw new Error( + `Failed to fetch balances: ${response.status} ${response.statusText}` + ); + } + const data = (await response.json()) as BalanceResponse; + return data.balances; +} export async function getBalance( address: string, denom: string ): Promise<number> { - const url = `${REST_ENDPOINT}/cosmos/bank/v1beta1/balances/${address}?pagination.limit=1000`; - const response = await fetch(url, { signal: AbortSignal.timeout(30_000) }); - if (!response.ok) { - throw new Error( - `Failed to fetch balances: ${response.status} ${response.statusText}` - ); - } - const data = (await response.json()) as BalanceResponse; - const balance = data.balances.find((b) => b.denom === denom); + const url = `${REST_ENDPOINT}/cosmos/bank/v1beta1/balances/${address}/${encodeURIComponent(denom)}`; + const response = await fetch(url, { signal: AbortSignal.timeout(30_000) }); + if (!response.ok) { + if (response.status === 404) return 0; // denom not held + throw new Error( + `Failed to fetch balance: ${response.status} ${response.statusText}` + ); + } + const data = (await response.json()) as { balance: { denom: string; amount: string } }; + const balance = data.balance; if (!balance) return 0; const tokenInfo = Object.values(TOKEN_DENOMS).find( (t) => t.denom === denom ); const decimals = tokenInfo?.decimals ?? 6; return parseFloat(balance.amount) / Math.pow(10, decimals); } export async function getAllBalances( address: string ): Promise<Record<string, number>> { - const url = `${REST_ENDPOINT}/cosmos/bank/v1beta1/balances/${address}?pagination.limit=1000`; - const response = await fetch(url, { signal: AbortSignal.timeout(30_000) }); - if (!response.ok) { - throw new Error( - `Failed to fetch balances: ${response.status} ${response.statusText}` - ); - } - const data = (await response.json()) as BalanceResponse; const result: Record<string, number> = {}; - for (const { denom, amount } of data.balances) { + for (const { denom, amount } of await fetchRawBalances(address)) { const raw = parseFloat(amount); if (raw === 0) continue; const tokenInfo = Object.values(TOKEN_DENOMS).find( (t) => t.denom === denom ); const decimals = tokenInfo?.decimals ?? 6; result[denom] = raw / Math.pow(10, decimals); } return result; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/e2e/utils/balance-checker.ts` around lines 148 - 201, Extract the duplicated REST call and JSON parsing into a shared helper (e.g., fetchAllBalances(address)) used by getAllBalances, and refactor getBalance to call the Cosmos single-denom endpoint instead of fetching all balances: construct URL `${REST_ENDPOINT}/cosmos/bank/v1beta1/balances/${address}/${encodeURIComponent(denom)}` (use encodeURIComponent for IBC denoms), perform the same response.ok check and JSON cast, then apply the same TOKEN_DENOMS decimals lookup and return 0 when missing; keep unique symbols getBalance and getAllBalances thin and reuse the new helper to avoid duplication.
247-251: Unknown token in a USD requirement silently aborts all balance checks
throw new Error(\Unknown token: ${r.token}`)at line 249 propagates out ofresolveUsdRequirements. Thecatchat line 314–318 ofensureBalances` returns immediately, silently skipping every remaining requirement — including valid token-unit ones — for that wallet. A typo in one USD requirement name suppresses all other checks.♻️ Proposed fix — warn and skip the bad requirement instead of throwing
- const denoms = usdReqs.map((r) => { - const info = TOKEN_DENOMS[r.token]; - if (!info) throw new Error(`Unknown token: ${r.token}`); - return info.denom; - }); + const validUsdReqs = usdReqs.filter((r) => { + if (!TOKEN_DENOMS[r.token]) { + console.warn(` Unknown token "${r.token}" in USD requirement; skipping.`); + return false; + } + return true; + }); + if (validUsdReqs.length === 0) return resolved; + const denoms = validUsdReqs.map((r) => TOKEN_DENOMS[r.token]!.denom);Then replace the
usdReqsreference inside theforloop withvalidUsdReqs:- for (const req of usdReqs) { + for (const req of validUsdReqs) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/e2e/utils/balance-checker.ts` around lines 247 - 251, The resolver currently throws on an unknown token inside resolveUsdRequirements (using TOKEN_DENOMS and usdReqs), which bubbles up to ensureBalances and aborts all checks; change resolveUsdRequirements to log or warn and skip invalid entries instead of throwing so it returns only validUsdReqs, and then update the for loop in ensureBalances to iterate over validUsdReqs (or the returned filtered list) rather than the original usdReqs so a single typo doesn't cancel all checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/e2e/utils/balance-checker.ts`:
- Around line 231-232: The module-level PRICE_BUFFER constant can become NaN
when PRICE_BUFFER_PERCENT is non-numeric; change its initialization to validate
parseFloat(process.env.PRICE_BUFFER_PERCENT ?? "1") with an isNaN guard and a
safe default (e.g. defaultPercent = 1), then compute PRICE_BUFFER = 1 +
(validatedPercent / 100); optionally emit a warning via console.warn or logger
when the env value is invalid so failures are not silent. Ensure you update the
constant initialization that references PRICE_BUFFER_PERCENT and parseFloat to
use the validated value.
---
Nitpick comments:
In `@packages/e2e/utils/balance-checker.ts`:
- Around line 148-201: Extract the duplicated REST call and JSON parsing into a
shared helper (e.g., fetchAllBalances(address)) used by getAllBalances, and
refactor getBalance to call the Cosmos single-denom endpoint instead of fetching
all balances: construct URL
`${REST_ENDPOINT}/cosmos/bank/v1beta1/balances/${address}/${encodeURIComponent(denom)}`
(use encodeURIComponent for IBC denoms), perform the same response.ok check and
JSON cast, then apply the same TOKEN_DENOMS decimals lookup and return 0 when
missing; keep unique symbols getBalance and getAllBalances thin and reuse the
new helper to avoid duplication.
- Around line 247-251: The resolver currently throws on an unknown token inside
resolveUsdRequirements (using TOKEN_DENOMS and usdReqs), which bubbles up to
ensureBalances and aborts all checks; change resolveUsdRequirements to log or
warn and skip invalid entries instead of throwing so it returns only
validUsdReqs, and then update the for loop in ensureBalances to iterate over
validUsdReqs (or the returned filtered list) rather than the original usdReqs so
a single typo doesn't cancel all checks.
Enhance balance checking to derive addresses from private keys, support price-aware USD requirements, and run as a CI precursor that blocks tests when accounts are under-funded. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
jasbanza
force-pushed
the
jason/mer-102-e2e-tests-account-balance-monitoring
branch
from
4f0412c to
1e10d33
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| { token: "USDC", amount: 3.2, unit: "usd" }, | ||
| { token: "BTC", amount: 1.6, unit: "usd" }, | ||
| { token: "OSMO", amount: 1.6, unit: "usd" }, | ||
| ]); |
There was a problem hiding this comment.
All-USD requirements silently produce false "passed" on price failure
Medium Severity
In monitoring.market.wallet.spec.ts, all three requirements use unit: "usd". Inside resolveUsdRequirements, tokenReqs (non-USD) will be empty. If fetchTokenPrices throws (network error, timeout, API issue), the catch block returns resolved — which is the empty tokenReqs array. ensureBalances then runs Promise.allSettled on zero items and logs "All balance checks passed" despite checking nothing. The same issue affects monitoring.limit.wallet.spec.ts for its USDC requirement and trade.wallet.spec.ts for its USDC requirement, though those files at least have some token-unit requirements that survive the failure.
Additional Locations (1)
| export const OSMOSIS_RPC = | ||
| process.env.OSMOSIS_RPC ?? | ||
| process.env.NEXT_PUBLIC_OSMOSIS_RPC_OVERWRITE ?? | ||
| "https://rpc.osmosis.zone"; |
There was a problem hiding this comment.
Exported OSMOSIS_RPC constant is never imported anywhere
Low Severity
OSMOSIS_RPC is exported from config.ts but never imported by any file in the codebase. Only REST_ENDPOINT and SQS_BASE_URL from this module are actually consumed (by balance-checker.ts and price-utils.ts respectively). This is dead code introduced in this PR.
| echo "Low balance warning — proceeding with tests." | ||
| else | ||
| echo "Balance check failed but no report — proceeding with tests." | ||
| fi |
There was a problem hiding this comment.
Slack failure silently skips critical balance gating step
High Severity
The "Fail if balance critically low" step uses if: steps.balance-check.outcome == 'failure' without a status function. GitHub Actions auto-prepends success(), making it success() && (...). Since the preceding Slack alert step lacks continue-on-error: true, a Slack failure (bad webhook, network error) causes success() to return false, silently skipping the critical balance gate and letting under-funded tests proceed — the exact scenario this PR aims to prevent.
Additional Locations (2)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
packages/e2e/scripts/check-balances.ts (2)
180-185: UnnecessarygetBalancenetwork call for zero-balance tokens
getAllBalancesalready fetched the complete on-chain balance list and omits zero-balance denoms. WhenallBalances[tokenInfo.denom]isundefined, the balance is already known to be0— there is no need to make a second HTTP request viagetBalance.♻️ Proposed simplification
- let balance: number; - if (allBalances[tokenInfo.denom] !== undefined) { - balance = allBalances[tokenInfo.denom]; - } else { - balance = await getBalance(address, tokenInfo.denom); - } + const balance = allBalances[tokenInfo.denom] ?? 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/e2e/scripts/check-balances.ts` around lines 180 - 185, The code makes an unnecessary network call to getBalance when allBalances[tokenInfo.denom] is undefined; since getAllBalances omits zero balances, treat undefined as 0 instead of calling getBalance. Update the block around the balance assignment (variables: allBalances, tokenInfo.denom, balance) to set balance = allBalances[tokenInfo.denom] ?? 0 and remove the await getBalance(address, tokenInfo.denom) call so no extra HTTP request is made.
218-236: Prefer a staticimport fsover a dynamicawait import("fs")
fsis a Node.js built-in available at module-load time; dynamic import is unusual here and slightly defers a syntax-level module reference without benefit.♻️ Proposed fix
At the top of the file, alongside other imports:
+import fs from "fs";Then inline:
- const fs = await import("fs"); - const reportPath = "balance-report.json"; - fs.writeFileSync( + const reportPath = "balance-report.json"; + fs.writeFileSync(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/e2e/scripts/check-balances.ts` around lines 218 - 236, The code uses a dynamic await import("fs") before writing the report (see fs.writeFileSync, reportPath, outcome, ACCOUNT_LABEL, address, reportLines), which is unnecessary; replace the dynamic import with a static top-level import of fs (add import fs from "fs" or import * as fs from "fs" alongside other imports) and remove the await import call so the write logic uses the statically imported fs.writeFileSync(reportPath, JSON.stringify(...)); keep the rest of the report object (outcome, account, address, details, timestamp) unchanged.packages/e2e/tests/swap.usdc.wallet.spec.ts (1)
26-33:deriveAddresscall is redundant —ensureBalancesaccepts a private key directly
ensureBalancesalready callsresolveAddressinternally, which detects and derives an address from a private key. CallingderiveAddressseparately beforeensureBalances(address, ...)adds a duplicate key derivation.♻️ Simplification
- const { address } = await deriveAddress(privateKey); - await ensureBalances(address, [ + await ensureBalances(privateKey, [ { token: "USDC", amount: 0.5 }, { token: "ATOM", amount: 0.015 }, { token: "TIA", amount: 0.02 }, { token: "INJ", amount: 0.01 }, { token: "AKT", amount: 0.025 }, ]);Note: remove the
deriveAddressimport from Line 6 as well if this simplification is applied and the address is not used elsewhere in this file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/e2e/tests/swap.usdc.wallet.spec.ts` around lines 26 - 33, The deriveAddress call is redundant because ensureBalances already resolves/derives an address via resolveAddress; remove the deriveAddress invocation and instead call ensureBalances with the privateKey directly (e.g., ensureBalances(privateKey, [...])) and drop the deriveAddress import if it’s no longer used in this file; update any variable usages that expected the explicit address to use ensureBalances' behavior or resolveAddress only when truly needed.packages/e2e/utils/balance-checker.ts (1)
148-201:getBalanceandgetAllBalancesduplicate the same fetch/parse logicBoth functions build the identical URL, make the same HTTP request, parse the same response, and apply the same decimal-conversion fallback. The only difference is that
getBalancereturns a single value whilegetAllBalancesreturns a map.♻️ Suggested refactor — single fetch helper
+/** Shared internal fetch: returns raw balances array. */ +async function fetchRawBalances( + address: string +): Promise<Array<{ denom: string; amount: string }>> { + const url = `${REST_ENDPOINT}/cosmos/bank/v1beta1/balances/${address}?pagination.limit=1000`; + const response = await fetch(url, { signal: AbortSignal.timeout(30_000) }); + if (!response.ok) { + throw new Error( + `Failed to fetch balances: ${response.status} ${response.statusText}` + ); + } + const data = (await response.json()) as BalanceResponse; + return data.balances; +} export async function getBalance(address: string, denom: string): Promise<number> { - const url = `${REST_ENDPOINT}/cosmos/bank/v1beta1/balances/${address}?pagination.limit=1000`; - const response = await fetch(url, { signal: AbortSignal.timeout(30_000) }); - if (!response.ok) { - throw new Error(`Failed to fetch balances: ${response.status} ${response.statusText}`); - } - const data = (await response.json()) as BalanceResponse; - const balance = data.balances.find((b) => b.denom === denom); + const balances = await fetchRawBalances(address); + const balance = balances.find((b) => b.denom === denom); if (!balance) return 0; const tokenInfo = Object.values(TOKEN_DENOMS).find((t) => t.denom === denom); const decimals = tokenInfo?.decimals ?? 6; return parseFloat(balance.amount) / Math.pow(10, decimals); } export async function getAllBalances(address: string): Promise<Record<string, number>> { - const url = `${REST_ENDPOINT}/cosmos/bank/v1beta1/balances/${address}?pagination.limit=1000`; - const response = await fetch(url, { signal: AbortSignal.timeout(30_000) }); - if (!response.ok) { - throw new Error(`Failed to fetch balances: ${response.status} ${response.statusText}`); - } - const data = (await response.json()) as BalanceResponse; - const result: Record<string, number> = {}; - for (const { denom, amount } of data.balances) { + const balances = await fetchRawBalances(address); + const result: Record<string, number> = {}; + for (const { denom, amount } of balances) { ... } return result; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/e2e/utils/balance-checker.ts` around lines 148 - 201, getBalance and getAllBalances duplicate the fetch/parse/decimal logic; extract a shared helper (e.g., fetchBalances or parseBalances) that performs the HTTP request to `${REST_ENDPOINT}/cosmos/bank/v1beta1/balances/${address}?pagination.limit=1000`, checks response.ok, parses the JSON as BalanceResponse and returns the parsed data or a normalized list of {denom, amount} with decimals applied using TOKEN_DENOMS lookup; then refactor getBalance to call the helper and return the matching denom value (or 0) and refactor getAllBalances to call the same helper and build the denom→number map, keeping the existing decimal fallback (?.decimals ?? 6).packages/e2e/tests/monitoring.limit.wallet.spec.ts (1)
17-20:unit: "usd"for USDC adds unnecessary SQS dependency — prefer token units
unit: "usd"triggers a live SQS price fetch for USDC. Since USDC is already pegged ~$1,{ token: "USDC", amount: 1.1, unit: "usd" }resolves to ~1.1 USDC — identical to the token-unit form. If the price fetch fails, the check is silently skipped (resolveUsdRequirementsreturns early on fetch error), leaving the balance requirement unenforced.♻️ Proposed fix
- { token: "USDC", amount: 1.1, unit: "usd" }, + { token: "USDC", amount: 1.1 },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/e2e/tests/monitoring.limit.wallet.spec.ts` around lines 17 - 20, The test is using unit: "usd" for USDC which triggers a live SQS price fetch and can silently skip the check on fetch failure; update the test to use token units instead by changing the USDC requirement from { token: "USDC", amount: 1.1, unit: "usd" } to { token: "USDC", amount: 1.1 } so ensureBalances uses token-unit comparisons and avoids resolveUsdRequirements’ external price fetch; verify ensureBalances/resolveUsdRequirements will treat the requirement as a token-based check and not call the SQS price path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/e2e/scripts/check-balances.ts`:
- Around line 159-162: The guard that checks "requirements" should distinguish
between missing ACCOUNT_LABEL and an unknown ACCOUNT_LABEL value: if
process.env.ACCOUNT_LABEL is unset, keep the current display-only log and
exit(0); if ACCOUNT_LABEL is set but ACCOUNT_REQUIREMENTS[ACCOUNT_LABEL] is
undefined, log a clear error including the provided ACCOUNT_LABEL and exit with
a non-zero code (e.g., process.exit(2)). Update the conditional around the
`requirements` variable (and any code that reads `process.env.ACCOUNT_LABEL`) to
perform this two-way check and adjust the console message and exit code
accordingly.
---
Duplicate comments:
In `@packages/e2e/README.md`:
- Around line 16-21: Remove the public mapping of CI secret names to live wallet
addresses from the README: delete or redact the table rows that pair
TEST_PRIVATE_KEY, TEST_PRIVATE_KEY_1, and TEST_PRIVATE_KEY_2 with specific
osmo1… addresses and replace them with a generic note that secret-to-address
mappings live in an internal, access-controlled wiki; for TEST_PRIVATE_KEY_3
either add the explicit address or add a short explanatory sentence stating why
the address is intentionally omitted (e.g., derived at runtime and not stored),
and update README.md to point to the internal doc for the actual mappings.
In `@packages/e2e/utils/balance-checker.ts`:
- Around line 231-232: The module-level PRICE_BUFFER can become NaN when
parseFloat(process.env.PRICE_BUFFER_PERCENT) returns NaN; change its
initialization to parse the env var into a numeric percent (e.g., const pct =
parseFloat(process.env.PRICE_BUFFER_PERCENT ?? "1")), validate it with
Number.isFinite or isNaN, and if invalid fallback to a safe default (e.g., 1);
then compute PRICE_BUFFER as 1 + pct / 100. Update references (PRICE_BUFFER,
parseFloat, process.env.PRICE_BUFFER_PERCENT) so tokensNeeded and downstream
comparisons never operate with NaN.
---
Nitpick comments:
In `@packages/e2e/scripts/check-balances.ts`:
- Around line 180-185: The code makes an unnecessary network call to getBalance
when allBalances[tokenInfo.denom] is undefined; since getAllBalances omits zero
balances, treat undefined as 0 instead of calling getBalance. Update the block
around the balance assignment (variables: allBalances, tokenInfo.denom, balance)
to set balance = allBalances[tokenInfo.denom] ?? 0 and remove the await
getBalance(address, tokenInfo.denom) call so no extra HTTP request is made.
- Around line 218-236: The code uses a dynamic await import("fs") before writing
the report (see fs.writeFileSync, reportPath, outcome, ACCOUNT_LABEL, address,
reportLines), which is unnecessary; replace the dynamic import with a static
top-level import of fs (add import fs from "fs" or import * as fs from "fs"
alongside other imports) and remove the await import call so the write logic
uses the statically imported fs.writeFileSync(reportPath, JSON.stringify(...));
keep the rest of the report object (outcome, account, address, details,
timestamp) unchanged.
In `@packages/e2e/tests/monitoring.limit.wallet.spec.ts`:
- Around line 17-20: The test is using unit: "usd" for USDC which triggers a
live SQS price fetch and can silently skip the check on fetch failure; update
the test to use token units instead by changing the USDC requirement from {
token: "USDC", amount: 1.1, unit: "usd" } to { token: "USDC", amount: 1.1 } so
ensureBalances uses token-unit comparisons and avoids resolveUsdRequirements’
external price fetch; verify ensureBalances/resolveUsdRequirements will treat
the requirement as a token-based check and not call the SQS price path.
In `@packages/e2e/tests/swap.usdc.wallet.spec.ts`:
- Around line 26-33: The deriveAddress call is redundant because ensureBalances
already resolves/derives an address via resolveAddress; remove the deriveAddress
invocation and instead call ensureBalances with the privateKey directly (e.g.,
ensureBalances(privateKey, [...])) and drop the deriveAddress import if it’s no
longer used in this file; update any variable usages that expected the explicit
address to use ensureBalances' behavior or resolveAddress only when truly
needed.
In `@packages/e2e/utils/balance-checker.ts`:
- Around line 148-201: getBalance and getAllBalances duplicate the
fetch/parse/decimal logic; extract a shared helper (e.g., fetchBalances or
parseBalances) that performs the HTTP request to
`${REST_ENDPOINT}/cosmos/bank/v1beta1/balances/${address}?pagination.limit=1000`,
checks response.ok, parses the JSON as BalanceResponse and returns the parsed
data or a normalized list of {denom, amount} with decimals applied using
TOKEN_DENOMS lookup; then refactor getBalance to call the helper and return the
matching denom value (or 0) and refactor getAllBalances to call the same helper
and build the denom→number map, keeping the existing decimal fallback
(?.decimals ?? 6).
Comment on lines
+159
to
+162
| if (!requirements) { | ||
| console.log("\nNo ACCOUNT_LABEL set — display-only mode. Exiting.\n"); | ||
| process.exit(0); | ||
| } |
Contributor
There was a problem hiding this comment.
Misleading exit message when ACCOUNT_LABEL is set but unrecognised
When ACCOUNT_LABEL has a value that doesn't match any key in ACCOUNT_REQUIREMENTS, requirements is undefined. The guard at Line 159 then prints "No ACCOUNT_LABEL set — display-only mode" and exits 0 — but the label was set, just unknown. A CI operator could easily miss that their label is misspelled and receive a false-healthy exit code.
🛡️ Proposed fix
if (!requirements) {
- console.log("\nNo ACCOUNT_LABEL set — display-only mode. Exiting.\n");
+ if (ACCOUNT_LABEL) {
+ console.warn(
+ `\nACCOUNT_LABEL "${ACCOUNT_LABEL}" was not found in ACCOUNT_REQUIREMENTS — exiting without validation.\n`
+ );
+ } else {
+ console.log("\nNo ACCOUNT_LABEL set — display-only mode. Exiting.\n");
+ }
process.exit(0);
}📝 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.
Suggested change
| if (!requirements) { | |
| console.log("\nNo ACCOUNT_LABEL set — display-only mode. Exiting.\n"); | |
| process.exit(0); | |
| } | |
| if (!requirements) { | |
| if (ACCOUNT_LABEL) { | |
| console.warn( | |
| `\nACCOUNT_LABEL "${ACCOUNT_LABEL}" was not found in ACCOUNT_REQUIREMENTS — exiting without validation.\n` | |
| ); | |
| } else { | |
| console.log("\nNo ACCOUNT_LABEL set — display-only mode. Exiting.\n"); | |
| } | |
| process.exit(0); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/e2e/scripts/check-balances.ts` around lines 159 - 162, The guard
that checks "requirements" should distinguish between missing ACCOUNT_LABEL and
an unknown ACCOUNT_LABEL value: if process.env.ACCOUNT_LABEL is unset, keep the
current display-only log and exit(0); if ACCOUNT_LABEL is set but
ACCOUNT_REQUIREMENTS[ACCOUNT_LABEL] is undefined, log a clear error including
the provided ACCOUNT_LABEL and exit with a non-zero code (e.g.,
process.exit(2)). Update the conditional around the `requirements` variable (and
any code that reads `process.env.ACCOUNT_LABEL`) to perform this two-way check
and adjust the console message and exit code accordingly.
JoseRFelix
requested changes
Feb 23, 2026
Collaborator
JoseRFelix
left a comment
JoseRFelix
left a comment
There was a problem hiding this comment.
Looking good! Left some comments
Comment on lines
+17
to
+20
| await ensureBalances(address, [ | ||
| { token: "OSMO", amount: 1.1 }, | ||
| { token: "USDC", amount: 1.1, unit: "usd" }, | ||
| ]); |
Collaborator
There was a problem hiding this comment.
These had great succint comments explaining the index position of these items. I believe we should've kept them.
Comment on lines
+44
to
+68
| // --------------------------------------------------------------------------- | ||
| // Config | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| const PRIVATE_KEY = process.env.PRIVATE_KEY; | ||
| const ACCOUNT_LABEL = process.env.ACCOUNT_LABEL ?? ""; | ||
| const DRY_RUN = process.env.DRY_RUN === "true"; | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Helpers | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| /** Returns the display decimal places for a token (matches on-chain precision, capped at 8). */ | ||
| function decimalsFor(token: string): number { | ||
| const info = TOKEN_DENOMS[token]; | ||
| return info ? Math.min(info.decimals, 8) : 6; | ||
| } | ||
|
|
||
| function fmtBalance(amount: number, token: string): string { | ||
| return amount.toFixed(decimalsFor(token)); | ||
| } | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Main | ||
| // --------------------------------------------------------------------------- |
Collaborator
There was a problem hiding this comment.
I suggest removing these code block separator comments, since they seem redundant.
| @@ -0,0 +1,96 @@ | |||
| name: Check E2E Account Balances | |||
Collaborator
There was a problem hiding this comment.
I suggest using a matrix strategy to simplify and reduce loc from this file considerably:
name: Check E2E Account Balances
on:
workflow_dispatch:
permissions:
contents: read
jobs:
check-balances:
name: ${{ matrix.account.label }}
timeout-minutes: 5
runs-on: ubuntu-latest
strategy:
matrix:
account:
- label: E2E Test Account
private_key_secret: TEST_PRIVATE_KEY
- label: Monitoring SG
private_key_secret: TEST_PRIVATE_KEY_1
- label: Monitoring EU
private_key_secret: TEST_PRIVATE_KEY_2
- label: Monitoring US
private_key_secret: TEST_PRIVATE_KEY_3
steps:
- name: Check out repository
uses: actions/checkout@v4
with:
sparse-checkout: |
packages/e2e
- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version: 22.x
- name: Install dependencies
run: yarn --cwd packages/e2e install --frozen-lockfile
- name: Check balances
env:
PRIVATE_KEY: ${{ secrets[matrix.account.private_key_secret] }}
ACCOUNT_LABEL: ${{ matrix.account.label }}
run: yarn --cwd packages/e2e check-balances || true
Comment on lines
+17
to
+21
| await ensureBalances(address, [ | ||
| { token: "USDC", amount: 3.2, unit: "usd" }, | ||
| { token: "BTC", amount: 1.6, unit: "usd" }, | ||
| { token: "OSMO", amount: 1.6, unit: "usd" }, | ||
| ]); |
Collaborator
There was a problem hiding this comment.
As previously mentioned, I suggest keeping the comments for clarity
Comment on lines
+18
to
+22
| await ensureBalances(address, [ | ||
| { token: "USDC", amount: 1.2 }, | ||
| { token: "USDC.eth.axl", amount: 0.6 }, | ||
| { token: "USDT", amount: 0.6 }, | ||
| ]); |
Comment on lines
+19
to
+22
| await ensureBalances(address, [ | ||
| { token: "OSMO", amount: 0.2 }, | ||
| { token: "ATOM", amount: 0.01 }, | ||
| ]); |
Comment on lines
+27
to
+33
| await ensureBalances(address, [ | ||
| { token: "USDC", amount: 0.5 }, | ||
| { token: "ATOM", amount: 0.015 }, | ||
| { token: "TIA", amount: 0.02 }, | ||
| { token: "INJ", amount: 0.01 }, | ||
| { token: "AKT", amount: 0.025 }, | ||
| ]); |
Comment on lines
+245
to
+261
| payload: | | ||
| { | ||
| "text": "E2E Low Balance Warning", | ||
| "blocks": [ | ||
| { | ||
| "type": "header", | ||
| "text": { "type": "plain_text", "text": "E2E Low Balance Warning" } | ||
| }, | ||
| { | ||
| "type": "section", | ||
| "text": { | ||
| "type": "mrkdwn", | ||
| "text": "*Account:* E2E Test Account\n*Workflow:* frontend-e2e-tests / preview-trade-tests\n*Details:* <${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}|View run logs>" | ||
| } | ||
| } | ||
| ] | ||
| } |
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
2 participants
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.
What is the purpose of the change:
Add comprehensive balance monitoring for e2e test accounts. The balance checker now derives addresses from private keys (removing the need for separate WALLET_ID secrets), supports price-aware USD-denominated requirements, and runs as a blocking CI precursor step that prevents under-funded test runs from wasting CI time.
A two-threshold model (minAmount / warnAmount) sends Slack alerts when balances are getting low before they become critically insufficient.
Linear Task
MER-102: E2E Tests - Account balance monitoring
Brief Changelog
config.ts,wallet-utils.ts,price-utils.ts,balance-config.tsinpackages/e2e/utils/packages/e2e/scripts/check-balances.tswith exit codes 0 (healthy), 1 (critical), 2 (low warning).github/workflows/check-balances.ymlfor on-demand balance reports across all 4 accountsbalance-checker.ts: always warn-only, supports private key derivation,unit: "usd"for price-aware checks with 1% bufferPRIVATE_KEYinstead ofWALLET_ID, with correctunittypes (USD for Buy tab, token for Swap)frontend-e2e-tests.ymlandmonitoring-limit-geo-e2e-tests.yml@cosmjs/proto-signingandtsxto e2e package dependenciespackages/e2e/README.mdwith full documentationTesting and Verifying
PRIVATE_KEY=<key> ACCOUNT_LABEL="E2E Test Account" npx tsx packages/e2e/scripts/check-balances.tslocally to verify output formatNote
Medium Risk
Touches CI gating logic and adds network-dependent checks (LCD + SQS) that can change when E2E jobs alert or fail, potentially impacting test pipeline reliability.
Overview
Adds a new
packages/e2ebalance-monitoring system: a standalonecheck-balancesCLI, centralized per-account/token thresholds (minAmount/warnAmount), price-based USD requirements via SQS, and address derivation fromPRIVATE_KEY(no separate wallet address input needed).Updates CI workflows to run balance checks before selected Playwright jobs and send Slack alerts on low balances, failing the job only when the generated
balance-report.jsonmarks a critical shortage; also adds an on-demandcheck-balances.ymlworkflow to report balances for all test accounts. E2E specs are adjusted to derive the bech32 address from the private key and to mark some requirements asunit: "usd", whilebalance-checker.tsis rewritten to be warn-only in-test and move blocking behavior to the precursor script.Written by Cursor Bugbot for commit 1e10d33. This will update automatically on new commits. Configure here.