Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test with Multi Signers into Robert's TxSign PR #8142

Merged
merged 7 commits into from
Dec 11, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions x/auth/client/cli/broadcast.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ $ <appd> tx broadcast ./mytxn.json
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx := client.GetClientContextFromCmd(cmd)
clientCtx, err := client.ReadTxCommandFlags(clientCtx, cmd.Flags())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forgetting the second line is error prone. We should have a single function for doing this. (idea for a new task)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


if offline, _ := cmd.Flags().GetBool(flags.FlagOffline); offline {
return errors.New("cannot broadcast tx during offline mode")
Expand Down
57 changes: 56 additions & 1 deletion x/auth/client/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (s *IntegrationTestSuite) SetupSuite() {
s.T().Log("setting up integration test suite")

cfg := network.DefaultConfig()
cfg.NumValidators = 1
cfg.NumValidators = 2

s.cfg = cfg
s.network = network.New(s.T(), cfg)
Expand Down Expand Up @@ -1024,6 +1024,61 @@ func (s *IntegrationTestSuite) createBankMsg(val *network.Validator, toAddr sdk.
return res
}

func (s *IntegrationTestSuite) TestSignWithMultiSigners() {
val0, val1 := s.network.Validators[0], s.network.Validators[1]
val0Coin := sdk.NewCoin(fmt.Sprintf("%stoken", val0.Moniker), sdk.NewInt(10))
val1Coin := sdk.NewCoin(fmt.Sprintf("%stoken", val1.Moniker), sdk.NewInt(10))
_, _, addr1 := testdata.KeyTestPubAddr()

// Creating a tx with 2 msgs from 2 signers: val0 and val1.
txBuilder := val0.ClientCtx.TxConfig.NewTxBuilder()
txBuilder.SetMsgs(
banktypes.NewMsgSend(val0.Address, addr1, sdk.NewCoins(val0Coin)),
banktypes.NewMsgSend(val1.Address, addr1, sdk.NewCoins(val1Coin)),
)
txBuilder.SetFeeAmount(sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))))
txBuilder.SetGasLimit(testdata.NewTestGasLimit())
// Set signer_infos for both signers. Note: we use the empty signature hack.
// TODO
s.Require().Equal([]sdk.AccAddress{val0.Address, val1.Address}, txBuilder.GetTx().GetSigners())

// Write the unsigned tx into a file.
txJSON, err := val0.ClientCtx.TxConfig.TxJSONEncoder()(txBuilder.GetTx())
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
s.Require().NoError(err)
unsignedTxFile := testutil.WriteToNewTempFile(s.T(), string(txJSON))

// Let val0 sign first the file with the unsignedTx.
signedByVal0, err := authtest.TxSignExec(val0.ClientCtx, val0.Address, unsignedTxFile.Name(), "--overwrite")
s.Require().NoError(err)
signedByVal0File := testutil.WriteToNewTempFile(s.T(), signedByVal0.String())
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved

// Then let val1 sign the file with signedByVal0.
val1AccNum, val1Seq, err := val0.ClientCtx.AccountRetriever.GetAccountNumberSequence(val0.ClientCtx, val1.Address)
s.Require().NoError(err)
signedTx, err := authtest.TxSignExec(
val1.ClientCtx, val1.Address, signedByVal0File.Name(),
"--offline", fmt.Sprintf("--account-number=%d", val1AccNum), fmt.Sprintf("--sequence=%d", val1Seq))
s.Require().NoError(err)
signedTxFile := testutil.WriteToNewTempFile(s.T(), signedTx.String())
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The most anticipated test is:

  1. User1 signs and sends the signature to Relayer (that's why we have --sign-only)
  2. User2 signs and sends to the signature to Relayer
  3. Relayer aggregates the signatures in the transaction and broadcasts it.

Note: here the Relayer is a test.


// Now let's try to send this tx.
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
res, err := authtest.TxBroadcastExec(val0.ClientCtx, signedTxFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock))
s.Require().NoError(err)
var txRes sdk.TxResponse
s.Require().NoError(val0.ClientCtx.JSONMarshaler.UnmarshalJSON(res.Bytes(), &txRes))
s.Require().Equal(uint32(0), txRes.Code)
Copy link
Contributor Author

@amaury1093 amaury1093 Dec 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tx fails, because sig verification for val0's signature is wrong.


// Make sure the addr1's balance got funded.
queryResJSON, err := bankcli.QueryBalancesExec(val0.ClientCtx, addr1)
s.Require().NoError(err)
var queryRes banktypes.QueryAllBalancesResponse
err = val0.ClientCtx.JSONMarshaler.UnmarshalJSON(queryResJSON.Bytes(), &queryRes)
s.Require().NoError(err)
s.Require().Equal(sdk.NewCoins(val0Coin, val1Coin), queryRes.Balances)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not declaring require := s.Require() at the top? it will make the code more readable IMHO:

s.Require().NoError
require.NoError


s.Require().False(true)
}

func TestIntegrationTestSuite(t *testing.T) {
suite.Run(t, new(IntegrationTestSuite))
}