-
Notifications
You must be signed in to change notification settings - Fork 1
Add CubeSigner keychain implementation #181
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
base: main
Are you sure you want to change the base?
Conversation
Add comprehensive documentation for the cubesigner keychain package including setup instructions, usage examples for subnet creation and cross-chain transfers, and integration details with Avalanche wallet.
Fix import grouping to comply with gci linter rules.
Replace icm-services dependency with local type definitions for AggregateSignatureRequest and AggregateSignatureResponse to avoid dependency conflicts with avalanchego/libevm/subnet-evm versions. This resolves linter errors related to incompatible icm-services peer package versions.
Updates the generated mock to reference the correct source package path and adds required golang.org/x/tools dependency for mockgen.
package interchain | ||
|
||
// AggregateSignatureRequest is the request structure for the signature aggregator API. | ||
// This is a local copy to avoid dependency issues with icm-services. |
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.
Does this structure ever get changed and who owns it on icm side? just want to make sure things will not break if the original icm-services struct will get changed
// It validates that the key exists in the CubeSigner organization, verifies | ||
// that the key type is supported (secp256k1 for Avalanche/Ethereum), and | ||
// converts the public key from hex format to an Avalanche public key. | ||
func processKey( |
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.
The function name is very generic, I would name it something like fetchAndVerifyAvaPublicKey
(if you're also not a fan of functions that perform multiple actions, this can be split into verifying and converting functions). Additionally, I would explain in the comment why we convert specifically into Ava key (since eth key can be retrieved from it), otherwise it's not obvious unless the next function is read (NewKeyChain
)
return &Keychain{ | ||
cubesignerClient: cubesignerClient, | ||
avaAddrToKeyInfo: avaAddrToKeyInfo, | ||
ethAddrToKeyInfo: ethAddrToKeyInfo, |
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.
what is the reason for supporting ethereum keys in general?
} | ||
|
||
// Get returns a signer for the given Avalanche address, if it exists in this keychain. | ||
func (kc *Keychain) Get(addr ids.ShortID) (keychain.Signer, bool) { |
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 would rename it to GetSigner
} | ||
|
||
// GetEth returns a signer for the given Ethereum address, if it exists in this keychain. | ||
func (kc *Keychain) GetEth(addr common.Address) (keychain.Signer, bool) { |
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.
Would rename it to GetEthSigner
|
||
// CubeSignerClient defines the interface for CubeSigner client operations | ||
// needed by the keychain implementation | ||
type CubeSignerClient interface { |
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.
Is the implementation going to be in a separate PR?
What is the reason for making it an interface and not a struct?
Why this should be merged
This PR adds CubeSigner integration to the SDK, enabling users to perform secure remote signing for Avalanche transactions without exposing private keys locally. CubeSigner is a widely-used remote signing service that provides enterprise-grade key management for blockchain operations.
This implementation allows users to:
How this works
The PR adds a new
keychain/cubesigner
package that implements bothkeychain.Keychain
andc.EthKeychain
interfaces:cubesigner_keychain.go
- Core keychain implementation that manages CubeSigner keys and provides signing capabilities for Avalanche addressesSign()
- Serialized transaction signing using CubeSigner'sAvaSerializedTxSign
API (recommended for P/X/C chain transactions)SignHash()
- Direct hash signing using CubeSigner'sBlobSign
API (for arbitrary data)How this was tested