Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update eth_accounts description #620

Merged
merged 1 commit into from
Sep 7, 2023
Merged

Update eth_accounts description #620

merged 1 commit into from
Sep 7, 2023

Conversation

asciiman
Copy link
Member

@asciiman asciiman commented Sep 7, 2023

No description provided.

@Hugoo Hugoo merged commit f2a8cfa into main Sep 7, 2023
2 checks passed
@Hugoo Hugoo deleted the update-rpc branch September 7, 2023 19:46
@@ -111,7 +111,7 @@ The method requires that a target chain ID is provided

### eth_accounts {#eth_accounts}

Similar to the `eth_requestAccounts` this method returns all of the addresses that are controlled by the application.
Similar to the `eth_requestAccounts` this method returns all of the addresses that the user has approved for the DApp. This method does not trigger a user interface.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But does it really "returns all of the addresses that the user has approved for the DApp"?
From what I found the current implementation returns an array with 1 address (or 0 in case of some issue).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thought going through my mind all day today: why do we need two different eth_requestAccounts and eth_accounts?

Once you have approved N profiles for some DApp - DApp should remember them.
After, I can only imagine that the DApp decided to ask for more addresses e.g. if a user triggered clicked some imaginable "Add profile" button to approve or make visible more profiles in the DApp.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The story of eth_requestAccounts and eth_accounts is somewhat similar to signing RPCs (though, I haven't read about these RPCs as much as about signing RPCs).

We had initially eth_accounts. Used by nodes on Ethereum network to be able to ask the node "What accounts do you have/control?". There is no expectation that node will report only part. It's all or none.

Then comes eth_requestAccounts. Now we have some UI and the user decides what addresses should be visible.

I can see one rather dumb use case for eth_accounts is when a DApp forgets what addresses were approved for it and needs a refresh.

Copy link
Member Author

@asciiman asciiman Sep 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But does it really "returns all of the addresses that the user has approved for the DApp"? From what I found the current implementation returns an array with 1 address (or 0 in case of some issue).

We only allow at the moment either 0 or 1 accounts to be approved per dapp, and we always return those 0 or 1. So technically we match the spec (returning "all" of the accounts the user approved). In the future we may allow more accounts to connect simultaneously, and if we do we should return all of them. It probably wouldn't hurt to mention this in the docs though, that at the moment it's a maximum of 1.

Copy link
Member Author

@asciiman asciiman Sep 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thought going through my mind all day today: why do we need two different eth_requestAccounts and eth_accounts?

Once you have approved N profiles for some DApp - DApp should remember them. After, I can only imagine that the DApp decided to ask for more addresses e.g. if a user triggered clicked some imaginable "Add profile" button to approve or make visible more profiles in the DApp.

The reason we need eth_accounts is because the user can disconnect their account from a dapp between page loads of the dapp. So each time the dapp loads it needs to call eth_accounts. If they called eth_requestAccounts on page load it would pop up the UI as soon as the page loads which is undesirable (though there are dapps that incorrectly do this!). eth_requestAccounts should only be shown in response to a user action (user clicks the Connect button).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants