Sanitizing State Manager Gas #212

ben-chain · 2020-08-12T00:49:17Z

Description

This PR implements a proxy contract which sits between the EM and SM. The proxy serves two purposes:

Burning any leftover gas from the calls to the SM. This makes the OVM's gas behavior more deterministic, so that different SM implementations do not change downstream results of the GAS opcode in code contracts.
Records all gas consumed by the proxied calls to the SM in storage. This allows the EM to "refund" the overhead of the PSM, so that the gas we meter in the OVM is based on L2 execution costs and not the L1 costs.

Questions

How should we configure the large set of constants in the Gas Sanitizer?

Metadata

Fixes

Fixes YAS 422

Contributing Agreement

I have read and understood the Optimism Contributing Guide and Code of Conduct and am following those guidelines in this pull request.

…-determinism

willmeister

Left a few comments, but LGTM 👍

Might also suggest @karlfloersch to take a look, as this is right at the core of our stuff.

packages/contracts/contracts/optimistic-ethereum/ovm/ExecutionManager.sol

willmeister · 2020-08-12T16:40:29Z

packages/contracts/contracts/optimistic-ethereum/ovm/StateManagerGasSanitizer.sol

+
+    // Internal Logic
+    function addToOVMRefund(uint _refund) internal {
+        OVMRefund += _refund;


Do we need to require(MAX_UINT - OVMRefund >= _refund, "overflow");? Based on context, we may not care.

I don't think it's possible for any reasonable value for the constants at the top of the file, simply due to the EVM's gas limit.

packages/contracts/contracts/optimistic-ethereum/ovm/StateManagerGasSanitizer.sol

willmeister · 2020-08-12T16:49:13Z

packages/contracts/contracts/optimistic-ethereum/ovm/StateManagerGasSanitizer.sol

+        // todo safemath negatives
+        GasConsumer gasConsumer = GasConsumer(resolveGasConsumer());
+        uint gasAlreadyConsumed = initialGas - gasleft();
+        uint gasLeftToConsume = _sanitizedGasCost - gasAlreadyConsumed;


Just want to make sure it's not possible for this to underflow.

We already have to make an assumption about the max gas an SM implementation will consume, which is why the constants above are called *_GAS_COST_UPPER_BOUND. If that breaks, then we will have bigger issues than the underflow here, so I figured it's fine.

willmeister · 2020-08-12T16:51:04Z

packages/contracts/contracts/optimistic-ethereum/ovm/StateManagerGasSanitizer.sol

+        uint _sanitizedGasCost,
+        uint _virtualGasCost
+    ) internal {
+        uint initialGas = gasleft();


Should there be a require(_sanitizedGasCost >= _virtualGasCost, "Logic will underflow if sanitized gas cost is lower than virtual gas cost"); ?

Since those are parameters/constants we must set ourselves, I think okay to omit such a check.

willmeister · 2020-08-12T16:53:36Z

packages/contracts/contracts/optimistic-ethereum/utils/libraries/GasConsumer.sol

+
+    // Overhead for checking methodId etc in this function before the actual call()
+    // This was figured out empirically during testing--adding methods or changing compiler settings will require recalibration.
+    uint constant constantOverheadEOA = 947;


Will this ever change through hardforks, and if so, how can we update it?

It could, but it's a case where other critical components will break first. In this particular instance I believe the impact would be a constant size change in OVM gas consumed, but still deterministic.

willmeister · 2020-08-12T16:54:25Z

packages/contracts/contracts/optimistic-ethereum/utils/libraries/GasConsumer.sol

-        uint gasToAlloc = _amount - constantOverhead;
+    // Overhead for checking methodId, etc. in this function before the actual call()
+    // This was figured out empirically during testing--adding methods or changing compiler settings will require recalibration.
+    uint constant constantOverheadInternal = 2514;


Same about updating

packages/contracts/contracts/optimistic-ethereum/utils/libraries/GasConsumer.sol

ben-chain · 2020-08-25T05:16:44Z

This PR didn't get in until some big upstream changes, closing in favor of #217 which built on top of this PR and then merged.

…m-optimism#212)

### Description Documents the `op-alloy-consensus` crate in the mdbook.

Closes #212 --------- Co-authored-by: Emilia Hane <[email protected]>

ben-chain added 24 commits August 6, 2020 18:20

add internal call gas consumer

7a47f43

update changed EM gas overhead constant

d52186c

add first pass of SM gas virtualization proxy

a852295

finish proxy unit tests

fbf1be0

tests failing but EM updated

d4f583d

old gas metering tests passing with proxy

e0d8e31

add gas consuming proxy, not working

17615f9

updates, need to refactor :(

f37cac1

first pass gas burn working

7a04554

more tests passing

f4686a2

all new gas tests passing

d44542c

cleaning

040d618

more cleanup, linting

cd06162

fixes, updating deployments

109c662

linting, renaming

40d1882

pre fmsuf

a67316e

fix state transitioner tests

d3ff9ec

linting

cc920e5

remove .only

aa84a07

add back EVM precompile association

26d9408

linting

384e9b2

fix most fullnode tests, skipping events

81c2509

linting, cleanup

74114ad

Merge remote-tracking branch 'origin' into feat/YAS422/enforce-SM-gas…

6bb7bf4

…-determinism

willmeister approved these changes Aug 12, 2020

View reviewed changes

skip all fullnode tests

3ecebe8

This was referenced Aug 13, 2020

[Error] ethereum/go-ethereum#21444

Closed

Remove broken gas metering attempt ethereum-optimism/go-ethereum#12

Closed

ben-chain mentioned this pull request Aug 17, 2020

YAS 430 OVM gasLimit-proportional burn for rate limiting #217

Merged

1 task

ben-chain closed this Aug 25, 2020

gakonst deleted the feat/YAS422/enforce-SM-gas-determinism branch March 18, 2021 15:02

snario pushed a commit that referenced this pull request Apr 14, 2021

Add ERC1820 Registry as a precompile (#212)

8f1e355

bap2pecs pushed a commit to babylonlabs-io/optimism that referenced this pull request Jul 31, 2024

chore: Remove dependency on covenant-emulator (ethereum-optimism#212)

d9a390f

xibao-nr pushed a commit to node-real/combo-optimism that referenced this pull request Feb 19, 2025

fix(devnet): Modify the blob configuration of the bsc devnet (ethereu…

db5b9eb

…m-optimism#212)

shenkeyao referenced this pull request in EspressoSystems/optimism-espresso-integration Mar 1, 2025

migration: Set fields correctly for migration block (#212)

cc7a931

QuentinI referenced this pull request in EspressoSystems/optimism-espresso-integration Mar 7, 2025

migration: Set fields correctly for migration block (#212)

0e7a450

Zena-park added a commit to tokamak-network/optimism that referenced this pull request Dec 30, 2025

[CAF-01S] Multiple Top-Level Declarations ethereum-optimism#212

7a2b19f

theochap pushed a commit that referenced this pull request Jan 15, 2026

feat(book): Consensus (#212)

1a9faf8

### Description Documents the `op-alloy-consensus` crate in the mdbook.

emhane added a commit that referenced this pull request Feb 3, 2026

feat: implemented mdbx trie cursor (#249)

e2e8c36

Closes #212 --------- Co-authored-by: Emilia Hane <[email protected]>

Sanitizing State Manager Gas #212

Sanitizing State Manager Gas #212

Uh oh!

Conversation

ben-chain commented Aug 12, 2020

Description

Questions

Metadata

Fixes

Contributing Agreement

Uh oh!

willmeister left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ben-chain commented Aug 25, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants