Problem: EVM keeper keeps intermediate states inside

[yihuang]

Closes: #664

Description

• Migrate the StateDB methods to keeper style, aka. passing sdk.Context explicitly, returns error when needed. see StateDBKeeper
• Create another StateDB struct to handle ctxStack and stateErr and implements Snapshot/RevertSnapshot/GetCommittedState, delegate all the other StateDB methods to the keeper.
• ApplyMessage create StateDB struct on the fly, either commit or discard it before return.

---

For contributor use:

• Targeted PR against correct branch (see CONTRIBUTING.md)
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Code follows the module structure standards.
• Wrote unit and integration tests
• Updated relevant documentation (docs/) or specification (x/<module>/spec/)
• Added relevant godoc comments.
• Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
• Re-reviewed Files changed in the Github PR explorer

---

For admin use:

• Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
• Reviewers assigned
• Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

[codecov]

Codecov Report

Merging #663 (c5719f7) into main (32c905a) will decrease coverage by 0.38%.
The diff coverage is 43.27%.

❗ Current head c5719f7 differs from pull request most recent head ef96e84. Consider uploading reports for the commit ef96e84 to get more accurate results

@@            Coverage Diff             @@
##             main     #663      +/-   ##
==========================================
- Coverage   55.81%   55.42%   -0.39%     
==========================================
  Files          64       64              
  Lines        5853     5470     -383     
==========================================
- Hits         3267     3032     -235     
+ Misses       2386     2301      -85     
+ Partials      200      137      -63     

Impacted Files	Coverage Δ
x/evm/keeper/abci.go	20.00% <0.00%> (-1.43%)	⬇️
x/evm/types/context_stack.go	0.00% <0.00%> (ø)
x/evm/types/statedb.go	0.00% <0.00%> (ø)
x/evm/genesis.go	45.28% <25.00%> (-2.94%)	⬇️
x/evm/keeper/keeper.go	74.09% <82.97%> (-1.91%)	⬇️
x/evm/keeper/state_transition.go	77.06% <88.63%> (+0.62%)	⬆️
x/evm/keeper/grpc_query.go	61.73% <91.66%> (-4.36%)	⬇️
x/evm/keeper/statedb.go	85.67% <91.89%> (+12.41%)	⬆️
app/ante/eth.go	87.84% <100.00%> (+5.56%)	⬆️
x/evm/keeper/msg_server.go	73.07% <100.00%> (-0.51%)	⬇️
... and 22 more

[yihuang]

hold it for now, since there are another refactoring PR ongoing, after that one fixed, this one will become cleaner.

[yihuang]

hold it for now, since there are another refactoring PR ongoing, after that one fixed, this one will become cleaner.

rebased, it's much cleaner now.

[yihuang]

• The test-solidity test failure also appears in the main branch, same error message.
• The test-rpc test failure doesn't reproduce locally, may be an accidental one.

[yihuang]

The test-rpc failure also happens on main branch: https://github.com/tharsis/ethermint/actions/runs/1375993842, probably just need a bigger timeout value.

[fedekunze]

I'm seeing a huge regression (>100%) on performance for this PR. We should tackle that before merging this

[yihuang]

I'm seeing a huge regression (>100%) on performance for this PR. We should tackle that before merging this

It seems like accidental, there's no regression now after merged main branch and rerun.

[yihuang]

But the estimateGas nondeterministic issue seems still exists: https://github.com/tharsis/ethermint/runs/4011483380?check_suite_focus=true

[fedekunze]

@yihuang sorry about the delay, I'll look into this in-depth during the next couple of days

[yihuang]

@yihuang sorry about the delay, I'll look into this in-depth during the next couple of days

Can you have a look at this issue: #710. Deep context stack is still slow, to fix that, we might need more code refactoring, journal logs?

[yihuang]

Closing now, I think it's better to review together here: #729.