feat: unify swap + Hyperliquid under one acp trade command#26
feat: unify swap + Hyperliquid under one acp trade command#26psmiratisu wants to merge 5 commits into
acp trade command#26Conversation
2ff7215 to
c39b34a
Compare
|
Addressed the Bugbot findings in the latest commit:
Typecheck clean; perp-vs-spot routing verified locally. |
c39b34a to
2a9f560
Compare
|
Addressed the round-2 review comments (pushed as an amend to keep the PR a single clean commit):
|
2a9f560 to
8e5b33f
Compare
|
Round-3 review — addressed the genuinely-open items and noting the ones already resolved by the rebase (amended into the single PR commit Fixed:
Already resolved / stale (verified against current
|
| info.clearinghouseState({ user: address }), | ||
| info.spotClearinghouseState({ user: address }), | ||
| ]); | ||
| const spotUsdc = Number(spot.balances.find((b) => b.coin === "USDC")?.total ?? "0"); |
There was a problem hiding this comment.
Auto-balance uses total instead of free spot balance
Medium Severity
ensureHlFunds reads b.total for spot USDC, which includes amounts held in open orders. When target is "spot", have is overestimated, so the function may skip a needed perp→spot transfer, causing the subsequent spot buy to fail with a confusing HL insufficient-balance error. When target is "perp", sourceAvail is overestimated, so the function may request a usdClassTransfer larger than the free spot balance, causing the transfer itself to fail. The code already knows about hold (it's mapped in runStatus), so the fix is to subtract it: total - hold.
Reviewed by Cursor Bugbot for commit 8e5b33f. Configure here.
8e5b33f to
145c987
Compare
|
Trade transport moved behind the ACP backend (amended into the single PR commit).
Why: a shared Changes:
|
A single `acp trade` command for token swaps (same-chain and cross-chain via
the trading-agent server's BondingV5 / LiFi routing) and Hyperliquid (deposit,
spot, perp, withdraw). Routes by the params you pass: Hyperliquid is chain 1337,
so the chains decide the venue; `--side long|short` is a perp. Auto-balances the
HL perp/spot wallets so `deposit → trade` just works. Signing stays client-side
via the keystore signer; HL orders are EIP-712 actions.
Adds @nktkas/hyperliquid and bumps @virtuals-protocol/acp-node-v2 to ^0.1.2
(required for the provider signTypedData / sendTransaction APIs the trade flow
uses). Requires Node >=20.19 (engines) for the HL SDK. Documents the command in
README.md and SKILL.md.
Addresses review:
- spot market orders look up mids by `@{pairIndex}` (PURR special-cased), not
the pair name, which returned no mid.
- detectIntent gates perp on --side long|short so a stray --token can't
pre-empt chain-based spot routing.
- declare engines.node >=20.19.0 to match @nktkas/hyperliquid.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
145c987 to
1285cf6
Compare
|
Dropped the
|
Hyperliquid lists leveraged perp markets beyond crypto (equities, currencies, commodities); the acp trade --side flags are identical across asset classes. Surface this in the SKILL.md frontmatter + trading section and the README perps section so agents know they can open stock/FX/commodity positions, not just crypto. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| ), | ||
| ]); | ||
| outputResult(json, summarizeOrder(res)); | ||
| } catch (err) { |
There was a problem hiding this comment.
Order errors reported as success
High Severity
After exchange.order resolves, summarizeOrder always sets top-level status: "ok" and forwards raw statuses. Hyperliquid often returns rejections as per-order { error: "…" } entries in that array without throwing, so --json output can look successful when the order was rejected (margin, min size, etc.).
Reviewed by Cursor Bugbot for commit 172c9ba. Configure here.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…trade` (#30) Adds a Treasures Finance route to `acp trade` — `--ticker` triggers a new "treasures-stock" intent and runs ownership-proof → quote → per-leg EIP-712 sign → submit → poll-status in one command. New `src/lib/treasures/client.ts` wraps the public Treasures API over `fetch` with HTTPS enforcement and TREASURES_API_URL / IS_TESTNET host selection. Also hardens the flow per review: - poll status once more after the final sleep so a late fill isn't misreported as TIMEOUT - non-zero exit code on terminal partial_failed / all_failed - validate --slippage-bps as a non-negative integer instead of sending null
A stock symbol is now named with --token everywhere; the companion flag picks the venue: --side -> HL perp, --amount-usdc/--amount-shares -> Treasures tokenized stock (spot). Drops the separate --ticker flag so an agent picks the asset once and the mode second, matching the CLI's params-decide-the-venue model. https://claude.ai/code/session_01C56LaYyFW7iRxtdY5tY3uA
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 340694f. Configure here.
| const issuedAt = Math.floor(Date.now() / 1000); | ||
| const challenge = buildTreasuresChallenge({ issuedAt, ethWallet }); | ||
| progress(json, `Signing Treasures ownership proof (issued_at=${issuedAt})`); | ||
| const ethSig = await provider.signMessage(TREASURES_PROOF_CHAIN_ID, challenge); |
There was a problem hiding this comment.
Treasures proof uses mainnet Base
Medium Severity
Treasures ownership proofs always call signMessage with chain id 8453, but when IS_TESTNET is true the provider is only wired for testnet chains (e.g. Base Sepolia 84532). Treasures stock trades on testnet can fail at signing before a quote is requested.
Reviewed by Cursor Bugbot for commit 340694f. Configure here.


What
Combines the two in-flight trading PRs into a single
acp tradecommand that routes by the params you pass — no subcommand to memorize for the common cases.Intent routing
--side long|short--spot --coin X --side buy|sell--chain-out 1337--token-in/--token-out/--chain-*Plus
acp trade status(positions/margin/balances) andacp trade withdraw.Signing
The CLI stays a thin signer. Swaps/deposits run through the trading-agent
/api/trade/plan+/nextstate machine — the server builds calldata, the CLI auto-signs+broadcasts each leg with the keystore-backed signer (no per-tx prompt). HL orders/withdrawals are EIP-712 actions signed by the same signer. Private keys never leave the OS keystore.Changes
src/commands/trade.ts— unifiedacp trade(swap + deposit + perp + spot + status + withdraw, param-routed) with an interactive pickersrc/lib/hl/client.ts— Hyperliquid client wiring (signer bridge, asset/price helpers)src/lib/agentFactory.ts—createProviderAdapterWithChainspackage.json—@nktkas/hyperliquiddependencybin/acp.ts— register unifiedacp trade(folds awayacp hl), help textREADME.md,SKILL.md, and economyOS CLI reference (separate repo)Examples
Test status
tsc --noEmitcleantsx🤖 Generated with Claude Code
Note
High Risk
The CLI signs and broadcasts real swaps, bridges, HL orders/withdrawals, and optional Treasures stock trades without per-tx prompts; mistakes or routing bugs can move funds on multiple chains.
Overview
Adds a unified
acp tradecommand that picks the venue from flags: EVM token pairs for DEX swaps and Hyperliquid USDC deposits (backend/trade/plan+/trade/next, keystore auto-sign per leg), chain 1337 pairs for HL spot,--side long|shortfor HL perps, plustrade status/trade withdraw. Newsrc/lib/hl/client.tsbridges the keystore to Hyperliquid EIP-712;createProviderAdapterregisters extra mainnet chains for multi-chain legs;getApiContextexposes bearer auth for the trade proxy.Also implements Treasures tokenized stock buy/sell (
--token+--amount-usdc/--amount-shares) viasrc/lib/treasures/client.ts(ownership proof, EIP-712 legs, submit + poll)—not yet documented inREADME.md/SKILL.md, which focus on swaps and HL.Docs and CLI help describe chain
1337routing, auto perp↔spot USDC balancing, and agent guidance (--json, no interactive picker).@nktkas/hyperliquid,acp-node-v2bump,engines.node≥ 20.19.Reviewed by Cursor Bugbot for commit 340694f. Bugbot is set up for automated code reviews on this repo. Configure here.