core/{state, vm}: update stateless gas costs to follow the verkle-gen-7 testnet #31014

gballet · 2025-01-10T09:17:27Z

Adding values to the witness introduces a new class of issues for computing gas: if there is not enough gas to cover adding an item to the witness, then the item should not be added to the witness.

The problem happens when several items are added together, and that process runs out of gas. The witness gas computation needs a way to signal that not enough gas was provided. These values can not be hardcoded, however, as they are context dependent, i.e. two calls to the same function with the same parameters can give two different results.

The approach is to return both the gas that was actually consumed, and the gas that was necessary. If the values don't match, then a witness update OOG'd. The caller should then charge the consumed value (remaining gas will be 0) and error out.

Why not return a boolean instead of the wanted value? Because when several items are touched, we want to distinguish which item lacked gas.

rjl493456442 · 2025-01-14T02:05:18Z

core/vm/evm.go

+			// list in write mode. If there is enough gas paying for the addition of the code
+			// hash leaf to the access list, then account creation will proceed unimpaired.
+			// Thus, only pay for the creation of the code hash leaf here.
+			wgas := evm.AccessEvents.CodeHashGas(addr, true, gas, false)


If the available gas is not sufficient to pay the cost, availableGas will be returned.

Originally, the evm execution should stop without executing this operation;
Now, the evm execution will execute this operation, and may stop later;

already responded during standup - good idea but that's a spec change so we should try to fix it over time but this is what the testnet does.

core/vm/evm.go

… testnet Co-authored-by: Ignacio Hagopian <[email protected]> Signed-off-by: Guillaume Ballet <[email protected]>

Copilot

Pull Request Overview

This pull request updates several gas cost functions and witness accounting logic to better match the verkle-gen-7 testnet requirements while also revising the instruction set and precompiled contract mappings. Key changes include:

Updating gas calculations across storage, call, and contract creation operations to return both consumed and required gas.
Changing the jump table from the Cancun to the Shanghai instruction set and remapping precompiled contracts.
Adapting tests and access events to align with the new gas API signatures.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
core/vm/operations_verkle.go	Revised gas functions and witness gas calculations for various operations.
core/vm/jump_table.go	Switched the underlying instruction set from Cancun to Shanghai.
core/vm/interpreter.go	Updated code chunk gas charging logic in the interpreter loop.
core/vm/gas_table.go	Removed ValueTransferGas charging in favor of the new gas model.
core/vm/evm.go	Modified contract creation and code chunk gas functions using updated APIs.
core/vm/eips.go	Adjusted gas charging in extcode copy and push operations.
core/vm/contracts.go	Remapped the Verkle precompiled contracts reference from Prague to Berlin.
core/state_transition.go	Updated transaction destination logic to account for account existence.
core/state/access_events_test.go	Refactored tests to use the updated access events gas functions.
core/state/access_events.go	Revised gas access functions to include availableGas and warm cost logic.

core/vm/eips.go

core/vm/operations_verkle.go

gballet · 2025-03-31T19:56:18Z

core/vm/eips.go

@@ -339,12 +339,10 @@ func opExtCodeCopyEIP4762(pc *uint64, interpreter *EVMInterpreter, scope *ScopeC
 	addr := common.Address(a.Bytes20())
 	code := interpreter.evm.StateDB.GetCode(addr)
 	paddedCodeCopy, copyOffset, nonPaddedCopyLength := getDataAndAdjustedBounds(code, uint64CodeOffset, length.Uint64())
-	if !scope.Contract.IsSystemCall {


Note that this implements the suggestion that extcodecopy of a "system contract" (yeah I know you guys don't like the name but it has the advantage that everyone gets what I'm talking about) will end up in the witness. Spec change is underway, I'll hack the stuff back in when I sync the testnet in the meantime.

MariusVanDerWijden · 2025-04-01T17:36:08Z

core/state/access_events.go

-	gas += ae.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.CodeHashLeafKey, isWrite)
+func (ae *AccessEvents) AddAccount(addr common.Address, isWrite bool, availableGas uint64) uint64 {
+	var gas uint64 // accumulate the consumed gas
+	consumed, wanted := ae.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.BasicDataLeafKey, isWrite, availableGas)


Maybe expected instead of wanted?

MariusVanDerWijden · 2025-04-01T17:40:16Z

core/state/access_events.go

-func (ae *AccessEvents) AddTxDestination(addr common.Address, sendsValue bool) {
-	ae.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.BasicDataLeafKey, sendsValue)
-	ae.touchAddressAndChargeGas(addr, zeroTreeIndex, utils.CodeHashLeafKey, false)
+func (ae *AccessEvents) AddTxDestination(addr common.Address, sendsValue, doesntExist bool) {


Maybe rename doesntExist to exists and inverse it when passing it to ae.touchAddressAndChargeGas

I'll push back against this one, because if in the future we have to check a negative condition, we'll get !doesntExist and double negatives are a nasty pitfall.

https://www.youtube.com/watch?v=63NCvjX7X8E

MariusVanDerWijden · 2025-04-01T17:43:19Z

core/state/access_events.go

+	if wanted == 0 && chargeWarmCosts {
+		wanted = params.WarmStorageReadCostEIP2929
+	}
+	return wanted
 }

 // touchAddressAndChargeGas adds any missing access event to the access event list, and returns the cold
 // access cost to be charged, if need be.


outdated comment, returns the consumed and required gas

MariusVanDerWijden · 2025-04-01T17:48:54Z

core/state/access_events.go

+	var gas uint64
+	if branchRead {
+		gas += params.WitnessBranchReadCost
+	}
+	if chunkRead {
+		gas += params.WitnessChunkReadCost
+	}
+	if branchWrite {
+		gas += params.WitnessBranchWriteCost
+	}
+	if chunkWrite {
+		gas += params.WitnessChunkWriteCost
+	}
+	if chunkFill {
+		gas += params.WitnessChunkFillCost
+	}
+
+	if availableGas < gas {
+		// consumed != wanted
+		return availableGas, gas
+	}
+
+	if branchRead {
+		ae.branches[branchKey] = AccessWitnessReadFlag
+	}
+	if branchWrite {
+		ae.branches[branchKey] |= AccessWitnessWriteFlag
+	}
+	if chunkRead {
+		ae.chunks[chunkKey] = AccessWitnessReadFlag
+	}
+	if chunkWrite {
+		ae.chunks[chunkKey] |= AccessWitnessWriteFlag
+	}


In the first block, you're doing read read, write write in the second block read write, read write.
Am I correct in assuming that writes MUST be preceded by a read?
If so the code I think doing something like this would make the relationship a bit clearer (same with the gas computation):
e.g.

if branchRead { ae.branches[branchKey] = AccessWitnessReadFlag if branchWrite { ae.branches[branchKey] |= AccessWitnessWriteFlag } }

it wouldn't work, because you can have the read happen at an instruction before the one doing the write, and so if you did condition the write to only happen with a read, you would skip setting the write flag.

MariusVanDerWijden · 2025-04-01T17:52:28Z

core/vm/contracts.go

@@ -139,7 +139,7 @@ var PrecompiledContractsPrague = PrecompiledContracts{

 var PrecompiledContractsBLS = PrecompiledContractsPrague

-var PrecompiledContractsVerkle = PrecompiledContractsPrague
+var PrecompiledContractsVerkle = PrecompiledContractsBerlin


No KZG precompile?

nope it's for a pre-cancun chain at the moment

MariusVanDerWijden · 2025-04-01T17:56:11Z

core/vm/eips.go

+	consumed, wanted := interpreter.evm.AccessEvents.CodeChunksRangeGas(addr, copyOffset, nonPaddedCopyLength, uint64(len(code)), false, scope.Contract.Gas)
+	scope.Contract.UseGas(consumed, interpreter.evm.Config.Tracer, tracing.GasChangeUnspecified)
+	if consumed < wanted {
+		return nil, ErrOutOfGas


Shouldn't you also consume all the gas of this scope here?
Actually UseGas might underflow the contracts gas here, no?

might be a bit confusing indeed: if you have enough gas, then consumed == wanted so it's fine. If consumed < wanted then consumed contains all the remaining gas anyway.

Signed-off-by: Guillaume Ballet <[email protected]>

rjl493456442 · 2025-04-08T11:50:17Z

core/vm/eips.go

-			scope.Contract.Gas = 0
-			return nil, ErrOutOfGas
-		}
+	consumed, wanted := interpreter.evm.AccessEvents.CodeChunksRangeGas(addr, copyOffset, nonPaddedCopyLength, uint64(len(code)), false, scope.Contract.Gas)


It's a bit weird to do the gas cost computation and deduction in opcode execution.

The information for computation the gas cost is: (1) code size (2) code offset (3) code length to copy

all these fields are available in gas func. Let's move it to gas func

rjl493456442 · 2025-04-08T11:54:32Z

core/state/access_events.go

-func (ae *AccessEvents) touchAddress(addr common.Address, treeIndex uint256.Int, subIndex byte, isWrite bool) (bool, bool, bool, bool, bool) {
+// touchAddressAndChargeGas adds any missing access event to the access event list, and returns the
+// consumed and required gas.
+func (ae *AccessEvents) touchAddressAndChargeGas(addr common.Address, treeIndex uint256.Int, subIndex byte, isWrite bool, availableGas uint64) (uint64, uint64) {


This function returns two parameters: (a) the number of gas required (b) the number of gas consumed

If the available gas is not sufficient to cover the gas cost, an error or a flag should be returned indicating the gas is not sufficient.

It's unnecessary to return availableGas as consumed one, as the error will ultimately be captured by the outer part and leftover gas will be all consumed (e.g. err != ErrExecutionReverted)

gballet added the verkle label Jan 10, 2025

gballet force-pushed the kaustinen7-gas-costs branch 2 times, most recently from 9f3ab25 to c92a0d4 Compare January 10, 2025 09:36

rjl493456442 reviewed Jan 14, 2025

View reviewed changes

gballet force-pushed the kaustinen7-gas-costs branch from 1ceb3e2 to e7dd2e9 Compare January 30, 2025 16:59

gballet marked this pull request as ready for review January 30, 2025 17:04

gballet requested a review from holiman as a code owner January 30, 2025 17:04

gballet added the status:triage label Jan 30, 2025

rjl493456442 self-assigned this Feb 1, 2025

gballet added this to the 1.15.1 milestone Feb 5, 2025

fjl modified the milestones: 1.15.1, 1.15.2, 1.15.3 Feb 13, 2025

fjl modified the milestones: 1.15.3, 1.15.4, 1.15.5 Feb 25, 2025

fjl modified the milestones: 1.15.5, 1.15.6 Mar 5, 2025

fjl removed this from the 1.15.6 milestone Mar 20, 2025

fjl mentioned this pull request Mar 24, 2025

Verkle Planning #31477

Open

gballet commented Mar 24, 2025

View reviewed changes

core/vm/evm.go Outdated Show resolved Hide resolved

gballet force-pushed the kaustinen7-gas-costs branch from 2bc5759 to a749131 Compare March 31, 2025 08:10

core/{state, vm}: update stateless gas costs to follow the kaustinen7…

13a09a4

… testnet Co-authored-by: Ignacio Hagopian <[email protected]> Signed-off-by: Guillaume Ballet <[email protected]>

gballet force-pushed the kaustinen7-gas-costs branch from a749131 to 13a09a4 Compare March 31, 2025 16:09

gballet requested a review from Copilot March 31, 2025 16:09

Copilot AI reviewed Mar 31, 2025

View reviewed changes

core/vm/eips.go Show resolved Hide resolved

core/vm/operations_verkle.go Show resolved Hide resolved

gballet commented Mar 31, 2025

View reviewed changes

rjl493456442 assigned MariusVanDerWijden Apr 1, 2025

MariusVanDerWijden reviewed Apr 1, 2025

View reviewed changes

review feedback

f4e0f1b

Signed-off-by: Guillaume Ballet <[email protected]>

rjl493456442 reviewed Apr 8, 2025

View reviewed changes

move extcodesize consumption to gas method

abb0094

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core/{state, vm}: update stateless gas costs to follow the verkle-gen-7 testnet #31014

core/{state, vm}: update stateless gas costs to follow the verkle-gen-7 testnet #31014

gballet commented Jan 10, 2025 •

edited

Loading

rjl493456442 Jan 14, 2025

gballet Jan 30, 2025

Copilot AI left a comment

gballet Mar 31, 2025

MariusVanDerWijden Apr 1, 2025

MariusVanDerWijden Apr 1, 2025

gballet Apr 7, 2025 •

edited

Loading

MariusVanDerWijden Apr 1, 2025

MariusVanDerWijden Apr 1, 2025

gballet Apr 7, 2025 •

edited

Loading

MariusVanDerWijden Apr 1, 2025

gballet Apr 7, 2025

MariusVanDerWijden Apr 1, 2025 •

edited

Loading

gballet Apr 7, 2025

rjl493456442 Apr 8, 2025

rjl493456442 Apr 8, 2025 •

edited

Loading

core/{state, vm}: update stateless gas costs to follow the verkle-gen-7 testnet #31014

Are you sure you want to change the base?

core/{state, vm}: update stateless gas costs to follow the verkle-gen-7 testnet #31014

Conversation

gballet commented Jan 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gballet Apr 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gballet Apr 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MariusVanDerWijden Apr 1, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rjl493456442 Apr 8, 2025 • edited Loading

Choose a reason for hiding this comment

gballet commented Jan 10, 2025 •

edited

Loading

gballet Apr 7, 2025 •

edited

Loading

gballet Apr 7, 2025 •

edited

Loading

MariusVanDerWijden Apr 1, 2025 •

edited

Loading

rjl493456442 Apr 8, 2025 •

edited

Loading