Skip to content

fix(wallet): report when balance skips Solana instead of dropping it silently - #93

Open
psmiratisu wants to merge 1 commit into
mainfrom
fix/wallet-balance-solana-unavailable
Open

fix(wallet): report when balance skips Solana instead of dropping it silently#93
psmiratisu wants to merge 1 commit into
mainfrom
fix/wallet-balance-solana-unavailable

Conversation

@psmiratisu

@psmiratisu psmiratisu commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Problem

acp wallet balance with no flags queries every sponsored chain plus Solana. When getSolanaWalletAddress() throws, the non-explicit path drops Solana from the queried set and the command still succeeds:

} catch (err) {
  if (explicit) throw err;
  // Drop the Solana network(s) and continue with EVM only.
  for (const [network, id] of [...networkToChainId.entries()]) {
    if (isSolanaChainId(id)) { networks.splice(...); networkToChainId.delete(network); }
  }
}

The result is indistinguishable from a wallet that genuinely holds nothing on Solana, and the three output modes disclose it unequally:

Mode Before
TTY Checked: … (N chains) footer — the only real hint
--json a shorter chains[], i.e. inference from absence
TSV nothing at all

A consumer that reads tokens and finds no Solana rows will report zero holdings when the truth is that Solana was never scanned. That is a silent false negative about someone's balance.

Fix

Track the drop and surface it the same three ways stocksUnavailable already does for the tokenized-stock fetch:

  • --jsonsolanaUnavailable: true, emitted next to stocksUnavailable
  • TTY — a yellow note under the Checked: line
  • TSV — a warning on stderr, so stdout stays parseable

This turns an inference-from-absence into an explicit boolean, matching the convention already established in the same file.

Scope

  • wallet sol balance calls getSolanaWalletAddress() directly and throws on failure, so it never reaches the drop path. Unchanged.
  • Explicit --chain-id / --cluster requests still throw rather than skipping. Unchanged.
  • The flag is omitted from JSON entirely when false, so existing consumers see no new key on the success path.

Verification

  • npx tsc --noEmit — no new errors. Two pre-existing errors in src/lib/agentFactory.ts and src/lib/config.ts are identical before and after this change (local acp-node-v2 version skew, unrelated).
  • npm run build succeeds; the flag and both warning strings are present in dist/bin/acp.js.

Note

Low Risk
CLI output-only change: same skip behavior, now explicitly reported. No auth, scanning, or API contract change except an optional JSON flag.

Overview
When the default all-chains wallet balance cannot resolve a Solana wallet, it still drops Solana and continues with EVM, but that skip is no longer silent.

solanaUnavailable is set on the drop path and surfaced the same way as stocksUnavailable: optional solanaUnavailable: true in JSON (omitted when false), a yellow TTY note under the Checked line, and a TSV warning on stderr. Callers can tell “no Solana tokens” from “Solana was never scanned.” Explicit Solana requests still throw.

Reviewed by Cursor Bugbot for commit 347d369. Bugbot is set up for automated code reviews on this repo. Configure here.

…silently

`wallet balance` with no flags queries every sponsored chain plus Solana. If
`getSolanaWalletAddress()` throws, the non-explicit path removes Solana from
the queried set and the command still succeeds — so the output is
indistinguishable from a wallet that genuinely holds nothing on Solana.

Only the TTY view hinted at it, via the trailing `Checked: … (N chains)`
line. `--json` exposed the skip solely as a shorter `chains[]` array, and
the piped TSV said nothing at all. A consumer reading `tokens` and finding
no Solana rows would reasonably report zero holdings when the truth is that
Solana was never scanned.

Track the drop and surface it the same three ways `stocksUnavailable`
already does:

- `--json`: `solanaUnavailable: true` alongside `stocksUnavailable`
- TTY: a yellow note under the `Checked:` line
- TSV: a warning on stderr, keeping stdout parseable

`wallet sol balance` and `--chain-id`/`--cluster` requests are unaffected —
those take the explicit path and still throw, so the flag stays false.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 347d369. Configure here.

Comment thread src/commands/wallet.ts
"Solana was not checked (wallet could not be resolved) — any Solana holdings are not shown. Retry in a moment."
)}\n`
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

TTY warning skips single-chain path

Medium Severity

The TTY solanaUnavailable note sits only inside the multi-network branch, while stocksUnavailable is emitted for both single and multi. After a non-explicit Solana drop that leaves one EVM network, single is true and the yellow warning never prints, so interactive output still looks like Solana was scanned and empty. JSON and TSV already surface the flag in that case.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 347d369. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant