test: migrate tron snap#15
Draft
mikesposito wants to merge 247 commits into
Draft
Conversation
feat: add test dApp
…ements More improvements
…tion feat: implement account derivation
feat: refactor accounts service
chore: TRX and TRC10 assets
chore: TRX and TRC10 assets
feat: fully support TRX + token assets balances
This pull request adds support for handling NFT asset types (ERC721 and ERC1155) in transaction scan simulations and refactors the asset mapping logic for clarity and extensibility. The changes ensure that NFT transfers are correctly displayed in scan results and thoroughly tested. **NFT and asset handling improvements:** * Added support for parsing and displaying ERC721 and ERC1155 NFT asset changes in transaction scan simulations, including handling `token_id`, `value`, and `logo_url` fields. * Refactored asset change mapping logic in `TransactionScanService` to use type-safe discriminated unions for different asset types and to compute display values appropriately for fungible and NFT assets. **Testing enhancements:** * Added comprehensive tests for ERC721 and ERC1155 asset changes to ensure correct mapping and display in scan results. **Schema and dependency updates:** * Updated the asset change struct definitions to introduce separate structures for fungible assets, ERC721, and ERC1155, and unified them under a union type. * Updated the `snap.manifest.json` source hash to reflect the latest build. **NOTES:** The new data structure in `structs.ts` is based **from the Blockaid openapi documentation.** **Screenshots:** <img width="473" height="1281" alt="NFT" src="https://github.com/user-attachments/assets/e3a0447e-fdb5-46f1-b7ec-be3c95c52256" />
This is the release candidate for version 1.25.1. --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Ulisses <ulisses@hey.com>
- Relax Trongrid legacy `internal_transactions` validation so sparse payloads do not cause transaction sync to fail unexpectedly. - Handle TRC10 token identifiers correctly in the current Trongrid account-history transaction flow, without mis-parsing endpoint-specific `asset_name` formats. - Decouple assets and transactions synchronization so one failure does not prevent the other from completing.
This is the release candidate for version 1.25.2. --------- Co-authored-by: github-actions <github-actions@github.com>
## Explanation <!-- 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? --> After a confirmation dialog is dismissed, its interface ID remained stored in state. Background cronjobs would then try to update the stale interface, causing "interface with id ... not found" errors. This adds a #clearInterfaceId helper that nullifies the stored ID right after the dialog resolves, for both the confirmTransactionRequest and #handleSignTransactionRequest flows. ## 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 --> NEB-322 ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] 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
This PR fixes stale Tron asset state by making asset persistence and event emission snapshot-aware. In `AssetsRepository.saveMany`, the latest fetched assets are now treated as the full snapshot for each account + network pair, so assets missing from a refreshed network slice are removed instead of being kept forever by upsert-only behavior. In `AssetsService.saveMany`, the current snapshot is compared with the previously saved assets to detect assets that disappeared completely from the latest response. Those assets are emitted in `AccountAssetListUpdated.removed`, and synthetic zero-balance entries are included in `AccountBalancesUpdated` so clients can clear stale cached balances even when the backend omits zero-balance tokens. This keeps the snap state and emitted keyring events aligned with the latest account asset snapshot, preventing stale assets and balances from remaining visible in clients. ## Explanation <!-- 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 - [X] I've updated the test suite for new or updated code as appropriate - [X] 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 --------- Signed-off-by: gabrieledm <gabriele.delmonte-ext@consensys.net> Co-authored-by: Michele Esposito <34438276+mikesposito@users.noreply.github.com>
This is the release candidate for version 1.25.3. --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Michele Esposito <michele@esposito.codes>
… Solana snap (#257) ## Explanation <!-- 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? --> - Simplified `SnapClient` interface methods by removing the defensive `*IfExists` wrappers (`isInterfaceNotFoundError`, `updateInterfaceIfExists`, `getInterfaceContextIfExists`) and replacing them with plain `getInterfaceContext` and `updateInterface` methods, aligning with the Solana snap's approach - Removed scattered state cleanup (`setKey(..., null)`) and complex error-recovery logic from background handlers (`cronjob.tsx`), letting the outer try/catch handle race conditions naturally - Removed null-check-on-update-result pattern from `render.tsx` files ## 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
## Explanation Fixes the Unreleased changelog entry for #298 so it links to the Tron wallet snap repository instead of the Solana wallet snap repository. The production snap manifest shasum was updated by the standard pre-commit hook. ## References - Corrects the changelog link for #298 ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] 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
This is the release candidate for version 1.25.4. --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
## Explanation <!-- 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? --> This change validates the transaction `owner_address` against the signer address derived from persisted account before signing or broadcasting. This ensures we only sign or broadcast transactions whose `owner_address` actually belongs to the keypair being used, preventing mismatched-signer flows and the related signature validation failures. ## 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 --> Little video record in which I forced a wrong address to check the UI behavior https://github.com/user-attachments/assets/c4b78f8e-c601-4a5a-9963-505fa60004ad ## Checklist - [X] I've updated the test suite for new or updated code as appropriate - [X] 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
This is the release candidate for version 1.25.5. --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: gabrieledm <gabriele.delmonte-ext@consensys.net>
## Summary - Add a shared TRON transaction metadata freshness check for expiration and TAPOS reference block fields. - Refresh stale or invalid `expiration`, `ref_block_bytes`, and `ref_block_hash` before signing in both send/broadcast paths. - Preserve valid transactions without rebuilding derived fields, and return a clear rebuild-and-retry error when metadata cannot be refreshed. - Update Snap coverage thresholds after the pre-push test hook reported improved coverage. --------- Co-authored-by: gabrieledm <gabriele.delmonte-ext@consensys.net>
## Summary - Add a post-mapping spam filter for tiny native TRX receive transactions. - Keep the filter conservative: sends, token transfers, swaps, staking withdrawals, failed/unknown transactions, unknown chains, and invalid metadata are preserved. - Cover the utility directly and prove `TransactionsService.fetchNewTransactionsForAccount()` filters only after `TransactionMapper.mapTransactions(...)`. ## Screenshots **Before** <img width="2191" height="302" alt="ExistingSpam" src="https://github.com/user-attachments/assets/94a169d4-1475-4d9b-8e4f-065cd3742bee" /> **After** <img width="2132" height="243" alt="NoMoreSpam" src="https://github.com/user-attachments/assets/47554112-5ead-4f55-af4e-caa6220b8591" /> Co-authored-by: Maxime OUAIRY <maxime.ouairy-ext@consensys.net>
This is the release candidate for version 1.25.6. --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: gabrieledm <gabriele.delmonte-ext@consensys.net>
## Explanation <!-- 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? --> - When a confirmation tx is displayed, we poll the scan (https://security-alerts.api.cx.metamask.io/tron/transaction/scan) on a regular basis. - Every time it is polling, status becomes FetchStatus.Fetching. Regardless of it being the first time or the nth time. - In the [UI we use that status](https://github.com/MetaMask/snap-tron-wallet/blob/1b88152082e68f7f260d21738f2a4db98ccd9b76/packages/snap/src/ui/confirmation/views/ConfirmTransactionRequest/ConfirmTransactionRequest.tsx#L49) to decide when to gray out the "Confirm" button, which means the button is grayed every xx secs whenever that request is back in-flight. - We now want this button to only be grayed during the first fetch. This PR adds `FetchStatus.Loading` ontop of `FetchStatus.Fetching` to have a nomenclature close to TanStack when fetching means first or nth time and loading means "first time". It is not perfect because loading overlaps fetching so it forces to change some checks in the code loading || fetching to reach the same behavior. For that reason the helper `isFetchStatusLoadingOrFetching` has also been created for when we don't want to differenciate first loading from subsequent loading. ## 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 --> Fixes https://consensyssoftware.atlassian.net/browse/NEB-1126 ## Test intructions - Go to [Sun](https://sun.io/#/home) (for example). - Initiate a Swap (ex: 1 TRX -> USDT). - Observe confirmation view: - Before this PR, both "estimated changes" block AND the Confirm button get disabled / refresh UX every 15-20 secs. - After this PR, the Confirm button will only be disabled when loading information for the first time. Refreshes should only reflect on the "estimated changes" block but not on the Confirm button anymore. Note: After 1 minute or so, and expiration alert is shown and the Confirmation button is permanently disabled. This is not in the scope of this PR and is being worked on in parallel. ## 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** > Targeted confirmation UX and enum helper changes; background refresh already used Fetching—behavior change is limited to button enablement and status naming. > > **Overview** > Introduces **`FetchStatus.Loading`** for the first security-scan / price fetch, while **`FetchStatus.Fetching`** is reserved for later poll refreshes. Confirmation flows now open with **`Loading`** instead of **`Fetching`**, and the **Confirm** button is disabled only during **`Loading`** (or failed simulation)—not during periodic scan refreshes. > > Shared UI (`EstimatedChanges`, `TransactionAlert`, `Fees`) still shows loading skeletons on both states via **`isFetchStatusLoadingOrFetching`**, so estimated changes can refresh visually without blocking confirmation again. Tests and changelog are updated accordingly; manifest shasum and Jest coverage thresholds are bumped. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit a6d13a983a527494c734d58b3b5a980ac7c56ba3. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
## Explanation <!-- 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? --> Introduces a `SnapClient.trackError(error)` method. The new method is a wrapper around the `snap_trackError` RPC method, enabling services and handlers to forward errors to Sentry for monitoring. This is a prerequisite for the remaining subtasks under `NEB-1232` (https://consensyssoftware.atlassian.net/browse/NEB-1232) (audit of caught and suppressed errors in the Tron Snap), which will use this wrapper to report previously swallowed errors. ## 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 --> * Fixes https://consensyssoftware.atlassian.net/browse/NEB-1241 ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] 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** > Small, additive API and permission change with defensive error handling; no changes to signing, keys, or transaction flows in this PR. > > **Overview** > Adds **`SnapClient.trackError`** so the snap can report errors to MetaMask/Sentry via the **`snap_trackError`** RPC, using **`getJsonError`** for serialization (including nested **`cause`**). Failures on that RPC are **logged and swallowed** (returns **`undefined`**) so callers in existing error paths are not disturbed. > > **`SnapClient`** now takes a required **`logger`** in its constructor (wired from **`context.ts`**). The snap manifest gains the **`snap_trackError`** permission and an updated bundle **shasum**; changelog and Jest coverage thresholds are bumped slightly. Unit tests cover success, RPC failure, and cause chaining. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 9a57cb9ba1fe9c1813bb3a0c72b6c82bb54d9500. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…ce NEB-1233 (#314) ## Explanation <!-- 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? --> Invoke the snap client `trackError` method in the catch block, and add a test covering this error tracking behavior. ## 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 --> * Fixes [NEB-1233](https://consensyssoftware.atlassian.net/browse/NEB-1233) ## Checklist - [x] 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 [NEB-1233]: https://consensyssoftware.atlassian.net/browse/NEB-1233?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Adds observability on an existing error path only; no change to scan logic, permissions, or user-facing transaction behavior. > > **Overview** > When transaction security scanning fails (e.g. the security alerts API throws), **`TransactionScanService.scanTransaction`** now calls **`SnapClient.trackError`** in the existing `catch` block so failures can be reported via the snap’s `snap_trackError` path (e.g. Sentry), alongside existing logging and optional `trackSecurityScanCompleted` analytics. > > A unit test asserts **`trackError`** is invoked with the thrown error when **`scanTransaction`** rejects. Jest global coverage thresholds are bumped slightly, and **`snap.manifest.json`** source shasum is updated for the rebuilt bundle. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 189bd346552d87b143353009a241c0022cb00735. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…5 (#315) ## Explanation <!-- 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? --> Invoke the snap client trackError method in the catch blocks of `onAddressInput` and `onAmountInput`, and add tests covering the error tracking behavior. ## 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 --> * Fixes [NEB-1235](https://consensyssoftware.atlassian.net/browse/NEB-1235) ## Checklist - [X] 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 [NEB-1235]: https://consensyssoftware.atlassian.net/browse/NEB-1235?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Changes only add error telemetry in existing catch paths; validation responses and business logic are unchanged. > > **Overview** > Send-flow validation in `ClientRequestHandler` now reports failures to MetaMask via **`SnapClient.trackError`** in the **`catch`** blocks of **`onAddressInput`** and **`onAmountInput`**, while still returning the same invalid validation responses to the client. > > Tests consolidate setup behind a shared **`withClientRequestHandler`** helper (including a mock **`trackError`**) and add coverage that both methods call **`trackError`** on errors. Jest global coverage thresholds and **`snap.manifest.json`** `shasum` were updated to match the new bundle/tests. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 7e4e819ee1526b2e5848d786b5c433154c898905. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
This is the release candidate for version 1.25.7. --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Maxime OUAIRY <maxime.ouairy-ext@consensys.net>
## Explanation <!-- 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? --> The `snap_trackError` permission was added to the snap manifest by mistake, and causes consumers to break with an error from SnapController, as the permission does not exist. ## 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** > Manifest-only fix with no runtime logic changes; reduces install failures without altering wallet behavior. > > **Overview** > Removes the invalid **`snap_trackError`** entry from **`snap.manifest.json`** `initialPermissions` so SnapController no longer rejects installation for an unknown permission. The manifest **`source.shasum`** is updated to match the rebuilt bundle, and **Unreleased** changelog documents the install fix. > > In-app **`SnapClient.trackError`** / Sentry reporting added in 1.25.7 is unchanged; only the mistaken manifest permission is dropped. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 38c91be02f3fb531b2858ba027ce0d836d7b3b08. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
This is the release candidate for version 1.25.8. --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: mikesposito <michele@esposito.codes>
Adds an atomic setKeyWith state helper and exclusive write locking so read-modify-write updates cannot interleave. Includes in-memory state support, mock updates, and concurrency coverage. In simple words, the main purpose of `setKeyWith` is to safely update state when the new value depends on the existing value.
## Explanation <!-- 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? --> Track errors from `AssetsService` and add tests verifying error tracking. ## 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 --> * Fixes [NEB-1238](https://consensyssoftware.atlassian.net/browse/NEB-1238) ## Checklist - [X] 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 [NEB-1238]: https://consensyssoftware.atlassian.net/browse/NEB-1238?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Changes add observability only; existing catch-and-degrade behavior is unchanged, and trackError is designed not to throw on RPC failure. > > **Overview** > **AssetsService** now depends on **`SnapClient`** and reports selected API failures to MetaMask via **`trackError`** (Sentry), without changing existing graceful fallbacks. > > On **`fetchAssetsAndBalancesForAccount`**, errors from the inactive-account TRC20 balance fallback and from **`getMultipleSpotPrices`** are tracked before returning empty data as before. **`getHistoricalPrice`** tracks per-interval failures the same way while still returning null intervals for failed periods. > > Wiring passes **`snapClient`** from **`context.ts`** into the service constructor; tests mock **`trackError`** and assert it is invoked for those three paths. Jest global coverage thresholds are raised slightly, and **`snap.manifest.json`** source shasum is updated for the rebuilt bundle. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit c275315ad65d8cb826b69be73183cdc6738cbf0d. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…NEB-1236 (#316) ## Explanation <!-- 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? --> Preserve original errors as cause data, and add tests for cause propagation. I removed trackError calls from the keyring & ConfirmationHandler, as we have a global error track in `withCatchAndThrowSnapError`, which we use at entry points. Here is a [link](https://github.com/MetaMask/snap-tron-wallet/pull/323/changes#diff-cb255c61da01dabc2ac38e717797a307783bd19eca0333925d7aa899a176fdf3R110) for the changes ## 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 --> * Fixes [NEB-1236](https://consensyssoftware.atlassian.net/browse/NEB-1236) ## Checklist - [X] 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 [NEB-1236]: https://consensyssoftware.atlassian.net/browse/NEB-1236?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Observability and error-shape changes only; no new user flows or security-sensitive logic beyond clearer failure reporting. > > **Overview** > Improves **error chaining** so failures from account listing/creation, response validation, and Snap preference loading keep the underlying error on **`cause`** (and clearer messages where applicable). > > **`KeyringHandler`** wraps `listAccounts` / `createAccount` failures with `{ cause: error }`. **`validateResponse`** no longer throws a generic `SnapError`; it throws a standard **`Error`** whose message includes the superstruct detail and sets **`cause`** to the validation error. **`ConfirmationHandler`** includes the preferences failure message in its **`InternalError`**. > > Adds unit tests for **`cause`** on list/create account and for invalid **`validateResponse`** output. Jest **coverage thresholds** and **`snap.manifest.json`** bundle **shasum** are updated for the new/changed code. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit f0d9f313ba1b2973c6bf575881ed46e9465be331. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
## Explanation <!-- 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? --> Track error in the `clearInterfaceId` method of `ConfirmationHandler` and add a test verifying error tracking. Extends `UrlStruct` validation error with the real cause of the failure and adds a relevant test. ## 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 --> * Fixes [NEB-1240](https://consensyssoftware.atlassian.net/browse/NEB-1240) ## Checklist - [X] 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 [NEB-1240]: https://consensyssoftware.atlassian.net/browse/NEB-1240?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Small observability and validation-message changes with tests; no auth, signing, or transaction logic changes beyond error reporting on state cleanup failure. > > **Overview** > When clearing confirmation UI state fails in **`ConfirmationHandler`**, failures are now reported via **`SnapClient.trackError`** in addition to existing logging, with a new unit test for that path. > > **`UrlStruct`** parse failures now surface the underlying parser message (e.g. `Invalid URL format: Invalid URL`) instead of a generic message, plus a matching test. Jest coverage thresholds and **`snap.manifest.json`** bundle shasum were bumped for the rebuilt snap. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 3473e5370dd090d1487a06bc82117b6b5ad7844b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
15 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
This is a test PR for validating instructions included in the Snap migration guide PR#5
References
Checklist