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

Implementation of getAccountKey() in DAppSigner #184

Closed
regcs opened this issue Jun 17, 2024 · 7 comments
Closed

Implementation of getAccountKey() in DAppSigner #184

regcs opened this issue Jun 17, 2024 · 7 comments
Labels
Needs Triage Further development work cannot be done New Feature A new feature, service, or documentation. Major changes that are not backwards compatible.

Comments

@regcs
Copy link

regcs commented Jun 17, 2024

Is your feature request related to a problem? Please describe. A clear and concise
description of what the problem is. Ex. I'm always frustrated when [...]

I wanted to extract the public key of the connected account via the getAccountKey() method. However, I encountered a Error: Method not Implemented. Obviously, this method is not implemented here:

throw new Error('Method not implemented.')

Describe the solution you'd like A clear and concise description of what you want to happen.

I would like to have this implemented and wonder what reason there is, that this was not implemented initially. Anyone, who can comment on this?

Describe alternatives you've considered A clear and concise description of any alternative
solutions or features you've considered.

None

Additional context Add any other context or screenshots about the feature request here.

None

@AdrianKBL
Copy link

Not sure why is not implemented @regcs , I'm gonna ping @hgraphql , in the meantime you could get that value doing a fetch to the MirrorNode with the account logged.

@regcs
Copy link
Author

regcs commented Jun 26, 2024

Thanks, Adrian. I am using the mirror node as a workaround already now. It's not super urgent, I just saw it's not implemented and thought it might just have been forgotten. Thanks for pinging @hgraphql 😊

@tmctl
Copy link
Contributor

tmctl commented Jul 19, 2024

hey @regcs, would you be willing to make a PR to implement this function?

@regcs
Copy link
Author

regcs commented Jul 19, 2024

Hi @tmctl, I have already worked on this and obtained some more understanding on the matter. I probably ran into the same issues as the initial developer who decided to not implement the method. Since we cannot derive the public key from the account ID, there are five options I can imagine:

  1. Query the account information from a consensus node (requires user interaction & costs the user $0.0001)
  2. Query the account information from a mirror node (requires no user interaction, but a mirror node provider input)
  3. Send a sign() request to the wallet and extract the public key from the signature (requires user interaction)
  4. Change the CAIP definition for hedera to include the public key of the account
  5. Implement a separate mechanism to exchange the information between dApps and wallet providers

IMO, options 1 to 3 are not really beneficial and can be handled by the developers themselves. Options 4 to 5 would require more efforts and maybe not an efficient time investment to solve this small issue.

What are your opinions on that?

@hgraphql hgraphql added the New Feature A new feature, service, or documentation. Major changes that are not backwards compatible. label Jul 29, 2024
@gregscullard
Copy link
Contributor

@regcs could you clarify the need here, I'm unsure what exactly you're looking for, is it

  • Getting the public key for any account via Hedera Wallet Connect
  • Getting the public key of the account currently used by a wallet connected to a dApp via Hedera Wallet Connect

Feel free to expand with user stories such as the below, these are often useful in helping others understand what you have in mind

  • As a dapp developer, I want to be able to retrieve the public key of the connected account

@regcs
Copy link
Author

regcs commented Jul 29, 2024

@gregscullard Thanks, there is not much to expand on from my side. It's exactly what you wrote as a user stories.

The origin was, that the DAppSigner has a method prepared for that and I expected that wallet connect would be able to output the public key of the connected account without querying a mirror or consensus node.

@tmctl tmctl added the Needs Triage Further development work cannot be done label Jul 29, 2024
@kantorcodes
Copy link
Contributor

Given the current architecture and the target solution being out of reach right now, it makes more sense to pursue this functionality as we are exploring the v2 upgrade to Reown. I will be closing this issue for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Triage Further development work cannot be done New Feature A new feature, service, or documentation. Major changes that are not backwards compatible.
Projects
None yet
Development

No branches or pull requests

6 participants