Add xgram support #405
Add xgram support #405
Conversation
src/swap/central/xgram.ts
Outdated
| 'x-api-key': apiKey | ||
| } | ||
|
|
||
| async function fetchSupportedAssets(): Promise<void> { |
There was a problem hiding this comment.
This is non-compliant with API requirements. There should not be a separate call to fetch assets. The single quoting endpoint should directly accept a chain name and contract address. Otherwise network latency could cause the quoting to take too long.
| fromCurrency: xgramCodes.fromCurrencyCode, | ||
| toCurrency: xgramCodes.toCurrencyCode, |
There was a problem hiding this comment.
again, these two lines are non-compliant. chain name/code and contract address should be specified
There was a problem hiding this comment.
again, these two lines are non-compliant. chain name/code and contract address should be specified
@paullinator We don’t specify a contract address when creating exchanges. Each coin has a unique name, which is defined on line 50 of this plugin.
There was a problem hiding this comment.
This doesn't adhere to our api requirements. Please update the API and resubmit the PR
|
Commit title is poor. Should be "Add Xgram swap support" |
src/swap/central/xgram.ts
Outdated
|
|
||
| if (orderResponseJson.result !== true) { | ||
| const errMsg = String(orderResponseJson.error ?? '') | ||
| throw new Error(`Xgram call returned error message: ${errMsg}`) |
There was a problem hiding this comment.
This is insufficient. API needs to provide all the errors and properly throw an error code like SwapAboveLimitError. API should return all errors at once and the plugin should surface the errors in this order or priority
- asset not supported
- region restricted
- above/below limit errors
There was a problem hiding this comment.
API should return all errors at once and the plugin should surface the errors in this order or priority
@paullinator We have added error handling for asset not supported and above/below limit errors, but the region restricted error is not currently handled.
There was a problem hiding this comment.
API must return all error types at once in an array and the code can choose which error to surface. Please update API and re-submit PR
|
Our workflow is to never merge master into a feature branch. Always rebase your commits on top of master. Please squash this all into one commit for review. |
devvasxbbtstyle
force-pushed
the
xgram/plugin
branch
from
c5aaf5a to
62d8e3f
Compare
|
cursor review verbose=true |
|
Bugbot request id: serverGenReqId_e478b76d-79f0-4afa-828f-74563cb5b902 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
|
||
| async function swapExchange(isSelling: boolean): Promise<SwapOrder> { | ||
| const buyUrl = `fromCcy=${xgramCodes.toCurrencyCode}&toCcy=${xgramCodes.fromMainnetCode}` | ||
| const sellUrl = `fromCcy=${xgramCodes.fromMainnetCode}&toCcy=${xgramCodes.toCurrencyCode}` |
There was a problem hiding this comment.
Bug: Rate lookup uses wrong code types causing token swap failures
The rate lookup URL construction incorrectly uses fromMainnetCode (chain/network code) where currency/token codes are expected. In sellUrl, fromCcy uses fromMainnetCode instead of fromCurrencyCode, and in buyUrl, toCcy uses fromMainnetCode instead of a currency code. This causes token swaps to fail because the API receives the chain code (e.g., "ETH") instead of the actual token code (e.g., "USDT"). Native coin swaps may work by coincidence since mainnet and currency codes are often identical, but all token swaps will fail or return incorrect rates.
|
|
||
| if (marketRangeResponseJson.error === 'Pair not found') { | ||
| throw new SwapCurrencyError(swapInfo, request) | ||
| } |
There was a problem hiding this comment.
Bug: Cleaner throws before "Pair not found" error can be handled
The asMarketRange cleaner is called before checking for the "Pair not found" error. When the API returns an unsupported pair error, the response likely won't include minFromCcyAmount and maxFromCcyAmount fields. The cleaner expects these as required numbers and will throw an unhandled cleaner error before the code reaches the "Pair not found" check on line 275. This causes users to see a cryptic cleaner error instead of the proper SwapCurrencyError for unsupported currency pairs. The "Pair not found" check needs to happen before calling asMarketRange.
|
cursor review verbose=true Use all *.md files in the https://github.com/EdgeApp/edge-conventions repo to determine coding and PR commit style. Use any *.md files in each repo to find docs and coding conventions to adhere to when doing reviews. For this repo use this file to determine if the PR adheres to API requirements https://github.com/EdgeApp/edge-exchange-plugins/blob/paul/apiReq/docs/API_REQUIREMENTS.md |
| isSelling ? undefined : 'to' | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Bug: Missing throw allows fallthrough when result is false
When marketRangeResponseJson.result is false but none of the specific error conditions are met (not "Pair not found", not below min, not above max), execution falls through the if block and continues to createOrder. This allows attempting to create an order even though the rate API indicated failure, potentially leading to unexpected behavior or invalid orders being submitted.
There was a problem hiding this comment.
see all comments. and re-review
https://github.com/EdgeApp/edge-exchange-plugins/blob/paul/apiReq/docs/API_REQUIREMENTS.md
2 participants
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneDescription
noneNote
Adds Xgram swap provider and new helpers for denomination/native conversions and contract-address lookups.
src/swap/central/xgram.tsintegrating Xgram API to fetch rates, create orders, enforce min/max limits, handle unsupported pairs, set memos/tags, and build spend actions with expiration handling.xgraminsrc/index.tsplugins map.src/util/swapHelpers.ts):nativeToDenomination,denominationToNative,getCurrencyMultiplier, andgetContractAddressesplus required math imports.Written by Cursor Bugbot for commit 62d8e3f. This will update automatically on new commits. Configure here.