diff --git a/tests/e2e/auth/suite.go b/tests/e2e/auth/suite.go index fd12683c485d..acc152de1815 100644 --- a/tests/e2e/auth/suite.go +++ b/tests/e2e/auth/suite.go @@ -217,6 +217,7 @@ func (s *E2ETestSuite) TestCLISignBatch() { val.ClientCtx.HomeDir = strings.Replace(val.ClientCtx.HomeDir, "simd", "simcli", 1) // sign-batch file - offline is set but account-number and sequence are not +<<<<<<< HEAD _, err = authclitestutil.TxSignBatchExec(val.ClientCtx, val.Address, outputFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--offline") s.Require().EqualError(err, "required flag(s) \"account-number\", \"sequence\" not set") @@ -230,27 +231,58 @@ func (s *E2ETestSuite) TestCLISignBatch() { // sign-batch file - sequence and account-number are set when offline is false res, err := authclitestutil.TxSignBatchExec(val.ClientCtx, val.Address, outputFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), fmt.Sprintf("--%s=%s", flags.FlagSequence, "1"), fmt.Sprintf("--%s=%s", flags.FlagAccountNumber, "1")) +======= + _, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--offline") + s.Require().EqualError(err, "required flag(s) \"account-number\", \"sequence\" not set") + + // sign-batch file - offline and sequence is set but account-number is not set + _, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), fmt.Sprintf("--%s=%s", flags.FlagSequence, "1"), "--offline") + s.Require().EqualError(err, "required flag(s) \"account-number\" not set") + + // sign-batch file - offline and account-number is set but sequence is not set + _, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), fmt.Sprintf("--%s=%s", flags.FlagAccountNumber, "1"), "--offline") + s.Require().EqualError(err, "required flag(s) \"sequence\" not set") + + // sign-batch file - sequence and account-number are set when offline is false + res, err := authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), fmt.Sprintf("--%s=%s", flags.FlagSequence, "1"), fmt.Sprintf("--%s=%s", flags.FlagAccountNumber, "1")) +>>>>>>> 7bdbc4629 (fix(x/auth): Add fees on batch sign (#18564)) s.Require().NoError(err) s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) // sign-batch file +<<<<<<< HEAD res, err = authclitestutil.TxSignBatchExec(val.ClientCtx, val.Address, outputFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID)) +======= + res, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID)) +>>>>>>> 7bdbc4629 (fix(x/auth): Add fees on batch sign (#18564)) s.Require().NoError(err) s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) // sign-batch file signature only +<<<<<<< HEAD res, err = authclitestutil.TxSignBatchExec(val.ClientCtx, val.Address, outputFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--signature-only") +======= + res, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--signature-only") +>>>>>>> 7bdbc4629 (fix(x/auth): Add fees on batch sign (#18564)) s.Require().NoError(err) s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) // Sign batch malformed tx file. malformedFile := testutil.WriteToNewTempFile(s.T(), fmt.Sprintf("%smalformed", generatedStd)) defer malformedFile.Close() +<<<<<<< HEAD _, err = authclitestutil.TxSignBatchExec(val.ClientCtx, val.Address, malformedFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID)) s.Require().Error(err) // Sign batch malformed tx file signature only. _, err = authclitestutil.TxSignBatchExec(val.ClientCtx, val.Address, malformedFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--signature-only") +======= + _, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), []string{malformedFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID)) + s.Require().Error(err) + + // Sign batch malformed tx file signature only. + _, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), []string{malformedFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--signature-only") +>>>>>>> 7bdbc4629 (fix(x/auth): Add fees on batch sign (#18564)) s.Require().Error(err) // make a txn to increase the sequence of sender @@ -278,7 +310,11 @@ func (s *E2ETestSuite) TestCLISignBatch() { s.Require().Equal(seq+1, seq1) // signing sign-batch should start from the last sequence. +<<<<<<< HEAD signed, err := authclitestutil.TxSignBatchExec(val.ClientCtx, val.Address, outputFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--signature-only") +======= + signed, err := authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--signature-only") +>>>>>>> 7bdbc4629 (fix(x/auth): Add fees on batch sign (#18564)) s.Require().NoError(err) signedTxs := strings.Split(strings.Trim(signed.String(), "\n"), "\n") s.Require().GreaterOrEqual(len(signedTxs), 1) @@ -1118,7 +1154,11 @@ func (s *E2ETestSuite) TestSignBatchMultisig() { addr1, err := account1.GetAddress() s.Require().NoError(err) // sign-batch file +<<<<<<< HEAD res, err := authclitestutil.TxSignBatchExec(val.ClientCtx, addr1, filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", addr.String(), "--signature-only") +======= + res, err := authclitestutil.TxSignBatchExec(clientCtx, addr1, []string{filename.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--multisig", addr.String(), "--signature-only") +>>>>>>> 7bdbc4629 (fix(x/auth): Add fees on batch sign (#18564)) s.Require().NoError(err) s.Require().Equal(1, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) // write sigs to file @@ -1128,7 +1168,11 @@ func (s *E2ETestSuite) TestSignBatchMultisig() { addr2, err := account2.GetAddress() s.Require().NoError(err) // sign-batch file with account2 +<<<<<<< HEAD res, err = authclitestutil.TxSignBatchExec(val.ClientCtx, addr2, filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", addr.String(), "--signature-only") +======= + res, err = authclitestutil.TxSignBatchExec(clientCtx, addr2, []string{filename.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--multisig", addr.String(), "--signature-only") +>>>>>>> 7bdbc4629 (fix(x/auth): Add fees on batch sign (#18564)) s.Require().NoError(err) s.Require().Equal(1, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) // write sigs to file2 @@ -1187,7 +1231,11 @@ func (s *E2ETestSuite) TestMultisignBatch() { // sign-batch file addr1, err := account1.GetAddress() s.Require().NoError(err) +<<<<<<< HEAD res, err := authclitestutil.TxSignBatchExec(val.ClientCtx, addr1, filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", addr.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())), "--signature-only") +======= + res, err := authclitestutil.TxSignBatchExec(clientCtx, addr1, []string{filename.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--multisig", addr.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())), "--signature-only") +>>>>>>> 7bdbc4629 (fix(x/auth): Add fees on batch sign (#18564)) s.Require().NoError(err) s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) // write sigs to file @@ -1197,7 +1245,11 @@ func (s *E2ETestSuite) TestMultisignBatch() { // sign-batch file with account2 addr2, err := account2.GetAddress() s.Require().NoError(err) +<<<<<<< HEAD res, err = authclitestutil.TxSignBatchExec(val.ClientCtx, addr2, filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID), "--multisig", addr.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())), "--signature-only") +======= + res, err = authclitestutil.TxSignBatchExec(clientCtx, addr2, []string{filename.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--multisig", addr.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())), "--signature-only") +>>>>>>> 7bdbc4629 (fix(x/auth): Add fees on batch sign (#18564)) s.Require().NoError(err) s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) diff --git a/tests/integration/auth/client/cli/suite_test.go b/tests/integration/auth/client/cli/suite_test.go index 937ae914ad32..219d499d597e 100644 --- a/tests/integration/auth/client/cli/suite_test.go +++ b/tests/integration/auth/client/cli/suite_test.go @@ -156,18 +156,75 @@ func (s *CLITestSuite) TestCLISignBatch() { s.clientCtx.HomeDir = strings.Replace(s.clientCtx.HomeDir, "simd", "simcli", 1) // sign-batch file - offline is set but account-number and sequence are not - _, err = authtestutil.TxSignBatchExec(s.clientCtx, s.val, outputFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, s.clientCtx.ChainID), "--offline") + _, err = authtestutil.TxSignBatchExec(s.clientCtx, s.val, []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, s.clientCtx.ChainID), "--offline") s.Require().EqualError(err, "required flag(s) \"account-number\", \"sequence\" not set") // sign-batch file - offline and sequence is set but account-number is not set - _, err = authtestutil.TxSignBatchExec(s.clientCtx, s.val, outputFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, s.clientCtx.ChainID), fmt.Sprintf("--%s=%s", flags.FlagSequence, "1"), "--offline") + _, err = authtestutil.TxSignBatchExec(s.clientCtx, s.val, []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, s.clientCtx.ChainID), fmt.Sprintf("--%s=%s", flags.FlagSequence, "1"), "--offline") s.Require().EqualError(err, "required flag(s) \"account-number\" not set") // sign-batch file - offline and account-number is set but sequence is not set - _, err = authtestutil.TxSignBatchExec(s.clientCtx, s.val, outputFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, s.clientCtx.ChainID), fmt.Sprintf("--%s=%s", flags.FlagAccountNumber, "1"), "--offline") + _, err = authtestutil.TxSignBatchExec(s.clientCtx, s.val, []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, s.clientCtx.ChainID), fmt.Sprintf("--%s=%s", flags.FlagAccountNumber, "1"), "--offline") s.Require().EqualError(err, "required flag(s) \"sequence\" not set") } +func (s *CLITestSuite) TestCLISignBatchTotalFees() { + txCfg := s.clientCtx.TxConfig + s.clientCtx.HomeDir = strings.Replace(s.clientCtx.HomeDir, "simd", "simcli", 1) + + testCases := []struct { + name string + args []string + numTransactions int + denom string + }{ + { + "Offline batch-sign one transaction", + []string{"--offline", "--account-number", "1", "--sequence", "1", "--append"}, + 1, + "stake", + }, + { + "Offline batch-sign two transactions", + []string{"--offline", "--account-number", "1", "--sequence", "1", "--append"}, + 2, + "stake", + }, + } + + for _, tc := range testCases { + s.Run(tc.name, func() { + // Create multiple transactions and write them to separate files + sendTokens := sdk.NewCoin(tc.denom, sdk.TokensFromConsensusPower(10, sdk.DefaultPowerReduction)) + expectedBatchedTotalFee := int64(0) + txFiles := make([]string, tc.numTransactions) + for i := 0; i < tc.numTransactions; i++ { + tx, err := s.createBankMsg(s.clientCtx, s.val, + sdk.NewCoins(sendTokens), clitestutil.TestTxConfig{GenOnly: true}) + s.Require().NoError(err) + txFile := testutil.WriteToNewTempFile(s.T(), tx.String()+"\n") + defer txFile.Close() + txFiles[i] = txFile.Name() + + unsignedTx, err := txCfg.TxJSONDecoder()(tx.Bytes()) + s.Require().NoError(err) + txBuilder, err := txCfg.WrapTxBuilder(unsignedTx) + expectedBatchedTotalFee += txBuilder.GetTx().GetFee().AmountOf(tc.denom).Int64() + } + + // Test batch sign + signedTx, err := authtestutil.TxSignBatchExec(s.clientCtx, s.val, txFiles, tc.args...) + s.Require().NoError(err) + signedFinalTx, err := txCfg.TxJSONDecoder()(signedTx.Bytes()) + s.Require().NoError(err) + txBuilder, err := txCfg.WrapTxBuilder(signedFinalTx) + s.Require().NoError(err) + finalTotalFee := txBuilder.GetTx().GetFee() + s.Require().Equal(expectedBatchedTotalFee, finalTotalFee.AmountOf(tc.denom).Int64()) + }) + } +} + func (s *CLITestSuite) TestCLIQueryTxCmdByHash() { sendTokens := sdk.NewInt64Coin("stake", 10) @@ -757,7 +814,7 @@ func (s *CLITestSuite) TestSignBatchMultisig() { addr1, err := account1.GetAddress() s.Require().NoError(err) // sign-batch file - res, err := authtestutil.TxSignBatchExec(s.clientCtx, addr1, filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, s.clientCtx.ChainID), "--multisig", addr.String(), "--signature-only") + res, err := authtestutil.TxSignBatchExec(s.clientCtx, addr1, []string{filename.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, s.clientCtx.ChainID), "--multisig", addr.String(), "--signature-only") s.Require().NoError(err) s.Require().Equal(1, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) // write sigs to file @@ -767,7 +824,7 @@ func (s *CLITestSuite) TestSignBatchMultisig() { addr2, err := account2.GetAddress() s.Require().NoError(err) // sign-batch file with account2 - res, err = authtestutil.TxSignBatchExec(s.clientCtx, addr2, filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, s.clientCtx.ChainID), "--multisig", addr.String(), "--signature-only") + res, err = authtestutil.TxSignBatchExec(s.clientCtx, addr2, []string{filename.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, s.clientCtx.ChainID), "--multisig", addr.String(), "--signature-only") s.Require().NoError(err) s.Require().Equal(1, len(strings.Split(strings.Trim(res.String(), "\n"), "\n"))) // write sigs to file2 diff --git a/x/auth/CHANGELOG.md b/x/auth/CHANGELOG.md new file mode 100644 index 000000000000..fcabad64e900 --- /dev/null +++ b/x/auth/CHANGELOG.md @@ -0,0 +1,36 @@ + + +# Changelog + +## [Unreleased] + +### Features + +### Improvements + +### API Breaking Changes + +### Bug Fixes + +* (client/cli) [#18564](https://github.com/cosmos/cosmos-sdk/pull/18564) Fix total fees calculation when batch signing diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index ec149167bb05..d4ddd09c9035 100644 --- a/x/auth/client/cli/tx_sign.go +++ b/x/auth/client/cli/tx_sign.go @@ -121,7 +121,12 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { appendMessagesToSingleTx, _ := cmd.Flags().GetBool(flagAppend) // Combines all tx msgs and create single signed transaction if appendMessagesToSingleTx { +<<<<<<< HEAD txBuilder := clientCtx.TxConfig.NewTxBuilder() +======= + var totalFees sdk.Coins + txBuilder := txCfg.NewTxBuilder() +>>>>>>> 7bdbc4629 (fix(x/auth): Add fees on batch sign (#18564)) msgs := make([]sdk.Msg, 0) newGasLimit := uint64(0) @@ -133,6 +138,13 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { } // increment the gas newGasLimit += fe.GetTx().GetGas() + // Individual fee values from each transaction need to be + // aggregated to calculate the total fee for the batch of transactions. + // https://github.com/cosmos/cosmos-sdk/issues/18064 + unmergedFees := fe.GetTx().GetFee() + for _, fee := range unmergedFees { + totalFees = totalFees.Add(fee) + } // append messages msgs = append(msgs, unsignedStdTx.GetMsgs()...) } @@ -141,13 +153,15 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error { // set the memo,fees,feeGranter,feePayer from cmd flags txBuilder.SetMemo(txFactory.Memo()) - txBuilder.SetFeeAmount(txFactory.Fees()) txBuilder.SetFeeGranter(clientCtx.FeeGranter) txBuilder.SetFeePayer(clientCtx.FeePayer) // set the gasLimit txBuilder.SetGasLimit(newGasLimit) + // set the feeAmount + txBuilder.SetFeeAmount(totalFees) + // sign the txs if ms == "" { from, _ := cmd.Flags().GetString(flags.FlagFrom) diff --git a/x/auth/client/testutil/helpers.go b/x/auth/client/testutil/helpers.go index 092b061a83c2..a6c4e4d1b9a3 100644 --- a/x/auth/client/testutil/helpers.go +++ b/x/auth/client/testutil/helpers.go @@ -62,12 +62,8 @@ func TxMultiSignExec(clientCtx client.Context, from, filename string, extraArgs return clitestutil.ExecTestCLICmd(clientCtx, cli.GetMultiSignCommand(), append(args, extraArgs...)) } -func TxSignBatchExec(clientCtx client.Context, from fmt.Stringer, filename string, extraArgs ...string) (testutil.BufferWriter, error) { - args := []string{ - fmt.Sprintf("--from=%s", from.String()), - filename, - } - +func TxSignBatchExec(clientCtx client.Context, from fmt.Stringer, filenames []string, extraArgs ...string) (testutil.BufferWriter, error) { + args := append([]string{fmt.Sprintf("--from=%s", from.String())}, filenames...) return clitestutil.ExecTestCLICmd(clientCtx, cli.GetSignBatchCommand(), append(args, extraArgs...)) }