Add n.exchange centralized swap integration#426
Conversation
0f22bf7 to
663b859
Compare
f895046 to
082583a
Compare
1d42f05 to
1166816
Compare
paullinator
left a comment
There was a problem hiding this comment.
Review Summary
The implementation is generally well-structured and follows existing plugin patterns. However, there are 3 critical issues that should be addressed before merge.
Critical Issues (Must Fix)
- JSON parsing errors not handled (line 269)
getContractAddresssilently falls back to native currency (lines 141-149)parseIntcould return NaN, bypassing expiration check (lines 349-351)
Additional Note
Commit Message Inaccuracy: The commit body mentions "updates to existing swap plugins to use the new mapping system" but there are no modifications to existing swap plugins in this commit. Consider amending the commit message for accuracy.
Positive Observations
- Good use of existing helpers (
getMaxSwappable,makeSwapPluginQuote, etc.) - Clean cleaner definitions with
asRateV2andasOrderV2 - Proper error handling with
SwapCurrencyError,SwapAboveLimitError,SwapBelowLimitError - Good use of
Promise.allfor parallel address fetching - Appropriate use of
ensureInFuturefor expiration handling - Good memo handling for chains like XRP/XLM
See inline comments for specific issues and recommendations.
| const expirationDate: Date = | ||
| order.fixed_rate_deadline != null | ||
| ? ensureInFuture(order.fixed_rate_deadline) ?? defaultExpiration | ||
| : ensureInFuture(defaultExpiration) ?? defaultExpiration |
There was a problem hiding this comment.
Redundant null coalescing on guaranteed non-null value
Low Severity
The expression ensureInFuture(defaultExpiration) ?? defaultExpiration on line 520 has a redundant null coalescing operator. Since defaultExpiration is always a non-null Date object (computed on lines 514-516), ensureInFuture will never return undefined for it per its implementation. The ?? defaultExpiration fallback can never be reached.
8c00d0d to
a72e791
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| asString, | ||
| asObject({ code: asString, network: asString }), | ||
| asObject({ contract_address: asString, network: asString }) | ||
| ) |
There was a problem hiding this comment.
Cleaner doesn't handle null contract_address in API response
High Severity
The asNexchangeCurrencyResponse cleaner's third option expects contract_address to be a string via asString, but the formatCurrency function sends contract_address: null for native currencies. If the API echoes this format back in the order response, parsing will fail because neither option in asEither can handle { contract_address: null, network: "..." }. Other plugins like changehero, changenow, and letsexchange correctly use asEither(asString, asNull) for nullable contract addresses. This could break all native currency swaps.
a72e791 to
b83b9cb
Compare
paullinator
left a comment
There was a problem hiding this comment.
Review Summary
This PR adds n.exchange as a new centralized swap provider. The implementation uses contract addresses for token identification and integrates with the n.exchange API v2.
Previous Review Comments - All Addressed ✅
All critical and warning issues raised by Paul and Bug Bot have been addressed:
getContractAddressnow throws errors instead of silently falling backparseIntNaN check added- JSON parsing wrapped in try/catch
- Cleaner failures handled with try/catch
checkInvalidTokenIdscalledensureInFutureused for expiration dates
Remaining Items
Before merge:
- Add
checkWhitelistedMainnetCodescall for consistency with other plugins - Consider moving
MAINNET_CODE_TRANSCRIPTIONtosrc/mappings/nexchange.ts
Nice to have (can be deferred):
- Add unit tests for
getFixedQuote()business logic - Consolidate repetitive documentation comments
Git
git fetch origin master
git rebase origin/master
git push --force-with-leaseReady for merge after adding checkWhitelistedMainnetCodes.
See inline comments for specific suggestions.
paullinator
left a comment
There was a problem hiding this comment.
Additional Inline Suggestions
See inline comments for specific code improvements. These are minor suggestions - the main review summary was posted earlier.
Key inline items:
- Line 560: Add
checkWhitelistedMainnetCodescall for consistency - Line 80: Consider using mapping file pattern
- Various type safety and naming improvements
src/swap/central/nexchange.ts
Outdated
| // See https://api.n.exchange/en/api/v2/currency/ for list of supported currencies | ||
| // Network codes map to Nexchange network identifiers | ||
| // Based on supported networks: ALGO, ATOM, SOL, BCH, BTC, DASH, DOGE, DOT, EOS, TON, HBAR, LTC, XLM, XRP, XTZ, ZEC, TRON, ADA, BASE, MATIC/POL, ETH, AVAXC, BSC, ETC, ARB, OP, FTM, SONIC | ||
| // eslint-disable-next-line @typescript-eslint/consistent-type-assertions |
There was a problem hiding this comment.
Suggestion: Other plugins use mapping files in src/mappings/ with mapToRecord(). Consider creating src/mappings/nexchange.ts for consistency:
import { mapToRecord } from '../utils'
export const MAINNET_CODE_TRANSCRIPTION = mapToRecord([
['ethereum', 'ETH'],
// ...
])| ): Promise<EdgeSwapQuote> { | ||
| const request = convertRequest(req) | ||
|
|
||
| checkInvalidTokenIds(INVALID_TOKEN_IDS, request, swapInfo) |
There was a problem hiding this comment.
Warning: Other plugins (changenow, exolix, letsexchange, sideshift) call checkWhitelistedMainnetCodes in addition to checkInvalidTokenIds. Add this for consistent validation:
import { checkWhitelistedMainnetCodes } from './common/swapHelpers'
checkWhitelistedMainnetCodes(allowedCurrencyCodes, request, swapInfo)
src/swap/central/nexchange.ts
Outdated
| let rates | ||
| try { | ||
| rates = asArray(asRateV2)(rateResponse) | ||
| } catch (e) { |
There was a problem hiding this comment.
Nit: Convention is to use error instead of e for caught exceptions.
src/swap/central/nexchange.ts
Outdated
| } catch (e) { | ||
| throw new Error(`Nexchange rate response parsing failed: ${String(e)}`) | ||
| } | ||
| const rate = rates[0] // Contract address query returns single rate |
There was a problem hiding this comment.
Suggestion: This assumes the API returns exactly one rate. Consider validating array length:
if (rates.length === 0) {
throw new SwapCurrencyError(swapInfo, request)
}
const rate = rates[0]
src/swap/central/nexchange.ts
Outdated
| headers?: { [key: string]: string } | ||
| } = {}, | ||
| request?: EdgeSwapRequestPlugin | ||
| ): Promise<any> { |
There was a problem hiding this comment.
Suggestion: Consider Promise<unknown> instead of Promise<any> for better type safety, since the data is cleaned after retrieval.
src/swap/central/nexchange.ts
Outdated
| const { fetchCors = io.fetch } = io | ||
| const { apiKey, referralCode } = asInitOptions(opts.initOptions) | ||
|
|
||
| const headers: { [key: string]: string } = { |
There was a problem hiding this comment.
Nit: Use the imported StringMap type instead of inline { [key: string]: string }.
src/swap/central/nexchange.ts
Outdated
| referralCode: asString | ||
| }) | ||
|
|
||
| const orderUri = 'https://n.exchange/order/' |
There was a problem hiding this comment.
Suggestion: Consider more descriptive constant names like ORDER_BASE_URL and API_BASE_URL for clarity.
Integrate n.exchange as a centralized swap provider supporting multiple networks and tokens. The implementation uses contract addresses for token identification, eliminating the need for currency code mapping. Changes: - Add n.exchange swap plugin (src/swap/central/nexchange.ts) - Add n.exchange mapping file (src/mappings/nexchange.ts) - Register nexchange plugin in src/index.ts - Add partner mapping JSON and tests - Update CHANGELOG.md
c1aefb3 to
b098dd3
Compare
All comments addressed |
|
We are getting a 400 error when transacting from our San Diego office: |
Add n.exchange centralized swap integration
Description
Integrate n.exchange as a centralized swap provider supporting multiple networks and tokens. The implementation uses contract addresses for token identification, eliminating the need for currency code mapping. This approach provides better support for tokens and aligns with API requirements.
The integration includes support for native currencies and ERC20 tokens across major networks including Ethereum, Polygon, Base, Arbitrum, Optimism, BSC, and others. The plugin handles rate queries, order creation, and payment processing through the n.exchange API v2.
Additional changes include mapctl tooling for automatic provider mappings and updates to existing swap plugins to use the new mapping system.
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
none
Changes
Testing
Checklist
Note
Medium Risk
Introduces a new external swap integration that constructs spend targets and order metadata based on third-party API responses, so parsing/mapping errors could impact swap availability and correctness. Changes are additive and isolated to a new plugin plus wiring/tests.
Overview
Adds n.exchange as a new centralized swap provider (
nexchange) and wires it intosrc/index.tsso it is available alongside existing swap plugins.Implements a full API v2 integration that fetches fixed-rate quotes, enforces min/max limits, creates orders, and builds
EdgeSpendInfo/swap metadata, using contract-address-based token identification (plus a pluginId→network-code transcription map) for native assets and tokens.Updates test/config coverage by adding a
nexchangepartner mapping JSON, extendingpartnerJson.test.tswith nexchange cases, and addingNEXCHANGE_INIToptions for API key/referral configuration; also documents the addition inCHANGELOG.md.Written by Cursor Bugbot for commit 3148f31. This will update automatically on new commits. Configure here.