-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add Support for Multisig Wallets #120
Conversation
… resign/reshare message and submitting signature
Gnosis multisig
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.
Hey,
can you delete all cli paths that work with using keystore directly?
… its signature at the same stage
… instead of secret keys
// Contruct the resign message | ||
rMsg, err := dkgInitiator.ConstructReshareMessage( | ||
oldOperatorIDs, | ||
newOperatorIDs, | ||
signedProofs[0][0].Proof.ValidatorPubKey, | ||
ethNetwork, | ||
cli_utils.WithdrawAddress[:], | ||
cli_utils.OwnerAddress, | ||
cli_utils.Nonce, | ||
signedProofs[0], | ||
) |
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.
In the issue I had the requirement:
In the cli pass the resign/reshare messages along with their signatures as a string in serialized form (Talk with product about the serialization).
But maybe you are doing this in bulk?
Anyhow, the current way is good enough for me and if @RaekwonIII wants to change this he can talk to @MatusKysel
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.
Indeed in this cli users pass the data for generating the reshare/resign messages instead of the messages themselves. Everything can be a string now including the signedProofs
(updated in the bulk PR), so from the users perspective it would be pretty much the same. But of course we can update this if required.
KeystorePass = viper.GetString("ethKeystorePass") | ||
if strings.Contains(KeystorePath, "../") { | ||
return fmt.Errorf("😥 ethKeystorePass should not contain traversal") | ||
Signatures = viper.GetString("signature") |
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.
Signatures = viper.GetString("signature") | |
Signatures = viper.GetString("signatures") |
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.
updated in Bulk PR
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.
looks good, left few minor comments
Solves issue #118
To support Gnosis multisig wallet, this PR modifies the workflow of reshare and resign. Before this PR, an initiator starts a reshare/resign by sending a signedReshare/signedResign message. This PR breaks this process into two stages:
In the case of multisig users, this allows the users collect signatures for the Reshare/Resign message from external processes and submit to the operators later.
We support signature verification for multisig wallets that follows standard EIP-1271 signature verification process (Gnosis Safe is tested to work successfully)