Skip to content
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.

Sign up for GitHub

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

fix(per-dapp-selected-network): restore per-dapp network picker in permissions view #13686

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft

fix(per-dapp-selected-network): restore per-dapp network picker in permissions view #13686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
988bdf1
feat: Add a temporary picker to test if per-dapp-selected-networks fe…
EtherWizard33 Feb 11, 2025
84a8171
fix: fix permissisons network switch
salimtb Feb 11, 2025
21bc3a2
style: remove console.logs
EtherWizard33 Feb 11, 2025
421fc5c
style: remove console.log from permission summary
EtherWizard33 Feb 11, 2025
ae28079
style: remove console.log from network selector
EtherWizard33 Feb 11, 2025
b82b7ac
feat: remove console.log from chain utils
EtherWizard33 Feb 11, 2025
dd41af7
refactor(network): use isPerDappSelectedNetworkEnabled helper
EtherWizard33 Feb 12, 2025
bd097ce
feat(permissions): add interactive favicon with network switching
EtherWizard33 Feb 13, 2025
5c02c47
style(lint): cleanup
EtherWizard33 Feb 13, 2025
339d998
feat: remove temporary network picker from AcounntPermissionsConnecte…
EtherWizard33 Feb 13, 2025
ae30f01
refactor: Move domain logo container behind per-dapp network feature …
EtherWizard33 Feb 13, 2025
1aa5cc5
fix(permission-summary): When the dapp is not yet connected, and the …
EtherWizard33 Feb 13, 2025
e7fa261
chore: in engine.ts, make use of the local feature flag isMultichainV…
EtherWizard33 Feb 17, 2025
2589252
Merge branch 'main' into feat-4144-per-dapp-selected-network-poc-2
EtherWizard33 Feb 24, 2025
f49d889
feat: remove non-permitted network flow check for dapps
EtherWizard33 Feb 26, 2025
3c2de6e
feat(AddressFrom): get network info per dapp origin
EtherWizard33 Feb 26, 2025
f6783d3
feat(TransactionReview): pass origin prop to AccountFromToInfoCard
EtherWizard33 Feb 26, 2025
f3dfe54
Use provided chainId to determine balance in useAddressBalance hook
OGPoyraz Mar 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion app/components/UI/AccountFromToInfoCard/AccountFromToInfoCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,12 @@ const AccountFromToInfoCard = (props: AccountFromToInfoCardProps) => {
return (
<View style={styles.container}>
{fromAddress && (
<AddressFrom asset={selectedAsset} from={fromAddress} origin={origin} />
<AddressFrom
chainId={transactionState?.chainId}
asset={selectedAsset}
from={fromAddress}
origin={origin}
/>
)}
{existingToAddress === undefined && confusableCollection.length ? (
<TouchableOpacity onPress={() => setShowWarningModal(true)}>
Expand Down
1 change: 1 addition & 0 deletions app/components/UI/AccountFromToInfoCard/AccountFromToInfoCard.types.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export interface Transaction {
transactionFromName: string;
selectedAsset: SelectedAsset;
ensRecipient?: string;
chainId?: string;
}

export interface AccountFromToInfoCardProps {
Expand Down
17 changes: 9 additions & 8 deletions app/components/UI/AccountFromToInfoCard/AddressFrom.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,14 @@ import { BadgeVariant } from '../../../component-library/components/Badges/Badge
import Text from '../../../component-library/components/Texts/Text';
import { useStyles } from '../../../component-library/hooks';
import { selectAccountsByChainId } from '../../../selectors/accountTrackerController';
import {
selectNetworkImageSource,
selectNetworkName,
} from '../../../selectors/networkInfos';
import {
getLabelTextByAddress,
renderAccountName,
} from '../../../util/address';
import useAddressBalance from '../../hooks/useAddressBalance/useAddressBalance';
import stylesheet from './AddressFrom.styles';
import { selectInternalAccounts } from '../../../selectors/accountsController';
import { useNetworkInfo } from '../../../selectors/selectedNetworkController';

interface Asset {
isETH?: boolean;
Expand All @@ -37,25 +34,29 @@ interface AddressFromProps {
dontWatchAsset?: boolean;
from: string;
origin?: string;
chainId?: string;
}

const AddressFrom = ({
asset,
chainId,
dontWatchAsset,
from,
origin,
}: AddressFromProps) => {
const [accountName, setAccountName] = useState('');

const { styles } = useStyles(stylesheet, {});
const { addressBalance } = useAddressBalance(asset, from, dontWatchAsset);
const { addressBalance } = useAddressBalance(asset, from, dontWatchAsset, chainId);

const accountsByChainId = useSelector(selectAccountsByChainId);

const internalAccounts = useSelector(selectInternalAccounts);
const activeAddress = toChecksumAddress(from);

const networkName = useSelector(selectNetworkName);
// FIXME: this could be the wrong selector, probably needs to be per dapp (per origin)
// console.log('>>> AddressFrom origin', origin);
const { networkName, networkImageSource } = useNetworkInfo(origin);
Comment on lines +57 to +59
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 if you're passing in the origin here, the useNetworkInfo selector should be selecting the correct state based on that parameter. Is the comment outdated?


const useBlockieIcon = useSelector(
// TODO: Replace "any" with type
Expand All @@ -74,7 +75,7 @@ const AddressFrom = ({
}
}, [accountsByChainId, internalAccounts, activeAddress, origin]);

const networkImage = useSelector(selectNetworkImageSource);
// const networkImage = useSelector(selectNetworkImageSource);

const accountTypeLabel = getLabelTextByAddress(activeAddress);

Expand All @@ -95,7 +96,7 @@ const AddressFrom = ({
badgeProps={{
variant: BadgeVariant.Network,
name: networkName,
imageSource: networkImage,
imageSource: networkImageSource,
}}
useBlockieIcon={useBlockieIcon}
/>
Expand Down
69 changes: 64 additions & 5 deletions app/components/UI/PermissionsSummary/PermissionsSummary.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ import Routes from '../../../constants/navigation/Routes';
import ButtonIcon, {
ButtonIconSizes,
} from '../../../component-library/components/Buttons/ButtonIcon';
import { getNetworkImageSource } from '../../../util/networks';
import {
getNetworkImageSource,
isPerDappSelectedNetworkEnabled,
} from '../../../util/networks';
import Engine from '../../../core/Engine';
import { SDKSelectorsIDs } from '../../../../e2e/selectors/Settings/SDK.selectors';
import { useSelector } from 'react-redux';
Expand All @@ -48,6 +51,12 @@ import { useNetworkInfo } from '../../../selectors/selectedNetworkController';
import { ConnectedAccountsSelectorsIDs } from '../../../../e2e/selectors/Browser/ConnectedAccountModal.selectors';
import { PermissionSummaryBottomSheetSelectorsIDs } from '../../../../e2e/selectors/Browser/PermissionSummaryBottomSheet.selectors';
import { NetworkNonPemittedBottomSheetSelectorsIDs } from '../../../../e2e/selectors/Network/NetworkNonPemittedBottomSheet.selectors';
import BadgeWrapper from '../../../component-library/components/Badges/BadgeWrapper';
import Badge, {
BadgeVariant,
} from '../../../component-library/components/Badges/Badge';
import AvatarFavicon from '../../../component-library/components/Avatars/Avatar/variants/AvatarFavicon';
import AvatarToken from '../../../component-library/components/Avatars/Avatar/variants/AvatarToken';

const PermissionsSummary = ({
currentPageInformation,
Expand Down Expand Up @@ -80,7 +89,7 @@ const PermissionsSummary = ({
() => new URL(currentPageInformation.url).hostname,
[currentPageInformation.url],
);
const networkInfo = useNetworkInfo(hostname);
const { networkName, networkImageSource } = useNetworkInfo(hostname);

// if network switch, we get the chain name from the customNetworkInformation
let chainName = '';
Expand Down Expand Up @@ -111,12 +120,62 @@ const PermissionsSummary = ({
onEditNetworks?.();
};

const switchNetwork = useCallback(() => {
navigate(Routes.MODAL.ROOT_MODAL_FLOW, {
screen: Routes.SHEET.NETWORK_SELECTOR,
});
}, [navigate]);

const renderTopIcon = () => {
const { currentEnsName, icon } = currentPageInformation;
const url = currentPageInformation.url;
const iconTitle = getHost(currentEnsName || url);

return (
if (!isAlreadyConnected) {
return (
<WebsiteIcon
style={styles.domainLogoContainer}
viewStyle={styles.assetLogoContainer}
title={iconTitle}
url={currentEnsName || url}
icon={typeof icon === 'string' ? icon : icon?.uri}
/>
);
}

return isPerDappSelectedNetworkEnabled() ? (
<View style={[styles.domainLogoContainer, styles.assetLogoContainer]}>
<TouchableOpacity
onPress={switchNetwork}
testID={ConnectedAccountsSelectorsIDs.NETWORK_PICKER}
>
<BadgeWrapper
badgeElement={
<Badge
variant={BadgeVariant.Network}
name={networkName}
imageSource={networkImageSource}
/>
}
>
{icon ? (
<AvatarFavicon
imageSource={{
uri: typeof icon === 'string' ? icon : icon?.uri,
}}
size={AvatarSize.Md}
/>
) : (
<AvatarToken
name={iconTitle}
isHaloEnabled
size={AvatarSize.Md}
/>
)}
</BadgeWrapper>
</TouchableOpacity>
</View>
) : (
Comment on lines +146 to +178
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this conditional required? Not understanding what's changed here. Could you explain?

<WebsiteIcon
style={styles.domainLogoContainer}
viewStyle={styles.assetLogoContainer}
Expand Down Expand Up @@ -361,7 +420,7 @@ const PermissionsSummary = ({
</TextComponent>
<TextComponent variant={TextVariant.BodySMMedium}>
{isNonDappNetworkSwitch
? networkInfo?.networkName || providerConfig.nickname
? networkName || providerConfig.nickname
: chainName}
</TextComponent>
</TextComponent>
Expand All @@ -371,7 +430,7 @@ const PermissionsSummary = ({
size={AvatarSize.Xs}
name={
isNonDappNetworkSwitch
? networkInfo?.networkName || providerConfig.nickname
? networkName || providerConfig.nickname
: chainName
}
imageSource={
Expand Down
1 change: 1 addition & 0 deletions app/components/UI/PermissionsSummary/PermissionsSummary.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export interface PermissionsSummaryProps {
accounts?: Account[];
accountAddresses?: string[];
networkAvatars?: ({ name: string; imageSource: string } | null)[];
// TODO: remove isNonDappNetworkSwitch prop once the per-dapp network switch is implemented
isNonDappNetworkSwitch?: boolean;
onChooseFromPermittedNetworks?: () => void;
}
1 change: 1 addition & 0 deletions app/components/UI/Tabs/TabThumbnail/TabThumbnail.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const TabThumbnail = ({
(account) => account.address.toLowerCase() === activeAddress?.toLowerCase(),
);
const { networkName, networkImageSource } = useNetworkInfo(tabTitle);

const faviconSource = useFavicon(tab.url);

return (
Expand Down
1 change: 1 addition & 0 deletions app/components/Views/AccountPermissions/AccountPermissions.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export interface AccountPermissionsProps {
};
isRenderedAsBottomSheet?: boolean;
initialScreen?: AccountPermissionsScreens;
// TODO: remove isNonDappNetworkSwitch prop once the per-dapp network switch is implemented
isNonDappNetworkSwitch?: boolean;
};
};
Expand Down
67 changes: 1 addition & 66 deletions app/components/Views/BrowserTab/BrowserTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ import downloadFile from '../../../util/browser/downloadFile';
import { MAX_MESSAGE_LENGTH } from '../../../constants/dapp';
import sanitizeUrlInput from '../../../util/url/sanitizeUrlInput';
import { getPermittedAccountsByHostname } from '../../../core/Permissions';
import Routes from '../../../constants/navigation/Routes';
import {
selectIpfsGateway,
selectIsIpfsGatewayEnabled,
Expand All @@ -75,10 +74,7 @@ import trackErrorAsAnalytics from '../../../util/metrics/TrackError/trackErrorAs
import { selectPermissionControllerState } from '../../../selectors/snaps/permissionController';
import { isTest } from '../../../util/test/utils.js';
import { EXTERNAL_LINK_TYPE } from '../../../constants/browser';
import { PermissionKeys } from '../../../core/Permissions/specifications';
import { CaveatTypes } from '../../../core/Permissions/constants';
import { AccountPermissionsScreens } from '../AccountPermissions/AccountPermissions.types';
import { useIsFocused, useNavigation } from '@react-navigation/native';
import { useNavigation } from '@react-navigation/native';
import { useStyles } from '../../hooks/useStyles';
import styleSheet from './styles';
import { type RootState } from '../../../reducers';
Expand Down Expand Up @@ -119,7 +115,6 @@ export const BrowserTab: React.FC<BrowserTabProps> = ({
addToWhitelist: triggerAddToWhitelist,
showTabs,
linkType,
isInTabsView,
wizardStep,
updateTabInfo,
addToBrowserHistory,
Expand Down Expand Up @@ -204,8 +199,6 @@ export const BrowserTab: React.FC<BrowserTabProps> = ({
*/
const whitelist = useSelector((state: RootState) => state.browser.whitelist);

const isFocused = useIsFocused();

/**
* Checks if a given url or the current url is the homepage
*/
Expand Down Expand Up @@ -607,57 +600,6 @@ export const BrowserTab: React.FC<BrowserTabProps> = ({
],
);

const checkTabPermissions = useCallback(() => {
if (!(isFocused && !isInTabsView && isTabActive)) {
return;
}
if (!resolvedUrlRef.current) return;
const hostname = new URLParse(resolvedUrlRef.current).hostname;
const permissionsControllerState =
Engine.context.PermissionController.state;
const permittedAccounts = getPermittedAccountsByHostname(
permissionsControllerState,
hostname,
);

const isConnected = permittedAccounts.length > 0;

if (isConnected) {
let permittedChains = [];
try {
const caveat = Engine.context.PermissionController.getCaveat(
hostname,
PermissionKeys.permittedChains,
CaveatTypes.restrictNetworkSwitching,
);
permittedChains = Array.isArray(caveat?.value) ? caveat.value : [];

const currentChainId = activeChainId;
const isCurrentChainIdAlreadyPermitted =
permittedChains.includes(currentChainId);

if (!isCurrentChainIdAlreadyPermitted) {
navigation.navigate(Routes.MODAL.ROOT_MODAL_FLOW, {
screen: Routes.SHEET.ACCOUNT_PERMISSIONS,
params: {
isNonDappNetworkSwitch: true,
hostInfo: {
metadata: {
origin: hostname,
},
},
isRenderedAsBottomSheet: true,
initialScreen: AccountPermissionsScreens.Connected,
},
});
}
} catch (e) {
const checkTabPermissionsError = e as Error;
Logger.error(checkTabPermissionsError, 'Error in checkTabPermissions');
}
}
}, [activeChainId, navigation, isFocused, isInTabsView, isTabActive]);

/**
* Handles state changes for when the url changes
*/
Expand Down Expand Up @@ -700,8 +642,6 @@ export const BrowserTab: React.FC<BrowserTabProps> = ({
name: siteInfo.title,
url: getMaskedUrl(siteInfo.url, sessionENSNamesRef.current),
});

checkTabPermissions();
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this no longer required?

},
[
isUrlBarFocused,
Expand All @@ -711,7 +651,6 @@ export const BrowserTab: React.FC<BrowserTabProps> = ({
updateTabInfo,
addToBrowserHistory,
navigation,
checkTabPermissions,
],
);

Expand Down Expand Up @@ -1019,10 +958,6 @@ export const BrowserTab: React.FC<BrowserTabProps> = ({
[linkType],
);

useEffect(() => {
checkTabPermissions();
}, [checkTabPermissions, isFocused, isInTabsView, isTabActive]);

const handleEnsUrl = useCallback(
async (ens: string) => {
try {
Expand Down
6 changes: 5 additions & 1 deletion app/components/Views/NetworkSelector/NetworkSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ const NetworkSelector = () => {

// origin is defined if network selector is opened from a dapp
const origin = route.params?.hostInfo?.metadata?.origin || '';

const parentSpan = trace({
name: TraceName.NetworkSwitch,
tags: getTraceTags(store.getState()),
Expand Down Expand Up @@ -245,12 +246,13 @@ const NetworkSelector = () => {
origin,
networkConfigurationId,
);
sheetRef.current?.dismissModal();
} else {
const { networkClientId } = rpcEndpoints[defaultRpcEndpointIndex];
try {
await MultichainNetworkController.setActiveNetwork(networkClientId);
} catch (error) {
Logger.error(new Error(`Error in setActiveNetwork: ${error}`));
Logger.error(new Error(`Error i setActiveNetwork: ${error}`));
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 whats this change about?

}
}

Expand Down Expand Up @@ -364,8 +366,10 @@ const NetworkSelector = () => {
AccountTrackerController,
SelectedNetworkController,
} = Engine.context;

if (domainIsConnectedDapp && isMultichainV1Enabled()) {
SelectedNetworkController.setNetworkClientIdForDomain(origin, type);
closeRpcModal();
} else {
const networkConfiguration =
networkConfigurations[BUILT_IN_NETWORKS[type].chainId];
Expand Down
2 changes: 1 addition & 1 deletion app/components/Views/confirmations/SendFlow/Confirm/__snapshots__/index.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,7 @@ exports[`Confirm should render correctly 1`] = `
}
}
>
Ethereum Main Network
Ethereum Network default RPC
</Text>
<View
style={
Expand Down
Loading