feat: use global private key for singing cluster messages. #400
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
debendraoli
changed the title
bcsainju
reviewed
Oct 24, 2024
| @@ -109,5 +109,5 @@ func (p *Provider) ImportKeystore(ctx context.Context, keyPath, passphrase strin | |||
|
|
|||
| // keystorePath is the path to the keystore file | |||
| func (p *Provider) keystorePath(addr string) string { | |||
| return path.Join(p.cfg.HomeDir, "keystore", p.NID(), addr) | |||
There was a problem hiding this comment.
since we already have NID folder for general wallet key, should we wrap it inside wallet folder as this would mean breaking changes which doesn't seem required.
There was a problem hiding this comment.
I was planning to write a migration script to facilitate the breaking change.
The keystore location seems logical because it houses all the keys inside.
There was a problem hiding this comment.
yes but is the new "wallet" folder really required ? All the NID folders contains wallet and the new one with cluster identifier contains cluster key.
debendraoli
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request introduces several changes to the
cmdandrelayerpackages, focusing on adding cluster mode support, enhancing key management, and updating message signing logic. The most important changes include adding new imports, modifying configurations, and updating key management and message signing functionalitiesBreaking Change
Chains are previously stored in a following format:
keystore/[nid]/[key]Now it is `keystore/wallets/[nid]/[key]
This choice is logical because keystore location houses all the keys and we need to differentiate the cluster key and the wallets.
Migration script will be needed for the upgrade.
Summary
Cluster Mode and Configuration Enhancements:
cmd/config.go: IntroducedClusterConfigstruct to manage cluster mode settings, including methods for enabling, signing messages, and verifying signatures.cmd/appstate.go: Addedclusterfield toappStatestruct to store cluster mode configuration.cmd/chains.go: UpdatedSetClusterModeto useClusterConfig.Enabledfor enabling cluster mode.Key Management Improvements:
cmd/keystore.go: AddedgenerateClusterKeycommand to generate and encrypt cluster keys, saving them to the keystore.cmd/config.go: Implemented logic inRuntimeConfigto decrypt and set the cluster private key if cluster mode is enabled.Message Signing Updates:
relayer/chains/evm/provider.go: ReplacedSignMessagemethod withGetSignMessageto return the message hash instead of signing it directly.relayer/chains/icon/provider.go: UpdatedSignMessagemethod toGetSignMessage, returning the message hash without signing.relayer/chains/sui/provider.go: ChangedSignMessagetoGetSignMessage, returning the message directly.Directory Structure Adjustments:
relayer/chains/evm/keys.go,relayer/chains/icon/keys.go,relayer/chains/steller/keys.go,relayer/chains/sui/keys.go: UpdatedkeystorePathfunction to include a "wallets" subdirectory for better organization. [1] [2] [3] [4]These changes collectively enhance the application's support for cluster mode, improve key management and security, and streamline the message signing process.