use stack of contexts to implement snapshot revert

CHANGELOG.md

@@ -49,6 +49,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (evm) [tharsis#342](https://github.com/tharsis/ethermint/issues/342) Don't clear balance when resetting the account.
 * (evm) [tharsis#334](https://github.com/tharsis/ethermint/pull/334) Log index changed to the index in block rather than
   tx.
+* (evm) [tharsis#399](https://github.com/tharsis/ethermint/pull/399) Exception in sub-message call reverts the call if it's not propagated.
 
 ### API Breaking
 

tests/solidity/suites/exception/contracts/TestRevert.sol

@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: MIT
+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 TestRevert {
+    State state;
+    uint256 b = 0;
+    uint256 c = 0;
+    constructor() {
+        state = new State();
+    }
+    function try_set(uint256 input) public {
+        b = input;
+        try state.set(input) {
+        } catch (bytes memory) {
+        }
+        c = input;
+    }
+    function set(uint256 input) public {
+        state.force_set(input);
+    }
+    function query_a() public view returns(uint256) {
+        return state.query();
+    }
+    function query_b() public view returns(uint256) {
+        return b;
+    }
+    function query_c() public view returns(uint256) {
+        return c;
+    }
+}

tests/solidity/suites/exception/contracts/test/Migrations.sol

@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: MIT
+pragma solidity >=0.8.0;
+
+contract Migrations {
+  address public owner = msg.sender;
+  uint public last_completed_migration;
+
+  modifier restricted() {
+    require(
+      msg.sender == owner,
+      "This function is restricted to the contract's owner"
+    );
+    _;
+  }
+
+  function setCompleted(uint completed) public restricted {
+    last_completed_migration = completed;
+  }
+}

tests/solidity/suites/exception/migrations/1_initial_migration.js

@@ -0,0 +1,5 @@
+const Migrations = artifacts.require("Migrations");
+
+module.exports = function (deployer) {
+  deployer.deploy(Migrations);
+};

tests/solidity/suites/exception/package.json

@@ -0,0 +1,15 @@
+{
+  "name": "exception",
+  "version": "1.0.0",
+  "author": "huangyi <[email protected]>",
+  "license": "GPL-3.0-or-later",
+  "scripts": {
+    "test-ganache": "yarn truffle test",
+    "test-ethermint": "yarn truffle test --network ethermint"
+  },
+  "devDependencies": {
+    "truffle": "^5.1.42",
+    "truffle-assertions": "^0.9.2",
+    "web3": "^1.2.11"
+  }
+}

tests/solidity/suites/exception/test/revert.js

@@ -0,0 +1,35 @@
+const TestRevert = artifacts.require("TestRevert")
+const truffleAssert = require('truffle-assertions');
+
+async function expectRevert(promise) {
+  try {
+    await promise;
+  } catch (error) {
+    if (error.message.indexOf('revert') === -1) {
+      expect('revert').to.equal(error.message, 'Wrong kind of exception received');
+    }
+    return;
+  }
+  expect.fail('Expected an exception but none was received');
+}
+
+contract('TestRevert', (accounts) => {
+  let revert
+
+  beforeEach(async () => {
+    revert = await TestRevert.new()
+  })
+  it('should revert', async () => {
+      await revert.try_set(10)
+      no = await revert.query_a()
+      assert.equal(no, '0', 'The modification on a should be reverted')
+      no = await revert.query_b()
+      assert.equal(no, '10', 'The modification on b should not be reverted')
+      no = await revert.query_c()
+      assert.equal(no, '10', 'The modification on c should not be reverted')
+
+      await revert.set(10)
+      no = await revert.query_a()
+      assert.equal(no, '10', 'The force set should not be reverted')
+  })
+})

tests/solidity/suites/exception/truffle-config.js

@@ -0,0 +1,17 @@
+module.exports = {
+  networks: {
+    // Development network is just left as truffle's default settings
+    ethermint: {
+      host: "127.0.0.1",     // Localhost (default: none)
+      port: 8545,            // Standard Ethereum port (default: none)
+      network_id: "*",       // Any network (default: none)
+      gas: 5000000,          // Gas sent with each transaction
+      gasPrice: 1000000000,  // 1 gwei (in wei)
+    },
+  },
+  compilers: {
+    solc: {
+      version: "0.8.6",
+    },
+  },
+}

x/evm/keeper/context_stack.go

@@ -0,0 +1,87 @@
+package keeper
+
+import (
+	"fmt"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+)
+
+// cachedContext is a pair of cache context and its corresponding commit method.
+// They are obtained from the return value of `context.CacheContext()`.
+type cachedContext struct {
+	ctx    sdk.Context
+	commit func()
+}
+
+// ContextStack manages the initial context and a stack of cached contexts,
+// to support the `StateDB.Snapshot` and `StateDB.RevertToSnapshot` methods.
+type ContextStack struct {
+	// Context of the initial state before transaction execution.
+	// It's the context used by `StateDB.CommitedState`.
+	initialCtx     sdk.Context
+	cachedContexts []cachedContext
+}
+
+// CurrentContext returns the top context of cached stack,
+// if the stack is empty, returns the initial context.
+func (cs *ContextStack) CurrentContext() sdk.Context {
+	l := len(cs.cachedContexts)
+	if l == 0 {
+		return cs.initialCtx
+	}
+	return cs.cachedContexts[l-1].ctx
+}
+
+// Reset sets the initial context and clear the cache context stack.
+func (cs *ContextStack) Reset(ctx sdk.Context) {
+	cs.initialCtx = ctx
+	if len(cs.cachedContexts) > 0 {
+		cs.cachedContexts = []cachedContext{}
+	}
+}
+
+// IsEmpty returns true if the cache context stack is empty.
+func (cs *ContextStack) IsEmpty() bool {
+	return len(cs.cachedContexts) == 0
+}
+
+// Commit commits all the cached contexts from top to bottom in order and clears the stack by setting an empty slice of cache contexts.
+func (cs *ContextStack) Commit() {
+	// commit in order from top to bottom
+	for i := len(cs.cachedContexts) - 1; i >= 0; i-- {
+		// keep all the cosmos events
+		cs.initialCtx.EventManager().EmitEvents(cs.cachedContexts[i].ctx.EventManager().Events())
+		if cs.cachedContexts[i].commit == nil {
+			panic("commit function should not be nil")
+		} else {
+			cs.cachedContexts[i].commit()
+		}
+	}
+	cs.cachedContexts = []cachedContext{}
+}
+
+// Snapshot pushes a new cached context to the stack,
+// and returns the index of it.
+func (cs *ContextStack) Snapshot() int {
+	i := len(cs.cachedContexts)
+	ctx, commit := cs.CurrentContext().CacheContext()
+	cs.cachedContexts = append(cs.cachedContexts, cachedContext{ctx: ctx, commit: commit})
+	return i
+}
+
+// RevertToSnapshot pops all the cached contexts after the target index (inclusive).
+// the target should be snapshot index returned by `Snapshot`.
+// This function panics if the index is out of bounds.
+func (cs *ContextStack) RevertToSnapshot(target int) {
+	if target < 0 || target >= len(cs.cachedContexts) {
+		panic(fmt.Errorf("snapshot index %d out of bound [%d..%d)", target, 0, len(cs.cachedContexts)))
+	}
+	cs.cachedContexts = cs.cachedContexts[:target]
+}
+
+// RevertAll discards all the cache contexts.
+func (cs *ContextStack) RevertAll() {
+	if len(cs.cachedContexts) > 0 {
+		cs.RevertToSnapshot(0)
+	}
+}

x/evm/keeper/grpc_query.go

@@ -265,7 +265,7 @@ func (k Keeper) BlockBloom(c context.Context, req *types.QueryBlockBloomRequest)
 	bloom, found := k.GetBlockBloom(ctx, req.Height)
 	if !found {
 		// if the bloom is not found, query the transient store at the current height
-		k.ctx = ctx
+		k.WithContext(ctx)
 		bloomInt := k.GetBlockBloomTransient()
 
 		if bloomInt.Sign() == 0 {
@@ -381,6 +381,7 @@ func (k Keeper) EthCall(c context.Context, req *types.EthCallRequest) (*types.Ms
 	evm := k.NewEVM(msg, ethCfg, params, coinbase)
 	// pass true means execute in query mode, which don't do actual gas refund.
 	res, err := k.ApplyMessage(evm, msg, ethCfg, true)
+	k.ctxStack.RevertAll()
 	if err != nil {
 		return nil, status.Error(codes.Internal, err.Error())
 	}
@@ -443,15 +444,15 @@ func (k Keeper) EstimateGas(c context.Context, req *types.EthCallRequest) (*type
 	executable := func(gas uint64) (bool, *types.MsgEthereumTxResponse, error) {
 		args.Gas = (*hexutil.Uint64)(&gas)
 
-		// Execute the call in an isolated context
-		k.BeginCachedContext()
+		// Reset to the initial context
+		k.WithContext(ctx)
 
 		msg := args.ToMessage(req.GasCap)
 		evm := k.NewEVM(msg, ethCfg, params, coinbase)
 		// pass true means execute in query mode, which don't do actual gas refund.
 		rsp, err := k.ApplyMessage(evm, msg, ethCfg, true)
 
-		k.EndCachedContext()
+		k.ctxStack.RevertAll()
 
 		if err != nil {
 			if errors.Is(stacktrace.RootCause(err), core.ErrIntrinsicGas) {