Skip to content
This repository has been archived by the owner on Jun 9, 2024. It is now read-only.

fix(precompile): Fix regression where eth_call can cause non-deterministic gas consumption #1136

Merged
merged 12 commits into from
Sep 24, 2023

Conversation

itsdevbear
Copy link

@itsdevbear itsdevbear commented Sep 24, 2023

You cannot manipulate the canonical state plugin in another plugin since there is no guarantee made about whether it is in a query or a block. Also setting gas consumption in the state plugin is not required, since the context with the updated gas only occurs in the precompile itself.

This can cause non-determinism in gas consumption between nodes.

@itsdevbear itsdevbear requested a review from calbera September 24, 2023 01:39
@codecov
Copy link

codecov bot commented Sep 24, 2023

Codecov Report

Merging #1136 (2a73ec8) into main (6a92c39) will decrease coverage by 0.08%.
The diff coverage is 73.68%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1136      +/-   ##
==========================================
- Coverage   54.30%   54.22%   -0.08%     
==========================================
  Files          99       99              
  Lines        5121     5123       +2     
==========================================
- Hits         2781     2778       -3     
- Misses       2173     2178       +5     
  Partials      167      167              
Files Changed Coverage Δ
cosmos/x/evm/plugins/block/plugin.go 35.00% <ø> (ø)
cosmos/x/evm/plugins/configuration/plugin.go 27.27% <ø> (ø)
cosmos/x/evm/plugins/gas/plugin.go 76.19% <ø> (ø)
cosmos/x/evm/plugins/historical/plugin.go 100.00% <ø> (ø)
cosmos/x/evm/plugins/state/plugin.go 73.96% <ø> (ø)
cosmos/x/evm/plugins/txpool/plugin.go 0.00% <ø> (ø)
eth/core/precompile/default_plugin.go 29.54% <0.00%> (-2.17%) ⬇️
eth/core/state/statedb.go 53.06% <0.00%> (-1.11%) ⬇️
cosmos/x/evm/keeper/host.go 100.00% <100.00%> (ø)
cosmos/x/evm/plugins/precompile/plugin.go 82.02% <100.00%> (-0.59%) ⬇️

@itsdevbear itsdevbear changed the title fix(precompile): Fix regression where eth_call can cause issues with block production fix(precompile): Fix regression where eth_call can cause non-deterministic gas consumption Sep 24, 2023
@itsdevbear
Copy link
Author

fwiw, I really hate this design, but it will get us unblocked for testnet

@itsdevbear itsdevbear requested review from calbera and ocnc September 24, 2023 03:10
@itsdevbear
Copy link
Author

We can chat monday, but I really think we should just simplify and prevent precompiles from calling back into the EVM. This would reduce so much complexity fromt the precompile plugin and stop just gross interface contagion. I think the fact that the precompile plugin is manually calling the state plugin is a huge antipattern and generally bad practice. Plugins in general, shouldnt need to know about the existance of other plugins.

@itsdevbear
Copy link
Author

@calbera @ocnc

Copy link
Contributor

@transmissions12 transmissions12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm thanks for addressing it @itsdevbear

@itsdevbear itsdevbear merged commit 15c1356 into main Sep 24, 2023
@itsdevbear itsdevbear deleted the fix-gas-consumption branch September 24, 2023 16:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants