From 82831656002c46df364382015817aa997a9a995d Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Thu, 30 Jul 2020 15:22:31 +0200 Subject: [PATCH] Revert `SetSignerInfo` (#6894) * Fix TestCLIMultisign tests * Fix lint Co-authored-by: SaReN --- client/tx/tx.go | 58 +++++++++---------- client/tx_generator.go | 4 -- x/auth/ante/testutil_test.go | 37 +++++++------ x/auth/tx/builder_test.go | 104 ----------------------------------- 4 files changed, 47 insertions(+), 156 deletions(-) diff --git a/client/tx/tx.go b/client/tx/tx.go index 30f8269f8..6047c45c0 100644 --- a/client/tx/tx.go +++ b/client/tx/tx.go @@ -18,7 +18,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/rest" - txtypes "github.com/cosmos/cosmos-sdk/types/tx" "github.com/cosmos/cosmos-sdk/types/tx/signing" authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" ) @@ -309,41 +308,16 @@ func PrepareFactory(clientCtx client.Context, txf Factory) (Factory, error) { return txf, nil } -// Helper function to retrieve sign bytes. -func getSignBytes( - signMode signing.SignMode, signerData authsigning.SignerData, - txBuilder client.TxBuilder, pubKey crypto.PubKey, txConfig client.TxConfig, -) ([]byte, error) { - // Set signer info, we only support single signature in this function. - err := txBuilder.SetSignerInfo(pubKey, &txtypes.ModeInfo{ - Sum: &txtypes.ModeInfo_Single_{ - Single: &txtypes.ModeInfo_Single{Mode: signMode}, - }, - }) - if err != nil { - return nil, err - } - - // generate the bytes to be signed - signBytes, err := txConfig.SignModeHandler().GetSignBytes(signMode, signerData, txBuilder.GetTx()) - if err != nil { - return nil, err - } - - return signBytes, nil -} - // SignWithPrivKey signs a given tx with the given private key, and returns the // corresponding SignatureV2 if the signing is successful. func SignWithPrivKey( signMode signing.SignMode, signerData authsigning.SignerData, txBuilder client.TxBuilder, priv crypto.PrivKey, txConfig client.TxConfig, ) (signing.SignatureV2, error) { - var sigV2 signing.SignatureV2 - // Generate the bytes to be signed - signBytes, err := getSignBytes(signMode, signerData, txBuilder, priv.PubKey(), txConfig) + // Generate the bytes to be signed. + signBytes, err := txConfig.SignModeHandler().GetSignBytes(signMode, signerData, txBuilder.GetTx()) if err != nil { return sigV2, err } @@ -394,8 +368,28 @@ func Sign(txf Factory, name string, txBuilder client.TxBuilder) error { AccountSequence: txf.sequence, } - // Generate the bytes to be signed - signBytes, err := getSignBytes(signMode, signerData, txBuilder, pubKey, txf.txConfig) + // For SIGN_MODE_DIRECT, calling SetSignatures calls SetSignerInfos on + // TxBuilder under the hood, and SignerInfos is needed to generated the + // sign bytes. This is the reason for setting SetSignatures here, with a + // nil signature. + // + // Note: this line is not needed for SIGN_MODE_LEGACY_AMINO, but putting it + // also doesn't affect its generated sign bytes, so for code's simplicity + // sake, we put it here. + sigData := signing.SingleSignatureData{ + SignMode: signMode, + Signature: nil, + } + sig := signing.SignatureV2{ + PubKey: pubKey, + Data: &sigData, + } + if err := txBuilder.SetSignatures(sig); err != nil { + return err + } + + // Generate the bytes to be signed. + signBytes, err := txf.txConfig.SignModeHandler().GetSignBytes(signMode, signerData, txBuilder.GetTx()) if err != nil { return err } @@ -407,11 +401,11 @@ func Sign(txf Factory, name string, txBuilder client.TxBuilder) error { } // Construct the SignatureV2 struct - sigData := signing.SingleSignatureData{ + sigData = signing.SingleSignatureData{ SignMode: signMode, Signature: sigBytes, } - sig := signing.SignatureV2{ + sig = signing.SignatureV2{ PubKey: pubKey, Data: &sigData, } diff --git a/client/tx_generator.go b/client/tx_generator.go index de1057956..b5cf81197 100644 --- a/client/tx_generator.go +++ b/client/tx_generator.go @@ -1,10 +1,7 @@ package client import ( - "github.com/tendermint/tendermint/crypto" - sdk "github.com/cosmos/cosmos-sdk/types" - txtypes "github.com/cosmos/cosmos-sdk/types/tx" signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" "github.com/cosmos/cosmos-sdk/x/auth/signing" ) @@ -40,7 +37,6 @@ type ( GetTx() signing.SigFeeMemoTx SetMsgs(msgs ...sdk.Msg) error - SetSignerInfo(pubKey crypto.PubKey, modeInfo *txtypes.ModeInfo) error SetSignatures(signatures ...signingtypes.SignatureV2) error SetMemo(memo string) SetFeeAmount(amount sdk.Coins) diff --git a/x/auth/ante/testutil_test.go b/x/auth/ante/testutil_test.go index 0725f8ce8..f81f7cb54 100644 --- a/x/auth/ante/testutil_test.go +++ b/x/auth/ante/testutil_test.go @@ -14,7 +14,6 @@ import ( "github.com/cosmos/cosmos-sdk/simapp" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" - txtypes "github.com/cosmos/cosmos-sdk/types/tx" "github.com/cosmos/cosmos-sdk/types/tx/signing" "github.com/cosmos/cosmos-sdk/x/auth/ante" xauthsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" @@ -87,21 +86,27 @@ func (suite *AnteTestSuite) CreateTestAccounts(numAccs int) []TestAccount { // CreateTestTx is a helper function to create a tx given multiple inputs. func (suite *AnteTestSuite) CreateTestTx(privs []crypto.PrivKey, accNums []uint64, accSeqs []uint64, chainID string) (xauthsigning.SigFeeMemoTx, error) { - // First round: we gather all the signer infos. - for _, priv := range privs { - err := suite.txBuilder.SetSignerInfo(priv.PubKey(), &txtypes.ModeInfo{ - Sum: &txtypes.ModeInfo_Single_{ - Single: &txtypes.ModeInfo_Single{ - Mode: suite.clientCtx.TxConfig.SignModeHandler().DefaultMode(), - }, - }, - }) - if err != nil { - return nil, err - } - } - // Second round: all signer infos are set, so each signer can sign. + // First round: we gather all the signer infos. We use the "set empty + // signature" hack to do that. var sigsV2 []signing.SignatureV2 + for _, priv := range privs { + sigV2 := signing.SignatureV2{ + PubKey: priv.PubKey(), + Data: &signing.SingleSignatureData{ + SignMode: suite.clientCtx.TxConfig.SignModeHandler().DefaultMode(), + Signature: nil, + }, + } + + sigsV2 = append(sigsV2, sigV2) + } + err := suite.txBuilder.SetSignatures(sigsV2...) + if err != nil { + return nil, err + } + + // Second round: all signer infos are set, so each signer can sign. + sigsV2 = []signing.SignatureV2{} for i, priv := range privs { signerData := xauthsigning.SignerData{ ChainID: chainID, @@ -115,7 +120,7 @@ func (suite *AnteTestSuite) CreateTestTx(privs []crypto.PrivKey, accNums []uint6 sigsV2 = append(sigsV2, sigV2) } - err := suite.txBuilder.SetSignatures(sigsV2...) + err = suite.txBuilder.SetSignatures(sigsV2...) if err != nil { return nil, err } diff --git a/x/auth/tx/builder_test.go b/x/auth/tx/builder_test.go index adcdbefab..8a1db1c98 100644 --- a/x/auth/tx/builder_test.go +++ b/x/auth/tx/builder_test.go @@ -1,12 +1,9 @@ package tx import ( - "errors" - "fmt" "testing" "github.com/stretchr/testify/require" - "github.com/tendermint/tendermint/crypto" "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" @@ -233,104 +230,3 @@ func TestBuilderValidateBasic(t *testing.T) { err = txBuilder.ValidateBasic() require.Error(t, err) } - -func TestBuilderSetSignerInfo(t *testing.T) { - // keys and addresses - _, pubKey1, addr1 := testdata.KeyTestPubAddr() - _, pubKey2, addr2 := testdata.KeyTestPubAddr() - - // msg and signatures - msg1 := testdata.NewTestMsg(addr1) - msg2 := testdata.NewTestMsg(addr2) - - // Variable data for each test - var ( - pubKey crypto.PubKey - modeInfo txtypes.ModeInfo - txBuilder *builder - ) - - testCases := []struct { - desc string - malleate func() - expPass bool - expErr error - }{ - { - "should fail if no msgs", - func() { - pubKey = pubKey1 - }, - false, sdkerrors.ErrInvalidPubKey, - }, - { - "should fail if no public key", - func() { - pubKey = nil - txBuilder.SetMsgs(msg1) - }, - false, sdkerrors.ErrInvalidPubKey, - }, - { - "should fail if signer not in msgs", - func() { - _, pubKey, _ = testdata.KeyTestPubAddr() - txBuilder.SetMsgs(msg1, msg2) - }, - false, sdkerrors.ErrInvalidPubKey, - }, - { - "should pass with 2 signers", - func() { - // Run manually for signer 1. - txBuilder.SetMsgs(msg1, msg2) - modeInfo = txtypes.ModeInfo{Sum: &txtypes.ModeInfo_Single_{Single: &txtypes.ModeInfo_Single{Mode: signing.SignMode_SIGN_MODE_DIRECT}}} - txBuilder.SetSignerInfo(pubKey1, &modeInfo) - - // Test case will handle signer 2. - pubKey = pubKey2 - }, - true, nil, - }, - { - "should reset cached authInfoBz and pubKeys bytes after each call", - func() { - // Run manually for signer 1. - txBuilder.SetMsgs(msg1, msg2) - modeInfo = txtypes.ModeInfo{Sum: &txtypes.ModeInfo_Single_{Single: &txtypes.ModeInfo_Single{Mode: signing.SignMode_SIGN_MODE_DIRECT}}} - txBuilder.SetSignerInfo(pubKey1, &modeInfo) - - // This populates the cached bytes. - _, _ = txBuilder.GetAuthInfoBytes(), txBuilder.GetPubKeys() - require.NotNil(t, txBuilder.authInfoBz) - require.NotNil(t, txBuilder.pubKeys) - - // Run again, for signer 2. It should reset the cached bytes - // to nil. - txBuilder.SetSignerInfo(pubKey2, &modeInfo) - require.Nil(t, txBuilder.authInfoBz) - require.Nil(t, txBuilder.pubKeys) - - // Run the test case, for fun. - pubKey = pubKey2 - }, - true, nil, - }, - } - - for _, tc := range testCases { - // Fresh txBuilder for each test case - txBuilder = newBuilder(std.DefaultPublicKeyCodec{}) - - tc.malleate() - - fmt.Println(modeInfo) - err := txBuilder.SetSignerInfo(pubKey, &modeInfo) - if tc.expPass { - require.Nil(t, err, tc.desc) - } else { - require.Error(t, err, tc.desc) - require.True(t, errors.Is(tc.expErr, err), tc.desc) - } - } -}