Conversation
There was a problem hiding this comment.
Pull request overview
Ports contract-ID (Soroban token) support into master so the extension can handle Stellar Expert asset search results that return contract token IDs instead of only {code}-{issuer} identifiers.
Changes:
- Update balance lookup to match Soroban balances by contract ID.
- Enhance asset search mapping to construct
ManageAssetCurrencyrows from Stellar Expert contract-ID records. - Add unit + e2e test coverage for contract-ID asset search and swap error messaging.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| extension/src/popup/helpers/balance.ts | Adds contract-ID matching branch in findAssetBalance. |
| extension/src/popup/helpers/tests/balance.test.js | Adds unit tests for classic vs contract-ID balance matching. |
| extension/src/popup/components/swap/SwapAmount/hooks/useSimulateSwapData.tsx | Centralizes swap error display logic and adds contract-ID-specific messaging. |
| extension/src/popup/components/swap/SwapAmount/hooks/tests/useSimulateSwapData.test.ts | Adds unit tests for getSwapErrorMessage. |
| extension/src/popup/components/manageAssets/SearchAsset/hooks/useAssetLookup.ts | Maps Stellar Expert contract-ID search records into Manage Assets rows. |
| extension/e2e-tests/helpers/stubs.ts | Adds Stellar Expert asset-search stub returning a contract-ID record. |
| extension/e2e-tests/addAssetContractSearch.test.ts | Adds e2e coverage for contract-ID search results (“Already Added”, “Add”, and add-flow). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (isContractId(record.asset)) { | ||
| return { | ||
| code: record.code || record.tomlInfo?.code || "", | ||
| issuer: record.asset, | ||
| contract: record.asset, |
There was a problem hiding this comment.
The Stellar Expert search mapping now emits contract-ID assets (issuer/contract both set to the contract ID). Downstream in this hook, BlockAid bulk scanning builds asset_ids as ${code}-${issuer} for all returned rows when the search term is not a contract ID; that will end up scanning code-<contractId> for these results, which the existing logic/comments indicate is intended only for classic assets. Consider filtering contract-ID rows out of the BlockAid scan path (e.g., skip rows where isContractId(row.issuer) / row.contract is set) or updating the scan identifier format if BlockAid supports contract assets.
| stubOverrides: async () => { | ||
| await stubAssetSearchWithContractId(page); | ||
| await stubAccountBalancesE2e(page); | ||
| await stubTokenDetails(page); | ||
| await stubIsSac(page); | ||
| await stubScanAssetSafe(page); |
There was a problem hiding this comment.
These stubs are added after stubAllExternalApis, which already registers route handlers for both **/asset?search** and **/account-balances/**. Without calling page.unroute(...) first, these overrides may not take effect (other tests override default stubs by unrouting first). Unroute the existing handlers before registering stubAssetSearchWithContractId / stubAccountBalancesE2e to avoid flaky or failing expectations.
| stubOverrides: async () => { | ||
| await stubAssetSearchWithContractId(page); | ||
| await stubTokenDetails(page); | ||
| await stubIsSac(page); | ||
| await stubScanAssetSafe(page); |
There was a problem hiding this comment.
stubAllExternalApis already stubs **/asset?search**. To ensure this contract-ID version is actually used, unroute the existing handler (e.g., await page.unroute("**/asset?search**")) before calling stubAssetSearchWithContractId(page) in this override.
| stubOverrides: async () => { | ||
| await stubAssetSearchWithContractId(page); | ||
| await stubTokenDetails(page); | ||
| await stubIsSac(page); | ||
| await stubScanAssetSafe(page); |
There was a problem hiding this comment.
stubAllExternalApis already stubs **/asset?search**. To ensure this contract-ID version is actually used, unroute the existing handler (e.g., await page.unroute("**/asset?search**")) before calling stubAssetSearchWithContractId(page) in this override.
| ); | ||
| expect(result).toBe( | ||
| "We had an issue retrieving your transaction details. Please try again.", | ||
| ); | ||
| }); |
There was a problem hiding this comment.
This test asserts the full unknown-error string literal, which is brittle if copy changes. Consider exporting the unknown-error constant from the hook module or asserting more flexibly (e.g., stable substring) to keep the test focused on behavior rather than exact wording.
Closes #2677
Porting the changes from #2679 to the master branch