From a0b304f87b9b4d3f2b5c89fc14af2b98c5ae9e5c Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Mon, 15 Jun 2026 09:44:17 +0800 Subject: [PATCH 01/10] feat: add asset sync --- packages/snap/snap.manifest.json | 10 +- packages/snap/src/context.ts | 10 +- .../snap/src/handlers/cronjob/api.test.ts | 56 +++++++ packages/snap/src/handlers/cronjob/api.ts | 18 +++ .../src/handlers/cronjob/syncAssets.test.ts | 43 +++++ .../snap/src/handlers/cronjob/syncAssets.ts | 40 +++++ .../handlers/cronjob/trackTransaction.test.ts | 1 + .../__mocks__/assets.fixtures.ts | 11 ++ .../OnChainAccountService.test.ts | 2 + .../on-chain-account/OnChainAccountService.ts | 8 +- .../OnChainAccountSynchronizeService.test.ts | 96 ++++++----- .../OnChainAccountSynchronizeService.ts | 10 +- .../__mocks__/onChainAccount.fixtures.ts | 3 - .../services/sync/SynchronizeService.test.ts | 152 +++++++++++++++++- .../src/services/sync/SynchronizeService.ts | 128 ++++++++++++--- .../transaction/TransactionService.ts | 8 +- .../TransactionSynchronizeService.test.ts | 14 +- .../TransactionSynchronizeService.ts | 34 ++-- .../__mocks__/transaction.fixtures.ts | 3 - 19 files changed, 522 insertions(+), 125 deletions(-) create mode 100644 packages/snap/src/handlers/cronjob/syncAssets.test.ts create mode 100644 packages/snap/src/handlers/cronjob/syncAssets.ts diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 3633ead6..707d4fa1 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-stellar-wallet.git" }, "source": { - "shasum": "YP/x/UxnvPyYy6yLPeQM2j1PZR8D8I4xUrgqoUnWbSE=", + "shasum": "N+qKlG+VmcnuPg3jYgYotB7PZEiFIoQz6FCKI2CIDjY=", "location": { "npm": { "filePath": "dist/bundle.js", @@ -43,6 +43,12 @@ "request": { "method": "synchronizeAccounts" } + }, + { + "duration": "PT1H", + "request": { + "method": "synchronizeAssets" + } } ] }, @@ -50,6 +56,6 @@ "scopes": ["stellar:pubnet"] } }, - "platformVersion": "10.3.0", + "platformVersion": "11.1.1", "manifestVersion": "0.1" } diff --git a/packages/snap/src/context.ts b/packages/snap/src/context.ts index 5e894993..29638f0d 100644 --- a/packages/snap/src/context.ts +++ b/packages/snap/src/context.ts @@ -25,6 +25,7 @@ import { RefreshConfirmationContextHandler, } from './handlers/cronjob/refreshConfirmationContext'; import { SyncAccountsHandler } from './handlers/cronjob/syncAccounts'; +import { SyncAssetsHandler } from './handlers/cronjob/syncAssets'; import { TrackTransactionHandler } from './handlers/cronjob/trackTransaction'; import type { IKeyringRequestHandler } from './handlers/keyring'; import { @@ -105,7 +106,6 @@ const onChainAccountService = new OnChainAccountService({ logger, networkService, onChainAccountRepository, - assetMetadataService, }); const transactionService = new TransactionService({ @@ -113,7 +113,6 @@ const transactionService = new TransactionService({ transactionRepository, networkService, transactionBuilder, - assetMetadataService, }); const priceService = new PriceService({ @@ -131,6 +130,7 @@ const transactionScanService = new TransactionScanService({ const synchronizeService = new SynchronizeService({ logger, onChainAccountService, + assetMetadataService, transactionService, }); @@ -229,6 +229,11 @@ const syncAccountsHandler = new SyncAccountsHandler({ synchronizeService, }); +const syncAssetsHandler = new SyncAssetsHandler({ + logger, + synchronizeService, +}); + const cronjobMethodHandlers: Record< BackgroundEventMethod, ICronjobRequestHandler @@ -237,6 +242,7 @@ const cronjobMethodHandlers: Record< refreshConfirmationContextHandler, [BackgroundEventMethod.TrackTransaction]: trackTransactionHandler, [BackgroundEventMethod.SynchronizeAccounts]: syncAccountsHandler, + [BackgroundEventMethod.SynchronizeAssets]: syncAssetsHandler, }; const cronjobHandler = new CronjobHandler({ diff --git a/packages/snap/src/handlers/cronjob/api.test.ts b/packages/snap/src/handlers/cronjob/api.test.ts index 7c5ab3ed..8d9e9542 100644 --- a/packages/snap/src/handlers/cronjob/api.test.ts +++ b/packages/snap/src/handlers/cronjob/api.test.ts @@ -7,6 +7,8 @@ import { RefreshConfirmationContextJsonRpcRequestStruct, SyncAccountJsonRpcRequestStruct, SyncAccountParamsStruct, + SyncAssetsJsonRpcRequestStruct, + SyncAssetsParamsStruct, TrackTransactionJsonRpcRequestStruct, } from './api'; import { KnownCaip2ChainId } from '../../api'; @@ -126,6 +128,60 @@ describe('Cronjob API structs', () => { }); }); + describe('SyncAssetsParamsStruct', () => { + it('accepts empty object when params are omitted', () => { + const value = {}; + assert(value, SyncAssetsParamsStruct); + expect(value).toStrictEqual({}); + }); + + it('rejects unknown properties', () => { + expect(() => + assert({ extra: 'not-allowed' }, SyncAssetsParamsStruct), + ).toThrow(StructError); + }); + }); + + describe('SyncAssetsJsonRpcRequestStruct', () => { + it('accepts synchronize assets requests without params', () => { + const value = { + ...jsonRpcBase, + method: BackgroundEventMethod.SynchronizeAssets, + }; + assert(value, SyncAssetsJsonRpcRequestStruct); + expect(value).toStrictEqual({ + ...jsonRpcBase, + method: BackgroundEventMethod.SynchronizeAssets, + }); + }); + + it('accepts synchronize assets requests with empty params object', () => { + const value = { + ...jsonRpcBase, + method: BackgroundEventMethod.SynchronizeAssets, + params: {}, + }; + assert(value, SyncAssetsJsonRpcRequestStruct); + expect(value).toStrictEqual({ + ...jsonRpcBase, + method: BackgroundEventMethod.SynchronizeAssets, + params: {}, + }); + }); + + it('rejects wrong method for synchronize assets request', () => { + expect(() => + assert( + { + ...jsonRpcBase, + method: BackgroundEventMethod.SynchronizeAccounts, + }, + SyncAssetsJsonRpcRequestStruct, + ), + ).toThrow(StructError); + }); + }); + describe('RefreshConfirmationContextJsonRpcRequestStruct', () => { it('accepts refresh confirmation context requests', () => { const value = { diff --git a/packages/snap/src/handlers/cronjob/api.ts b/packages/snap/src/handlers/cronjob/api.ts index 4d831061..fcb638b1 100644 --- a/packages/snap/src/handlers/cronjob/api.ts +++ b/packages/snap/src/handlers/cronjob/api.ts @@ -36,6 +36,7 @@ export type ICronjobRequestHandler = { }; export enum BackgroundEventMethod { + SynchronizeAssets = 'synchronizeAssets', SynchronizeAccounts = 'synchronizeAccounts', TrackTransaction = 'trackTransaction', RefreshConfirmationContext = 'refreshConfirmationContext', @@ -100,6 +101,17 @@ export const SyncAccountJsonRpcRequestStruct = assign( }), ); +export const SyncAssetsParamsStruct = object({}); + +export const SyncAssetsJsonRpcRequestStruct = assign( + JsonRpcRequestStruct, + object({ + method: literal(BackgroundEventMethod.SynchronizeAssets), + // Omitted in declarative manifest cron jobs; handler ignores params if present. + params: optional(SyncAssetsParamsStruct), + }), +); + export const CronjobJsonRpcRequestStruct = object({ status: boolean(), }); @@ -118,6 +130,12 @@ export type SyncAccountJsonRpcRequest = Infer< export type SyncAccountParams = Infer; +export type SyncAssetsParams = Infer; + +export type SyncAssetsJsonRpcRequest = Infer< + typeof SyncAssetsJsonRpcRequestStruct +>; + export type RefreshConfirmationContextJsonRpcRequest = Infer< typeof RefreshConfirmationContextJsonRpcRequestStruct >; diff --git a/packages/snap/src/handlers/cronjob/syncAssets.test.ts b/packages/snap/src/handlers/cronjob/syncAssets.test.ts new file mode 100644 index 00000000..33d3ee86 --- /dev/null +++ b/packages/snap/src/handlers/cronjob/syncAssets.test.ts @@ -0,0 +1,43 @@ +import { BackgroundEventMethod } from './api'; +import { SyncAssetsHandler } from './syncAssets'; +import { KnownCaip2ChainId } from '../../api'; +import type { SynchronizeService } from '../../services/sync/SynchronizeService'; +import { logger } from '../../utils/logger'; + +jest.mock('../../utils/logger'); + +describe('SyncAssetsHandler', () => { + const setupTest = () => { + const synchronizeService: jest.Mocked< + Pick + > = { + synchronizeAssets: jest.fn(), + }; + + const handler = new SyncAssetsHandler({ + logger, + synchronizeService: synchronizeService as unknown as SynchronizeService, + }); + + return { + handler, + synchronizeService, + }; + }; + + it('synchronizes assets on mainnet', async () => { + const { handler, synchronizeService } = setupTest(); + + const request = { + jsonrpc: '2.0', + id: 1, + method: BackgroundEventMethod.SynchronizeAssets, + }; + + await handler.handle(request); + + expect(synchronizeService.synchronizeAssets).toHaveBeenCalledWith( + KnownCaip2ChainId.Mainnet, + ); + }); +}); diff --git a/packages/snap/src/handlers/cronjob/syncAssets.ts b/packages/snap/src/handlers/cronjob/syncAssets.ts new file mode 100644 index 00000000..4bf86593 --- /dev/null +++ b/packages/snap/src/handlers/cronjob/syncAssets.ts @@ -0,0 +1,40 @@ +import type { SyncAssetsJsonRpcRequest } from './api'; +import { SyncAssetsJsonRpcRequestStruct } from './api'; +import { CronjobBaseHandler } from './base'; +import { KnownCaip2ChainId } from '../../api'; +import type { SynchronizeService } from '../../services/sync/SynchronizeService'; +import { createPrefixedLogger } from '../../utils/logger'; +import type { ILogger } from '../../utils/logger'; + +export class SyncAssetsHandler extends CronjobBaseHandler { + readonly #synchronizeService: SynchronizeService; + + constructor({ + logger, + synchronizeService, + }: { + logger: ILogger; + synchronizeService: SynchronizeService; + }) { + const prefixedLogger = createPrefixedLogger(logger, '[SyncAssetsHandler]'); + super({ + logger: prefixedLogger, + requestStruct: SyncAssetsJsonRpcRequestStruct, + }); + this.#synchronizeService = synchronizeService; + } + + /** + * Declarative cron job with no `request.params` in `snap.manifest.json`. + * Asset metadata is only available on mainnet, so synchronization always uses + * mainnet scope regardless of the user's selected network. + * + * @param _request - Cron JSON-RPC request (method only). + */ + protected async handleCronJobRequest( + _request: SyncAssetsJsonRpcRequest, + ): Promise { + const scope = KnownCaip2ChainId.Mainnet; + await this.#synchronizeService.synchronizeAssets(scope); + } +} diff --git a/packages/snap/src/handlers/cronjob/trackTransaction.test.ts b/packages/snap/src/handlers/cronjob/trackTransaction.test.ts index 59e4d75e..4151e694 100644 --- a/packages/snap/src/handlers/cronjob/trackTransaction.test.ts +++ b/packages/snap/src/handlers/cronjob/trackTransaction.test.ts @@ -104,6 +104,7 @@ describe('TrackTransactionHandler', () => { synchronizeService: new SynchronizeService({ logger, onChainAccountService: {} as never, + assetMetadataService: {} as never, transactionService: {} as never, }), accountService: new AccountService({ diff --git a/packages/snap/src/services/asset-metadata/__mocks__/assets.fixtures.ts b/packages/snap/src/services/asset-metadata/__mocks__/assets.fixtures.ts index 773cb58b..7646f206 100644 --- a/packages/snap/src/services/asset-metadata/__mocks__/assets.fixtures.ts +++ b/packages/snap/src/services/asset-metadata/__mocks__/assets.fixtures.ts @@ -9,6 +9,7 @@ import { State } from '../../state'; import type { AssetMetadataByAssetId, KeyringAssetMetadataByAssetId, + StellarAssetMetadata, } from '../api'; import { AssetMetadataRepository } from '../AssetMetadataRepository'; import { AssetMetadataService } from '../AssetMetadataService'; @@ -65,6 +66,16 @@ export const generateMockStellarAssetMetadata = (): AssetMetadataByAssetId => { } as AssetMetadataByAssetId; }; +export const getMockSep41Assets = (): StellarAssetMetadata[] => { + const metadata = generateMockStellarAssetMetadata(); + const usdcSep41 = metadata[USDC_SEP41]; + const usdtSep41 = metadata[USDT_SEP41]; + if (!usdcSep41 || !usdtSep41) { + throw new Error('expected SEP-41 assets in mock metadata'); + } + return [usdcSep41, usdtSep41]; +}; + export const generateMockKeyringAssetMetadata = (): KeyringAssetMetadataByAssetId => { return { diff --git a/packages/snap/src/services/on-chain-account/OnChainAccountService.test.ts b/packages/snap/src/services/on-chain-account/OnChainAccountService.test.ts index 279a9069..372186e3 100644 --- a/packages/snap/src/services/on-chain-account/OnChainAccountService.test.ts +++ b/packages/snap/src/services/on-chain-account/OnChainAccountService.test.ts @@ -233,11 +233,13 @@ describe('OnChainAccountService', () => { await onChainAccountService.synchronize( activatedAccountPairs, KnownCaip2ChainId.Mainnet, + [], ); expect(synchronizeSpy).toHaveBeenCalledWith( activatedAccountPairs, KnownCaip2ChainId.Mainnet, + [], ); }); }); diff --git a/packages/snap/src/services/on-chain-account/OnChainAccountService.ts b/packages/snap/src/services/on-chain-account/OnChainAccountService.ts index f13de981..4bed1597 100644 --- a/packages/snap/src/services/on-chain-account/OnChainAccountService.ts +++ b/packages/snap/src/services/on-chain-account/OnChainAccountService.ts @@ -5,7 +5,7 @@ import type { ILogger } from '../../utils'; import { assertSameAddress } from '../account/utils'; import { AccountNotActivatedException, type NetworkService } from '../network'; import type { OnChainAccountRepository } from './OnChainAccountRepository'; -import type { AssetMetadataService } from '../asset-metadata/AssetMetadataService'; +import type { StellarAssetMetadata } from '../asset-metadata'; import type { ActivatedAccountPair } from '../sync/api'; /** @@ -22,12 +22,10 @@ export class OnChainAccountService { constructor({ networkService, onChainAccountRepository, - assetMetadataService, logger, }: { networkService: NetworkService; onChainAccountRepository: OnChainAccountRepository; - assetMetadataService: AssetMetadataService; logger: ILogger; }) { this.#networkService = networkService; @@ -35,7 +33,6 @@ export class OnChainAccountService { new OnChainAccountSynchronizeService({ networkService, onChainAccountRepository, - assetMetadataService, logger, }); this.#onChainAccountRepository = onChainAccountRepository; @@ -114,14 +111,17 @@ export class OnChainAccountService { * * @param activatedAccountPairs - Activated account pairs to synchronize. * @param scope - CAIP-2 network. + * @param sep41Assets - Preloaded SEP-41 assets from {@link SynchronizeService}. */ async synchronize( activatedAccountPairs: ActivatedAccountPair[], scope: KnownCaip2ChainId, + sep41Assets: StellarAssetMetadata[], ): Promise { await this.#onChainAccountSynchronizeService.synchronize( activatedAccountPairs, scope, + sep41Assets, ); } } diff --git a/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.test.ts b/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.test.ts index b2971a70..125df43d 100644 --- a/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.test.ts +++ b/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.test.ts @@ -27,9 +27,8 @@ import { USDT_SEP41, USDC_SEP41, generateMockStellarAssetMetadata, + getMockSep41Assets, } from '../asset-metadata/__mocks__/assets.fixtures'; -import type { StellarAssetMetadata } from '../asset-metadata/api'; -import { AssetMetadataService } from '../asset-metadata/AssetMetadataService'; import { NetworkService } from '../network'; import type { ActivatedAccountPair } from '../sync/api'; @@ -115,6 +114,7 @@ describe('OnChainAccountSynchronizeService', () => { ); const sep41Id = USDC_SEP41 as KnownCaip19Sep41AssetId; const backupSep41Id = USDT_SEP41 as KnownCaip19Sep41AssetId; + const mockSep41Assets = getMockSep41Assets(); const getNetworkServiceSpies = () => ({ loadOnChainAccountSpy: jest.spyOn( @@ -170,28 +170,6 @@ describe('OnChainAccountSynchronizeService', () => { const setupTest = () => { jest.mocked(emitSnapKeyringEvent).mockResolvedValue(undefined); - const metadata = generateMockStellarAssetMetadata(); - const usdcSep41Row = metadata[USDC_SEP41]; - if (!usdcSep41Row) { - throw new Error('expected USDC_SEP41 in mock asset metadata'); - } - const usdtSep41Row = metadata[USDT_SEP41]; - if (!usdtSep41Row) { - throw new Error('expected USDT_SEP41 in mock asset metadata'); - } - jest - .spyOn(AssetMetadataService.prototype, 'getPersistedSep41AssetsMetadata') - .mockResolvedValue([usdcSep41Row, usdtSep41Row]); - // getKey('assets') in tests does not merge defaultState, so getAllByScope is empty unless mocked. - jest - .spyOn(AssetMetadataService.prototype, 'getAllByScope') - .mockImplementation(async (scope) => { - const byAssetId = generateMockStellarAssetMetadata(); - return Object.values(byAssetId).filter( - (asset): asset is StellarAssetMetadata => - asset !== undefined && asset.chainId === scope, - ); - }); }; const buildActivatedAccountPair = ( @@ -235,7 +213,7 @@ describe('OnChainAccountSynchronizeService', () => { const { onChainAccountService, saveManySpy } = setupSynchronizeService(); - await onChainAccountService.synchronize([], KnownCaip2ChainId.Mainnet); + await onChainAccountService.synchronize([], KnownCaip2ChainId.Mainnet, []); expect(saveManySpy).not.toHaveBeenCalled(); }); @@ -257,6 +235,7 @@ describe('OnChainAccountSynchronizeService', () => { await onChainAccountService.synchronize( [activatedAccountPair], KnownCaip2ChainId.Mainnet, + mockSep41Assets, ); const saved = getSavedSnapshotFromFirstSave( @@ -282,9 +261,7 @@ describe('OnChainAccountSynchronizeService', () => { assuredUsdcSep41Row.units = [ { ...assuredUsdcSep41Row.units[0], decimals: 2 }, ]; - jest - .spyOn(AssetMetadataService.prototype, 'getPersistedSep41AssetsMetadata') - .mockResolvedValue([assuredUsdcSep41Row, assuredUsdtSep41Row]); + const customSep41Assets = [assuredUsdcSep41Row, assuredUsdtSep41Row]; const { signer, keyringAccount, binding } = setupOnChainAccountWithBalance('entropy-sync-2'); @@ -312,7 +289,11 @@ describe('OnChainAccountSynchronizeService', () => { [keyringAccount.id]: binding, }); - await onChainAccountService.synchronize([pair], KnownCaip2ChainId.Mainnet); + await onChainAccountService.synchronize( + [pair], + KnownCaip2ChainId.Mainnet, + customSep41Assets, + ); expect(emitSnapKeyringEventSpy).toHaveBeenCalledTimes(2); expect(emitSnapKeyringEventSpy).toHaveBeenNthCalledWith( @@ -382,6 +363,7 @@ describe('OnChainAccountSynchronizeService', () => { await onChainAccountService.synchronize( [buildActivatedAccountPair(keyringAccount, onChainAccount)], KnownCaip2ChainId.Mainnet, + mockSep41Assets, ); expect(emitSnapKeyringEventSpy).toHaveBeenCalledTimes(2); @@ -446,10 +428,26 @@ describe('OnChainAccountSynchronizeService', () => { const { emitSnapKeyringEventSpy } = getKeyringEventSpies(); const { onChainAccountService } = setupSynchronizeService(); - await onChainAccountService.synchronize([pair], KnownCaip2ChainId.Mainnet); - await onChainAccountService.synchronize([pair], KnownCaip2ChainId.Mainnet); - await onChainAccountService.synchronize([pair], KnownCaip2ChainId.Mainnet); - await onChainAccountService.synchronize([pair], KnownCaip2ChainId.Mainnet); + await onChainAccountService.synchronize( + [pair], + KnownCaip2ChainId.Mainnet, + mockSep41Assets, + ); + await onChainAccountService.synchronize( + [pair], + KnownCaip2ChainId.Mainnet, + mockSep41Assets, + ); + await onChainAccountService.synchronize( + [pair], + KnownCaip2ChainId.Mainnet, + mockSep41Assets, + ); + await onChainAccountService.synchronize( + [pair], + KnownCaip2ChainId.Mainnet, + mockSep41Assets, + ); const balanceEventCalls = emitSnapKeyringEventSpy.mock.calls.filter( (call) => isKeyringEmitCall(call, KeyringEvent.AccountBalancesUpdated), @@ -560,6 +558,7 @@ describe('OnChainAccountSynchronizeService', () => { await onChainAccountService.synchronize( [sync1Pair], KnownCaip2ChainId.Mainnet, + mockSep41Assets, ); // sync 1 const afterSync1 = saveManySpy.mock.calls[0]?.[0]?.[keyringAccount.id]; expect( @@ -569,6 +568,7 @@ describe('OnChainAccountSynchronizeService', () => { await onChainAccountService.synchronize( [syncLaterPair], KnownCaip2ChainId.Mainnet, + mockSep41Assets, ); // sync 2 (missed by client) expect(saveManySpy.mock.calls).toHaveLength(2); const afterSync2 = saveManySpy.mock.calls[1]?.[0]?.[keyringAccount.id]; @@ -586,10 +586,12 @@ describe('OnChainAccountSynchronizeService', () => { await onChainAccountService.synchronize( [syncLaterPair], KnownCaip2ChainId.Mainnet, + mockSep41Assets, ); // sync 3 (missed by client) await onChainAccountService.synchronize( [syncLaterPair], KnownCaip2ChainId.Mainnet, + mockSep41Assets, ); // sync 4 const balanceEventCalls = emitSnapKeyringEventSpy.mock.calls @@ -768,18 +770,22 @@ describe('OnChainAccountSynchronizeService', () => { await onChainAccountService.synchronize( [sync1Pair], KnownCaip2ChainId.Mainnet, + mockSep41Assets, ); await onChainAccountService.synchronize( [sync2Pair], KnownCaip2ChainId.Mainnet, + mockSep41Assets, ); await onChainAccountService.synchronize( [sync3And4Pair], KnownCaip2ChainId.Mainnet, + mockSep41Assets, ); await onChainAccountService.synchronize( [sync3And4Pair], KnownCaip2ChainId.Mainnet, + mockSep41Assets, ); expect(saveManySpy).toHaveBeenCalledTimes(4); @@ -877,7 +883,11 @@ describe('OnChainAccountSynchronizeService', () => { // Mock no persisted snapshot is found for the account. findByKeyringAccountIdsSpy.mockResolvedValue({ [keyringAccount.id]: null }); - await onChainAccountService.synchronize([pair], KnownCaip2ChainId.Mainnet); + await onChainAccountService.synchronize( + [pair], + KnownCaip2ChainId.Mainnet, + mockSep41Assets, + ); expect(setAssetSpy).not.toHaveBeenCalled(); }); @@ -909,7 +919,11 @@ describe('OnChainAccountSynchronizeService', () => { [keyringAccount.id]: withPersistedSep41, }); - await onChainAccountService.synchronize([pair], KnownCaip2ChainId.Mainnet); + await onChainAccountService.synchronize( + [pair], + KnownCaip2ChainId.Mainnet, + mockSep41Assets, + ); const saved = getSavedSnapshotFromFirstSave(saveManySpy, keyringAccount.id); const persistedSep41Row = saved.balances.find((b) => b.assetId === sep41Id); @@ -962,7 +976,11 @@ describe('OnChainAccountSynchronizeService', () => { [keyringAccount.id]: withPersistedBackupSep41, }); - await onChainAccountService.synchronize([pair], KnownCaip2ChainId.Mainnet); + await onChainAccountService.synchronize( + [pair], + KnownCaip2ChainId.Mainnet, + mockSep41Assets, + ); const saved = getSavedSnapshotFromFirstSave(saveManySpy, keyringAccount.id); const resolvedSep41Row = saved.balances.find((b) => b.assetId === sep41Id); @@ -990,7 +1008,11 @@ describe('OnChainAccountSynchronizeService', () => { saveManySpy.mockRejectedValue(new Error('saveMany failed')); await expect( - onChainAccountService.synchronize([pair], KnownCaip2ChainId.Mainnet), + onChainAccountService.synchronize( + [pair], + KnownCaip2ChainId.Mainnet, + mockSep41Assets, + ), ).rejects.toThrow('saveMany failed'); expect(saveManySpy).toHaveBeenCalled(); diff --git a/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.ts b/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.ts index eb4b8307..ab87abb0 100644 --- a/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.ts +++ b/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.ts @@ -24,7 +24,6 @@ import { toDisplayBalance, } from '../../utils'; import type { StellarAssetMetadata } from '../asset-metadata'; -import type { AssetMetadataService } from '../asset-metadata/AssetMetadataService'; import type { NetworkService } from '../network'; import type { ActivatedAccountPair } from '../sync/api'; @@ -51,8 +50,6 @@ export class OnChainAccountSynchronizeService { readonly #onChainAccountRepository: OnChainAccountRepository; - readonly #assetMetadataService: AssetMetadataService; - readonly #logger: ILogger; /** Serializes full sync runs; see class JSDoc. */ @@ -61,17 +58,14 @@ export class OnChainAccountSynchronizeService { constructor({ networkService, onChainAccountRepository, - assetMetadataService, logger, }: { networkService: NetworkService; onChainAccountRepository: OnChainAccountRepository; - assetMetadataService: AssetMetadataService; logger: ILogger; }) { this.#networkService = networkService; this.#onChainAccountRepository = onChainAccountRepository; - this.#assetMetadataService = assetMetadataService; this.#logger = createPrefixedLogger( logger, '[💼 OnChainAccountSynchronizeService]', @@ -86,10 +80,12 @@ export class OnChainAccountSynchronizeService { * * @param activatedAccountPairs - Activated keyring/on-chain account pairs to sync for `scope`. * @param scope - CAIP-2 network. + * @param sep41Assets - Preloaded SEP-41 assets from {@link SynchronizeService}. */ async synchronize( activatedAccountPairs: ActivatedAccountPair[], scope: KnownCaip2ChainId, + sep41Assets: StellarAssetMetadata[], ): Promise { if (activatedAccountPairs.length === 0) { this.#logger.debug('No accounts to synchronize'); @@ -123,8 +119,6 @@ export class OnChainAccountSynchronizeService { this.#logger.debug('Load SEP-41 token balances'); let sep41BalanceFetchResult: Sep41BalanceFetchResult | null = null; try { - const sep41Assets = - await this.#assetMetadataService.fetchSep41AssetsOrSyncOnce(scope); sep41BalanceFetchResult = await this.#synchronizeSep41AssetBalances({ stellarAccountIds, scope, diff --git a/packages/snap/src/services/on-chain-account/__mocks__/onChainAccount.fixtures.ts b/packages/snap/src/services/on-chain-account/__mocks__/onChainAccount.fixtures.ts index 4d8cd14a..fe179a36 100644 --- a/packages/snap/src/services/on-chain-account/__mocks__/onChainAccount.fixtures.ts +++ b/packages/snap/src/services/on-chain-account/__mocks__/onChainAccount.fixtures.ts @@ -6,7 +6,6 @@ import type { KnownCaip2ChainId } from '../../../api'; import { logger, noOpLogger } from '../../../utils/logger'; import { AccountService } from '../../account/AccountService'; import { AccountsRepository } from '../../account/AccountsRepository'; -import { createMockAssetMetadataService } from '../../asset-metadata/__mocks__/assets.fixtures'; import { InMemoryCache } from '../../cache'; import { NetworkService } from '../../network'; import { State } from '../../state/State'; @@ -171,12 +170,10 @@ export function mockOnChainAccountService() { cache: new InMemoryCache(noOpLogger), }); const onChainAccountRepository = new OnChainAccountRepository(state); - const { service: assetMetadataService } = createMockAssetMetadataService(); const onChainAccountService = new OnChainAccountService({ logger, networkService, onChainAccountRepository, - assetMetadataService, }); return { diff --git a/packages/snap/src/services/sync/SynchronizeService.test.ts b/packages/snap/src/services/sync/SynchronizeService.test.ts index 664ca6c3..26be65eb 100644 --- a/packages/snap/src/services/sync/SynchronizeService.test.ts +++ b/packages/snap/src/services/sync/SynchronizeService.test.ts @@ -4,6 +4,11 @@ import { AppConfig } from '../../config'; import { logger } from '../../utils/logger'; import type { StellarKeyringAccount } from '../account'; import { generateMockStellarKeyringAccounts } from '../account/__mocks__/account.fixtures'; +import { AssetMetadataService } from '../asset-metadata'; +import { + createMockAssetMetadataService, + getMockSep41Assets, +} from '../asset-metadata/__mocks__/assets.fixtures'; import { AccountNotActivatedException } from '../network'; import { createMockAccountWithBalances, @@ -22,9 +27,15 @@ describe('SynchronizeService', () => { const setup = () => { const { onChainAccountService } = mockOnChainAccountService(); const { transactionService } = createMockTransactionService(); + const { service: assetMetadataService } = createMockAssetMetadataService(); + const sep41Assets = getMockSep41Assets(); + const fetchSep41AssetsOrSyncOnceSpy = jest + .spyOn(AssetMetadataService.prototype, 'fetchSep41AssetsOrSyncOnce') + .mockResolvedValue(sep41Assets); const service = new SynchronizeService({ logger, onChainAccountService, + assetMetadataService, transactionService, }); @@ -32,8 +43,15 @@ describe('SynchronizeService', () => { service, onChainAccountService, transactionService, - onChainSynchronizeSpy: jest.spyOn(onChainAccountService, 'synchronize'), - transactionSynchronizeSpy: jest.spyOn(transactionService, 'synchronize'), + assetMetadataService, + sep41Assets, + fetchSep41AssetsOrSyncOnceSpy, + onChainSynchronizeSpy: jest + .spyOn(onChainAccountService, 'synchronize') + .mockResolvedValue(undefined), + transactionSynchronizeSpy: jest + .spyOn(transactionService, 'synchronize') + .mockResolvedValue(undefined), resolveOnChainAccountSpy: jest.spyOn( onChainAccountService, 'resolveOnChainAccount', @@ -47,11 +65,13 @@ describe('SynchronizeService', () => { onChainSynchronizeSpy, transactionSynchronizeSpy, resolveOnChainAccountSpy, + fetchSep41AssetsOrSyncOnceSpy, } = setup(); await service.synchronize([]); expect(resolveOnChainAccountSpy).not.toHaveBeenCalled(); + expect(fetchSep41AssetsOrSyncOnceSpy).not.toHaveBeenCalled(); expect(onChainSynchronizeSpy).not.toHaveBeenCalled(); expect(transactionSynchronizeSpy).not.toHaveBeenCalled(); }); @@ -62,6 +82,8 @@ describe('SynchronizeService', () => { onChainSynchronizeSpy, transactionSynchronizeSpy, resolveOnChainAccountSpy, + fetchSep41AssetsOrSyncOnceSpy, + sep41Assets, } = setup(); const [account] = generateMockStellarKeyringAccounts( 1, @@ -90,13 +112,16 @@ describe('SynchronizeService', () => { syncTransactions: true, }); + expect(fetchSep41AssetsOrSyncOnceSpy).toHaveBeenCalledWith(scope); expect(onChainSynchronizeSpy).toHaveBeenCalledWith( [{ keyringAccount: account, onChainAccount }], scope, + sep41Assets, ); expect(transactionSynchronizeSpy).toHaveBeenCalledWith( [{ keyringAccount: account, onChainAccount }], scope, + sep41Assets, ); }); @@ -106,6 +131,7 @@ describe('SynchronizeService', () => { onChainSynchronizeSpy, transactionSynchronizeSpy, resolveOnChainAccountSpy, + sep41Assets, } = setup(); const [account] = generateMockStellarKeyringAccounts( 1, @@ -118,8 +144,12 @@ describe('SynchronizeService', () => { await service.synchronize([account], { scope }); - expect(onChainSynchronizeSpy).toHaveBeenCalledWith([], scope); - expect(transactionSynchronizeSpy).toHaveBeenCalledWith([], scope); + expect(onChainSynchronizeSpy).toHaveBeenCalledWith([], scope, sep41Assets); + expect(transactionSynchronizeSpy).toHaveBeenCalledWith( + [], + scope, + sep41Assets, + ); }); it('respects syncAccounts and syncTransactions options', async () => { @@ -128,6 +158,7 @@ describe('SynchronizeService', () => { onChainSynchronizeSpy, transactionSynchronizeSpy, resolveOnChainAccountSpy, + sep41Assets, } = setup(); const [account] = generateMockStellarKeyringAccounts( 1, @@ -160,6 +191,119 @@ describe('SynchronizeService', () => { expect(transactionSynchronizeSpy).toHaveBeenCalledWith( [{ keyringAccount: account, onChainAccount }], AppConfig.selectedNetwork, + sep41Assets, ); }); + + it('continues sync when SEP-41 asset loading fails', async () => { + const { + service, + onChainSynchronizeSpy, + transactionSynchronizeSpy, + resolveOnChainAccountSpy, + fetchSep41AssetsOrSyncOnceSpy, + } = setup(); + const [account] = generateMockStellarKeyringAccounts( + 1, + 'sync-sep41-failure-entropy', + ) as [StellarKeyringAccount]; + const onChainAccount = new OnChainAccount( + createMockAccountWithBalances(account.address, '1', { + ...DEFAULT_MOCK_ACCOUNT_WITH_BALANCES, + nativeBalance: 10, + }), + scope, + horizonSource( + createMockAccountWithBalances(account.address, '1', { + ...DEFAULT_MOCK_ACCOUNT_WITH_BALANCES, + nativeBalance: 10, + }), + scope, + ), + ); + + fetchSep41AssetsOrSyncOnceSpy.mockRejectedValue( + new Error('token api down'), + ); + resolveOnChainAccountSpy.mockResolvedValue(onChainAccount); + + await service.synchronize([account], { scope }); + + expect(onChainSynchronizeSpy).toHaveBeenCalledWith( + [{ keyringAccount: account, onChainAccount }], + scope, + [], + ); + expect(transactionSynchronizeSpy).toHaveBeenCalledWith( + [{ keyringAccount: account, onChainAccount }], + scope, + [], + ); + }); + + it('continues sync when account synchronization fails', async () => { + const { + service, + onChainSynchronizeSpy, + transactionSynchronizeSpy, + resolveOnChainAccountSpy, + sep41Assets, + } = setup(); + const [account] = generateMockStellarKeyringAccounts( + 1, + 'sync-account-failure-entropy', + ) as [StellarKeyringAccount]; + const onChainAccount = new OnChainAccount( + createMockAccountWithBalances(account.address, '1', { + ...DEFAULT_MOCK_ACCOUNT_WITH_BALANCES, + nativeBalance: 10, + }), + scope, + horizonSource( + createMockAccountWithBalances(account.address, '1', { + ...DEFAULT_MOCK_ACCOUNT_WITH_BALANCES, + nativeBalance: 10, + }), + scope, + ), + ); + + resolveOnChainAccountSpy.mockResolvedValue(onChainAccount); + onChainSynchronizeSpy.mockRejectedValue(new Error('account sync failed')); + + await service.synchronize([account], { scope }); + + expect(onChainSynchronizeSpy).toHaveBeenCalledWith( + [{ keyringAccount: account, onChainAccount }], + scope, + sep41Assets, + ); + expect(transactionSynchronizeSpy).toHaveBeenCalledWith( + [{ keyringAccount: account, onChainAccount }], + scope, + sep41Assets, + ); + }); + + it('delegates asset synchronization to AssetMetadataService', async () => { + const { service, assetMetadataService } = setup(); + const synchronizeSpy = jest + .spyOn(assetMetadataService, 'synchronize') + .mockResolvedValue(undefined); + + await service.synchronizeAssets(scope); + + expect(synchronizeSpy).toHaveBeenCalledWith(scope); + }); + + it('logs and does not throw when asset synchronization fails', async () => { + const { service, assetMetadataService } = setup(); + const synchronizeSpy = jest + .spyOn(assetMetadataService, 'synchronize') + .mockRejectedValue(new Error('token api down')); + + await service.synchronizeAssets(scope); + + expect(synchronizeSpy).toHaveBeenCalledWith(scope); + }); }); diff --git a/packages/snap/src/services/sync/SynchronizeService.ts b/packages/snap/src/services/sync/SynchronizeService.ts index 03a690e2..e597518b 100644 --- a/packages/snap/src/services/sync/SynchronizeService.ts +++ b/packages/snap/src/services/sync/SynchronizeService.ts @@ -3,11 +3,20 @@ import { AppConfig } from '../../config'; import type { ILogger } from '../../utils/logger'; import { createPrefixedLogger } from '../../utils/logger'; import type { StellarKeyringAccount } from '../account'; +import type { + AssetMetadataService, + StellarAssetMetadata, +} from '../asset-metadata'; import { AccountNotActivatedException } from '../network'; import type { OnChainAccountService } from '../on-chain-account'; import type { TransactionService } from '../transaction'; import type { ActivatedAccountPair, SynchronizeOptions } from './api'; +/** + * Orchestrates Stellar wallet synchronization: on-chain account snapshots, transaction + * history, and asset metadata. Loads SEP-41 assets once per account sync run and passes + * them to downstream services. + */ export class SynchronizeService { readonly #logger: ILogger; @@ -15,20 +24,35 @@ export class SynchronizeService { readonly #transactionService: TransactionService; + readonly #assetMetadataService: AssetMetadataService; + constructor({ logger, onChainAccountService, + assetMetadataService, transactionService, }: { logger: ILogger; onChainAccountService: OnChainAccountService; + assetMetadataService: AssetMetadataService; transactionService: TransactionService; }) { this.#logger = createPrefixedLogger(logger, '[🔄 SynchronizeService]'); this.#onChainAccountService = onChainAccountService; this.#transactionService = transactionService; + this.#assetMetadataService = assetMetadataService; } + /** + * Synchronizes activated keyring accounts for the given scope. + * + * Loads on-chain account pairs and SEP-41 asset metadata in parallel, then runs enabled + * sync tasks (accounts and/or transactions) concurrently. Unfunded accounts are skipped; + * per-task failures are logged and do not fail the overall run. + * + * @param accounts - Keyring accounts to synchronize. + * @param options - Optional flags for scope and which sync steps to run. Defaults to synchronizing accounts and transactions on the selected network. + */ async synchronize( accounts: StellarKeyringAccount[], options?: SynchronizeOptions, @@ -44,44 +68,88 @@ export class SynchronizeService { return; } - this.#logger.debug('number of accounts to synchronize', { - noOfAccounts: accounts.length, - }); - - const activatedAccountPairs = await this.#loadActivatedPairs( - accounts, - scope, - ); + // Both async loads are fail-safe, so we can use Promise.all. + const [activatedAccountPairs, sep41Assets] = await Promise.all([ + this.#loadActivatedPairs(accounts, scope), + this.#loadSep41Assets(scope), + ]); - this.#logger.debug('number of activated account pairs', { - noOfAccounts: activatedAccountPairs.length, - }); + const tasks: { name: string; task: Promise }[] = []; if (syncAccounts) { - try { - await this.#onChainAccountService.synchronize( + tasks.push({ + name: 'synchronize accounts', + task: this.#onChainAccountService.synchronize( activatedAccountPairs, scope, - ); - } catch (error: unknown) { - this.#logger.logErrorWithDetails('Failed to synchronize accounts', { - error, - }); - } + sep41Assets, + ), + }); } - // we sync transactions after accounts to ensure that the asset metadata is synced before the transactions are mapped. if (syncTransactions) { - try { - await this.#transactionService.synchronize( + tasks.push({ + name: 'synchronize transactions', + task: this.#transactionService.synchronize( activatedAccountPairs, scope, - ); - } catch (error: unknown) { - this.#logger.logErrorWithDetails('Failed to synchronize transactions', { - error, + sep41Assets, + ), + }); + } + + if (tasks.length === 0) { + return; + } + + const results = await Promise.allSettled( + tasks.map(async ({ task }) => task), + ); + + results.forEach((result, index) => { + if (result.status === 'rejected') { + const taskName = tasks[index]?.name ?? 'synchronize'; + this.#logger.logErrorWithDetails(`Failed to ${taskName}`, { + error: result.reason, }); } + }); + } + + /** + * Fetches and persists the full asset catalog from the token API for the given scope. + * + * Intended for the declarative `synchronizeAssets` cron job. Failures are logged and + * do not propagate to the caller. + * + * @param scope - CAIP-2 network to synchronize asset metadata for. + */ + async synchronizeAssets(scope: KnownCaip2ChainId): Promise { + try { + await this.#assetMetadataService.synchronize(scope); + } catch (error: unknown) { + this.#logger.logErrorWithDetails('Failed to synchronize assets', { + error, + }); + } + } + + /** + * Loads the full asset catalog from the token API for the given scope. + * + * @param scope - CAIP-2 network to load asset metadata for. + * @returns The full asset catalog. + */ + async #loadSep41Assets( + scope: KnownCaip2ChainId, + ): Promise { + try { + return await this.#assetMetadataService.fetchSep41AssetsOrSyncOnce(scope); + } catch (error: unknown) { + this.#logger.logErrorWithDetails('Failed to load SEP-41 assets', { + error, + }); + return []; } } @@ -96,6 +164,10 @@ export class SynchronizeService { accounts: StellarKeyringAccount[], scope: KnownCaip2ChainId, ): Promise { + this.#logger.debug('number of accounts to synchronize', { + noOfAccounts: accounts.length, + }); + const pairs: ActivatedAccountPair[] = []; const results = await Promise.allSettled( @@ -122,6 +194,10 @@ export class SynchronizeService { }); }); + this.#logger.debug('number of activated account pairs', { + noOfAccounts: pairs.length, + }); + return pairs; } } diff --git a/packages/snap/src/services/transaction/TransactionService.ts b/packages/snap/src/services/transaction/TransactionService.ts index 882bb1a7..608a5287 100644 --- a/packages/snap/src/services/transaction/TransactionService.ts +++ b/packages/snap/src/services/transaction/TransactionService.ts @@ -29,7 +29,7 @@ import type { import { getSnapProvider, isSep41Id, isSlip44Id } from '../../utils'; import type { ILogger } from '../../utils/logger'; import { createPrefixedLogger } from '../../utils/logger'; -import type { AssetMetadataService } from '../asset-metadata'; +import type { StellarAssetMetadata } from '../asset-metadata'; import type { NetworkService } from '../network'; import { AccountNotActivatedException, @@ -57,13 +57,11 @@ export class TransactionService { transactionRepository, networkService, transactionBuilder, - assetMetadataService, }: { logger: ILogger; transactionRepository: TransactionRepository; networkService: NetworkService; transactionBuilder: TransactionBuilder; - assetMetadataService: AssetMetadataService; }) { this.#logger = createPrefixedLogger(logger, '[🧾 TransactionService]'); this.#transactionRepository = transactionRepository; @@ -78,7 +76,6 @@ export class TransactionService { networkService, transactionRepository, transactionMapper, - assetMetadataService, logger, }); } @@ -641,14 +638,17 @@ export class TransactionService { * * @param activatedAccountPairs - Activated keyring/on-chain account pairs whose history will be synced. * @param scope - Network scope for Horizon fetches. + * @param sep41Assets - Preloaded SEP-41 assets from {@link SynchronizeService}. */ async synchronize( activatedAccountPairs: ActivatedAccountPair[], scope: KnownCaip2ChainId, + sep41Assets: StellarAssetMetadata[], ): Promise { await this.#transactionSynchronizeService.synchronize( activatedAccountPairs, scope, + sep41Assets, ); } diff --git a/packages/snap/src/services/transaction/TransactionSynchronizeService.test.ts b/packages/snap/src/services/transaction/TransactionSynchronizeService.test.ts index 874f96c3..aad9749f 100644 --- a/packages/snap/src/services/transaction/TransactionSynchronizeService.test.ts +++ b/packages/snap/src/services/transaction/TransactionSynchronizeService.test.ts @@ -17,8 +17,6 @@ import { TransactionSynchronizeService } from './TransactionSynchronizeService'; import { logger } from '../../utils/logger'; import { getSnapProvider } from '../../utils/snap'; import { generateStellarKeyringAccount } from '../account/__mocks__/account.fixtures'; -import { createMockAssetMetadataService } from '../asset-metadata/__mocks__/assets.fixtures'; -import { AssetMetadataService } from '../asset-metadata/AssetMetadataService'; import { createMemoryCache } from '../cache/__mocks__/cache.fixtures'; import { NetworkService } from '../network'; import { @@ -85,10 +83,6 @@ describe('TransactionSynchronizeService', () => { const setup = () => { const { cache } = createMemoryCache(); const networkService = new NetworkService({ logger, cache }); - const { service: assetMetadataService } = createMockAssetMetadataService(); - jest - .spyOn(AssetMetadataService.prototype, 'fetchSep41AssetsOrSyncOnce') - .mockResolvedValue([]); const transactionRepository = new TransactionRepository( new State({ encrypted: false, @@ -106,14 +100,12 @@ describe('TransactionSynchronizeService', () => { networkService, transactionRepository, transactionMapper, - assetMetadataService, logger, }); return { service, networkService, - assetMetadataService, transactionRepository, getTransactionsSpy: jest.spyOn(networkService, 'getTransactions'), getTransactionSpy: jest.spyOn(networkService, 'getTransaction'), @@ -170,7 +162,7 @@ describe('TransactionSynchronizeService', () => { emitSnapKeyringEventSpy, } = setup(); - await service.synchronize([], scope); + await service.synchronize([], scope, []); expect(getTransactionsSpy).not.toHaveBeenCalled(); expect(saveManySpy).not.toHaveBeenCalled(); @@ -202,7 +194,7 @@ describe('TransactionSynchronizeService', () => { }); getTransactionsSpy.mockResolvedValue([onChainTransaction]); - await service.synchronize([activatedAccountPair], scope); + await service.synchronize([activatedAccountPair], scope, []); expect(getTransactionsSpy).toHaveBeenCalledWith( expect.objectContaining({ @@ -276,7 +268,7 @@ describe('TransactionSynchronizeService', () => { getTransactionsSpy.mockResolvedValue([]); getTransactionSpy.mockResolvedValue(onChainTransaction); - await service.synchronize([activatedAccountPair], scope); + await service.synchronize([activatedAccountPair], scope, []); expect(getTransactionsSpy).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/packages/snap/src/services/transaction/TransactionSynchronizeService.ts b/packages/snap/src/services/transaction/TransactionSynchronizeService.ts index 07e4728a..d0f83ae4 100644 --- a/packages/snap/src/services/transaction/TransactionSynchronizeService.ts +++ b/packages/snap/src/services/transaction/TransactionSynchronizeService.ts @@ -22,10 +22,7 @@ import { TransactionOrder } from './api'; import type { Transaction } from './Transaction'; import type { TransactionMapper } from './TransactionMapper'; import type { TransactionRepository } from './TransactionRepository'; -import type { - AssetMetadataService, - StellarAssetMetadata, -} from '../asset-metadata'; +import type { StellarAssetMetadata } from '../asset-metadata'; import type { NetworkService } from '../network'; import { isPendingTransactionStatus } from './utils'; import type { ActivatedAccountPair } from '../sync/api'; @@ -67,8 +64,6 @@ export class TransactionSynchronizeService { readonly #transactionRepository: TransactionRepository; - readonly #assetMetadataService: AssetMetadataService; - readonly #networkService: NetworkService; readonly #logger: ILogger; @@ -80,19 +75,16 @@ export class TransactionSynchronizeService { networkService, transactionMapper, transactionRepository, - assetMetadataService, logger, }: { networkService: NetworkService; transactionRepository: TransactionRepository; transactionMapper: TransactionMapper; - assetMetadataService: AssetMetadataService; logger: ILogger; }) { this.#networkService = networkService; this.#transactionRepository = transactionRepository; this.#transactionMapper = transactionMapper; - this.#assetMetadataService = assetMetadataService; this.#logger = createPrefixedLogger( logger, '[💼 TransactionSynchronizeService]', @@ -102,6 +94,7 @@ export class TransactionSynchronizeService { async synchronize( activatedAccountPairs: ActivatedAccountPair[], scope: KnownCaip2ChainId, + sep41Assets: StellarAssetMetadata[], ): Promise { if (activatedAccountPairs.length === 0) { this.#logger.debug('No accounts to synchronize'); @@ -120,6 +113,7 @@ export class TransactionSynchronizeService { const context = await this.#createSyncContext( activatedAccountPairs, scope, + sep41Assets, ); // Step 2: Fetch Horizon history per account → map to keyring transactions. @@ -142,6 +136,7 @@ export class TransactionSynchronizeService { async #createSyncContext( activatedAccountPairs: ActivatedAccountPair[], scope: KnownCaip2ChainId, + sep41Assets: StellarAssetMetadata[], ): Promise { const keyringAccounts: StellarKeyringAccount[] = []; const keyringAccountIds: KeyringAccountId[] = []; @@ -156,12 +151,11 @@ export class TransactionSynchronizeService { } // Use Promise.all (not allSettled): if state cannot be loaded, sync cannot continue. - const [pendingByAccount, lastScanTokenByAccountId, sep41AssetsMetadata] = - await Promise.all([ - this.#loadPendingTransactionsFromState(keyringAccountIds, scope), - this.#fetchLastScanTokens(keyringAccountIds, scope), - this.#getPersistedSep41AssetsMetadata(scope), - ]); + const [pendingByAccount, lastScanTokenByAccountId] = await Promise.all([ + this.#loadPendingTransactionsFromState(keyringAccountIds, scope), + this.#fetchLastScanTokens(keyringAccountIds, scope), + ]); + const sep41AssetsMetadata = this.#toSep41AssetsMetadata(sep41Assets); return { scope, @@ -174,12 +168,10 @@ export class TransactionSynchronizeService { }; } - async #getPersistedSep41AssetsMetadata( - scope: KnownCaip2ChainId, - ): Promise> { - const persistedAssets = - await this.#assetMetadataService.fetchSep41AssetsOrSyncOnce(scope); - return persistedAssets.reduce< + #toSep41AssetsMetadata( + sep41Assets: StellarAssetMetadata[], + ): Record { + return sep41Assets.reduce< Record >((acc, asset) => { acc[asset.assetId as KnownCaip19Sep41AssetId] = asset; diff --git a/packages/snap/src/services/transaction/__mocks__/transaction.fixtures.ts b/packages/snap/src/services/transaction/__mocks__/transaction.fixtures.ts index d246ba4c..feb8863b 100644 --- a/packages/snap/src/services/transaction/__mocks__/transaction.fixtures.ts +++ b/packages/snap/src/services/transaction/__mocks__/transaction.fixtures.ts @@ -15,7 +15,6 @@ import { import type { KnownCaip19AssetIdOrSlip44Id } from '../../../api'; import { KnownCaip2ChainId } from '../../../api'; import { getSlip44AssetId, logger } from '../../../utils'; -import { createMockAssetMetadataService } from '../../asset-metadata/__mocks__/assets.fixtures'; import { createMemoryCache } from '../../cache/__mocks__/cache.fixtures'; import { NetworkService } from '../../network'; import { State } from '../../state/State'; @@ -29,7 +28,6 @@ export const createMockTransactionService = () => { const { cache } = createMemoryCache(); const networkService = new NetworkService({ logger, cache }); const transactionBuilder = new TransactionBuilder({ logger }); - const { service: assetMetadataService } = createMockAssetMetadataService(); const transactionService = new TransactionService({ logger, transactionRepository: new TransactionRepository( @@ -43,7 +41,6 @@ export const createMockTransactionService = () => { ), networkService, transactionBuilder, - assetMetadataService, }); const transactionRepositorySaveSpy = jest.spyOn( From 7ddc02be8c1fde3e61b325b35967e01e574702e7 Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Mon, 15 Jun 2026 09:46:09 +0800 Subject: [PATCH 02/10] chore: rollback shasum --- packages/snap/snap.manifest.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 707d4fa1..fb936f26 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-stellar-wallet.git" }, "source": { - "shasum": "N+qKlG+VmcnuPg3jYgYotB7PZEiFIoQz6FCKI2CIDjY=", + "shasum": "YP/x/UxnvPyYy6yLPeQM2j1PZR8D8I4xUrgqoUnWbSE=", "location": { "npm": { "filePath": "dist/bundle.js", From 20816fdc1708c93c117a223164f5d0af357bed71 Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Mon, 15 Jun 2026 09:54:29 +0800 Subject: [PATCH 03/10] fix: comment --- packages/snap/src/services/sync/SynchronizeService.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/snap/src/services/sync/SynchronizeService.ts b/packages/snap/src/services/sync/SynchronizeService.ts index e597518b..2d63ade7 100644 --- a/packages/snap/src/services/sync/SynchronizeService.ts +++ b/packages/snap/src/services/sync/SynchronizeService.ts @@ -68,6 +68,11 @@ export class SynchronizeService { return; } + if (!syncAccounts && !syncTransactions) { + this.#logger.debug('No sync steps to run'); + return; + } + // Both async loads are fail-safe, so we can use Promise.all. const [activatedAccountPairs, sep41Assets] = await Promise.all([ this.#loadActivatedPairs(accounts, scope), @@ -98,10 +103,6 @@ export class SynchronizeService { }); } - if (tasks.length === 0) { - return; - } - const results = await Promise.allSettled( tasks.map(async ({ task }) => task), ); From 82c01ddf0eeb57e1072c9e94f16a60ddf8f5ee37 Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Mon, 15 Jun 2026 11:13:35 +0800 Subject: [PATCH 04/10] chore: add sep41 receive sync --- packages/snap/src/context.ts | 1 + .../src/services/account/AccountService.ts | 13 ++ .../transaction/TransactionService.ts | 4 + .../TransactionSynchronizeService.test.ts | 194 +++++++++++++++++- .../TransactionSynchronizeService.ts | 161 ++++++++++++++- .../__mocks__/transaction.fixtures.ts | 3 + 6 files changed, 368 insertions(+), 8 deletions(-) diff --git a/packages/snap/src/context.ts b/packages/snap/src/context.ts index 29638f0d..bd9c5d7f 100644 --- a/packages/snap/src/context.ts +++ b/packages/snap/src/context.ts @@ -113,6 +113,7 @@ const transactionService = new TransactionService({ transactionRepository, networkService, transactionBuilder, + accountService, }); const priceService = new PriceService({ diff --git a/packages/snap/src/services/account/AccountService.ts b/packages/snap/src/services/account/AccountService.ts index 15fbb9d4..e3010c8a 100644 --- a/packages/snap/src/services/account/AccountService.ts +++ b/packages/snap/src/services/account/AccountService.ts @@ -288,6 +288,19 @@ export class AccountService { return await this.#accountsRepository.getAll(); } + /** + * Lists all Stellar accounts in the keyring by scope. + * + * @param scope - The scope of the accounts to list. + * @returns A Promise that resolves to the list of accounts. + */ + async listAccountsByScope( + scope: KnownCaip2ChainId, + ): Promise { + const accounts = await this.#accountsRepository.getAll(); + return accounts.filter((account) => account.scopes.includes(scope)); + } + /** * Lists all Stellar accounts in the keyring by their IDs. * diff --git a/packages/snap/src/services/transaction/TransactionService.ts b/packages/snap/src/services/transaction/TransactionService.ts index 608a5287..002e8ca5 100644 --- a/packages/snap/src/services/transaction/TransactionService.ts +++ b/packages/snap/src/services/transaction/TransactionService.ts @@ -29,6 +29,7 @@ import type { import { getSnapProvider, isSep41Id, isSlip44Id } from '../../utils'; import type { ILogger } from '../../utils/logger'; import { createPrefixedLogger } from '../../utils/logger'; +import type { AccountService } from '../account'; import type { StellarAssetMetadata } from '../asset-metadata'; import type { NetworkService } from '../network'; import { @@ -57,11 +58,13 @@ export class TransactionService { transactionRepository, networkService, transactionBuilder, + accountService, }: { logger: ILogger; transactionRepository: TransactionRepository; networkService: NetworkService; transactionBuilder: TransactionBuilder; + accountService: AccountService; }) { this.#logger = createPrefixedLogger(logger, '[🧾 TransactionService]'); this.#transactionRepository = transactionRepository; @@ -76,6 +79,7 @@ export class TransactionService { networkService, transactionRepository, transactionMapper, + accountService, logger, }); } diff --git a/packages/snap/src/services/transaction/TransactionSynchronizeService.test.ts b/packages/snap/src/services/transaction/TransactionSynchronizeService.test.ts index aad9749f..3585e5d2 100644 --- a/packages/snap/src/services/transaction/TransactionSynchronizeService.test.ts +++ b/packages/snap/src/services/transaction/TransactionSynchronizeService.test.ts @@ -1,10 +1,15 @@ -import { KeyringEvent, TransactionStatus } from '@metamask/keyring-api'; import type { Transaction as KeyringTransaction } from '@metamask/keyring-api'; +import { + KeyringEvent, + TransactionStatus, + TransactionType, +} from '@metamask/keyring-api'; import { emitSnapKeyringEvent } from '@metamask/keyring-snap-sdk'; import type { Horizon } from '@stellar/stellar-sdk'; import { Keypair, Networks } from '@stellar/stellar-sdk'; import { KnownCaip2ChainId } from '../../api'; +import { sep41SendTransactionResponse } from './__mocks__/horizon-transaction-responses.fixtures'; import { buildMockClassicTransaction, generateMockTransactions, @@ -14,9 +19,13 @@ import { Transaction } from './Transaction'; import { TransactionMapper } from './TransactionMapper'; import { TransactionRepository } from './TransactionRepository'; import { TransactionSynchronizeService } from './TransactionSynchronizeService'; +import { toCaip19Sep41AssetId } from '../../utils'; import { logger } from '../../utils/logger'; import { getSnapProvider } from '../../utils/snap'; import { generateStellarKeyringAccount } from '../account/__mocks__/account.fixtures'; +import type { AccountService } from '../account/AccountService'; +import type { StellarAssetMetadata } from '../asset-metadata/api'; +import { toStellarAssetMetadata } from '../asset-metadata/utils'; import { createMemoryCache } from '../cache/__mocks__/cache.fixtures'; import { NetworkService } from '../network'; import { @@ -79,8 +88,23 @@ function buildOnChainPaymentTransaction(params: { describe('TransactionSynchronizeService', () => { const scope = KnownCaip2ChainId.Mainnet; const destinationAddress = Keypair.random().publicKey(); + const sep41SenderAddress = + 'GA7UCNSASSOPQYTRGJ2NC7TDBSXHMWK6JHS7AO6X2ZQAIQSTB5ELNFSO'; + const sep41RecipientAddress = + 'GB327AMKGJDXEMQREZRRVW7Y6KEKWPOWTJKCCYUQK7KKXVMCTNZEOYXU'; + const sep41AssetId = toCaip19Sep41AssetId( + scope, + 'CBIJBDNZNF4X35BJ4FFZWCDBSCKOP5NB4PLG4SNENRMLAPYG4P5FM6VN', + ); + const sep41AssetMetadata: StellarAssetMetadata = toStellarAssetMetadata({ + assetId: sep41AssetId, + decimals: 8, + symbol: 'SolvBTC', + }); - const setup = () => { + const setup = (params?: { + allAccounts?: ReturnType[]; + }) => { const { cache } = createMemoryCache(); const networkService = new NetworkService({ logger, cache }); const transactionRepository = new TransactionRepository( @@ -96,10 +120,16 @@ describe('TransactionSynchronizeService', () => { keyringTransactionBuilder: new KeyringTransactionBuilder(), logger, }); + const accountService = { + listAccountsByScope: jest + .fn() + .mockResolvedValue(params?.allAccounts ?? []), + } as unknown as AccountService; const service = new TransactionSynchronizeService({ networkService, transactionRepository, transactionMapper, + accountService, logger, }); @@ -107,6 +137,7 @@ describe('TransactionSynchronizeService', () => { service, networkService, transactionRepository, + accountService, getTransactionsSpy: jest.spyOn(networkService, 'getTransactions'), getTransactionSpy: jest.spyOn(networkService, 'getTransaction'), findByAccountIdsSpy: jest.spyOn( @@ -295,4 +326,163 @@ describe('TransactionSynchronizeService', () => { }, ); }); + + it('maps SEP-41 receive for a snap-managed recipient when send is confirmed', async () => { + const senderAccount = generateStellarKeyringAccount( + 'sender-account', + sep41SenderAddress, + 'sync-entropy', + 0, + ); + const recipientAccount = generateStellarKeyringAccount( + 'recipient-account', + sep41RecipientAddress, + 'sync-entropy', + 1, + ); + const { + service, + getTransactionsSpy, + findByAccountIdsSpy, + findLastScanTokenSpy, + saveManySpy, + emitSnapKeyringEventSpy, + } = setup({ + allAccounts: [senderAccount, recipientAccount], + }); + const { activatedAccountPair } = buildActivatedPair( + senderAccount.id, + senderAccount.address, + ); + const onChainTransaction = Transaction.fromHorizon({ + horizonTransaction: sep41SendTransactionResponse, + scope, + }); + + findByAccountIdsSpy.mockResolvedValue([]); + findLastScanTokenSpy.mockResolvedValue({ + [senderAccount.id]: null, + }); + getTransactionsSpy.mockResolvedValue([onChainTransaction]); + + await service.synchronize([activatedAccountPair], scope, [ + sep41AssetMetadata, + ]); + + expect(emitSnapKeyringEventSpy).toHaveBeenCalledWith( + getSnapProvider(), + KeyringEvent.AccountTransactionsUpdated, + { + transactions: { + [senderAccount.id]: [ + expect.objectContaining({ + id: onChainTransaction.id, + account: senderAccount.id, + type: TransactionType.Send, + }), + ], + [recipientAccount.id]: [ + expect.objectContaining({ + id: onChainTransaction.id, + account: recipientAccount.id, + type: TransactionType.Receive, + from: [ + expect.objectContaining({ + address: sep41SenderAddress, + }), + ], + to: [ + expect.objectContaining({ + address: sep41RecipientAddress, + }), + ], + }), + ], + }, + }, + ); + expect(saveManySpy).toHaveBeenCalledWith( + expect.arrayContaining([ + expect.objectContaining({ + id: onChainTransaction.id, + account: senderAccount.id, + type: TransactionType.Send, + }), + expect.objectContaining({ + id: onChainTransaction.id, + account: recipientAccount.id, + type: TransactionType.Receive, + }), + ]), + { + [senderAccount.id]: { + [scope]: sep41SendTransactionResponse.paging_token, + }, + }, + ); + }); + + it('skips SEP-41 receive mapping when send is not confirmed', async () => { + const senderAccount = generateStellarKeyringAccount( + 'sender-account-pending', + sep41SenderAddress, + 'sync-entropy', + 0, + ); + const recipientAccount = generateStellarKeyringAccount( + 'recipient-account-pending', + sep41RecipientAddress, + 'sync-entropy', + 1, + ); + const { + service, + getTransactionsSpy, + findByAccountIdsSpy, + findLastScanTokenSpy, + emitSnapKeyringEventSpy, + } = setup({ + allAccounts: [senderAccount, recipientAccount], + }); + const { activatedAccountPair } = buildActivatedPair( + senderAccount.id, + senderAccount.address, + ); + const onChainTransaction = Transaction.fromHorizon({ + horizonTransaction: { + ...sep41SendTransactionResponse, + successful: false, + }, + scope, + }); + + findByAccountIdsSpy.mockResolvedValue([]); + findLastScanTokenSpy.mockResolvedValue({ + [senderAccount.id]: null, + }); + getTransactionsSpy.mockResolvedValue([onChainTransaction]); + + await service.synchronize([activatedAccountPair], scope, [ + sep41AssetMetadata, + ]); + + expect(emitSnapKeyringEventSpy).toHaveBeenCalledWith( + getSnapProvider(), + KeyringEvent.AccountTransactionsUpdated, + { + transactions: { + [senderAccount.id]: [ + expect.objectContaining({ + id: onChainTransaction.id, + account: senderAccount.id, + status: TransactionStatus.Failed, + }), + ], + }, + }, + ); + expect(emitSnapKeyringEventSpy.mock.calls[0]?.[2]).not.toHaveProperty( + `transactions.${recipientAccount.id}`, + ); + }); }); diff --git a/packages/snap/src/services/transaction/TransactionSynchronizeService.ts b/packages/snap/src/services/transaction/TransactionSynchronizeService.ts index d0f83ae4..ec26d2b1 100644 --- a/packages/snap/src/services/transaction/TransactionSynchronizeService.ts +++ b/packages/snap/src/services/transaction/TransactionSynchronizeService.ts @@ -1,13 +1,18 @@ import { KeyringEvent, + FungibleAssetStruct, + TransactionStatus, + TransactionType, type Transaction as KeyringTransaction, } from '@metamask/keyring-api'; import { emitSnapKeyringEvent } from '@metamask/keyring-snap-sdk'; import { Mutex } from 'async-mutex'; +import { cloneDeep } from 'lodash'; import type { KnownCaip19Sep41AssetId, KnownCaip2ChainId, + StellarAddress, TransactionId, } from '../../api'; import type { ILogger } from '../../utils'; @@ -15,9 +20,15 @@ import { batchesAllSettled, createPrefixedLogger, getSnapProvider, + isSameStr, + isSep41Id, pushToRecordArray, } from '../../utils'; -import type { KeyringAccountId, StellarKeyringAccount } from '../account'; +import type { + AccountService, + KeyringAccountId, + StellarKeyringAccount, +} from '../account'; import { TransactionOrder } from './api'; import type { Transaction } from './Transaction'; import type { TransactionMapper } from './TransactionMapper'; @@ -40,6 +51,8 @@ type SyncContext = { scope: KnownCaip2ChainId; keyringAccounts: StellarKeyringAccount[]; keyringAccountsById: Map; + /** All snap-managed accounts on `scope`, keyed by address for SEP-41 receive mapping. */ + allAccountsByAddress: Map; pendingByAccount: PendingTransactionsByAccount; lastScanTokenByAccountId: Record; /** Mapped keyring txs collected during steps 2–3; persisted and emitted in step 4. */ @@ -52,7 +65,7 @@ type SyncContext = { * * Sync run: * 1. Create {@link SyncContext} — pending txs from snap state, scan cursors, empty collector. - * 2. Scan paginated on-chain history per account and map to keyring transactions. + * 2. Scan paginated on-chain history per account, map to keyring transactions, and apply best-effort SEP-41 receive mapping. * 3. Reconcile remaining snap pending txs against on-chain (by hash), then map. * 4. Save snap state and emit `AccountTransactionsUpdated` to the controller. * @@ -66,6 +79,8 @@ export class TransactionSynchronizeService { readonly #networkService: NetworkService; + readonly #accountService: AccountService; + readonly #logger: ILogger; /** Prevents overlapping sync runs from interleaving read–merge–write. */ @@ -75,16 +90,19 @@ export class TransactionSynchronizeService { networkService, transactionMapper, transactionRepository, + accountService, logger, }: { networkService: NetworkService; transactionRepository: TransactionRepository; transactionMapper: TransactionMapper; + accountService: AccountService; logger: ILogger; }) { this.#networkService = networkService; this.#transactionRepository = transactionRepository; this.#transactionMapper = transactionMapper; + this.#accountService = accountService; this.#logger = createPrefixedLogger( logger, '[💼 TransactionSynchronizeService]', @@ -151,16 +169,19 @@ export class TransactionSynchronizeService { } // Use Promise.all (not allSettled): if state cannot be loaded, sync cannot continue. - const [pendingByAccount, lastScanTokenByAccountId] = await Promise.all([ - this.#loadPendingTransactionsFromState(keyringAccountIds, scope), - this.#fetchLastScanTokens(keyringAccountIds, scope), - ]); + const [pendingByAccount, lastScanTokenByAccountId, allAccountsByAddress] = + await Promise.all([ + this.#loadPendingTransactionsFromState(keyringAccountIds, scope), + this.#fetchLastScanTokens(keyringAccountIds, scope), + this.#loadAllAccountsByAddress(scope), + ]); const sep41AssetsMetadata = this.#toSep41AssetsMetadata(sep41Assets); return { scope, keyringAccounts, keyringAccountsById, + allAccountsByAddress, pendingByAccount, lastScanTokenByAccountId, transactionsToSave: {}, @@ -168,6 +189,23 @@ export class TransactionSynchronizeService { }; } + async #loadAllAccountsByAddress( + scope: KnownCaip2ChainId, + ): Promise> { + const accounts = await this.#accountService.listAccountsByScope(scope); + const allAccountsByAddress = new Map< + StellarAddress, + StellarKeyringAccount + >(); + + // Lowercase keys so recipient lookup matches regardless of StrKey casing. + for (const account of accounts) { + allAccountsByAddress.set(account.address.toLowerCase(), account); + } + + return allAccountsByAddress; + } + #toSep41AssetsMetadata( sep41Assets: StellarAssetMetadata[], ): Record { @@ -403,6 +441,77 @@ export class TransactionSynchronizeService { keyringAccount.id, mappedTransaction, ); + + this.#appendSep41ReceiveMappingSafe(context, { + keyringTransaction: mappedTransaction, + senderKeyringAccount: keyringAccount, + }); + } + } + + /** + * Best-effort SEP-41 receive mapping without failing the sync run. + * + * Horizon omits SEP-41 receives from the recipient's history. When a confirmed + * sender-side SEP-41 send maps to another snap-managed account, clone that mapped + * send as a receive for the recipient. Ineligible transactions are skipped silently; + * unexpected errors are logged and ignored. + * + * @param context - Mutable sync run state including account lookup and save queue. + * @param params - Mapped sender transaction and the account that produced it. + * @param params.keyringTransaction - Mapped send from the sender's scan or reconcile. + * @param params.senderKeyringAccount - Keyring account whose scan produced the send. + */ + #appendSep41ReceiveMappingSafe( + context: SyncContext, + params: { + keyringTransaction: KeyringTransaction; + senderKeyringAccount: StellarKeyringAccount; + }, + ): void { + try { + const { keyringTransaction, senderKeyringAccount } = params; + + const toAccountAddress = + this.#getSep41RecipientAddressFromKeyringTransaction( + keyringTransaction, + senderKeyringAccount, + ); + + if (!toAccountAddress) { + return; + } + + const recipientAccount = context.allAccountsByAddress.get( + toAccountAddress.toLowerCase(), + ); + if (!recipientAccount) { + return; + } + + const mappedReceive = cloneDeep(keyringTransaction); + mappedReceive.type = TransactionType.Receive; + mappedReceive.account = recipientAccount.id; + + // Wallet-created sends can accumulate multiple status events; keep the latest only. + if (mappedReceive.events.length > 1) { + const latestEvent = mappedReceive.events.at(-1); + mappedReceive.events = latestEvent + ? [latestEvent] + : mappedReceive.events; + } + + pushToRecordArray( + context.transactionsToSave, + recipientAccount.id, + mappedReceive, + ); + } catch (error) { + // Not throwing the error to avoid blocking the sync. + this.#logger.logErrorWithDetails( + 'Failed to append SEP-41 receive mapping', + { error }, + ); } } @@ -557,4 +666,44 @@ export class TransactionSynchronizeService { pendingById.set(transaction.id, transaction); } + + /** + * Returns the recipient address when `transaction` is a confirmed SEP-41 send from + * `senderKeyringAccount`. + * + * @param transaction - Mapped keyring transaction from the sender scan or reconcile. + * @param senderKeyringAccount - Keyring account that produced the mapped send. + * @returns Recipient Stellar address, or `null` when ineligible. + */ + #getSep41RecipientAddressFromKeyringTransaction( + transaction: KeyringTransaction, + senderKeyringAccount: StellarKeyringAccount, + ): string | null { + const to = transaction.to?.[0]; + const from = transaction.from?.[0]; + const fromAsset = from?.asset; + const toAsset = to?.asset; + + if ( + transaction.type !== TransactionType.Send || + !to || + !from || + !FungibleAssetStruct.is(fromAsset) || + !FungibleAssetStruct.is(toAsset) || + // The transaction is not confirmed. + transaction.status !== TransactionStatus.Confirmed || + // The transaction is not a SEP-41 send from the sender's account. + !isSameStr(from.address, senderKeyringAccount.address) || + // The transaction is a self transfer. + isSameStr(from.address, to.address) || + // The transaction's from and to asset types do not match. + !isSameStr(fromAsset.type, toAsset.type) || + // The transaction's asset is not a SEP-41 asset. + !isSep41Id(fromAsset.type) + ) { + return null; + } + + return to.address; + } } diff --git a/packages/snap/src/services/transaction/__mocks__/transaction.fixtures.ts b/packages/snap/src/services/transaction/__mocks__/transaction.fixtures.ts index feb8863b..30c5cbe3 100644 --- a/packages/snap/src/services/transaction/__mocks__/transaction.fixtures.ts +++ b/packages/snap/src/services/transaction/__mocks__/transaction.fixtures.ts @@ -15,6 +15,7 @@ import { import type { KnownCaip19AssetIdOrSlip44Id } from '../../../api'; import { KnownCaip2ChainId } from '../../../api'; import { getSlip44AssetId, logger } from '../../../utils'; +import { mockAccountService } from '../../account/__mocks__/account.fixtures'; import { createMemoryCache } from '../../cache/__mocks__/cache.fixtures'; import { NetworkService } from '../../network'; import { State } from '../../state/State'; @@ -28,6 +29,7 @@ export const createMockTransactionService = () => { const { cache } = createMemoryCache(); const networkService = new NetworkService({ logger, cache }); const transactionBuilder = new TransactionBuilder({ logger }); + const { accountService } = mockAccountService(); const transactionService = new TransactionService({ logger, transactionRepository: new TransactionRepository( @@ -41,6 +43,7 @@ export const createMockTransactionService = () => { ), networkService, transactionBuilder, + accountService, }); const transactionRepositorySaveSpy = jest.spyOn( From 3da160967c6e3052618eeda67e22f8e4cd8810ec Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Mon, 15 Jun 2026 12:11:23 +0800 Subject: [PATCH 05/10] chore: remove async --- .../services/transaction/TransactionMapper.ts | 16 ++++------------ .../transaction/TransactionSynchronizeService.ts | 8 ++++---- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/packages/snap/src/services/transaction/TransactionMapper.ts b/packages/snap/src/services/transaction/TransactionMapper.ts index 1ceae769..3f0ad290 100644 --- a/packages/snap/src/services/transaction/TransactionMapper.ts +++ b/packages/snap/src/services/transaction/TransactionMapper.ts @@ -408,9 +408,7 @@ export class TransactionMapper { const [firstOperation] = transaction.transactionOperations; if (!isInvokeHostFunctionOperation(firstOperation)) { - throw new TransactionMapperException( - 'Unable to map a SEP-41 send transaction - not an invoke host function operation', - ); + return undefined; } const parsedSep41TransferInvoke = parseSep41TransferInvokeSafe( @@ -419,26 +417,20 @@ export class TransactionMapper { ); if (!parsedSep41TransferInvoke) { - throw new TransactionMapperException( - 'Unable to map a SEP-41 send transaction - invalid transfer invoke operation', - ); + return undefined; } const { assetId, amount, toAccountId, fromAccountId } = parsedSep41TransferInvoke; if (fromAccountId !== keyringAccount.address) { - throw new TransactionMapperException( - 'Unable to map a SEP-41 send transaction - from account id does not match the keyring account address', - ); + return undefined; } // TODO: Fall back to NetworkService token metadata when state is missing (RPC cost per tx). const asset = assetMetadata[assetId]; if (!asset) { - throw new TransactionMapperException( - 'Unable to map a SEP-41 send transaction - asset metadata not found', - ); + return undefined; } const assetRow = this.#toKeyringAssetRow( diff --git a/packages/snap/src/services/transaction/TransactionSynchronizeService.ts b/packages/snap/src/services/transaction/TransactionSynchronizeService.ts index ec26d2b1..72f4b496 100644 --- a/packages/snap/src/services/transaction/TransactionSynchronizeService.ts +++ b/packages/snap/src/services/transaction/TransactionSynchronizeService.ts @@ -304,7 +304,7 @@ export class TransactionSynchronizeService { noOfResolved += 1; } - await this.#appendMappedTransaction(context, { + this.#appendMappedTransaction(context, { keyringAccount, onChainTransaction: transaction, pendingFromState, @@ -393,7 +393,7 @@ export class TransactionSynchronizeService { continue; } - await this.#appendMappedTransaction(context, { + this.#appendMappedTransaction(context, { keyringAccount, onChainTransaction, pendingFromState, @@ -417,14 +417,14 @@ export class TransactionSynchronizeService { }); } - async #appendMappedTransaction( + #appendMappedTransaction( context: SyncContext, params: { keyringAccount: StellarKeyringAccount; onChainTransaction: Transaction; pendingFromState?: KeyringTransaction; }, - ): Promise { + ): void { const { keyringAccount, onChainTransaction, pendingFromState } = params; const mappedTransaction = this.#transactionMapper.mapTransactionSafe({ From 2ce7d4526b2e3c6752975fae1d54c74994767422 Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:57:41 +0800 Subject: [PATCH 06/10] fix: account and txn sync issue --- packages/snap/src/config.ts | 4 +- packages/snap/src/handlers/keyring/keyring.ts | 12 ++- .../OnChainAccountSynchronizeService.test.ts | 95 +++++++++++-------- .../OnChainAccountSynchronizeService.ts | 88 +++++++++-------- .../KeyringTransactionBuilder.test.ts | 10 +- .../transaction/KeyringTransactionBuilder.ts | 43 ++++++++- 6 files changed, 170 insertions(+), 82 deletions(-) diff --git a/packages/snap/src/config.ts b/packages/snap/src/config.ts index 0ad4f1e7..803d082c 100644 --- a/packages/snap/src/config.ts +++ b/packages/snap/src/config.ts @@ -88,14 +88,14 @@ const ConfigStruct = object({ /** * The base fee multiplier for the Stellar network. */ - baseFeeMultiplier: parseIntegerStruct(1, 1.2), + baseFeeMultiplier: parseIntegerStruct(1, 1.5), /** * The smart contract transaction fee multiplier for the Stellar network. * The multiplier is higher because smart contract transactions have tighter ledger limits than normal transactions. * * @see https://developers.stellar.org/docs/learn/fundamentals/fees-resource-limits-metering#ledger-limits */ - simulationFeeMultiplier: parseIntegerStruct(1, 1.5), + simulationFeeMultiplier: parseIntegerStruct(1, 2), }), api: object({ tokenApi: object({ diff --git a/packages/snap/src/handlers/keyring/keyring.ts b/packages/snap/src/handlers/keyring/keyring.ts index 8e0c940b..9e07823e 100644 --- a/packages/snap/src/handlers/keyring/keyring.ts +++ b/packages/snap/src/handlers/keyring/keyring.ts @@ -122,8 +122,18 @@ export class KeyringHandler implements Keyring { async handle(origin: string, request: JsonRpcRequest): Promise { const result = (await withCatchAndThrowSnapError(async () => { + this.#logger.debug('Handle keyring request', { + origin, + request, + }); validateOrigin(origin, request.method); - return handleKeyringRequest(this, request); + const result = await handleKeyringRequest(this, request); + this.#logger.debug('Keyring request handled', { + origin, + request, + result, + }); + return result; }, this.#logger)) ?? null; return result; diff --git a/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.test.ts b/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.test.ts index 125df43d..4956cb90 100644 --- a/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.test.ts +++ b/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.test.ts @@ -330,7 +330,7 @@ describe('OnChainAccountSynchronizeService', () => { ); }); - it('keeps SEP-41 persisted with zero balance when new sync returns zero', async () => { + it('omits SEP-41 zero balance from balance payload while persisting it in state', async () => { setupTest(); const { @@ -373,9 +373,9 @@ describe('OnChainAccountSynchronizeService', () => { KeyringEvent.AccountBalancesUpdated, expect.objectContaining({ balances: { - [keyringAccount.id]: expect.objectContaining({ - [sep41Id]: { unit: 'USDC', amount: '0' }, - }), + [keyringAccount.id]: { + [NATIVE]: { unit: NATIVE_ASSET_SYMBOL, amount: '1' }, + }, }, }), ); @@ -394,7 +394,7 @@ describe('OnChainAccountSynchronizeService', () => { expect(sep41Row?.balance).toBe('0'); }); - it('still emits SEP-41 zero balance on later syncs after missed events', async () => { + it('omits SEP-41 zero balance from balance payload on later syncs after missed events', async () => { setupTest(); const { @@ -456,15 +456,18 @@ describe('OnChainAccountSynchronizeService', () => { expect(balanceEventCalls[3]?.[2]).toStrictEqual( expect.objectContaining({ balances: { - [keyringAccount.id]: expect.objectContaining({ - [sep41Id]: { unit: 'USDC', amount: '0' }, - }), + [keyringAccount.id]: { + [NATIVE]: { unit: NATIVE_ASSET_SYMBOL, amount: '1' }, + }, }, }), ); + expect(balanceEventCalls[3]?.[2]).not.toHaveProperty( + `balances.${keyringAccount.id}.${sep41Id}`, + ); }); - it('simulates client receiving sync 1 and 4 while missing 2 and 3 with trustline removal and SEP-41 zero', async () => { + it('replays asset-list add/remove on sync 4 while balance payload omits tombstones and zero SEP-41', async () => { setupTest(); const { @@ -599,19 +602,28 @@ describe('OnChainAccountSynchronizeService', () => { isKeyringEmitCall(call, KeyringEvent.AccountBalancesUpdated), ) .map((call) => call[2]); + const assetListEventCalls = emitSnapKeyringEventSpy.mock.calls + .filter((call) => + isKeyringEmitCall(call, KeyringEvent.AccountAssetListUpdated), + ) + .map((call) => call[2]); expect(balanceEventCalls).toHaveLength(4); - const sync1Payload = balanceEventCalls[0] as { + expect(assetListEventCalls).toHaveLength(4); + const sync1BalancePayload = balanceEventCalls[0] as { balances: Record< string, Record >; }; - const sync4Payload = balanceEventCalls[3] as { + const sync4BalancePayload = balanceEventCalls[3] as { balances: Record< string, Record >; }; + const sync4AssetListPayload = assetListEventCalls[3] as { + assets: Record; + }; const simulatedClientBalances: Record< string, @@ -621,7 +633,7 @@ describe('OnChainAccountSynchronizeService', () => { // Client receives sync 1. Object.assign( simulatedClientBalances, - sync1Payload.balances[keyringAccount.id] as Record< + sync1BalancePayload.balances[keyringAccount.id] as Record< string, { amount: string; unit: string } >, @@ -631,26 +643,32 @@ describe('OnChainAccountSynchronizeService', () => { // Client misses sync 2 and 3. - // Client receives sync 4 and catches up. + // Client receives sync 4 balance patch: visible assets only; stale keys from sync 1 remain. Object.assign( simulatedClientBalances, - sync4Payload.balances[keyringAccount.id] as Record< + sync4BalancePayload.balances[keyringAccount.id] as Record< string, { amount: string; unit: string } >, ); - // Trustline removal happened on sync 2 and was missed by client. - // Classic removals persist as tombstone entries (`limit` 0), so sync 4 still sends balance 0. - expect(simulatedClientBalances[USDC_CLASSIC]?.amount).toBe('0'); - // SEP-41 zero entries are persisted and continue to be sent by sync balance update events, - // so sync 4 corrects the client if it missed a previous update. - expect(simulatedClientBalances[sep41Id]).toStrictEqual({ - unit: 'USDC', - amount: '0', + expect(simulatedClientBalances[USDC_CLASSIC]?.amount).toBe('25'); + expect(simulatedClientBalances[sep41Id]?.amount).toBe('0.00005'); + expect(simulatedClientBalances[NATIVE]?.amount).toBe('1'); + + // Asset list replay on sync 4 reconciles visibility the client missed. + expect(sync4AssetListPayload.assets[keyringAccount.id]).toStrictEqual({ + added: expect.arrayContaining([NATIVE]), + removed: expect.arrayContaining([USDC_CLASSIC, sep41Id]), }); + expect(sync4BalancePayload.balances[keyringAccount.id]).not.toHaveProperty( + USDC_CLASSIC, + ); + expect(sync4BalancePayload.balances[keyringAccount.id]).not.toHaveProperty( + sep41Id, + ); }); - it('runs four syncs with emit failures then full balance and asset-list reconciliation', async () => { + it('runs four syncs with emit failures then reconciles via replayed asset-list and visible balances', async () => { setupTest(); const { @@ -810,34 +828,37 @@ describe('OnChainAccountSynchronizeService', () => { return row!; }; - // 1st `AccountBalancesUpdated` (after sync 1): USDC trustline at 0 + full snapshot. + // 1st `AccountBalancesUpdated` (after sync 1): USDC trustline at 0 + native. expect(accountBalancesFromNthBalanceEmit(0)).toMatchObject({ + [NATIVE]: { unit: NATIVE_ASSET_SYMBOL, amount: '1' }, [USDC_CLASSIC]: { unit: 'USDC', amount: '0' }, }); - // 2nd `AccountBalancesUpdated` (after sync 2): USDC removed on Horizon → tombstone balance 0 in payload. - expect(accountBalancesFromNthBalanceEmit(1)).toMatchObject({ - [USDC_CLASSIC]: { unit: 'USDC', amount: '0' }, + // 2nd `AccountBalancesUpdated` (after sync 2): USDC tombstone omitted; native only. + expect(accountBalancesFromNthBalanceEmit(1)).toStrictEqual({ + [NATIVE]: { unit: NATIVE_ASSET_SYMBOL, amount: '1' }, }); - // 3rd `AccountBalancesUpdated` (after sync 3): EURC added on Horizon + USDC tombstone still in snapshot. + // 3rd `AccountBalancesUpdated` (after sync 3): EURC visible; USDC tombstone omitted. expect(accountBalancesFromNthBalanceEmit(2)).toMatchObject({ - [USDC_CLASSIC]: { unit: 'USDC', amount: '0' }, + [NATIVE]: { unit: NATIVE_ASSET_SYMBOL, amount: '1' }, [EURC_CLASSIC]: { unit: 'EURC', amount: '10' }, }); + expect(accountBalancesFromNthBalanceEmit(2)).not.toHaveProperty( + USDC_CLASSIC, + ); - // 4th `AccountBalancesUpdated` (after sync 4): same on-chain entries as sync 3, but `find` read a stale - // baseline so asset-list deltas fire; balances stay a full per-asset map (native + SEP-41 + classics). + // 4th `AccountBalancesUpdated` (after sync 4): same visible balances as sync 3. const fourthBalanceSnapshot = accountBalancesFromNthBalanceEmit(3); - expect(Object.keys(fourthBalanceSnapshot).length).toBeGreaterThan(2); - expect(fourthBalanceSnapshot).toMatchObject({ - [USDC_CLASSIC]: { unit: 'USDC', amount: '0' }, + expect(fourthBalanceSnapshot).toStrictEqual({ + [NATIVE]: { unit: NATIVE_ASSET_SYMBOL, amount: '1' }, [EURC_CLASSIC]: { unit: 'EURC', amount: '10' }, }); const assetListCalls = emitSnapKeyringEventSpy.mock.calls.filter((call) => isKeyringEmitCall(call, KeyringEvent.AccountAssetListUpdated), ); + // Asset-list emit runs after balance emit in the same try block; sync 2 and 3 balance failures skip it. expect(assetListCalls).toHaveLength(2); const assetListDeltaFromNthAssetEmit = ( @@ -857,13 +878,13 @@ describe('OnChainAccountSynchronizeService', () => { // 1st `AccountAssetListUpdated` (after sync 1): USDC trustline becomes visible (limit > 0, balance 0). expect(assetListDeltaFromNthAssetEmit(0)).toStrictEqual({ added: expect.arrayContaining([NATIVE, USDC_CLASSIC]), - removed: [], + removed: expect.arrayContaining([sep41Id]), }); - // 2nd `AccountAssetListUpdated` (after sync 4): stale baseline vs current → EURC added, USDC removed from list. + // 2nd `AccountAssetListUpdated` (after sync 4): replays on-chain visibility — EURC added, USDC removed. expect(assetListDeltaFromNthAssetEmit(1)).toStrictEqual({ added: expect.arrayContaining([NATIVE, EURC_CLASSIC]), - removed: [USDC_CLASSIC], + removed: expect.arrayContaining([USDC_CLASSIC, sep41Id]), }); }); diff --git a/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.ts b/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.ts index ab87abb0..3071c80e 100644 --- a/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.ts +++ b/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.ts @@ -432,7 +432,7 @@ export class OnChainAccountSynchronizeService { * * @param stateSnapshotOnChainAccount - Last saved account from state, or `null` when none exists. * @param synchronizedOnChainAccount - Same account after Horizon, SEP-41, and merge steps. - * @returns `balanceChanges` for all known assets plus optional `assetListChanges`. + * @returns `balanceChanges` for on-chain-visible assets plus `assetListChanges` replayed each sync. */ #computeKeyringSyncDeltas( stateSnapshotOnChainAccount: OnChainAccount | null, @@ -458,53 +458,61 @@ export class OnChainAccountSynchronizeService { : stateSnapshotOnChainAccount.getRawAsset(assetId); const onChainEntry = synchronizedOnChainAccount.getRawAsset(assetId); - // Always send the full balance snapshot, even when values did not change. - // This lets the client recover if it missed a previous balances event. - // Example with 4 assets: - // - XLM (native), USDC classic trustline, EURC classic trustline, SOLBTC SEP-41. + // assetIds = union of state + on-chain rawAssetIds (includes tombstones and zero SEP-41). + // Asset list replays every sync from on-chain visibility so a client can recover missed events: + // visible on-chain → added; not visible on-chain (but still in assetIds) → removed. + // Balances are emitted only for on-chain-visible assets. + // Example: XLM (native), USDC/EURC/AQUA (classic trustlines), SOLBTC (SEP-41). // ------------------------- Sync 1 ------------------------------------------ // - Sync 1 (success): payload received by client: - // XLM=10 (raw total), USDC=25, EURC=0, SOLBTC=5. + // state view: none (new account). + // onChain view: XLM=10, USDC=25, EURC=0, SOLBTC=5. + // balance payload: XLM=10, USDC=25, EURC=0, SOLBTC=5. + // asset list payload: added XLM, USDC, EURC, SOLBTC; removed none. // ------------------------- Sync 2 ------------------------------------------ - // - Sync 2 (client misses event): chain updates to - // XLM=11, USDC=30, EURC trustline removed (it was already zero), SOLBTC=0. - // Payload for this sync would include EURC amount=0 and SOLBTC amount=0, - // but client missed it. + // - Sync 2 (client misses event): + // state view: XLM=10, USDC=25, EURC=0, SOLBTC=5. + // onChain view: XLM=11, USDC=30, AQUA trustline added, EURC trustline removed, SOLBTC=0. + // balance payload: XLM=11, USDC=30, AQUA=0. + // asset list payload: added XLM, USDC, AQUA; removed EURC, SOLBTC. // ------------------------- Sync 3 ------------------------------------------ - // - Sync 3 (client misses event): chain updates to - // XLM=9, USDC=30, SOLBTC still 0. - // Because SEP-41 zero balances are persisted, payload still includes SOLBTC=0. + // - Sync 3 (client misses event): + // state view: XLM=11, USDC=30, AQUA trustline, EURC tombstone, SOLBTC=0. + // onChain view: XLM=9, USDC=30, AQUA trustline, EURC tombstone, SOLBTC=0. + // balance payload: XLM=9, USDC=30, AQUA. + // asset list payload: added XLM, USDC, AQUA; removed EURC, SOLBTC (replayed). // ------------------------- Sync 4 ------------------------------------------ - // - Sync 4 (success): we still emit full balances for on-chain view + latest snapshot, - // so payload includes XLM=9 (raw total), USDC=30, SOLBTC=0. - // Classic trustlines removed on chain are persisted as internal tombstones (`limit` 0), - // so sync 4 can still send balance `0` for those asset ids if the client missed earlier events. - balanceChanges[assetId as string] = - assetId === nativeAssetId - ? { - unit: NATIVE_ASSET_SYMBOL, - amount: toDisplayBalance( - synchronizedOnChainAccount.nativeRawBalance, - ), - } - : this.#buildBalancePayloadFromEntries( - onChainEntry, - latestStateEntry, - ); - - const isVisibleFromState = this.#isAssetVisible( - assetId, - latestStateEntry, - ); + // - Sync 4 (success): payload received by client: + // state view: XLM=9, USDC=30, AQUA trustline, EURC tombstone, SOLBTC=0. + // onChain view: XLM=9, USDC=30, AQUA trustline, EURC tombstone, SOLBTC=0. + // balance payload: XLM=9, USDC=30, AQUA. + // asset list payload: added XLM, USDC, AQUA; removed EURC, SOLBTC (replayed). const isVisibleFromOnChain = this.#isAssetVisible(assetId, onChainEntry); - // Add/remove is based on visibility transition between snapshots. - if (isVisibleFromOnChain && !isVisibleFromState) { + + // Asset list: replay add/remove from on-chain visibility (not transition-based). + if (isVisibleFromOnChain) { addedAssets.push(assetId); } - if (isVisibleFromState && !isVisibleFromOnChain) { + if (!isVisibleFromOnChain) { removedAssets.push(assetId); } + + // Balance: on-chain-visible assets only (tombstones and zero SEP-41 omitted). + if (isVisibleFromOnChain) { + balanceChanges[assetId as string] = + assetId === nativeAssetId + ? { + unit: NATIVE_ASSET_SYMBOL, + amount: toDisplayBalance( + synchronizedOnChainAccount.nativeRawBalance, + ), + } + : this.#buildBalancePayloadFromEntries( + onChainEntry, + latestStateEntry, + ); + } } // Native is always persisted on activated accounts; @@ -565,6 +573,9 @@ export class OnChainAccountSynchronizeService { ): Promise { try { if (balancesPayload !== null) { + this.#logger.debug('Emit balances updated event', { + balancesPayload, + }); await emitSnapKeyringEvent( getSnapProvider(), KeyringEvent.AccountBalancesUpdated, @@ -572,6 +583,9 @@ export class OnChainAccountSynchronizeService { ); } if (assetsPayload !== null) { + this.#logger.debug('Emit asset list updated event', { + assetsPayload, + }); await emitSnapKeyringEvent( getSnapProvider(), KeyringEvent.AccountAssetListUpdated, diff --git a/packages/snap/src/services/transaction/KeyringTransactionBuilder.test.ts b/packages/snap/src/services/transaction/KeyringTransactionBuilder.test.ts index 4fb36a13..d952be94 100644 --- a/packages/snap/src/services/transaction/KeyringTransactionBuilder.test.ts +++ b/packages/snap/src/services/transaction/KeyringTransactionBuilder.test.ts @@ -106,9 +106,12 @@ describe('KeyringTransactionBuilder', () => { }, }); - expect(transaction.type).toBe(TransactionType.Unknown); + expect(transaction.type).toBe(TransactionType.TokenApprove); expect(transaction.id).toBe('tx-opt-in-1'); expect(transaction.status).toBe(TransactionStatus.Unconfirmed); + expect(transaction.details).toStrictEqual({ + typeLabel: 'trustline-approve', + }); expect(transaction.events).toStrictEqual([ { status: TransactionStatus.Unconfirmed, timestamp: fixedTimestamp }, ]); @@ -155,8 +158,11 @@ describe('KeyringTransactionBuilder', () => { }, }); - expect(transaction.type).toBe(TransactionType.Unknown); + expect(transaction.type).toBe(TransactionType.TokenDisapprove); expect(transaction.status).toBe(TransactionStatus.Confirmed); + expect(transaction.details).toStrictEqual({ + typeLabel: 'trustline-disapprove', + }); expect(transaction.events).toStrictEqual([ { status: TransactionStatus.Confirmed, timestamp: fixedTimestamp }, ]); diff --git a/packages/snap/src/services/transaction/KeyringTransactionBuilder.ts b/packages/snap/src/services/transaction/KeyringTransactionBuilder.ts index 93285dd9..1f92bf74 100644 --- a/packages/snap/src/services/transaction/KeyringTransactionBuilder.ts +++ b/packages/snap/src/services/transaction/KeyringTransactionBuilder.ts @@ -19,6 +19,11 @@ export enum KeyringTransactionType { Unknown = 'unknown', } +enum KeyringTransactionTypeLabel { + ChangeTrustOptIn = 'trustline-approve', + ChangeTrustOptOut = 'trustline-disapprove', +} + export type KeyringTransactionAsset = { unit: string; type: KnownCaip19AssetIdOrSlip44Id; @@ -126,7 +131,10 @@ export class KeyringTransactionBuilder { switch (request.type) { case KeyringTransactionType.ChangeTrustOptOut: case KeyringTransactionType.ChangeTrustOptIn: - return this.#createChangeTrustTransaction(request.request); + return this.#createChangeTrustTransaction( + request.request, + request.type, + ); case KeyringTransactionType.Send: return this.#createSendTransaction(request.request); case KeyringTransactionType.Swap: @@ -143,8 +151,23 @@ export class KeyringTransactionBuilder { } } + /** + * Builds a change-trust (trustline) keyring transaction. + * + * Maps opt-in to {@link TransactionType.TokenApprove} and opt-out to + * {@link TransactionType.TokenDisapprove}. Sets `details.typeLabel` so the UI can + * distinguish trustline approve vs disapprove until keyring types are fully adopted. + * + * @param request - Change-trust transaction fields (asset, fees, status, etc.). + * @param optInOrOut - Whether this is a trustline opt-in or opt-out. + * @returns A keyring transaction with token approve/disapprove type and UI type label. + * @see {@link https://github.com/MetaMask/accounts/pull/568} + */ #createChangeTrustTransaction( request: ChangeTrustTransactionRequest, + optInOrOut: + | KeyringTransactionType.ChangeTrustOptIn + | KeyringTransactionType.ChangeTrustOptOut, ): KeyringTransaction { const { txId, @@ -154,10 +177,18 @@ export class KeyringTransactionBuilder { status = TransactionStatus.Unconfirmed, } = request; const timestamp = this.#resolveTimestamp(request.timestamp); + const type = + optInOrOut === KeyringTransactionType.ChangeTrustOptIn + ? TransactionType.TokenApprove + : TransactionType.TokenDisapprove; + + const typeLabel = + optInOrOut === KeyringTransactionType.ChangeTrustOptIn + ? KeyringTransactionTypeLabel.ChangeTrustOptIn + : KeyringTransactionTypeLabel.ChangeTrustOptOut; return this.#buildKeyringTransaction({ - // TODO: Add the correct type - type: TransactionType.Unknown, + type, id: txId, account, scope, @@ -166,6 +197,9 @@ export class KeyringTransactionBuilder { status, timestamp, fees: request.fees ?? [], + details: { + typeLabel, + }, }); } @@ -318,6 +352,7 @@ export class KeyringTransactionBuilder { status, timestamp, fees, + details, }: { type: TransactionType; id: string; @@ -328,6 +363,7 @@ export class KeyringTransactionBuilder { status: TransactionStatus; timestamp: number; fees: KeyringTransaction['fees']; + details?: KeyringTransaction['details']; }): KeyringTransaction { return { type, @@ -345,6 +381,7 @@ export class KeyringTransactionBuilder { account: account.id, timestamp, fees, + ...(details === undefined ? {} : { details }), }; } From a13852b5ff947ffc91913aa6aec5d61a920cb1a3 Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Mon, 15 Jun 2026 17:11:30 +0800 Subject: [PATCH 07/10] fix: lint --- packages/snap/src/handlers/keyring/keyring.ts | 4 ++-- .../on-chain-account/OnChainAccountSynchronizeService.ts | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/snap/src/handlers/keyring/keyring.ts b/packages/snap/src/handlers/keyring/keyring.ts index 9e07823e..be01b065 100644 --- a/packages/snap/src/handlers/keyring/keyring.ts +++ b/packages/snap/src/handlers/keyring/keyring.ts @@ -127,13 +127,13 @@ export class KeyringHandler implements Keyring { request, }); validateOrigin(origin, request.method); - const result = await handleKeyringRequest(this, request); + const keyringRequestResult = await handleKeyringRequest(this, request); this.#logger.debug('Keyring request handled', { origin, request, result, }); - return result; + return keyringRequestResult; }, this.#logger)) ?? null; return result; diff --git a/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.ts b/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.ts index 3071c80e..f3b71286 100644 --- a/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.ts +++ b/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.ts @@ -499,6 +499,8 @@ export class OnChainAccountSynchronizeService { } // Balance: on-chain-visible assets only (tombstones and zero SEP-41 omitted). + // There is no need to emit balance for assets that are not visible from on-chain. + // The client controller will remove the balance entry if they receive a asset list event with the asset removed. if (isVisibleFromOnChain) { balanceChanges[assetId as string] = assetId === nativeAssetId From 9bc63de9d20164c77f7a9a2a4bb0f7737c10af5b Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Mon, 15 Jun 2026 17:15:19 +0800 Subject: [PATCH 08/10] fix: comment --- packages/snap/src/config.ts | 14 ++++++++++++-- packages/snap/src/handlers/keyring/keyring.ts | 4 ++-- .../OnChainAccountSynchronizeService.ts | 6 ------ 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/snap/src/config.ts b/packages/snap/src/config.ts index 803d082c..dd10d2a5 100644 --- a/packages/snap/src/config.ts +++ b/packages/snap/src/config.ts @@ -40,6 +40,16 @@ const parseIntegerStruct = ( (value: string) => (value === '' ? undefined : parseInt(value, 10)), ); +const parseFloatStruct = ( + minValue: number, + defaultValue: number, +): Struct => + coerce( + defaulted(min(number(), minValue), defaultValue), + string(), + (value: string) => (value === '' ? undefined : parseFloat(value)), + ); + /** * A struct for validating the network config. */ @@ -88,14 +98,14 @@ const ConfigStruct = object({ /** * The base fee multiplier for the Stellar network. */ - baseFeeMultiplier: parseIntegerStruct(1, 1.5), + baseFeeMultiplier: parseFloatStruct(1, 1.5), /** * The smart contract transaction fee multiplier for the Stellar network. * The multiplier is higher because smart contract transactions have tighter ledger limits than normal transactions. * * @see https://developers.stellar.org/docs/learn/fundamentals/fees-resource-limits-metering#ledger-limits */ - simulationFeeMultiplier: parseIntegerStruct(1, 2), + simulationFeeMultiplier: parseFloatStruct(1, 2), }), api: object({ tokenApi: object({ diff --git a/packages/snap/src/handlers/keyring/keyring.ts b/packages/snap/src/handlers/keyring/keyring.ts index be01b065..edbb9ab7 100644 --- a/packages/snap/src/handlers/keyring/keyring.ts +++ b/packages/snap/src/handlers/keyring/keyring.ts @@ -124,13 +124,13 @@ export class KeyringHandler implements Keyring { (await withCatchAndThrowSnapError(async () => { this.#logger.debug('Handle keyring request', { origin, - request, + method: request.method, }); validateOrigin(origin, request.method); const keyringRequestResult = await handleKeyringRequest(this, request); this.#logger.debug('Keyring request handled', { origin, - request, + method: request.method, result, }); return keyringRequestResult; diff --git a/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.ts b/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.ts index f3b71286..f5f140fa 100644 --- a/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.ts +++ b/packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.ts @@ -575,9 +575,6 @@ export class OnChainAccountSynchronizeService { ): Promise { try { if (balancesPayload !== null) { - this.#logger.debug('Emit balances updated event', { - balancesPayload, - }); await emitSnapKeyringEvent( getSnapProvider(), KeyringEvent.AccountBalancesUpdated, @@ -585,9 +582,6 @@ export class OnChainAccountSynchronizeService { ); } if (assetsPayload !== null) { - this.#logger.debug('Emit asset list updated event', { - assetsPayload, - }); await emitSnapKeyringEvent( getSnapProvider(), KeyringEvent.AccountAssetListUpdated, From 0528d766fd21b10c8dd61fd481bc186747d8f757 Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Mon, 15 Jun 2026 17:35:35 +0800 Subject: [PATCH 09/10] fix: test --- packages/snap/src/handlers/keyring/keyring.ts | 2 +- .../snap/src/services/network/NetworkService.test.ts | 10 ++++------ .../services/transaction/TransactionMapper.test.ts | 12 ++++++++++-- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/snap/src/handlers/keyring/keyring.ts b/packages/snap/src/handlers/keyring/keyring.ts index edbb9ab7..f0fa3bbe 100644 --- a/packages/snap/src/handlers/keyring/keyring.ts +++ b/packages/snap/src/handlers/keyring/keyring.ts @@ -131,7 +131,7 @@ export class KeyringHandler implements Keyring { this.#logger.debug('Keyring request handled', { origin, method: request.method, - result, + result: keyringRequestResult, }); return keyringRequestResult; }, this.#logger)) ?? null; diff --git a/packages/snap/src/services/network/NetworkService.test.ts b/packages/snap/src/services/network/NetworkService.test.ts index 1bf64904..0af5f566 100644 --- a/packages/snap/src/services/network/NetworkService.test.ts +++ b/packages/snap/src/services/network/NetworkService.test.ts @@ -193,12 +193,10 @@ describe('NetworkService', () => { await Promise.resolve(); const second = await networkService.getBaseFeeWithCache(scope); - expect(first).toStrictEqual( - new BigNumber(55).multipliedBy(AppConfig.transaction.baseFeeMultiplier), - ); - expect(second).toStrictEqual( - new BigNumber(55).multipliedBy(AppConfig.transaction.baseFeeMultiplier), - ); + // It doesn't equal to 55 * 1.5 = 82.5, + // because rounded up to 83. + expect(first).toStrictEqual(83); + expect(second).toStrictEqual(83); expect(fetchBaseFeeSpy).toHaveBeenCalledTimes(1); }); diff --git a/packages/snap/src/services/transaction/TransactionMapper.test.ts b/packages/snap/src/services/transaction/TransactionMapper.test.ts index 1cee0c74..beb8c95d 100644 --- a/packages/snap/src/services/transaction/TransactionMapper.test.ts +++ b/packages/snap/src/services/transaction/TransactionMapper.test.ts @@ -231,7 +231,10 @@ describe('TransactionMapper', () => { toAmount: '0', toAddress: accountAddress, fromAddress: accountAddress, - txnType: TransactionType.Unknown, + txnType: TransactionType.TokenApprove, + details: { + typeLabel: 'trustline-approve', + }, }, { testCase: 'remove change trust transaction', @@ -252,7 +255,10 @@ describe('TransactionMapper', () => { toAmount: '0', toAddress: accountAddress, fromAddress: accountAddress, - txnType: TransactionType.Unknown, + txnType: TransactionType.TokenDisapprove, + details: { + typeLabel: 'trustline-disapprove', + }, }, { testCase: 'send transaction', @@ -362,6 +368,7 @@ describe('TransactionMapper', () => { txnType, fromAddress, toAddress, + details, }) => { const { keyringAccount, transactionMapper } = setup(); @@ -421,6 +428,7 @@ describe('TransactionMapper', () => { }, }, ], + ...(details ? { details } : {}), }); }, ); From 6bc4f88fd7237c005913f2e55418b65dd6027993 Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Mon, 15 Jun 2026 17:42:27 +0800 Subject: [PATCH 10/10] fix: test and lint --- packages/snap/src/services/network/NetworkService.test.ts | 4 ++-- .../snap/src/services/transaction/TransactionMapper.test.ts | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/snap/src/services/network/NetworkService.test.ts b/packages/snap/src/services/network/NetworkService.test.ts index 0af5f566..5690df17 100644 --- a/packages/snap/src/services/network/NetworkService.test.ts +++ b/packages/snap/src/services/network/NetworkService.test.ts @@ -195,8 +195,8 @@ describe('NetworkService', () => { // It doesn't equal to 55 * 1.5 = 82.5, // because rounded up to 83. - expect(first).toStrictEqual(83); - expect(second).toStrictEqual(83); + expect(first).toStrictEqual(new BigNumber(83)); + expect(second).toStrictEqual(new BigNumber(83)); expect(fetchBaseFeeSpy).toHaveBeenCalledTimes(1); }); diff --git a/packages/snap/src/services/transaction/TransactionMapper.test.ts b/packages/snap/src/services/transaction/TransactionMapper.test.ts index beb8c95d..65c34452 100644 --- a/packages/snap/src/services/transaction/TransactionMapper.test.ts +++ b/packages/snap/src/services/transaction/TransactionMapper.test.ts @@ -428,6 +428,7 @@ describe('TransactionMapper', () => { }, }, ], + // eslint-disable-next-line jest/no-conditional-in-test ...(details ? { details } : {}), }); },