From 9592f34cde4c9ae96a6799f36540cd2cbaa4d6b5 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Wed, 29 Jul 2020 11:52:22 -0400 Subject: [PATCH] Fully remove StdTx usage in ante handler tests (#6845) * WIP on removing StdTx from ante handler tests * Fix recheck * Fix basic_test * Don't use StdTx in ante_test * Use TxConfig sign mode handler in anteHandler * Fix TestAnteHandlerAccountNumbers * Amino don't panic * Fix more tests * All proto ante tests pass * Fix last tests * Amino register concrete * Add tests Co-authored-by: Amaury Martiny --- client/tx/tx.go | 27 ++-- client/tx_generator.go | 4 + x/auth/ante/ante_test.go | 111 ++++++++------ x/auth/ante/basic_test.go | 52 +++---- x/auth/ante/fee_test.go | 22 ++- x/auth/ante/integration_test.go | 18 --- x/auth/ante/setup_test.go | 18 +-- x/auth/ante/sigverify_test.go | 33 ++--- x/auth/ante/testutil_test.go | 81 +++++++++-- x/auth/signing/verify.go | 6 +- x/auth/tx/builder.go | 76 ++++++++-- x/auth/tx/builder_test.go | 247 ++++++++++++++++++++++---------- x/auth/types/client_tx.go | 11 +- 13 files changed, 451 insertions(+), 255 deletions(-) delete mode 100644 x/auth/ante/integration_test.go diff --git a/client/tx/tx.go b/client/tx/tx.go index 27c11a5f5..caff6e374 100644 --- a/client/tx/tx.go +++ b/client/tx/tx.go @@ -18,6 +18,7 @@ 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" ) @@ -308,25 +309,13 @@ func getSignBytes( signMode signing.SignMode, signerData authsigning.SignerData, txBuilder client.TxBuilder, pubKey crypto.PubKey, txConfig client.TxConfig, ) ([]byte, error) { - - sigData := signing.SingleSignatureData{ - SignMode: signMode, - Signature: nil, - } - sig := signing.SignatureV2{ - PubKey: pubKey, - Data: &sigData, - } - - // 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. - if err := txBuilder.SetSignatures(sig); err != nil { + // 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 } diff --git a/client/tx_generator.go b/client/tx_generator.go index 2452b45dd..37d95db31 100644 --- a/client/tx_generator.go +++ b/client/tx_generator.go @@ -1,7 +1,10 @@ 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" ) @@ -35,6 +38,7 @@ 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/ante_test.go b/x/auth/ante/ante_test.go index a6f565f52..d3165d526 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/stretchr/testify/require" - "github.com/stretchr/testify/suite" "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/ed25519" "github.com/tendermint/tendermint/crypto/secp256k1" @@ -104,8 +103,9 @@ func (suite *AnteTestSuite) TestAnteHandlerSigErrors() { privs, accNums, accSeqs = []crypto.PrivKey{}, []uint64{}, []uint64{} // Create tx manually to test the tx's signers - suite.txBuilder.SetMsgs(msgs...) - tx := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(suite.txBuilder.SetMsgs(msgs...)) + tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(err) // tx.GetSigners returns addresses in correct order: addr1, addr2, addr3 expectedSigners := []sdk.AccAddress{addr0, addr1, addr2} suite.Require().Equal(expectedSigners, tx.GetSigners()) @@ -810,51 +810,71 @@ func (suite *AnteTestSuite) TestAnteHandlerSetPubKey() { { "make sure public key has been set (tx itself should fail because of replay protection)", func() { + // Make sure public key has been set from previous test. acc0 := suite.app.AccountKeeper.GetAccount(suite.ctx, accounts[0].acc.GetAddress()) suite.Require().Equal(acc0.GetPubKey(), accounts[0].priv.PubKey()) }, false, false, + sdkerrors.ErrUnauthorized, // because of wrong accSeq + }, + { + "test public key not found", + func() { + // See above, `privs` still holds the private key of accounts[0]. + msgs = []sdk.Msg{testdata.NewTestMsg(accounts[1].acc.GetAddress())} + }, + false, + false, + sdkerrors.ErrInvalidPubKey, + }, + { + "make sure public key is not set, when tx has no pubkey or signature", + func() { + // Make sure public key has not been set from previous test. + acc1 := suite.app.AccountKeeper.GetAccount(suite.ctx, accounts[1].acc.GetAddress()) + suite.Require().Nil(acc1.GetPubKey()) + + privs, accNums, accSeqs = []crypto.PrivKey{accounts[1].priv}, []uint64{1}, []uint64{0} + msgs = []sdk.Msg{testdata.NewTestMsg(accounts[1].acc.GetAddress())} + suite.txBuilder.SetMsgs(msgs...) + suite.txBuilder.SetFeeAmount(feeAmount) + suite.txBuilder.SetGasLimit(gasLimit) + + // Manually create tx, and remove signature. + tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(err) + txBuilder, err := suite.clientCtx.TxConfig.WrapTxBuilder(tx) + suite.Require().NoError(err) + suite.Require().NoError(txBuilder.SetSignatures()) + + // Run anteHandler manually, expect ErrNoSignatures. + _, err = suite.anteHandler(suite.ctx, txBuilder.GetTx(), false) + suite.Require().Error(err) + suite.Require().True(errors.Is(err, sdkerrors.ErrNoSignatures)) + + // Make sure public key has not been set. + acc1 = suite.app.AccountKeeper.GetAccount(suite.ctx, accounts[1].acc.GetAddress()) + suite.Require().Nil(acc1.GetPubKey()) + + // Set incorrect accSeq, to generate incorrect signature. + privs, accNums, accSeqs = []crypto.PrivKey{accounts[1].priv}, []uint64{1}, []uint64{1} + }, + false, + false, sdkerrors.ErrUnauthorized, }, { - "test public key not found", - func() { - accNums = []uint64{1} - msgs = []sdk.Msg{testdata.NewTestMsg(accounts[1].acc.GetAddress())} - - // Manually create tx, and remove signature - tx := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) - sigs := tx.(types.StdTx).Signatures - sigs[0].PubKey = nil - _, err := suite.anteHandler(suite.ctx, tx, false) - suite.Require().Error(err) - suite.Require().Equal(err.Error(), "wrong number of signers; expected 0, got 1: unauthorized") - suite.Require().True(errors.Is(err, sdkerrors.ErrUnauthorized)) - }, - false, - false, - sdkerrors.ErrInvalidPubKey, - }, - { - "make sure previous public key has NOT been set, test invalid signature and public key", + "make sure previous public key has been set after wrong signature", func() { + // Make sure public key has been set, as SetPubKeyDecorator + // is called before all signature verification decorators. acc1 := suite.app.AccountKeeper.GetAccount(suite.ctx, accounts[1].acc.GetAddress()) - suite.Require().Nil(acc1.GetPubKey()) + suite.Require().Equal(acc1.GetPubKey(), accounts[1].priv.PubKey()) }, false, false, - sdkerrors.ErrInvalidPubKey, - }, - { - "make sure previous public key has NOT been set", - func() { - acc1 := suite.app.AccountKeeper.GetAccount(suite.ctx, accounts[1].acc.GetAddress()) - suite.Require().Nil(acc1.GetPubKey()) - }, - false, - false, - sdkerrors.ErrInvalidPubKey, + sdkerrors.ErrUnauthorized, }, } @@ -988,7 +1008,7 @@ func (suite *AnteTestSuite) TestCustomSignatureVerificationGasConsumer() { default: return sdkerrors.Wrapf(sdkerrors.ErrInvalidPubKey, "unrecognized public key type: %T", pubkey) } - }, types.LegacyAminoJSONHandler{}) + }, suite.clientCtx.TxConfig.SignModeHandler()) // Same data for every test cases accounts := suite.CreateTestAccounts(1) @@ -1061,23 +1081,26 @@ func (suite *AnteTestSuite) TestAnteHandlerReCheck() { msg := testdata.NewTestMsg(accounts[0].acc.GetAddress()) msgs := []sdk.Msg{msg} - suite.txBuilder.SetMsgs(msgs...) + suite.Require().NoError(suite.txBuilder.SetMsgs(msgs...)) suite.txBuilder.SetMemo("thisisatestmemo") // test that operations skipped on recheck do not run privs, accNums, accSeqs := []crypto.PrivKey{accounts[0].priv}, []uint64{0}, []uint64{0} - tx := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(err) // make signature array empty which would normally cause ValidateBasicDecorator and SigVerificationDecorator fail // since these decorators don't run on recheck, the tx should pass the antehandler - stdTx := tx.(types.StdTx) - stdTx.Signatures = []types.StdSignature{} + txBuilder, err := suite.clientCtx.TxConfig.WrapTxBuilder(tx) + suite.Require().NoError(err) + suite.Require().NoError(txBuilder.SetSignatures()) - _, err := suite.anteHandler(suite.ctx, stdTx, false) + _, err = suite.anteHandler(suite.ctx, txBuilder.GetTx(), false) suite.Require().Nil(err, "AnteHandler errored on recheck unexpectedly: %v", err) - tx = suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + tx, err = suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(err) txBytes, err := json.Marshal(tx) suite.Require().Nil(err, "Error marshalling tx: %v", err) suite.ctx = suite.ctx.WithTxBytes(txBytes) @@ -1121,7 +1144,3 @@ func (suite *AnteTestSuite) TestAnteHandlerReCheck() { _, err = suite.anteHandler(suite.ctx, tx, false) suite.Require().NotNil(err, "antehandler on recheck did not fail once feePayer no longer has sufficient funds") } - -func TestAnteTestSuite(t *testing.T) { - suite.Run(t, new(AnteTestSuite)) -} diff --git a/x/auth/ante/basic_test.go b/x/auth/ante/basic_test.go index 4e8c83e9c..fc4e50ae5 100644 --- a/x/auth/ante/basic_test.go +++ b/x/auth/ante/basic_test.go @@ -1,19 +1,15 @@ package ante_test import ( - "encoding/json" "strings" - "testing" - - "github.com/stretchr/testify/suite" "github.com/cosmos/cosmos-sdk/testutil/testdata" + "github.com/cosmos/cosmos-sdk/types/tx/signing" "github.com/tendermint/tendermint/crypto" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth/ante" - "github.com/cosmos/cosmos-sdk/x/auth/types" ) func (suite *AnteTestSuite) TestValidateBasic() { @@ -27,21 +23,23 @@ func (suite *AnteTestSuite) TestValidateBasic() { msg := testdata.NewTestMsg(addr1) feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() - suite.txBuilder.SetMsgs(msg) + suite.Require().NoError(suite.txBuilder.SetMsgs(msg)) suite.txBuilder.SetFeeAmount(feeAmount) suite.txBuilder.SetGasLimit(gasLimit) privs, accNums, accSeqs := []crypto.PrivKey{}, []uint64{}, []uint64{} - invalidTx := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + invalidTx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(err) vbd := ante.NewValidateBasicDecorator() antehandler := sdk.ChainAnteDecorators(vbd) - _, err := antehandler(suite.ctx, invalidTx, false) + _, err = antehandler(suite.ctx, invalidTx, false) suite.Require().NotNil(err, "Did not error on invalid tx") privs, accNums, accSeqs = []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} - validTx := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + validTx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(err) _, err = antehandler(suite.ctx, validTx, false) suite.Require().Nil(err, "ValidateBasicDecorator returned error on valid tx. err: %v", err) @@ -66,23 +64,25 @@ func (suite *AnteTestSuite) TestValidateMemo() { msg := testdata.NewTestMsg(addr1) feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() - suite.txBuilder.SetMsgs(msg) + suite.Require().NoError(suite.txBuilder.SetMsgs(msg)) suite.txBuilder.SetFeeAmount(feeAmount) suite.txBuilder.SetGasLimit(gasLimit) privs, accNums, accSeqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} suite.txBuilder.SetMemo(strings.Repeat("01234567890", 500)) - invalidTx := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + invalidTx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(err) // require that long memos get rejected vmd := ante.NewValidateMemoDecorator(suite.app.AccountKeeper) antehandler := sdk.ChainAnteDecorators(vmd) - _, err := antehandler(suite.ctx, invalidTx, false) + _, err = antehandler(suite.ctx, invalidTx, false) suite.Require().NotNil(err, "Did not error on tx with high memo") suite.txBuilder.SetMemo(strings.Repeat("01234567890", 10)) - validTx := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + validTx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(err) // require small memos pass ValidateMemo Decorator _, err = antehandler(suite.ctx, validTx, false) @@ -100,14 +100,16 @@ func (suite *AnteTestSuite) TestConsumeGasForTxSize() { msg := testdata.NewTestMsg(addr1) feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() - suite.txBuilder.SetMsgs(msg) + suite.Require().NoError(suite.txBuilder.SetMsgs(msg)) suite.txBuilder.SetFeeAmount(feeAmount) suite.txBuilder.SetGasLimit(gasLimit) suite.txBuilder.SetMemo(strings.Repeat("01234567890", 10)) privs, accNums, accSeqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} - tx := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) - txBytes, err := json.Marshal(tx) + tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(err) + + txBytes, err := suite.clientCtx.TxConfig.TxJSONEncoder()(tx) suite.Require().Nil(err, "Cannot marshal tx: %v", err) cgtsd := ante.NewConsumeGasForTxSizeDecorator(suite.app.AccountKeeper) @@ -134,11 +136,15 @@ func (suite *AnteTestSuite) TestConsumeGasForTxSize() { suite.Require().Equal(expectedGas, consumedGas, "Decorator did not consume the correct amount of gas") // simulation must not underestimate gas of this decorator even with nil signatures - sigTx := tx.(types.StdTx) - sigTx.Signatures = []types.StdSignature{{}} + txBuilder, err := suite.clientCtx.TxConfig.WrapTxBuilder(tx) + suite.Require().NoError(err) + suite.Require().NoError(txBuilder.SetSignatures(signing.SignatureV2{ + PubKey: priv1.PubKey(), + })) + tx = txBuilder.GetTx() - simTxBytes, err := json.Marshal(sigTx) - suite.Require().Nil(err) + simTxBytes, err := suite.clientCtx.TxConfig.TxJSONEncoder()(tx) + suite.Require().Nil(err, "Cannot marshal tx: %v", err) // require that simulated tx is smaller than tx with signatures suite.Require().True(len(simTxBytes) < len(txBytes), "simulated tx still has signatures") @@ -148,14 +154,10 @@ func (suite *AnteTestSuite) TestConsumeGasForTxSize() { beforeSimGas := suite.ctx.GasMeter().GasConsumed() // run antehandler with simulate=true - suite.ctx, err = antehandler(suite.ctx, sigTx, true) + suite.ctx, err = antehandler(suite.ctx, tx, true) consumedSimGas := suite.ctx.GasMeter().GasConsumed() - beforeSimGas // require that antehandler passes and does not underestimate decorator cost suite.Require().Nil(err, "ConsumeTxSizeGasDecorator returned error: %v", err) suite.Require().True(consumedSimGas >= expectedGas, "Simulate mode underestimates gas on AnteDecorator. Simulated cost: %d, expected cost: %d", consumedSimGas, expectedGas) } - -func TestAnteBasicTestSuite(t *testing.T) { - suite.Run(t, new(AnteTestSuite)) -} diff --git a/x/auth/ante/fee_test.go b/x/auth/ante/fee_test.go index 80f200c61..4264baf39 100644 --- a/x/auth/ante/fee_test.go +++ b/x/auth/ante/fee_test.go @@ -1,10 +1,6 @@ package ante_test import ( - "testing" - - "github.com/stretchr/testify/suite" - "github.com/cosmos/cosmos-sdk/testutil/testdata" "github.com/tendermint/tendermint/crypto" @@ -27,12 +23,13 @@ func (suite *AnteTestSuite) TestEnsureMempoolFees() { msg := testdata.NewTestMsg(addr1) feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() - suite.txBuilder.SetMsgs(msg) + suite.Require().NoError(suite.txBuilder.SetMsgs(msg)) suite.txBuilder.SetFeeAmount(feeAmount) suite.txBuilder.SetGasLimit(gasLimit) privs, accNums, accSeqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} - tx := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(err) // Set high gas price so standard test fee fails atomPrice := sdk.NewDecCoinFromDec("atom", sdk.NewDec(200).Quo(sdk.NewDec(100000))) @@ -43,7 +40,7 @@ func (suite *AnteTestSuite) TestEnsureMempoolFees() { suite.ctx = suite.ctx.WithIsCheckTx(true) // antehandler errors with insufficient fees - _, err := antehandler(suite.ctx, tx, false) + _, err = antehandler(suite.ctx, tx, false) suite.Require().NotNil(err, "Decorator should have errored on too low fee for local gasPrice") // Set IsCheckTx to false @@ -75,12 +72,13 @@ func (suite *AnteTestSuite) TestDeductFees() { msg := testdata.NewTestMsg(addr1) feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() - suite.txBuilder.SetMsgs(msg) + suite.Require().NoError(suite.txBuilder.SetMsgs(msg)) suite.txBuilder.SetFeeAmount(feeAmount) suite.txBuilder.SetGasLimit(gasLimit) privs, accNums, accSeqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} - tx := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(err) // Set account with insufficient funds acc := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, addr1) @@ -90,7 +88,7 @@ func (suite *AnteTestSuite) TestDeductFees() { dfd := ante.NewDeductFeeDecorator(suite.app.AccountKeeper, suite.app.BankKeeper) antehandler := sdk.ChainAnteDecorators(dfd) - _, err := antehandler(suite.ctx, tx, false) + _, err = antehandler(suite.ctx, tx, false) suite.Require().NotNil(err, "Tx did not error when fee payer had insufficient funds") @@ -102,7 +100,3 @@ func (suite *AnteTestSuite) TestDeductFees() { suite.Require().Nil(err, "Tx errored after account has been set with sufficient funds") } - -func TestAnteFeeTestSuite(t *testing.T) { - suite.Run(t, new(AnteTestSuite)) -} diff --git a/x/auth/ante/integration_test.go b/x/auth/ante/integration_test.go deleted file mode 100644 index 4c232b2bb..000000000 --- a/x/auth/ante/integration_test.go +++ /dev/null @@ -1,18 +0,0 @@ -package ante_test - -import ( - abci "github.com/tendermint/tendermint/abci/types" - - "github.com/cosmos/cosmos-sdk/simapp" - sdk "github.com/cosmos/cosmos-sdk/types" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" -) - -// returns context and app with params set on account keeper -func createTestApp(isCheckTx bool) (*simapp.SimApp, sdk.Context) { - app := simapp.Setup(isCheckTx) - ctx := app.BaseApp.NewContext(isCheckTx, abci.Header{}) - app.AccountKeeper.SetParams(ctx, authtypes.DefaultParams()) - - return app, ctx -} diff --git a/x/auth/ante/setup_test.go b/x/auth/ante/setup_test.go index a6e185d14..8454ec26a 100644 --- a/x/auth/ante/setup_test.go +++ b/x/auth/ante/setup_test.go @@ -1,10 +1,6 @@ package ante_test import ( - "testing" - - "github.com/stretchr/testify/suite" - "github.com/cosmos/cosmos-sdk/testutil/testdata" "github.com/tendermint/tendermint/crypto" @@ -25,12 +21,13 @@ func (suite *AnteTestSuite) TestSetup() { msg := testdata.NewTestMsg(addr1) feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() - suite.txBuilder.SetMsgs(msg) + suite.Require().NoError(suite.txBuilder.SetMsgs(msg)) suite.txBuilder.SetFeeAmount(feeAmount) suite.txBuilder.SetGasLimit(gasLimit) privs, accNums, accSeqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} - tx := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(err) sud := ante.NewSetUpContextDecorator() antehandler := sdk.ChainAnteDecorators(sud) @@ -59,12 +56,13 @@ func (suite *AnteTestSuite) TestRecoverPanic() { msg := testdata.NewTestMsg(addr1) feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() - suite.txBuilder.SetMsgs(msg) + suite.Require().NoError(suite.txBuilder.SetMsgs(msg)) suite.txBuilder.SetFeeAmount(feeAmount) suite.txBuilder.SetGasLimit(gasLimit) privs, accNums, accSeqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} - tx := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(err) sud := ante.NewSetUpContextDecorator() antehandler := sdk.ChainAnteDecorators(sud, OutOfGasDecorator{}) @@ -101,7 +99,3 @@ type PanicDecorator struct{} func (pd PanicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { panic("random error") } - -func TestAnteSetupTestSuite(t *testing.T) { - suite.Run(t, new(AnteTestSuite)) -} diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index e128b9e26..c980878cd 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -2,9 +2,6 @@ package ante_test import ( "fmt" - "testing" - - "github.com/stretchr/testify/suite" "github.com/cosmos/cosmos-sdk/testutil/testdata" @@ -42,7 +39,7 @@ func (suite *AnteTestSuite) TestSetPubKey() { suite.app.AccountKeeper.SetAccount(suite.ctx, acc) msgs[i] = testdata.NewTestMsg(addr) } - suite.txBuilder.SetMsgs(msgs...) + suite.Require().NoError(suite.txBuilder.SetMsgs(msgs...)) feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() @@ -50,7 +47,8 @@ func (suite *AnteTestSuite) TestSetPubKey() { suite.txBuilder.SetGasLimit(gasLimit) privs, accNums, accSeqs := []crypto.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{0, 0, 0} - tx := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(err) spkd := ante.NewSetPubKeyDecorator(suite.app.AccountKeeper) antehandler := sdk.ChainAnteDecorators(spkd) @@ -143,7 +141,7 @@ func (suite *AnteTestSuite) TestSigVerification() { gasLimit := testdata.NewTestGasLimit() spkd := ante.NewSetPubKeyDecorator(suite.app.AccountKeeper) - svd := ante.NewSigVerificationDecorator(suite.app.AccountKeeper, types.LegacyAminoJSONHandler{}) + svd := ante.NewSigVerificationDecorator(suite.app.AccountKeeper, suite.clientCtx.TxConfig.SignModeHandler()) antehandler := sdk.ChainAnteDecorators(spkd, svd) type testCase struct { @@ -167,13 +165,14 @@ func (suite *AnteTestSuite) TestSigVerification() { suite.ctx = suite.ctx.WithIsReCheckTx(tc.recheck) suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() // Create new txBuilder for each test - suite.txBuilder.SetMsgs(msgs...) + suite.Require().NoError(suite.txBuilder.SetMsgs(msgs...)) suite.txBuilder.SetFeeAmount(feeAmount) suite.txBuilder.SetGasLimit(gasLimit) - tx := suite.CreateTestTx(tc.privs, tc.accNums, tc.accSeqs, suite.ctx.ChainID()) + tx, err := suite.CreateTestTx(tc.privs, tc.accNums, tc.accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(err) - _, err := antehandler(suite.ctx, tx, false) + _, err = antehandler(suite.ctx, tx, false) if tc.shouldErr { suite.Require().NotNil(err, "TestCase %d: %s did not error as expected", i, tc.name) } else { @@ -219,18 +218,19 @@ func (suite *AnteTestSuite) runSigDecorators(params types.Params, _ bool, privs accNums[i] = uint64(i) accSeqs[i] = uint64(0) } - suite.txBuilder.SetMsgs(msgs...) + suite.Require().NoError(suite.txBuilder.SetMsgs(msgs...)) feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() suite.txBuilder.SetFeeAmount(feeAmount) suite.txBuilder.SetGasLimit(gasLimit) - tx := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(err) spkd := ante.NewSetPubKeyDecorator(suite.app.AccountKeeper) svgc := ante.NewSigGasConsumeDecorator(suite.app.AccountKeeper, ante.DefaultSigVerificationGasConsumer) - svd := ante.NewSigVerificationDecorator(suite.app.AccountKeeper, types.LegacyAminoJSONHandler{}) + svd := ante.NewSigVerificationDecorator(suite.app.AccountKeeper, suite.clientCtx.TxConfig.SignModeHandler()) antehandler := sdk.ChainAnteDecorators(spkd, svgc, svd) // Determine gas consumption of antehandler with default params @@ -251,7 +251,7 @@ func (suite *AnteTestSuite) TestIncrementSequenceDecorator() { suite.app.AccountKeeper.SetAccount(suite.ctx, acc) msgs := []sdk.Msg{testdata.NewTestMsg(addr)} - suite.txBuilder.SetMsgs(msgs...) + suite.Require().NoError(suite.txBuilder.SetMsgs(msgs...)) privs := []crypto.PrivKey{priv} accNums := []uint64{suite.app.AccountKeeper.GetAccount(suite.ctx, addr).GetAccountNumber()} accSeqs := []uint64{suite.app.AccountKeeper.GetAccount(suite.ctx, addr).GetSequence()} @@ -260,7 +260,8 @@ func (suite *AnteTestSuite) TestIncrementSequenceDecorator() { suite.txBuilder.SetFeeAmount(feeAmount) suite.txBuilder.SetGasLimit(gasLimit) - tx := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) + suite.Require().NoError(err) isd := ante.NewIncrementSequenceDecorator(suite.app.AccountKeeper) antehandler := sdk.ChainAnteDecorators(isd) @@ -283,7 +284,3 @@ func (suite *AnteTestSuite) TestIncrementSequenceDecorator() { suite.Require().Equal(tc.expectedSeq, suite.app.AccountKeeper.GetAccount(suite.ctx, addr).GetSequence()) } } - -func TestAnteSigverifyTestSuite(t *testing.T) { - suite.Run(t, new(AnteTestSuite)) -} diff --git a/x/auth/ante/testutil_test.go b/x/auth/ante/testutil_test.go index b82844d88..0725f8ce8 100644 --- a/x/auth/ante/testutil_test.go +++ b/x/auth/ante/testutil_test.go @@ -3,20 +3,23 @@ package ante_test import ( "errors" "fmt" + "testing" "github.com/stretchr/testify/suite" + abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/tx" "github.com/cosmos/cosmos-sdk/simapp" - simappparams "github.com/cosmos/cosmos-sdk/simapp/params" "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" "github.com/cosmos/cosmos-sdk/x/auth/types" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" ) // TestAccount represents an account used in the tests in x/auth/ante. @@ -36,16 +39,29 @@ type AnteTestSuite struct { txBuilder client.TxBuilder } +// returns context and app with params set on account keeper +func createTestApp(isCheckTx bool) (*simapp.SimApp, sdk.Context) { + app := simapp.Setup(isCheckTx) + ctx := app.BaseApp.NewContext(isCheckTx, abci.Header{}) + app.AccountKeeper.SetParams(ctx, authtypes.DefaultParams()) + + return app, ctx +} + // SetupTest setups a new test, with new app, context, and anteHandler. func (suite *AnteTestSuite) SetupTest(isCheckTx bool) { suite.app, suite.ctx = createTestApp(isCheckTx) suite.ctx = suite.ctx.WithBlockHeight(1) - suite.anteHandler = ante.NewAnteHandler(suite.app.AccountKeeper, suite.app.BankKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) - // set up the TxBuilder - encodingConfig := simappparams.MakeEncodingConfig() + // Set up TxConfig. + encodingConfig := simapp.MakeEncodingConfig() + // We're using TestMsg amino encoding in some tests, so register it here. + encodingConfig.Amino.RegisterConcrete(&testdata.TestMsg{}, "testdata.TestMsg", nil) + suite.clientCtx = client.Context{}. WithTxConfig(encodingConfig.TxConfig) + + suite.anteHandler = ante.NewAnteHandler(suite.app.AccountKeeper, suite.app.BankKeeper, ante.DefaultSigVerificationGasConsumer, encodingConfig.TxConfig.SignModeHandler()) } // CreateTestAccounts creates `numAccs` accounts, and return all relevant @@ -70,7 +86,21 @@ 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 { +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. var sigsV2 []signing.SignatureV2 for i, priv := range privs { signerData := xauthsigning.SignerData{ @@ -79,13 +109,18 @@ func (suite *AnteTestSuite) CreateTestTx(privs []crypto.PrivKey, accNums []uint6 AccountSequence: accSeqs[i], } sigV2, err := tx.SignWithPrivKey(suite.clientCtx.TxConfig.SignModeHandler().DefaultMode(), signerData, suite.txBuilder, priv, suite.clientCtx.TxConfig) - suite.Require().NoError(err) + if err != nil { + return nil, err + } sigsV2 = append(sigsV2, sigV2) } - suite.txBuilder.SetSignatures(sigsV2...) + err := suite.txBuilder.SetSignatures(sigsV2...) + if err != nil { + return nil, err + } - return suite.txBuilder.GetTx() + return suite.txBuilder.GetTx(), nil } // TestCase represents a test case used in test tables. @@ -100,21 +135,39 @@ type TestCase struct { // CreateTestTx is a helper function to create a tx given multiple inputs. func (suite *AnteTestSuite) RunTestCase(privs []crypto.PrivKey, msgs []sdk.Msg, feeAmount sdk.Coins, gasLimit uint64, accNums, accSeqs []uint64, chainID string, tc TestCase) { suite.Run(fmt.Sprintf("Case %s", tc.desc), func() { - suite.txBuilder.SetMsgs(msgs...) + suite.Require().NoError(suite.txBuilder.SetMsgs(msgs...)) suite.txBuilder.SetFeeAmount(feeAmount) suite.txBuilder.SetGasLimit(gasLimit) - tx := suite.CreateTestTx(privs, accNums, accSeqs, chainID) - newCtx, err := suite.anteHandler(suite.ctx, tx, tc.simulate) + // Theoretically speaking, ante handler unit tests should only test + // ante handlers, but here we sometimes also test the tx creation + // process. + tx, txErr := suite.CreateTestTx(privs, accNums, accSeqs, chainID) + newCtx, anteErr := suite.anteHandler(suite.ctx, tx, tc.simulate) if tc.expPass { - suite.Require().NoError(err) + suite.Require().NoError(txErr) + suite.Require().NoError(anteErr) suite.Require().NotNil(newCtx) suite.ctx = newCtx } else { - suite.Require().Error(err) - suite.Require().True(errors.Is(err, tc.expErr)) + switch { + case txErr != nil: + suite.Require().Error(txErr) + suite.Require().True(errors.Is(txErr, tc.expErr)) + + case anteErr != nil: + suite.Require().Error(anteErr) + suite.Require().True(errors.Is(anteErr, tc.expErr)) + + default: + suite.Fail("expected one of txErr,anteErr to be an error") + } } }) } + +func TestAnteTestSuite(t *testing.T) { + suite.Run(t, new(AnteTestSuite)) +} diff --git a/x/auth/signing/verify.go b/x/auth/signing/verify.go index 99a6d39b4..9f9de4c4a 100644 --- a/x/auth/signing/verify.go +++ b/x/auth/signing/verify.go @@ -12,10 +12,10 @@ import ( // VerifySignature verifies a transaction signature contained in SignatureData abstracting over different signing modes // and single vs multi-signatures. -func VerifySignature(pubKey crypto.PubKey, signingData SignerData, sigData signing.SignatureData, handler SignModeHandler, tx sdk.Tx) error { +func VerifySignature(pubKey crypto.PubKey, signerData SignerData, sigData signing.SignatureData, handler SignModeHandler, tx sdk.Tx) error { switch data := sigData.(type) { case *signing.SingleSignatureData: - signBytes, err := handler.GetSignBytes(data.SignMode, signingData, tx) + signBytes, err := handler.GetSignBytes(data.SignMode, signerData, tx) if err != nil { return err } @@ -30,7 +30,7 @@ func VerifySignature(pubKey crypto.PubKey, signingData SignerData, sigData signi return fmt.Errorf("expected %T, got %T", (multisig.PubKey)(nil), pubKey) } err := multiPK.VerifyMultisignature(func(mode signing.SignMode) ([]byte, error) { - return handler.GetSignBytes(mode, signingData, tx) + return handler.GetSignBytes(mode, signerData, tx) }, data) if err != nil { return err diff --git a/x/auth/tx/builder.go b/x/auth/tx/builder.go index d29e805ad..dc3a92082 100644 --- a/x/auth/tx/builder.go +++ b/x/auth/tx/builder.go @@ -1,26 +1,21 @@ package tx import ( + "bytes" "fmt" "github.com/gogo/protobuf/proto" - - "github.com/cosmos/cosmos-sdk/client" - "github.com/cosmos/cosmos-sdk/x/auth/signing/direct" - - authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" - - "github.com/cosmos/cosmos-sdk/types/tx" - - "github.com/cosmos/cosmos-sdk/types/tx/signing" - - codectypes "github.com/cosmos/cosmos-sdk/codec/types" - "github.com/tendermint/tendermint/crypto" + "github.com/cosmos/cosmos-sdk/client" + codectypes "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/types/tx" + "github.com/cosmos/cosmos-sdk/types/tx/signing" + authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" + "github.com/cosmos/cosmos-sdk/x/auth/signing/direct" ) type builder struct { @@ -319,6 +314,63 @@ func (t *builder) setSignerInfos(infos []*tx.SignerInfo) { t.pubKeys = nil } +// getSignerIndex returns the index of a public key in the GetSigners array. It +// returns an error if the publicKey is not in GetSigners. +func (t *builder) getSignerIndex(pubKey crypto.PubKey) (int, error) { + if pubKey == nil { + return -1, sdkerrors.Wrap( + sdkerrors.ErrInvalidPubKey, + "public key is empty", + ) + } + + for i, signer := range t.GetSigners() { + if bytes.Equal(signer.Bytes(), pubKey.Address().Bytes()) { + return i, nil + } + } + + return -1, sdkerrors.Wrapf( + sdkerrors.ErrInvalidPubKey, + "public key %s is not a signer of this tx, call SetMsgs first", pubKey, + ) +} + +// SetSignerInfo implements TxBuilder.SetSignerInfo. +func (t *builder) SetSignerInfo(pubKey crypto.PubKey, modeInfo *tx.ModeInfo) error { + signerIndex, err := t.getSignerIndex(pubKey) + if err != nil { + return err + } + + pk, err := t.pubkeyCodec.Encode(pubKey) + if err != nil { + return err + } + + n := len(t.GetSigners()) + // If t.tx.AuthInfo.SignerInfos is empty, we just initialize with some + // empty data. + if len(t.tx.AuthInfo.SignerInfos) == 0 { + t.tx.AuthInfo.SignerInfos = make([]*tx.SignerInfo, n) + for i := 1; i < n; i++ { + t.tx.AuthInfo.SignerInfos[i] = &tx.SignerInfo{} + } + } + + t.tx.AuthInfo.SignerInfos[signerIndex] = &tx.SignerInfo{ + PublicKey: pk, + ModeInfo: modeInfo, + } + + // set authInfoBz to nil because the cached authInfoBz no longer matches tx.AuthInfo + t.authInfoBz = nil + // set cached pubKeys to nil because they no longer match tx.AuthInfo + t.pubKeys = nil + + return nil +} + func (t *builder) setSignatures(sigs [][]byte) { t.tx.Signatures = sigs } diff --git a/x/auth/tx/builder_test.go b/x/auth/tx/builder_test.go index f1a7b1140..adcdbefab 100644 --- a/x/auth/tx/builder_test.go +++ b/x/auth/tx/builder_test.go @@ -1,28 +1,28 @@ package tx import ( + "errors" + "fmt" "testing" - "github.com/cosmos/cosmos-sdk/testutil/testdata" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - - tx2 "github.com/cosmos/cosmos-sdk/types/tx" - - "github.com/cosmos/cosmos-sdk/types/tx/signing" - "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/crypto" "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/std" + "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + txtypes "github.com/cosmos/cosmos-sdk/types/tx" + "github.com/cosmos/cosmos-sdk/types/tx/signing" ) func TestTxBuilder(t *testing.T) { _, pubkey, addr := testdata.KeyTestPubAddr() marshaler := codec.NewHybridCodec(codec.New(), codectypes.NewInterfaceRegistry()) - tx := newBuilder(std.DefaultPublicKeyCodec{}) + txBuilder := newBuilder(std.DefaultPublicKeyCodec{}) cdc := std.DefaultPublicKeyCodec{} @@ -33,12 +33,12 @@ func TestTxBuilder(t *testing.T) { pk, err := cdc.Encode(pubkey) require.NoError(t, err) - var signerInfo []*tx2.SignerInfo - signerInfo = append(signerInfo, &tx2.SignerInfo{ + var signerInfo []*txtypes.SignerInfo + signerInfo = append(signerInfo, &txtypes.SignerInfo{ PublicKey: pk, - ModeInfo: &tx2.ModeInfo{ - Sum: &tx2.ModeInfo_Single_{ - Single: &tx2.ModeInfo_Single{ + ModeInfo: &txtypes.ModeInfo{ + Sum: &txtypes.ModeInfo_Single_{ + Single: &txtypes.ModeInfo_Single{ Mode: signing.SignMode_SIGN_MODE_DIRECT, }, }, @@ -54,10 +54,10 @@ func TestTxBuilder(t *testing.T) { }, } - fee := tx2.Fee{Amount: sdk.NewCoins(sdk.NewInt64Coin("atom", 150)), GasLimit: 20000} + fee := txtypes.Fee{Amount: sdk.NewCoins(sdk.NewInt64Coin("atom", 150)), GasLimit: 20000} t.Log("verify that authInfo bytes encoded with DefaultTxEncoder and decoded with DefaultTxDecoder can be retrieved from GetAuthInfoBytes") - authInfo := &tx2.AuthInfo{ + authInfo := &txtypes.AuthInfo{ Fee: &fee, SignerInfos: signerInfo, } @@ -77,43 +77,43 @@ func TestTxBuilder(t *testing.T) { } } - txBody := &tx2.TxBody{ + txBody := &txtypes.TxBody{ Memo: memo, Messages: anys, } bodyBytes := marshaler.MustMarshalBinaryBare(txBody) require.NotEmpty(t, bodyBytes) - require.Empty(t, tx.GetBodyBytes()) + require.Empty(t, txBuilder.GetBodyBytes()) t.Log("verify that calling the SetMsgs, SetMemo results in the correct GetBodyBytes") - require.NotEqual(t, bodyBytes, tx.GetBodyBytes()) - err = tx.SetMsgs(msgs...) + require.NotEqual(t, bodyBytes, txBuilder.GetBodyBytes()) + err = txBuilder.SetMsgs(msgs...) require.NoError(t, err) - require.NotEqual(t, bodyBytes, tx.GetBodyBytes()) - tx.SetMemo(memo) - require.Equal(t, bodyBytes, tx.GetBodyBytes()) - require.Equal(t, len(msgs), len(tx.GetMsgs())) - require.Equal(t, 0, len(tx.GetPubKeys())) + require.NotEqual(t, bodyBytes, txBuilder.GetBodyBytes()) + txBuilder.SetMemo(memo) + require.Equal(t, bodyBytes, txBuilder.GetBodyBytes()) + require.Equal(t, len(msgs), len(txBuilder.GetMsgs())) + require.Equal(t, 0, len(txBuilder.GetPubKeys())) t.Log("verify that updated AuthInfo results in the correct GetAuthInfoBytes and GetPubKeys") - require.NotEqual(t, authInfoBytes, tx.GetAuthInfoBytes()) - tx.SetFeeAmount(fee.Amount) - require.NotEqual(t, authInfoBytes, tx.GetAuthInfoBytes()) - tx.SetGasLimit(fee.GasLimit) - require.NotEqual(t, authInfoBytes, tx.GetAuthInfoBytes()) - err = tx.SetSignatures(sig) + require.NotEqual(t, authInfoBytes, txBuilder.GetAuthInfoBytes()) + txBuilder.SetFeeAmount(fee.Amount) + require.NotEqual(t, authInfoBytes, txBuilder.GetAuthInfoBytes()) + txBuilder.SetGasLimit(fee.GasLimit) + require.NotEqual(t, authInfoBytes, txBuilder.GetAuthInfoBytes()) + err = txBuilder.SetSignatures(sig) require.NoError(t, err) // once fee, gas and signerInfos are all set, AuthInfo bytes should match - require.Equal(t, authInfoBytes, tx.GetAuthInfoBytes()) + require.Equal(t, authInfoBytes, txBuilder.GetAuthInfoBytes()) - require.Equal(t, len(msgs), len(tx.GetMsgs())) - require.Equal(t, 1, len(tx.GetPubKeys())) - require.Equal(t, pubkey.Bytes(), tx.GetPubKeys()[0].Bytes()) + require.Equal(t, len(msgs), len(txBuilder.GetMsgs())) + require.Equal(t, 1, len(txBuilder.GetPubKeys())) + require.Equal(t, pubkey.Bytes(), txBuilder.GetPubKeys()[0].Bytes()) - tx = &builder{} + txBuilder = &builder{} require.NotPanics(t, func() { - _ = tx.GetMsgs() + _ = txBuilder.GetMsgs() }) } @@ -130,7 +130,7 @@ func TestBuilderValidateBasic(t *testing.T) { // require to fail validation upon invalid fee badFeeAmount := testdata.NewTestFeeAmount() badFeeAmount[0].Amount = sdk.NewInt(-5) - bldr := newBuilder(std.DefaultPublicKeyCodec{}) + txBuilder := newBuilder(std.DefaultPublicKeyCodec{}) var sig1, sig2 signing.SignatureV2 sig1 = signing.SignatureV2{ @@ -149,87 +149,188 @@ func TestBuilderValidateBasic(t *testing.T) { }, } - err := bldr.SetMsgs(msgs...) + err := txBuilder.SetMsgs(msgs...) require.NoError(t, err) - bldr.SetGasLimit(200000) - err = bldr.SetSignatures(sig1, sig2) + txBuilder.SetGasLimit(200000) + err = txBuilder.SetSignatures(sig1, sig2) require.NoError(t, err) - bldr.SetFeeAmount(badFeeAmount) - err = bldr.ValidateBasic() + txBuilder.SetFeeAmount(badFeeAmount) + err = txBuilder.ValidateBasic() require.Error(t, err) _, code, _ := sdkerrors.ABCIInfo(err, false) require.Equal(t, sdkerrors.ErrInsufficientFee.ABCICode(), code) // require to fail validation when no signatures exist - err = bldr.SetSignatures() + err = txBuilder.SetSignatures() require.NoError(t, err) - bldr.SetFeeAmount(feeAmount) - err = bldr.ValidateBasic() + txBuilder.SetFeeAmount(feeAmount) + err = txBuilder.ValidateBasic() require.Error(t, err) _, code, _ = sdkerrors.ABCIInfo(err, false) require.Equal(t, sdkerrors.ErrNoSignatures.ABCICode(), code) // require to fail with nil values for tx, authinfo - err = bldr.SetMsgs(msgs...) + err = txBuilder.SetMsgs(msgs...) require.NoError(t, err) - err = bldr.ValidateBasic() + err = txBuilder.ValidateBasic() require.Error(t, err) // require to fail validation when signatures do not match expected signers - err = bldr.SetSignatures(sig1) + err = txBuilder.SetSignatures(sig1) require.NoError(t, err) - err = bldr.ValidateBasic() + err = txBuilder.ValidateBasic() require.Error(t, err) _, code, _ = sdkerrors.ABCIInfo(err, false) require.Equal(t, sdkerrors.ErrUnauthorized.ABCICode(), code) require.Error(t, err) - bldr.SetFeeAmount(feeAmount) - err = bldr.SetSignatures(sig1, sig2) + txBuilder.SetFeeAmount(feeAmount) + err = txBuilder.SetSignatures(sig1, sig2) require.NoError(t, err) - err = bldr.ValidateBasic() + err = txBuilder.ValidateBasic() require.NoError(t, err) // gas limit too high - bldr.SetGasLimit(MaxGasWanted + 1) - err = bldr.ValidateBasic() + txBuilder.SetGasLimit(MaxGasWanted + 1) + err = txBuilder.ValidateBasic() require.Error(t, err) - bldr.SetGasLimit(MaxGasWanted - 1) - err = bldr.ValidateBasic() + txBuilder.SetGasLimit(MaxGasWanted - 1) + err = txBuilder.ValidateBasic() require.NoError(t, err) // bad builder structs // missing body - body := bldr.tx.Body - bldr.tx.Body = nil - err = bldr.ValidateBasic() + body := txBuilder.tx.Body + txBuilder.tx.Body = nil + err = txBuilder.ValidateBasic() require.Error(t, err) - bldr.tx.Body = body - err = bldr.ValidateBasic() + txBuilder.tx.Body = body + err = txBuilder.ValidateBasic() require.NoError(t, err) // missing fee - f := bldr.tx.AuthInfo.Fee - bldr.tx.AuthInfo.Fee = nil - err = bldr.ValidateBasic() + f := txBuilder.tx.AuthInfo.Fee + txBuilder.tx.AuthInfo.Fee = nil + err = txBuilder.ValidateBasic() require.Error(t, err) - bldr.tx.AuthInfo.Fee = f - err = bldr.ValidateBasic() + txBuilder.tx.AuthInfo.Fee = f + err = txBuilder.ValidateBasic() require.NoError(t, err) // missing AuthInfo - authInfo := bldr.tx.AuthInfo - bldr.tx.AuthInfo = nil - err = bldr.ValidateBasic() + authInfo := txBuilder.tx.AuthInfo + txBuilder.tx.AuthInfo = nil + err = txBuilder.ValidateBasic() require.Error(t, err) - bldr.tx.AuthInfo = authInfo - err = bldr.ValidateBasic() + txBuilder.tx.AuthInfo = authInfo + err = txBuilder.ValidateBasic() require.NoError(t, err) // missing tx - bldr.tx = nil - err = bldr.ValidateBasic() + txBuilder.tx = nil + 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) + } + } +} diff --git a/x/auth/types/client_tx.go b/x/auth/types/client_tx.go index b0f8fc613..ecede31bd 100644 --- a/x/auth/types/client_tx.go +++ b/x/auth/types/client_tx.go @@ -3,10 +3,13 @@ package types import ( "fmt" + "github.com/tendermint/tendermint/crypto" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/codec/legacy" sdk "github.com/cosmos/cosmos-sdk/types" + 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" ) @@ -32,7 +35,13 @@ func (s *StdTxBuilder) SetMsgs(msgs ...sdk.Msg) error { return nil } -// SetSignatures implements TxBuilder.SetSignatures +// SetSignerInfo implements TxBuilder.SetSignerInfo. +func (s *StdTxBuilder) SetSignerInfo(_ crypto.PubKey, _ *txtypes.ModeInfo) error { + // SetSignerInfo is a no-op for amino StdTx + return nil +} + +// SetSignatures implements TxBuilder.SetSignatures. func (s *StdTxBuilder) SetSignatures(signatures ...signing.SignatureV2) error { sigs := make([]StdSignature, len(signatures))