feat: Initial version of a key generation utility for ream

[ch4r10t33r]

What was wrong?

• Generation of a key pair for each message_type to be signed.
• Automatic storing of the generated keystore in the default paths.
• Partial implementation of the keystore file as per the new EIP (https://github.com/ReamLabs/EIPs/blob/010eb5698ccb94d12cc6b551aa61d7f02cb9b49a/EIPS/eip-new.md)

To-Do

• I have read CONTRIBUTING.md.
• This PR title follows conventional commits.

[syjn99]

Most of the comments are about cosmetics for codebase consistency (like inlining variable in the logs...), and left some questions that comes into my mind while reading your code. Great job!

[KolbyML]

Here is some feedback

[syjn99]

@ch4r10t33r
Based on the last interop call, I guess MessageType is too overkill in this phase. Key management simplification can be discussed more, and it seems adding one of the method preemptively might make more code changes in the future.

[ch4r10t33r]

@ch4r10t33r Based on the last interop call, I guess MessageType is too overkill in this phase. Key management simplification can be discussed more, and it seems adding one of the method preemptively might make more code changes in the future.

It may seem like overkill at the moment, but this is the direction we’re heading. We should keep the code we’ve already built instead of removing it now and having to add it back later.

[syjn99]

@ch4r10t33r Not all comments are resolved, please take a look.

[ch4r10t33r]

@ch4r10t33r Not all comments are resolved, please take a look.

My apologies, the comments didnt show up at my end. I have now addressed them all.

[syjn99]

Spotted some suspicious things with several issues (dependency, code formatting...)

Also there are some comments that are not resolved but marked as resolved. Please check those again.

[KolbyML]

Left some feedback

[syjn99]

Also could you please change the PR title? account_manager changes are quite vague

[ch4r10t33r]

Also could you please change the PR title? account_manager changes are quite vague

Updated.

[syjn99]

LGTM

[KolbyML]

Here is some feedback

[KolbyML]

I think this function should be implemented on the PrivateKey struct, the PublicKey is a derivative of the PrivateKey

[ch4r10t33r]

I am not sure what you mean. Are you referring to the passphrase? That is used as a seed to generate the keypair.

[KolbyML]

pub fn generate_keys(

[ch4r10t33r]

There is already a generate function in the private key module:

ream/crates/crypto/post_quantum/src/hashsig/private_key.rs

Line 19 in c914186

pub fn generate<R: Rng>(

The generate_keys function in the account manager is simply a wrapper around it, adding functionality such as determining the salt.

[KolbyML]

it is fine to add another, generate_keys is just floating around right now, well it is a constructor for an object

[KolbyML]

I am not sure why this was marked as resolved, pub fn generate_key_pair_with_salt( should be a member of the PrivateKey struct
`

[ch4r10t33r]

I'm not clear on your point. I thought you wanted me to rename both functions.

The generate_key_pair_with_salt function belongs to the account_manager crate, as it’s a core feature of account_manager, not the private key. We’ll expand this functionality with alternative approaches. The private key remains unaffected by these changes and shouldn’t need updates whenever we adjust the strategy in account_manager.

[syjn99]

Dropping my thoughts here:

How about adding a function

  pub fn derive_wallet_rng(
      seed_phrase: &str,
      wallet_index: u32,
      passphrase: &str,
  ) -> ChaCha20Rng

And use it like:

PrivateKey::generate(&mut derive_wallet_rng(...), ...)

?

So that we can remove generate_key* function name which derives some convo here.

[ch4r10t33r]

Sure, I can rename the function. But my main point of contention is that the functionality should remain in account_manager rather than PrivateKey.

[ch4r10t33r]

@KolbyML @syjn99 I will be raising a new PR in the next 2 days which requires additional changes and inclusion of new features related to keystore encryption etc. I can always handle these suggestions as part of that PR. Please can you approve this PR now?

[KolbyML]

PR looks great, good work 🥳