From 6a5de7537fc085bbb6965c94f2dc62bc04e19e69 Mon Sep 17 00:00:00 2001
From: Arran Schlosberg <me@arranschlosberg.com>
Date: Wed, 18 Sep 2024 08:33:37 -0400
Subject: [PATCH] refactor: replace `NewOperation` with `OperationBuilder`

---
 core/vm/jump_table.libevm.go      | 49 ++++++++++++++++++++++---------
 core/vm/jump_table.libevm_test.go | 22 +++++++++-----
 2 files changed, 49 insertions(+), 22 deletions(-)

diff --git a/core/vm/jump_table.libevm.go b/core/vm/jump_table.libevm.go
index 793fb3395883..22082ff05d0e 100644
--- a/core/vm/jump_table.libevm.go
+++ b/core/vm/jump_table.libevm.go
@@ -15,21 +15,42 @@ func overrideJumpTable(r params.Rules, jt *JumpTable) *JumpTable {
 	return libevmHooks.OverrideJumpTable(r, jt)
 }
 
-// NewOperation constructs a new operation for inclusion in a [JumpTable].
-func NewOperation(
-	execute func(pc *uint64, interpreter *EVMInterpreter, callContext *ScopeContext) ([]byte, error),
-	constantGas uint64,
-	dynamicGas func(e *EVM, c *Contract, s *Stack, m *Memory, u uint64) (uint64, error),
-	minStack, maxStack int,
-	memorySize func(s *Stack) (size uint64, overflow bool),
-) *operation {
-	return &operation{
-		execute,
-		constantGas,
-		dynamicGas,
-		minStack, maxStack,
-		memorySize,
+// An OperationBuilder is a factory for a new operations to include in a
+// [JumpTable]. All of its fields are required.
+type OperationBuilder[G interface {
+	uint64 | func(_ *EVM, _ *Contract, _ *Stack, _ *Memory, requestedMemorySize uint64) (uint64, error)
+}] struct {
+	Execute            func(pc *uint64, interpreter *EVMInterpreter, callContext *ScopeContext) ([]byte, error)
+	Gas                G
+	MinStack, MaxStack int
+	MemorySize         func(s *Stack) (size uint64, overflow bool)
+}
+
+type (
+	// OperationBuilderConstantGas is the constant-gas version of an
+	// OperationBuilder.
+	OperationBuilderConstantGas = OperationBuilder[uint64]
+	// OperationBuilderDynamicGas is the dynamic-gas version of an
+	// OperationBuilder.
+	OperationBuilderDynamicGas = OperationBuilder[func(_ *EVM, _ *Contract, _ *Stack, _ *Memory, requestedMemorySize uint64) (uint64, error)]
+)
+
+// Build constructs the operation.
+func (b OperationBuilder[G]) Build() *operation {
+	o := &operation{
+		execute:    b.Execute,
+		minStack:   b.MinStack,
+		maxStack:   b.MaxStack,
+		memorySize: b.MemorySize,
+	}
+
+	switch g := any(b.Gas).(type) {
+	case uint64:
+		o.constantGas = g
+	case gasFunc:
+		o.dynamicGas = g
 	}
+	return o
 }
 
 // Hooks are arbitrary configuration functions to modify default VM behaviour.
diff --git a/core/vm/jump_table.libevm_test.go b/core/vm/jump_table.libevm_test.go
index 42e92a950844..602fc0545d79 100644
--- a/core/vm/jump_table.libevm_test.go
+++ b/core/vm/jump_table.libevm_test.go
@@ -3,6 +3,7 @@ package vm_test
 import (
 	"fmt"
 	"math/big"
+	"reflect"
 	"testing"
 
 	"github.com/holiman/uint256"
@@ -53,24 +54,22 @@ func TestOverrideJumpTable(t *testing.T) {
 
 	vmHooks := &vmHooksStub{
 		replacement: &vm.JumpTable{
-			opcode: vm.NewOperation(
-				func(pc *uint64, interpreter *vm.EVMInterpreter, callContext *vm.ScopeContext) ([]byte, error) {
+			opcode: vm.OperationBuilderConstantGas{
+				Execute: func(pc *uint64, interpreter *vm.EVMInterpreter, callContext *vm.ScopeContext) ([]byte, error) {
 					executed = true
 					return nil, nil
 				},
-				gasCost, nil,
-				0, 0,
-				func(s *vm.Stack) (size uint64, overflow bool) {
+				Gas: gasCost,
+				MemorySize: func(s *vm.Stack) (size uint64, overflow bool) {
 					return 0, false
 				},
-			),
+			}.Build(),
 		},
 	}
 	vm.RegisterHooks(vmHooks)
 
 	t.Run("LookupInstructionSet", func(t *testing.T) {
-		_, evm := ethtest.NewZeroEVM(t)
-		rules := evm.ChainConfig().Rules(big.NewInt(0), false, 0)
+		rules := new(params.ChainConfig).Rules(big.NewInt(0), false, 0)
 
 		for _, b := range []bool{false, true} {
 			vmHooks.overridden = false
@@ -99,3 +98,10 @@ func TestOverrideJumpTable(t *testing.T) {
 		assert.Equal(t, gasLimit-gasCost, gasRemaining, "gas remaining")
 	})
 }
+
+func TestOperationFieldCount(t *testing.T) {
+	// The libevm OperationBuilder assumes that the 6 struct fields are the only
+	// ones.
+	op := vm.OperationBuilderConstantGas{}.Build()
+	require.Equalf(t, 6, reflect.TypeOf(*op).NumField(), "number of fields in %T struct", *op)
+}