feat(stellar): asset migration - #331
Draft
stanleyyconsensys wants to merge 8 commits into
Draft
stanleyyconsensys wants to merge 8 commits into
stanleyyconsensys wants to merge 8 commits into
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
There are confirmed blocking issues that force migration behavior on unconditionally and bypass the intended migration guard when backfilling the catalog, plus a formatting issue likely to fail lint/format checks.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (4)
What changed in this PR
Adds initial “Core assets migration” plumbing for the Stellar snap so that (when a remote flag enables it) holdings and catalog metadata can be read from Core’s AssetsController instead of snap state, while still allowing fallback to existing snap persistence where needed.
Changes:
- Introduces
AssetsService+CoreAssetsAdapterand wires Core messenger/endowments to fetch account assets and asset metadata. - Updates on-chain account resolution/sync to optionally bind balances from Core assets and (optionally) overlay account ledger meta from Soroban RPC.
- Extends network layer with
getAccountLedgerMetaand adds additional validation helpers for Stellar int64-like values.
| File | Description |
|---|---|
| yarn.lock | Adds Core migration-related dependencies for the Stellar snap workspace. |
| packages/stellar-wallet-snap/src/services/on-chain-account/utils.ts | Makes minimum-balance calc use a named constant; adds helper to invert minimum reserve into subentry count. |
| packages/stellar-wallet-snap/src/services/on-chain-account/utils.test.ts | Adds tests for subentryCountFromMinimumReserveStroops. |
| packages/stellar-wallet-snap/src/services/on-chain-account/OnChainAccountService.ts | Adds Core-backed account resolution and skips snap-state sync when migration is enabled. |
| packages/stellar-wallet-snap/src/services/on-chain-account/OnChainAccountService.test.ts | Adds tests for Core-backed account resolution and migration-enabled sync skip. |
| packages/stellar-wallet-snap/src/services/on-chain-account/__mocks__/onChainAccount.fixtures.ts | Wires AssetsService into on-chain account service test fixture. |
| packages/stellar-wallet-snap/src/services/network/NetworkService.ts | Adds getAccountLedgerMeta via Soroban RPC getAccountEntry. |
| packages/stellar-wallet-snap/src/services/network/NetworkService.test.ts | Adds unit tests for getAccountLedgerMeta success/error paths. |
| packages/stellar-wallet-snap/src/services/network/api.ts | Adds AccountLedgerMeta type for RPC-derived account fields. |
| packages/stellar-wallet-snap/src/services/assets/utils.ts | Adds migration-flag helper (currently with a blocking issue noted in comments). |
| packages/stellar-wallet-snap/src/services/assets/index.ts | Exposes new assets facade, adapter, parsing helpers, and migration util. |
| packages/stellar-wallet-snap/src/services/assets/AssetsService.ts | Adds Core AssetsController facade with parsing/validation of Core payloads. |
| packages/stellar-wallet-snap/src/services/assets/AssetsService.test.ts | Adds tests for catalog metadata parsing and account-asset validation behavior. |
| packages/stellar-wallet-snap/src/services/assets/api.ts | Defines superstruct validation for Core “Stellar-shaped” asset rows/metadata. |
| packages/stellar-wallet-snap/src/services/assets/api.test.ts | Adds tests for the Core asset validation/parsing helpers. |
| packages/stellar-wallet-snap/src/services/assets/adapters/CoreAssetsAdapter.ts | Implements adapter over AssetsProvider calls (account assets + metadata). |
| packages/stellar-wallet-snap/src/services/assets/__mocks__/assetsService.fixtures.ts | Adds AssetsService test fixture with mocked adapter + feature flag values. |
| packages/stellar-wallet-snap/src/services/asset-metadata/utils.ts | Allows overriding icon URL (e.g., from Core metadata image). |
| packages/stellar-wallet-snap/src/services/asset-metadata/AssetMetadataService.ts | Adds Core metadata lookup path + migration-enabled sync skip (with one bypass issue noted in comments). |
| packages/stellar-wallet-snap/src/services/asset-metadata/AssetMetadataService.test.ts | Adds tests for migration-enabled sync skip and Core-first metadata resolution. |
| packages/stellar-wallet-snap/src/services/asset-metadata/__mocks__/assets.fixtures.ts | Wires AssetsService into asset-metadata service mock fixture. |
| packages/stellar-wallet-snap/src/handlers/keyring/keyring.ts | Updates call to resolveOnChainAccountByKeyringAccountId with account address. |
| packages/stellar-wallet-snap/src/handlers/clientRequest/onAmountInput.test.ts | Updates expectations for the new resolveOnChainAccountByKeyringAccountId signature. |
| packages/stellar-wallet-snap/src/handlers/accountResolver.ts | Updates call to resolveOnChainAccountByKeyringAccountId with account address. |
| packages/stellar-wallet-snap/src/handlers/accountResolver.test.ts | Updates expectations for the new resolveOnChainAccountByKeyringAccountId signature. |
| packages/stellar-wallet-snap/src/context.ts | Wires Core messenger providers + adapter/service and injects into dependent services. |
| packages/stellar-wallet-snap/src/constants.ts | Adds ACCOUNT_MINIMUM_BASE_RESERVE_UNIT constant for minimum-balance math. |
| packages/stellar-wallet-snap/src/api/integer.ts | Fixes max-int comparison usage and adds ValidStellarInt64Struct. |
| packages/stellar-wallet-snap/src/api/integer.test.ts | Adds tests for ValidStellarInt64Struct. |
| packages/stellar-wallet-snap/src/api/core-messenger.ts | Defines types/actions namespace for Core messenger endowment (formatting issue noted in comments). |
| packages/stellar-wallet-snap/snap.manifest.json | Adds endowment:messenger actions for feature flag + assets controller reads. |
| packages/stellar-wallet-snap/package.json | Adds devDependencies needed for Core messenger + assets controller integration. |
| packages/snap-networks-utils/src/providers/assets/AssetsProvider.ts | Adds AssetsController getAssetMetadata action support. |
| packages/snap-networks-utils/src/providers/assets/AssetsProvider.test.ts | Adds test for AssetsProvider.getAssetMetadata. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+20
to
+23
| return true; | ||
| const flagValue = await remoteFeatureFlagsProvider.getFeatureFlag( | ||
| SNAPS_ASSETS_MIGRATION_FLAG_KEYS.stellar, | ||
| ); |
Comment on lines
+143
to
+144
| const tokensMetadata = await this.#getAssetsByChainId(scope); | ||
| await this.#assetMetadataRepository.saveMany(tokensMetadata); |
Comment on lines
+1
to
+5
| import type { | ||
| AssetsControllerGetAccountAssetByIDAction, | ||
| AssetsControllerGetAccountAssetsByIDsAction, | ||
| AssetsControllerGetAccountAssetsByScopeAction, | ||
| AssetsControllerGetAssetMetadataAction, |
|
|
||
| if (assets.length === 0) { | ||
| // If the account has no assets from core, | ||
| // Possiblly not indexed, or not activated |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
References
Checklist