Skip to content

chore: remove allDetectedTokens assets references#30242

Open
bergarces wants to merge 6 commits into
mainfrom
all-detected-tokens-assets-references
Open

chore: remove allDetectedTokens assets references#30242
bergarces wants to merge 6 commits into
mainfrom
all-detected-tokens-assets-references

Conversation

@bergarces
Copy link
Copy Markdown
Contributor

@bergarces bergarces commented May 15, 2026

Description

allDetectedTokens has been deprecated and empty for a long time. We are now removing all remaining references to that piece of state since we are in the process of deprecating many assets controllers.

This should not have any effect in the app, as the content of that piece of state is always empty.

Changelog

CHANGELOG entry: null

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/ASSETS-3197

Manual testing steps

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

Before

After

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • Ideally on a mid-range device; emulator is acceptable
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens
  • I've instrumented key operations with Sentry traces for production performance metrics

For performance guidelines and tooling, see the Performance Guide.

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.

Note

Medium Risk
Removes navigation routes, UI screens, and selector logic for the detected-tokens import flow; if any remaining code paths still navigate to these screens or depend on allDetectedTokens, it could cause runtime navigation errors despite the state being deprecated.

Overview
Removes the deprecated Detected Tokens UI/flows: deletes the DetectedTokens and DetectedTokensConfirmation screens (and the Token row component), unregisters the modal route from App.tsx and Routes.ts, and strips the associated mocks/tests.

Eliminates remaining allDetectedTokens/detected-token selector plumbing by deleting selectDetectedTokens and the detected-token flattening selectors from tokensController.ts, plus removing related fixture/test helpers and references in Wallet tests (including the auto-import side effect coverage).

Reviewed by Cursor Bugbot for commit 45e7a76. Bugbot is set up for automated code reviews on this repo. Configure here.

@bergarces bergarces requested a review from a team as a code owner May 15, 2026 11:59
@github-actions
Copy link
Copy Markdown
Contributor

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.

[chainId],
);

useEffect(() => {
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.

This is not doing anything. Those arrays of detected tokens are always empty. Adding tokens to TokensController occurs at controller level.

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.

These components look like dead code.

@bergarces bergarces requested a review from a team as a code owner May 15, 2026 12:13
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ 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 8c3a204. Configure here.

Comment thread app/components/Views/Wallet/index.tsx
Comment thread app/selectors/tokensController.ts
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.

This is not how tokens get automatically imported anymore. It hasn't been for quite a while.

@bergarces
Copy link
Copy Markdown
Contributor Author

@metamaskbot update-mobile-fixture

@github-actions
Copy link
Copy Markdown
Contributor

🔄 Fixture update started. Running workflow from branch all-detected-tokens-assets-references. View workflow runs

@github-actions
Copy link
Copy Markdown
Contributor

E2E fixture update failed.\n\nCommon causes:\n- CI workflow is still running — wait for 'Build iOS Apps' to complete\n- CI workflow was skipped — ensure your PR has iOS-impacting changes or use skip-smart-e2e-selection label\n- iOS build failed — check the CI workflow for errors\n\nView logs and retry

@bergarces
Copy link
Copy Markdown
Contributor Author

@metamaskbot update-mobile-fixture

@github-actions
Copy link
Copy Markdown
Contributor

🔄 Fixture update started. Running workflow from branch all-detected-tokens-assets-references. View workflow runs

@github-actions
Copy link
Copy Markdown
Contributor

E2E fixture update failed.\n\nCommon causes:\n- CI workflow is still running — wait for 'Build iOS Apps' to complete\n- CI workflow was skipped — ensure your PR has iOS-impacting changes or use skip-smart-e2e-selection label\n- iOS build failed — check the CI workflow for errors\n\nView logs and retry

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

E2E fixture update failed.\n\nCommon causes:\n- CI workflow is still running — wait for 'Build iOS Apps' to complete\n- CI workflow was skipped — ensure your PR has iOS-impacting changes or use skip-smart-e2e-selection label\n- iOS build failed — check the CI workflow for errors\n\nView logs and retry

@github-actions
Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeWalletPlatform, SmokeNetworkAbstractions
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: medium
  • AI Confidence: 78%
click to see 🤖 AI reasoning details

E2E Test Selection:
This PR removes the deprecated "Detected Tokens" UI flow and related code:

  1. Deleted components: DetectedTokens bottom sheet, Token component, DetectedTokensConfirmation modal - these were the UI for manually reviewing/importing detected tokens.

  2. Removed from Wallet/index.tsx: The useEffect that auto-imported detected tokens when token detection was enabled. This is a significant behavior change in the core Wallet view.

  3. Removed from App.tsx navigation: The DetectedTokensFlow stack navigator and its route registration in RootModalFlow. This changes the navigation structure.

  4. Removed from Routes.ts: The DETECTED_TOKENS route constant.

  5. Removed deprecated selectors: selectDetectedTokens, selectAllDetectedTokensFlat, selectAllDetectedTokensForSelectedAddress, selectTokensControllerState from tokensController.ts.

  6. Removed from FixtureBuilder: withDetectedTokens() method - only used in the deleted test.

  7. Deleted E2E test: token-detection-import-all.spec.ts and DetectedTokensView.ts page object.

Why SmokeWalletPlatform: The Wallet view (index.tsx) had significant logic removed (auto-import useEffect, multiple selectors). This is the core wallet home screen and needs validation that it still renders correctly and doesn't break with the removed logic.

Why SmokeNetworkAbstractions: Token detection is network-aware (per-chain token detection). The removal of multi-network token detection logic and the network-related selectors from the wallet view warrants validation of network-related token display.

Not selecting SmokeConfirmations: The removed code was about token detection/import, not transaction confirmations.

Not selecting all tags: The changes are focused removals of deprecated code. The deleted E2E test was the only test using withDetectedTokens(). No other E2E tests reference the deleted components or selectors. The navigation change removes a route but doesn't modify existing routes.

Performance Test Selection:
The changes remove deprecated token detection UI and auto-import logic from the Wallet view. While the Wallet view is modified, the removed code was a useEffect for token auto-import which is not a performance-critical rendering path. No performance-sensitive components like account lists, asset loading, or app startup are affected. The removals should not negatively impact performance (if anything, removing the auto-import useEffect could slightly improve performance). No performance tests are warranted.

View GitHub Actions results

@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants