Skip to content

feat: non evm transaction type lifecycle events - #393

Merged
Battambang merged 11 commits into
mainfrom
feat/non-evm-transaction-type-lifecycle-events
Oct 1, 2026
Merged

Battambang merged 11 commits into
mainfrom
feat/non-evm-transaction-type-lifecycle-events

Conversation

@Battambang

Copy link
Copy Markdown
Contributor

Explanation

Transaction lifecycle events emitted by the non-EVM snaps could not be attributed to the flow that produced them. A Transaction Approved event, for example, is emitted by the classic wallet send, the dApp send, and the swap flow alike, and the payload carried only origin and no transaction_type, so nothing distinguished an in-app send from a signPsbt/sendTransfer confirmation.

This change distinguishes them by combining the two dimensions the schema already models: origin says who initiated the operation (metamask, a dApp origin, or cron), and transaction_type says what kind of operation it was, using the @metamask/keyring-api TransactionType vocabulary (send, receive, swap, ...). Together they identify a flow: classic wallet send is metamask + send, a dApp send is <dapp> + send, and so on.

@metamask/snap-networks-utils — AnalyticsService now accepts an optional transactionType and emits it as transaction_type on Transaction Added, Transaction Approved, Transaction Rejected, and Transaction Submitted, matching the existing behaviour of Transaction Finalized. The property is omitted entirely when not supplied, so existing callers are unaffected.

@metamask/bitcoin-wallet-snap — threads transaction_type through the send and sign confirmations plus the broadcast/cron tracking events. The type is derived from the transaction by a new mapToTransactionType helper: a positive sent amount maps to send, otherwise receive. The signPsbt confirmation reports unknown, since an arbitrary PSBT cannot be classified.

The helper lives in entities/transaction.ts rather than handlers/mappings.ts on purpose: it is a pure function over an already-fetched transaction, and mappings.ts imports runtime values from @metamask/bitcoindevkit, which breaks the Jest CommonJS environment. mapToTransactionType uses only types from that package, and mapToTransaction now consumes it so the send/receive classification has a single source of truth.

Only Bitcoin is covered here; the other non-EVM snaps can adopt the same optional property incrementally.

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

Transaction lifecycle events could not be attributed to a specific flow
across non-EVM snaps. Distinguish them by combining `origin` (who
initiated the operation) with `transaction_type` (what kind of operation
it was), using the `@metamask/keyring-api` `TransactionType` vocabulary.

- `snap-networks-utils`: `AnalyticsService` now accepts an optional
  `transactionType` and emits it as `transaction_type` on the
  `Transaction Added`, `Transaction Approved`, `Transaction Rejected`,
  and `Transaction Submitted` events, matching `Transaction Finalized`.
- `bitcoin-wallet-snap`: thread `transaction_type` through the send and
  sign confirmations plus the broadcast/cron tracking events, deriving
  it from the transaction with a new `mapToTransactionType` helper.
Reformat the files touched by the previous commit so `yarn lint:misc:check`
passes. Line wrapping only.
@Battambang Battambang changed the title Feat/non evm transaction type lifecycle events feat: non evm transaction type lifecycle events Sep 30, 2026
`applyUnconfirmedTx` takes ownership of the underlying wasm transaction,
so calling `mapToTransactionType(account, tx)` afterwards panicked with
"null pointer passed to rust" on every broadcast path (signPsbt,
broadcastPsbt, sendTransfer). Resolve the classification first and reuse
it for the submitted event.

Update the integration test assertions to include the new
`transaction_type` property, and ratchet the coverage thresholds.

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.

Copilot review overview

🟡 Changes recommended

The analytics API exposes a property that security methods silently discard, alongside maintainability and export-convention issues.

Review effort: Balanced
Findings: 1 Medium severity · 2 Low severity

Open (3)
What changed in this PR

Adds transaction_type attribution to Bitcoin transaction lifecycle analytics and shared analytics utilities.

Changes:

  • Adds optional lifecycle transaction classification.
  • Classifies Bitcoin transactions as send, receive, or unknown.
  • Updates tests, changelogs, coverage thresholds, and Snap manifest.
File Description
packages/​snap-networks-utils/​src/​services/​analytics/​AnalyticsService.ts Adds optional transaction types to lifecycle events.
packages/​snap-networks-utils/​src/​services/​analytics/​AnalyticsService.test.ts Tests lifecycle transaction types.
packages/​snap-networks-utils/​CHANGELOG.md Documents analytics changes.
packages/​bitcoin-wallet-snap/​src/​use-cases/​SendFlowUseCases.ts Classifies send confirmations.
packages/​bitcoin-wallet-snap/​src/​use-cases/​SendFlowUseCases.test.ts Tests send classifications.
packages/​bitcoin-wallet-snap/​src/​use-cases/​AccountUseCases.ts Classifies synchronized and broadcast transactions.
packages/​bitcoin-wallet-snap/​src/​use-cases/​AccountUseCases.test.ts Tests account lifecycle classifications.
packages/​bitcoin-wallet-snap/​src/​store/​JSXConfirmationRepository.tsx Classifies send and PSBT confirmations.
packages/​bitcoin-wallet-snap/​src/​store/​JSXConfirmationRepository.test.tsx Tests confirmation classifications.
packages/​bitcoin-wallet-snap/​src/​infra/​SnapClientAdapter.ts Emits transaction_type analytics properties.
packages/​bitcoin-wallet-snap/​src/​infra/​SnapClientAdapter.test.ts Tests emitted analytics payloads.
packages/​bitcoin-wallet-snap/​src/​handlers/​mappings.ts Reuses shared transaction classification.
packages/​bitcoin-wallet-snap/​src/​handlers/​CronHandler.ts Classifies cron-discovered transactions.
packages/​bitcoin-wallet-snap/​src/​handlers/​CronHandler.test.ts Tests cron classifications.
packages/​bitcoin-wallet-snap/​src/​entities/​transaction.ts Adds the classification helper.
packages/​bitcoin-wallet-snap/​src/​entities/​transaction.test.ts Tests send and receive mapping.
packages/​bitcoin-wallet-snap/​src/​entities/​snap.ts Extends Snap client tracking contracts.
packages/​bitcoin-wallet-snap/​src/​entities/​index.ts Exports the classification helper.
packages/​bitcoin-wallet-snap/​snap.manifest.json Updates the bundle checksum.
packages/​bitcoin-wallet-snap/​jest.config.js Raises coverage thresholds.
packages/​bitcoin-wallet-snap/​integration-test/​cron-sync.test.ts Verifies cron event transaction types.
packages/​bitcoin-wallet-snap/​integration-test/​client-request.test.ts Verifies submitted and finalized types.
packages/​bitcoin-wallet-snap/​CHANGELOG.md Documents Bitcoin analytics changes.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/snap-networks-utils/src/services/analytics/AnalyticsService.ts Outdated
Comment thread packages/bitcoin-wallet-snap/src/entities/index.ts Outdated
Comment thread packages/snap-networks-utils/src/services/analytics/AnalyticsService.ts Outdated
`SecurityAlertDetectedEventProperties` and
`SecurityScanCompletedEventProperties` extended `TransactionEventProperties`,
so adding the optional `transactionType` there advertised it to the security
tracking methods as well. Those methods never emit the property, so a
type-valid caller value was silently discarded.

Introduce an `AccountEventProperties` base with the fields every account-scoped
event shares, and extend that from the security event types instead. The
transaction lifecycle types keep `transactionType`.
Replacing `export type * from './transaction'` with `export *` reintroduced a
wildcard value export, contrary to the explicit-export convention in
AGENTS.md. Name the new runtime symbol and the existing type individually so
the module surface stays explicit.
Adding the optional `transaction_type` field had inlined the shared payload
into all four lifecycle methods, so the common properties would have to be
changed in four places and could drift.

Restore the `#trackTransactionEvent` helper and build the payload, including
the conditional `transaction_type`, in that single place.

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.

Copilot review overview

🟢 Approval recommended

The implementation and tests are coherent; the only finding is a non-blocking JSDoc omission.

Review effort: Balanced
Findings: None

Resolved since last review (3)
Previously missed (1)

In code that hasn't changed since last review

Low severity Add missing JSDoc @​param entry for required parameter

packages/​bitcoin-wallet-snap/​src/​entities/​snap.ts:286

The new required parameter is missing from this public interface method's JSDoc, while the other parameters and neighboring tracking methods are fully documented. Add an @param entry so generated API documentation explains the argument.

@Battambang

Copy link
Copy Markdown
Contributor Author

@metamaskbot publish-preview

@github-actions

Copy link
Copy Markdown
Contributor

Preview builds have been published. Learn how to use preview builds in other projects.

Expand for full list of packages and versions.
@metamask-previews/bitcoin-wallet-snap@3.1.0-preview-f8e3e3d0
@metamask-previews/snap-networks-utils@1.0.0-preview-f8e3e3d0
@metamask-previews/solana-wallet-snap@7.0.0-preview-f8e3e3d0
@metamask-previews/stellar-wallet-snap@1.1.0-preview-f8e3e3d0
@metamask-previews/tron-wallet-snap@4.0.0-preview-f8e3e3d0

@Battambang
Battambang marked this pull request as ready for review September 30, 2026 21:09
@Battambang
Battambang requested a review from a team as a code owner September 30, 2026 21:09
@Battambang
Battambang deployed to default-branch September 30, 2026 21:10 — with GitHub Actions Active
@Battambang
Battambang added this pull request to stack #400 October 1, 2026 08:53
Battambang added a commit that referenced this pull request Oct 1, 2026
The entry referenced the stack base (#393) because the branch had no PR
number yet. Use the actual PR.
);

await this.#snapClient.trackTransactionAdded(account, origin);
await this.#snapClient.trackTransactionAdded(

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.

This transaction type could be categorized as Send when the PSBT spends the account's own inputs and otherwise unknown. (Applied to the 3 trackTransaction in this function)

const [sent] = account.sentAndReceived(psbt.unsigned_tx);
const transactionType = sent.to_sat() > 0n ? TransactionType.Send : TransactionType.Unknown;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have fixed by classifying a spending signPsbt confirmation as a send in this commit
53441fb

account: BitcoinAccount,
tx: WalletTx,
origin: string,
transactionType: TransactionType,

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.

Nit: miss the JSDoc for transactionType

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Documentation updated with commit: a4239f8

* callers pass a value that is silently discarded.
*/
export type TransactionEventProperties = AccountEventProperties & {
transactionType?: string;

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.

nit: should be TransactionType type instead of string no ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated the type: 6ab67ef

A signPsbt confirmation always reported unknown, so a PSBT that spends
this account's inputs disagreed with the submitted event. Report send
when the account spends its own inputs, and keep unknown otherwise.
The strict enum rejected `Transaction['type']`, which the keyring API
types as a string literal union, so the Solana finalized event failed
type-checking. Use the template literal form, which accepts both the
enum members and that union while keeping the vocabulary constrained.
@sonarqubecloud

sonarqubecloud Bot commented Oct 1, 2026

Copy link
Copy Markdown

@Battambang
Battambang requested a review from Julink-eth October 1, 2026 14:01
@Battambang
Battambang added this pull request to the merge queue Oct 1, 2026
Merged via the queue into main with commit b07c477 Oct 1, 2026
56 checks passed
@Battambang
Battambang deleted the feat/non-evm-transaction-type-lifecycle-events branch October 1, 2026 14:09
Battambang added a commit that referenced this pull request Oct 1, 2026
The entry referenced the stack base (#393) because the branch had no PR
number yet. Use the actual PR.
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