-
Notifications
You must be signed in to change notification settings - Fork 5.4k
fix: Fix ens domain name resolution in confirmation after send flow #37047
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -176,7 +176,7 @@ export async function fetchResolutions({ domain, chainId, state }) { | |
| return filteredResults; | ||
| } | ||
|
|
||
| export function lookupDomainName(domainName) { | ||
| export function lookupDomainName(domainName, chainId) { | ||
| return async (dispatch, getState) => { | ||
| const trimmedDomainName = domainName.trim(); | ||
| let state = getState(); | ||
|
|
@@ -186,9 +186,8 @@ export function lookupDomainName(domainName) { | |
| await dispatch(lookupStart(trimmedDomainName)); | ||
| state = getState(); | ||
| log.info(`Resolvers attempting to resolve name: ${trimmedDomainName}`); | ||
|
|
||
| const chainId = getCurrentChainId(state); | ||
| const chainIdInt = parseInt(chainId, 16); | ||
| const finalChainId = chainId || getCurrentChainId(state); | ||
| const chainIdInt = parseInt(finalChainId, 16); | ||
| const resolutions = await fetchResolutions({ | ||
| domain: trimmedDomainName, | ||
| chainId: `eip155:${chainIdInt}`, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are avoiding use of redux in redesigned confirmations, we should try to avoid it here also.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is already in use unfortunately and it's not redesigned confirmation but There is no other fix in order to show reverse lookup ENS domain in the |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,17 @@ | ||
| import { formatChainIdToCaip } from '@metamask/bridge-controller'; | ||
| import { useDispatch } from 'react-redux'; | ||
| import { useCallback } from 'react'; | ||
|
|
||
| import { isValidDomainName } from '../../../../helpers/utils/util'; | ||
| import { useSnapNameResolution } from '../../../../hooks/snaps/useSnapNameResolution'; | ||
| import { findConfusablesInRecipient } from '../../utils/sendValidations'; | ||
| import { lookupDomainName } from '../../../../ducks/domains'; | ||
| import { useSendType } from './useSendType'; | ||
|
|
||
| export const useNameValidation = () => { | ||
| const { fetchResolutions } = useSnapNameResolution(); | ||
| const dispatch = useDispatch(); | ||
| const { isEvmSendType } = useSendType(); | ||
|
|
||
| const validateName = useCallback( | ||
| async (chainId: string, to: string) => { | ||
|
|
@@ -25,6 +30,12 @@ export const useNameValidation = () => { | |
| const resolvedLookup = resolutions[0]?.resolvedAddress; | ||
| const protocol = resolutions[0]?.protocol; | ||
|
|
||
| // In order to display the ENS name component we need to initiate reverse lookup by calling lookupDomainName | ||
| // so the domain resolution will be available in the store then Name component will use it in the confirmation | ||
| if (isEvmSendType) { | ||
| dispatch(lookupDomainName(to, chainId)); | ||
| } | ||
|
|
||
| return { | ||
| resolvedLookup, | ||
| protocol, | ||
|
|
@@ -36,7 +47,7 @@ export const useNameValidation = () => { | |
| error: 'nameResolutionFailedError', | ||
| }; | ||
| }, | ||
| [fetchResolutions], | ||
| [dispatch, fetchResolutions], | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| ); | ||
|
|
||
| return { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Bug
The
lookupEndaction dispatches with the initialchainIdparameter instead of thefinalChainIdused for resolution. If thechainIdparameter was omitted andgetCurrentChainIdwas used as a fallback, the state will record an incorrect (undefined)chainIdfor the completed resolution.