Skip to content

Commit 99ff9c5

Browse files
authored
feat: replace assets state references confirmations (#29726)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until this PR meets the canonical Definition of Ready For Review in `docs/readme/ready-for-review.md`. In short: the template must be materially complete (not just section titles present), all status checks must be currently passing, and the only expected follow-up commits must be reviewer-driven. --> ## **Description** <!-- 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? --> The new `AssetsController` is being introduced to replace most controllers from `@metamask/assets-controllers`. This is one of many PRs that replace direct access to legacy state with selectors that, using a feature flag, handle the transition between the legacy state and the new state when the flag is turned on. ## **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: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/ASSETS-2827 ## **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** <!-- Every checklist item must be consciously assessed before marking this PR as "Ready for review". A checked box means you deliberately considered that responsibility, not that you literally performed every action listed. Unchecked boxes are ambiguous: they are not an implicit "N/A" and they are not a silent "skip". See `docs/readme/ready-for-review.md` for the full checklist semantics. --> - [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 - [X] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [X] 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. #### 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](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [ ] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **Pre-merge reviewer checklist** <!-- Reviewer checklist items follow the same semantics as the author checklist: an unchecked box is ambiguous, a checked box means the reviewer consciously assessed that responsibility. See `docs/readme/ready-for-review.md`. --> - [ ] 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** > Updates how native balances are looked up for `gas_insufficient_native_asset`, which could change metrics output if address casing/normalization differs across callers. Scoped to analytics/metrics code with test adjustments, but affects transaction-finalized event properties. > > **Overview** > Updates gas metrics to read account balances via the `selectAccountsByChainId` selector instead of directly accessing `AccountTrackerController` state, aligning with the ongoing assets-state migration. > > `getNativeBalance` now resolves the sender address with `safeToChecksumAddress` and looks up balances by checksummed key (removing the previous `.toLowerCase()` access). Tests were updated to use a realistic checksummed address and store balances under that exact key. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit f71e6c6. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 3e1cdcf commit 99ff9c5

2 files changed

Lines changed: 23 additions & 8 deletions

File tree

  • app/core/Engine/controllers/transaction-controller/metrics_properties

app/core/Engine/controllers/transaction-controller/metrics_properties/gas.test.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,20 @@ const mockGetNativeTokenAddress = getNativeTokenAddress as jest.MockedFunction<
1414
typeof getNativeTokenAddress
1515
>;
1616

17+
const MOCK_CHECKSUMMED_ADDRESS = '0x44934055428d2eF7E3F97D98187f2459007fa49F';
18+
1719
const createMockState = (
1820
balance: string = '0x100000000000000000',
1921
chainId: string = '0x1',
20-
address: string = '0xuser',
22+
address: string = MOCK_CHECKSUMMED_ADDRESS,
2123
): RootState =>
2224
({
2325
engine: {
2426
backgroundState: {
2527
AccountTrackerController: {
2628
accountsByChainId: {
2729
[chainId]: {
28-
[address.toLowerCase()]: { balance },
30+
[address]: { balance },
2931
},
3032
},
3133
},
@@ -40,7 +42,11 @@ const createMockRequest = (
4042
eventType: TRANSACTION_EVENTS.TRANSACTION_FINALIZED,
4143
transactionMeta: {
4244
chainId: '0x1',
43-
txParams: { from: '0xuser', gas: '0x5208', gasPrice: '0x1' },
45+
txParams: {
46+
from: MOCK_CHECKSUMMED_ADDRESS,
47+
gas: '0x5208',
48+
gasPrice: '0x1',
49+
},
4450
...overrides,
4551
} as TransactionMeta,
4652
allTransactions: [],
@@ -139,7 +145,7 @@ describe('getGasMetricsProperties', () => {
139145
const request = createMockRequest(
140146
{
141147
txParams: {
142-
from: '0xuser',
148+
from: MOCK_CHECKSUMMED_ADDRESS,
143149
gas: '0x5208',
144150
gasPrice: '0x100000000000000000',
145151
},
@@ -156,7 +162,11 @@ describe('getGasMetricsProperties', () => {
156162
const state = createMockState('0x100000000000000000');
157163
const request = createMockRequest(
158164
{
159-
txParams: { from: '0xuser', gas: '0x1', gasPrice: '0x1' },
165+
txParams: {
166+
from: MOCK_CHECKSUMMED_ADDRESS,
167+
gas: '0x1',
168+
gasPrice: '0x1',
169+
},
160170
},
161171
state,
162172
);

app/core/Engine/controllers/transaction-controller/metrics_properties/gas.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import type {
1111
TransactionMetricsBuilderRequest,
1212
} from '../types';
1313
import type { RootState } from '../../../../../reducers';
14+
import { selectAccountsByChainId } from '../../../../../selectors/accountTrackerController';
15+
import { safeToChecksumAddress } from '../../../../../util/address';
1416

1517
export function getGasMetricsProperties({
1618
transactionMeta,
@@ -88,10 +90,13 @@ function getNativeBalance(
8890
chainId: string,
8991
address: string,
9092
): BigNumber {
91-
const accountsByChainId =
92-
state.engine?.backgroundState?.AccountTrackerController?.accountsByChainId;
93+
const accountsByChainId = selectAccountsByChainId(state);
9394

94-
const account = accountsByChainId?.[chainId]?.[address?.toLowerCase()];
95+
const checksummedAddress = safeToChecksumAddress(address);
96+
const account =
97+
checksummedAddress !== undefined
98+
? accountsByChainId?.[chainId]?.[checksummedAddress]
99+
: undefined;
95100

96101
return new BigNumber((account?.balance as Hex) ?? '0x0');
97102
}

0 commit comments

Comments
 (0)