Skip to content

Commit

Permalink
refactor: Key the address book by chain ID instead of network ID
Browse files Browse the repository at this point in the history 
The `AddressBookController` stores addresses by chain ID, but we had
been storing and retrieving them using the network ID instead in many
places. It has been updated to consistently use the chain ID for all
address book access.

Existing address book entries have been migrated to be grouped by chain
ID. For all known networks, we attempt to map the network ID to a chain
ID that exists as a locally configured or built-in network. In cases
where multiple matches are found, the entries are duplicated on each
matching chain. Address book entries that don't correspond with any
local networks are discarded.

This relates to MetaMask/mobile-planning#1226
  • Loading branch information
@Gudahtt
Gudahtt committed Sep 15, 2023
1 parent 98199ab commit 224efed
Show file tree
Hide file tree
Showing 20 changed files with 781 additions and 118 deletions.
6 changes: 3 additions & 3 deletions app/components/UI/AddToAddressBookWrapper/AddToAddressBookWrapper.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { strings } from '../../../../locales/i18n';
import Engine from '../../../core/Engine';
import { ADD_ADDRESS_MODAL_CONTAINER_ID } from '../../../constants/test-ids';
import { baseStyles } from '../../../styles/common';
import { selectNetwork } from '../../../selectors/networkController';
import { selectChainId } from '../../../selectors/networkController';
import { useTheme } from '../../../util/theme';
import Text from '../../Base/Text';
import useExistingAddress from '../../hooks/useExistingAddress';
Expand All @@ -36,7 +36,7 @@ export const AddToAddressBookWrapper = ({
setToAddressName,
defaultNull = false,
}: AddToAddressBookWrapperProps) => {
const networkId = useSelector(selectNetwork);
const chainId = useSelector(selectChainId);

const existingContact = useExistingAddress(address);
const { colors, themeAppearance } = useTheme();
Expand All @@ -53,7 +53,7 @@ export const AddToAddressBookWrapper = ({

const onSaveToAddressBook = () => {
const { AddressBookController } = Engine.context;
AddressBookController.set(address, alias, networkId);
AddressBookController.set(address, alias, chainId);
!!alias && setToAddressName?.(alias);
setAlias(undefined);
};
Expand Down
10 changes: 3 additions & 7 deletions app/components/UI/ApproveTransactionReview/AddNickname/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import {
} from '../../../../constants/error';
import {
selectChainId,
selectNetwork,
selectNetworkConfigurations,
selectProviderType,
selectRpcTarget,
Expand All @@ -50,7 +49,6 @@ const AddNickname = (props: AddNicknameProps) => {
addressNickname,
providerType,
providerChainId,
providerNetwork,
providerRpcTarget,
addressBook,
identities,
Expand All @@ -75,17 +73,16 @@ const AddNickname = (props: AddNicknameProps) => {
const validateAddressOrENSFromInput = useCallback(async () => {
const { addressError, errorContinue } = await validateAddressOrENS({
toAccount: address,
// TODO: This parameters is effectively ignored, it should be named `networkId`
providerNetwork,
addressBook,
identities,
// TODO: This parameters is effectively ignored, it should be named `chainId`
providerChainId,
});

setAddressErr(addressError);
setErrContinue(errorContinue);
setAddressHasError(addressError);
}, [address, providerNetwork, addressBook, identities, providerChainId]);
}, [address, addressBook, identities, providerChainId]);

useEffect(() => {
validateAddressOrENSFromInput();
Expand Down Expand Up @@ -123,7 +120,7 @@ const AddNickname = (props: AddNicknameProps) => {
AddressBookController.set(
toChecksumAddress(address),
newNickname,
providerNetwork,
providerChainId,
);
closeModal();
AnalyticsV2.trackEvent(
Expand Down Expand Up @@ -264,7 +261,6 @@ const mapStateToProps = (state: any) => ({
providerType: selectProviderType(state),
providerRpcTarget: selectRpcTarget(state),
providerChainId: selectChainId(state),
providerNetwork: selectNetwork(state),
addressBook: state.engine.backgroundState.AddressBookController.addressBook,
identities: selectIdentities(state),
networkConfigurations: selectNetworkConfigurations(state),
Expand Down
9 changes: 1 addition & 8 deletions app/components/Views/ApproveView/Approve/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ import {
} from '../../../../core/GasPolling/GasPolling';
import {
selectChainId,
selectNetwork,
selectProviderType,
selectTicker,
selectRpcTarget,
Expand Down Expand Up @@ -139,10 +138,6 @@ class Approve extends PureComponent {
* An object of all saved addresses
*/
addressBook: PropTypes.object,
/**
* The current network of the app
*/
networkId: PropTypes.string,
networkConfigurations: PropTypes.object,
providerRpcTarget: PropTypes.string,
/**
Expand Down Expand Up @@ -649,7 +644,6 @@ class Approve extends PureComponent {
const {
transaction,
addressBook,
networkId,
gasEstimateType,
gasFeeEstimates,
primaryCurrency,
Expand Down Expand Up @@ -678,7 +672,7 @@ class Approve extends PureComponent {

const savedContactList = checkIfAddressIsSaved(
addressBook,
networkId,
chainId,
transaction,
);

Expand Down Expand Up @@ -848,7 +842,6 @@ const mapStateToProps = (state) => ({
nativeCurrency: selectNativeCurrency(state),
showCustomNonce: state.settings.showCustomNonce,
addressBook: state.engine.backgroundState.AddressBookController.addressBook,
networkId: selectNetwork(state),
providerType: selectProviderType(state),
providerRpcTarget: selectRpcTarget(state),
networkConfigurations: selectNetworkConfigurations(state),
Expand Down
18 changes: 6 additions & 12 deletions app/components/Views/Send/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ import { KEYSTONE_TX_CANCELED } from '../../../constants/error';
import { ThemeContext, mockTheme } from '../../../util/theme';
import {
selectChainId,
selectNetwork,
selectProviderType,
} from '../../../selectors/networkController';
import { selectTokenList } from '../../../selectors/tokenListController';
Expand Down Expand Up @@ -116,10 +115,6 @@ class Send extends PureComponent {
* Map representing the address book
*/
addressBook: PropTypes.object,
/**
* Network id
*/
networkId: PropTypes.string,
/**
* The chain ID of the current selected network
*/
Expand Down Expand Up @@ -309,7 +304,7 @@ class Send extends PureComponent {
function_name = null, // eslint-disable-line no-unused-vars
parameters = null,
}) => {
const { addressBook, networkId, identities, selectedAddress } = this.props;
const { addressBook, chainId, identities, selectedAddress } = this.props;

let newTxMeta = {};
let txRecipient;
Expand All @@ -334,7 +329,7 @@ class Send extends PureComponent {

newTxMeta.transactionToName = getTransactionToName({
addressBook,
networkId,
chainId,
toAddress: newTxMeta.to,
identities,
ensRecipient: newTxMeta.ensRecipient,
Expand Down Expand Up @@ -374,7 +369,7 @@ class Send extends PureComponent {
};
newTxMeta.transactionToName = getTransactionToName({
addressBook,
networkId,
chainId,
toAddress: to,
identities,
ensRecipient,
Expand Down Expand Up @@ -545,7 +540,7 @@ class Send extends PureComponent {
this.setState({ transactionConfirmed: true });
const {
transaction: { selectedAsset, assetType },
networkId,
chainId,
addressBook,
} = this.props;
let { transaction } = this.props;
Expand Down Expand Up @@ -597,9 +592,9 @@ class Send extends PureComponent {
}
}
const existingContact =
addressBook[networkId] && addressBook[networkId][checksummedAddress];
addressBook[chainId] && addressBook[chainId][checksummedAddress];
if (!existingContact) {
AddressBookController.set(checksummedAddress, '', networkId);
AddressBookController.set(checksummedAddress, '', chainId);
}
await new Promise((resolve) => {
resolve(result);
Expand Down Expand Up @@ -774,7 +769,6 @@ const mapStateToProps = (state) => ({
transaction: state.transaction,
networkType: selectProviderType(state),
tokens: selectTokens(state),
networkId: selectNetwork(state),
chainId: selectChainId(state),
identities: selectIdentities(state),
selectedAddress: selectSelectedAddress(state),
Expand Down
10 changes: 5 additions & 5 deletions app/components/Views/SendFlow/AddressList/AddressList.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import AddressElement from '../AddressElement';
import { useTheme } from '../../../../util/theme';
import Text from '../../../../component-library/components/Texts/Text/Text';
import { TextVariant } from '../../../../component-library/components/Texts/Text';
import { selectNetwork } from '../../../../selectors/networkController';
import { selectChainId } from '../../../../selectors/networkController';
import { selectIdentities } from '../../../../selectors/preferencesController';

// Internal dependencies
Expand Down Expand Up @@ -45,16 +45,16 @@ const AddressList: React.FC<AddressListProps> = ({
const styles = styleSheet(colors);
const [contactElements, setContactElements] = useState<Contact[]>([]);
const [fuse, setFuse] = useState<any>(undefined);
const networkId = useSelector(selectNetwork);
const chainId = useSelector(selectChainId);
const identities = useSelector(selectIdentities);
const addressBook = useSelector(
(state: any) =>
state.engine.backgroundState.AddressBookController.addressBook,
);

const networkAddressBook: { [address: string]: AddressBookEntry } = useMemo(
() => addressBook[networkId] || {},
[addressBook, networkId],
() => addressBook[chainId] || {},
[addressBook, chainId],
);

const parseAddressBook = useCallback(
Expand Down Expand Up @@ -145,7 +145,7 @@ const AddressList: React.FC<AddressListProps> = ({
}, [
inputSearch,
addressBook,
networkId,
chainId,
reloadAddressList,
getNetworkAddressBookList,
parseAddressBook,
Expand Down
27 changes: 10 additions & 17 deletions app/components/Views/SendFlow/SendTo/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ import {
import generateTestId from '../../../../../wdio/utils/generateTestId';
import {
selectChainId,
selectNetwork,
selectProviderType,
selectTicker,
} from '../../../../selectors/networkController';
Expand Down Expand Up @@ -89,10 +88,6 @@ class SendFlow extends PureComponent {
* Network provider chain id
*/
chainId: PropTypes.string,
/**
* Network id
*/
networkId: PropTypes.string,
/**
* Object that represents the navigator
*/
Expand Down Expand Up @@ -181,7 +176,7 @@ class SendFlow extends PureComponent {
const {
addressBook,
ticker,
networkId,
chainId,
navigation,
providerType,
route,
Expand All @@ -190,7 +185,7 @@ class SendFlow extends PureComponent {
this.updateNavBar();
// For analytics
navigation.setParams({ providerType, isPaymentRequest });
const networkAddressBook = addressBook[networkId] || {};
const networkAddressBook = addressBook[chainId] || {};
if (!Object.keys(networkAddressBook).length) {
setTimeout(() => {
this.addressToInputRef &&
Expand Down Expand Up @@ -223,8 +218,8 @@ class SendFlow extends PureComponent {

isAddressSaved = () => {
const { toAccount } = this.state;
const { addressBook, networkId, identities } = this.props;
const networkAddressBook = addressBook[networkId] || {};
const { addressBook, chainId, identities } = this.props;
const networkAddressBook = addressBook[chainId] || {};
const checksummedAddress = toChecksumAddress(toAccount);
return !!(
networkAddressBook[checksummedAddress] || identities[checksummedAddress]
Expand Down Expand Up @@ -366,10 +361,10 @@ class SendFlow extends PureComponent {
};

getAddressNameFromBookOrIdentities = (toAccount) => {
const { addressBook, identities, networkId } = this.props;
const { addressBook, identities, chainId } = this.props;
if (!toAccount) return;

const networkAddressBook = addressBook[networkId] || {};
const networkAddressBook = addressBook[chainId] || {};

const checksummedAddress = toChecksumAddress(toAccount);

Expand All @@ -381,7 +376,7 @@ class SendFlow extends PureComponent {
};

validateAddressOrENSFromInput = async (toAccount) => {
const { addressBook, identities, chainId, networkId } = this.props;
const { addressBook, identities, chainId } = this.props;
const {
addressError,
toEnsName,
Expand All @@ -394,7 +389,6 @@ class SendFlow extends PureComponent {
confusableCollection,
} = await validateAddressOrENS({
toAccount,
networkId,
addressBook,
identities,
chainId,
Expand Down Expand Up @@ -441,7 +435,7 @@ class SendFlow extends PureComponent {
};

render = () => {
const { ticker, addressBook, networkId } = this.props;
const { ticker, addressBook, chainId } = this.props;
const {
toAccount,
toSelectedAddressReady,
Expand All @@ -464,8 +458,8 @@ class SendFlow extends PureComponent {
);
const existingContact =
checksummedAddress &&
addressBook[networkId] &&
addressBook[networkId][checksummedAddress];
addressBook[chainId] &&
addressBook[chainId][checksummedAddress];
const displayConfusableWarning =
!existingContact && confusableCollection && !!confusableCollection.length;
const displayAsWarning =
Expand Down Expand Up @@ -636,7 +630,6 @@ const mapStateToProps = (state) => ({
selectedAsset: state.transaction.selectedAsset,
identities: selectIdentities(state),
ticker: selectTicker(state),
networkId: selectNetwork(state),
providerType: selectProviderType(state),
isPaymentRequest: state.transaction.paymentRequest,
isNativeTokenBuySupported: isNetworkBuyNativeTokenSupported(
Expand Down
Loading

0 comments on commit 224efed

Please sign in to comment.