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