increase nonce in ante handler for contract creation tx

Closes: evmos#808 Solution: - move nonce increment to ante handler - revert nonce increment in apply message
yihuang · Dec 10, 2021 · 767244a · 767244a
1 parent 9219fb7
commit 767244a
Show file tree

Hide file tree

Showing 7 changed files with 189 additions and 117 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -37,6 +37,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## Unreleased
 
+### State Machine Breaking
+
+- (evm) [tharsis#808](https://github.com/tharsis/ethermint/issues/808) increase nonce in ante handler for contract creation transaction.
+
 ## [v0.9.0] - 2021-12-01
 
 ### State Machine Breaking

diff --git a/app/ante/ante_test.go b/app/ante/ante_test.go
@@ -60,7 +60,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
 			func() sdk.Tx {
 				signedContractTx := evmtypes.NewTxContract(
 					suite.app.EvmKeeper.ChainID(),
-					1,
+					2,
 					big.NewInt(10),
 					100000,
 					big.NewInt(150),
@@ -81,7 +81,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
 			func() sdk.Tx {
 				signedContractTx := evmtypes.NewTxContract(
 					suite.app.EvmKeeper.ChainID(),
-					1,
+					3,
 					big.NewInt(10),
 					100000,
 					big.NewInt(150),
@@ -102,7 +102,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
 			func() sdk.Tx {
 				signedTx := evmtypes.NewTx(
 					suite.app.EvmKeeper.ChainID(),
-					1,
+					4,
 					&to,
 					big.NewInt(10),
 					100000,
@@ -124,7 +124,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
 			func() sdk.Tx {
 				signedTx := evmtypes.NewTx(
 					suite.app.EvmKeeper.ChainID(),
-					2,
+					5,
 					&to,
 					big.NewInt(10),
 					100000,
@@ -351,7 +351,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
 				signedContractTx :=
 					evmtypes.NewTxContract(
 						suite.app.EvmKeeper.ChainID(),
-						1,
+						2,
 						big.NewInt(10),
 						100000,
 						nil,
@@ -373,7 +373,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
 				signedContractTx :=
 					evmtypes.NewTxContract(
 						suite.app.EvmKeeper.ChainID(),
-						1,
+						3,
 						big.NewInt(10),
 						100000,
 						nil,
@@ -395,7 +395,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
 				signedTx :=
 					evmtypes.NewTx(
 						suite.app.EvmKeeper.ChainID(),
-						1,
+						4,
 						&to,
 						big.NewInt(10),
 						100000,
@@ -418,7 +418,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
 				signedTx :=
 					evmtypes.NewTx(
 						suite.app.EvmKeeper.ChainID(),
-						2,
+						5,
 						&to,
 						big.NewInt(10),
 						100000,

diff --git a/app/ante/eth.go b/app/ante/eth.go
@@ -402,24 +402,6 @@ func NewEthIncrementSenderSequenceDecorator(ak evmtypes.AccountKeeper) EthIncrem
 // this AnteHandler decorator.
 func (issd EthIncrementSenderSequenceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) {
 	for _, msg := range tx.GetMsgs() {
-		msgEthTx, ok := msg.(*evmtypes.MsgEthereumTx)
-		if !ok {
-			return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid transaction type %T, expected %T", tx, (*evmtypes.MsgEthereumTx)(nil))
-		}
-
-		txData, err := evmtypes.UnpackTxData(msgEthTx.Data)
-		if err != nil {
-			return ctx, sdkerrors.Wrap(err, "failed to unpack tx data")
-		}
-
-		// NOTE: on contract creation, the nonce is incremented within the EVM Create function during tx execution
-		// and not previous to the state transition ¯\_(ツ)_/¯
-		if txData.GetTo() == nil {
-			// contract creation, don't increment sequence on AnteHandler but on tx execution
-			// continue to the next item
-			continue
-		}
-
 		// increment sequence of all signers
 		for _, addr := range msg.GetSigners() {
 			acc := issd.ak.GetAccount(ctx, addr)
@@ -439,7 +421,6 @@ func (issd EthIncrementSenderSequenceDecorator) AnteHandle(ctx sdk.Context, tx s
 		}
 	}
 
-	// set the original gas meter
 	return next(ctx, tx, simulate)
 }
 

diff --git a/app/ante/eth_test.go b/app/ante/eth_test.go
@@ -382,15 +382,18 @@ func (suite AnteTestSuite) TestEthIncrementSenderSequenceDecorator() {
 
 	contract := evmtypes.NewTxContract(suite.app.EvmKeeper.ChainID(), 0, big.NewInt(10), 1000, big.NewInt(1), nil, nil, nil, nil)
 	contract.From = addr.Hex()
+	err := contract.Sign(suite.ethSigner, tests.NewSigner(privKey))
+	suite.Require().NoError(err)
 
 	to := tests.GenerateAddress()
 	tx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 0, &to, big.NewInt(10), 1000, big.NewInt(1), nil, nil, nil, nil)
 	tx.From = addr.Hex()
-
-	err := contract.Sign(suite.ethSigner, tests.NewSigner(privKey))
+	err = tx.Sign(suite.ethSigner, tests.NewSigner(privKey))
 	suite.Require().NoError(err)
 
-	err = tx.Sign(suite.ethSigner, tests.NewSigner(privKey))
+	tx2 := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 1, &to, big.NewInt(10), 1000, big.NewInt(1), nil, nil, nil, nil)
+	tx2.From = addr.Hex()
+	err = tx2.Sign(suite.ethSigner, tests.NewSigner(privKey))
 	suite.Require().NoError(err)
 
 	testCases := []struct {
@@ -404,7 +407,7 @@ func (suite AnteTestSuite) TestEthIncrementSenderSequenceDecorator() {
 			"invalid transaction type",
 			&invalidTx{},
 			func() {},
-			false, false,
+			false, true,
 		},
 		{
 			"no signers",
@@ -429,7 +432,7 @@ func (suite AnteTestSuite) TestEthIncrementSenderSequenceDecorator() {
 		},
 		{
 			"success - call",
-			tx,
+			tx2,
 			func() {},
 			true, false,
 		},
@@ -456,11 +459,7 @@ func (suite AnteTestSuite) TestEthIncrementSenderSequenceDecorator() {
 				suite.Require().NoError(err)
 
 				nonce := suite.app.EvmKeeper.GetNonce(addr)
-				if txData.GetTo() == nil {
-					suite.Require().Equal(txData.GetNonce(), nonce)
-				} else {
-					suite.Require().Equal(txData.GetNonce()+1, nonce)
-				}
+				suite.Require().Equal(txData.GetNonce()+1, nonce)
 			} else {
 				suite.Require().Error(err)
 			}