From 77668a3c23cfb28b9866c65616202f302dc92152 Mon Sep 17 00:00:00 2001 From: SaReN Date: Thu, 25 Feb 2021 01:26:10 +0530 Subject: [PATCH] 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> --- 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 f76948d59cd5..836036bcf08c 100644 --- a/simapp/simd/cmd/root.go +++ b/simapp/simd/cmd/root.go @@ -136,6 +136,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 178dcd01a823..6cd102cade97 100644 --- a/x/auth/client/cli/cli_test.go +++ b/x/auth/client/cli/cli_test.go @@ -812,6 +812,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 ebffeaf0b3a4..61ec84bdc219 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) @@ -196,6 +189,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 { @@ -203,3 +361,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 251282bb8e0c..955554512125 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 e4f31727768e..6d68ef9236f2 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