feat: implement multi-asset payment gateway (#103)#128
Conversation
- Implemented MultiAssetPaymentService with methods for executing path payments, estimating payments, and retrieving supported asset pairs. - Added error handling for unsupported assets and invalid payment amounts. - Integrated AssetConfigurationService and AssetPriceService for asset validation and pricing. - Created unit tests for MultiAssetPaymentService covering various scenarios including successful payments, fallback mechanisms, and estimation of payments. test: add unit tests for AssetConfigurationService - Developed comprehensive tests for AssetConfigurationService methods including create, update, delete, and retrieval of asset configurations. - Ensured proper error handling and validation logic is tested. test: add unit tests for AssetPriceService - Implemented tests for AssetPriceService methods to validate exchange rate retrieval, path finding, and orderbook fetching. - Included tests for handling native assets and error scenarios. test: add unit tests for MultiAssetPaymentService - Created unit tests for MultiAssetPaymentService to validate path payment execution, fallback logic, and payment estimation. - Ensured coverage for various edge cases and error handling.
- Created AssetConfigurationService for merchant asset preferences - Implemented AssetPriceService for real-time Stellar DEX rates - Built MultiAssetPaymentService with path payment operations - Added fallback logic for failed payment paths - Created 8 API endpoints for configuration and payments - Implemented support for XLM, USDC, and custom tokens - Added 42 tests covering all services (all passing) - Created database migration for asset_configurations table - Added complete API documentation Closes PayStell#103
📝 WalkthroughWalkthroughImplements a multi-asset payment gateway that allows merchants to accept various Stellar-based assets. Includes asset configuration management, Stellar DEX price discovery, path payment execution with fallback logic, database schema, comprehensive services, API endpoints, and test coverage. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as MultiAssetPaymentController
participant PaymentService as MultiAssetPaymentService
participant AssetConfigService as AssetConfigurationService
participant PriceService as AssetPriceService
participant Stellar as Stellar Horizon
participant DB as Database
Client->>Controller: executePathPayment(merchantId, sourceAsset, destAsset, amount)
Controller->>PaymentService: executePathPayment(...)
PaymentService->>AssetConfigService: isAssetSupported(merchantId, sourceAsset)
AssetConfigService->>DB: findOne(merchantId, assetCode, isEnabled=true)
DB-->>AssetConfigService: config
AssetConfigService-->>PaymentService: boolean
PaymentService->>AssetConfigService: validatePaymentAmount(merchantId, sourceAsset, amount)
AssetConfigService->>DB: findOne(merchantId, assetCode)
DB-->>AssetConfigService: config with min/max
AssetConfigService-->>PaymentService: boolean
PaymentService->>PriceService: findBestPath(sourceAsset, destAsset, amount)
PriceService->>Stellar: strictSendPaths(...)
Stellar-->>PriceService: path with intermediaries
PriceService-->>PaymentService: path
PaymentService->>Stellar: submit path payment transaction (sign + submit)
Stellar-->>PaymentService: transaction result
PaymentService-->>Controller: PathPaymentResult
Controller-->>Client: 200 {success: true, data: {transactionHash, ...}}
sequenceDiagram
participant Client
participant Controller as MultiAssetPaymentController
participant PaymentService as MultiAssetPaymentService
participant AssetConfigService as AssetConfigurationService
participant PriceService as AssetPriceService
participant Stellar as Stellar Horizon
Client->>Controller: executeWithFallback(merchantId, sourceAsset, destAsset)
Controller->>PaymentService: executeWithFallback(...)
Note over PaymentService: Primary Path Attempt
PaymentService->>PriceService: findBestPath(sourceAsset, destAsset)
PriceService->>Stellar: strictSendPaths(...)
Stellar-->>PriceService: error: no path
PriceService-->>PaymentService: null
Note over PaymentService: Fallback to Alternative Assets
PaymentService->>AssetConfigService: getEnabledAssetConfigs(merchantId)
AssetConfigService-->>PaymentService: [config1, config2, config3]
loop Each Alternative Asset
PaymentService->>PriceService: findBestPath(altAsset, destAsset)
alt Path Found
PriceService-->>PaymentService: path
PaymentService->>Stellar: submit path payment
Stellar-->>PaymentService: success
break Success
end
else No Path
PriceService-->>PaymentService: null
end
end
PaymentService-->>Controller: PathPaymentResult
Controller-->>Client: 200 {success: true, data: {...}}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@src/controllers/MultiAssetPaymentController.ts`:
- Around line 66-91: The updateAssetConfig handler is reading merchantId from
req.body which allows spoofing; change it to use the authenticated merchant ID
provided by the authenticateMerchant middleware (req.merchant.id) when calling
this.assetConfigService.updateAssetConfig (and likewise for createAssetConfig
and deleteAssetConfig) so the controller always passes the verified merchant id
instead of any value from req.body; locate references to merchantId in
updateAssetConfig, createAssetConfig, and deleteAssetConfig and replace them
with the authenticated req.merchant.id before invoking the service.
In `@src/routes/multiAssetPayment.routes.ts`:
- Around line 70-73: The route currently accepts a sensitive Stellar secret via
the body field "sourceSecret"; remove that validator and any code that consumes
secrets, replace with accepting a public key (e.g., "sourcePublicKey") and
validate it instead, and change the handler to build the unsigned transaction
XDR (use your existing transaction building logic—e.g., where you previously
used the secret to set the source account) and return the XDR to the client for
local signing; ensure you also add validation for the public key format and
remove any logging of secret material in the handler that referenced
"sourceSecret".
In `@src/services/AssetConfigurationService.ts`:
- Around line 205-210: The parsed numeric values amountNum and minAmount (from
parseFloat(amount) and parseFloat(config.minAmount)) must be validated for NaN
before comparing; update the code that creates amountNum and minAmount to check
isNaN(amountNum) || isNaN(minAmount) and handle this case (e.g., return false or
throw a validation error consistent with the surrounding method in
AssetConfigurationService), then proceed with the existing comparison if both
are valid numbers.
In `@src/services/AssetPriceService.ts`:
- Around line 47-52: The createAsset method currently treats any non-XLM asset
missing an issuer as native; update createAsset so that it only returns
Asset.native() when code === "XLM" and otherwise throws an error if issuer is
missing (e.g., throw new Error("Issuer cannot be null for non-native asset") or
mirror the Stellar SDK message) instead of returning Asset.native(); locate the
createAsset function and replace the fallback path that returns Asset.native()
with an explicit error throw before creating a new Asset(code, issuer).
In `@src/services/MultiAssetPaymentService.ts`:
- Around line 200-218: The fallback loop in MultiAssetPaymentService uses the
original data.amount for every alternative asset (in the for-of over configs and
the call to this.executePathPayment), which produces incorrect value semantics
and can fail if the payer lacks that asset balance; update the logic in the loop
that iterates over configs to (1) compute an equivalent amount for each fallback
asset before calling executePathPayment (e.g., by calling or adding a conversion
helper that uses market rates/rateProvider), (2) check the source account
balance for the candidate asset (use the account/balance lookup function in this
service or the ledger client) and skip if insufficient, and (3) pass the
converted amount and sourceAssetIssuer via the same call to executePathPayment;
ensure you touch the code around executePathPayment, configs, data.amount,
sourceAssetCode, and sourceAssetIssuer so the fallback uses the converted amount
and performs a balance check.
- Around line 257-262: The returned object in MultiAssetPaymentService assigns
fees: BASE_FEE (a number) but the PaymentEstimate interface expects a string;
fix by converting the SDK constant to a string where the object is constructed
(replace fees: BASE_FEE with fees: BASE_FEE.toString() or String(BASE_FEE)) or
alternatively update the PaymentEstimate.fees type to number if you intend fees
to be numeric throughout — apply the chosen change near the method that returns
estimatedDestAmount/rate/path/fees so types align.
- Around line 139-153: The code sets destMin directly to
bestPath.destination_amount which is brittle; update the txBuilder.addOperation
call (Operation.pathPaymentStrictSend) to apply a small slippage tolerance
(e.g., 0.5%-1%) to bestPath.destination_amount before assigning destMin: parse
bestPath.destination_amount to a numeric type, multiply by (1 - tolerance)
(e.g., 0.995 for 0.5%), then convert back to the string representation expected
by pathPaymentStrictSend and use that value for destMin; ensure this
transformation is applied where
txBuilder.addOperation/Operation.pathPaymentStrictSend is invoked and handle
potential precision/formatting issues (use a decimal library or safe rounding)
so you don't underflow to zero.
- Around line 65-70: The createAsset method currently returns Asset.native()
when issuer is missing, which silently converts non-XLM assets to XLM; change
createAsset(code: string, issuer?: string): Asset so that if code !== "XLM" and
issuer is null/undefined/empty it throws a descriptive error (e.g., "Missing
issuer for non-XLM asset") instead of returning Asset.native(), otherwise return
Asset.native() for code === "XLM" and return new Asset(code, issuer) for valid
issuer; update any callers to handle the thrown error if necessary.
🧹 Nitpick comments (16)
src/scripts/create-asset-configuration-table.sql (2)
1-16: Ensure UUID generation support forgen_random_uuid().If pgcrypto isn’t enabled in your DB, this migration will fail. Consider adding an extension guard or confirm it exists in earlier migrations.
🛠️ Suggested addition
--- Create asset_configurations table for multi-asset payment gateway +-- Ensure UUID generation support (PostgreSQL) +CREATE EXTENSION IF NOT EXISTS pgcrypto; +-- Create asset_configurations table for multi-asset payment gateway CREATE TABLE IF NOT EXISTS asset_configurations (
33-65: Make constraints/triggers idempotent if this script is re-run.The table/indexes are guarded with
IF NOT EXISTS, but constraints and the trigger aren’t—reruns will fail. Consider conditional checks or aDO $$block.src/routes/multiAssetPayment.routes.ts (3)
137-144: Consider adding rate limiting to public endpoints.The
/config/merchant/:merchantIdendpoint is public and could expose merchant configuration details. Additionally, endpoints like/exchange-rate,/estimate, and/orderbookmake external calls to Stellar Horizon. Without rate limiting, these could be abused for reconnaissance or to cause excessive API load.
121-130: DELETE with request body may cause issues.HTTP DELETE requests with a body can be problematic as some HTTP clients, proxies, and load balancers may strip the body. Consider either:
- Using the authenticated merchant from
req.merchant(already set byauthenticateMerchantmiddleware)- Moving
merchantIdto a query parameter♻️ Suggested approach using authenticated merchant
router.delete( "/config/:id", authenticateMerchant, [ param("id").isUUID().withMessage("Valid configuration ID is required"), - body("merchantId").isUUID().withMessage("Valid merchant ID is required"), handleValidationErrors, ], asyncHandler(controller.deleteAssetConfig), );Then in the controller, use
req.merchant.idinstead ofreq.body.merchantId.
51-51: Amount validation should verify numeric format.The
amountfield is validated as a non-empty string, but there's no check that it represents a valid positive decimal number. Invalid values like"abc"or"-100"would pass validation but fail at the service layer or Stellar network.♻️ Suggested validation enhancement
- body("amount").isString().notEmpty().withMessage("Amount is required"), + body("amount") + .isString() + .notEmpty() + .withMessage("Amount is required") + .matches(/^\d+(\.\d+)?$/) + .withMessage("Amount must be a valid positive decimal"),Also applies to: 69-69, 88-88
src/tests/services/MultiAssetPaymentService.test.ts (2)
67-198: Consider adding tests for transaction submission failures.The
executePathPaymenttests cover validation failures (unsupported asset, invalid amount, no path), but don't test scenarios where:
loadAccountfails (account not found or Horizon error)submitTransactionfails (insufficient funds, network error, etc.)These are important edge cases for a payment system.
Would you like me to generate additional test cases for these failure scenarios?
287-295: Test doesn't fully exercise all-paths-failure scenario.The test mocks
validatePaymentAmountto returnfalse, which causes early failure before attempting path finding. To properly test "all payment paths failed" when path finding genuinely fails for all alternatives, the test should allow validation to pass but havefindBestPathreturnnullfor all attempts.src/controllers/MultiAssetPaymentController.ts (1)
13-17: Consider dependency injection for better testability.Services are instantiated directly in the constructor, which makes unit testing difficult without modifying the service instance after construction. Consider accepting services as constructor parameters with defaults.
♻️ Suggested refactor
- constructor() { - this.assetConfigService = new AssetConfigurationService(); - this.assetPriceService = new AssetPriceService(); - this.multiAssetPaymentService = new MultiAssetPaymentService(); - } + constructor( + assetConfigService?: AssetConfigurationService, + assetPriceService?: AssetPriceService, + multiAssetPaymentService?: MultiAssetPaymentService, + ) { + this.assetConfigService = assetConfigService ?? new AssetConfigurationService(); + this.assetPriceService = assetPriceService ?? new AssetPriceService(); + this.multiAssetPaymentService = multiAssetPaymentService ?? new MultiAssetPaymentService(); + }src/entities/AssetConfiguration.ts (1)
15-23: Add unique constraint for merchant+asset combination.The service layer checks for duplicate configurations before creating, but a database-level unique constraint would provide an additional safeguard against race conditions.
♻️ Suggested enhancement
+import { + Entity, + PrimaryGeneratedColumn, + Column, + CreateDateColumn, + UpdateDateColumn, + Index, + Unique, +} from "typeorm"; + +@Unique(["merchantId", "assetCode", "assetIssuer"]) `@Entity`("asset_configurations") export class AssetConfiguration {Note: For the unique constraint to work correctly with nullable
assetIssuer, you may need to use a partial unique index depending on your database (PostgreSQL handles this differently than MySQL).src/dtos/MultiAssetPaymentDTO.ts (3)
1-11: Unused import:IsArray
IsArrayis imported but never used in any of the DTO classes.🧹 Remove unused import
import { IsString, IsNumber, IsOptional, IsBoolean, Min, Max, - IsArray, ValidateNested, } from "class-validator";
13-20: Consider adding format validation for Stellar asset fields.Stellar asset codes must be 1-12 alphanumeric characters, and issuers must be valid Stellar public keys (56 characters starting with 'G'). Adding validation here would catch invalid inputs early.
✨ Suggested validation enhancements
+import { Length, Matches } from "class-validator"; + export class AssetDTO { `@IsString`() + `@Length`(1, 12) + `@Matches`(/^[a-zA-Z0-9]+$/, { message: 'Asset code must be alphanumeric' }) code: string; `@IsString`() `@IsOptional`() + `@Matches`(/^G[A-Z2-7]{55}$/, { message: 'Invalid Stellar public key format' }) issuer?: string; }
91-92: Consider validatingamountas a positive numeric string.The
amountfield accepts any string, but should be a valid positive number for Stellar transactions. Invalid amounts will cause runtime errors during payment execution.✨ Add amount validation
+import { IsNumberString, IsPositive } from "class-validator"; + +// Or use a custom validator: +import { Matches } from "class-validator"; + `@IsString`() + `@Matches`(/^\d+(\.\d{1,7})?$/, { message: 'Amount must be a valid positive number with up to 7 decimal places' }) amount: string;src/services/MultiAssetPaymentService.ts (2)
53-63: Hardcoded dependencies reduce testability.Instantiating
AssetConfigurationServiceandAssetPriceServicedirectly in the constructor makes unit testing harder and prevents injecting mocks. Consider accepting dependencies via constructor parameters.♻️ Constructor injection pattern
- constructor() { + constructor( + assetConfigService?: AssetConfigurationService, + assetPriceService?: AssetPriceService, + ) { const horizonUrl = process.env.STELLAR_HORIZON_URL || "https://horizon-testnet.stellar.org"; this.horizon = new Horizon.Server(horizonUrl); this.networkPassphrase = process.env.STELLAR_NETWORK === "PUBLIC" ? Networks.PUBLIC : Networks.TESTNET; - this.assetConfigService = new AssetConfigurationService(); - this.assetPriceService = new AssetPriceService(); + this.assetConfigService = assetConfigService ?? new AssetConfigurationService(); + this.assetPriceService = assetPriceService ?? new AssetPriceService(); }
146-151: Non-null assertions on path assets could cause runtime errors.The
!assertions onp.asset_codeandp.asset_issuerwill throw if a path asset is malformed. The native asset check on line 147-148 handlesnativetype, but defensive coding would be safer.✨ Add defensive checks
path: bestPath.path.map((p: PathAsset) => { if (p.asset_type === "native") { return Asset.native(); } + if (!p.asset_code || !p.asset_issuer) { + throw new AppError("Invalid path asset: missing code or issuer", 500); + } - return new Asset(p.asset_code!, p.asset_issuer!); + return new Asset(p.asset_code, p.asset_issuer); }),src/services/AssetConfigurationService.ts (2)
42-53: Inconsistent use of default value operators.Line 46 correctly uses
??forisEnabled, but lines 47-52 use||. While the current defaults (0, false, null) work with||, using??consistently is clearer and safer for future changes.✨ Use nullish coalescing consistently
const config = this.assetConfigRepo.create({ merchantId: data.merchantId, assetCode: data.assetCode, assetIssuer: data.assetIssuer, isEnabled: data.isEnabled ?? true, - minAmount: data.minAmount || "0", - maxAmount: data.maxAmount || null, - priority: data.priority || 0, - autoConvert: data.autoConvert || false, - settlementAssetCode: data.settlementAssetCode || null, - settlementAssetIssuer: data.settlementAssetIssuer || null, + minAmount: data.minAmount ?? "0", + maxAmount: data.maxAmount ?? null, + priority: data.priority ?? 0, + autoConvert: data.autoConvert ?? false, + settlementAssetCode: data.settlementAssetCode ?? null, + settlementAssetIssuer: data.settlementAssetIssuer ?? null, });
89-90:Object.assignmay unintentionally set fields toundefined.If the
dataobject contains explicitundefinedvalues (e.g., from a parsed JSON body where optional fields are explicitly null),Object.assignwill overwrite existing values withundefined. Consider filtering out undefined values.✨ Filter undefined values before assignment
- Object.assign(config, data); + // Only assign defined values to avoid overwriting with undefined + Object.entries(data).forEach(([key, value]) => { + if (value !== undefined) { + (config as any)[key] = value; + } + });Or using a utility function:
const definedData = Object.fromEntries( Object.entries(data).filter(([_, v]) => v !== undefined) ); Object.assign(config, definedData);
| updateAssetConfig = async (req: Request, res: Response): Promise<void> => { | ||
| try { | ||
| const { id } = req.params; | ||
| const { merchantId } = req.body; | ||
| const { | ||
| isEnabled, | ||
| minAmount, | ||
| maxAmount, | ||
| priority, | ||
| autoConvert, | ||
| settlementAsset, | ||
| } = req.body; | ||
|
|
||
| const config = await this.assetConfigService.updateAssetConfig( | ||
| id, | ||
| merchantId, | ||
| { | ||
| isEnabled, | ||
| minAmount, | ||
| maxAmount, | ||
| priority, | ||
| autoConvert, | ||
| settlementAssetCode: settlementAsset?.code || null, | ||
| settlementAssetIssuer: settlementAsset?.issuer || null, | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Use authenticated merchant ID instead of request body.
The updateAssetConfig method takes merchantId from req.body, but the route uses authenticateMerchant middleware which sets req.merchant. Using the body value could allow a merchant to attempt modifying another merchant's configuration (though the service layer may prevent this). For defense in depth, use the authenticated merchant's ID.
♻️ Suggested fix
updateAssetConfig = async (req: Request, res: Response): Promise<void> => {
try {
const { id } = req.params;
- const { merchantId } = req.body;
+ const merchantId = (req as any).merchant?.id;
const {
isEnabled,Apply similar changes to deleteAssetConfig and createAssetConfig.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| updateAssetConfig = async (req: Request, res: Response): Promise<void> => { | |
| try { | |
| const { id } = req.params; | |
| const { merchantId } = req.body; | |
| const { | |
| isEnabled, | |
| minAmount, | |
| maxAmount, | |
| priority, | |
| autoConvert, | |
| settlementAsset, | |
| } = req.body; | |
| const config = await this.assetConfigService.updateAssetConfig( | |
| id, | |
| merchantId, | |
| { | |
| isEnabled, | |
| minAmount, | |
| maxAmount, | |
| priority, | |
| autoConvert, | |
| settlementAssetCode: settlementAsset?.code || null, | |
| settlementAssetIssuer: settlementAsset?.issuer || null, | |
| }, | |
| ); | |
| updateAssetConfig = async (req: Request, res: Response): Promise<void> => { | |
| try { | |
| const { id } = req.params; | |
| const merchantId = (req as any).merchant?.id; | |
| const { | |
| isEnabled, | |
| minAmount, | |
| maxAmount, | |
| priority, | |
| autoConvert, | |
| settlementAsset, | |
| } = req.body; | |
| const config = await this.assetConfigService.updateAssetConfig( | |
| id, | |
| merchantId, | |
| { | |
| isEnabled, | |
| minAmount, | |
| maxAmount, | |
| priority, | |
| autoConvert, | |
| settlementAssetCode: settlementAsset?.code || null, | |
| settlementAssetIssuer: settlementAsset?.issuer || null, | |
| }, | |
| ); |
🤖 Prompt for AI Agents
In `@src/controllers/MultiAssetPaymentController.ts` around lines 66 - 91, The
updateAssetConfig handler is reading merchantId from req.body which allows
spoofing; change it to use the authenticated merchant ID provided by the
authenticateMerchant middleware (req.merchant.id) when calling
this.assetConfigService.updateAssetConfig (and likewise for createAssetConfig
and deleteAssetConfig) so the controller always passes the verified merchant id
instead of any value from req.body; locate references to merchantId in
updateAssetConfig, createAssetConfig, and deleteAssetConfig and replace them
with the authenticated req.merchant.id before invoking the service.
| body("sourceSecret") | ||
| .isString() | ||
| .notEmpty() | ||
| .withMessage("Source secret is required"), |
There was a problem hiding this comment.
Critical security concern: Private key transmitted over the network.
The sourceSecret field expects a Stellar secret key to be sent in the HTTP request body. This is a significant security risk because:
- Private keys should never leave the client
- Network interception could expose the key
- Server logs might inadvertently capture it
Consider client-side transaction signing instead, where the server builds the transaction and returns the XDR for the client to sign locally.
🤖 Prompt for AI Agents
In `@src/routes/multiAssetPayment.routes.ts` around lines 70 - 73, The route
currently accepts a sensitive Stellar secret via the body field "sourceSecret";
remove that validator and any code that consumes secrets, replace with accepting
a public key (e.g., "sourcePublicKey") and validate it instead, and change the
handler to build the unsigned transaction XDR (use your existing transaction
building logic—e.g., where you previously used the secret to set the source
account) and return the XDR to the client for local signing; ensure you also add
validation for the public key format and remove any logging of secret material
in the handler that referenced "sourceSecret".
| const amountNum = parseFloat(amount); | ||
| const minAmount = parseFloat(config.minAmount); | ||
|
|
||
| if (amountNum < minAmount) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Missing NaN validation after parseFloat.
If amount or config.minAmount contain invalid numeric strings, parseFloat returns NaN. Comparisons with NaN always return false, which could incorrectly allow invalid amounts or reject valid ones.
🔧 Add NaN checks
const amountNum = parseFloat(amount);
const minAmount = parseFloat(config.minAmount);
+ if (isNaN(amountNum) || isNaN(minAmount)) {
+ logger.warn(`Invalid numeric value: amount=${amount}, minAmount=${config.minAmount}`);
+ return false;
+ }
+
if (amountNum < minAmount) {
return false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const amountNum = parseFloat(amount); | |
| const minAmount = parseFloat(config.minAmount); | |
| if (amountNum < minAmount) { | |
| return false; | |
| } | |
| const amountNum = parseFloat(amount); | |
| const minAmount = parseFloat(config.minAmount); | |
| if (isNaN(amountNum) || isNaN(minAmount)) { | |
| logger.warn(`Invalid numeric value: amount=${amount}, minAmount=${config.minAmount}`); | |
| return false; | |
| } | |
| if (amountNum < minAmount) { | |
| return false; | |
| } |
🤖 Prompt for AI Agents
In `@src/services/AssetConfigurationService.ts` around lines 205 - 210, The parsed
numeric values amountNum and minAmount (from parseFloat(amount) and
parseFloat(config.minAmount)) must be validated for NaN before comparing; update
the code that creates amountNum and minAmount to check isNaN(amountNum) ||
isNaN(minAmount) and handle this case (e.g., return false or throw a validation
error consistent with the surrounding method in AssetConfigurationService), then
proceed with the existing comparison if both are valid numbers.
| private createAsset(code: string, issuer?: string): Asset { | ||
| if (code === "XLM" || !issuer) { | ||
| return Asset.native(); | ||
| } | ||
| return new Asset(code, issuer); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat src/services/AssetPriceService.tsRepository: PayStell/paystell-backend
Length of output: 5597
🏁 Script executed:
rg -r "AssetPriceService" --type ts --type js -A 3 -B 3Repository: PayStell/paystell-backend
Length of output: 465
🏁 Script executed:
rg "AssetPriceService" --type ts --type js -A 3 -B 3Repository: PayStell/paystell-backend
Length of output: 12918
🏁 Script executed:
cat src/controllers/MultiAssetPaymentController.tsRepository: PayStell/paystell-backend
Length of output: 9297
🏁 Script executed:
head -150 src/tests/services/AssetPriceService.test.tsRepository: PayStell/paystell-backend
Length of output: 3669
🏁 Script executed:
wc -l src/tests/services/AssetPriceService.test.tsRepository: PayStell/paystell-backend
Length of output: 117
🏁 Script executed:
grep -n "createAsset\|missing issuer\|issuer.*undefined" src/tests/services/AssetPriceService.test.tsRepository: PayStell/paystell-backend
Length of output: 142
🏁 Script executed:
sed -n '100,250p' src/tests/services/AssetPriceService.test.tsRepository: PayStell/paystell-backend
Length of output: 3659
🏁 Script executed:
sed -n '250,373p' src/tests/services/AssetPriceService.test.tsRepository: PayStell/paystell-backend
Length of output: 3674
🏁 Script executed:
web_search
What does Stellar SDK Asset.native() do and how does new Asset(code, issuer) differ?Repository: PayStell/paystell-backend
Length of output: 289
🌐 Web query:
Stellar SDK Asset.native() constructor behavior non-native assets
💡 Result:
In the JavaScript Stellar SDK (js-stellar-base / @stellar/stellar-sdk), Asset.native() is just a convenience factory that returns a new Asset constructed with code "XLM" and no issuer:
static native() { return new Asset('XLM'); }→ issuer isundefined, so the asset is considered native. (stellar.github.io)
For non-native (issued) assets, you must use the constructor new Asset(code, issuer):
- If
codeis anything other than"XLM"(case-insensitive) andissueris missing/falsey, the constructor throws: “Issuer cannot be null”. (stellar.github.io) isNative()is determined purely by the issuer:return !this.issuer;so any truthy issuer makes it non-native, even if the code is"XLM". (stellar.github.io)- Typical usage is:
StellarSdk.Asset.native()for XLMnew StellarSdk.Asset('USD', issuerPublicKey)for issued assets (stellar-docs.overcat.me)
Citations:
- 1: https://stellar.github.io/js-stellar-base/asset.js.html?utm_source=openai
- 2: https://stellar.github.io/js-stellar-base/asset.js.html?utm_source=openai
- 3: https://stellar.github.io/js-stellar-base/asset.js.html
- 4: https://stellar-docs.overcat.me/js-stellar-base/reference/base-examples.html?utm_source=openai
Reject non-native assets without issuer in createAsset.
Currently, createAsset silently converts any non-XLM asset code with missing issuer to native XLM, causing incorrect asset to be sent to Horizon API calls. The Stellar SDK itself throws "Issuer cannot be null" for non-native assets without issuer. Throw an error instead of falling back to native.
Suggested fix
private createAsset(code: string, issuer?: string): Asset {
- if (code === "XLM" || !issuer) {
- return Asset.native();
- }
- return new Asset(code, issuer);
+ if (code === "XLM") {
+ return Asset.native();
+ }
+ if (!issuer) {
+ throw new AppError("Issuer is required for non-native assets", 400);
+ }
+ return new Asset(code, issuer);
}🤖 Prompt for AI Agents
In `@src/services/AssetPriceService.ts` around lines 47 - 52, The createAsset
method currently treats any non-XLM asset missing an issuer as native; update
createAsset so that it only returns Asset.native() when code === "XLM" and
otherwise throws an error if issuer is missing (e.g., throw new Error("Issuer
cannot be null for non-native asset") or mirror the Stellar SDK message) instead
of returning Asset.native(); locate the createAsset function and replace the
fallback path that returns Asset.native() with an explicit error throw before
creating a new Asset(code, issuer).
| private createAsset(code: string, issuer?: string): Asset { | ||
| if (code === "XLM" || !issuer) { | ||
| return Asset.native(); | ||
| } | ||
| return new Asset(code, issuer); | ||
| } |
There was a problem hiding this comment.
Potential silent data corruption: missing issuer treated as native XLM.
When issuer is undefined/null for a non-XLM asset, this returns Asset.native() instead of throwing an error. This could cause payments to be sent as XLM when they should fail validation.
🐛 Proposed fix
private createAsset(code: string, issuer?: string): Asset {
- if (code === "XLM" || !issuer) {
+ if (code === "XLM") {
return Asset.native();
}
+ if (!issuer) {
+ throw new AppError(`Issuer required for non-native asset: ${code}`, 400);
+ }
return new Asset(code, issuer);
}🤖 Prompt for AI Agents
In `@src/services/MultiAssetPaymentService.ts` around lines 65 - 70, The
createAsset method currently returns Asset.native() when issuer is missing,
which silently converts non-XLM assets to XLM; change createAsset(code: string,
issuer?: string): Asset so that if code !== "XLM" and issuer is
null/undefined/empty it throws a descriptive error (e.g., "Missing issuer for
non-XLM asset") instead of returning Asset.native(), otherwise return
Asset.native() for code === "XLM" and return new Asset(code, issuer) for valid
issuer; update any callers to handle the thrown error if necessary.
| txBuilder.addOperation( | ||
| Operation.pathPaymentStrictSend({ | ||
| sendAsset: sourceAsset, | ||
| sendAmount: data.amount, | ||
| destination: data.destinationAddress, | ||
| destAsset: destAsset, | ||
| destMin: bestPath.destination_amount, | ||
| path: bestPath.path.map((p: PathAsset) => { | ||
| if (p.asset_type === "native") { | ||
| return Asset.native(); | ||
| } | ||
| return new Asset(p.asset_code!, p.asset_issuer!); | ||
| }), | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Setting destMin to exact destination amount is fragile.
Using bestPath.destination_amount as destMin means the transaction will fail if the exchange rate changes even slightly between path discovery and submission. Consider applying a small slippage tolerance (e.g., 0.5-1%).
🔧 Apply slippage tolerance
+ // Apply 0.5% slippage tolerance to account for rate fluctuations
+ const slippageTolerance = 0.995;
+ const destMin = (parseFloat(bestPath.destination_amount) * slippageTolerance).toFixed(7);
+
txBuilder.addOperation(
Operation.pathPaymentStrictSend({
sendAsset: sourceAsset,
sendAmount: data.amount,
destination: data.destinationAddress,
destAsset: destAsset,
- destMin: bestPath.destination_amount,
+ destMin: destMin,
path: bestPath.path.map((p: PathAsset) => {🤖 Prompt for AI Agents
In `@src/services/MultiAssetPaymentService.ts` around lines 139 - 153, The code
sets destMin directly to bestPath.destination_amount which is brittle; update
the txBuilder.addOperation call (Operation.pathPaymentStrictSend) to apply a
small slippage tolerance (e.g., 0.5%-1%) to bestPath.destination_amount before
assigning destMin: parse bestPath.destination_amount to a numeric type, multiply
by (1 - tolerance) (e.g., 0.995 for 0.5%), then convert back to the string
representation expected by pathPaymentStrictSend and use that value for destMin;
ensure this transformation is applied where
txBuilder.addOperation/Operation.pathPaymentStrictSend is invoked and handle
potential precision/formatting issues (use a decimal library or safe rounding)
so you don't underflow to zero.
| for (const config of configs) { | ||
| if (config.assetCode === data.sourceAssetCode) continue; | ||
|
|
||
| try { | ||
| const fallbackResult = await this.executePathPayment({ | ||
| ...data, | ||
| sourceAssetCode: config.assetCode, | ||
| sourceAssetIssuer: config.assetIssuer || undefined, | ||
| }); | ||
|
|
||
| logger.info(`Fallback payment succeeded with ${config.assetCode}`); | ||
| return fallbackResult; | ||
| } catch (fallbackError) { | ||
| logger.warn( | ||
| `Fallback with ${config.assetCode} failed:`, | ||
| fallbackError, | ||
| ); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Critical: Fallback uses same amount for different assets.
When falling back to a different source asset, the same amount value is used. However, "100 USDC" and "100 XLM" are vastly different values. The fallback should either convert the amount to the equivalent value in the fallback asset, or this approach needs reconsideration.
Additionally, the fallback assumes the payer has sufficient balance in the fallback asset, which may not be true. Consider:
- Converting the amount to the fallback asset's equivalent value
- Checking the source account's balance before attempting fallback
- Documenting this behavior clearly if it's intentional
🤖 Prompt for AI Agents
In `@src/services/MultiAssetPaymentService.ts` around lines 200 - 218, The
fallback loop in MultiAssetPaymentService uses the original data.amount for
every alternative asset (in the for-of over configs and the call to
this.executePathPayment), which produces incorrect value semantics and can fail
if the payer lacks that asset balance; update the logic in the loop that
iterates over configs to (1) compute an equivalent amount for each fallback
asset before calling executePathPayment (e.g., by calling or adding a conversion
helper that uses market rates/rateProvider), (2) check the source account
balance for the candidate asset (use the account/balance lookup function in this
service or the ledger client) and skip if insufficient, and (3) pass the
converted amount and sourceAssetIssuer via the same call to executePathPayment;
ensure you touch the code around executePathPayment, configs, data.amount,
sourceAssetCode, and sourceAssetIssuer so the fallback uses the converted amount
and performs a balance check.
| return { | ||
| estimatedDestAmount: quote.destinationAmount, | ||
| rate: quote.rate, | ||
| path: quote.path, | ||
| fees: BASE_FEE, | ||
| }; |
There was a problem hiding this comment.
Type mismatch: BASE_FEE is a number but fees expects string.
BASE_FEE from Stellar SDK is a number (100), but the PaymentEstimate interface defines fees as string. This will work at runtime due to JavaScript coercion, but is inconsistent with the interface definition.
🔧 Fix type consistency
return {
estimatedDestAmount: quote.destinationAmount,
rate: quote.rate,
path: quote.path,
- fees: BASE_FEE,
+ fees: BASE_FEE.toString(),
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return { | |
| estimatedDestAmount: quote.destinationAmount, | |
| rate: quote.rate, | |
| path: quote.path, | |
| fees: BASE_FEE, | |
| }; | |
| return { | |
| estimatedDestAmount: quote.destinationAmount, | |
| rate: quote.rate, | |
| path: quote.path, | |
| fees: BASE_FEE.toString(), | |
| }; |
🤖 Prompt for AI Agents
In `@src/services/MultiAssetPaymentService.ts` around lines 257 - 262, The
returned object in MultiAssetPaymentService assigns fees: BASE_FEE (a number)
but the PaymentEstimate interface expects a string; fix by converting the SDK
constant to a string where the object is constructed (replace fees: BASE_FEE
with fees: BASE_FEE.toString() or String(BASE_FEE)) or alternatively update the
PaymentEstimate.fees type to number if you intend fees to be numeric throughout
— apply the chosen change near the method that returns
estimatedDestAmount/rate/path/fees so types align.
|
@respp pls reveiw this pr |
2 participants
Closes #103
Summary
Implemented a complete multi-asset payment gateway that enables merchants to accept various Stellar assets with dynamic routing through Stellar's DEX for optimal exchange rates.
Changes
asset_configurationstable with proper indexes and constraintsTechnical Implementation
pathPaymentStrictSendfor multi-asset routingAPI Endpoints
✅
POST /api/multi-asset/config- Create asset configuration✅
PUT /api/multi-asset/config/:id- Update configuration✅
DELETE /api/multi-asset/config/:id- Delete configuration✅
GET /api/multi-asset/config/merchant/:merchantId- Get merchant configs✅
POST /api/multi-asset/exchange-rate- Get real-time exchange rate✅
POST /api/multi-asset/payment- Execute path payment✅
POST /api/multi-asset/estimate- Estimate payment cost✅
GET /api/multi-asset/pairs/:merchantId- Get supported asset pairs✅
POST /api/multi-asset/orderbook- Get DEX orderbookTesting
✅ 42/42 tests passing (100%)
All tests pass successfully. Services tested for:
Example Usage
Configure Merchant Assets
POST /api/multi-asset/config { "merchantId": "uuid", "asset": {"code": "USDC", "issuer": "GA5ZSE..."}, "minAmount": "1", "maxAmount": "10000", "priority": 1 }Get Real-Time Exchange Rate
POST /api/multi-asset/exchange-rate { "sourceAsset": {"code": "USDC", "issuer": "GA5ZSE..."}, "destinationAsset": {"code": "XLM"}, "amount": "100" } Response: { "rate": "9.455", "destinationAmount": "945.5", "path": [] }Migration
Database migration included:
src/scripts/create-asset-configuration-table.sqlRun with:
psql -h localhost -p 5432 -U postgres -d paystell < src/scripts/create-asset-configuration-table.sqlDocumentation
Complete implementation guide in code comments and test files.
Acceptance Criteria Met
✅ Supports at least 3 assets (XLM, USDC, custom tokens)
✅ Successfully routes payments via path payments
✅ Real-time exchange rates available
✅ Merchants can configure accepted assets
✅ Fallback logic handles failed path payments
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.