fix: nested multisig signatures using CLI (#20438)

This commit is contained in:
Facundo Medica 2024-06-14 18:57:00 +02:00 committed by GitHub
parent 32bdca3600
commit b9ca318121
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 134 additions and 28 deletions

View File

@ -45,6 +45,30 @@ func TestBech32KeysOutput(t *testing.T) {
require.Equal(t, "{Name:multisig Type:multi Address:cosmos1nf8lf6n4wa43rzmdzwe6hkrnw5guekhqt595cw PubKey:{\"@type\":\"/cosmos.crypto.multisig.LegacyAminoPubKey\",\"threshold\":1,\"public_keys\":[{\"@type\":\"/cosmos.crypto.secp256k1.PubKey\",\"key\":\"AurroA7jvfPd1AadmmOvWM2rJSwipXfRf8yD6pLbA2DJ\"}]} Mnemonic:}", fmt.Sprintf("%+v", out))
}
// TestBech32KeysOutputNestedMsig tests that the output of a nested multisig key is correct
func TestBech32KeysOutputNestedMsig(t *testing.T) {
sk := secp256k1.PrivKey{Key: []byte{154, 49, 3, 117, 55, 232, 249, 20, 205, 216, 102, 7, 136, 72, 177, 2, 131, 202, 234, 81, 31, 208, 46, 244, 179, 192, 167, 163, 142, 117, 246, 13}}
tmpKey := sk.PubKey()
nestedMultiSig := kmultisig.NewLegacyAminoPubKey(1, []types.PubKey{tmpKey})
multisigPk := kmultisig.NewLegacyAminoPubKey(2, []types.PubKey{tmpKey, nestedMultiSig})
k, err := keyring.NewMultiRecord("multisig", multisigPk)
require.NotNil(t, k)
require.NoError(t, err)
pubKey, err := k.GetPubKey()
require.NoError(t, err)
accAddr := sdk.AccAddress(pubKey.Address())
expectedOutput, err := NewKeyOutput(k.Name, k.GetType(), accAddr, multisigPk, addresscodec.NewBech32Codec("cosmos"))
require.NoError(t, err)
out, err := MkAccKeyOutput(k, addresscodec.NewBech32Codec("cosmos"))
require.NoError(t, err)
require.Equal(t, expectedOutput, out)
require.Equal(t, "{Name:multisig Type:multi Address:cosmos1nffp6v2j7wva4y4975exlrv8x5vh39axxt3swz PubKey:{\"@type\":\"/cosmos.crypto.multisig.LegacyAminoPubKey\",\"threshold\":2,\"public_keys\":[{\"@type\":\"/cosmos.crypto.secp256k1.PubKey\",\"key\":\"AurroA7jvfPd1AadmmOvWM2rJSwipXfRf8yD6pLbA2DJ\"},{\"@type\":\"/cosmos.crypto.multisig.LegacyAminoPubKey\",\"threshold\":1,\"public_keys\":[{\"@type\":\"/cosmos.crypto.secp256k1.PubKey\",\"key\":\"AurroA7jvfPd1AadmmOvWM2rJSwipXfRf8yD6pLbA2DJ\"}]}]} Mnemonic:}", fmt.Sprintf("%+v", out))
}
func TestProtoMarshalJSON(t *testing.T) {
require := require.New(t)
pubkeys := generatePubKeys(3)

View File

@ -169,6 +169,11 @@ func packPubKeys(pubKeys []cryptotypes.PubKey) ([]*types.Any, error) {
return nil, err
}
anyPubKeys[i] = any
// sets the compat.aminoBz value
if err := anyPubKeys[i].UnmarshalAmino(pubKeys[i].Bytes()); err != nil {
return nil, err
}
}
return anyPubKeys, nil
}

View File

@ -447,6 +447,22 @@ simd tx multi-sign transaction.json k1k2k3 k1sig.json k2sig.json k3sig.json
Where `k1k2k3` is the multisig account address, `k1sig.json` is the signature of the first signer, `k2sig.json` is the signature of the second signer, and `k3sig.json` is the signature of the third signer.
##### Nested multisig transactions
To allow transactions to be signed by nested multisigs, meaning that a participant of a multisig account can be another multisig account, the `--skip-signature-verification` flag must be used.
```bash
# First aggregate signatures of the multisig participant
simd tx multi-sign transaction.json ms1 ms1p1sig.json ms1p2sig.json --signature-only --skip-signature-verification > ms1sig.json
# Then use the aggregated signatures and the other signatures to sign the final transaction
simd tx multi-sign transaction.json k1ms1 k1sig.json ms1sig.json --skip-signature-verification
```
Where `ms1` is the nested multisig account address, `ms1p1sig.json` is the signature of the first participant of the nested multisig account, `ms1p2sig.json` is the signature of the second participant of the nested multisig account, and `ms1sig.json` is the aggregated signature of the nested multisig account.
`k1ms1` is a multisig account comprised of an individual signer and another nested multisig account (`ms1`). `k1sig.json` is the signature of the first signer of the individual member.
More information about the `multi-sign` command can be found running `simd tx multi-sign --help`.
#### `multisign-batch`

View File

@ -47,6 +47,10 @@ If the --offline flag is on, the client will not reach out to an external node.
Account number or sequence number lookups are not performed so you must
set these parameters manually.
If the --skip-signature-verification flag is on, the command will not verify the
signatures in the provided signature files. This is useful when the multisig
account is a signer in a nested multisig scenario.
The current multisig implementation defaults to amino-json sign mode.
The SIGN_MODE_DIRECT sign mode is not supported.'
`,
@ -57,6 +61,7 @@ The SIGN_MODE_DIRECT sign mode is not supported.'
Args: cobra.MinimumNArgs(3),
}
cmd.Flags().Bool(flagSkipSignatureVerification, false, "Skip signature verification")
cmd.Flags().Bool(flagSigOnly, false, "Print only the generated signature, then exit")
cmd.Flags().String(flags.FlagOutputDocument, "", "The document is written to the given file instead of STDOUT")
flags.AddTxFlagsToCmd(cmd)
@ -109,6 +114,10 @@ func makeMultiSignCmd() func(cmd *cobra.Command, args []string) (err error) {
return err
}
// avoid signature verification if the sender of the tx is different than
// the multisig key (useful for nested multisigs).
skipSigVerify, _ := cmd.Flags().GetBool(flagSkipSignatureVerification)
multisigPub := pubKey.(*kmultisig.LegacyAminoPubKey)
multisigSig := multisig.NewMultisig(len(multisigPub.PubKeys))
if !clientCtx.Offline {
@ -153,11 +162,13 @@ func makeMultiSignCmd() func(cmd *cobra.Command, args []string) (err error) {
}
txData := adaptableTx.GetSigningTxData()
err = signing.VerifySignature(cmd.Context(), sig.PubKey, txSignerData, sig.Data,
txCfg.SignModeHandler(), txData)
if err != nil {
addr, _ := sdk.AccAddressFromHexUnsafe(sig.PubKey.Address().String())
return fmt.Errorf("couldn't verify signature for address %s", addr)
if !skipSigVerify {
err = signing.VerifySignature(cmd.Context(), sig.PubKey, txSignerData, sig.Data,
txCfg.SignModeHandler(), txData)
if err != nil {
addr, _ := sdk.AccAddressFromHexUnsafe(sig.PubKey.Address().String())
return fmt.Errorf("couldn't verify signature for address %s %w", addr, err)
}
}
if err := multisig.AddSignatureV2(multisigSig, sig, multisigPub.GetPubKeys()); err != nil {

View File

@ -14,15 +14,17 @@ import (
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/client/tx"
kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
sdk "github.com/cosmos/cosmos-sdk/types"
)
const (
flagMultisig = "multisig"
flagOverwrite = "overwrite"
flagSigOnly = "signature-only"
flagNoAutoIncrement = "no-auto-increment"
flagAppend = "append"
flagMultisig = "multisig"
flagOverwrite = "overwrite"
flagSigOnly = "signature-only"
flagSkipSignatureVerification = "skip-signature-verification"
flagNoAutoIncrement = "no-auto-increment"
flagAppend = "append"
)
// GetSignBatchCommand returns the transaction sign-batch command.
@ -224,11 +226,40 @@ func sign(clientCtx client.Context, txBuilder client.TxBuilder, txFactory tx.Fac
}
func multisigSign(clientCtx client.Context, txBuilder client.TxBuilder, txFactory tx.Factory, multisig string) error {
multisigAddr, _, _, err := client.GetFromFields(clientCtx, txFactory.Keybase(), multisig)
multisigAddr, multisigName, _, err := client.GetFromFields(clientCtx, txFactory.Keybase(), multisig)
if err != nil {
return fmt.Errorf("error getting account from keybase: %w", err)
}
fromRecord, err := clientCtx.Keyring.Key(clientCtx.FromName)
if err != nil {
return fmt.Errorf("error getting account from keybase: %w", err)
}
fromPubKey, err := fromRecord.GetPubKey()
if err != nil {
return err
}
multisigkey, err := clientCtx.Keyring.Key(multisigName)
if err != nil {
return err
}
multisigPubKey, err := multisigkey.GetPubKey()
if err != nil {
return err
}
isSigner, err := isMultisigSigner(clientCtx, multisigPubKey, fromPubKey)
if err != nil {
return err
}
if !isSigner {
return fmt.Errorf("signing key is not a part of multisig key")
}
if err = authclient.SignTxWithSignerAddress(
txFactory,
clientCtx,
@ -244,6 +275,33 @@ func multisigSign(clientCtx client.Context, txBuilder client.TxBuilder, txFactor
return nil
}
// isMultisigSigner checks if the given pubkey is a signer in the multisig or in
// any of the nested multisig signers.
func isMultisigSigner(clientCtx client.Context, multisigPubKey, fromPubKey cryptotypes.PubKey) (bool, error) {
multisigLegacyPub := multisigPubKey.(*kmultisig.LegacyAminoPubKey)
var found bool
for _, pubkey := range multisigLegacyPub.GetPubKeys() {
if pubkey.Equals(fromPubKey) {
found = true
break
}
if nestedMultisig, ok := pubkey.(*kmultisig.LegacyAminoPubKey); ok {
var err error
found, err = isMultisigSigner(clientCtx, nestedMultisig, fromPubKey)
if err != nil {
return false, err
}
if found {
break
}
}
}
return found, nil
}
func setOutputFile(cmd *cobra.Command) (func(), error) {
outputDoc, _ := cmd.Flags().GetString(flags.FlagOutputDocument)
if outputDoc == "" {
@ -376,7 +434,6 @@ func signTx(cmd *cobra.Command, clientCtx client.Context, txFactory tx.Factory,
if err != nil {
return err
}
multisigLegacyPub := multisigPubKey.(*kmultisig.LegacyAminoPubKey)
fromRecord, err := clientCtx.Keyring.Key(fromName)
if err != nil {
@ -387,15 +444,14 @@ func signTx(cmd *cobra.Command, clientCtx client.Context, txFactory tx.Factory,
return err
}
var found bool
for _, pubkey := range multisigLegacyPub.GetPubKeys() {
if pubkey.Equals(fromPubKey) {
found = true
}
isSigner, err := isMultisigSigner(clientCtx, multisigPubKey, fromPubKey)
if err != nil {
return err
}
if !found {
if !isSigner {
return fmt.Errorf("signing key is not a part of multisig key")
}
err = authclient.SignTxWithSignerAddress(
txFactory, clientCtx, multisigAddr, fromName, txBuilder, clientCtx.Offline, overwrite)
if err != nil {

View File

@ -78,16 +78,6 @@ func SignTxWithSignerAddress(txFactory tx.Factory, clientCtx client.Context, add
txFactory = txFactory.WithSignMode(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)
}
// check whether the address is a signer
signers, err := txBuilder.GetTx().GetSigners()
if err != nil {
return err
}
if !isTxSigner(addr, signers) {
return fmt.Errorf("%w: %s", errors.ErrorInvalidSigner, name)
}
if !offline {
txFactory, err = populateAccountFromState(txFactory, clientCtx, addr)
if err != nil {

View File

@ -33,6 +33,10 @@ Since v0.13.0, x/tx follows Cosmos SDK semver: https://github.com/cosmos/cosmos-
## [Unreleased]
### Improvements
* [#20438](https://github.com/cosmos/cosmos-sdk/pull/20438) Add `--skip-signature-verification` flag to multisign command to allow nested multisigs.
## [v0.13.3](https://github.com/cosmos/cosmos-sdk/releases/tag/x/tx/v0.13.3) - 2024-04-22
### Improvements