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

Values supported in WalletConfig 's wallet_key_derivation field are not obvious. Should be enum, or documented. #1094

Open
nain-F49FF806 opened this issue Dec 21, 2023 · 2 comments

Comments

@nain-F49FF806
Copy link
Contributor

nain-F49FF806 commented Dec 21, 2023

Indy Wallet's create_indy_wallet function takes a WalletConfig structure as input for wallet creation.

One of its fields wallet_key_derivation is typed as String, but the implementation supports only a limited set of values.
Users wishing to create a wallet should not have to search through the codebase to find valid values to populate the string field with.

Consider changing its type to enum, or documenting supported values in the WalletConfig structure (and other places the field is exposed to library users).

@gmulhearn-anonyome
Copy link
Contributor

IMO it might be appropriate to redefine the enum for KeyDerivationMethod alongside WalletConfig (here). i think it's appropriate to redefine/duplicate that enum bcus the KeyDerivationMethod is owned by the misc/legacy/libvdrtools/indy-api-types crate, but it's preferred IMO if aries_vcx_core (where WalletConfig lives) exports it's own enum, rather than exporting an enum from a dependency crate. Then you'd create a function which converts the aries_vcx_core KeyDerivationMethod into the indy-api-types KeyDerivationMethod. One of the reason it'd be good for aries_vcx_core to own it's own type, is bcus KeyDerivationMethod might be applicable for other non-indy wallets in the future (e.g. "aries-askar" wallets might have a config using this enum too eventually). Open for discussion on this tho

@gmulhearn-anonyome
Copy link
Contributor

default of argon2i should be appropriate. we also might be able to now remove some of the strings from

pub const WALLET_KDF_ARGON2I_MOD: &str = "ARGON2I_MOD";
which are dedicated to providing string constants for this wallet conf key deriviation field

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

No branches or pull requests

2 participants