Skip to content

feat: map SEP41 asset receive on transaction sync#109

Merged
stanleyyconsensys merged 6 commits into
mainfrom
feat/map-sep41-receive
Jun 15, 2026
Merged

feat: map SEP41 asset receive on transaction sync#109
stanleyyconsensys merged 6 commits into
mainfrom
feat/map-sep41-receive

Conversation

@stanleyyconsensys

@stanleyyconsensys stanleyyconsensys commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Explanation

This PR improves SEP-41 activity coverage by synthesizing “receive” entries for snap-managed recipients during transaction synchronization (since Horizon omits SEP-41 receives), and refactors sync orchestration so SEP-41 asset metadata is loaded once per sync run and passed into downstream sync steps. It also introduces a dedicated cron job to keep the asset catalog synchronized.

image

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

@stanleyyconsensys stanleyyconsensys requested a review from a team as a code owner June 15, 2026 03:14
@stanleyyconsensys stanleyyconsensys changed the base branch from main to feat/add-asset-sync June 15, 2026 03:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves SEP-41 activity coverage by synthesizing “receive” entries for snap-managed recipients during transaction synchronization (since Horizon omits SEP-41 receives), and refactors sync orchestration so SEP-41 asset metadata is loaded once per sync run and passed into downstream sync steps. It also introduces a dedicated cron job to keep the asset catalog synchronized.

Changes:

  • Add best-effort SEP-41 “receive” mapping during transaction history sync by cloning confirmed sender-side SEP-41 sends onto recipient accounts.
  • Load SEP-41 asset metadata once in SynchronizeService and pass it to on-chain account sync + transaction sync (plus add synchronizeAssets and a new cron job/handler).
  • Add AccountService.listAccountsByScope to support recipient lookup across all snap-managed accounts for a scope.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/snap/src/services/transaction/TransactionSynchronizeService.ts Adds SEP-41 receive mapping logic and replaces persisted asset lookup with preloaded metadata + account lookup.
packages/snap/src/services/transaction/TransactionSynchronizeService.test.ts Adds tests validating SEP-41 receive mapping for confirmed vs non-confirmed sends.
packages/snap/src/services/transaction/TransactionService.ts Threads preloaded SEP-41 assets through to TransactionSynchronizeService.
packages/snap/src/services/transaction/mocks/transaction.fixtures.ts Updates mock wiring to use AccountService instead of AssetMetadataService.
packages/snap/src/services/sync/SynchronizeService.ts Loads SEP-41 assets once per run, runs sync tasks concurrently, and adds synchronizeAssets.
packages/snap/src/services/sync/SynchronizeService.test.ts Updates tests for preloaded SEP-41 assets, failure modes, and asset sync delegation.
packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.ts Accepts preloaded SEP-41 assets instead of fetching internally.
packages/snap/src/services/on-chain-account/OnChainAccountSynchronizeService.test.ts Updates test setup/calls for the new sep41Assets parameter.
packages/snap/src/services/on-chain-account/OnChainAccountService.ts Threads preloaded SEP-41 assets through to OnChainAccountSynchronizeService.
packages/snap/src/services/on-chain-account/OnChainAccountService.test.ts Updates tests for the new sep41Assets parameter.
packages/snap/src/services/on-chain-account/mocks/onChainAccount.fixtures.ts Updates mock service construction after removing AssetMetadataService dependency.
packages/snap/src/services/asset-metadata/mocks/assets.fixtures.ts Adds getMockSep41Assets() helper for SEP-41 asset lists.
packages/snap/src/services/account/AccountService.ts Adds listAccountsByScope helper used by SEP-41 receive mapping.
packages/snap/src/handlers/cronjob/trackTransaction.test.ts Updates SynchronizeService construction with the new assetMetadataService dependency.
packages/snap/src/handlers/cronjob/syncAssets.ts Adds a cron handler to synchronize the asset catalog on mainnet.
packages/snap/src/handlers/cronjob/syncAssets.test.ts Adds tests verifying mainnet asset synchronization behavior.
packages/snap/src/handlers/cronjob/api.ts Adds JSON-RPC structs/types + method enum entry for synchronizeAssets.
packages/snap/src/handlers/cronjob/api.test.ts Adds struct validation tests for the new synchronizeAssets request.
packages/snap/src/context.ts Wires the new SyncAssets handler and updates service constructors.
packages/snap/snap.manifest.json Schedules the new synchronizeAssets cron job and bumps platformVersion.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Base automatically changed from feat/add-asset-sync to main June 15, 2026 03:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Comment on lines +54 to +55
/** All snap-managed accounts on `scope`, keyed by address for SEP-41 receive mapping. */
allAccountsByAddress: Map<StellarAddress, StellarKeyringAccount>;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StellarAddress is string, it is for for clear type explanation


async #loadAllAccountsByAddress(
scope: KnownCaip2ChainId,
): Promise<Map<StellarAddress, StellarKeyringAccount>> {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StellarAddress is string, it is for for clear type explanation

Comment on lines +196 to +199
const allAccountsByAddress = new Map<
StellarAddress,
StellarKeyringAccount
>();

@stanleyyconsensys stanleyyconsensys Jun 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

throw new TransactionMapperException(
'Unable to map a SEP-41 send transaction - not an invoke host function operation',
);
return undefined;

@stanleyyconsensys stanleyyconsensys Jun 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mute it for avoid too many error appear

as those are checking if the txn is a sep41 txn

it is false positive

@khanti42 khanti42 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@stanleyyconsensys stanleyyconsensys merged commit bf91a18 into main Jun 15, 2026
11 checks passed
@stanleyyconsensys stanleyyconsensys deleted the feat/map-sep41-receive branch June 15, 2026 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants