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

Refactor key types #1829

Merged
merged 8 commits into from
Oct 9, 2023
Merged

Refactor key types #1829

merged 8 commits into from
Oct 9, 2023

Conversation

xDimon
Copy link
Member

@xDimon xDimon commented Oct 5, 2023

Referenced issues

no specified

Description of the Change

Transparent conversion KeyType <-> KeyTypeId.
Literals for key (for human readability)

Benefits

Possible Drawbacks

Usage Examples or Tests

Alternate Designs

Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
core/crypto/crypto_store/key_type.hpp Outdated Show resolved Hide resolved
core/crypto/crypto_store/key_type.hpp Outdated Show resolved Hide resolved
core/crypto/crypto_store/key_type.hpp Show resolved Hide resolved
Comment on lines 68 to 75
store_->generateEd25519Keypair(KeyTypes::GRANDPA, *dev).value();
store_->generateSr25519Keypair(KeyTypes::BABE, *dev).value();
store_->generateSr25519Keypair(KeyTypes::IM_ONLINE, *dev).value();
store_->generateSr25519Keypair(KeyTypes::AUTHORITY_DISCOVERY, *dev)
.value();
store_->generateSr25519Keypair(KeyTypes::KEY_TYPE_ASGN, *dev).value();
store_->generateSr25519Keypair(KeyTypes::KEY_TYPE_PARA, *dev).value();
store_->generateEcdsaKeypair(KeyTypes::BEEFY, *dev).value();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't this be done with a list and a loop?

Copy link
Contributor

@turuslan turuslan Oct 9, 2023

Choose a reason for hiding this comment

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

Would need 3 loops, because of interface, for each crypto type

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it might, but it generated different crypto (Ed*, Sr*, Ecdsa) and different purposes. Simplifying with cycle does not seem simpler.

Copy link
Contributor

Choose a reason for hiding this comment

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

There are five sr keys in a row, for example, this can be refactored to a loop. It's not necessary to make everything one loop.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, done

Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
@xDimon xDimon requested a review from Harrm October 9, 2023 09:43
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
@xDimon xDimon enabled auto-merge (squash) October 9, 2023 10:15

/**
* Types are 32bit integers, which represent encoded 4-char strings
* Makes 32bit integer uin which represent encoded 4-char strings
Copy link
Contributor

Choose a reason for hiding this comment

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

'uin' seems to be a typo.

@xDimon xDimon merged commit e55bef8 into master Oct 9, 2023
9 of 12 checks passed
@xDimon xDimon deleted the refactor/key_types branch October 9, 2023 10: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.

3 participants