diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e60f1535cf..f193ec6d65e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* (x/auth) [#18564](https://github.com/cosmos/cosmos-sdk/pull/18564) Fix total fees calculation when batch signing. * (server) [#18537](https://github.com/cosmos/cosmos-sdk/pull/18537) Fix panic when defining minimum gas config as `100stake;100uatom`. Use a `,` delimiter instead of `;`. Fixes the server config getter to use the correct delimiter. * [#18531](https://github.com/cosmos/cosmos-sdk/pull/18531) Baseapp's `GetConsensusParams` returns an empty struct instead of panicking if no params are found. * (client/tx) [#18472](https://github.com/cosmos/cosmos-sdk/pull/18472) Utilizes the correct Pubkey when simulating a transaction. diff --git a/tests/integration/auth/client/cli/suite_test.go b/tests/integration/auth/client/cli/suite_test.go index 937ae914ad3..290568b705f 100644 --- a/tests/integration/auth/client/cli/suite_test.go +++ b/tests/integration/auth/client/cli/suite_test.go @@ -168,6 +168,65 @@ func (s *CLITestSuite) TestCLISignBatch() { 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), fmt.Sprintf("--%s=true", flags.FlagGenerateOnly)) + s.Require().NoError(err) + txFile := testutil.WriteToNewTempFile(s.T(), tx.String()) + defer txFile.Close() + txFiles[i] = txFile.Name() + + unsignedTx, err := txCfg.TxJSONDecoder()(tx.Bytes()) + s.Require().NoError(err) + txBuilder, err := txCfg.WrapTxBuilder(unsignedTx) + s.Require().NoError(err) + expectedBatchedTotalFee += txBuilder.GetTx().GetFee().AmountOf(tc.denom).Int64() + } + + // Test batch sign + batchSignArgs := append([]string{fmt.Sprintf("--from=%s", s.val.String())}, append(txFiles, tc.args...)...) + signedTx, err := clitestutil.ExecTestCLICmd(s.clientCtx, authcli.GetSignBatchCommand(), batchSignArgs) + 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) diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index ec149167bb0..fc993c9393b 100644 --- a/x/auth/client/cli/tx_sign.go +++ b/x/auth/client/cli/tx_sign.go @@ -121,7 +121,8 @@ 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 { - txBuilder := clientCtx.TxConfig.NewTxBuilder() + var totalFees sdk.Coins + txBuilder := txCfg.NewTxBuilder() msgs := make([]sdk.Msg, 0) newGasLimit := uint64(0) @@ -133,6 +134,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 +149,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)