status-im / status-desktop

Status Desktop client made in Nim & QML
https://status.app
Mozilla Public License 2.0
271 stars 76 forks source link

[Browser Plugin Connector] Implement request accounts for the browser plugin #15347

Open MishkaRogachev opened 2 days ago

MishkaRogachev commented 2 days ago

Description

Feature should be implemented in status-go as a part of connector service.

Here are steps for eth_requestAccounts:

1) Browser plugin connector receives a rpc call from the dApp. Next in should convert the request, validate it (in case it's an unexpected call) and finally forward it to the wallet section (desktop code) with a signal. It also should hang on a channel to wait for a response from a user input. (It also makes sense to add a 20 min timeout here)

Question: should we redirect to 'eth_accounts' in case it's a connected DApp?

2) Wallet section receives a signal and shows the accounts popup, allowing user to choose accounts to share or dismiss the request. (Part of https://github.com/status-im/status-desktop/issues/15337) Results should be returned to status-go with a rpc call and forwarded to the same channel to finish a rpc.

For the initial step and testing proposes, this step should have an option to process without UI-interaction.

3) The initial rpc execution wakes up on a signal response from the wallet section. In case a user has accepted the request and shared some accounts, this dApp (id) should be saved in the database as connected.

Question: can we use same database as Wallet Connect feature?

Here are steps for eth_accounts:

2) Browser plugin connector receives a rpc call from the dApp. in case it's a connected DApp we should fetch associated accounts from the database and return it into response.

stefandunca commented 2 days ago

Great plan @MishkaRogachev.

Here are some notes that come to my mind reading this.

Question: should we redirect to 'eth_accounts' in case it's a connected DApp?

The current descoped designs allow only one account at a time to connect. Taking this in consideration would mean that you report only the connected account. That would require you keep this information persisted somewhere and use it to serve requests.

Question: can we use same database as Wallet Connect feature?

We already discussed this in the past to have separate dbs for each dApp provider not to mix requirements. So have your own table with its own requirements it seems still the best approach.

Browser plugin connector receives a rpc call from the dApp. in case it's a connected DApp we should fetch associated accounts from the database and return it into response.

This make sense only if we want a separate handling for plugin to provide all the available accounts. However, @benjthayer already confirmed that we want to keep the current approach where user provides only one account to connect with. That means UX will provide the selected account with the approve status-go API.