Skip to content

Commit

Permalink
fix(x/auth): Add fees on batch sign (backport #18564) (#18592)
Browse files Browse the repository at this point in the history
Co-authored-by: Ezequiel Raynaudo <raynaudo.ee@gmail.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>
  • Loading branch information
3 people authored Nov 30, 2023
1 parent 3611662 commit 5051495
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,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.
Expand Down
59 changes: 59 additions & 0 deletions tests/integration/auth/client/cli/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
14 changes: 12 additions & 2 deletions x/auth/client/cli/tx_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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()...)
}
Expand All @@ -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)
Expand Down

0 comments on commit 5051495

Please sign in to comment.