Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reuse existing EVM instance in interop calls #1731

Merged
merged 2 commits into from
Jul 22, 2024
Merged

Reuse existing EVM instance in interop calls #1731

merged 2 commits into from
Jul 22, 2024

Conversation

codchen
Copy link
Collaborator

@codchen codchen commented Jun 13, 2024

Describe your changes and provide context

Using new EVM instance for interop calls can cause discrepancies in behavior against ethereum. This PR changes the behavior to be reusing existing EVM instance, with an additional tweak which returns the latest sdk.Context from the evm instance to the wasmd message handler, so that there will never be a scenario where messages dispatched by a wasm call operate on different sdk.Context.

Testing performed to validate your change

existing integration test should still work

Copy link

codecov bot commented Jun 13, 2024

Codecov Report

Attention: Patch coverage is 69.23077% with 20 lines in your changes missing coverage. Please review.

Project coverage is 61.39%. Comparing base (bdf72f2) to head (01677c5).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1731      +/-   ##
==========================================
- Coverage   61.52%   61.39%   -0.14%     
==========================================
  Files         255      256       +1     
  Lines       22122    22162      +40     
==========================================
- Hits        13611    13606       -5     
- Misses       7556     7603      +47     
+ Partials      955      953       -2     
Files Coverage Δ
evmrpc/simulate.go 65.23% <100.00%> (+0.50%) ⬆️
x/evm/ante/fee.go 64.47% <100.00%> (+0.47%) ⬆️
x/evm/keeper/msg_server.go 76.05% <100.00%> (+0.10%) ⬆️
x/evm/state/statedb.go 52.33% <100.00%> (+3.31%) ⬆️
x/evm/types/context.go 100.00% <100.00%> (ø)
x/evm/keeper/evm.go 66.45% <51.21%> (-0.22%) ⬇️

... and 1 file with indirect coverage changes

@codchen codchen force-pushed the wasmd-return-ctx branch 5 times, most recently from 3cf6d41 to fed14b5 Compare June 19, 2024 02:59
@codchen codchen force-pushed the wasmd-return-ctx branch 3 times, most recently from 42caff6 to 73b3993 Compare July 18, 2024 04:34
@codchen codchen force-pushed the wasmd-return-ctx branch from 73b3993 to da05c88 Compare July 22, 2024 03:35
@codchen codchen force-pushed the wasmd-return-ctx branch from da05c88 to 01677c5 Compare July 22, 2024 03:39
@codchen codchen merged commit f285e6f into main Jul 22, 2024
48 checks passed
@codchen codchen deleted the wasmd-return-ctx branch July 22, 2024 03:51
codchen added a commit that referenced this pull request Jul 24, 2024
philipsu522 pushed a commit that referenced this pull request Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants