Add `wallet_` methods to improve dapp to wallet interaction

ux

#21

yes if feels a bit duplicated - and usually I am preaching for DRY - but in this case I see it a bit different.

The difference between eth_accounts and wallet_requestAccounts is that eth_account returns all accounts and I see wallet_requestAccounts as the possibility to incrementally expose accounts to a dapp (user chooses which accounts are exposed to a dapp). As far as I understand eth_accounts it is always returning all accounts from a client.

with eth_sendTransaction the problem is a bit more complex. The main problem is that there is no real versioning of the JSON RPC interface. So adding a parameter is really messy. Also I think a wallet should not really send the transaction - this is responsibility of the dApp. The wallet should just sign the transaction and return the signature. Think e.g. about offline signing use cases.

So long story short - I would still signal for adding these methods to the wallet_ namespace and keep the eth_ methods untouched. There can be a translation layer in between that translates eth_sendTransaction to wallet_signTransaction for the migration phase.

Also as a learning from the past we should really add versioning - so I signal the need for wallet_rpcversion - returning a semver string which in the first iteration could be “1.0.0”


#22

@pedrouid I also normally am not a fan of duplicating functionality. My idea was to start clearly separating wallet related calls from general ethereum interaction. Right now everything is part of eth_ and it is not clearly defined what eth_ calls should be implemented by a wallet and which not. This is why the injected MetaMask provider implements nearly every eth_ call somehow (which IMO is not the job of MetaMask or any other wallet provider). Separating this would also make it clear that shared notes like infura are not providing any wallet_ functionality.

To clearly separate this I would move everything wallet/account releated to a new wallet_ group. This will mean that some of the functionality is duplicated, but for me this would make sense.


#23

I still think we are early enough to make these “breaking” changes for a more interoperable ecosystem.

The EIP 1102 and EIP 712 included far more aggressive breaking changes that this proposal which would literally not even change the API for Web3.js which is widely used by 99% of Dapps today.

We would be gaining instant multi-chain compatibility from a single parameter in already existing Dapps!

Adding a new JSON RPC group would require advocating Dapps to adapt while this change would be backwards compatible as the chainId is already handled by middleware in a non-standard way regardless.


#24

But these are different topics, right?

Adding a new JSON RPC group that is not implemented by nodes provides far more flexibility.

Changing an existing JSON RPC call requires you do get everybody (or at least the major) clients (nodes + providers) onboard.

I don’t have a strong opinion on adding the chain id to the JSON RPC call, but for me it is a different discussion :wink:

Also EIP712 didn’t do any breaking changes afaik, as this also is just an additional JSON RPC call and the behaviour of existing calls was not touched. For EIP1102 it only changed the behaviour of the very outer layer for javascript (web3 provider) and that already triggered quite some discussion. Changing a JSON RPC call that influences every client is on a different level for me.


#25

@ligi I updated the EIP draft with the points of our discussion (mentioned Pop up sessions in Berlin today and Friday & sticker transport to ETHDenver)

Here the link to the latest version: https://github.com/rmeissner/EIPs/blob/rmeissner-wallet-rpc/EIPS/eip-xxx.md


#26

thanks - great work! looks good - just some nits:

String - The current wallet protocol version.

I would add that it MUST be given in SemanticVersioning and maybe even give the value “1.0.0” for the state of the current EIP

CHAIN_ID - String

wonder if we should make it an int - if we leave it an int we should define as : decimal / no 0x prefix

WILDCARD - String

should define there it is ‘*’ - you can extract this information from the samples - but I think it should be defined there - perhaps like this:

WILDCARD - String = “*”

not 100% what the correct syntax here is - perhaps someone else can chime in

"error": {
     "code": -32602,
     "message": "Invalid params: Transaction handling type <specified type> not supported"
 }

or

"error": {
     "code": -32602,
     "message": "Invalid params: Chain id <specified chain id> is not supported"
 }

I think we should define different error-codes here - mainly for translations later on.

that’s it for now - hope to do a bit a deeper look later today.


#27

wonder if we should make it an int - if we leave it an int we should define as : decimal / no 0x prefix

I just used a string to avoid possible limits in some languages. Also https://github.com/ethereum/wiki/wiki/JSON-RPC#net_version uses a string. Will specify that it should be an decimal without 0x prefix

not 100% what the correct syntax here is - perhaps someone else can chime in

Yeah not sure about that either, was just copying what is returned when I do an invalid request for other rpc calls. Not even sure if this is a standard or depends on the client. Maybe I should remove the examples for this from the EIP and just state that it should return an error.

Updated the EIP again :wink:


#28

Great - gets better and better!

just nits. the parameter enumeration in wallet_handleTransaction is wrong currently (is:1,1,2 should be 1,2,3)

and I would really specify the current version in wallet_version.

And not yet sure how ERC1271 signing would look like on the wallet side in practice. How let the user know what he is signing?


#29

and I would really specify the current version in wallet_version.

What version would you propose? 1.0.0 or rather 0.1.0 to indicate that this WIP?

And not yet sure how ERC1271 signing would look like on the wallet side in practice. How let the user know what he is signing?

Bascially that works the same as with a normal wallet, just that the generated signature is different. So if we take the Safe (App + Chrome extension) for example it would prompt the user in the chrome extension with the typed data, once the user confirms it there the user would also need to confirm this in the App and then the generated signature bytes (a combination of multiple ecdsa signatures in the case of the Safe) is returned to the app.

To verify this you generate the data blob for the typed data (based on EIP-712) then you call isValidSignature on the wallet contract (address of the wallet for which you called signTypedData) with the data blob bytes and the signature bytes.

I am in the progress of working on a PoC for that, once I have more I can share it :slight_smile:


#30

no strong opinion on this - usually with projects i start x<1.0.0 - but as this is a standard I think it could make sense to do 1.0.0


#31

I would like to follow up on the proposed method wallet_changeChain to update the name to wallet_updateChain and include the following parameters suggested by @danfinlay on an earlier discussion last week on Twitter

The suggested parameters are: chainId, networkId, rpcUrl, nativeCurrencyName.

A JSON RPC request would look like the following for requesting the user to add/connect to a xDai remote node:

{
  "id":1,
  "jsonrpc": "2.0",
  "method": "wallet_updateChain",
  "params": [{ "chainId": 100, "networkId": 1, "rpcUrl": "https://dai.poa.network", "nativeCurrencyName": "xDAI"  }]
}

This would provide a Stateless Dapp like for example @austingriffith’s Burner Wallet a better UX to quickly request within the Wallet to switch the chain/network/node that the Dapp wishes to connect.

Austin has already quick fix for this by displaying a popup to easily guide the user to change the chain on Metamask.

This pattern is very common in a lot of Dapps and this simple JSON RPC request can improve the UX dramatically.

I was planning to include this as a WalletConnect-specific method within the v1.0 SDK but I think this would be more beneficial as a standard that can be shared with other Wallet patterns like Metamask, Ledger, etc


#32

Awesome, thanks for writing this up. I think we could move to another thread, but before we do:

What if the provider could return another provider, allowing Dapps to potentially maintain connections to multiple blockchains at once?

This also limits possible errors related to specifying the wrong network when sending a transaction (which funnily enough, you presented a solution for in another thread by adding a network/chain ID parameter to the sendTransaction method :slight_smile:), clearly a pain you’re hitting as a dapp dev, and that’s the gold of our developer experience, so I’m really eager to add this.

We need to also consider that concern about price feeds. It may be beyond the scope of this initial proposal, and maybe we make-do without exchange rates on custom networks for a while. If we do want to provide a permission for providing a price feed, we will probably want it to use a syntax that can allow for exchange rates pegged to any of the user’s known-priced currencies.

Clients need to do some validation here, because if a dapp can suggest chainId then there’s a replay attack vector if they can suggest a chain ID that the user already uses.

There’s also a UX opportunity for the client, to potentially show the new network’s currency alongside other currencies, as “selected network” becomes increasingly abstracted away from the UI.

This method’s parameters should also be built into the provider.enable() method’s parameters (which I’m thinking of also exposing as wallet_enable()). This would allow the network suggestion to be unified with the account request.

Naturally on that screen we’re going to need to allow account selection, and that login could potentially only be permitted to send transactions on behalf of that requested/approved network.

In fact, the account-on-network is maybe the most important part, since if you wanted to read from this provider you’d just be doing it. What this is really about is making the user aware of a new account they have, with a new base currency, and potentially making it easy to transfer funds into it.

Okay that’s enough thoughts for now!


#33

Agreed. These are really great suggestions. I think returing extra providers for connecting to multiple networks is a good use-case.

This assumes that price feeds would have a standard API and this will just lead us down into another rabbit hole. I think Wallets should handle this with simply not displaying price data for this. Disabling inputs for native currency conversion is faster, easier and less prone to error. Plus I can imagine a bunch of attack vectors here where the user is a approving a transaction for 5 USD equivalent amount when in fact the price feed was purposely tweaked to hide that it was actually 100 times more.

I really like this because that’s how WalletConnect is also designed, in fact it could actually bring both providers to be more similarly designed.


#34

Hi Guys!

We are buiding IOV-core which is a very strong typescript library well tested to connect to different chains from a unique API format including Ethereum, Cosmos and other Tendermint chains.

it is still a work in progress but you can have a look and play with it with the CLI interface:

It seems we want to pursue the same goal with a unify API to access blockchains from a wallet.

Cheers,

Antoine


#35

The idea of triggering a network switch is really cool. I am not sure if this should be part of the initial version of this EIP and I also think that some of the ideas should be coordinated with the provider ring :wink:

Theoretically it is also possible for dapps implement custom chain handling with this EIP. @ligi wanted to have a method that just signs a transaction and returns the signed raw transaction (wallet_handleTransaction). With this it is possible that the dapp uses their own rpc endpoint for their chain with the connected wallet.

We also had a discussion with Martin and Stefan recently that it should be possible for a wallet to tell a dapp what rpc endpoint to use (to allow full decentralization where everbody could use their own node)

Many interesting ideas :slight_smile:


#36

The wallet_handleTransaction method sounds the same as eth_signTransaction. Am I missing something?


#37

yea - sign is a special case of handle. But sometimes you need to do more than just sign. E.g. think about contract wallets.


#38

Hey guys, I was going to start a new thread but after replying to this existing thread on Automatic Authentication Signature. I realised this should be part of the wallet_ methods.

As you can read on the other thread, the proposal describes including a standard authentication signature that would be automatically signed by the Wallet to verify the account ownership on the Dapp side. Perhaps this could be part of either wallet_accounts or wallet_requestAccounts method where we could provide an array of addresses accompanied with a matching signature that would follow a standard authentication message that could be verified by the Dapp. This message doesn’t even require EIP-712 in my opinion, since it’s not meant to be read by the user. Something as simple as doing a personal_sign for a fixed message I own this account that return a JSON RPC response as follows.

{
  "id": 1,
  "jsonrpc": "2.0",
  "result": [
    {
      "address": "0x9b7b2B4f7a391b6F14A81221AE0920A9735B67Fb",
      "signature": "0x30755ed65396facf86c53e6217c52b4daebe72aa4941d89635409de4c9c7f9466d4e9aaec7977f05e923889b33c0d0dd27d7226b6e6f56ce737465c5cfd04be400"
    {
  ]
}


Automatic Authentication Signature
Automatic Authentication Signature