Skip to content

Commit 90d9a9d

Browse files
authored
fix: Fix ens domain name resolution in confirmation after send flow (#37047)
<!-- 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** <!-- 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? --> [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/37047?quickstart=1) ## **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: Fix showing ENS recipient if it's typed in the send flow ## **Related issues** Fixes: #36966 ## **Manual testing steps** 1. Go to send flow 2. Pick ENS recipient 3. Proceed into confirmation - you should be able to see ENS in the confirmation now ## **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 Extension Coding Standards](https://github.com/MetaMask/metamask-extension/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-extension/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] > On EVM sends, dispatch a domain reverse lookup to populate store for ENS display in confirmation; `lookupDomainName` now accepts an explicit `chainId` and tests updated accordingly. > > - **Hooks (send confirmations)**: > - In `useNameValidation`, after a successful name resolution, dispatch `lookupDomainName(to, chainId)` when `useSendType().isEvmSendType` is true to populate store for ENS display. > - Add `useDispatch` and `useSendType` usage; update hook dependencies. > - **State (domains duck)**: > - Update `lookupDomainName(domainName, chainId)` to accept optional `chainId`, defaulting to `getCurrentChainId(state)`; compute `chainIdInt` from the final value. > - **Tests**: > - Mock `useDispatch`, `lookupDomainName`, and `useSendType` in `useNameValidation.test.ts`. > - Add test asserting `lookupDomainName` is called with `('test.eth', chainId)` on EVM send after resolution. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 0478753. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent fd107ea commit 90d9a9d

File tree

3 files changed

+75
-5
lines changed

3 files changed

+75
-5
lines changed

ui/ducks/domains.js

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ export async function fetchResolutions({ domain, chainId, state }) {
176176
return filteredResults;
177177
}
178178

179-
export function lookupDomainName(domainName) {
179+
export function lookupDomainName(domainName, chainId) {
180180
return async (dispatch, getState) => {
181181
const trimmedDomainName = domainName.trim();
182182
let state = getState();
@@ -186,9 +186,8 @@ export function lookupDomainName(domainName) {
186186
await dispatch(lookupStart(trimmedDomainName));
187187
state = getState();
188188
log.info(`Resolvers attempting to resolve name: ${trimmedDomainName}`);
189-
190-
const chainId = getCurrentChainId(state);
191-
const chainIdInt = parseInt(chainId, 16);
189+
const finalChainId = chainId || getCurrentChainId(state);
190+
const chainIdInt = parseInt(finalChainId, 16);
192191
const resolutions = await fetchResolutions({
193192
domain: trimmedDomainName,
194193
chainId: `eip155:${chainIdInt}`,

ui/pages/confirmations/hooks/send/useNameValidation.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,46 @@ import { AddressResolution } from '@metamask/snaps-sdk';
22

33
import mockState from '../../../../../test/data/mock-state.json';
44
import { renderHookWithProvider } from '../../../../../test/lib/render-helpers';
5+
import { lookupDomainName } from '../../../../ducks/domains';
56
// eslint-disable-next-line import/no-namespace
67
import * as SnapNameResolution from '../../../../hooks/snaps/useSnapNameResolution';
78
// eslint-disable-next-line import/no-namespace
89
import * as SendValidationUtils from '../../utils/sendValidations';
910
import { useNameValidation } from './useNameValidation';
11+
import { useSendType } from './useSendType';
12+
13+
jest.mock('react-redux', () => ({
14+
...jest.requireActual('react-redux'),
15+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
16+
useDispatch: jest.fn().mockReturnValue((callback: any) => callback?.()),
17+
}));
1018

1119
jest.mock('@metamask/bridge-controller', () => ({
1220
...jest.requireActual('@metamask/bridge-controller'),
1321
formatChainIdToCaip: jest.fn(),
1422
}));
1523

24+
jest.mock('../../../../ducks/domains', () => ({
25+
lookupDomainName: jest.fn(),
26+
}));
27+
28+
jest.mock('./useSendType', () => ({
29+
useSendType: jest.fn().mockReturnValue({
30+
isEvmSendType: false,
31+
}),
32+
}));
33+
1634
describe('useNameValidation', () => {
35+
const lookupDomainNameMock = jest.mocked(lookupDomainName);
36+
const useSendTypeMock = jest.mocked(useSendType);
37+
38+
beforeEach(() => {
39+
jest.clearAllMocks();
40+
useSendTypeMock.mockReturnValue({
41+
isEvmSendType: false,
42+
} as unknown as ReturnType<typeof useSendType>);
43+
});
44+
1745
it('return function to validate name', () => {
1846
const { result } = renderHookWithProvider(
1947
() => useNameValidation(),
@@ -47,6 +75,38 @@ describe('useNameValidation', () => {
4775
});
4876
});
4977

78+
it('dispatch lookupDomainName when name is resolved', async () => {
79+
useSendTypeMock.mockReturnValue({
80+
isEvmSendType: true,
81+
} as unknown as ReturnType<typeof useSendType>);
82+
jest.spyOn(SnapNameResolution, 'useSnapNameResolution').mockReturnValue({
83+
fetchResolutions: () =>
84+
Promise.resolve([
85+
{
86+
resolvedAddress: 'dummy_address',
87+
protocol: 'dummy_protocol',
88+
} as unknown as AddressResolution,
89+
]),
90+
});
91+
const { result } = renderHookWithProvider(
92+
() => useNameValidation(),
93+
mockState,
94+
);
95+
expect(
96+
await result.current.validateName(
97+
'5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp',
98+
'test.eth',
99+
),
100+
).toStrictEqual({
101+
protocol: 'dummy_protocol',
102+
resolvedLookup: 'dummy_address',
103+
});
104+
expect(lookupDomainNameMock).toHaveBeenCalledWith(
105+
'test.eth',
106+
'5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp',
107+
);
108+
});
109+
50110
it('return confusable error and warning as name is resolved', async () => {
51111
jest
52112
.spyOn(SendValidationUtils, 'findConfusablesInRecipient')

ui/pages/confirmations/hooks/send/useNameValidation.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,17 @@
11
import { formatChainIdToCaip } from '@metamask/bridge-controller';
2+
import { useDispatch } from 'react-redux';
23
import { useCallback } from 'react';
34

45
import { isValidDomainName } from '../../../../helpers/utils/util';
56
import { useSnapNameResolution } from '../../../../hooks/snaps/useSnapNameResolution';
67
import { findConfusablesInRecipient } from '../../utils/sendValidations';
8+
import { lookupDomainName } from '../../../../ducks/domains';
9+
import { useSendType } from './useSendType';
710

811
export const useNameValidation = () => {
912
const { fetchResolutions } = useSnapNameResolution();
13+
const dispatch = useDispatch();
14+
const { isEvmSendType } = useSendType();
1015

1116
const validateName = useCallback(
1217
async (chainId: string, to: string) => {
@@ -25,6 +30,12 @@ export const useNameValidation = () => {
2530
const resolvedLookup = resolutions[0]?.resolvedAddress;
2631
const protocol = resolutions[0]?.protocol;
2732

33+
// In order to display the ENS name component we need to initiate reverse lookup by calling lookupDomainName
34+
// so the domain resolution will be available in the store then Name component will use it in the confirmation
35+
if (isEvmSendType) {
36+
dispatch(lookupDomainName(to, chainId));
37+
}
38+
2839
return {
2940
resolvedLookup,
3041
protocol,
@@ -36,7 +47,7 @@ export const useNameValidation = () => {
3647
error: 'nameResolutionFailedError',
3748
};
3849
},
39-
[fetchResolutions],
50+
[dispatch, fetchResolutions],
4051
);
4152

4253
return {

0 commit comments

Comments
 (0)