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

feat: Add SEDA Keys #320

Merged
merged 7 commits into from
Aug 26, 2024
Merged

feat: Add SEDA Keys #320

merged 7 commits into from
Aug 26, 2024

Conversation

hacheigriega
Copy link
Member

@hacheigriega hacheigriega commented Aug 13, 2024

Explanation of Changes

This PR adds the last component of the pubkey module, the SEDA keys. This implements a CLI command for generating a set of SEDA keys and publishing their public keys on chain. Each of the SEDA keys are generated from a random seed, and they are stored in a single key file seda_keys.json under the same directory where the consensus private key is saved to. This means that, if any of the keys is lost, the user must re-generate and rotate all of his or her SEDA keys.

To generate the SEDA keys and register their public keys on chain, run:

$ sedad tx pubkey add-seda-keys --from satoshi --keyring-backend test --fees 10000000000000000aseda --gas auto --gas-adjustment 1.5

To query the validator's list of SEDA public keys:

$ sedad query pubkey validator-keys sedavaloper1nsnv4g2ejq36asujaettw4ge73pzzpjf86208z
validator_pub_keys:
  indexed_pub_keys:
  - index: 0
    pub_key:
      '@type': /cosmos.crypto.secp256k1.PubKey
      key: A7G7jKH0nldfGvWcLf+KudfJha4iYr4Gq5PRAXvnqVZA
  validator_addr: sedavaloper1nsnv4g2ejq36asujaettw4ge73pzzpjf86208z1000000 10000000000aseda                                                  

When the SEDA Protocol requires a new key to be registered on chain, the add-key command would be updated to generate the new key at some unused index.

Related PRs and Issues

TODO

  • Add tests involving adding multiple keys at once
  • Rename x/pkr to x/pubkey
  • CLI test

Closes #162

@hacheigriega hacheigriega changed the base branch from main to feat/pkr August 13, 2024 17:38
@hacheigriega hacheigriega marked this pull request as ready for review August 22, 2024 19:19
app/app.go Outdated Show resolved Hide resolved
@@ -206,20 +206,15 @@ func LoadVRFKey(keyFilePath string) (*VRFKey, error) {
// If loadPath is specified, it loads the VRF key file at the specified
// path. Otherwise, it generates a new VRF key, whose entropy is randomly
// generated or obtained from the mnemonic, if provided.
func LoadOrGenVRFKey(config *cfg.Config, loadPath, mnemonic string) (vrfPubKey sdkcrypto.PubKey, err error) {
func LoadOrGenVRFKey(config *cfg.Config, loadPath string) (vrfPubKey sdkcrypto.PubKey, err error) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not 100% sure how the dependency between the PKR and other modules is going to look. I thought the pubkeys was going to generate all required keys and the modules that utilise them 'just' send data to the module to have it signed with the correct key.

x/pubkey/client/cli/tx.go Outdated Show resolved Hide resolved
Comment on lines +48 to +50
if err := m.Keeper.PubKeys.Set(ctx, collections.Join(valAddr, indPubKey.Index), pubKey); err != nil {
return nil, err
}
Copy link
Member

Choose a reason for hiding this comment

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

For a different PR: move the knowledge of the store/collections out of the message server?

x/pubkey/types/errors.go Outdated Show resolved Hide resolved
@hacheigriega
Copy link
Member Author

Thanks for the reviews.. I will address them after fixing the JSON unmarshalling issue I found while adding CLI tests.

@hacheigriega
Copy link
Member Author

Done but there is a gas estimation issue to be addressed in a separate PR (See Issue #341)

@hacheigriega hacheigriega merged commit 7bb630d into feat/pkr Aug 26, 2024
1 of 11 checks passed
@hacheigriega hacheigriega deleted the hy/pkr-refactor branch August 26, 2024 14:44
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

Successfully merging this pull request may close these issues.

✨ Pubkeys module
2 participants