state is not reverted when exception get recovered in smart contract

[yihuang]

System info: ethermint main branch

Steps to reproduce:

Since we don't implement StateDB's Snapshot and RevertToSnapshot methods, which are critical for exception implementation in evm.

Expected behavior: exception should undo all changes made to the state in the current call

Actual behavior: I'm afraid state is not reverted when exception raised and caught
Additional info:

[yihuang]

just confirmed with this test case:

// SPDX-License-Identifier: MIT-Modern-Variant
pragma solidity >=0.8.0;

contract State {
    uint256 a = 0;
    function set(uint256 input) public { 
        a = input; 
        require(a < 10);
    }
    function force_set(uint256 input) public { 
        a = input; 
    }
    function query() public view returns(uint256) {
        return a;
    }
}

contract Consumer {
    State state;
    constructor() {
        state = new State();
    }
    function try_set(uint256 input) public {
        try state.set(input) {
        } catch (bytes memory) {
        }
    }
    function set(uint256 input) public {
        state.force_set(input);
    }
    function query() public view returns(uint256) { 
        return state.query();
    }
}

In geth, the call consumer.try_set(10) succeed, but consumer.query() is 0.
but in ethermint, consumer.query() is 10.

[fedekunze]

can you create a solidity test for the revert logic? I can work on the fix

[fedekunze]

the EVM Call and Create both end with RevertToSnapshot when there is a vmError

func (evm *EVM) EvmFunc {
	// core logic
	if err != nil {
		evm.StateDB.RevertToSnapshot(snapshot)
		if err != ErrExecutionReverted {
			gas = 0
		}
	}
	return ret, gas, err

but then we don't commit the changes unless the state transition is successful:

func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumTxResponse, error) {
	// ...
	cacheCtx, commit := k.ctx.CacheContext()
	
	// ...
	// pass false to execute in real mode, which do actual gas refunding
	res, err := k.ApplyMessage(evm, msg, ethCfg, false)
	if err != nil {
		return nil, stacktrace.Propagate(err, "failed to apply ethereum core message")
	}

	txHash := tx.Hash()
	res.Hash = txHash.Hex()
	logs := k.GetTxLogs(txHash)

	// Commit and switch to original context
	if !res.Failed() {
		commit()
	}
	
	// ...

we should investigate why the state is getting persisted regardless of this condition

[yihuang]

The thing is the Consumer.try_set don't result in a vm error, because the internal error is caught and recovered, but the sub call should be reverted while the tx itself not.

[yihuang]

Added the test case in the PR, it currently fail with ethermint, while success with ganache