From 91ad6387bfbc5ab86e638485881bd7ccf02c0f66 Mon Sep 17 00:00:00 2001 From: yihuang Date: Mon, 21 Jun 2021 18:44:37 +0800 Subject: [PATCH] ante: ignore no signature error for evm transactions (#143) fixes #118 don't sign unit test tx since that's the same case as real tx comment EthValidateBasicDecorator --- app/ante/ante.go | 2 +- app/ante/ante_test.go | 40 ++++++++++++++++----- app/ante/eth.go | 24 +++++++++++++ app/ante/utils_test.go | 79 +++++++++++++++++++++-------------------- ethereum/rpc/eth_api.go | 4 +++ 5 files changed, 100 insertions(+), 49 deletions(-) diff --git a/app/ante/ante.go b/app/ante/ante.go index 55425583..d4552220 100644 --- a/app/ante/ante.go +++ b/app/ante/ante.go @@ -62,9 +62,9 @@ func NewAnteHandler( anteHandler = sdk.ChainAnteDecorators( authante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first authante.NewMempoolFeeDecorator(), - authante.NewValidateBasicDecorator(), authante.TxTimeoutHeightDecorator{}, authante.NewValidateMemoDecorator(ak), + NewEthValidateBasicDecorator(), NewEthSigVerificationDecorator(evmKeeper), NewEthAccountVerificationDecorator(ak, bankKeeper, evmKeeper), NewEthNonceVerificationDecorator(ak), diff --git a/app/ante/ante_test.go b/app/ante/ante_test.go index cbac5211..a66d46fe 100644 --- a/app/ante/ante_test.go +++ b/app/ante/ante_test.go @@ -33,7 +33,7 @@ func (suite AnteTestSuite) TestAnteHandler() { signedContractTx := evmtypes.NewMsgEthereumTxContract(suite.app.EvmKeeper.ChainID(), 1, big.NewInt(10), 100000, big.NewInt(1), nil, nil) signedContractTx.From = addr.Hex() - tx := suite.CreateTestTx(signedContractTx, privKey, 1) + tx := suite.CreateTestTx(signedContractTx, privKey, 1, true) return tx }, false, false, true, @@ -44,7 +44,7 @@ func (suite AnteTestSuite) TestAnteHandler() { signedContractTx := evmtypes.NewMsgEthereumTxContract(suite.app.EvmKeeper.ChainID(), 1, big.NewInt(10), 100000, big.NewInt(1), nil, nil) signedContractTx.From = addr.Hex() - tx := suite.CreateTestTx(signedContractTx, privKey, 1) + tx := suite.CreateTestTx(signedContractTx, privKey, 1, true) return tx }, true, false, true, @@ -55,7 +55,7 @@ func (suite AnteTestSuite) TestAnteHandler() { signedContractTx := evmtypes.NewMsgEthereumTxContract(suite.app.EvmKeeper.ChainID(), 1, big.NewInt(10), 100000, big.NewInt(1), nil, nil) signedContractTx.From = addr.Hex() - tx := suite.CreateTestTx(signedContractTx, privKey, 1) + tx := suite.CreateTestTx(signedContractTx, privKey, 1, true) return tx }, false, true, true, @@ -66,7 +66,7 @@ func (suite AnteTestSuite) TestAnteHandler() { signedTx := evmtypes.NewMsgEthereumTx(suite.app.EvmKeeper.ChainID(), 1, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil) signedTx.From = addr.Hex() - tx := suite.CreateTestTx(signedTx, privKey, 1) + tx := suite.CreateTestTx(signedTx, privKey, 1, true) return tx }, false, false, true, @@ -77,7 +77,7 @@ func (suite AnteTestSuite) TestAnteHandler() { signedTx := evmtypes.NewMsgEthereumTx(suite.app.EvmKeeper.ChainID(), 2, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil) signedTx.From = addr.Hex() - tx := suite.CreateTestTx(signedTx, privKey, 1) + tx := suite.CreateTestTx(signedTx, privKey, 1, true) return tx }, true, false, true, @@ -88,17 +88,39 @@ func (suite AnteTestSuite) TestAnteHandler() { signedTx := evmtypes.NewMsgEthereumTx(suite.app.EvmKeeper.ChainID(), 3, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil) signedTx.From = addr.Hex() - tx := suite.CreateTestTx(signedTx, privKey, 1) + tx := suite.CreateTestTx(signedTx, privKey, 1, true) return tx }, false, true, true, }, { - "fail - CheckTx (memo too long)", + "success - CheckTx (cosmos tx not signed)", func() sdk.Tx { - signedTx := evmtypes.NewMsgEthereumTx(suite.app.EvmKeeper.ChainID(), 3, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil) + signedTx := evmtypes.NewMsgEthereumTx(suite.app.EvmKeeper.ChainID(), 4, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil) signedTx.From = addr.Hex() - txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1) + tx := suite.CreateTestTx(signedTx, privKey, 1, false) + return tx + }, false, true, true, + }, + { + "fail - CheckTx (cosmos tx is not valid)", + func() sdk.Tx { + signedTx := evmtypes.NewMsgEthereumTx(suite.app.EvmKeeper.ChainID(), 4, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil) + signedTx.From = addr.Hex() + + txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false) + // bigger than MaxGasWanted + txBuilder.SetGasLimit(uint64(1 << 63)) + return txBuilder.GetTx() + }, false, true, false, + }, + { + "fail - CheckTx (memo too long)", + func() sdk.Tx { + signedTx := evmtypes.NewMsgEthereumTx(suite.app.EvmKeeper.ChainID(), 5, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil) + signedTx.From = addr.Hex() + + txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, true) txBuilder.SetMemo(strings.Repeat("*", 257)) return txBuilder.GetTx() }, true, false, false, diff --git a/app/ante/eth.go b/app/ante/eth.go index d0eeb95e..9314b868 100644 --- a/app/ante/eth.go +++ b/app/ante/eth.go @@ -549,3 +549,27 @@ func (issd EthIncrementSenderSequenceDecorator) AnteHandle(ctx sdk.Context, tx s // set the original gas meter return next(ctx, tx, simulate) } + +// EthValidateBasicDecorator is adapted from ValidateBasicDecorator from cosmos-sdk, it ignores ErrNoSignatures +type EthValidateBasicDecorator struct{} + +// NewEthValidateBasicDecorator creates a new EthValidateBasicDecorator +func NewEthValidateBasicDecorator() EthValidateBasicDecorator { + return EthValidateBasicDecorator{} +} + +// AnteHandle handles basic validation of tx +func (vbd EthValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { + // no need to validate basic on recheck tx, call next antehandler + if ctx.IsReCheckTx() { + return next(ctx, tx, simulate) + } + + err := tx.ValidateBasic() + // ErrNoSignatures is fine with eth tx + if err != nil && err != sdkerrors.ErrNoSignatures { + return ctx, err + } + + return next(ctx, tx, simulate) +} diff --git a/app/ante/utils_test.go b/app/ante/utils_test.go index 31589f8f..049dea30 100644 --- a/app/ante/utils_test.go +++ b/app/ante/utils_test.go @@ -37,7 +37,6 @@ type AnteTestSuite struct { ctx sdk.Context app *app.EthermintApp clientCtx client.Context - txBuilder client.TxBuilder anteHandler sdk.AnteHandler ethSigner ethtypes.Signer } @@ -59,7 +58,6 @@ func (suite *AnteTestSuite) SetupTest() { encodingConfig.Amino.RegisterConcrete(&testdata.TestMsg{}, "testdata.TestMsg", nil) suite.clientCtx = client.Context{}.WithTxConfig(encodingConfig.TxConfig) - suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() suite.anteHandler = ante.NewAnteHandler(suite.app.AccountKeeper, suite.app.BankKeeper, suite.app.EvmKeeper, encodingConfig.TxConfig.SignModeHandler()) suite.ethSigner = ethtypes.LatestSignerForChainID(suite.app.EvmKeeper.ChainID()) @@ -71,20 +69,21 @@ func TestAnteTestSuite(t *testing.T) { // CreateTestTx is a helper function to create a tx given multiple inputs. func (suite *AnteTestSuite) CreateTestTx( - msg *evmtypes.MsgEthereumTx, priv cryptotypes.PrivKey, accNum uint64, + msg *evmtypes.MsgEthereumTx, priv cryptotypes.PrivKey, accNum uint64, signCosmosTx bool, ) authsigning.Tx { - return suite.CreateTestTxBuilder(msg, priv, accNum).GetTx() + return suite.CreateTestTxBuilder(msg, priv, accNum, signCosmosTx).GetTx() } // CreateTestTxBuilder is a helper function to create a tx builder given multiple inputs. func (suite *AnteTestSuite) CreateTestTxBuilder( - msg *evmtypes.MsgEthereumTx, priv cryptotypes.PrivKey, accNum uint64, + msg *evmtypes.MsgEthereumTx, priv cryptotypes.PrivKey, accNum uint64, signCosmosTx bool, ) client.TxBuilder { option, err := codectypes.NewAnyWithValue(&evmtypes.ExtensionOptionsEthereumTx{}) suite.Require().NoError(err) - builder, ok := suite.txBuilder.(authtx.ExtensionOptionsTxBuilder) + txBuilder := suite.clientCtx.TxConfig.NewTxBuilder() + builder, ok := txBuilder.(authtx.ExtensionOptionsTxBuilder) suite.Require().True(ok) builder.SetExtensionOptions(option) @@ -98,41 +97,43 @@ func (suite *AnteTestSuite) CreateTestTxBuilder( builder.SetFeeAmount(fees) builder.SetGasLimit(msg.GetGas()) - // First round: we gather all the signer infos. We use the "set empty - // signature" hack to do that. - sigV2 := signing.SignatureV2{ - PubKey: priv.PubKey(), - Data: &signing.SingleSignatureData{ - SignMode: suite.clientCtx.TxConfig.SignModeHandler().DefaultMode(), - Signature: nil, - }, - Sequence: msg.Data.Nonce, + if signCosmosTx { + // First round: we gather all the signer infos. We use the "set empty + // signature" hack to do that. + sigV2 := signing.SignatureV2{ + PubKey: priv.PubKey(), + Data: &signing.SingleSignatureData{ + SignMode: suite.clientCtx.TxConfig.SignModeHandler().DefaultMode(), + Signature: nil, + }, + Sequence: msg.Data.Nonce, + } + + sigsV2 := []signing.SignatureV2{sigV2} + + err = txBuilder.SetSignatures(sigsV2...) + suite.Require().NoError(err) + + // Second round: all signer infos are set, so each signer can sign. + + signerData := authsigning.SignerData{ + ChainID: suite.ctx.ChainID(), + AccountNumber: accNum, + Sequence: msg.Data.Nonce, + } + sigV2, err = tx.SignWithPrivKey( + suite.clientCtx.TxConfig.SignModeHandler().DefaultMode(), signerData, + txBuilder, priv, suite.clientCtx.TxConfig, msg.Data.Nonce, + ) + suite.Require().NoError(err) + + sigsV2 = []signing.SignatureV2{sigV2} + + err = txBuilder.SetSignatures(sigsV2...) + suite.Require().NoError(err) } - sigsV2 := []signing.SignatureV2{sigV2} - - err = suite.txBuilder.SetSignatures(sigsV2...) - suite.Require().NoError(err) - - // Second round: all signer infos are set, so each signer can sign. - - signerData := authsigning.SignerData{ - ChainID: suite.ctx.ChainID(), - AccountNumber: accNum, - Sequence: msg.Data.Nonce, - } - sigV2, err = tx.SignWithPrivKey( - suite.clientCtx.TxConfig.SignModeHandler().DefaultMode(), signerData, - suite.txBuilder, priv, suite.clientCtx.TxConfig, msg.Data.Nonce, - ) - suite.Require().NoError(err) - - sigsV2 = []signing.SignatureV2{sigV2} - - err = suite.txBuilder.SetSignatures(sigsV2...) - suite.Require().NoError(err) - - return suite.txBuilder + return txBuilder } func newTestAddrKey() (common.Address, cryptotypes.PrivKey) { diff --git a/ethereum/rpc/eth_api.go b/ethereum/rpc/eth_api.go index ab3ed39b..2924ea9b 100644 --- a/ethereum/rpc/eth_api.go +++ b/ethereum/rpc/eth_api.go @@ -550,6 +550,10 @@ func (e *PublicEthAPI) doCall( // Create new call message msg := evmtypes.NewMsgEthereumTx(e.chainIDEpoch, seq, args.To, value, gas, gasPrice, data, accessList) msg.From = args.From.String() + signer := ethtypes.LatestSignerForChainID(e.chainIDEpoch) + if err := msg.Sign(signer, e.clientCtx.Keyring); err != nil { + return nil, err + } if err := msg.ValidateBasic(); err != nil { return nil, err