anza-xyz / wallet-standard

Solana extensions to the Wallet Standard.
Apache License 2.0
75 stars 38 forks source link

Support dapps suggesting chains to be added to the user's wallet application #9

Open neelsomani opened 1 year ago

neelsomani commented 1 year ago

Overview

Inspired by EIP-3085, this design allows Solana applications ("dapps") to suggest chains to be added to the user's wallet application. The wallet application may arbitrarily refuse or accept the request. We have received a request for this feature from many dapps.

Desired Outcome

The desired outcome is for dapps that are on testnets, canary chains, or other SVM chains to be able to conveniently prompt their users to switch networks and seamlessly interact with their dapp and see their assets rather than requiring complex manual custom RPC changes. As more SVM chains spin up, this can prevent a mess with multiple standards and one-off implementations down the line.

Implementation

  1. Solana Wallet Standard: At first glance there are some changes needed to the Solana wallet standard. There are places where the wallet is required to return all chains that it supports, so we would need some interface for the dapp to pass back a new chain to be added: https://github.com/solana-labs/wallet-standard/blob/master/packages/wallets/solflare/src/account.ts#L25 This change seems like it could be made immediately if it is done in a non-breaking way. The wallet should not be required to add a chain even if the dapp requests it.
  2. Wallet Adapter: We would need to make some change to the wallet adapter so the dapp can specify the desired RPC network. This would need to be rolled out after the Wallet Standard change.
  3. Dapp Support: The dapp updates their Wallet Adapter library and specify their desired RPC network. If no RPC network is specified, by default this change has no effect - the behavior is exactly the same as SVM wallets function right now (default to Solana mainnet). This is the last change to occur.
jordaaash commented 1 year ago

Thanks for opening discussion on this. First a few unstructured thoughts about security that come to mind --

I haven't noodled on this very hard yet, but I could imagine that allowing applications to add custom RPCs increases the possibility of signing and/or simulating transactions against malicious RPCs that may deceive the wallet user.

Unlike EVM transactions, Solana transactions don't include a chain ID in the message that gets signed, only a recent blockhash / nonce account. Wallets would likely need to check blockhashes they get back from custom RPCs against, at minimum, Solana mainnet blockhashes / nonces. This problem will become worse if users have value on multiple SVM chains that wallets aren't checking.

Will consider this some more, but how are you thinking about this?

neelsomani commented 1 year ago

Could we elaborate a bit on the threat of a dapp suggesting a malicious RPC? Is this threat worse than what a dapp can accomplish already?

Wallets would likely need to check blockhashes they get back from custom RPCs against, at minimum, Solana mainnet blockhashes / nonces

If a user is connected to a custom RPC, could you elaborate on the reason to check blockhashes against Solana mainnet? The user might be connected to a different cluster altogether.

jordaaash commented 1 year ago

the threat of a dapp suggesting a malicious RPC

A wallet that exposes a signAndSendTransaction (or signTransaction) API will typically use an RPC's simulateTransaction API. This works (in the sense that the result is somewhat trustworthy) because the RPC is chosen by the wallet. If a malicious RPC is provided by a malicious app, the RPC can lie about the result of transaction simulation, or any other state or transaction history of the chain. This can be used to induce a user into signing unsafe transactions.

the reason to check blockhashes against Solana mainnet

By extension of the above, if I'm a malicious app providing a malicious RPC, I might ask you to add an RPC for something arbitrary, let's say it's called mychain:testnet. You do this, thinking it's safe. But this RPC returns a blockhash (e.g. using getLatestBlockhash) that would be valid on solana:mainnet. You sign it and send it to the RPC, the RPC replays the transaction on Solana mainnet, you get rugged.

neelsomani commented 1 year ago

The simulateTransaction concern is a tricky one. To be fair even simulateTransaction under normal circumstances could lead to an unintended outcome.

On the RPC point - what if we optionally allow folks to include a restriction on the cluster name in their signed transaction? Seems to mitigate this risk.

I think without this feature there will be substantial headwinds in pushing the SVM as a standard so interested to work with y'all to address these threats!

neelsomani commented 1 year ago

Another possible approach is having wallets maintain a registry of approved RPCs, so a reputable dapp with an app chain can get whitelisted.

jordaaash commented 1 year ago

what if we optionally allow folks to include a restriction on the cluster name in their signed transaction

This cannot be done without changing the wire transaction format. If a malicious dapp is using a malicious RPC, it doesn't matter what either of them says about the cluster they are targeting. Solana needs to implement something like EIP-155 for this to work.

jordaaash commented 1 year ago

Some discussion about this @ https://discord.com/channels/428295358100013066/885976714646290473/1073664382011191396

PrasoonPratham commented 1 year ago

Hey, this is Pratham👋 I recently joined Eclipse as a software engineer.

On Keplr, when you first create a wallet, you get 10 of the most popular cosmos chains added by default; it would also make sense to do this for Solana wallets with SVM chains.

The workflow for adding new chains should be like this: User clicks add chain -> Wallet checks if the RPC is whitelisted -> New chain is added.

Where would we want to store the registry of the whitelisted RPCs?

From what I see, all Solana wallets support 4 RPCs (local, test, dev, and mainnet). They're hardcoded as constants.

We could make a generalized function that checks whether the RPC is valid with the whitelist(which could be easily done with Regex) and then only adds it to a wallet. I'm working on a basic implementation as we speak. Happy to hear your thoughts.

silostack commented 1 year ago

Would love to see this implemented as I think making it easy for a user to switch to another chain (especially Devnet) will be crucial to allow users to explore and experiment on Devnet.

I get there's a bit of complexity here, but I think @PrasoonPratham 's whitelist suggestion is a good starting point. At a minimum, being able to at least easily switch between the hardcoded dev/test/mainnet RPCs would be a huge win imo.

NorbertBodziony commented 3 weeks ago

I think this might be 1st good step to enable full cluster API https://github.com/anza-xyz/wallet-standard/pull/65

In later improvements, we can think about adding custom clusters.