Skip to content

feat: Add Horizon-based transaction tracking and keyring settlement for trustline operation status tracking#51

Open
khanti42 wants to merge 17 commits intomainfrom
feat/chg-trust-refine
Open

feat: Add Horizon-based transaction tracking and keyring settlement for trustline operation status tracking#51
khanti42 wants to merge 17 commits intomainfrom
feat/chg-trust-refine

Conversation

@khanti42
Copy link
Copy Markdown
Contributor

@khanti42 khanti42 commented May 5, 2026

Explanation

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

@khanti42 khanti42 requested a review from a team as a code owner May 5, 2026 07:29
@khanti42 khanti42 changed the title feat: trustline operation status tracking feat: Add Horizon-based transaction tracking and keyring settlement for trustline operation status tracking May 5, 2026
Base automatically changed from feat/chg-trust to main May 5, 2026 07:40
Comment thread packages/snap/src/services/transaction/TransactionRepository.ts
Comment thread packages/snap/src/services/transaction/TransactionService.ts Outdated
Comment thread packages/snap/src/handlers/cronjob/trackTransaction.ts Outdated
Comment thread packages/snap/src/handlers/cronjob/trackTransaction.ts Outdated
Comment thread packages/snap/src/handlers/cronjob/trackTransaction.ts Outdated
Comment thread packages/snap/src/handlers/cronjob/trackTransaction.ts Outdated
@@ -48,10 +50,15 @@ export const TrackTransactionParamsStruct = type({
txId: nonempty(string()),
scope: KnownCaip2ChainIdStruct,
accountIds: nonempty(array(UuidStruct)),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

just wonder do we need accountIds

becoz txId is the source of truth, and we should only play with the accounts in the tx

attempt: attemptRaw ?? 0,
});

const accounts = await this.#accountService.findByIds(accountIds);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i think the idea is first poll transactions,

so we have to make the txid is exist first, and we know what accounts is invoke from that transaction

then if a status update
we update the tx status from the sender (add todo to remove it, becoz if we do txn history sync, it will be racing condition)

then we find the accounts (
becoz you dont use background job, but it will ensure the sync happen after status confirm,
however, keep in mind horizon is later than RPC
)
and we start to sync accounts in the transaction

* @param scope - CAIP-2 chain id (Horizon endpoint).
* @returns `pending` when the tx is not yet available (404); `success` / `failed` when present.
*/
async getHorizonTransactionInclusionStatus(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

do we still need this method?

Comment thread packages/snap/package.json Outdated
{
"name": "@metamask/stellar-wallet-snap",
"version": "0.0.1",
"version": "0.0.1-dev.170",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

do we need to update the version?

* @param params.accountIds - Accounts that may hold the tx (from the track job).
* @param params.status - {@link TransactionStatus.Confirmed} or {@link TransactionStatus.Failed}.
*/
async updateKeyringTransactionStatus(params: {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

add a todo comment to make sure we will drop this txn history scanning was done

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.

2 participants