Skip to content

refactor: remove multichain accounts State 2 feature flag from confirmations#28136

Merged
jpuri merged 3 commits intomainfrom
vs/remove-multichain-state2-flag-confirmations
Mar 31, 2026
Merged

refactor: remove multichain accounts State 2 feature flag from confirmations#28136
jpuri merged 3 commits intomainfrom
vs/remove-multichain-state2-flag-confirmations

Conversation

@vinistevam
Copy link
Copy Markdown
Contributor

@vinistevam vinistevam commented Mar 31, 2026

Description

Removes selectMultichainAccountsState2Enabled selector usage from the confirmations area now that the multichain accounts State 2 / BIP-44 feature is considered stable. The legacy code path (when the flag returned false) is no longer needed.

Changes:

  • Simplified useSendScope hook to always return State 2 behavior (isBIP44: true, isSolanaOnly: false, isEvmOnly: false), removing the selectMultichainAccountsState2Enabled and selectSelectedInternalAccount selector dependencies.
  • Removed isBIP44 branching in RecipientList -- accounts always render as BIP44-grouped list; contacts always render as flat list.
  • Removed isBIP44 prop from Recipient component -- always displays accountGroupName (State 2 behavior) instead of conditionally choosing between accountGroupName and accountName.
  • Removed dead isSolanaOnly early-return in useEVMNfts hook (always false under State 2).
  • Cleaned up 7 test files: removed selector mocks, deleted legacy-path test suites, updated mock data.

Changelog

CHANGELOG entry: null

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/CONF-1088

Manual testing steps

Feature: Confirmation screens after removing multichain State 2 feature flag

  Scenario: Send flow shows grouped recipient list
    Given the user has multiple wallets with accounts
    When the user navigates to the Send screen
    And selects a recipient from the account list
    Then the accounts are grouped by wallet name
    And each account displays its account group name

  Scenario: Send flow shows flat contact list
    Given the user has saved contacts
    When the user navigates to the Send screen
    And switches to the Contacts tab
    Then the contacts are displayed in a flat list under a "Contacts" header

  Scenario: Personal sign confirmation renders correctly
    Given the user has a pending personal_sign request
    When the user views the confirmation screen
    Then the request origin, message, and account info are displayed correctly

  Scenario: Typed sign confirmation renders correctly
    Given the user has a pending eth_signTypedData request
    When the user views the confirmation screen
    Then the typed data fields and network info are displayed correctly

  Scenario: Approve/send transaction confirmation renders correctly
    Given the user initiates a token send or approve transaction
    When the user views the confirmation screen
    Then the sender account info, network, and transaction details are displayed correctly

Screenshots/Recordings

Before

After

Pre-merge author checklist

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 feature-flag/legacy branching in confirmation send UI and NFT hook behavior, which can change what names/lists render in edge cases. Mostly refactor/deletion, but touches user-facing recipient display and list grouping.

Overview
Confirmations no longer branch on the multichain State 2/BIP-44 feature flag. Recipient UI is simplified to always display accountGroupName (falling back to contactName) and drops the isBIP44 prop.

Send recipient lists now always group accounts by walletName (BIP-44-style) while contact lists stay flat with a fixed "Contacts" header, and the useSendScope hook and its tests are removed. useEVMNfts also drops the Solana-only early-return, with tests updated/trimmed to reflect the new single-path behavior and removed selector mocks.

Written by Cursor Bugbot for commit 7dceb22. This will update automatically on new commits. Configure here.

@vinistevam vinistevam added team-confirmations Push issues to confirmations team no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed labels Mar 31, 2026
@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.

@vinistevam vinistevam marked this pull request as ready for review March 31, 2026 05:26
@vinistevam vinistevam requested a review from a team as a code owner March 31, 2026 05:26
Copy link
Copy Markdown

@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.

@github-actions github-actions bot added the risk-medium Moderate testing recommended · Possible bug introduction risk label Mar 31, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

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

E2E Test Selection:
The changes in this PR involve:

  1. Deletion of useSendScope hook - This hook determined isSolanaOnly, isEvmOnly, and isBIP44 flags based on the selected account type and a multichain feature flag. Its removal simplifies the send flow logic.

  2. useNfts.ts change - Removed the isSolanaOnly early return guard. Previously, when a Solana account was selected, NFT loading would short-circuit and return empty. Now NFT loading proceeds for all account types. This could affect Solana send flows.

  3. recipient.tsx change - Removed isBIP44 prop; now always uses accountGroupName for display (previously fell back to accountName when not BIP44). This affects how recipient names are displayed in the send flow.

  4. recipient-list.tsx change - Removed isBIP44 gating; the BIP44 grouped layout is now always used (previously only when isBIP44 was true). The list now always shows the grouped wallet view rather than a flat account list.

SmokeConfirmations is selected because:

  • These changes directly affect the send transaction flow (recipient selection UI, NFT display in send)
  • The recipient list and recipient display components are used in transaction confirmation flows
  • Send flows for ETH, ERC-20 tokens are covered by SmokeConfirmations

SmokeNetworkExpansion is selected because:

  • The isSolanaOnly guard removal in useNfts.ts directly affects Solana account behavior
  • Solana send flows and account handling are tested in SmokeNetworkExpansion
  • Per the tag description, Solana transaction/signing flows hit confirmations, so SmokeConfirmations is already included

The changes are test-only for some files (test updates to reflect removed mocks), but the production code changes to recipient.tsx, recipient-list.tsx, and useNfts.ts affect real user-facing send flows that need E2E validation.

Performance Test Selection:
The changes are focused on UI component logic simplification (removing a feature flag hook and its conditional rendering). There are no changes to data loading performance, list rendering at scale, Redux state management, or app initialization. The recipient list and NFT hooks are not performance-critical paths that would warrant performance test execution.

View GitHub Actions results

@github-actions
Copy link
Copy Markdown
Contributor

E2E Fixture Validation — Schema is up to date
17 value mismatches detected (expected — fixture represents an existing user).
View details

@sonarqubecloud
Copy link
Copy Markdown

@jpuri jpuri added this pull request to the merge queue Mar 31, 2026
Merged via the queue into main with commit 44e69f3 Mar 31, 2026
99 checks passed
@jpuri jpuri deleted the vs/remove-multichain-state2-flag-confirmations branch March 31, 2026 11:06
@github-actions github-actions bot locked and limited conversation to collaborators Mar 31, 2026
@weitingsun weitingsun added release-7.73.0 Issue or pull request that will be included in release 7.73.0 and removed release-100.10.0 labels Mar 31, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed release-7.73.0 Issue or pull request that will be included in release 7.73.0 risk-medium Moderate testing recommended · Possible bug introduction risk size-M team-confirmations Push issues to confirmations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants