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

ARC-52 Contextual Wallet API #239

Open
wants to merge 55 commits into
base: main
Choose a base branch
from

Conversation

ehanoc
Copy link

@ehanoc ehanoc commented Aug 29, 2023

Embracing wallets ability to be (or use) a KMS for more functions that are web3 friendly / compliant.

This also addresses problems for some projects in the ecosystem where wallets aren't able to generate, hold and recover keys that related to something else than transactions (i.e Identity / DIDs, Authentication, Secret Sharing, etc)

@ehanoc ehanoc changed the title Contextual Wallet API ARC-?? Contextual Wallet API Aug 29, 2023
@SudoWeezy SudoWeezy changed the title ARC-?? Contextual Wallet API ARC-52 Contextual Wallet API Aug 30, 2023
@joe-p
Copy link
Contributor

joe-p commented Sep 4, 2023

One thing that needs to be considered when it comes to signing is that currently the Algorand SDK does NOT support signing arbitrary bytes unless they are prefixed with MX. All the current signing contexts (txns, messages, lsigs, etc.) have their own prefix for domain seperation. It seems like we want more general signing capabilities, in which case the SDK should be expanded to support signing ANY arbitrary bytes. I imagine this function should still check to ensure that the bytes being signed are NOT a program/txn.

@ehanoc
Copy link
Author

ehanoc commented Sep 6, 2023

One thing that needs to be considered when it comes to signing is that currently the Algorand SDK does NOT support signing arbitrary bytes unless they are prefixed with MX. All the current signing contexts (txns, messages, lsigs, etc.) have their own prefix for domain seperation. It seems like we want more general signing capabilities, in which case the SDK should be expanded to support signing ANY arbitrary bytes. I imagine this function should still check to ensure that the bytes being signed are NOT a program/txn.

I think this is okay. I think we can just defined the API (or Data Models) that any wallet could, if it chooses to, implement. The SDK can choose to update (or not). But there's not really specific to Algorand about key-generation, bip39/32/44 derivations or signing. In fact i think most Contextual keygen/signing past transactions (&logicsigs maybe) will be done with dedicated keys, not directly associated with addresses / accounts. Which i think it would make cryptographic sense to have dedicated keys for dedicated purposes; but still all of them recoverable from the same seed.

@k13n
Copy link
Contributor

k13n commented Sep 8, 2023

I think method sign should take some extra parameters that allows an application to communicate to the wallet & user what is being signed. This additional information can be structured or unstructured. If we consider the example of signing logicsig templates (ARC-47), how would the dapp communicate the ARC-47 template and the necessary placeholder replacements to the wallet?

Why is it only possible to export GPG keys but not any other kind of keys? It seems like we should either support exporting all keys or no keys.

Why is the interface written in typescript? Not all wallets are web-wallets written in TS/JS. How is a mobile wallet supposed to implement this ARC? Maybe this is meant as some abstract pseudo-code, but if so, it seems too specific to typescript since promises are returned. This also applies to many other ARCs that are typically written with some JS runtime in mind.

@pbennett
Copy link
Contributor

One thing that needs to be considered when it comes to signing is that currently the Algorand SDK does NOT support signing arbitrary bytes unless they are prefixed with MX.

The SDK has a sign bytes function but IT prepends MX prior to the received bytes and then hashes it. The bytes passed into it can be anything.

@emg110
Copy link
Contributor

emg110 commented Sep 26, 2023

Great ARC Proposal! So excited to see how it goes!

@ehanoc ehanoc marked this pull request as ready for review October 25, 2023 12:15
@ehanoc ehanoc self-assigned this Oct 27, 2023
@ehanoc
Copy link
Author

ehanoc commented Oct 29, 2023

I think method sign should take some extra parameters that allows an application to communicate to the wallet & user what is being signed. This additional information can be structured or unstructured. If we consider the example of signing logicsig templates (ARC-47), how would the dapp communicate the ARC-47 template and the necessary placeholder replacements to the wallet?

Why is it only possible to export GPG keys but not any other kind of keys? It seems like we should either support exporting all keys or no keys.

Why is the interface written in typescript? Not all wallets are web-wallets written in TS/JS. How is a mobile wallet supposed to implement this ARC? Maybe this is meant as some abstract pseudo-code, but if so, it seems too specific to typescript since promises are returned. This also applies to many other ARCs that are typically written with some JS runtime in mind.

Taken this into account @k13n . Thanks for this.

I've removed the GPG bit. Might introduce too much confusion.


- `context`: The context of the key to be generated
- `account`: The account index to be used for key derivation. The value value should be hardened as per BIP44
- `keyIndex`: The key index to be used for key derivation.

Choose a reason for hiding this comment

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

Bip-44 includes the change type but we are not incorporating it?

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

Successfully merging this pull request may close these issues.