diff --git a/client/testutil/suite.go b/client/testutil/suite.go index 4bc02572385e..3f6e138e5af8 100644 --- a/client/testutil/suite.go +++ b/client/testutil/suite.go @@ -119,7 +119,6 @@ func (s *TxConfigTestSuite) TestTxBuilderSetSignatures() { err = txBuilder.SetSignatures(sig1, msig) s.Require().NoError(err) sigTx = txBuilder.GetTx() - s.Require().Len(sigTx.GetSignatures(), 2) sigsV2, err := sigTx.GetSignaturesV2() s.Require().NoError(err) s.Require().Len(sigsV2, 2) @@ -163,7 +162,6 @@ func (s *TxConfigTestSuite) TestTxBuilderSetSignatures() { err = txBuilder.SetSignatures(sig1, msig) s.Require().NoError(err) sigTx = txBuilder.GetTx() - s.Require().Len(sigTx.GetSignatures(), 2) sigsV2, err = sigTx.GetSignaturesV2() s.Require().NoError(err) s.Require().Len(sigsV2, 2) @@ -267,7 +265,9 @@ func (s *TxConfigTestSuite) TestTxEncodeDecode() { s.Require().Equal(feeAmount, tx3.GetFee()) s.Require().Equal(gasLimit, tx3.GetGas()) s.Require().Equal(memo, tx3.GetMemo()) - s.Require().Equal([][]byte{dummySig}, tx3.GetSignatures()) + tx3Sigs, err := tx3.GetSignaturesV2() + s.Require().NoError(err) + s.Require().Equal([]signingtypes.SignatureV2{sig}, tx3Sigs) s.Require().Equal([]crypto.PubKey{pubkey}, tx3.GetPubKeys()) s.T().Log("JSON encode transaction") @@ -284,7 +284,9 @@ func (s *TxConfigTestSuite) TestTxEncodeDecode() { s.Require().Equal(feeAmount, tx3.GetFee()) s.Require().Equal(gasLimit, tx3.GetGas()) s.Require().Equal(memo, tx3.GetMemo()) - s.Require().Equal([][]byte{dummySig}, tx3.GetSignatures()) + tx3Sigs, err = tx3.GetSignaturesV2() + s.Require().NoError(err) + s.Require().Equal([]signingtypes.SignatureV2{sig}, tx3Sigs) s.Require().Equal([]crypto.PubKey{pubkey}, tx3.GetPubKeys()) } diff --git a/client/tx/tx_test.go b/client/tx/tx_test.go index 587e497db67c..a2834f08dd59 100644 --- a/client/tx/tx_test.go +++ b/client/tx/tx_test.go @@ -116,7 +116,10 @@ func TestBuildUnsignedTx(t *testing.T) { tx, err := tx.BuildUnsignedTx(txf, msg) require.NoError(t, err) require.NotNil(t, tx) - require.Empty(t, tx.GetTx().(signing.SigVerifiableTx).GetSignatures()) + + sigs, err := tx.GetTx().(signing.SigVerifiableTx).GetSignaturesV2() + require.NoError(t, err) + require.Empty(t, sigs) } func TestSign(t *testing.T) { diff --git a/x/auth/ante/basic.go b/x/auth/ante/basic.go index 2a11f35dc4d9..df874a63b1f4 100644 --- a/x/auth/ante/basic.go +++ b/x/auth/ante/basic.go @@ -7,7 +7,8 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/types/multisig" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - "github.com/cosmos/cosmos-sdk/x/auth/signing" + "github.com/cosmos/cosmos-sdk/types/tx/signing" + authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" "github.com/cosmos/cosmos-sdk/x/auth/types" ) @@ -90,7 +91,7 @@ func NewConsumeGasForTxSizeDecorator(ak AccountKeeper) ConsumeTxSizeGasDecorator } func (cgts ConsumeTxSizeGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { - sigTx, ok := tx.(signing.SigVerifiableTx) + sigTx, ok := tx.(authsigning.SigVerifiableTx) if !ok { return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "invalid tx type") } @@ -101,10 +102,15 @@ func (cgts ConsumeTxSizeGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim // simulate gas cost for signatures in simulate mode if simulate { // in simulate mode, each element should be a nil signature - sigs := sigTx.GetSignatures() + sigs, err := sigTx.GetSignaturesV2() + if err != nil { + return ctx, err + } + n := len(sigs) + for i, signer := range sigTx.GetSigners() { // if signature is already filled in, no need to simulate gas cost - if sigs[i] != nil { + if i < n && !isIncompleteSignature(sigs[i].Data) { continue } @@ -141,6 +147,29 @@ func (cgts ConsumeTxSizeGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim return next(ctx, tx, simulate) } +// isIncompleteSignature tests whether SignatureData is fully filled in for simulation purposes +func isIncompleteSignature(data signing.SignatureData) bool { + if data == nil { + return true + } + + switch data := data.(type) { + case *signing.SingleSignatureData: + return len(data.Signature) == 0 + case *signing.MultiSignatureData: + if len(data.Signatures) == 0 { + return true + } + for _, s := range data.Signatures { + if isIncompleteSignature(s) { + return true + } + } + } + + return false +} + type ( // TxTimeoutHeightDecorator defines an AnteHandler decorator that checks for a // tx height timeout. diff --git a/x/auth/ante/basic_test.go b/x/auth/ante/basic_test.go index 0247e3c2bc34..8b304d180e51 100644 --- a/x/auth/ante/basic_test.go +++ b/x/auth/ante/basic_test.go @@ -8,6 +8,7 @@ import ( "github.com/tendermint/tendermint/crypto" + "github.com/cosmos/cosmos-sdk/crypto/types/multisig" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth/ante" ) @@ -91,7 +92,6 @@ func (suite *AnteTestSuite) TestValidateMemo() { func (suite *AnteTestSuite) TestConsumeGasForTxSize() { suite.SetupTest(true) // setup - suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() // keys and addresses priv1, _, addr1 := testdata.KeyTestPubAddr() @@ -100,66 +100,80 @@ func (suite *AnteTestSuite) TestConsumeGasForTxSize() { msg := testdata.NewTestMsg(addr1) feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() - 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, 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) antehandler := sdk.ChainAnteDecorators(cgtsd) - params := suite.app.AccountKeeper.GetParams(suite.ctx) - expectedGas := sdk.Gas(len(txBytes)) * params.TxSizeCostPerByte + testCases := []struct { + name string + sigV2 signing.SignatureV2 + }{ + {"SingleSignatureData", signing.SignatureV2{PubKey: priv1.PubKey()}}, + {"MultiSignatureData", signing.SignatureV2{PubKey: priv1.PubKey(), Data: multisig.NewMultisig(2)}}, + } + + for _, tc := range testCases { + suite.Run(tc.name, func() { + suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() + 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, 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) - // Set suite.ctx with TxBytes manually - suite.ctx = suite.ctx.WithTxBytes(txBytes) + params := suite.app.AccountKeeper.GetParams(suite.ctx) + expectedGas := sdk.Gas(len(txBytes)) * params.TxSizeCostPerByte - // track how much gas is necessary to retrieve parameters - beforeGas := suite.ctx.GasMeter().GasConsumed() - suite.app.AccountKeeper.GetParams(suite.ctx) - afterGas := suite.ctx.GasMeter().GasConsumed() - expectedGas += afterGas - beforeGas + // Set suite.ctx with TxBytes manually + suite.ctx = suite.ctx.WithTxBytes(txBytes) - beforeGas = suite.ctx.GasMeter().GasConsumed() - suite.ctx, err = antehandler(suite.ctx, tx, false) - suite.Require().Nil(err, "ConsumeTxSizeGasDecorator returned error: %v", err) + // track how much gas is necessary to retrieve parameters + beforeGas := suite.ctx.GasMeter().GasConsumed() + suite.app.AccountKeeper.GetParams(suite.ctx) + afterGas := suite.ctx.GasMeter().GasConsumed() + expectedGas += afterGas - beforeGas - // require that decorator consumes expected amount of gas - consumedGas := suite.ctx.GasMeter().GasConsumed() - beforeGas - suite.Require().Equal(expectedGas, consumedGas, "Decorator did not consume the correct amount of gas") + beforeGas = suite.ctx.GasMeter().GasConsumed() + suite.ctx, err = antehandler(suite.ctx, tx, false) + suite.Require().Nil(err, "ConsumeTxSizeGasDecorator returned error: %v", err) - // simulation must not underestimate gas of this decorator even with nil signatures - txBuilder, err := suite.clientCtx.TxConfig.WrapTxBuilder(tx) - suite.Require().NoError(err) - suite.Require().NoError(txBuilder.SetSignatures(signing.SignatureV2{ - PubKey: priv1.PubKey(), - })) - tx = txBuilder.GetTx() + // require that decorator consumes expected amount of gas + consumedGas := suite.ctx.GasMeter().GasConsumed() - beforeGas + 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 + txBuilder, err := suite.clientCtx.TxConfig.WrapTxBuilder(tx) + suite.Require().NoError(err) + suite.Require().NoError(txBuilder.SetSignatures(tc.sigV2)) + tx = txBuilder.GetTx() - 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") + 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") - // Set suite.ctx with smaller simulated TxBytes manually - suite.ctx = suite.ctx.WithTxBytes(txBytes) + // Set suite.ctx with smaller simulated TxBytes manually + suite.ctx = suite.ctx.WithTxBytes(simTxBytes) - beforeSimGas := suite.ctx.GasMeter().GasConsumed() + beforeSimGas := suite.ctx.GasMeter().GasConsumed() - // run antehandler with simulate=true - suite.ctx, err = antehandler(suite.ctx, tx, true) - consumedSimGas := suite.ctx.GasMeter().GasConsumed() - beforeSimGas + // run antehandler with simulate=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) + + }) + } - // 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 (suite *AnteTestSuite) TestTxHeightTimeoutDecorator() { diff --git a/x/auth/client/cli/cli_test.go b/x/auth/client/cli/cli_test.go index 9e102abda040..8440aea5b002 100644 --- a/x/auth/client/cli/cli_test.go +++ b/x/auth/client/cli/cli_test.go @@ -193,7 +193,9 @@ func (s *IntegrationTestSuite) TestCLISendGenerateSignAndBroadcast() { s.Require().NoError(err) s.Require().Equal(txBuilder.GetTx().GetGas(), uint64(flags.DefaultGasLimit)) s.Require().Equal(len(txBuilder.GetTx().GetMsgs()), 1) - s.Require().Equal(0, len(txBuilder.GetTx().GetSignatures())) + sigs, err := txBuilder.GetTx().GetSignaturesV2() + s.Require().NoError(err) + s.Require().Equal(0, len(sigs)) // Test generate sendTx with --gas=$amount limitedGasGeneratedTx, err := bankcli.MsgSendExec( @@ -215,7 +217,9 @@ func (s *IntegrationTestSuite) TestCLISendGenerateSignAndBroadcast() { s.Require().NoError(err) s.Require().Equal(txBuilder.GetTx().GetGas(), uint64(100)) s.Require().Equal(len(txBuilder.GetTx().GetMsgs()), 1) - s.Require().Equal(0, len(txBuilder.GetTx().GetSignatures())) + sigs, err = txBuilder.GetTx().GetSignaturesV2() + s.Require().NoError(err) + s.Require().Equal(0, len(sigs)) resp, err := bankcli.QueryBalancesExec(val1.ClientCtx, val1.Address) s.Require().NoError(err) @@ -274,7 +278,9 @@ func (s *IntegrationTestSuite) TestCLISendGenerateSignAndBroadcast() { txBuilder, err = val1.ClientCtx.TxConfig.WrapTxBuilder(signedFinalTx) s.Require().NoError(err) s.Require().Equal(len(txBuilder.GetTx().GetMsgs()), 1) - s.Require().Equal(1, len(txBuilder.GetTx().GetSignatures())) + sigs, err = txBuilder.GetTx().GetSignaturesV2() + s.Require().NoError(err) + s.Require().Equal(1, len(sigs)) s.Require().Equal(val1.Address.String(), txBuilder.GetTx().GetSigners()[0].String()) // Write the output to disk diff --git a/x/auth/signing/sig_verifiable_tx.go b/x/auth/signing/sig_verifiable_tx.go index cc7a3935524a..6621d3fcb085 100644 --- a/x/auth/signing/sig_verifiable_tx.go +++ b/x/auth/signing/sig_verifiable_tx.go @@ -13,7 +13,6 @@ type SigVerifiableTx interface { types.Tx GetSigners() []types.AccAddress GetPubKeys() []crypto.PubKey // If signer already has pubkey in context, this list will have nil in its place - GetSignatures() [][]byte GetSignaturesV2() ([]signing.SignatureV2, error) } diff --git a/x/auth/tx/builder.go b/x/auth/tx/builder.go index 4bf3c74f1384..cfb4bc925179 100644 --- a/x/auth/tx/builder.go +++ b/x/auth/tx/builder.go @@ -237,15 +237,23 @@ func (w *wrapper) GetSignaturesV2() ([]signing.SignatureV2, error) { res := make([]signing.SignatureV2, n) for i, si := range signerInfos { - var err error - sigData, err := ModeInfoAndSigToSignatureData(si.ModeInfo, sigs[i]) - if err != nil { - return nil, err - } - res[i] = signing.SignatureV2{ - PubKey: pubKeys[i], - Data: sigData, - Sequence: si.GetSequence(), + // handle nil signatures (in case of simulation) + if si.ModeInfo == nil { + res[i] = signing.SignatureV2{ + PubKey: pubKeys[i], + } + } else { + var err error + sigData, err := ModeInfoAndSigToSignatureData(si.ModeInfo, sigs[i]) + if err != nil { + return nil, err + } + res[i] = signing.SignatureV2{ + PubKey: pubKeys[i], + Data: sigData, + Sequence: si.GetSequence(), + } + } }