Skip to content

feat(defi): add Soroswap and SDEX quote aggregation with split-route support#204

Merged
KevinMB0220 merged 3 commits intoGalaxy-KJ:mainfrom
salazarsebas:feat/issue-154-dex-aggregator
Mar 30, 2026
Merged

feat(defi): add Soroswap and SDEX quote aggregation with split-route support#204
KevinMB0220 merged 3 commits intoGalaxy-KJ:mainfrom
salazarsebas:feat/issue-154-dex-aggregator

Conversation

@salazarsebas
Copy link
Copy Markdown
Contributor

@salazarsebas salazarsebas commented Mar 30, 2026

Summary

This pull request introduces cross-venue quote aggregation for Stellar DeFi swaps by adding a new DexAggregatorService to @galaxy-kj/core-defi-protocols and exposing it through the REST API.

The implementation compares quotes from Soroswap and the Stellar DEX (SDEX), evaluates split execution across both venues, and returns the route with the best expected output. It also improves Soroswap quote quality by calculating price impact from pool reserves instead of returning a placeholder value.

Closes #154

What Changed

Core aggregator service

  • added DexAggregatorService under packages/core/defi-protocols/src/aggregator/
  • added aggregator-specific route and quote types
  • implemented getBestQuote() to compare Soroswap and SDEX quotes and evaluate default split candidates
  • implemented getSplitQuote() to support explicit venue allocations such as 60/40

Soroswap quote quality

  • updated SoroswapProtocol.getSwapQuote() to calculate price impact using the constant-product reserve formula
  • preserved the existing quote contract while improving quote accuracy

REST API

  • added GET /api/v1/defi/aggregator/quote
  • supports:
    • best-route discovery via assetIn, assetOut, and amountIn
    • explicit split evaluation via splits=60,40
  • added validation for malformed split parameters

Tests and documentation

  • added dedicated unit coverage for the new aggregator service
  • expanded Soroswap protocol tests to cover the updated quote behavior
  • added REST route coverage for the new endpoint
  • updated DeFi protocol documentation and aggregator usage documentation

Design Notes

  • amounts are represented as decimal strings rather than bigint to remain consistent with the existing core-defi-protocols interfaces and REST serialization model
  • the aggregator was implemented in core-defi-protocols to avoid duplicating routing logic in multiple packages
  • SDEX failures are tolerated during best-route discovery when another venue still returns a valid quote

Validation

Validated locally for the package that is currently exercised by the repository CI workflow:

  • npm run type-check in packages/core/defi-protocols
  • npm run lint in packages/core/defi-protocols
  • npm run build in packages/core/defi-protocols
  • focused unit tests for:
    • aggregator service
    • updated Soroswap quote behavior

I also reviewed the active GitHub Actions workflows to confirm alignment with the current CI expectations.

Known Limitations

  • local full coverage runs in this environment still encounter pre-existing live Blend integration test failures because external network access to friendbot.stellar.org is unavailable here
  • the packages/api/rest workspace also has pre-existing local dependency/configuration issues unrelated to this feature; the active repository CI workflows currently do not run that package directly

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced DexAggregatorService for comparing quotes across multiple liquidity venues and selecting best execution routes
    • Added REST API endpoint for retrieving aggregated quotes with optional explicit split allocation across venues
    • Implemented split-quote evaluation with customizable weight distribution
  • Improvements

    • Enhanced price impact calculation using constant-product formula for more accurate execution estimates
  • Documentation

    • Updated DeFi aggregator guide with new quote flow, API endpoints, and split allocation specifications

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 30, 2026

📝 Walkthrough

Walkthrough

Implements a DEX aggregator service that compares swap routes across Soroswap and SDEX, optionally splitting trades to optimize execution price. Adds DexAggregatorService with getBestQuote and getSplitQuote methods, exposes a REST endpoint for quote retrieval, enhances Soroswap price-impact calculation using constant-product math, and includes comprehensive test coverage.

Changes

Cohort / File(s) Summary
Aggregator Service Core
packages/core/defi-protocols/src/aggregator/DexAggregatorService.ts, packages/core/defi-protocols/src/aggregator/types.ts
New service with getBestQuote and getSplitQuote methods that fetch routes from Soroswap and SDEX venues, compare outputs, and compute aggregate pricing metrics. Defines AggregatorVenue, AggregatorRoute, and AggregatorQuote types.
REST API Integration
packages/api/rest/src/routes/defi.routes.ts, packages/api/rest/src/routes/defi.routes.test.ts
Added unauthenticated /api/v1/defi/aggregator/quote endpoint with query parameter validation for assetIn, assetOut, amountIn, and optional splits. Includes parseSplits helper for weight normalization and error handling for malformed input.
Test Coverage
packages/core/defi-protocols/__tests__/aggregator/DexAggregator.test.ts, packages/core/defi-protocols/__tests__/protocols/soroswap-protocol.test.ts
New comprehensive aggregator test suite covering best-quote selection, split execution, venue failure scenarios, and edge cases. Extended Soroswap tests with source-account fallback and factory-initialization validation.
Soroswap Protocol Enhancement
packages/core/defi-protocols/src/protocols/soroswap/soroswap-protocol.ts
Updated getSwapQuote to compute priceImpact via constant-product formula using getLiquidityPool, replacing hardcoded '0' with dynamic percentage-based calculation.
Module Exports & Documentation
packages/core/defi-protocols/src/index.ts, packages/core/defi-protocols/README.md, docs/defi/aggregator-guide.md
Added public exports for DexAggregatorService and aggregator types. Updated main README and aggregator guide with usage examples, error handling, and API endpoint documentation.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant REST as REST API
    participant Agg as DexAggregatorService
    participant Soroswap as SoroswapProtocol
    participant SDEX as Stellar DEX<br/>(Horizon)

    Client->>REST: GET /api/v1/defi/aggregator/quote<br/>(assetIn, assetOut, amountIn)
    activate REST
    REST->>REST: Validate query params
    REST->>Agg: new DexAggregatorService(config)
    activate Agg
    Agg->>Agg: Validate assets & amounts
    Agg->>Agg: Parse split weights (if provided)
    Agg->>Soroswap: getSwapQuote(assetIn, assetOut, amountIn)
    activate Soroswap
    Soroswap->>Soroswap: Query liquidity pools
    Soroswap->>Soroswap: Calculate amountOut & priceImpact
    Soroswap-->>Agg: Return quote
    deactivate Soroswap
    Agg->>SDEX: strictSendPaths(assetIn, amountIn, [assetOut])
    activate SDEX
    SDEX-->>Agg: Return path records
    deactivate SDEX
    Agg->>Agg: Compare venues & evaluate splits
    Agg->>Agg: Build AggregatorQuote with<br/>totalAmountOut & savings
    Agg-->>REST: Return aggregated quote
    deactivate Agg
    REST-->>Client: 200 JSON (routes, totalAmountOut, etc.)
    deactivate REST
Loading
sequenceDiagram
    participant User
    participant Agg as DexAggregatorService

    User->>Agg: getBestQuote(assetIn, assetOut, amountIn)
    activate Agg
    Agg->>Agg: Fetch best single-venue route
    Agg->>Agg: Evaluate predefined splits<br/>(e.g. 60/40, 40/60)
    Agg->>Agg: Compare split vs single-venue output
    alt Split wins
        Agg->>Agg: Return 2-leg split with savings
    else Single venue wins
        Agg->>Agg: Return single best route
    end
    Agg-->>User: AggregatorQuote
    deactivate Agg

    User->>Agg: getSplitQuote(assetIn, assetOut,<br/>amountIn, splits: [60, 40])
    activate Agg
    Agg->>Agg: Validate & normalize splits to 100%
    Agg->>Agg: Allocate amounts per venue
    Agg->>Agg: Fetch routes concurrently
    Agg->>Agg: Combine routes & compute metrics
    Agg-->>User: AggregatorQuote with 2 routes
    deactivate Agg
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Suggested reviewers

  • KevinMB0220

Poem

🐰 A rabbit hops through two DEX pools,
Comparing swaps with clever rules—
Soroswap and SDEX aligned,
Split routes merge to find best-priced! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main feature: adding quote aggregation across Soroswap and SDEX with split-route support.
Description check ✅ Passed The PR description comprehensively covers the changes, design decisions, validation performed, and known limitations, meeting the template requirements.
Linked Issues check ✅ Passed The implementation fully satisfies issue #154: DexAggregatorService with getBestQuote/getSplitQuote, split-route evaluation, REST endpoint, price-impact calculation, tests, and documentation.
Out of Scope Changes check ✅ Passed All code changes directly support the aggregator feature and issue #154 objectives; Soroswap price-impact calculation improvements are in-scope enhancements to quote accuracy.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (2)
packages/core/defi-protocols/src/aggregator/DexAggregatorService.ts (2)

149-170: Consider caching or reusing protocol instances.

A new SoroswapProtocol instance is created and initialized for every quote request. For high-frequency usage, this overhead could be significant. Consider caching the initialized protocol instance or making initialization idempotent.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/defi-protocols/src/aggregator/DexAggregatorService.ts` around
lines 149 - 170, fetchSoroswapRoute currently calls
this.protocolFactory.createProtocol(this.soroswapConfig) and await
protocol.initialize() on every request which is expensive; change this to reuse
a cached initialized instance (e.g., store it on the DexAggregatorService as a
private soroswapProtocol field) or ensure protocol.initialize() is idempotent by
checking an initialized flag on the protocol before calling initialize() again;
update fetchSoroswapRoute to get the protocol from the cache (or call a helper
getOrInitSoroswapProtocol) instead of creating/initializing a new instance each
time.

109-112: Minor inefficiency: getSplitQuote re-fetches single-venue routes.

When getSplitQuote is called from getBestQuote, the single-venue routes have already been fetched at line 53. Passing the pre-fetched bestSingleAmountOut as a parameter could avoid this redundant network call.

♻️ Potential optimization
  async getSplitQuote(
    assetIn: Asset,
    assetOut: Asset,
    amountIn: string,
-   splits: number[]
+   splits: number[],
+   precomputedBestSingleOut?: string
  ): Promise<AggregatorQuote> {
    // ...
-   const bestSingleRoutes = await this.fetchSingleVenueRoutes(assetIn, assetOut, amountIn);
-   const bestSingleRoute = this.getBestRoute(bestSingleRoutes);
-   return this.buildQuote(assetIn, assetOut, amountIn, routes, bestSingleRoute.amountOut);
+   const bestSingleOut = precomputedBestSingleOut ?? (
+     await this.fetchSingleVenueRoutes(assetIn, assetOut, amountIn)
+   ).reduce((best, r) => this.compareAmount(r.amountOut, best) > 0 ? r.amountOut : best, '0');
+   return this.buildQuote(assetIn, assetOut, amountIn, routes, bestSingleOut);
  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/defi-protocols/src/aggregator/DexAggregatorService.ts` around
lines 109 - 112, getSplitQuote currently re-fetches single-venue routes via
fetchSingleVenueRoutes/getBestRoute causing redundant network calls; change
getSplitQuote to accept an optional bestSingleAmountOut parameter (or
bestSingleRouteAmount) and, if provided, skip calling
fetchSingleVenueRoutes/getBestRoute inside getSplitQuote, otherwise keep the
existing behavior; update the caller getBestQuote to pass the pre-fetched
bestSingleRoute.amountOut to getSplitQuote when available and remove the
duplicate fetch in that path so buildQuote still receives the same
bestSingleAmountOut value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/api/rest/src/routes/defi.routes.ts`:
- Around line 125-134: The handler currently only checks presence of assetIn,
assetOut, and amountIn; validate amountIn's format and value before calling the
aggregator by parsing it (e.g., Number/parseFloat or BigNumber), ensuring it's a
finite number greater than zero and rejecting non-numeric or non-positive values
with a 400 VALIDATION_ERROR and clear message; update the validation blocks
around the existing checks (variables assetIn, assetOut, amountIn in the route
handler) and apply the same numeric validation to the other occurrence mentioned
(the later check at ~140-142) so invalid amountIn inputs like "abc" or negative
numbers return a 400 instead of propagating to protocol calls.
- Around line 56-73: parseSplits currently treats an empty query param
(?splits=) as "not provided", accepts [0,0], and relies on brittle
message.includes('splits') elsewhere to classify the error; fix by (1) changing
the presence check to value === undefined so empty-string inputs are parsed as
provided, (2) validate that the parsed array contains exactly two finite
non-negative numbers and reject the all-zero case (e.g., ensure !(splits[0] ===
0 && splits[1] === 0)), (3) throw a specific ValidationError (create class
ValidationError extends Error or attach a stable code/name like
'SPLITS_VALIDATION_ERROR') with a clear constant message from parseSplits, and
(4) replace any downstream message.includes('splits') checks with a reliable
instance/type or code check against ValidationError/SPLITS_VALIDATION_ERROR;
reference parseSplits and the error-handling logic that currently inspects
message.includes('splits').

In `@packages/core/defi-protocols/src/aggregator/DexAggregatorService.ts`:
- Around line 300-313: The toHorizonAssetParams function always sets non-native
assets to 'credit_alphanum4', which breaks assets with 5-12 character codes;
update toHorizonAssetParams to inspect the asset code length via asset.getCode()
and set [`${prefix}_asset_type`] to 'credit_alphanum4' when length <= 4 and to
'credit_alphanum12' when length is between 5 and 12, preserving the existing
[`${prefix}_asset_code`] and [`${prefix}_asset_issuer`] entries and the native
branch that returns 'native'.

In `@packages/core/defi-protocols/src/protocols/soroswap/soroswap-protocol.ts`:
- Around line 543-557: getSwapQuote currently calls getLiquidityPool which
triggers two extra network simulations (get_pair and get_reserves) per quote; to
avoid doubling latency, modify SoroswapProtocol.getSwapQuote to accept an
optional reserves parameter (or a pre-fetched liquidityPool) and use that if
provided, and implement a short-TTL in-memory cache inside SoroswapProtocol
keyed by pair (or tokenIn+tokenOut) that stores liquidityPool.reserveA/reserveB
and expires after e.g. N seconds; update references to getLiquidityPool,
get_pair and get_reserves so callers can either supply reserves or rely on the
cache-before-falling-back-to-getLiquidityPool behavior, ensuring existing code
paths still call getLiquidityPool when no cached/passed reserves exist.

---

Nitpick comments:
In `@packages/core/defi-protocols/src/aggregator/DexAggregatorService.ts`:
- Around line 149-170: fetchSoroswapRoute currently calls
this.protocolFactory.createProtocol(this.soroswapConfig) and await
protocol.initialize() on every request which is expensive; change this to reuse
a cached initialized instance (e.g., store it on the DexAggregatorService as a
private soroswapProtocol field) or ensure protocol.initialize() is idempotent by
checking an initialized flag on the protocol before calling initialize() again;
update fetchSoroswapRoute to get the protocol from the cache (or call a helper
getOrInitSoroswapProtocol) instead of creating/initializing a new instance each
time.
- Around line 109-112: getSplitQuote currently re-fetches single-venue routes
via fetchSingleVenueRoutes/getBestRoute causing redundant network calls; change
getSplitQuote to accept an optional bestSingleAmountOut parameter (or
bestSingleRouteAmount) and, if provided, skip calling
fetchSingleVenueRoutes/getBestRoute inside getSplitQuote, otherwise keep the
existing behavior; update the caller getBestQuote to pass the pre-fetched
bestSingleRoute.amountOut to getSplitQuote when available and remove the
duplicate fetch in that path so buildQuote still receives the same
bestSingleAmountOut value.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d180d738-0fd3-4aa3-8541-556482d24b09

📥 Commits

Reviewing files that changed from the base of the PR and between f45c7a6 and 78f0626.

📒 Files selected for processing (10)
  • docs/defi/aggregator-guide.md
  • packages/api/rest/src/routes/defi.routes.test.ts
  • packages/api/rest/src/routes/defi.routes.ts
  • packages/core/defi-protocols/README.md
  • packages/core/defi-protocols/__tests__/aggregator/DexAggregator.test.ts
  • packages/core/defi-protocols/__tests__/protocols/soroswap-protocol.test.ts
  • packages/core/defi-protocols/src/aggregator/DexAggregatorService.ts
  • packages/core/defi-protocols/src/aggregator/types.ts
  • packages/core/defi-protocols/src/index.ts
  • packages/core/defi-protocols/src/protocols/soroswap/soroswap-protocol.ts

Comment on lines +56 to +73
function parseSplits(value: Request['query']['splits']): number[] | null {
if (!value) {
return null;
}

const raw = Array.isArray(value) ? value.join(',') : String(value);
const splits = raw
.split(',')
.map((item) => item.trim())
.filter(Boolean)
.map((item) => Number(item));

if (splits.length !== 2 || splits.some((split) => !Number.isFinite(split) || split < 0)) {
throw new Error('splits must contain exactly two non-negative numeric weights');
}

return splits;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

splits validation can be bypassed and error classification is brittle

At Line 57, if (!value) treats ?splits= as “not provided”, so malformed input can silently fall back to best-route mode. At Line 68, [0,0] is also accepted (invalid zero-allocation). At Line 146, message.includes('splits') is too fragile for classifying validation failures.

Proposed fix
+class SplitsValidationError extends Error {}
+
 function parseSplits(value: Request['query']['splits']): number[] | null {
-    if (!value) {
+    if (value === undefined) {
         return null;
     }

     const raw = Array.isArray(value) ? value.join(',') : String(value);
+    if (!raw.trim()) {
+        throw new SplitsValidationError('splits must contain exactly two non-negative numeric weights');
+    }
+
     const splits = raw
         .split(',')
         .map((item) => item.trim())
         .filter(Boolean)
         .map((item) => Number(item));

-    if (splits.length !== 2 || splits.some((split) => !Number.isFinite(split) || split < 0)) {
-        throw new Error('splits must contain exactly two non-negative numeric weights');
+    const totalWeight = splits.reduce((sum, split) => sum + split, 0);
+    if (
+        splits.length !== 2 ||
+        splits.some((split) => !Number.isFinite(split) || split < 0) ||
+        totalWeight <= 0
+    ) {
+        throw new SplitsValidationError('splits must contain exactly two non-negative numeric weights');
     }

     return splits;
 }
...
-            if (error instanceof Error && error.message.includes('splits')) {
+            if (error instanceof SplitsValidationError) {
                 res.status(400).json({

Also applies to: 146-147

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api/rest/src/routes/defi.routes.ts` around lines 56 - 73,
parseSplits currently treats an empty query param (?splits=) as "not provided",
accepts [0,0], and relies on brittle message.includes('splits') elsewhere to
classify the error; fix by (1) changing the presence check to value ===
undefined so empty-string inputs are parsed as provided, (2) validate that the
parsed array contains exactly two finite non-negative numbers and reject the
all-zero case (e.g., ensure !(splits[0] === 0 && splits[1] === 0)), (3) throw a
specific ValidationError (create class ValidationError extends Error or attach a
stable code/name like 'SPLITS_VALIDATION_ERROR') with a clear constant message
from parseSplits, and (4) replace any downstream message.includes('splits')
checks with a reliable instance/type or code check against
ValidationError/SPLITS_VALIDATION_ERROR; reference parseSplits and the
error-handling logic that currently inspects message.includes('splits').

Comment on lines +125 to +134
if (!assetIn || !assetOut || !amountIn) {
res.status(400).json({
error: {
code: 'VALIDATION_ERROR',
message: 'assetIn, assetOut, and amountIn are required query parameters',
details: {},
},
});
return;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Validate amountIn format/value before calling the aggregator

At Line 125, only presence is checked. Invalid inputs like amountIn=abc or negative values can propagate to protocol calls and return avoidable 5xx errors instead of a clean 400.

Proposed fix
             if (!assetIn || !assetOut || !amountIn) {
                 res.status(400).json({
                     error: {
                         code: 'VALIDATION_ERROR',
                         message: 'assetIn, assetOut, and amountIn are required query parameters',
                         details: {},
                     },
                 });
                 return;
             }
+
+            const amountInStr = String(amountIn).trim();
+            if (!/^\d+(\.\d+)?$/.test(amountInStr) || Number(amountInStr) <= 0) {
+                res.status(400).json({
+                    error: {
+                        code: 'VALIDATION_ERROR',
+                        message: 'amountIn must be a positive numeric string',
+                        details: {},
+                    },
+                });
+                return;
+            }

             const tokenIn = parseAsset(assetIn as string);
             const tokenOut = parseAsset(assetOut as string);
             const aggregator = new DexAggregatorService({ ...defaultConfig, protocolId: 'soroswap' });
             const parsedSplits = parseSplits(splits);
             const quote = parsedSplits
-                ? await aggregator.getSplitQuote(tokenIn, tokenOut, amountIn as string, parsedSplits)
-                : await aggregator.getBestQuote(tokenIn, tokenOut, amountIn as string);
+                ? await aggregator.getSplitQuote(tokenIn, tokenOut, amountInStr, parsedSplits)
+                : await aggregator.getBestQuote(tokenIn, tokenOut, amountInStr);

Also applies to: 140-142

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api/rest/src/routes/defi.routes.ts` around lines 125 - 134, The
handler currently only checks presence of assetIn, assetOut, and amountIn;
validate amountIn's format and value before calling the aggregator by parsing it
(e.g., Number/parseFloat or BigNumber), ensuring it's a finite number greater
than zero and rejecting non-numeric or non-positive values with a 400
VALIDATION_ERROR and clear message; update the validation blocks around the
existing checks (variables assetIn, assetOut, amountIn in the route handler) and
apply the same numeric validation to the other occurrence mentioned (the later
check at ~140-142) so invalid amountIn inputs like "abc" or negative numbers
return a 400 instead of propagating to protocol calls.

Comment on lines +300 to +313
private toHorizonAssetParams(
asset: StellarAsset,
prefix: 'source' | 'destination'
): Record<string, string> {
if (asset.isNative()) {
return { [`${prefix}_asset_type`]: 'native' };
}

return {
[`${prefix}_asset_type`]: 'credit_alphanum4',
[`${prefix}_asset_code`]: asset.getCode(),
[`${prefix}_asset_issuer`]: asset.getIssuer(),
};
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Hardcoded credit_alphanum4 may fail for 5-12 character asset codes.

The toHorizonAssetParams method always returns credit_alphanum4 for non-native assets. Stellar supports credit_alphanum12 for asset codes with 5-12 characters. This could cause SDEX path queries to fail or return incorrect results for such assets.

🐛 Proposed fix
  private toHorizonAssetParams(
    asset: StellarAsset,
    prefix: 'source' | 'destination'
  ): Record<string, string> {
    if (asset.isNative()) {
      return { [`${prefix}_asset_type`]: 'native' };
    }

+   const assetType = asset.getCode().length <= 4 ? 'credit_alphanum4' : 'credit_alphanum12';
    return {
-     [`${prefix}_asset_type`]: 'credit_alphanum4',
+     [`${prefix}_asset_type`]: assetType,
      [`${prefix}_asset_code`]: asset.getCode(),
      [`${prefix}_asset_issuer`]: asset.getIssuer(),
    };
  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/defi-protocols/src/aggregator/DexAggregatorService.ts` around
lines 300 - 313, The toHorizonAssetParams function always sets non-native assets
to 'credit_alphanum4', which breaks assets with 5-12 character codes; update
toHorizonAssetParams to inspect the asset code length via asset.getCode() and
set [`${prefix}_asset_type`] to 'credit_alphanum4' when length <= 4 and to
'credit_alphanum12' when length is between 5 and 12, preserving the existing
[`${prefix}_asset_code`] and [`${prefix}_asset_issuer`] entries and the native
branch that returns 'native'.

Comment on lines +543 to +557

const amountOutRaw = amounts[1];
const amountOut = (Number(amountOutRaw) / 1e7).toString();
const minimumReceived = (parseFloat(amountOut) * (1 - SoroswapProtocol.SLIPPAGE_TOLERANCE)).toFixed(7);
const amountOut = new BigNumber(amountOutRaw.toString()).dividedBy(1e7).toFixed(7);
const minimumReceived = new BigNumber(amountOut)
.multipliedBy(1 - SoroswapProtocol.SLIPPAGE_TOLERANCE)
.toFixed(7);
const liquidityPool = await this.getLiquidityPool(tokenIn, tokenOut);
const reserveIn = new BigNumber(liquidityPool.reserveA).dividedBy(1e7);
const priceImpact = reserveIn.isZero()
? '0'
: new BigNumber(amountIn)
.dividedBy(reserveIn.plus(amountIn))
.multipliedBy(100)
.decimalPlaces(4)
.toFixed();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify getLiquidityPool returns reserveA in stroops (raw integer string)
rg -n -A 10 'reserveA =' packages/core/defi-protocols/src/protocols/soroswap/soroswap-protocol.ts

Repository: Galaxy-KJ/Galaxy-DevKit

Length of output: 838


🏁 Script executed:

#!/bin/bash
# Get the full getLiquidityPool function to see return type and units
rg -n -B 5 -A 40 'getLiquidityPool' packages/core/defi-protocols/src/protocols/soroswap/soroswap-protocol.ts | head -100

Repository: Galaxy-KJ/Galaxy-DevKit

Length of output: 3993


🏁 Script executed:

#!/bin/bash
# Get the getSwapQuote function around lines 543-557 to see context of amountIn
sed -n '500,580p' packages/core/defi-protocols/src/protocols/soroswap/soroswap-protocol.ts

Repository: Galaxy-KJ/Galaxy-DevKit

Length of output: 2964


🏁 Script executed:

#!/bin/bash
# Check how amountIn is calculated/prepared before line 543
sed -n '520,545p' packages/core/defi-protocols/src/protocols/soroswap/soroswap-protocol.ts

Repository: Galaxy-KJ/Galaxy-DevKit

Length of output: 1067


🏁 Script executed:

#!/bin/bash
# Find the getSwapQuote function signature
rg -n -B 5 'getSwapQuote' packages/core/defi-protocols/src/protocols/soroswap/soroswap-protocol.ts | grep -A 15 'public async getSwapQuote'

Repository: Galaxy-KJ/Galaxy-DevKit

Length of output: 348


🏁 Script executed:

#!/bin/bash
# Find the full getLiquidityPool implementation to confirm it's an async network call
sed -n '776,850p' packages/core/defi-protocols/src/protocols/soroswap/soroswap-protocol.ts

Repository: Galaxy-KJ/Galaxy-DevKit

Length of output: 2579


🏁 Script executed:

#!/bin/bash
# Check if there's any caching mechanism in the class
rg -n 'cache\|Cache' packages/core/defi-protocols/src/protocols/soroswap/soroswap-protocol.ts

Repository: Galaxy-KJ/Galaxy-DevKit

Length of output: 49


🏁 Script executed:

#!/bin/bash
# Get the getSwapQuote full signature and documentation
sed -n '470,510p' packages/core/defi-protocols/src/protocols/soroswap/soroswap-protocol.ts

Repository: Galaxy-KJ/Galaxy-DevKit

Length of output: 1248


🏁 Script executed:

#!/bin/bash
# Check the amountToI128ScVal function to understand how amountIn is converted
rg -n -A 5 'amountToI128ScVal' packages/core/defi-protocols/src/protocols/soroswap/soroswap-protocol.ts

Repository: Galaxy-KJ/Galaxy-DevKit

Length of output: 1641


🏁 Script executed:

#!/bin/bash
# Double-check the reserveA return value by looking at the complete getLiquidityPool return statement
sed -n '850,880p' packages/core/defi-protocols/src/protocols/soroswap/soroswap-protocol.ts

Repository: Galaxy-KJ/Galaxy-DevKit

Length of output: 1034


Performance: getLiquidityPool adds extra network round-trips per quote.

Each getSwapQuote call incurs two additional Soroban simulations via getLiquidityPool: one for get_pair (line ~828) and another for get_reserves (line ~854). For high-frequency quote requests, this doubles latency. Consider caching pool reserves with a short TTL or accepting reserves as an optional parameter when the caller already has them.

Unit consistency in priceImpact calculation is correct.

amountIn is documented as a decimal string (e.g., "100"), and reserveA from getLiquidityPool is returned as stroops (raw bigint), then divided by 1e7 to convert to decimal units (line 550). Both values are in the same decimal unit space when used in the priceImpact calculation on line 553–557, so the logic is valid.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/defi-protocols/src/protocols/soroswap/soroswap-protocol.ts`
around lines 543 - 557, getSwapQuote currently calls getLiquidityPool which
triggers two extra network simulations (get_pair and get_reserves) per quote; to
avoid doubling latency, modify SoroswapProtocol.getSwapQuote to accept an
optional reserves parameter (or a pre-fetched liquidityPool) and use that if
provided, and implement a short-TTL in-memory cache inside SoroswapProtocol
keyed by pair (or tokenIn+tokenOut) that stores liquidityPool.reserveA/reserveB
and expires after e.g. N seconds; update references to getLiquidityPool,
get_pair and get_reserves so callers can either supply reserves or rely on the
cache-before-falling-back-to-getLiquidityPool behavior, ensuring existing code
paths still call getLiquidityPool when no cached/passed reserves exist.

@KevinMB0220 KevinMB0220 self-requested a review March 30, 2026 05:51
@KevinMB0220 KevinMB0220 merged commit 2104036 into Galaxy-KJ:main Mar 30, 2026
7 checks passed
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.

[FEATURE] Build DEX aggregator service with Soroswap + SDEX routing

2 participants