feat: transaction scanning patch 1 - refactor transaction object and add get transaction(s) network utils by stanleyyconsensys · Pull Request #87 · MetaMask/snap-stellar-wallet

stanleyyconsensys · 2026-06-02T08:28:10Z

Explanation

This PR refactors transaction deserialization into the Transaction model (via factory methods) and
adds Horizon-backed transaction fetch/scan utilities to NetworkService, s
upporting transaction scanning and richer on-chain metadata (e.g., fee_charged).

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

Copilot

Pull request overview

This PR refactors transaction deserialization into the Transaction model (via factory methods) and adds Horizon-backed transaction fetch/scan utilities to NetworkService, supporting transaction scanning and richer on-chain metadata (e.g., fee_charged).

Changes:

Moved XDR deserialization from TransactionBuilder into Transaction (fromXdr, fromHorizon, fromRaw) and added feeCharged + id.
Added NetworkService.getTransaction and NetworkService.getTransactions (cursor pagination, bounded scanning).
Updated keyring signing flow and tests to use Transaction.fromXdr, plus new Horizon test fixtures/constants.

Reviewed changes

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

Show a summary per file

File	Description
packages/snap/src/services/transaction/TransactionService.ts	Uses `Transaction.fromXdr` for swap XDR validation flow.
packages/snap/src/services/transaction/TransactionBuilder.ts	Removes `deserialize` method (deserialization moved to `Transaction`).
packages/snap/src/services/transaction/TransactionBuilder.test.ts	Removes tests for the removed `deserialize` method.
packages/snap/src/services/transaction/Transaction.ts	Adds factory constructors, `feeCharged`, and `id` accessors.
packages/snap/src/services/transaction/Transaction.test.ts	Adds tests for `fromXdr`, `fromHorizon`, and invalid XDR behavior.
packages/snap/src/services/transaction/exceptions.ts	Introduces `TransactionDeserializationException`.
packages/snap/src/services/transaction/mocks/transaction.fixtures.ts	Adds Horizon transaction/page fixtures for tests.
packages/snap/src/services/network/NetworkService.ts	Adds `getTransaction`/`getTransactions` and maps Horizon records to `Transaction`.
packages/snap/src/services/network/NetworkService.test.ts	Adds test coverage for new Horizon transaction fetch/scan utilities.
packages/snap/src/handlers/keyring/signTransaction.ts	Switches signing flow deserialization to `Transaction.fromXdr`.
packages/snap/src/handlers/keyring/signTransaction.test.ts	Updates tests to align with new deserialization path.
packages/snap/src/handlers/keyring/exceptions.ts	Maps `TransactionDeserializationException` to SEP-43 InvalidRequest.
packages/snap/src/context.ts	Removes `transactionBuilder` injection into `SignTransactionHandler`.
packages/snap/src/constants.ts	Adds scan/page-size constants for Horizon transaction pagination.
packages/snap/src/api/xdr.ts	Uses `Transaction.fromXdr` for op-type inspection; adjusts networkId hashing logic.

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

stanleyyconsensys · 2026-06-02T08:59:55Z

+      const initialTransactionsResponse = await client
+        .transactions()
+        .forAccount(accountAddress)
+        .order(order)
+        .cursor(lastScanToken)
+        .limit(pageSize)
+        .call();


horizon will not reject if cursor= empty
e.g https://horizon.stellar.org/accounts/GA7UCNSASSOPQYTRGJ2NC7TDBSXHMWK6JHS7AO6X2ZQAIQSTB5ELNFSO/transactions?cursor=&limit=200&order=asc
where you find this incorrect information

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

stanleyyconsensys · 2026-06-03T07:06:45Z

+    let maxScanRemaining = maxScan;
+    let nextScanToken: string = lastScanToken;
+
+    try {


will use default page = 1 , instead of return empty

wantedsystem · 2026-06-03T12:56:20Z

+    includeSelfTransactionsOnly: boolean,
+  ): Transaction[] {
+    const result: Transaction[] = [];
+
+    for (const transaction of transactions) {
+      if (
+        (includeSelfTransactionsOnly &&
+          transaction.source_account === accountAddress) ||
+        !includeSelfTransactionsOnly


This filters self transactions only by Horizon source_account. That can skip valid wallet transactions where the wallet is the fee source or an operation source. Since the scan cursor still advances, those skipped records won’t be picked up later.

I’d deserialize first, then filter with transaction.isInvokedByAccount(accountAddress), or include fee_account in the check

im not sure yet

for fee bump txn
the txn can be not belong to me, should i show them, i think we didnt do that for other network

for sponser transaction
the txn is made for others, but im the one who pay the fee and execute it, should i show it on my broad?

for now, i only consider if the txn is execute by me

let reconfirm this later with others, and create a ticket to follow up?

Yes, i agree let's defer and open a follow-up ticket. Just let's reuse Transaction.isInvokedByAccount when we revisit so the scan matches the rest of the codebase

wantedsystem · 2026-06-03T13:08:20Z

+
+        // Non-empty page from `next()` should always expose a paging token on the last row.
+        const lastPageRecord = currentResponse.records.at(-1);
+        nextScanToken = lastPageRecord?.paging_token ?? '';


We support order: 'desc', but nextScanToken is always set to the last record in the page. For descending scans, that token points to the oldest fetched transaction, so using it for the next scan can skip newer transactions.
If desc is only for loading older history, we should return a separate cursor or document that clearly

it is very confusing if the nextScanToken has different position by order

i would suggest we store the horizon data in the txn object
and remove the return of last scan token from the func response

and the caller decide which scan token to be store

so if there is a case that the caller need continue scanning from desc order, it works
so if there is a case that the caller need scanning once from desc order, and then scan in asc order base on the first record return from desc order, it works too

the caller decide, rather than the function decide

we should make the func more general

wdyt?

Ok, sounds good just one thing: make sure the returned records keep their paging_token so the caller can actually choose where to resume from, and let's document which token to use for incremental (asc) vs backfill (desc)

wantedsystem

LGTM

stanleyyconsensys added 3 commits June 2, 2026 16:15

refactor: transaction object

08dd140

fix: lint

6dc0a61

fix: test

d6abbb5

stanleyyconsensys requested a review from a team as a code owner June 2, 2026 08:28

stanleyyconsensys requested a review from Copilot June 2, 2026 08:28

Copilot started reviewing on behalf of stanleyyconsensys June 2, 2026 08:28 View session

Merge branch 'main' into feat/transaction-scannning-1

996ed45

Copilot AI reviewed Jun 2, 2026

View reviewed changes

stanleyyconsensys added 3 commits June 2, 2026 18:25

fix: test and comment

0fc0719

fix: test

5328eba

add: transaction status

1cbfaf9

stanleyyconsensys requested a review from Copilot June 3, 2026 06:48

Copilot started reviewing on behalf of stanleyyconsensys June 3, 2026 06:48 View session

Copilot AI reviewed Jun 3, 2026

View reviewed changes

stanleyyconsensys added 2 commits June 3, 2026 15:05

fix: comment

240983a

fix: lint

8f0452b

wantedsystem reviewed Jun 3, 2026

View reviewed changes

stanleyyconsensys added 3 commits June 4, 2026 17:56

fix: address comment

459b136

Merge branch 'main' into feat/transaction-scannning-1

e2a8170

chore: remove txn builder import

a21f039

wantedsystem approved these changes Jun 4, 2026

View reviewed changes

stanleyyconsensys merged commit e811dd1 into main Jun 4, 2026
10 checks passed

stanleyyconsensys deleted the feat/transaction-scannning-1 branch June 4, 2026 10:31

Conversation

stanleyyconsensys commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation

References

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

stanleyyconsensys Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

stanleyyconsensys Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wantedsystem Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

stanleyyconsensys Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wantedsystem Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

wantedsystem Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

stanleyyconsensys Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wantedsystem Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

wantedsystem left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

stanleyyconsensys commented Jun 2, 2026 •

edited

Loading

stanleyyconsensys Jun 3, 2026 •

edited

Loading

stanleyyconsensys Jun 4, 2026 •

edited

Loading

stanleyyconsensys Jun 4, 2026 •

edited

Loading