imp(vm): define default JumpTable (backport ethereum#3) (ethereum#13)

Co-authored-by: Federico Kunze Küllmer <[email protected]>
Co-authored-by: Freddy Caceres <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -1,4 +1,3 @@
-
 <!--
 Guiding Principles:
 
@@ -40,4 +39,5 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+* [#3](https://github.com/evmos/go-ethereum/pull/3) Move the `JumpTable` defaults to a separate function.
 * [#2](https://github.com/evmos/go-ethereum/pull/2) Define `Interpreter` interface for the EVM.

core/vm/interpreter.go

@@ -70,36 +70,16 @@ type EVMInterpreter struct {
 func NewEVMInterpreter(evm *EVM, cfg Config) *EVMInterpreter {
 	// If jump table was not initialised we set the default one.
 	if cfg.JumpTable == nil {
-		switch {
-		case evm.chainRules.IsMerge:
-			cfg.JumpTable = &mergeInstructionSet
-		case evm.chainRules.IsLondon:
-			cfg.JumpTable = &londonInstructionSet
-		case evm.chainRules.IsBerlin:
-			cfg.JumpTable = &berlinInstructionSet
-		case evm.chainRules.IsIstanbul:
-			cfg.JumpTable = &istanbulInstructionSet
-		case evm.chainRules.IsConstantinople:
-			cfg.JumpTable = &constantinopleInstructionSet
-		case evm.chainRules.IsByzantium:
-			cfg.JumpTable = &byzantiumInstructionSet
-		case evm.chainRules.IsEIP158:
-			cfg.JumpTable = &spuriousDragonInstructionSet
-		case evm.chainRules.IsEIP150:
-			cfg.JumpTable = &tangerineWhistleInstructionSet
-		case evm.chainRules.IsHomestead:
-			cfg.JumpTable = &homesteadInstructionSet
-		default:
-			cfg.JumpTable = &frontierInstructionSet
-		}
+		cfg.JumpTable = DefaultJumpTable(evm.chainRules)
 		for i, eip := range cfg.ExtraEips {
-			copy := *cfg.JumpTable
-			if err := EnableEIP(eip, &copy); err != nil {
+			// Deep-copy jumptable to prevent modification of opcodes in other tables
+			copy := CopyJumpTable(cfg.JumpTable)
+			if err := EnableEIP(eip, copy); err != nil {
 				// Disable it, so caller can check if it's activated or not
 				cfg.ExtraEips = append(cfg.ExtraEips[:i], cfg.ExtraEips[i+1:]...)
 				log.Error("EIP activation failed", "eip", eip, "error", err)
 			}
-			cfg.JumpTable = &copy
+			cfg.JumpTable = copy
 		}
 	}
 

core/vm/jump_table.go

@@ -45,37 +45,76 @@ type operation struct {
 }
 
 var (
-	frontierInstructionSet         = newFrontierInstructionSet()
-	homesteadInstructionSet        = newHomesteadInstructionSet()
-	tangerineWhistleInstructionSet = newTangerineWhistleInstructionSet()
-	spuriousDragonInstructionSet   = newSpuriousDragonInstructionSet()
-	byzantiumInstructionSet        = newByzantiumInstructionSet()
-	constantinopleInstructionSet   = newConstantinopleInstructionSet()
-	istanbulInstructionSet         = newIstanbulInstructionSet()
-	berlinInstructionSet           = newBerlinInstructionSet()
-	londonInstructionSet           = newLondonInstructionSet()
-	mergeInstructionSet            = newMergeInstructionSet()
+	FrontierInstructionSet         = newFrontierInstructionSet()
+	HomesteadInstructionSet        = newHomesteadInstructionSet()
+	TangerineWhistleInstructionSet = newTangerineWhistleInstructionSet()
+	SpuriousDragonInstructionSet   = newSpuriousDragonInstructionSet()
+	ByzantiumInstructionSet        = newByzantiumInstructionSet()
+	ConstantinopleInstructionSet   = newConstantinopleInstructionSet()
+	IstanbulInstructionSet         = newIstanbulInstructionSet()
+	BerlinInstructionSet           = newBerlinInstructionSet()
+	LondonInstructionSet           = newLondonInstructionSet()
+	MergeInstructionSet            = newMergeInstructionSet()
 )
 
 // JumpTable contains the EVM opcodes supported at a given fork.
 type JumpTable [256]*operation
 
-func validate(jt JumpTable) JumpTable {
+// DefaultJumpTable defines the default jump table used by the EVM interpreter.
+func DefaultJumpTable(rules params.Rules) (jumpTable *JumpTable) {
+	switch {
+	case rules.IsMerge:
+		jumpTable = &MergeInstructionSet
+	case rules.IsLondon:
+		jumpTable = &LondonInstructionSet
+	case rules.IsBerlin:
+		jumpTable = &BerlinInstructionSet
+	case rules.IsIstanbul:
+		jumpTable = &IstanbulInstructionSet
+	case rules.IsConstantinople:
+		jumpTable = &ConstantinopleInstructionSet
+	case rules.IsByzantium:
+		jumpTable = &ByzantiumInstructionSet
+	case rules.IsEIP158:
+		jumpTable = &SpuriousDragonInstructionSet
+	case rules.IsEIP150:
+		jumpTable = &TangerineWhistleInstructionSet
+	case rules.IsHomestead:
+		jumpTable = &HomesteadInstructionSet
+	default:
+		jumpTable = &FrontierInstructionSet
+	}
+
+	return jumpTable
+}
+
+// Validate checks if all the operations are set and if they are valid according to the
+// interpreter assumptions.
+func (jt JumpTable) Validate() error {
 	for i, op := range jt {
 		if op == nil {
-			panic(fmt.Sprintf("op %#x is not set", i))
+			return fmt.Errorf("op %#x is not set", i)
 		}
+
 		// The interpreter has an assumption that if the memorySize function is
 		// set, then the dynamicGas function is also set. This is a somewhat
 		// arbitrary assumption, and can be removed if we need to -- but it
 		// allows us to avoid a condition check. As long as we have that assumption
 		// in there, this little sanity check prevents us from merging in a
 		// change which violates it.
 		if op.memorySize != nil && op.dynamicGas == nil {
-			panic(fmt.Sprintf("op %v has dynamic memory but not dynamic gas", OpCode(i).String()))
+			return fmt.Errorf("op %v has dynamic memory but not dynamic gas", OpCode(i).String())
 		}
 	}
-	return jt
+
+	return nil
+}
+
+// MustValidate panics if the operations are not valid.
+func (jt JumpTable) MustValidate() {
+	if err := jt.Validate(); err != nil {
+		panic(err)
+	}
 }
 
 func newMergeInstructionSet() JumpTable {
@@ -86,7 +125,8 @@ func newMergeInstructionSet() JumpTable {
 		minStack:    minStack(0, 1),
 		maxStack:    maxStack(0, 1),
 	}
-	return validate(instructionSet)
+	instructionSet.MustValidate()
+	return instructionSet
 }
 
 // newLondonInstructionSet returns the frontier, homestead, byzantium,
@@ -95,15 +135,17 @@ func newLondonInstructionSet() JumpTable {
 	instructionSet := newBerlinInstructionSet()
 	enable3529(&instructionSet) // EIP-3529: Reduction in refunds https://eips.ethereum.org/EIPS/eip-3529
 	enable3198(&instructionSet) // Base fee opcode https://eips.ethereum.org/EIPS/eip-3198
-	return validate(instructionSet)
+	instructionSet.MustValidate()
+	return instructionSet
 }
 
 // newBerlinInstructionSet returns the frontier, homestead, byzantium,
 // contantinople, istanbul, petersburg and berlin instructions.
 func newBerlinInstructionSet() JumpTable {
 	instructionSet := newIstanbulInstructionSet()
 	enable2929(&instructionSet) // Access lists for trie accesses https://eips.ethereum.org/EIPS/eip-2929
-	return validate(instructionSet)
+	instructionSet.MustValidate()
+	return instructionSet
 }
 
 // newIstanbulInstructionSet returns the frontier, homestead, byzantium,
@@ -115,7 +157,8 @@ func newIstanbulInstructionSet() JumpTable {
 	enable1884(&instructionSet) // Reprice reader opcodes - https://eips.ethereum.org/EIPS/eip-1884
 	enable2200(&instructionSet) // Net metered SSTORE - https://eips.ethereum.org/EIPS/eip-2200
 
-	return validate(instructionSet)
+	instructionSet.MustValidate()
+	return instructionSet
 }
 
 // newConstantinopleInstructionSet returns the frontier, homestead,
@@ -154,7 +197,8 @@ func newConstantinopleInstructionSet() JumpTable {
 		maxStack:    maxStack(4, 1),
 		memorySize:  memoryCreate2,
 	}
-	return validate(instructionSet)
+	instructionSet.MustValidate()
+	return instructionSet
 }
 
 // newByzantiumInstructionSet returns the frontier, homestead and
@@ -190,14 +234,16 @@ func newByzantiumInstructionSet() JumpTable {
 		maxStack:   maxStack(2, 0),
 		memorySize: memoryRevert,
 	}
-	return validate(instructionSet)
+	instructionSet.MustValidate()
+	return instructionSet
 }
 
 // EIP 158 a.k.a Spurious Dragon
 func newSpuriousDragonInstructionSet() JumpTable {
 	instructionSet := newTangerineWhistleInstructionSet()
 	instructionSet[EXP].dynamicGas = gasExpEIP158
-	return validate(instructionSet)
+	instructionSet.MustValidate()
+	return instructionSet
 }
 
 // EIP 150 a.k.a Tangerine Whistle
@@ -210,7 +256,8 @@ func newTangerineWhistleInstructionSet() JumpTable {
 	instructionSet[CALL].constantGas = params.CallGasEIP150
 	instructionSet[CALLCODE].constantGas = params.CallGasEIP150
 	instructionSet[DELEGATECALL].constantGas = params.CallGasEIP150
-	return validate(instructionSet)
+	instructionSet.MustValidate()
+	return instructionSet
 }
 
 // newHomesteadInstructionSet returns the frontier and homestead
@@ -225,7 +272,8 @@ func newHomesteadInstructionSet() JumpTable {
 		maxStack:    maxStack(6, 1),
 		memorySize:  memoryDelegateCall,
 	}
-	return validate(instructionSet)
+	instructionSet.MustValidate()
+	return instructionSet
 }
 
 // newFrontierInstructionSet returns the frontier instructions
@@ -1041,5 +1089,18 @@ func newFrontierInstructionSet() JumpTable {
 		}
 	}
 
-	return validate(tbl)
+	tbl.MustValidate()
+	return tbl
+}
+
+// CopyJumpTable creates copy of the operations from the provided source JumpTable.
+func CopyJumpTable(source *JumpTable) *JumpTable {
+	dest := *source
+	for i, op := range source {
+		if op != nil {
+			opCopy := *op
+			dest[i] = &opCopy
+		}
+	}
+	return &dest
 }

core/vm/jump_table_test.go

@@ -0,0 +1,35 @@
+// Copyright 2022 The go-ethereum Authors
+// This file is part of the go-ethereum library.
+//
+// The go-ethereum library is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// The go-ethereum library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
+
+package vm
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// TestJumpTableCopy tests that deep copy is necessery to prevent modify shared jump table
+func TestJumpTableCopy(t *testing.T) {
+	tbl := newMergeInstructionSet()
+	require.Equal(t, uint64(0), tbl[SLOAD].constantGas)
+
+	// a deep copy won't modify the shared jump table
+	deepCopy := CopyJumpTable(&tbl)
+	deepCopy[SLOAD].constantGas = 100
+	require.Equal(t, uint64(100), deepCopy[SLOAD].constantGas)
+	require.Equal(t, uint64(0), tbl[SLOAD].constantGas)
+}