LaconicNetwork/cosmos-sdk
7
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

fix(x/auth): Add fees on batch sign (#18564)

Browse Source
This commit is contained in:
Ezequiel Raynaudo 2023-11-29 11:38:08 -03:00 committed by GitHub
parent cc85702956
commit 7bdbc4629f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 90 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
tests/e2e/auth/suite.go
View File

@ -226,40 +226,40 @@ func (s *E2ETestSuite) TestCLISignBatch() {
clientCtx.HomeDir = strings.Replace(clientCtx.HomeDir, "simd", "simcli", 1)
// sign-batch file - offline is set but account-number and sequence are not
_, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), outputFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--offline")
_, 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(), outputFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), fmt.Sprintf("--%s=%s", flags.FlagSequence, "1"), "--offline")
_, 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(), outputFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), fmt.Sprintf("--%s=%s", flags.FlagAccountNumber, "1"), "--offline")
_, 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(), outputFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), fmt.Sprintf("--%s=%s", flags.FlagSequence, "1"), fmt.Sprintf("--%s=%s", flags.FlagAccountNumber, "1"))
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"))
s.Require().NoError(err)
s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n")))
// sign-batch file
res, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), outputFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID))
res, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID))
s.Require().NoError(err)
s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n")))
// sign-batch file signature only
res, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), outputFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--signature-only")
res, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--signature-only")
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("malformed%s", generatedStd))
defer malformedFile.Close()
_, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), malformedFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID))
_, 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(), malformedFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--signature-only")
_, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), []string{malformedFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--signature-only")
s.Require().Error(err)
// make a txn to increase the sequence of sender
@ -288,7 +288,7 @@ func (s *E2ETestSuite) TestCLISignBatch() {
s.Require().Equal(seq+1, seq1)
// signing sign-batch should start from the last sequence.
signed, err := authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), outputFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--signature-only")
signed, err := authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--signature-only")
s.Require().NoError(err)
signedTxs := strings.Split(strings.Trim(signed.String(), "\n"), "\n")
s.Require().GreaterOrEqual(len(signedTxs), 1)
@ -1171,7 +1171,7 @@ func (s *E2ETestSuite) TestSignBatchMultisig() {
addr1, err := account1.GetAddress()
s.Require().NoError(err)
// sign-batch file
res, err := authclitestutil.TxSignBatchExec(clientCtx, addr1, filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, 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")
s.Require().NoError(err)
s.Require().Equal(1, len(strings.Split(strings.Trim(res.String(), "\n"), "\n")))
// write sigs to file
@ -1181,7 +1181,7 @@ func (s *E2ETestSuite) TestSignBatchMultisig() {
addr2, err := account2.GetAddress()
s.Require().NoError(err)
// sign-batch file with account2
res, err = authclitestutil.TxSignBatchExec(clientCtx, addr2, filename.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, 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")
s.Require().NoError(err)
s.Require().Equal(1, len(strings.Split(strings.Trim(res.String(), "\n"), "\n")))
// write sigs to file2
@ -1244,7 +1244,7 @@ func (s *E2ETestSuite) TestMultisignBatch() {
// sign-batch file
addr1, err := account1.GetAddress()
s.Require().NoError(err)
res, err := authclitestutil.TxSignBatchExec(clientCtx, addr1, 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")
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")
s.Require().NoError(err)
s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n")))
// write sigs to file
@ -1254,7 +1254,7 @@ func (s *E2ETestSuite) TestMultisignBatch() {
// sign-batch file with account2
addr2, err := account2.GetAddress()
s.Require().NoError(err)
res, err = authclitestutil.TxSignBatchExec(clientCtx, addr2, 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")
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")
s.Require().NoError(err)
s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n")))

67
tests/integration/auth/client/cli/suite_test.go
View File

@ -159,18 +159,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)
@ -739,7 +796,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
@ -749,7 +806,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

2
x/auth/CHANGELOG.md
View File

@ -32,3 +32,5 @@ Ref: https://keepachangelog.com/en/1.0.0/
### API Breaking Changes
### Bug Fixes
* (client/cli) [#18564](https://github.com/cosmos/cosmos-sdk/pull/18564) Fix total fees calculation when batch signing

12
x/auth/client/cli/tx_sign.go
View File

@ -117,6 +117,7 @@ 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 {
var totalFees sdk.Coins
txBuilder := txCfg.NewTxBuilder()
msgs := make([]sdk.Msg, 0)
newGasLimit := uint64(0)
@ -129,6 +130,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()...)
}
@ -140,13 +148,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
from, _ := cmd.Flags().GetString(flags.FlagFrom)
err := sigTxOrMultisig(clientCtx, txBuilder, txFactory, from, multisigKey)

8
x/auth/client/testutil/helpers.go
View File

@ -63,12 +63,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...))
}