Implementation discussion at Status: https://github.com/status-im/status-react/issues/10036
This seems super important. Was it implemented in the end?
While I understand your point, I don’t think it makes a huge difference. A user confirmation popup is requested already. A scammy website could still force the popup onto the user and scam them this way.
If a website shows many popups unrequested, it’s fairly likely a user will just exit anyway. Popups are an annoyance on any website. Abusing this is likely to get the browser tab closed anyhow.
I guess this is one of the rationales behind the wallet requiring a unique Symbol for each token added?
Well wallets can still cache, but yes I agree there’s a privacy leak through the image URL.
I agree a multiformat/cid is a better thing to use, as the user can provide their own method of accessing it that could be more privacy-preserving.
I think this is an interesting idea, but I don’t think a permissions request is less secure for loading on page load. I do think there’s some benefit to requiring user interaction, but as a browser extension I don’t have the ability to enforce this, and neither would a library-based wallet like WalletConnect, so that particular feature will need to be reserved for full browser manufacturers.
Implementation discussion at Status: https://github.com/status-im/status-react/issues/10036
Thanks for sharing!
2 Likes
@danfinlay @phraktle
Hi! The link is broken. I found a similar article here: Registering Tokens with Users | MetaMask Docs
But there is no information about “checking if a token already exists in Metamask”.
How to check an existance?
There are a couple of issues with wallet_watchAsset:
(1) options does not include chainId.
The same token can be deployed on different chains, and can be bridged between the chains using something like the Multichain router or the Polygon PoS bridge. Sometimes in these situations, the token uses the same symbol on all chains.
The contract address may be the same or different on different chains. The wallet needs to keep track of which address is used on which chain.
Basically the problem is that your wallet may end up with two tokens of the same name, with no distinguishing factors, and/or the token may be inaccessible on the current chain that the wallet is connected to (because it is deployed on a different chain, and/or at a different address on the current chain).
Ideally the wallet should display the network name based on the chainId, e.g. for a token XYZ that is added with addresses for two different chainIds:
XYZ (Ethereum Mainnet)
XYZ (Polygon Mumbai)
etc.
(2) wallet_watchAsset should be idempotent if the token has already previously been added to the wallet. The spec needs to specify this, and MetaMask needs to implement this.
The problem is that there is no API for querying a wallet to see if a token has already been added. Therefore, if a dapp is trying to helpfully suggest a token to add to a wallet, it has to suggest it every time the user uses the dapp (which is annoying). If they don’t reject the suggestion the 2nd and subsequent time, then they end up with an ugly error that suggests that this may be a scam token:
I think specifying the chain ID is useful because we can’t always assume the request is for the current chain. For example if a dapp offers a bridging transaction, it’d be useful to invoke wallet_watchAsset to add the token in the destination chain.
The token symbol and decimals are available via the contract, so they shouldn’t be supplied by the dapp. We should assume that dapps are actively malicious and will lie about all sorts of things, and anything we can trustlessly acquire we should. Imagine a dapp trying to add WETH with 6 decimals and a symbol ABC. Future attempts to add WETH would fail because it already exists, and it would perpetually have the wrong symbol/decimals.
I agree with comments above about idempotency (you should not get an error if you try to add a token the user already has, it should return a success without any indication that the token was already present) and chainId should be included in the parameters.
In addition to adding chainId, if the wallet doesn’t have an RPC for a particular chain, it should not add the token. This is because the wallet cannot lookup the address on-chain to verify that it exists and get the symbol/decimals which means a malicious dapp could do whatever they wanted with zero validation.
With regards to the image URL, I feel like instead of adding it here, efforts should be made to adopt a standard that includes an image in the token contract itself. This could either be a link or an inline (data) image. As mentioned above, the wallet should never be trusting dapps, and they should be retrieving all metadata about a token from the token contract itself rather than from the dapp. Adding image to this call will just discourage standardization of token images being embedded into contracts and make the situation worse long term.
Security considerations section should mention that the dapp could lie and it should not be trusted. If all of the above feedback is applied, then this call becomes much safer (and no need for additional text in security considerations section) as it basically is just a hint to the wallet “hey, there is a token at address X, chain Y that the user might be interested in” and the dapp doesn’t have to be trusted for anything.
Great minds think alike. Yesterday, I began work to move EIP-1046 out of Stagnant 
I, in fact, plan to include a link to it in this EIP and recommend its usage, but am currently limited by the fact that EIP-1046 is still stagnant.
Done, done, done, and done!
1 Like
Are there any standards that extend ERC-20 or ERC-1193 that add a standard mechanism for getting the image from the token contract? Perhaps something like imageUri() or image()? If so, this standard should specify that if such a method is available it MUST override the value provided by the dapp.
On a related note, I think we should just outright remove name, symbol, and decimals because all supported contracts already MUST have those fields, so the spec essentially asserts that you MUST always ignore the values the dapp provides for them.
I am not a fan of asserting what image types are supported in this specification. The list of supported image types is already out of date (doesn’t include webp) and even if we update it now it’ll just be out of date again in a year. We should leave it up to the community to decide what image types to use, and both dapps and wallets can add support for whatever makes sense at the time (which will constantly change).
If the resolved image is a bitmap, its dimensions SHOULD not exceed 512x512 pixels. Neither the URI nor the image it resolves to SHOULD have a size larger than 256kb.
What is the purpose of this line? Why call out bitmaps specifically? Why not just say that the image size should not exceed 512x512 or 256kb for all image types?
Hmm, I didn’t see this one in your PR. Did I miss it?
I made two PRs. The second one, among other things, added the following line:
If the asset is already listed in the user’s wallet, a prompt to “update” the token information MUST be displayed to the user, even if no information is changed (this is to avoid fingerprinting). Regardless of whether the user chooses to update the token information, the call SHOULD succeed.
Dimensions don’t make sense for vector graphics
This makes sense. I’ll remove that bit.
Actually, ERC-20 lists them as optional:
name
Returns the name of the token - e.g.
"MyToken".OPTIONAL - This method can be used to improve usability, but interfaces and other contracts MUST NOT expect these values to be present.
function name() public view returns (string)symbol
Returns the symbol of the token. E.g.
“HIX”.OPTIONAL - This method can be used to improve usability, but interfaces and other contracts MUST NOT expect these values to be present.
function symbol() public view returns (string)decimals
Returns the number of decimals the token uses - e.g.
8, means to divide the token amount by100000000to get its user representation.OPTIONAL - This method can be used to improve usability, but interfaces and other contracts MUST NOT expect these values to be present.
function decimals() public view returns (uint8)
Regardless, I agree that those fields are effectively redundant, and will eventually remove them (I won’t right now because there’s another change I want to make simultaneously).
Yes, there is, and that’s the other thing I plan to do in the aforementioned change!
I don’t think the specification should specify how wallets should behave internally (what to prompt the user with and what not to). This is unenforceable and over-prescriptive. However, I do think that the API should be clear on expected responses. I do not believe there should be an error for “already added”. The response should just be “added” regardless of whether the token was added or already existed.
So if the dapp makes a wallet_watchAsset request, there should be two possible responses:
- Watched.
- Not-watched.
Whether the user is actually watching the asset, or whether they were already watching the asset, or if they previously rejected, or the wallet auto-rejected, or the user rejected, or the user rejected but the wallet returns “watched” anyway to protect the user while allowing them to use the dapp.
If you want to go even more extreme on protecting user privacy, you can have only a single allowed response of OK, which would essentially be the wallet responding with “I understand the request”. This way the dapp can gain no information from the request other than whether the user is using a wallet that has some form of EIP-747 implementation.
Is there a reason that the dapp needs to know whether the watch has been successful or not? On the surface at least, it feels like the dapp shouldn’t need to know this information so they shouldn’t be informed.
That seems like a good idea. I’ll change that.
EIP-747 now uses ERC-1046!