From 3c8ddabcaaf5c65d8fb8d9a6fef1672a316fdb23 Mon Sep 17 00:00:00 2001 From: Ezequiel Raynaudo Date: Thu, 30 Nov 2023 14:23:15 -0300 Subject: [PATCH] refactor(x/auth): Revert signature of TxSignBatchExec (#18602) --- tests/e2e/auth/suite.go | 26 +++++++++---------- .../integration/auth/client/cli/suite_test.go | 14 +++++----- x/auth/client/testutil/helpers.go | 8 ++++-- 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/tests/e2e/auth/suite.go b/tests/e2e/auth/suite.go index c17d47ba12eb..cee85a1be372 100644 --- a/tests/e2e/auth/suite.go +++ b/tests/e2e/auth/suite.go @@ -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(), []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--offline") + _, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), 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") + _, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), 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") + _, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), 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")) + 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")) 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(), []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID)) + res, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), 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(), []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--signature-only") + res, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), 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(), []string{malformedFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID)) + _, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), 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") + _, err = authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), 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(), []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--signature-only") + signed, err := authclitestutil.TxSignBatchExec(clientCtx, val.GetAddress(), 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, []string{filename.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--multisig", addr.String(), "--signature-only") + res, err := authclitestutil.TxSignBatchExec(clientCtx, addr1, 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, []string{filename.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, clientCtx.ChainID), "--multisig", addr.String(), "--signature-only") + res, err = authclitestutil.TxSignBatchExec(clientCtx, addr2, 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, []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") + 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") 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, []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") + 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") 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 d74cf198b639..350cd833327d 100644 --- a/tests/integration/auth/client/cli/suite_test.go +++ b/tests/integration/auth/client/cli/suite_test.go @@ -159,15 +159,15 @@ 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, []string{outputFile.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, s.clientCtx.ChainID), "--offline") + _, err = authtestutil.TxSignBatchExec(s.clientCtx, s.val, 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, []string{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, 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, []string{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, 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") } @@ -212,11 +212,13 @@ func (s *CLITestSuite) TestCLISignBatchTotalFees() { 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 - signedTx, err := authtestutil.TxSignBatchExec(s.clientCtx, s.val, txFiles, tc.args...) + 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) @@ -796,7 +798,7 @@ func (s *CLITestSuite) TestSignBatchMultisig() { addr1, err := account1.GetAddress() s.Require().NoError(err) // sign-batch file - res, err := authtestutil.TxSignBatchExec(s.clientCtx, addr1, []string{filename.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, s.clientCtx.ChainID), "--multisig", addr.String(), "--signature-only") + res, err := authtestutil.TxSignBatchExec(s.clientCtx, addr1, 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 @@ -806,7 +808,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, []string{filename.Name()}, fmt.Sprintf("--%s=%s", flags.FlagChainID, s.clientCtx.ChainID), "--multisig", addr.String(), "--signature-only") + res, err = authtestutil.TxSignBatchExec(s.clientCtx, addr2, 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/client/testutil/helpers.go b/x/auth/client/testutil/helpers.go index 9dabe4c242d0..69d0b6ff380f 100644 --- a/x/auth/client/testutil/helpers.go +++ b/x/auth/client/testutil/helpers.go @@ -63,8 +63,12 @@ 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, filenames []string, extraArgs ...string) (testutil.BufferWriter, error) { - args := append([]string{fmt.Sprintf("--from=%s", from.String())}, filenames...) +func TxSignBatchExec(clientCtx client.Context, from fmt.Stringer, filename string, extraArgs ...string) (testutil.BufferWriter, error) { + args := []string{ + fmt.Sprintf("--from=%s", from.String()), + filename, + } + return clitestutil.ExecTestCLICmd(clientCtx, cli.GetSignBatchCommand(), append(args, extraArgs...)) }