hyperledger / aries-vcx

aries-vcx is set of crates to work with DIDs, DID Documents, DIDComm, Verifiable Credentials and Hyperledger Aries.
https://didcomm.org
Apache License 2.0
125 stars 83 forks source link

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

Open nain-F49FF806 opened 11 months ago

nain-F49FF806 commented 11 months ago

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 commented 10 months ago

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 commented 10 months ago

default of argon2i should be appropriate. we also might be able to now remove some of the strings from https://github.com/hyperledger/aries-vcx/blob/8a17c6b9ebf396a0488e3267bf07e238f8d921fc/aries/aries_vcx_core/src/global/settings.rs#L8 which are dedicated to providing string constants for this wallet conf key deriviation field