Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
✨ Files requiring CODEOWNER review ✨✅ @MetaMask/confirmations (2 files, +6 -2)
|
Builds ready [3a7e4a4]
⚡ Performance Benchmarks (Total: 🟢 6 pass · 🟡 12 warn · 🔴 0 fail)
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs
|
## Explanation The goal of this PR is to add a `submissionMethod` property in the TransactionMetadata object in order to record how the transaction was submitted to the network and add it to the metrics. This datapoint can be recording at submission in the https://github.com/MetaMask/metamask-extension/blob/main/app/scripts/controller-init/confirmations/transaction-controller-init.ts Draft PR on extension to demonstrate the use case: MetaMask/metamask-extension#41465 <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## 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](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: this is a type-only change adding an optional metadata field and enum export, with no behavioral changes to transaction processing. > > **Overview** > Adds an optional `submissionMethod` field to `TransactionMeta` along with a new `TransactionSubmissionMethod` enum (currently `sentinel_stx` and `sentinel_relay`) so clients can persist how a transaction was submitted. > > Re-exports `TransactionSubmissionMethod` from the package entrypoint and documents the addition in the `transaction-controller` changelog. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit dd12d37. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
## Explanation The goal of this PR is to add a `submissionMethod` property in the TransactionMetadata object in order to record how the transaction was submitted to the network and add it to the metrics. This datapoint can be recording at submission in the https://github.com/MetaMask/metamask-extension/blob/main/app/scripts/controller-init/confirmations/transaction-controller-init.ts Draft PR on extension to demonstrate the use case: MetaMask/metamask-extension#41465 <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## 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](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: this is a type-only change adding an optional metadata field and enum export, with no behavioral changes to transaction processing. > > **Overview** > Adds an optional `submissionMethod` field to `TransactionMeta` along with a new `TransactionSubmissionMethod` enum (currently `sentinel_stx` and `sentinel_relay`) so clients can persist how a transaction was submitted. > > Re-exports `TransactionSubmissionMethod` from the package entrypoint and documents the addition in the `transaction-controller` changelog. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit dd12d37. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Builds ready [c53b994]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs
|
Builds ready [26df931]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs
|
Builds ready [52bec20]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 454987d. Configure here.
app/scripts/messenger-client-init/confirmations/transaction-controller-init.ts
Outdated
Show resolved
Hide resolved
Builds ready [7fbe3d4]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
Builds ready [8711eb7]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR - Cleans-up/Clarifies the semantic of the smartTransactions properties on the metametrics events - Adds a transaction_submission_method in the properties for transaction events to record the method of submission The current state is a first implementation that can be enriched with what submissions from the TransactionPay controller and the RPC provider if we need more granularity. PR on extension MetaMask/metamask-extension#41465 <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: update transaction transactions metrics to add details on smart transactions and submission method ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [ ] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches transaction submission hooks to dispatch new confirmation metrics fragments, which could affect transaction publishing flows if dispatch or store access misbehaves (though guarded by try/catch). Metrics semantics also change, which may impact downstream analytics expectations. > > **Overview** > **STX metrics semantics are updated** so events always include base flags (`is_smart_transactions_user_opt_in`, `is_smart_transactions_available`, `is_smart_transaction`), and finalized STX flows can additionally attach `stx_original_transaction_status` (via updated `getSmartTransactionMetricsProperties` / `getStxMetricsProperties`). > > **Transaction submission method is now recorded** by dispatching `updateConfirmationMetric` from `transaction-controller-init` when a publish path returns a hash, tagging `transaction_submission_method` as `sentinel_stx` (STX single + batch) or `sentinel_relay` (7702 delegation). Tests were updated/added to cover the new STX property behavior and the new metric dispatches. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 2aa038e. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->




This PR
The current state is a first implementation that can be enriched with what submissions from the TransactionPay controller and the RPC provider if we need more granularity.
Description
Changelog
CHANGELOG entry: update transaction transactions metrics to add details on smart transactions and submission method
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Touches transaction publish/publishBatch hooks and metrics plumbing, so a mistake could skew analytics or add noisy console errors, but it does not change transaction signing/broadcast behavior beyond extra try/catch instrumentation.
Overview
Improves transaction metrics for Smart Transactions (STX). Replaces the prior STX metrics fields (e.g.,
gas_included/timeout/proxy flags) with clearer booleans:is_smart_transactions_user_opt_in,is_smart_transactions_available, and the (now deprecated)is_smart_transaction, plusstx_original_transaction_statuswhen available.Adds submission-path attribution to transaction UI metrics.
TransactionControllerInitnow upserts atransaction_submission_methodfragment on successfulpublish/publishBatchSTX and delegation (Sentinel Relay) submissions, guarded so metrics failures don’t block returning atransactionHash; tests and console baseline are updated accordingly.Reviewed by Cursor Bugbot for commit 8711eb7. Bugbot is set up for automated code reviews on this repo. Configure here.