Skip to content

Conversation

gligneul
Copy link
Contributor

This PR adds up the dynamic and constant opcode multi-gas cost in the EVM interpreter, and then return it in the ExecutionResult struct.

Close NIT-3468

This PR adds up the dynamic and constant opcode multi-gas cost in the
EVM interpreter, and then return it in the ExecutionResult struct.

Close NIT-3468
@cla-bot cla-bot bot added the s CLA signed label Jul 15, 2025
@gligneul gligneul requested a review from Copilot July 15, 2025 12:40
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds tracking of multi-dimensional gas (“multi-gas”) throughout the EVM execution and surfaces it in the ExecutionResult.

  • Extend the EVM.Call API to return a usedMultiGas value and update all call sites.
  • Record static and dynamic multi-gas usage in the interpreter and aggregate it in state transition.
  • Propagate UsedMultiGas into the ExecutionResult struct.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/state_test.go Adjusted benchmark call to accept the extra blank return value
core/vm/runtime/runtime.go Updated Execute/Call wrappers to ignore the new multi-gas return
core/vm/interpreter_test.go Updated test to accept the additional blank return from Call
core/vm/interpreter.go Track and accumulate multi-gas for static and dynamic operations
core/vm/instructions.go Updated CALL opcode handler to unpack the new return value
core/vm/gas_table_test.go Adapted tests to ignore the new multi-gas return
core/vm/evm.go Extended EVM.Call signature and added zero‐gas fallbacks
core/vm/contract.go Added UsedMultiGas field to Contract
core/state_transition.go Added UsedMultiGas to ExecutionResult and propagate refunds
core/state_processor.go Updated system call sites to unpack extra return from Call
Comments suppressed due to low confidence (3)

core/vm/interpreter.go:294

  • [nitpick] The variable multigasDynamicCost could be renamed to dynamicMultiGasCost to align with dynamicCost and improve readability.
			var multigasDynamicCost *multigas.MultiGas

core/state_transition.go:754

  • There are no tests verifying the new UsedMultiGas field in ExecutionResult; consider adding test cases to validate multi-gas is correctly computed and returned.
		UsedMultiGas:     usedMultiGas,

core/vm/evm.go:202

  • Update the doc comment above EVM.Call to include the new usedMultiGas return parameter, explaining its purpose and units.
func (evm *EVM) Call(caller common.Address, addr common.Address, input []byte, gas uint64, value *uint256.Int) (ret []byte, leftOverGas uint64, usedMultiGas *multigas.MultiGas, err error) {

@gligneul gligneul marked this pull request as ready for review July 24, 2025 12:50
@eljobe eljobe enabled auto-merge July 24, 2025 13:27
@eljobe eljobe merged commit 848bf15 into master Jul 24, 2025
15 checks passed
@eljobe eljobe deleted the gligneul/store-multigas branch July 24, 2025 13:27
gligneul added a commit to OffchainLabs/nitro that referenced this pull request Jul 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants