Address book controller chainId potentially misleading

[rekmarks]

Address book entries are defined as:

interface AddressBookEntry {
  address: string;
  name: string;
  chainId: number;
  memo: string;
}

This leads us to believe that, perhaps, the user should be able to have entries with identical address fields and different chainId fields.

However, we store AddressBookEntrys by normalized addresses, and we normalize the address using toChecksumAddress(address), which returns the same address regardless of the chainId.

According to the ethereumjs-util documentation, we can add the chainId as an optional parameter to generate unique checksummed addresses by chainId. It appears they have yet to publish this version of the library, however, see: ethereumjs/ethereumjs-util#218

Question: if we compute checksummed addresses with chainId, can we still use those addresses directly for sending transactions? In other words, will they be different from what's currently displayed in the UI?

[jennypollack]

will they be different from what's currently displayed in the UI?

based on the spec of 1191, https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1191.md

This proposal is fully backward compatible. The checksum calculation is changed only for new networks that choose to adopt this EIP and add their chain numbers to the Adoption Table included in this document.

if a network chooses to add itself to the adoption table we would have a problem, but for now I think we're safe to continue as we are, as the current prioritized networks (arguably anything hard coded in our config) are not using this specification
(sidenote for the chainIds in the adoption table we currently are not supporting this)

[Gudahtt]

This was fixed in #152