Solana wallet + Treasures sol-venue trade signing#44
Open
psmiratisu wants to merge 24 commits into
Open
Conversation
…lance, message signing, and transfers
…ersioned txs Wires the trade loop to the backend's new Solana sign actions: - sigType 'solana-message': Privy signs the raw challenge bytes (no envelope); the adapter's base58 output is re-encoded to BASE64 — the Treasures ownership-proof contract (base58 is their #1 documented rejection cause). - sigType 'solana-tx': sign the serialized versioned tx WITHOUT broadcasting (server/venue broadcast the signed bytes), via the adapter's Privy signTransaction (base64 in/out). - solWallet rides every /plan that could route through Solana: explicit sol venue/source, or a tokenized-stock BUY with no venue pinned — the backend then quotes both venues and executes the better one. Sells stay explicit (the backend can't see which venue holds shares). Verified against the live Privy signer for Mochi3DSTest: the signature checks out as raw ed25519 over the exact challenge bytes (local ed25519.verify — the same check Treasures runs server-side). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s top-up) The backend emits a 'solana-instructions' action before a sol-venue trade when the wallet's native SOL is below the gas floor — a Jupiter USDC→SOL swap. Run it through the adapter's sponsored sendInstructions (Alchemy fee payer) so a zero-SOL wallet can bootstrap, mapping the backend's role strings to AccountRole bitflags and posting back the broadcast signature. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e-SOL gas top-up)" This reverts commit d561aef.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
- package-lock: re-resolve @virtuals-protocol/acp-node-v2 to the published 0.1.4 registry tarball instead of the sibling ../acp-node-v2 link, so `npm ci` works on machines without that local path (was breaking CI/releases). - trade: recognize the Privy Solana chain ids (500 devnet / 501 mainnet) in isSolanaChainRef, so a swap with --chain-in 501 attaches solWallet. - trade: route opts.chain through isSolanaChainRef instead of an exact `=== "sol"` match, so a Treasures sell with --chain solana (or other casing) also attaches solWallet. isSolanaChainRef is now the single source of truth. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bugbot follow-up on PR #44: couldRouteViaSolana covered --chain and --chain-in but not --chain-out, so a swap/bridge whose destination is Solana reached /trade/plan without the agent's Solana pubkey — the recipient the backend needs to route/sign the destination leg. Add the symmetric isSolanaChainRef(opts.chainOut) check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bugbot follow-up on PR #44: the unpinned-buy heuristic fired on --token alone, so `--token AAPL --amount-usdc 1 --chain eth` still attached solWallet. With solWallet present the backend quotes both venues and may pick sol, overriding the user's explicit --chain eth pin. Gate the buy clause on opts.chain === undefined; an explicit --chain sol still routes via the isSolanaChainRef(opts.chain) clause. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mount) Bugbot follow-up on PR #44: the negative formulation classified any --token without --side/--amount-shares/--chain as an unpinned buy, so a malformed perp like `--token BTC --size 0.01` (missing --side) attached solWallet. Define a buy positively: --token plus a spend amount (--amount-usdc on eth, or --amount-in funded from another chain). This covers both documented buy shapes and excludes perp/incomplete shapes that carry no spend signal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bugbot follow-up on PR #44: the empty catch around getSolanaWalletAddress flattened every failure (network, auth, agent lookup) into "no wallet", silently dropping solWallet. Swallow only the NO_SOLANA_WALLET signal, and only for a speculative unpinned buy; real failures now surface, and an explicit Solana route (--chain/--chain-in/--chain-out sol) propagates any error rather than planning a route it can't sign. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| return { | ||
| solWalletAddress: agent.solWalletAddress, | ||
| walletId: solProvider.metadata.walletId, | ||
| }; |
There was a problem hiding this comment.
Solana walletId ignores local cache
Medium Severity
New Solana wallet resolution treats a missing walletId on the SOLANA walletProviders row as no Solana wallet, even when solWalletAddress is set and the same Privy walletId is already stored locally from acp agent add-signer. The EVM adapter falls back to that cached id; Solana paths do not, so wallet sol and trade signing can fail despite a working EVM signer.
Reviewed by Cursor Bugbot for commit d6e5fc0. Configure here.
Contributor
Author
There was a problem hiding this comment.
Looked into this — I don't think this one should be "fixed" as suggested, because the cached walletId is EVM-specific by construction.
persistSignercaches only the EVM provider's id:setWalletId(agent.walletAddress, evmProvider.metadata.walletId)(src/commands/agent.ts:186, comment: "Persist the public key + EVM walletId").- EVM and Solana are separate
walletProvidersrows (chainTypeEVM vs SOLANA), each with its ownmetadata.walletId— i.e. distinct Privy server wallets. The shared thing is the P256 signer, not the wallet id. - Config stores a single
walletIdfield (the EVM one); there is no cached Solana wallet id to fall back to.
So having the Solana path fall back to the cached id like the EVM path does would address the Privy Solana wallet with the EVM wallet's id — signing against the wrong wallet (or a hard Privy error), which is worse than the current behavior. Throwing NO_SOLANA_WALLET when the SOLANA row has no walletId is correct: without it there's no way to address the Privy Solana wallet.
Leaving the resolution to a human reviewer, but flagging this as a likely false positive.
The Solana/Treasures work added spot tokenized-stock buy/sell, sol-venue auto-routing, and swaps in/out of Solana — none of which were in the docs. Adds to both README and SKILL.md: - intent-routing rows for Solana-source swaps and tokenized-stock spot - the stock-vs-perp rule (route by FLAG --amount-usdc/-shares vs --side, never by the ticker — AAPL is both a stock and an HL equity perp) - examples: buy with held USDC, buy funded from another chain, sell, USDC@sol → USDC@Base - command-table row + capability description for tokenized stocks Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Read-only discovery wrapping the backend's GET /trade/instruments:
- no symbol → spot markets { stocks, hlSpot } (tokenized stocks + HL spot)
- with a symbol → every route for that asset, each naming the exact
`token` ticker to pass (e.g. xyz:AAPL for the equity perp, AAPL spot)
Adds a GET helper (the trade client only had POST). Documents the command
and the funding model in README.md and SKILL.md — USDC is the settlement
currency, not a prerequisite; trades fund from any chain/token and the
backend auto-bridges, so the listing never implies pre-holding USDC on HL.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e.json and package-lock.json
…46) * feat: add wallet policy management commands and update documentation * refactor: update CLI commands to clarify dashboard approval requirements and improve user guidance for policy changes --------- Co-authored-by: Zuhwa <zuhwa@virtuals.io>
#47) * feat: enhance wallet balance command to support querying all supported chain * feat: add native currency resolution for token display in wallet commands --------- Co-authored-by: Zuhwa <zuhwa@virtuals.io>
…rove documentation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 23fc7fb. Configure here.
| throw new CliError(message, isKnownCode(code) ? code : "API_ERROR", recovery); | ||
| } | ||
| return (await res.json()) as T; | ||
| } |
There was a problem hiding this comment.
Duplicated HTTP helper logic between get and post
Low Severity
The new get() helper duplicates the HTTPS validation guard, error body parsing (JSON try/catch), error message construction, isKnownCode check, and CliError throw logic from post(). These ~20 lines of identical error-handling could be extracted into a shared helper, reducing the maintenance surface and the risk of future fixes applying to one but not the other.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 23fc7fb. Configure here.
…n balance support for all sponsored EVM chains and Solana
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
4 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
Adds Solana support to the CLI: standalone wallet commands plus the trade-loop signing needed to trade Treasures tokenized stocks on the Solana venue (and Solana-source LiFi legs) driven by the backend.
Wallet commands (
acp wallet sol)address,balance,sign-message,transfer(SOL + SPL),send-instructions— backed by the agent's Privy Solana wallet (same P256 signer as EVM).Trade-loop signing (
acp trade)The backend's
/plan→/nextstate machine now emits Solana sign actions; the CLI handles them with the keystore/Privy signer (server never holds keys):solana-message— ed25519 over the ownership-proof challenge, returned base64 (Treasures' contract; base58 is their refactor: remove job offering isPrivate field #1 documented rejection cause).solana-tx— sign the versioned tx (Treasures order leg, or a Solana-source LiFi bridge) without broadcasting; the server/venue broadcasts.solWalletis auto-attached to any request that could route through Solana (explicit--chain sol, a Solana source, or a tokenized-stock buy with no venue pinned), so the backend can quote/route the sol venue.Validated live
End-to-end on mainnet against the deployed backend + Treasures staging:
acp trade --token AAPL --amount-usdc 1 --chain sol→ ed25519 proof signed → versioned-tx leg signed → submitted → 0.00342 AAPL (xStocks) received for ~1 USDC, confirmed on-chain.Notes
sendInstructionsgas path: acp-be no longer sponsors Solana gas, so the design moved to seed-at-funding + a self-paid top-up (the revert is intentional, net-zero).🤖 Generated with Claude Code
Note
High Risk
Touches live fund movement and trade signing on Solana plus EVM wallet policy behavior that gates broadcasts; mistakes or signing bugs can lose user funds or block transactions.
Overview
Adds Solana to agent identity and trading:
acp wallet sol(address, balance, sign-message, SOL/SPL transfer, rawsend-instructions) viaPrivySolanaProviderAdapterand the same P256 signer as EVM.acp wallet balancenow defaults to all sponsored EVM chains plus Solana (optional--chain-id/--cluster).Introduces
acp policy(create/list/show/global; edit/delete deep-link to the dashboard) andacp agent signer-policy/set-signer-policyso CLI users can inspect policies and attach custom allowlists at signer setup; live policy changes still need dashboard owner approval.acp tradegains Solana legs in the plan/next loop (solana-message/solana-txsigning, optionalsolWalleton Solana or unpinned tokenized-stock buys), read-onlyacp trade stock-list, and documented spot stock flows (--amount-usdc/--amount-sharesvs perp--side). Bumps@virtuals-protocol/acp-node-v2to^0.1.5(Solana kit stack). README and SKILL.md document the new surfaces for humans and agents.Reviewed by Cursor Bugbot for commit d359fca. Bugbot is set up for automated code reviews on this repo. Configure here.