increase nonce for reverted contract deployment transaction

Closes: evmos#808 Solution: - Move nonce increaesment from ante handler into `ApplyMessage`. - Handle state revert correct. - More sophsticated unit test for transaction revert behaviour.
yihuang · Dec 3, 2021 · 12acbe4 · 12acbe4
1 parent 4e87775
commit 12acbe4
Show file tree

Hide file tree

Showing 5 changed files with 180 additions and 264 deletions.
diff --git a/app/ante/ante.go b/app/ante/ante.go
@@ -63,7 +63,6 @@ func NewAnteHandler(
 						NewEthNonceVerificationDecorator(ak),
 						NewEthGasConsumeDecorator(evmKeeper),
 						NewCanTransferDecorator(evmKeeper, feeMarketKeeper),
-						NewEthIncrementSenderSequenceDecorator(ak), // innermost AnteDecorator.
 					)
 
 				default:

diff --git a/app/ante/eth.go b/app/ante/eth.go
@@ -385,64 +385,6 @@ func (ctd CanTransferDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate
 	return next(ctx, tx, simulate)
 }
 
-// EthIncrementSenderSequenceDecorator increments the sequence of the signers.
-type EthIncrementSenderSequenceDecorator struct {
-	ak evmtypes.AccountKeeper
-}
-
-// NewEthIncrementSenderSequenceDecorator creates a new EthIncrementSenderSequenceDecorator.
-func NewEthIncrementSenderSequenceDecorator(ak evmtypes.AccountKeeper) EthIncrementSenderSequenceDecorator {
-	return EthIncrementSenderSequenceDecorator{
-		ak: ak,
-	}
-}
-
-// AnteHandle handles incrementing the sequence of the signer (i.e sender). If the transaction is a
-// contract creation, the nonce will be incremented during the transaction execution and not within
-// 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)
-
-			if acc == nil {
-				return ctx, sdkerrors.Wrapf(
-					sdkerrors.ErrUnknownAddress,
-					"account %s (%s) is nil", common.BytesToAddress(addr.Bytes()), addr,
-				)
-			}
-
-			if err := acc.SetSequence(acc.GetSequence() + 1); err != nil {
-				return ctx, sdkerrors.Wrapf(err, "failed to set sequence to %d", acc.GetSequence()+1)
-			}
-
-			issd.ak.SetAccount(ctx, acc)
-		}
-	}
-
-	// set the original gas meter
-	return next(ctx, tx, simulate)
-}
-
 // EthValidateBasicDecorator is adapted from ValidateBasicDecorator from cosmos-sdk, it ignores ErrNoSignatures
 type EthValidateBasicDecorator struct {
 	evmKeeper EVMKeeper

diff --git a/app/ante/eth_test.go b/app/ante/eth_test.go
@@ -376,98 +376,6 @@ func (suite AnteTestSuite) TestCanTransferDecorator() {
 	}
 }
 
-func (suite AnteTestSuite) TestEthIncrementSenderSequenceDecorator() {
-	dec := ante.NewEthIncrementSenderSequenceDecorator(suite.app.AccountKeeper)
-	addr, privKey := tests.NewAddrKey()
-
-	contract := evmtypes.NewTxContract(suite.app.EvmKeeper.ChainID(), 0, big.NewInt(10), 1000, big.NewInt(1), nil, nil, nil, nil)
-	contract.From = addr.Hex()
-
-	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))
-	suite.Require().NoError(err)
-
-	err = tx.Sign(suite.ethSigner, tests.NewSigner(privKey))
-	suite.Require().NoError(err)
-
-	testCases := []struct {
-		name     string
-		tx       sdk.Tx
-		malleate func()
-		expPass  bool
-		expPanic bool
-	}{
-		{
-			"invalid transaction type",
-			&invalidTx{},
-			func() {},
-			false, false,
-		},
-		{
-			"no signers",
-			evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 1, &to, big.NewInt(10), 1000, big.NewInt(1), nil, nil, nil, nil),
-			func() {},
-			false, true,
-		},
-		{
-			"account not set to store",
-			tx,
-			func() {},
-			false, false,
-		},
-		{
-			"success - create contract",
-			contract,
-			func() {
-				acc := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, addr.Bytes())
-				suite.app.AccountKeeper.SetAccount(suite.ctx, acc)
-			},
-			true, false,
-		},
-		{
-			"success - call",
-			tx,
-			func() {},
-			true, false,
-		},
-	}
-
-	for _, tc := range testCases {
-		suite.Run(tc.name, func() {
-			tc.malleate()
-
-			if tc.expPanic {
-				suite.Require().Panics(func() {
-					_, _ = dec.AnteHandle(suite.ctx, tc.tx, false, nextFn)
-				})
-				return
-			}
-
-			_, err := dec.AnteHandle(suite.ctx, tc.tx, false, nextFn)
-
-			if tc.expPass {
-				suite.Require().NoError(err)
-				msg := tc.tx.(*evmtypes.MsgEthereumTx)
-
-				txData, err := evmtypes.UnpackTxData(msg.Data)
-				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)
-				}
-			} else {
-				suite.Require().Error(err)
-			}
-		})
-	}
-}
-
 func (suite AnteTestSuite) TestEthSetupContextDecorator() {
 	dec := ante.NewEthSetUpContextDecorator()
 	tx := evmtypes.NewTxContract(suite.app.EvmKeeper.ChainID(), 1, big.NewInt(10), 1000, big.NewInt(1), nil, nil, nil, nil)