feat: add multi-DEX swap router with optimal route finding#47
Open
HERO570 wants to merge 1 commit intoStellar-Tools:mainfrom
Open
feat: add multi-DEX swap router with optimal route finding#47HERO570 wants to merge 1 commit intoStellar-Tools:mainfrom
HERO570 wants to merge 1 commit intoStellar-Tools:mainfrom
Conversation
Add a swap router that finds the best multi-hop trade routes across all major Stellar liquidity sources (Soroswap, Phoenix, SDEX) with mainnet support. Key features: - Adapter pattern for pluggable DEX integrations - Graph-based pathfinding (BFS with hop limit) for optimal routes - Pool registry with time-based caching and live reserve refresh - Backward-compatible API: existing swap() calls work unchanged - New strategy: "best-route" opt-in for routed swaps - Route preview via getSwapRoute() without executing - LangChain tool support via routed_swap action New files: - router/types.ts - Core interfaces (DexAdapter, Pool, Quote, Route) - router/config.ts - Network configs (mainnet/testnet) - router/graph.ts - Token graph with AMM math and pathfinding - router/registry.ts - Pool registry with caching - router/router.ts - SwapRouter orchestrator - router/adapters/soroswap.ts - Soroswap DEX adapter - router/adapters/phoenix.ts - Phoenix DEX adapter - router/adapters/sdex.ts - SDEX adapter (Horizon order book) Modified files: - agent.ts - Added strategy param, getSwapRoute(), lazy router init - index.ts - Export router modules - tools/contract.ts - Added routed_swap action - utils/buildTransaction.ts - Added "route" operation type
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| - | - | Generic High Entropy Secret | 173cb53 | tests/unit/router/types.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 22 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/unit/router/adapters/sdex.test.ts">
<violation number="1" location="tests/unit/router/adapters/sdex.test.ts:45">
P2: Pool discovery test uses a tautological length assertion, so it cannot catch regressions where no pools are discovered.</violation>
</file>
<file name="router/adapters/sdex.ts">
<violation number="1" location="router/adapters/sdex.ts:92">
P2: Converting bigint amounts to Number before formatting can lose precision for large trades, producing incorrect Horizon quotes and slippage limits. Format the 7-decimal amount directly from bigint (or use a bigint-capable decimal library) to avoid precision loss.</violation>
<violation number="2" location="router/adapters/sdex.ts:146">
P1: SDEX swap operation uses a hardcoded placeholder destination, but the router never overwrites it before submission, so swaps will target the placeholder account or fail.</violation>
</file>
<file name="router/router.ts">
<violation number="1" location="router/router.ts:55">
P2: Converting bigint expectedAmountOut to Number can lose precision for large on-chain amounts, producing an incorrect minOut and undermining slippage protection.</violation>
<violation number="2" location="router/router.ts:61">
P2: Route execution always uses the precomputed leg.amountIn for each swap, so if a prior leg yields less than expected (but above minOut), the next leg can fail due to insufficient balance. This breaks multi-hop execution under slippage because actual outputs aren’t propagated to subsequent legs.</violation>
</file>
<file name="router/registry.ts">
<violation number="1" location="router/registry.ts:31">
P2: Cache is bypassed when the pool list is empty, so callers will re-run discovery on every getPools() call even within refreshIntervalMs. This defeats the refresh interval for valid empty states and can hammer adapters.</violation>
</file>
<file name="router/graph.ts">
<violation number="1" location="router/graph.ts:34">
P2: Converting bigint reserves/amounts to Number in price impact calculation can lose precision for large pools, producing incorrect price impact values.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const op = Operation.pathPaymentStrictSend({ | ||
| sendAsset: sourceAsset, | ||
| sendAmount: sendAmount, | ||
| destination: "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF", // placeholder, set by router |
There was a problem hiding this comment.
P1: SDEX swap operation uses a hardcoded placeholder destination, but the router never overwrites it before submission, so swaps will target the placeholder account or fail.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At router/adapters/sdex.ts, line 146:
<comment>SDEX swap operation uses a hardcoded placeholder destination, but the router never overwrites it before submission, so swaps will target the placeholder account or fail.</comment>
<file context>
@@ -0,0 +1,154 @@
+ const op = Operation.pathPaymentStrictSend({
+ sendAsset: sourceAsset,
+ sendAmount: sendAmount,
+ destination: "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF", // placeholder, set by router
+ destAsset: destAsset,
+ destMin: destMin,
</file context>
| }); | ||
|
|
||
| const pools = await adapter.discoverPools(); | ||
| expect(pools.length).toBeGreaterThanOrEqual(0); |
There was a problem hiding this comment.
P2: Pool discovery test uses a tautological length assertion, so it cannot catch regressions where no pools are discovered.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/unit/router/adapters/sdex.test.ts, line 45:
<comment>Pool discovery test uses a tautological length assertion, so it cannot catch regressions where no pools are discovered.</comment>
<file context>
@@ -0,0 +1,78 @@
+ });
+
+ const pools = await adapter.discoverPools();
+ expect(pools.length).toBeGreaterThanOrEqual(0);
+ });
+
</file context>
Suggested change
| expect(pools.length).toBeGreaterThanOrEqual(0); | |
| expect(pools.length).toBeGreaterThan(0); |
| const sourceParams = assetToQueryParams(sourceAsset); | ||
| const destParams = assetToQueryParams(destAsset); | ||
|
|
||
| const amountStr = (Number(amountIn) / 10_000_000).toFixed(7); |
There was a problem hiding this comment.
P2: Converting bigint amounts to Number before formatting can lose precision for large trades, producing incorrect Horizon quotes and slippage limits. Format the 7-decimal amount directly from bigint (or use a bigint-capable decimal library) to avoid precision loss.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At router/adapters/sdex.ts, line 92:
<comment>Converting bigint amounts to Number before formatting can lose precision for large trades, producing incorrect Horizon quotes and slippage limits. Format the 7-decimal amount directly from bigint (or use a bigint-capable decimal library) to avoid precision loss.</comment>
<file context>
@@ -0,0 +1,154 @@
+ const sourceParams = assetToQueryParams(sourceAsset);
+ const destParams = assetToQueryParams(destAsset);
+
+ const amountStr = (Number(amountIn) / 10_000_000).toFixed(7);
+
+ const url = `${this.config.horizonUrl}/paths/strict-send?source_${sourceParams}&source_amount=${amountStr}&destination_${destParams}`;
</file context>
|
|
||
| const slippageMultiplier = 1 - slippage / 100; | ||
| const minOut = BigInt( | ||
| Math.floor(Number(leg.expectedAmountOut) * slippageMultiplier) |
There was a problem hiding this comment.
P2: Converting bigint expectedAmountOut to Number can lose precision for large on-chain amounts, producing an incorrect minOut and undermining slippage protection.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At router/router.ts, line 55:
<comment>Converting bigint expectedAmountOut to Number can lose precision for large on-chain amounts, producing an incorrect minOut and undermining slippage protection.</comment>
<file context>
@@ -0,0 +1,110 @@
+
+ const slippageMultiplier = 1 - slippage / 100;
+ const minOut = BigInt(
+ Math.floor(Number(leg.expectedAmountOut) * slippageMultiplier)
+ );
+
</file context>
| const op = await adapter.buildSwapOp( | ||
| leg.pool, | ||
| leg.tokenIn, | ||
| leg.amountIn, |
There was a problem hiding this comment.
P2: Route execution always uses the precomputed leg.amountIn for each swap, so if a prior leg yields less than expected (but above minOut), the next leg can fail due to insufficient balance. This breaks multi-hop execution under slippage because actual outputs aren’t propagated to subsequent legs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At router/router.ts, line 61:
<comment>Route execution always uses the precomputed leg.amountIn for each swap, so if a prior leg yields less than expected (but above minOut), the next leg can fail due to insufficient balance. This breaks multi-hop execution under slippage because actual outputs aren’t propagated to subsequent legs.</comment>
<file context>
@@ -0,0 +1,110 @@
+ const op = await adapter.buildSwapOp(
+ leg.pool,
+ leg.tokenIn,
+ leg.amountIn,
+ minOut
+ );
</file context>
|
|
||
| async getPools(): Promise<Pool[]> { | ||
| const now = Date.now(); | ||
| if (this.pools.length > 0 && now - this.lastRefresh < this.refreshIntervalMs) { |
There was a problem hiding this comment.
P2: Cache is bypassed when the pool list is empty, so callers will re-run discovery on every getPools() call even within refreshIntervalMs. This defeats the refresh interval for valid empty states and can hammer adapters.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At router/registry.ts, line 31:
<comment>Cache is bypassed when the pool list is empty, so callers will re-run discovery on every getPools() call even within refreshIntervalMs. This defeats the refresh interval for valid empty states and can hammer adapters.</comment>
<file context>
@@ -0,0 +1,64 @@
+
+ async getPools(): Promise<Pool[]> {
+ const now = Date.now();
+ if (this.pools.length > 0 && now - this.lastRefresh < this.refreshIntervalMs) {
+ return this.pools;
+ }
</file context>
| reserveOut: bigint | ||
| ): number { | ||
| if (reserveIn === BigInt(0) || reserveOut === BigInt(0)) return 100; | ||
| const spotPrice = Number(reserveOut) / Number(reserveIn); |
There was a problem hiding this comment.
P2: Converting bigint reserves/amounts to Number in price impact calculation can lose precision for large pools, producing incorrect price impact values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At router/graph.ts, line 34:
<comment>Converting bigint reserves/amounts to Number in price impact calculation can lose precision for large pools, producing incorrect price impact values.</comment>
<file context>
@@ -0,0 +1,185 @@
+ reserveOut: bigint
+): number {
+ if (reserveIn === BigInt(0) || reserveOut === BigInt(0)) return 100;
+ const spotPrice = Number(reserveOut) / Number(reserveIn);
+ const executionPrice = Number(amountOut) / Number(amountIn);
+ return Math.max(0, ((spotPrice - executionPrice) / spotPrice) * 100);
</file context>
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
1 participant
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
swap()calls work unchanged, newstrategy: "best-route"opt-in for routed swapsgetSwapRoute()and LangChain tool support viarouted_swapactionHow it works
Architecture: Adapter pattern with graph-based pathfinding. Each DEX implements a
DexAdapterinterface. APoolRegistrycaches pool discovery with live reserve refresh at swap time. ASwapRouterbuilds a token graph and runs BFS to find the optimal multi-hop route (up to 3 hops).New files
router/types.tsrouter/config.tsrouter/graph.tsrouter/registry.tsrouter/router.tsrouter/adapters/soroswap.tsrouter/adapters/phoenix.tsrouter/adapters/sdex.tsTest plan
Summary by cubic
Adds a multi-DEX swap router that finds the best multi-hop route across Soroswap, Phoenix, and Stellar SDEX, with mainnet/testnet support and a backward-compatible
swap()API. Opt-in routed swaps usestrategy: "best-route"and include route preview.New Features
SoroswapAdapter,PhoenixAdapter, andSdexAdapter.PoolRegistryand live reserve refresh.strategy: "best-route"; existingswap()stays unchanged.getSwapRoute()androuted_swapsupport in the contract tool.SwapRouter,PoolRegistry,TokenGraph,SoroswapAdapter,PhoenixAdapter,SdexAdapter,getNetworkConfig,MAINNET_CONFIG,TESTNET_CONFIG.Migration
agent.swap({ tokenIn, tokenOut, amount, slippage, strategy: "best-route", maxHops }). Useagent.getSwapRoute(...)to preview.Written for commit 173cb53. Summary will update on new commits.