Skip to content

feat: unify swap + Hyperliquid under one acp trade command#26

Open
psmiratisu wants to merge 5 commits into
devfrom
feat/acp-trade
Open

feat: unify swap + Hyperliquid under one acp trade command#26
psmiratisu wants to merge 5 commits into
devfrom
feat/acp-trade

Conversation

@psmiratisu
Copy link
Copy Markdown
Contributor

@psmiratisu psmiratisu commented Jun 1, 2026

What

Combines the two in-flight trading PRs into a single acp trade command that routes by the params you pass — no subcommand to memorize for the common cases.

Intent routing

You pass… Intent
--side long|short Hyperliquid perp (leveraged)
--spot --coin X --side buy|sell Hyperliquid spot (USDC-quoted)
--chain-out 1337 Deposit USDC into Hyperliquid
--token-in/--token-out/--chain-* Swap — same-chain or cross-chain (BondingV5 / LiFi)
(no flags, in a terminal) Interactive picker (humans only)

Plus acp trade status (positions/margin/balances) and acp trade withdraw.

Signing

The CLI stays a thin signer. Swaps/deposits run through the trading-agent /api/trade/plan + /next state 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 — unified acp trade (swap + deposit + perp + spot + status + withdraw, param-routed) with an interactive picker
  • src/lib/hl/client.ts — Hyperliquid client wiring (signer bridge, asset/price helpers)
  • src/lib/agentFactory.tscreateProviderAdapterWithChains
  • package.json@nktkas/hyperliquid dependency
  • bin/acp.ts — register unified acp trade (folds away acp hl), help text
  • Docs: README.md, SKILL.md, and economyOS CLI reference (separate repo)

Examples

acp trade --token-in usdc --chain-in 8453 --amount-in 50 --token-out virtual --chain-out 8453   # same-chain swap
acp trade --token-in usdc --chain-in 1 --amount-in 100 --token-out usdc --chain-out 8453         # cross-chain swap
acp trade --amount-in 25 --chain-out 1337                                                         # deposit to Hyperliquid
acp trade --coin BTC --side long --size 0.01 --leverage 5                                         # HL perp
acp trade --spot --coin PURR --side buy --size 100                                                # HL spot
acp trade status
acp trade withdraw --amount 25

Test status

  • tsc --noEmit clean
  • ✅ Routing verified (swap/deposit/perp/ambiguous/help) via tsx
  • ⚠️ Perp/spot/withdraw perform real signed actions — not exhaustively smoke-tested here on purpose. Needs a full manual test (with funded HL account) before the old branches are dropped.

🤖 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 trade command 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|short for HL perps, plus trade status / trade withdraw. New src/lib/hl/client.ts bridges the keystore to Hyperliquid EIP-712; createProviderAdapter registers extra mainnet chains for multi-chain legs; getApiContext exposes bearer auth for the trade proxy.

Also implements Treasures tokenized stock buy/sell (--token + --amount-usdc / --amount-shares) via src/lib/treasures/client.ts (ownership proof, EIP-712 legs, submit + poll)—not yet documented in README.md / SKILL.md, which focus on swaps and HL.

Docs and CLI help describe chain 1337 routing, auto perp↔spot USDC balancing, and agent guidance (--json, no interactive picker). @nktkas/hyperliquid, acp-node-v2 bump, engines.node ≥ 20.19.

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

Comment thread src/lib/hl/client.ts
Comment thread package.json
Comment thread src/commands/trade.ts
Comment thread src/commands/trade.ts Outdated
Comment thread src/commands/trade.ts
Comment thread src/commands/trade.ts
Comment thread src/commands/agent.ts Outdated
Comment thread src/lib/api/agent.ts Outdated
Comment thread src/lib/agentFactory.ts
Comment thread src/commands/client.ts Outdated
@psmiratisu psmiratisu force-pushed the feat/acp-trade branch 2 times, most recently from 2ff7215 to c39b34a Compare June 2, 2026 15:50
@psmiratisu
Copy link
Copy Markdown
Contributor Author

Addressed the Bugbot findings in the latest commit:

  • Spot mid price (High)resolveSpotAsset now returns a midKey and marketPrice looks spot mids up by @{pairIndex} (with PURR/USDCPURR) instead of the pair name, so spot market orders get a mid. Limit orders were already fine.
  • Coin/perp routing (High)detectIntent now gates perp on --side long|short. A stray --token no longer pre-empts chain-based routing, so a 1337→1337 spot command routes to runHlSpot correctly. Bare --token (no side) still falls to the perp path for a clear "--side required" error.
  • Node engines (Medium) — added "engines": { "node": ">=20.19.0" } to match @nktkas/hyperliquid.
  • Arbitrum adapter never used (Medium) — already resolved in the rebase: createProviderAdapterWithChains was removed (the trade flow only uses createProviderAdapter), so there's no unused adapter.

Typecheck clean; perp-vs-spot routing verified locally.

Comment thread src/commands/trade.ts
Comment thread src/commands/trade.ts
@psmiratisu
Copy link
Copy Markdown
Contributor Author

Addressed the round-2 review comments (pushed as an amend to keep the PR a single clean commit):

  1. post() double-consumes the response body — it called res.json() then res.text() in the catch, which throws "body already used" and masks the real error on a non-JSON error response. Now reads the body once as text, then JSON.parses it (falling back to the raw text).

  2. isKnownCode's s is ErrorCode guard was unsoundKNOWN_CODES contained ALREADY_TOKENIZED, SLIPPAGE_TOO_LOW, and INSUFFICIENT_GAS, none of which were in the ErrorCode union. Added SLIPPAGE_TOO_LOW + INSUFFICIENT_GAS to the union in src/lib/errors.ts and removed the non-trade ALREADY_TOKENIZED. KNOWN_CODES now matches the union exactly, so the type guard is honest.

  3. detectIntent silently routed --chain-in 1337 with no --chain-out to a withdraw — almost always a spot order missing --chain-out 1337, so this risked moving funds off HL unintentionally. Withdraw now requires an explicit EVM --chain-out; otherwise it throws a clear VALIDATION_ERROR pointing to --chain-out 1337 (spot) vs an EVM chain id (withdraw).

npx tsc --noEmit passes.

Comment thread src/commands/trade.ts
@psmiratisu
Copy link
Copy Markdown
Contributor Author

Round-3 review — addressed the genuinely-open items and noting the ones already resolved by the rebase (amended into the single PR commit 8e5b33f).

Fixed:

  1. parsePerpSide accepts values detectIntent won't route to perp (the new finding). parsePerpSide accepted buy/sell/b/s but detectIntent only routed long/short to perp, so --side sell --chain-in 8453 fell through to swap. Extracted a single shared PERP_LONG_SIDES/PERP_SHORT_SIDES source of truth used by both, so they can no longer disagree (both also .trim() now).

  2. Withdraw ignores --chain-out. runWithdraw never read --chain-out. Since HL withdraw3 only ever settles on Arbitrum, it now takes chainOut and rejects any non-42161 value with a clear VALIDATION_ERROR instead of silently settling on Arbitrum. (detectIntent already requires an explicit --chain-out for a withdraw from the round-2 fix.)

  3. Extended-chain provider never wired. The old createProviderAdapterWithChains is gone after the rebase, but the underlying concern was real: the installed SDK's EVM_MAINNET_CHAINS is just [base], so createProviderAdapter() couldn't sign send legs the trading-agent returns on Arbitrum/BSC/Polygon/etc. It now binds mainnet, arbitrum, optimism, polygon, bsc (deduped against the SDK set, SDK entries win). Listing a chain here only lets the adapter build a client — end-to-end signing still depends on SDK paymaster support for that chain.

Already resolved / stale (verified against current 8e5b33f):

  • Spot market mid price key — already handled: resolveSpotAsset sets midKey to @{pair.index} (with PURR/USDCPURR), and marketPrice looks up allMids by that key, not pair.name.
  • Node 18 vs Hyperliquidengines.node is already >=20.19.0 (the comment cites stale line numbers).
  • Response body consumed twice and isKnownCode type guard — fixed in round 2.

npx tsc --noEmit passes.

Comment thread src/commands/trade.ts
info.clearinghouseState({ user: address }),
info.spotClearinghouseState({ user: address }),
]);
const spotUsdc = Number(spot.balances.find((b) => b.coin === "USDC")?.total ?? "0");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8e5b33f. Configure here.

@psmiratisu
Copy link
Copy Markdown
Contributor Author

Trade transport moved behind the ACP backend (amended into the single PR commit).

acp trade swaps/deposits no longer call the trading-agent directly. They now POST to the ACP backend's /trade/plan + /trade/next proxy using the same bearer token as every other command, and the backend forwards to the trading-agent (injecting x-acp-cli-key server-side).

Why: a shared ACP_TRADE_API_KEY baked into a public CLI isn't a real secret, and a public trading-agent endpoint exposes its LiFi/RPC quota to abuse. This removes both.

Changes:

  • Dropped the TRADING_AGENT_URL + ACP_TRADE_API_KEY env vars entirely — acp configure auth is now all that's required.
  • post() uses Authorization: Bearer <token> and refuses a non-https base URL (calldata-to-sign flows back over this hop).
  • README / SKILL updated accordingly.

⚠️ Deploy ordering: the backend proxy (agentic-commerce-be PR #191) must ship before this CLI change is released, or acp trade swaps/deposits will 404. HL spot/perp/withdraw are unaffected (they talk to Hyperliquid directly).

npx tsc --noEmit passes.

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>
@psmiratisu
Copy link
Copy Markdown
Contributor Author

--side now accepts only long or short (amended into the PR commit).

Dropped the buy/sell/b/s aliases — --side is a perps directional flag, and accepting spot terms (buy/sell) there was ambiguous: --side sell could read as a spot sell but actually opened a perp short. A spot buy/sell is expressed via token-in/out direction, never --side.

  • detectIntent + parsePerpSide share one {long, short} set, so they can't disagree.
  • An invalid --side (e.g. sell) now throws a clear VALIDATION_ERROR ("Use --side long or --side short … a spot buy/sell is set by token-in/out direction") instead of silently falling through to a swap.

npx tsc --noEmit passes.

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>
Comment thread src/commands/trade.ts
Comment thread src/commands/trade.ts
Comment thread src/commands/trade.ts
),
]);
outputResult(json, summarizeOrder(res));
} catch (err) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.).

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 172c9ba. Configure here.

Comment thread src/commands/trade.ts
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ai-virtual-b and others added 2 commits June 4, 2026 03:43
…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
Copy link
Copy Markdown

@cursor cursor Bot left a comment

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 and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

Fix All in Cursor

❌ 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.

Comment thread src/commands/trade.ts
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);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 340694f. 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.

3 participants