feat(trading): unify the route surface, and fix what validating it surfaced - #683
Open
fengtality wants to merge 71 commits into
Open
feat(trading): unify the route surface, and fix what validating it surfaced#683fengtality wants to merge 71 commits into
fengtality wants to merge 71 commits into
Conversation
…ard; orca position-info contract Gateway-side changes for the gateway#678 retry-ownership work (see docs/retry-architecture.md, included here as the canonical cross-repo design): - solana-error-parser: map Orca Whirlpool 6018 TokenMinSubceeded to SLIPPAGE_EXCEEDED, and attribute custom program errors to the program on the "failed: custom program error" log line instead of the first "invoke" line — simulation-shaped errors open with ComputeBudget, so the DEX-specific error tables were never consulted (the actual #678 MATH_OVERFLOW misreport mechanism). Regression-tested with a full simulation-shaped log. - solana: reject transactions whose compute-estimation simulation returned an error, in both send paths — stale-state failures become a typed 400 before broadcast (zero fees) instead of a broadcast failure. - orca: getPositionInfo returns null ONLY when fetchMaybePosition reports the account does not exist; transient errors now propagate. Callers treat null as "position closed", so a swallowed RPC blip could abandon a live funded position while reporting success. Deliberately NOT included: the in-route close retry loop from 040e99e. Gateway stays a stateless transaction oracle — one request, one attempt, typed errors; retry ownership lives in the Hummingbot connector/executor (see the doc, §6). Validated live on mainnet: forced-failure cascade (fault-injected minimums) had every doomed close rejected pre-broadcast at zero fee cost across 33 attempts, with 6018 correctly surfaced as SLIPPAGE_EXCEEDED. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HahKfEY9rvKnZijrzUAFSq
getTransaction (commitment 'confirmed') returns null both for a transaction awaiting confirmation and for one the cluster has never seen, so /poll reported txStatus 0 (pending) for dropped transactions forever — pollers had no signal to stop waiting on a transaction that can never land once its blockhash expires. The poll route now consults getSignatureStatuses (with history search) when txData is null: a signature the cluster has seen stays UNCONFIRMED (0); an unknown signature returns the new NOT_FOUND (-2), as does a malformed signature. -2 avoids colliding with the Ethereum poll's existing 2/3 mempool heuristics. Transient RPC errors still report UNCONFIRMED so callers keep polling rather than giving up on an unknown outcome. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replaces the design-evolution narrative (proposals, verdicts, review logs, deployment diaries) with a clean description: the ownership principle, the sixteen issues found across the four repos, and the architecture as it now stands — layered ownership, close-vs-open asymmetry, the close lifecycle, terminal semantics, the two topologies with the orphan lifecycle, and the bounded transaction-status polling contract (including the new NOT_FOUND poll status). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…reverts as FAILED
The two poll routes spoke different dialects: Solana used a local enum
(-1/0/1), Ethereum used raw numbers including 2 ('likely to be processed')
and 3 ('likely stuck') that no consumer understood, reported not-found as -1
after blocking the request for three 1-second in-route retries, and — via
'typeof receipt.status === number ? 1 : -1' — reported REVERTED transactions
(status 0, which is a number) as CONFIRMED, so a reverted swap polled as
filled.
Both routes now share TransactionStatusCode in chain-schema:
NOT_FOUND (-2) / FAILED (-1) / PENDING (0) / CONFIRMED (1).
Ethereum: not-found returns -2 immediately (no in-route sleeps — the caller
owns pacing and the not-found deadline), mempool is plain PENDING (gas-price
heuristics dropped), and receipt status 0 maps to FAILED.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Updates the retry-architecture doc for the two follow-up changes: the connector's retryable-code opt-in and inner budget are gone (the executor's CLOSING re-entry with max_retries=0 per attempt is the only close retry loop), and both chains' poll routes now share one TransactionStatusCode contract — including the Ethereum findings (2/3 heuristics, in-route retry sleeps, reverts reported as confirmed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Found in live testing: /poll parsed only JSON.stringify(meta.err), which carries the error code but names no program — so extractProgramId never matched, every program-specific code fell through to the generic map, and a confirmed-but-failed Orca transaction reported 'UNKNOWN (0x1782)' instead of SLIPPAGE_EXCEEDED. Same misreporting as gateway#678, on the async path. The err object is now parsed together with meta.logMessages, whose 'Program X failed: custom program error' line is what the parser attributes on. Errors raised by programs with no registered table (e.g. a third-party router that CPIs into Whirlpool) correctly stay generic rather than being misattributed to the DEX they called. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…Swap GET /trading/clmm/pool-info could never return `bins`: its querystring schema had no binCount and it called every connector as (fastify, network, poolAddress), dropping the parameter that orca/raydium/uniswap already support. Since hummingbot-api and condor read pool info through the unified route, the bin distribution was unreachable outside the per-connector routes. - Unified route accepts binCount and forwards it. Meteora is called without it, as it always returns its own bins. - PancakeSwap CLMM gains binCount. The V3 tick walk moves to a shared clmm-v3-utils helper: the two SDKs disagree on numeric type (@uniswap/v3-sdk is JSBI, @pancakeswap/v3-sdk is native bigint), so the helper works in bigint and each connector adapts its own SDK rather than importing the other's. - Fixes PancakeSwap pool-info token amounts, which were pool.liquidity (V3 virtual liquidity in sqrt-price space) scaled by each token's decimals — reporting the same meaningless figure for both sides. Now ERC20 balanceOf on the pool contract, the same fix Uniswap already carries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
binance.llamarpc.com is dead — it answers nothing, so every BSC read failed (pancakeswap pool-info reported 'Pool not found' for pools that exist). bsc-dataseed.bnbchain.org is BNB Chain's official public endpoint and returns chainId 0x38. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
eth.llamarpc.com was unreachable — gateway logged 'Unable to fetch block number' on every startup and all mainnet reads failed. eth-mainnet.g.alchemy.com/public returns chainId 0x1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The audit that #678 triggered found defects on two read paths the LP flow depends on but that the issue never named — the transaction-status contract the poller reads, and the pool-info contract the dashboard and agents read. Both belong here: they share #678's root cause, a caller unable to tell a definitive answer from a transient one, or unable to ask for what it needs. Adds the CLMM pool-info issues (binCount unreachable through the unified route, PancakeSwap missing bins and reporting virtual liquidity as token amounts, Raydium bypassing Gateway, two dead default RPCs) and a section describing the request chain, the per-connector cost of binCount, and the bin output shape. Notes the fifth repository now in the family. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
add.ts rejects a deposit with neither amount positive at the route level; open let the same body run into connector code before failing. Apply the identical guard (single-sided opens stay valid). The unsupported-connector test gains an amount so it still exercises connector routing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
rewardTokenAddress/rewardAmount had no producer — the only assignments (pancakeswap-sol) are commented out — so every consumer saw permanently absent optionals. Removed from the schema; hummingbot-api drops its passthrough in step. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
…nge amounts binCount on the unified /trading/clmm/pool-info now works for every connector except Meteora (which always returns its bins): pancakeswap-sol was the one connector ignoring it. New computeBinDistribution mirrors the raydium/orca walk against the fork's identical TickArrayState layout — tick-array PDAs fetched in one getMultipleAccountsInfo, liquidity_net (i128) propagated outward from the active bin. Verifying the bins exposed a real connector bug: getAmountsFromLiquidity had its out-of-range branches swapped (price below a range put the liquidity in token1 instead of token0, and vice versa), so bins — and out-of-range position-info and quote-position amounts — came back inverted. In-range amounts were always correct, which is why it went unnoticed. Verified live on the SOL-USDC pool: below-price bins now hold quote, above-price bins hold base, matching raydium and orca exactly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
…hemas
/trading/clmm/create-pool shrinks to three connector params, each meaning
the same thing everywhere:
- binStep: bin/tick granularity (Meteora DLMM bin step; Orca tick spacing)
- feeBps: base fee in basis points (Meteora base fee; Uniswap/PancakeSwap
V3 tier mapped x100 to the protocol's hundredths-of-a-bip units, with a
bps-worded required guard)
- ammConfigIndex: Raydium-family fee-config index — Raydium API list
index, and pancakeswap-sol amm_config PDA index (["amm_config", index],
big-endian with little-endian fallback, validated on-chain) replacing
the raw ammConfig address parameter
Dropped: fee, tickSpacing, ammConfig, gasPrice, maxGas.
The request and response are now canonical in clmm-schema.ts; the unified
route composes its body from ClmmCreatePoolRequest. The response drops
the AMM-inherited baseTokenAmountAdded/quoteTokenAmountAdded — every
connector hardcoded them to 0 because CLMM create-pool initializes an
empty pool — leaving data = { fee }.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
…ig-index name - gasPrice/maxGas removed from the unified /trading/amm/create-pool and from the uniswap/pancakeswap connector routes (add/remove-liquidity, create-pool request schemas): gas is configured at the network level, and the EVM connectors were the only ones exposing per-request overrides. Transactions now use prepareGasOptions(undefined, <route gas limit>). - feeConfigIndex renamed ammConfigIndex on the unified AMM create-pool, matching the CLMM create-pool vocabulary for the Raydium-family fee-config index. configAddress stays: DAMM v2 configs are permissionless accounts with no index derivation. - The unified AMM create-pool body is now composed from the canonical CreatePoolRequest (amm-schema.ts), like its CLMM counterpart. - slippagePct on every unified AMM route now declares default 1 (with the same description/examples as the CLMM routes) — swagger previously rendered the bare maximum (100) as the example, which read as a 100% default slippage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
…NoExactOut only /trading/swap/quote and /trading/swap/execute now expose exactly two optional knobs beyond the core fields: slippagePct and approximateIfNoExactOut (default true; for BUY orders when a router has no ExactOut route, approximate via a sell-leg ExactIn quote instead of failing). approximateIfNoExactOut is threaded through the unified dispatch and supported by all four Solana routers — jupiter, dflow, okx and titan — each keeping its refuse-with-explanation path when a caller passes false. EVM routers quote ExactOut natively and ignore it. Removed from request surfaces, per the config-over-request principle: - 0x gasPrice/maxGas (gas is network-level; tx gas limit comes from the 0x quote's own estimate) - jupiter restrictIntermediateTokens/onlyDirectRoutes (routing policy, connector config) and priorityLevel/maxLamports (Solana priority fees, connector config — the gas knobs of Solana) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
…l the schema openTime was Raydium CPMM's scheduled trading-start (startTime) — a fair-launch nicety only one connector reads, not something needed to create a pool; the connector defaults it to open-immediately. The composite now orders required fields first (connector, chainNetwork, walletAddress, canonical create fields) with the optional per-protocol fee-config selectors last. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
The Meteora connector's addLiquidity always needed a DLMM distribution shape and fell back to the connector-config default when the unified route didn't pass one — so a position opened as one shape through /trading/clmm/open silently accreted liquidity in a different shape on /trading/clmm/add. Same optional Meteora-only parameter as open; other connectors ignore it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
- expose slippagePct on /trading/clmm/remove (orca-only; was hardcoded 1) - pass slippagePct through to pancakeswap-sol in /trading/clmm/quote-position - drop slippagePct||1 coercion for uniswap/0x router quotes (honors explicit 0, falls back to connector config) - enum-constrain connector on every unified route so unknown connectors 400 at the schema; refresh stale connector lists in descriptions - default percentageToRemove to 100 on /trading/amm/remove-liquidity, matching CLMM - walletAddress uniformly required-with-default - hoist parseChainNetwork/defaultWallet/connector+chainNetwork fields into src/trading/common.ts (malformed chainNetwork now 400s everywhere) - remove legacy tradingRoutes alias; app registers tradingSwapRoutes directly Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
- new rethrowRouteError in trading/common.ts: errors with an HTTP statusCode (connector badRequest/notFound, chain errors) propagate untouched; anything else becomes a 500 that keeps the underlying message instead of a generic label - applied to all 21 unified swap/clmm/amm route handlers (tx routes previously swallowed the cause; query routes rethrew raw; swap routes had their own wrapper) - open.ts: consolidate the mid-file import block at the top — the eslint hook had sorted the ../common import below the schema const that uses it at module evaluation, breaking require-order-sensitive loads Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
…everywhere - unified routes drop the schema-level slippagePct default (Fastify injected it before the handler, shadowing connector defaults): omitted slippage now falls through to each connector's configured slippagePct, fallback 1 - orca: replace hardcoded '= 1' slippage defaults with OrcaConfig.config.slippagePct ?? 1 across quoteSwap/executeSwap/quotePosition/ openPosition/addLiquidity/removeLiquidity; remove-request schema default now config-driven too - meteora/orca CLMM remove-liquidity: rename liquidityPct -> percentageToRemove, matching every other connector, the unified routes, and meteora's own AMM route. The old name let clients sending percentageToRemove (hummingbot's gateway_http_client does) silently remove 100% via the schema default. - jupiter swap test: assert the current execute-swap surface (slippagePct + approximateIfNoExactOut) instead of the removed priority-fee fields Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
Snapshot was ~318 commits stale — still carried liquidityPct on the meteora/orca CLMM remove routes and the removed schema-level slippagePct defaults on the unified trading routes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
A single not.objectContaining({a, b}) passes when either key is missing;
check each removed priority-fee field on the actual call body instead.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
…oute /trading/swap/quote|execute already dispatches by connector/type (jupiter/router, raydium/amm, meteora/clmm, ...) and resolves the pool internally, so the pool-scoped /trading/amm/quote-swap and execute-swap duplicated it with a narrower surface. Wire the missing meteora/amm branch into the unified route and delete the pool-scoped pair; callers (order executor included) always go through /trading/swap regardless of connector type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
…ed slippagePct Three defect clusters: - pancakeswap-sol (G3): all six clmm routes now call the shared solana.throwIfLandedWithError before returning PENDING, so a transaction that landed on-chain and failed throws 400 TRANSACTION_FAILED instead of being reported as status 0 forever. collectFees no longer removes 1% of the position's liquidity and reports the removed amounts as "fees" — the program (a Raydium CLMM fork) collects owed fees via decrease_liquidity_v2, so the route now sends a zero-liquidity decrease that collects the real fees without touching liquidity. - Solana route confirmation (G4): retire the `txData !== null` == confirmed pattern. New Solana.getConfirmedTransactionData(signature) does the route-level re-fetch with retry (a just-confirmed tx whose data lags RPC visibility is no longer misreported as PENDING) and throws the shared landed-but-failed error on meta.err; every meteora/raydium/orca route and the jupiter/dflow/okx/titan executeQuote paths use it. handleConfirmation now takes raw txData (no confirmed flag), checks meta.err itself, and re-fetches with retry when no data is passed. throwIfLandedWithError is public, accepts optional txData, and throws the new 400 TRANSACTION_FAILED (error-handler) instead of mislabeling a landed tx as SIMULATION_FAILED. - slippagePct echo (D4): the four swap-execute response schemas (chain/router/amm/clmm) gain an optional data.slippagePct, populated by every executeSwap/executeQuote implementation (Solana amm/clmm/router and uniswap/pancakeswap/0x) with the slippage actually applied — the request value when given, else the connector's configured default. quote-cache gains getRequest() so 0x can recover the applied value at execution. Tests: new confirmation-helpers suite pins the retry/meta.err/slippage contract; new pancakeswap-sol collectFees suite pins zero-liquidity collect and the loud landed-but-failed 400; meteora/okx suites extended for the slippagePct echo and serializer survival. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
The unified swap route resolves a pool from the configured pool list by token pair, which a pool that is not in that list cannot be — a freshly created pool, or one on an unlisted token. Folding the pool-scoped AMM swap routes into it therefore left those pools unreachable. An optional poolAddress restores the pin on the one surface: amm/clmm providers trade against it directly, routers reject it since they choose their own route, and the not-found message now says the pin exists. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
…emoves The orca remove-liquidity tests sent 'percentage' and the unified CLMM route tests sent 'network' — both were renamed (percentageToRemove, chainNetwork) and Fastify silently dropped the stale keys, so every case ran against schema defaults instead of the values under test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
The spec still published /trading/amm/quote-swap and execute-swap, which were folded into the unified swap route, and carried neither the poolAddress pin nor the applied-slippage echo. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
collectFees removed 1% of the position and reported the withdrawn principal as fees — mutating the position on a read-shaped verb and mis-stating the amounts. The Raydium CLMM program transfers owed fees on any decrease_liquidity, so a zero-liquidity decrease collects them and leaves the principal intact, matching the pancakeswap-sol fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
GW-9 left 32 of 100 components referenced by nothing, read at the time as dead classes a generator would emit and nobody would use. They were worse than dead. Every one was a pre-refactor base, and the GET routes had no component of their own, so the obvious names were held by shapes no route serves: GET /trading/clmm/quote-swap ClmmQuoteSwapRequest network, no connector GET /trading/clmm/fetch-pools FetchPoolsRequest 4 fields, not 9 GET /trading/clmm/position-info GetPositionInfoRequest walletAddress, not a param The same held for responses: ClmmQuoteSwapResponse and AmmQuoteSwapResponse were published while both routes answer with ChainQuoteSwapResponse. Exactly the trap GW-9 fixed for the bodies, still live for the reads — and the reads are most of this API. The premise that blocked it was false. A GET's fields were assumed unable to become a component because @fastify/swagger expands a querystring into `parameters`. Registering a schema and referencing it are independent: addSchema publishes it, and the operation still expands its parameters unchanged. So the $id moves off each stale base and onto the route's own querystring, the same move GW-9 made for the bodies. All 12 trading GETs now publish a component matching their query exactly. 28 stale bases and 3 orphaned `data` shapes lose their $id and keep their shape as composition sources. src/trading/clmm gains a barrel — its four read routes live outside trading-clmm-routes, so nothing was collecting them. TokenSchema publishes as Token, the name the stale TokensResponse was holding. 100 components down to 80. The 14 still unreferenced are all GET request models, which cannot be referenced by construction. request-components.test.ts grows two assertions: every /trading GET publishes a component matching its query, and no component is both unreferenced and not some GET's query shape — which is what fails if a stale base is republished. Mutation-checked both ways.
Fifteen routes parsed chainNetwork and kept only the network half, then dispatched on connector alone. The chain the caller named changed nothing: "ethereum-mainnet" with a Solana connector ran that connector against network "mainnet", and "banana-mainnet-beta" ran it, successfully, against "mainnet-beta". On /add or /open that submits a transaction for a request Gateway could have proved wrong before signing. The pool-scoped swap routes never had this, because they fetch their ops through the registry's lookup, which compares connector and chain. The trading/clmm reads switch on chain and reject an unknown one. Only the liquidity routes, which call their connector module directly and so have no ops to fetch, went unchecked. Two guards, because neither implies the other. chainNetworkField carries an enum of the configured chain-networks, read from the config rather than listed, which rejects a selector naming no real network. A valid selector paired with the wrong connector is invisible to that enum, so resolveChainNetwork replaces the bare parse and asks the registry the same question the swap routes ask. CLMM_SWAP_CONNECTORS/AMM_SWAP_CONNECTORS become CLMM_CONNECTORS/AMM_CONNECTORS and common.ts re-exports them. The hardcoded copies it held were a second roster that could drift from the table that dispatches them. The new guard test includes a structural case asserting no route under trading-amm-routes or trading-clmm-routes parses its own chainNetwork, so a sixteenth cannot reopen this quietly. Two suites had been sending incoherent pairs and relying on a downstream error; they now name each connector with the chain it runs on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
Gateway resolves a pool by token pair from its configured list, so a pool that is not in that list can only be traded by passing its address every time, and a token that is not in the token list has no symbol to pair by. Both facts are recoverable from the chain at the moment they are first used, so they are. Chain-only, deliberately. /pools/save and /tokens/save read the same facts from GeckoTerminal, which is ~30 calls a minute for the whole process and spends up to five of them per pool — one for the pool, then the two token lookups twice over. Name, symbol and decimals are all on-chain, so this reads the RPC already being paid for and no third-party quota is involved. Solana keeps only decimals on the mint. The name and symbol are in one of two places and both are read, because neither covers the other's tokens: the Token-2022 metadata extension, which most newly minted tokens use, and the Metaplex metadata account, where every legacy SPL token keeps them. The latter is read directly — a PDA and three Borsh strings after a fixed header — rather than by adding the Metaplex SDK for it. Ethereum needed nothing new: the ERC-20 ABI in getContract already declares name, symbol and decimals. Three things it refuses to do, each of which would corrupt the lookups everything else depends on. It never stores a token the chain would not name, so the DUMMY_xxxx placeholder getToken invents to keep a swap priced stays out of the list. It never stores a token whose symbol is already held at another address, because addToken treats that as an update and would repoint the existing entry — wrapped SOL reports its symbol as "SOL". And it records no pool when either side is unnamed, since a pool is filed under its pair. Recording is bookkeeping and never reaches the caller: a swap that priced correctly has answered the question it was asked, whatever happened to the write behind it. recordQuietly makes that guarantee where a caller is waiting rather than trusting each recording path to keep it. Hooked into the pool-info routes and both pool-scoped swap routes, which learn the pool and its two tokens, and the router swap routes, which have no pool to record and learn the two token addresses they were given. A known pool costs one list read and no RPC, so the cost falls on the first use and nowhere else. Verified against mainnet: USDC and BONK through Metaplex, PYUSD through the Token-2022 extension, and null for a wallet address and for a non-address. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
getToken searched the configured list and nothing else, so a token the list omitted did not exist as far as Ethereum was concerned. Every route that describes a pool by its token addresses gave up on it: Uniswap's pool-info answers "Token information not found for pool" for a real pool on real tokens, because it asks getToken for both sides and throws when either is missing. That also put the pool out of reach of the recording added in f0d8097 — the connector failed before there was anything to record, so on Ethereum a pool could only be learned when its tokens were already known. Solana has never had this. Its getToken reads the mint when the list misses, which is why the same flow works there. This is the Ethereum half of that, using the fetchTokenFromChain added alongside it: three ERC-20 view calls whose ABI getContract already declares. Contained to addresses. A symbol names nothing the chain can be asked about, so an unlisted one still resolves to nothing and every caller that passes a symbol — wrap, unwrap, approve by symbol — is unchanged. Only a checksummable address that answers name, symbol and decimals resolves where it previously did not, and an address that is not an ERC-20 still resolves to nothing. Successful reads are remembered for the life of the process, because getToken runs in loops — over the tokens of a balance request, over every position a wallet owns — and an omitted address would otherwise cost three calls on every pass for three values that cannot change. Misses are not remembered: an address with no contract today may have one tomorrow. Verified against mainnet: PEPE-WETH on Uniswap answered 400 before and now returns pool info, records PEPE (Pepe/18) and the pool, and the pair quotes by name with no address. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
…e routes The AMM surface had four liquidity routes where two carry the meaning. open was a synonym. add declares positionAddress as "omit to open a new position", meteora's addLiquidity delegates to openPosition when it is absent, and AmmOpenPositionResponseData and AmmAddLiquidityResponseData had identical properties. The only thing open.ts did on its own was re-frame a fungible-LP add to report positionRent: 0 with no address. close was not a synonym, and that was the problem: remove at 100% withdrew all the liquidity and left the empty position NFT behind still holding its rent, while close withdrew and closed the account in one transaction. Nothing downstream called close — hummingbot-api implements amm_add_liquidity and amm_remove_liquidity and neither of the others — so every full exit through the API stranded the rent. On a position opened with 0.0053 SOL of liquidity that is 0.0099 SOL left behind, more than the position held. So removeLiquidity at 100% now delegates to closePosition, and both routes are gone. positionRentRefunded joins the remove response as optional: present when the removal closed the account, absent on a partial removal and on fungible-LP AMMs, which have no account to close — rather than a 0 that would read as "closed, refunded nothing". openPosition and closePosition stay as the internal calls add and remove delegate to, now typed by the add and remove response shapes. app.integration asserts the two paths are NOT registered, so re-adding one is a decision rather than a drift back. The open/close route suite is replaced by a delegation test that pins the 100% boundary from both sides. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
Opening a DAMM v2 position with 3000 base and the quote side the pool asked for recorded quoteTokenAmountAdded as 0.015220830 — the whole SOL outflow net of gas — for a position that holds 0.005323709. The difference is the rent for the position account, which the chain returns on close and which is therefore locked, not spent. The stored position read 2.86x its real size, and against a close that does net the refund out it fabricates a ~186% loss on a round trip whose real cost is the pool fee. closePosition already backed the refund out of the native side. openPosition computed the same quantity, returned it, and did not apply it. Both now go through one helper, liquidityWithoutRent, rather than holding a copy each of the same subtraction. Two supporting renames, both because the units are what make this silent. accountLamports returns SOL — it divides the raw balances — so it is now accountBalanceSol and says so; callers subtract it from token amounts, where being out by 1e9 would clamp every native side to zero without a word. The magic native-mint string in closePosition is now NATIVE_MINT from @solana/spl-token. The tests run against the helper the routes call, not a restatement of it, and cover the live case, the non-native side, the clamp, the direction, and the lamports-vs-SOL failure. Mutation-checked: dropping the subtraction, dropping the clamp, inverting the side, and returning lamports each fail them. Response shapes are unchanged; openapi.json regenerates identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166iQoxKce23GkUwuQJxdkr
… wallet moved Three Raydium sites passed the raw balance change through to `…Added`. A deposit moves tokens out, so the value was negative, and hummingbot-api stores it verbatim: an add of 0.01 SOL was recorded live as -0.01. Every other connector, and the whole removed side including Raydium's own remove, reports magnitudes — so summing an event table nets a round trip on this connector and double-counts it on all the others, with nothing in a stored row saying which convention produced it. The two adds take Math.abs. clmm/openPosition carried a second defect as well: opening a position locks rent in the position account, and when a side is SOL that outflow sits inside the same balance change, so the raw delta was both negative and larger than the deposit. That is GW-20 on a second connector — e3eb7b1 reached only the DAMM v2 open — and it now uses the same liquidityWithoutRent helper. Two existing tests asserted toHaveProperty('baseTokenAmountAdded'), the key rather than the value, so they passed on every negative that reached the event table. Both now assert amounts. clmm/addLiquidity had no test at all and has one now. openPosition's suite mocked the whole solana.utils module with a single function, which is why the helper was undefined there; it spreads the real module now, as the chain-config mocks already do. Also drops a console.log of the whole quote response on every CLMM add. Rows already written carry the old signs, so reading history across the cutover still needs care — that part is not fixed by this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
…, GW-24) Two of the spec's values came from whoever ran the generator rather than from Gateway. The walletAddress default is the configured defaultWallet — right at runtime, since a caller who omits it means "the wallet I configured", and wrong in a checked-in artifact: a real trading wallet was committed 21 times over, to a repo with a GitHub remote, and vendored onward into hummingbot-api's copy and its generated models. servers[0].url carried this machine's port. src/templates/chains/*.yml already ship placeholders for the first; the artifact simply disagreed with them. The generator now substitutes both. Verified by generating against a templates-only conf, the way CI builds it, and getting a byte-identical file — which is what makes the drift check possible at all. That check is the other half. test/spec/request-components.test.ts read the committed spec and claimed that this also caught a spec left unregenerated. It does not: adding a field to a route body without regenerating left all 30 cases passing. Only regenerating and comparing catches drift, so CI does that, and the file's comment now says what it does and does not cover. The twin blind spot is closed separately rather than left to CI, because the two answer different questions — CI catches "the file is stale", the test catches "the component a client imports is missing". Each /trading GET is pinned to the component name derived from its own path, then to its shape. A name is unique where a shape is not: AmmQuoteSwapRequest and ClmmQuoteSwapRequest have identical fields, so dropping one $id removed a component from the spec with every case still passing. That mutation now fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
increaseLiquidityQuote{A,B} returns tokenEst* — what the deposit should be — and
tokenMax*, the same figure inflated by slippageBps. Both the open and the add handed
tokenMax* to the instruction builder as the amount to deposit, and passed
slippageToleranceBps alongside it, so the tolerance was applied twice and the ceiling
became the target.
Found live on Orca one-sided opens: 1 USDC funded deposited 1.009999, and 0.01 SOL
deposited 0.010099999 — slippagePct over, exactly. A two-sided open in the same minute
came in under on both sides, which is why this survived: with both sides funded the pool
ratio pins the deposit and the bound is never reached, so only a one-sided range shows it.
addLiquidity had the identical call and was not part of that session's testing. Its log
line already reported tokenEst* while the transaction spent tokenMax*, which is as clear a
statement of the bug as the live numbers.
Both tests kept tokenEst and tokenMax equal or absent, so neither could tell the two
apart; they now differ in the fixtures, and the open's assertion moves from the ceiling to
the estimate. Reverting either site fails its own case.
Left alone: whether any tolerance belongs in that amount at all. Slippage protects a
ratio, and a one-sided position has none — Meteora passes the caller's figure and keeps
slippage purely as a tolerance. That is a design question, not this bug.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
…14, GW-15, GW-16)
Three issues, one surface: what a generated client can actually import.
GW-14. No operation carried an operationId, so every generator invented one from the
method and path and a caller's client.foo() was renamed by any path change — the churn
$ids were added to keep out of the component names. Deriving them here would reproduce it
exactly, so operation-ids.ts chooses them: a rename moves the key and leaves the name a
caller depends on alone. They read as <verb><Subject>, because a client calls
openClmmPosition(), not postTradingClmmOpen().
Three of 56 operations declared any non-2xx response, so a client had models for success
and nothing for failure — while `code` is the field callers branch on to decide whether to
retry. ErrorResponse is published once and attached to every operation by the swagger
transform; 400 and 500 are reachable everywhere, since Fastify answers 400 for anything
its schema rejects and rethrowRouteError turns the rest into 500. Three pool routes had
hand-written {message: string} error shapes, which is not what Gateway sends; they are
gone, and the real envelope covers them. The two ethereum routes tagged /chain/ethereum —
singular, and not among the declared tags — join /chains.
GW-15. The requests were prefixed and the responses were not, so the unprefixed name was
the CLMM one and a reader had to know that. PoolInfo/PositionInfo/AddLiquidityResponse and
five more take Clmm, the AMM create-pool and quote-liquidity take Amm, and
QuotePositionResponse — whose route was renamed to quote-liquidity in the refactor —
becomes ClmmQuoteLiquidityResponse. Only the $ids move: the component namespace is global
and the TypeScript one is not, which is why AddLiquidityResponse reads fine inside
clmm-schema.ts. The two ethereum responses, the last on /chains without a name, get one.
GW-16. 36 schema exports left behind by the deleted per-connector routes, ~1300 lines,
including three files that exported nothing at all. 22 tests asserted those shapes were
supersets of a base — a contract test for an API that no longer exists — and are gone with
them; the response cases in the same files still describe live schemas and stay.
parseChainNetwork had three implementations that disagreed on the same input: trading
rejected a value with no hyphen, ConfigManagerV2 answered network '' for it, and findPools
hand-rolled the lenient split inline and stamped that empty network onto every pool it
returned. One implementation now, in services/chain-network, with trading adding only the
400. The fourth, in config/utils, asks a genuinely different question — is this namespace a
chain-network one — and is renamed to say so.
Guards for each: operation-ids.test.ts holds the table to the route table in both
directions and rejects a duplicate name; response-components.test.ts pins Amm/Clmm
symmetry and names the five components that must not come back.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
Three conventions coexisted, two of them inside a single router: `/pools/` took `chain`
and `network` separately, `/pools/find` took `chainNetwork`, and `/chains/{chain}/*` took
a path parameter plus a query. A caller had to learn which route wanted which, and the
split form let the two halves disagree — `chain=solana&network=mainnet` names nothing that
exists.
Everything outside `/chains/{chain}/` now takes one `chainNetwork`: eight routes across
/pools and /tokens, and the response of GET /tokens/{symbolOrAddress}, which echoed the
selector in a form the request could no longer use. `/chains/{chain}/*` keeps chain in the
path, which is the one place the two halves cannot disagree.
/wallet is the deliberate exception and keeps `chain` alone. Wallets are stored per chain
on disk because a keypair works on every network of that chain; giving them a network
would invent a distinction that does not exist.
The selector moved to schemas/chain-network-field so pools and tokens can share the field
rather than declaring their own — importing it from the trading layer would have been the
wrong direction. It could not live beside the parser in services/chain-network: that is
what ConfigManagerV2 delegates its own parsing to, so reading the config from there closes
a cycle, which is what the failing suites were telling me.
Its default is now opt-out, and /pools and /tokens opt out. Fastify injects a schema
default before the handler runs, so a defaulted chainNetwork on DELETE /pools/{address}
picks a network and deletes from it — the delete test caught exactly that, having gone from
400 to 200. GET /tokens likewise stops answering an empty list when the caller named no
chain, which read as "no tokens here" and meant "you did not say where".
Also removes 38 dead imports across 22 files. Only imports: a local binding may be a
deliberately-ignored destructure or a signature parameter, where deleting it changes
behaviour rather than removing dead weight.
test/spec/addressing.test.ts holds the rule for all 56 operations, and separately that no
write to /pools or /tokens carries a default.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
`isSensitivePath` named /wallet, /config/update, /restart and the fund-moving
/trading verbs. It never named /chains/*, so with GATEWAY_API_KEY set and Gateway
reachable over a network, an unauthenticated POST /chains/ethereum/approve had the
hot wallet sign an unlimited allowance to an address the caller chose — and
transferFrom did the rest. /chains/{chain}/wrap and unwrap sign too. Reported
privately as GHSA-r4q2-79mv-2355, reproduced end to end on Sepolia; confirmed here
against the route list rather than taken on trust.
The gap is not really the missing pattern, it is that the list is written by hand
beside a route table that grows. So the check is now derived from the spec: a route
that broadcasts a transaction answers with the identifier of the transaction it just
signed, while a route that merely reports on one is handed that identifier. Every
operation returning a `signature` it was not given must be behind the gate — 17 of
them today, and any new one fails the test rather than shipping open.
The read-only chain routes stay public, as the read-only trading routes do: status,
estimate-gas, balances, allowances and poll sign nothing, and gating poll would break
a co-located bot following its own transaction for no security gain.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
…ange Three live positions closed on 2026-08-20 and every one of them reported a number that was not the liquidity it moved. One cause, three shapes. **Rent was read from one account.** A position is several rent-bearing accounts: on DAMM v2 the position, the NFT that represents it and that NFT's token account; on a PancakeSwap CLMM five, counting the shared protocol position and a first-touch tick array. Subtracting only the position's own rent left the rest inside the reported amount. Closing `F1YcTMd6…` recorded 0.021879499 SOL withdrawn against a pool payout of 0.005299128 — 4.1x — and called 0.00373056 of a 0.01194840 refund the whole refund. Opening `3ttZ9Nfq…` recorded 0.023636031 against 0.009987471 deposited, where the rent was larger than the position it was attached to. `accountLifecycleSol` reads it from the transaction instead: what every account created or closed moved, and the rent share of that, which differ only when a wrapped-SOL account the wallet already had a balance in was closed. Raydium's opens and closes stop asserting a hardcoded 0.00204928 — one token account's worth, for positions that lock four or five accounts' worth. **pancakeswap-sol never unwrapped what it withdrew.** Closing a SOL position moved the SOL into the wallet's WSOL account and stopped there, so the only thing that touched the native balance was the rent — and the close reported that rent, to the lamport, as the liquidity withdrawn, with rent of 0 beside it. Every close left another balance parked in that account and its rent with it. The withdraw paths now close it, which is the unwrap; the swap path in the same connector always did. Every number in the tests is a real mainnet transaction reduced to the balances the helper reads — 67ZzMAHv…, 2CMNt7Bk…, Cy8wiJaS… — so they assert what happened rather than restate the arithmetic. Fees on a pancakeswap-sol close are still folded into the principal: that program moves a position's fees and its principal in one `decrease_liquidity_v2` transfer, so telling them apart needs the fees collected by an instruction of their own, and that is a transaction-shape change worth proving against a live position first. GW-18 (in part), GW-20 (generalised), GW-29, GW-30, GW-31. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
96 bindings that nothing read, across 52 files: imports, example constants left over
from route schemas that moved, request/response types replaced by the unified
surface, quote values computed and then not returned, and SDK results destructured
for a field nobody wanted. The previous sweep stopped at imports because a local
binding can be a deliberately-ignored destructure; each of these was read instead of
guessed, which is what turned up the two below.
**Adding liquidity with ETH as the base token could never have worked.** That branch
of uniswap's AMM add called `uniswap.getToken('WETH')` against a binding declared
further down the same function — a guaranteed ReferenceError, reachable by anyone
passing baseToken='ETH'. Deleting the unused later declaration turned it into a
compile error. It now resolves its own instance, as the quote-token branch beside it
always did.
**pool-info-helpers made an RPC call for nothing.** The V2 branch fetched the pair's
factory address, built a contract on it, and then set the fee from a constant,
because a V2 pair does not expose its fee on-chain. The call and both ABIs are gone;
the constant and the comment explaining it stay.
Two computations that read like dropped guards are neither, and were checked before
being removed: uniswap's clmm remove and close computed amount0Min/amount1Min beside
the `slippageTolerance` they hand to `removeCallParameters`, which derives the same
minimums itself; raydium's amm add destructured the quote's ...Max fields beside the
`Percent` it passes to the transaction builder.
`ignoreRestSiblings` is on, because `const { poolType, ...rest }` is how you drop a
key, not dead code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
Raydium's CLMM removal had the same arithmetic GW-31 found on Meteora: at 100% it closes the position and its NFT account in the same transaction, so their rent lands in the same native balance change as the withdrawal, and `Math.abs(change)` published all of it as liquidity removed. The fee on a close is derived by difference — the whole balance change less the liquidity the removal reported — so the two have to be measured on the same basis. Netting the removal alone would have turned the rent into fee income on every close, which is why both sides move together and a test pins each direction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
…e units promised Two ways a quote described itself wrongly, both in fields a caller acts on. **The no-route error named a route nobody tried (GW-34).** A SELL is ExactIn base -> quote; a BUY is ExactOut quote -> base. Every router built this message from the SELL shape and reused it for both, so a BUY that declined approximation came back as "No route found for DOGE-1 -> SOL (ExactIn)" — opposite direction, opposite mode, about a token that routes ExactIn perfectly well, since ExactIn is what the approximation would have used. It matters because the message is a NO_ROUTE_FOUND: jupiter's own file carries a fix for mislabelling a failure that way, after callers blacklisted good pools over it. `attemptedRoute` derives both halves from `side`. dflow and titan reach their branch only on a SELL, where the old label happened to be right; they go through the helper so it stays right if the branch ever serves a BUY. **priceImpactPct was a fraction in a field documented as a percentage (GW-38).** Jupiter's field of that name is a decimal fraction — 0.0126 is 1.26% — and it was passed through unconverted, so the number was 100x low in the direction that makes a bad trade look harmless, and `if (priceImpactPct > 5) reject` could never fire. Measured on SOL-USDC: 20,000 SOL reported 0.001260 against 0.134% computed from the quoted prices. dflow serves Jupiter's quote schema field for field and is converted with it — inferred from the schema, not measured, because its public endpoint refuses an unkeyed request. Meteora's CLMM returned a hardcoded 0 for every quote, so a swap of any size through a Meteora pool claimed zero impact and no caller could tell that from a measurement. It now measures the quote against the pool's active bin, in the same orientation the response prices the pair, which is what Orca has always done. The others were checked rather than assumed: raydium, uniswap and pancakeswap read an SDK `Percent` (already a percentage), meteora's AMM SDK multiplies by 100 itself, orca and pancakeswap-sol compute a percentage, and okx's field is named `priceImpactPercent`. **0x is left alone deliberately**: it reads v1's `estimatedPriceImpact` from a v2 endpoint, so the reported impact is either a silent 0 or 100x high, and settling which needs one live quote — the configured key 401s. **Slippage stopped being a literal on four CLMM routes (GW-37).** uniswap's and pancakeswap's remove and close each wrote `new Percent(100, 10000)`, a flat 1% that ignored both the caller's slippagePct and the operator's configured one, so widening the config for a volatile pair changed nothing and the revert still cost gas. All four take the connector's configured value now, and `/trading/clmm/remove` passes the caller's through to uniswap and pancakeswap as it already did to orca. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
`/trading/clmm/close` had no slippagePct at all, so a close that failed on slippage could only be retried at the same tolerance that had just failed — which is GW-25: a narrow in-range position whose close reverted, twice, with no layer able to widen. The field is honoured by orca, uniswap and pancakeswap. meteora, raydium and pancakeswap-sol close with no minimum-amount check at all — they pass zero minimums — so it changes nothing there, and the description says so rather than implying a protection that is not applied. This is the Gateway half of the executor-level slippage ramp: the executor owns the policy (how far to widen, and when to stop), Gateway stays stateless and does exactly what one request asks for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
**The open was a coin flip (GW-28).** It failed on chain with
`PriceSlippageCheck Left: 891739 Right: 891740` — one unit of USDC against a 2%
tolerance that should have allowed seventeen thousand — and the same request succeeded
four minutes later. Two defects, one symptom.
The first is an encoding trap. `base_flag` is an `Option<bool>` and the route passed
`baseFlag ? { some: true } : { some: false }`. Borsh writes an option as 0x00, or 0x01
followed by the value, and its bool layout is `value ? 1 : 0` — an object is truthy, so
*both* branches encoded Some(true). Every request that meant "size from the quote side"
told the program to size from the base side. Verified by encoding all five forms and
reading the bytes; the test asserts the trap as well as the fix, so nobody reintroduces
the object form believing it works.
The second is the design the doc named: handing the program a side makes that side's max
both the amount to size liquidity from and the ceiling to check against, so the check has
no headroom by construction — it computes the deposit that liquidity requires, rounds it
up in the pool's favour, and asserts the result against the number it started from. One
unit of rounding fails the open, a wider slippagePct only buys a larger deposit, and
which way it lands is decided by where the live price puts a float's fraction. The open
now sends `base_flag: None` with the liquidity the quote already computed from the
amounts the caller asked for — which is what the add route has always done — so the maxes
are ceilings again. It also unwraps afterwards: the deposit lands below the wrapped
maximum now rather than exactly on it, and the difference would otherwise stay wrapped.
**The close said fees were zero (GW-18).** This program moves a position's fees and its
principal through the same `decrease_liquidity_v2` transfer, so one instruction cannot be
taken apart. The close now collects first, in a zero-liquidity decrease — exactly what
the collect-fees route does — and reads the two by their instruction, with the principal
net of what the fee instruction paid. An unreadable transaction leaves fees at 0 and the
amounts whole, which is the shape this route already had.
**`position_info` said the same (GW-18).** It returned a hardcoded 0 for both sides with
a TODO reasoning that a wrong number is worse than none. But zero is a number and it was
being stored: nothing could tell "this connector does not compute fees" from "this
position earned nothing", and a position that sat in range while the pool traded through
it reported 0 while collect-fees harvested a real amount minutes later. Everything the
calculation needs was already read and written to logger.debug before being discarded.
The arithmetic is verified against the program rather than argued: for three live
mainnet positions, the computed figure was compared with what a simulated collect
actually transfers. Two with fees matched to the unit — 16224872 / 1195910 on an
in-range position and 60792125 / 13625723 on an out-of-range one, which exercises the
tick-boundary flip — and the third computed zero, where the program moved nothing. The
tick arrays are decoded with the program's own IDL rather than by counting bytes, and
the subtractions wrap, because fee-growth accumulators are allowed to overflow and only
their difference means anything.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
Three ways a caller's intent was dropped in silence. None returned a wrong answer; each accepted a request and then acted on something other than what was asked. **An undeclared key was deleted, not refused.** No request component set `additionalProperties: false` — 0 of 80 — and Fastify's AJV defaults to `removeAdditional: true`, so `slippagePc: 5` on an execute-swap was quietly stripped and the trade went out at the connector's configured slippage. Both halves are needed: the declaration says which keys exist, and turning off removeAdditional makes the declaration mean 400 rather than delete. The error names the offending property, because "must NOT have additional properties" tells a caller that something is wrong and not what. Turning it on found five payloads in this repo's own tests sending keys the routes never read: a legacy `fee` beside `feeBps` on two create-pool routes, `ammConfig` where the schema says `ammConfigIndex`, `protocols` on a router quote that filters nothing, and `accountIndex`/`name` on add-hardware — a caller naming their Ledger lost the name without being told. **`x-connectors` was documentation.** `configAddress` is meteora's and `ammConfigIndex` is raydium's; passing the wrong one created a pool with the connector's defaults instead of erroring. One preValidation hook checks every marked field against the connector named in the same request. preValidation deliberately: it runs on the body as sent, before AJV fills defaults, and checking afterwards would reject every 0x quote — because `approximateIfNoExactOut` defaults to true and is marked for the Solana routers. **A write picked its own venue.** `connector` carried a schema default, so `POST /trading/clmm/add` with only a positionAddress got `meteora`, and "whichever connector is first in the registry" is not an answer to "which venue?" on a request that signs. The nine signing routes require it now; the reads keep the default, which is what fills the Swagger form. The router routes lose their default for a different reason: `connector` there is optional and documented as "defaults to the network's swapProvider", but a schema default meant `resolveSwapConnector` was handed 'jupiter' before it could read the config — on Ethereum too, where jupiter is a Solana connector. GW-12. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
… wire Response money fields were typed `number`, so a JSON float carried every amount: 0.1 + 0.2 arrives as 0.30000000000000004, and a Solana lamport amount past 2^53 arrives rounded. The Python clients type these as Decimal, so they were reconstructing precision that the wire had already thrown away. DecimalNumber declares `string` + `format: decimal` in the spec while keeping the TypeScript type `number`. Fastify's serializer stringifies at the boundary, so connector arithmetic is untouched — typing the schemas as strings outright produced 721 type errors, because these schemas are the domain types the connectors compute with. Request fields stay numeric: a string there would arrive as a string while the type still claimed number, and handlers would do arithmetic on it. That half needs parsing at each boundary and is a separate change. Tests read the body through parseWire(), which revives numeric strings the way a Decimal-typed client does. Identifier fields are excluded by name — a Uniswap position is addressed by a numeric NFT token id, which must stay a string. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
|
Too many files changed for review (534 files, 100 file limit). |
This was referenced Aug 21, 2026
Two cases omit walletAddress on purpose, to exercise the schema default. That
default is read from conf/chains/solana.yml at module load — and conf/ is
gitignored, so a developer machine supplies a real address while CI falls back
to the template's literal '<solana-wallet-address>'. That is not base58, so
`new PublicKey(...)` throws and the route 500s:
Expected: 200
Received: 500
Failed to collect fees: Expected base58-encoded address string of
length in the range [32, 44]. Actual length: 7
So both tests passed locally, on every machine that happened to have a wallet
configured, and failed on a clean checkout. They now mock getSolanaChainConfig
to a known address, which is also what they meant to assert: that the route
falls back to the CONFIGURED wallet, not to whatever the machine happens to
hold.
Verified by reproducing CI exactly — setting conf/chains/solana.yml to the
template placeholder locally reproduces the 500, and passes with this change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK
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.
Summary
Route unification for
/trading, plus the defects found while validating it against Solana mainnet. Supersedes #679, which was opened from a branch that has since been replaced; the work continues here againstfeat/unified-trading-routes.The spec went from 182 paths to 54: the trading type is now a path segment and the connector a parameter, and
openapi.jsonis generated from the route table bypnpm generate:openapiwithout a running server./connectors/{dex}/{type}/*(128 paths)/trading/swap/{quote,execute}/trading/router/{quote-swap,execute-quote,execute-swap}/connectors/{dex}/{amm,clmm}/quote-swap/trading/{amm,clmm}/quote-swap/trading/amm/{add,remove}-liquidity/trading/amm/{add,remove}/chains/{solana,ethereum}/*(14 paths)/chains/{chain}/*(6) + 2 EVM-onlySecurity: every route that signs is gated
isSensitivePathenumerated paths, and/chains/*was not among them — so unauthenticatedPOST /chains/ethereum/approvecould drain ERC-20 allowances (GHSA-r4q2-79mv-2355). The gate now derives from what a route does, with a spec-driven test asserting that every route returning a signature it was not given is gated. Enumerating is what let this open in the first place.Money survives the wire
Response money fields were typed
number, so a JSON float carried every amount:0.1 + 0.2arrives as0.30000000000000004, and a lamport amount past 2^53 arrives rounded.DecimalNumberdeclaresstring+format: decimalin the spec while keeping the TypeScript typenumber, so Fastify serializes at the boundary and no connector arithmetic changes — typing the schemas as strings outright produced 721 type errors, because these schemas are the domain types the connectors compute with. Both Python generators map it to the sameDecimalas before.A request that would be misread is now rejected
Fastify's AJV defaulted to
removeAdditional: true, so a key a route never read was stripped in silence. Turning strictness on immediately caught five payloads in this repo's own tests: a legacyfeebesidefeeBps(twice),ammConfigwhere the schema saysammConfigIndex,protocolson a router quote that filters nothing, andaccountIndex/nameon add-hardware — so a caller naming their Ledger lost the name silently.Reported amounts are the money that moved
accountLifecycleSolreports every opened/closed account's lamports, with the rent share separated.0on close and on read. Fees are now computed from tick-array growth, verified to the unit against a simulated collect on three live mainnet positions; the open's{ some: false }Borsh encoding (which encodes asSome(TRUE)) is fixed, so the slippage maxes are ceilings again.priceImpactPctwas a fraction in a field documented as a percentage — 100x low. Converted at the router boundary; Meteora's hardcoded0now computed. Five connectors verified to agree.A close can say how much slippage it will accept
/trading/clmm/closetakesslippagePct, which is what lets an executor widen its tolerance across retries rather than repeating an identical request (see the hummingbot PR). uniswap and pancakeswap stop hardcoding 1%.Companion PRs
chainNetworkon tokens/pools, volume from fees, Gateway swap observabilityexecute_quote, and the spec refreshed for bothbin_countonget_pool_infoValidation
tscclean; 1440 tests across 158 suites pass. Validated live on Solana mainnet: all three swap types, the full CLMM lifecycle on Meteora and Orca across all three range shapes, the Raydium AMM round trip, a Meteora DAMM v2 open, and fee collection against a position that had fees.The pancakeswap-sol fee math was verified against the program itself: for three live positions, computed fees equalled a simulated collect's transfers to the unit (16224872/1195910 in-range; 60792125/13625723 out-of-range; 0/0 where nothing moved).
🤖 Generated with Claude Code
https://claude.ai/code/session_01GuR4iDtH4ZfT51rJfs3DhK