-
Notifications
You must be signed in to change notification settings - Fork 4
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: use global private key for singing cluster messages. #400
base: feature/relayer-security-cluster
Are you sure you want to change the base?
feat: use global private key for singing cluster messages. #400
Conversation
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
This pull request introduces several changes to the
cmd
andrelayer
packages, 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
: IntroducedClusterConfig
struct to manage cluster mode settings, including methods for enabling, signing messages, and verifying signatures.cmd/appstate.go
: Addedcluster
field toappState
struct to store cluster mode configuration.cmd/chains.go
: UpdatedSetClusterMode
to useClusterConfig.Enabled
for enabling cluster mode.Key Management Improvements:
cmd/keystore.go
: AddedgenerateClusterKey
command to generate and encrypt cluster keys, saving them to the keystore.cmd/config.go
: Implemented logic inRuntimeConfig
to decrypt and set the cluster private key if cluster mode is enabled.Message Signing Updates:
relayer/chains/evm/provider.go
: ReplacedSignMessage
method withGetSignMessage
to return the message hash instead of signing it directly.relayer/chains/icon/provider.go
: UpdatedSignMessage
method toGetSignMessage
, returning the message hash without signing.relayer/chains/sui/provider.go
: ChangedSignMessage
toGetSignMessage
, 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
: UpdatedkeystorePath
function 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.