From d7a14a2dceae9da25ccb4f80af29b0d10cd500d8 Mon Sep 17 00:00:00 2001 From: SaReN Date: Thu, 25 Feb 2021 01:26:10 +0530 Subject: [PATCH 1/2] Add multisign batch command (#7787) * initial commit * update signing data * Update signature * code cleanup * code cleanup * Add test for ms batch * update test * add build flag * update flags * update tests * add test for signbatch multisig * update test * fix sign batch multisig * add test * update offline usage * update with sign batch fix * fix lint * update tests * update test * update tests * fix signature only * update seq * fix conflicts * update multisign * revert unintended * fix tests * rename flags * code refactor * fix typo * update docs * update test * Update x/auth/client/cli/tx_multisign.go * use named return values and explicit return * Update x/auth/client/cli/tx_multisign.go Co-authored-by: Robert Zaremba * Update x/auth/client/cli/tx_multisign.go Co-authored-by: Robert Zaremba Co-authored-by: Alessio Treglia Co-authored-by: Jonathan Gimeno Co-authored-by: Jonathan Gimeno Co-authored-by: Robert Zaremba Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 77668a3c23cfb28b9866c65616202f302dc92152) --- simapp/simd/cmd/root.go | 1 + x/auth/client/cli/cli_test.go | 77 +++++++++++ x/auth/client/cli/tx_multisign.go | 221 ++++++++++++++++++++++++++++-- x/auth/client/cli/tx_sign.go | 15 +- x/auth/client/testutil/helpers.go | 14 ++ 5 files changed, 307 insertions(+), 21 deletions(-) diff --git a/simapp/simd/cmd/root.go b/simapp/simd/cmd/root.go index e7713a28454..7f1c542d63c 100644 --- a/simapp/simd/cmd/root.go +++ b/simapp/simd/cmd/root.go @@ -133,6 +133,7 @@ func txCommand() *cobra.Command { authcmd.GetSignCommand(), authcmd.GetSignBatchCommand(), authcmd.GetMultiSignCommand(), + authcmd.GetMultiSignBatchCmd(), authcmd.GetValidateSignaturesCommand(), flags.LineBreak, authcmd.GetBroadcastCommand(), diff --git a/x/auth/client/cli/cli_test.go b/x/auth/client/cli/cli_test.go index fdd558b7971..21f8ef021e7 100644 --- a/x/auth/client/cli/cli_test.go +++ b/x/auth/client/cli/cli_test.go @@ -817,6 +817,83 @@ func (s *IntegrationTestSuite) TestSignBatchMultisig() { s.Require().NoError(err) } +func (s *IntegrationTestSuite) TestMultisignBatch() { + val := s.network.Validators[0] + + // Fetch 2 accounts and a multisig. + account1, err := val.ClientCtx.Keyring.Key("newAccount1") + s.Require().NoError(err) + account2, err := val.ClientCtx.Keyring.Key("newAccount2") + s.Require().NoError(err) + multisigInfo, err := val.ClientCtx.Keyring.Key("multi") + + // Send coins from validator to multisig. + sendTokens := sdk.NewInt64Coin(s.cfg.BondDenom, 1000) + _, err = bankcli.MsgSendExec( + val.ClientCtx, + val.Address, + multisigInfo.GetAddress(), + sdk.NewCoins(sendTokens), + fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), + fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), + fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()), + fmt.Sprintf("--gas=%d", flags.DefaultGasLimit), + ) + s.Require().NoError(err) + s.Require().NoError(s.network.WaitForNextBlock()) + + generatedStd, err := bankcli.MsgSendExec( + val.ClientCtx, + multisigInfo.GetAddress(), + val.Address, + sdk.NewCoins( + sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(1)), + ), + fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), + fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), + fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()), + fmt.Sprintf("--%s=true", flags.FlagGenerateOnly), + ) + s.Require().NoError(err) + + // Write the output to disk + filename := testutil.WriteToNewTempFile(s.T(), strings.Repeat(generatedStd.String(), 3)) + val.ClientCtx.HomeDir = strings.Replace(val.ClientCtx.HomeDir, "simd", "simcli", 1) + + queryResJSON, err := authtest.QueryAccountExec(val.ClientCtx, multisigInfo.GetAddress()) + s.Require().NoError(err) + var account authtypes.AccountI + s.Require().NoError(val.ClientCtx.JSONMarshaler.UnmarshalInterfaceJSON(queryResJSON.Bytes(), &account)) + + // sign-batch file + res, err := authtest.TxSignBatchExec(val.ClientCtx, account1.GetAddress(), filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", multisigInfo.GetAddress().String(), fmt.Sprintf("--%s", flags.FlagOffline), fmt.Sprintf("--%s=%s", flags.FlagAccountNumber, fmt.Sprint(account.GetAccountNumber())), fmt.Sprintf("--%s=%s", flags.FlagSequence, fmt.Sprint(account.GetSequence()))) + s.Require().NoError(err) + s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) + // write sigs to file + file1 := testutil.WriteToNewTempFile(s.T(), res.String()) + + // sign-batch file with account2 + res, err = authtest.TxSignBatchExec(val.ClientCtx, account2.GetAddress(), filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", multisigInfo.GetAddress().String(), fmt.Sprintf("--%s", flags.FlagOffline), fmt.Sprintf("--%s=%s", flags.FlagAccountNumber, fmt.Sprint(account.GetAccountNumber())), fmt.Sprintf("--%s=%s", flags.FlagSequence, fmt.Sprint(account.GetSequence()))) + s.Require().NoError(err) + s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) + + // multisign the file + file2 := testutil.WriteToNewTempFile(s.T(), res.String()) + res, err = authtest.TxMultiSignBatchExec(val.ClientCtx, filename.Name(), multisigInfo.GetName(), file1.Name(), file2.Name()) + s.Require().NoError(err) + signedTxs := strings.Split(strings.Trim(res.String(), "\n"), "\n") + + // Broadcast transactions. + for _, signedTx := range signedTxs { + signedTxFile := testutil.WriteToNewTempFile(s.T(), signedTx) + val.ClientCtx.BroadcastMode = flags.BroadcastBlock + res, err = authtest.TxBroadcastExec(val.ClientCtx, signedTxFile.Name()) + s.T().Log(res) + s.Require().NoError(err) + s.Require().NoError(s.network.WaitForNextBlock()) + } +} + func (s *IntegrationTestSuite) TestGetAccountCmd() { val := s.network.Validators[0] _, _, addr1 := testdata.KeyTestPubAddr() diff --git a/x/auth/client/cli/tx_multisign.go b/x/auth/client/cli/tx_multisign.go index 2910244f6bf..6432a4b8e14 100644 --- a/x/auth/client/cli/tx_multisign.go +++ b/x/auth/client/cli/tx_multisign.go @@ -1,13 +1,13 @@ package cli import ( - "bufio" "fmt" "io/ioutil" "os" "strings" "github.com/spf13/cobra" + "github.com/spf13/viper" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" @@ -16,6 +16,7 @@ import ( kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/types/multisig" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/errors" signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" "github.com/cosmos/cosmos-sdk/version" authclient "github.com/cosmos/cosmos-sdk/x/auth/client" @@ -35,7 +36,7 @@ Read signature(s) from [signature] file(s), generate a multisig signature compli multisig key [name], and attach it to the transaction read from [file]. Example: -$ %s multisign transaction.json k1k2k3 k1sig.json k2sig.json k3sig.json +$ %s tx multisign transaction.json k1k2k3 k1sig.json k2sig.json k3sig.json If the flag --signature-only flag is on, it outputs a JSON representation of the generated signature only. @@ -43,6 +44,9 @@ of the generated signature only. The --offline flag makes sure that the client will not reach out to an external node. Thus account number or sequence number lookups will not be performed and it is recommended to set such parameters manually. + +The current multisig implementation doesn't support SIGN_MORE_DIRECT and defaults +to amino-json sign mode.' `, version.AppName, ), @@ -82,20 +86,9 @@ func makeMultiSignCmd() func(cmd *cobra.Command, args []string) (err error) { return err } - backend, _ := cmd.Flags().GetString(flags.FlagKeyringBackend) - - inBuf := bufio.NewReader(cmd.InOrStdin()) - kb, err := keyring.New(sdk.KeyringServiceName(), backend, clientCtx.HomeDir, inBuf) - if err != nil { - return - } - - multisigInfo, err := kb.Key(args[1]) + multisigInfo, err := getMultisigInfo(clientCtx, args[1]) if err != nil { - return - } - if multisigInfo.GetType() != keyring.TypeMulti { - return fmt.Errorf("%q must be of type %s: %s", args[1], keyring.TypeMulti, multisigInfo.GetType()) + return err } multisigPub := multisigInfo.GetPubKey().(*kmultisig.LegacyAminoPubKey) @@ -195,6 +188,171 @@ func makeMultiSignCmd() func(cmd *cobra.Command, args []string) (err error) { } } +func GetMultiSignBatchCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "multisign-batch [file] [name] [[signature-file]...]", + Short: "Assemble multisig transactions in batch from batch signatures", + Long: strings.TrimSpace( + fmt.Sprintf(`Assemble a batch of multisig transactions generated by batch sign command. + +Read signature(s) from [signature] file(s), generates multisig signatures compliant to the +multisig key [name], and attach it to the transactions read from [file]. + +Example: +$ %s tx multisign-batch transactions.json multisigk1k2k3 k1sigs.json k2sigs.json k3sig.json + +The current multisig implementation doesn't support sign_mode_direct and defaults +to amino-json sign mode.' +`, version.AppName, + ), + ), + PreRun: preSignCmd, + RunE: makeBatchMultisignCmd(), + Args: cobra.MinimumNArgs(3), + } + + cmd.Flags().Bool(flagNoAutoIncrement, false, "disable sequence auto increment") + cmd.Flags().String( + flagMultisig, "", + "Address of the multisig account on behalf of which the transaction shall be signed", + ) + flags.AddTxFlagsToCmd(cmd) + + return cmd +} + +func makeBatchMultisignCmd() func(cmd *cobra.Command, args []string) error { + return func(cmd *cobra.Command, args []string) (err error) { + var clientCtx client.Context + + clientCtx, err = client.GetClientTxContext(cmd) + if err != nil { + return err + } + + txCfg := clientCtx.TxConfig + txFactory := tx.NewFactoryCLI(clientCtx, cmd.Flags()) + if txFactory.SignMode() == signingtypes.SignMode_SIGN_MODE_UNSPECIFIED { + txFactory = txFactory.WithSignMode(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON) + } + + var infile = os.Stdin + if args[0] != "-" { + infile, err = os.Open(args[0]) + defer func() { + err2 := infile.Close() + if err == nil { + err = err2 + } + }() + + if err != nil { + return fmt.Errorf("couldn't open %s: %w", args[0], err) + } + } + scanner := authclient.NewBatchScanner(txCfg, infile) + + multisigInfo, err := getMultisigInfo(clientCtx, args[1]) + if err != nil { + return err + } + + var signatureBatch [][]signingtypes.SignatureV2 + for i := 2; i < len(args); i++ { + sigs, err := readSignaturesFromFile(clientCtx, args[i]) + if err != nil { + return err + } + + signatureBatch = append(signatureBatch, sigs) + } + + if !clientCtx.Offline { + accnum, seq, err := clientCtx.AccountRetriever.GetAccountNumberSequence(clientCtx, multisigInfo.GetAddress()) + if err != nil { + return err + } + + txFactory = txFactory.WithAccountNumber(accnum).WithSequence(seq) + } + + for i := 0; scanner.Scan(); i++ { + txBldr, err := txCfg.WrapTxBuilder(scanner.Tx()) + if err != nil { + return err + } + + multisigPub := multisigInfo.GetPubKey().(*kmultisig.LegacyAminoPubKey) + multisigSig := multisig.NewMultisig(len(multisigPub.PubKeys)) + signingData := signing.SignerData{ + ChainID: txFactory.ChainID(), + AccountNumber: txFactory.AccountNumber(), + Sequence: txFactory.Sequence(), + } + + for _, sig := range signatureBatch { + err = signing.VerifySignature(sig[i].PubKey, signingData, sig[i].Data, txCfg.SignModeHandler(), txBldr.GetTx()) + if err != nil { + return fmt.Errorf("couldn't verify signature: %w %v", err, sig) + } + + if err := multisig.AddSignatureV2(multisigSig, sig[i], multisigPub.GetPubKeys()); err != nil { + return err + } + } + + sigV2 := signingtypes.SignatureV2{ + PubKey: multisigPub, + Data: multisigSig, + Sequence: txFactory.Sequence(), + } + + err = txBldr.SetSignatures(sigV2) + if err != nil { + return err + } + + sigOnly, _ := cmd.Flags().GetBool(flagSigOnly) + aminoJSON, _ := cmd.Flags().GetBool(flagAmino) + + var json []byte + + if aminoJSON { + stdTx, err := tx.ConvertTxToStdTx(clientCtx.LegacyAmino, txBldr.GetTx()) + if err != nil { + return err + } + + req := rest.BroadcastReq{ + Tx: stdTx, + Mode: "block|sync|async", + } + + json, _ = clientCtx.LegacyAmino.MarshalJSON(req) + + } else { + json, err = marshalSignatureJSON(txCfg, txBldr, sigOnly) + if err != nil { + return err + } + } + + err = clientCtx.PrintString(fmt.Sprintf("%s\n", json)) + if err != nil { + return err + } + + if viper.GetBool(flagNoAutoIncrement) { + continue + } + sequence := txFactory.Sequence() + 1 + txFactory = txFactory.WithSequence(sequence) + } + + return nil + } +} + func unmarshalSignatureJSON(clientCtx client.Context, filename string) (sigs []signingtypes.SignatureV2, err error) { var bytes []byte if bytes, err = ioutil.ReadFile(filename); err != nil { @@ -202,3 +360,36 @@ func unmarshalSignatureJSON(clientCtx client.Context, filename string) (sigs []s } return clientCtx.TxConfig.UnmarshalSignatureJSON(bytes) } + +func readSignaturesFromFile(ctx client.Context, filename string) (sigs []signingtypes.SignatureV2, err error) { + bz, err := ioutil.ReadFile(filename) + if err != nil { + return nil, err + } + + newString := strings.TrimSuffix(string(bz), "\n") + lines := strings.Split(newString, "\n") + + for _, bz := range lines { + sig, err := ctx.TxConfig.UnmarshalSignatureJSON([]byte(bz)) + if err != nil { + return nil, err + } + + sigs = append(sigs, sig...) + } + return sigs, nil +} + +func getMultisigInfo(clientCtx client.Context, name string) (keyring.Info, error) { + kb := clientCtx.Keyring + multisigInfo, err := kb.Key(name) + if err != nil { + return nil, errors.Wrap(err, "error getting keybase multisig account") + } + if multisigInfo.GetType() != keyring.TypeMulti { + return nil, fmt.Errorf("%q must be of type %s: %s", name, keyring.TypeMulti, multisigInfo.GetType()) + } + + return multisigInfo, nil +} diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index 251282bb8e0..95555451212 100644 --- a/x/auth/client/cli/tx_sign.go +++ b/x/auth/client/cli/tx_sign.go @@ -16,10 +16,11 @@ import ( ) const ( - flagMultisig = "multisig" - flagOverwrite = "overwrite" - flagSigOnly = "signature-only" - flagAmino = "amino" + flagMultisig = "multisig" + flagOverwrite = "overwrite" + flagSigOnly = "signature-only" + flagAmino = "amino" + flagNoAutoIncrement = "no-auto-increment" ) // GetSignBatchCommand returns the transaction sign-batch command. @@ -203,8 +204,10 @@ func preSignCmd(cmd *cobra.Command, _ []string) { } func makeSignCmd() func(cmd *cobra.Command, args []string) error { - return func(cmd *cobra.Command, args []string) error { - clientCtx, err := client.GetClientTxContext(cmd) + return func(cmd *cobra.Command, args []string) (err error) { + var clientCtx client.Context + + clientCtx, err = client.GetClientTxContext(cmd) if err != nil { return err } diff --git a/x/auth/client/testutil/helpers.go b/x/auth/client/testutil/helpers.go index e4f31727768..6d68ef9236f 100644 --- a/x/auth/client/testutil/helpers.go +++ b/x/auth/client/testutil/helpers.go @@ -92,4 +92,18 @@ func QueryAccountExec(clientCtx client.Context, address fmt.Stringer, extraArgs return clitestutil.ExecTestCLICmd(clientCtx, cli.GetAccountCmd(), append(args, extraArgs...)) } +func TxMultiSignBatchExec(clientCtx client.Context, filename string, from string, sigFile1 string, sigFile2 string, extraArgs ...string) (testutil.BufferWriter, error) { + args := []string{ + fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest), + filename, + from, + sigFile1, + sigFile2, + } + + args = append(args, extraArgs...) + + return clitestutil.ExecTestCLICmd(clientCtx, cli.GetMultiSignBatchCmd(), args) +} + // DONTCOVER From 45a5166bd771fce56daa11b85766240e4a1ffa28 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Wed, 24 Feb 2021 22:18:34 +0000 Subject: [PATCH 2/2] fix build failure --- x/auth/client/cli/tx_multisign.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/auth/client/cli/tx_multisign.go b/x/auth/client/cli/tx_multisign.go index 6432a4b8e14..330d5dc5eda 100644 --- a/x/auth/client/cli/tx_multisign.go +++ b/x/auth/client/cli/tx_multisign.go @@ -15,7 +15,6 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/keyring" kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/types/multisig" - sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/errors" signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" "github.com/cosmos/cosmos-sdk/version"