Skip to content

Commit

Permalink
refactor(x/auth): Revert signature of TxSignBatchExec (#18602)
Browse files Browse the repository at this point in the history
  • Loading branch information
@raynaudoe
raynaudoe authored Nov 30, 2023
1 parent d52bd92 commit 3c8ddab
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 21 deletions.
26 changes: 13 additions & 13 deletions tests/e2e/auth/suite.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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")))

Expand Down
14 changes: 8 additions & 6 deletions tests/integration/auth/client/cli/suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
8 changes: 6 additions & 2 deletions x/auth/client/testutil/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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...))
}

Expand Down

0 comments on commit 3c8ddab

Please sign in to comment.