From a0e306a686aeee0f3b2291f52a8e5a88dd264339 Mon Sep 17 00:00:00 2001 From: Daniel Liu Date: Tue, 24 Sep 2024 15:04:08 +0800 Subject: [PATCH 01/12] core/vm: avoid memory expansion check for trivial ops (#24048) --- core/vm/interpreter.go | 54 +++++++++++++++++++----------------------- core/vm/jump_table.go | 44 ++++++++++++++++++++++++---------- 2 files changed, 57 insertions(+), 41 deletions(-) diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index 7d8351f70278..ae7f1b377f16 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -219,54 +219,50 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) ( // enough stack items available to perform the operation. op = contract.GetOp(pc) operation := in.cfg.JumpTable[op] + cost = operation.constantGas // For tracing // Validate stack if sLen := stack.len(); sLen < operation.minStack { return nil, &ErrStackUnderflow{stackLen: sLen, required: operation.minStack} } else if sLen > operation.maxStack { return nil, &ErrStackOverflow{stackLen: sLen, limit: operation.maxStack} } - // Static portion of gas - cost = operation.constantGas // For tracing - if !contract.UseGas(operation.constantGas) { + if !contract.UseGas(cost) { return nil, ErrOutOfGas } - - var memorySize uint64 - // calculate the new memory size and expand the memory to fit - // the operation - // Memory check needs to be done prior to evaluating the dynamic gas portion, - // to detect calculation overflows - if operation.memorySize != nil { - memSize, overflow := operation.memorySize(stack) - if overflow { - return nil, ErrGasUintOverflow - } - // memory is expanded in words of 32 bytes. Gas - // is also calculated in words. - if memorySize, overflow = math.SafeMul(toWordSize(memSize), 32); overflow { - return nil, ErrGasUintOverflow - } - } - // Dynamic portion of gas - // consume the gas and return an error if not enough gas is available. - // cost is explicitly set so that the capture state defer method can get the proper cost if operation.dynamicGas != nil { + // All ops with a dynamic memory usage also has a dynamic gas cost. + var memorySize uint64 + // calculate the new memory size and expand the memory to fit + // the operation + // Memory check needs to be done prior to evaluating the dynamic gas portion, + // to detect calculation overflows + if operation.memorySize != nil { + memSize, overflow := operation.memorySize(stack) + if overflow { + return nil, ErrGasUintOverflow + } + // memory is expanded in words of 32 bytes. Gas + // is also calculated in words. + if memorySize, overflow = math.SafeMul(toWordSize(memSize), 32); overflow { + return nil, ErrGasUintOverflow + } + } + // Consume the gas and return an error if not enough gas is available. + // cost is explicitly set so that the capture state defer method can get the proper cost var dynamicCost uint64 dynamicCost, err = operation.dynamicGas(in.evm, contract, stack, mem, memorySize) - cost += dynamicCost // total cost, for debug tracing + cost += dynamicCost // for tracing if err != nil || !contract.UseGas(dynamicCost) { return nil, ErrOutOfGas } + if memorySize > 0 { + mem.Resize(memorySize) + } } - if memorySize > 0 { - mem.Resize(memorySize) - } - if in.cfg.Debug { in.cfg.Tracer.CaptureState(in.evm, pc, op, gasCopy, cost, callContext, in.returnData, in.evm.depth, err) logged = true } - // execute the operation res, err = operation.execute(&pc, in, callContext) if err != nil { diff --git a/core/vm/jump_table.go b/core/vm/jump_table.go index 4ae7c1e8ce1e..ca46f9166aad 100644 --- a/core/vm/jump_table.go +++ b/core/vm/jump_table.go @@ -17,6 +17,8 @@ package vm import ( + "fmt" + "github.com/XinFinOrg/XDPoSChain/params" ) @@ -60,16 +62,34 @@ var ( // JumpTable contains the EVM opcodes supported at a given fork. type JumpTable [256]*operation +func validate(jt JumpTable) JumpTable { + for i, op := range jt { + if op == nil { + panic(fmt.Sprintf("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 jt +} + func newEip1559InstructionSet() JumpTable { instructionSet := newShanghaiInstructionSet() enable2929(&instructionSet) // Gas cost increases for state access opcodes https://eips.ethereum.org/EIPS/eip-2929 - return instructionSet + return validate(instructionSet) } func newShanghaiInstructionSet() JumpTable { instructionSet := newMergeInstructionSet() enable3855(&instructionSet) // PUSH0 instruction - return instructionSet + return validate(instructionSet) } func newMergeInstructionSet() JumpTable { @@ -80,7 +100,7 @@ func newMergeInstructionSet() JumpTable { minStack: minStack(0, 1), maxStack: maxStack(0, 1), } - return instructionSet + return validate(instructionSet) } // newLondonInstructionSet returns the frontier, homestead, byzantium, @@ -89,7 +109,7 @@ 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 instructionSet + return validate(instructionSet) } // newBerlinInstructionSet returns the frontier, homestead, byzantium, @@ -97,7 +117,7 @@ func newLondonInstructionSet() JumpTable { func newBerlinInstructionSet() JumpTable { instructionSet := newIstanbulInstructionSet() // enable2929(&instructionSet) // Gas cost increases for state access opcodes https://eips.ethereum.org/EIPS/eip-2929 - return instructionSet + return validate(instructionSet) } // newIstanbulInstructionSet returns the frontier, homestead @@ -109,7 +129,7 @@ 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 instructionSet + return validate(instructionSet) } // newConstantinopleInstructionSet returns the frontier, homestead @@ -148,7 +168,7 @@ func newConstantinopleInstructionSet() JumpTable { maxStack: maxStack(4, 1), memorySize: memoryCreate2, } - return instructionSet + return validate(instructionSet) } // newByzantiumInstructionSet returns the frontier, homestead and @@ -184,14 +204,14 @@ func newByzantiumInstructionSet() JumpTable { maxStack: maxStack(2, 0), memorySize: memoryRevert, } - return instructionSet + return validate(instructionSet) } // EIP 158 a.k.a Spurious Dragon func newSpuriousDragonInstructionSet() JumpTable { instructionSet := newTangerineWhistleInstructionSet() instructionSet[EXP].dynamicGas = gasExpEIP158 - return instructionSet + return validate(instructionSet) } @@ -205,7 +225,7 @@ func newTangerineWhistleInstructionSet() JumpTable { instructionSet[CALL].constantGas = params.CallGasEIP150 instructionSet[CALLCODE].constantGas = params.CallGasEIP150 instructionSet[DELEGATECALL].constantGas = params.CallGasEIP150 - return instructionSet + return validate(instructionSet) } // newHomesteadInstructionSet returns the frontier and homestead @@ -220,7 +240,7 @@ func newHomesteadInstructionSet() JumpTable { maxStack: maxStack(6, 1), memorySize: memoryDelegateCall, } - return instructionSet + return validate(instructionSet) } // newFrontierInstructionSet returns the frontier instructions @@ -1036,5 +1056,5 @@ func newFrontierInstructionSet() JumpTable { } } - return tbl + return validate(tbl) } From 9207afbaf999e76501605f1a6d96fc464c7af8ec Mon Sep 17 00:00:00 2001 From: Daniel Liu Date: Tue, 24 Sep 2024 15:45:09 +0800 Subject: [PATCH 02/12] core/vm: fix some typos --- core/vm/interpreter.go | 2 +- core/vm/runtime/runtime_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index ae7f1b377f16..1e48dbd0979a 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -186,7 +186,7 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) ( logged bool // deferred EVMLogger should ignore already logged steps res []byte // result of the opcode execution function ) - // Don't move this deferrred function, it's placed before the capturestate-deferred method, + // Don't move this deferred function, it's placed before the capturestate-deferred method, // so that it get's executed _after_: the capturestate needs the stacks before // they are returned to the pools defer func() { diff --git a/core/vm/runtime/runtime_test.go b/core/vm/runtime/runtime_test.go index 21f5e1a57529..34031a95e996 100644 --- a/core/vm/runtime/runtime_test.go +++ b/core/vm/runtime/runtime_test.go @@ -453,7 +453,7 @@ func BenchmarkSimpleLoop(b *testing.B) { byte(vm.JUMP), } - calllRevertingContractWithInput := []byte{ + callRevertingContractWithInput := []byte{ byte(vm.JUMPDEST), // // push args for the call byte(vm.PUSH1), 0, // out size @@ -481,7 +481,7 @@ func BenchmarkSimpleLoop(b *testing.B) { benchmarkNonModifyingCode(100000000, loopingCode, "loop-100M", b) benchmarkNonModifyingCode(100000000, callInexistant, "call-nonexist-100M", b) benchmarkNonModifyingCode(100000000, callEOA, "call-EOA-100M", b) - benchmarkNonModifyingCode(100000000, calllRevertingContractWithInput, "call-reverting-100M", b) + benchmarkNonModifyingCode(100000000, callRevertingContractWithInput, "call-reverting-100M", b) //benchmarkNonModifyingCode(10000000, staticCallIdentity, "staticcall-identity-10M", b) //benchmarkNonModifyingCode(10000000, loopingCode, "loop-10M", b) From 296d31408a2624a5b3882ae37e3f71a4d5e64d4d Mon Sep 17 00:00:00 2001 From: Daniel Liu Date: Tue, 24 Sep 2024 16:01:34 +0800 Subject: [PATCH 03/12] core/vm: update benchmark to use Errorf instead of Sprintf (#24845) --- core/vm/contracts_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/vm/contracts_test.go b/core/vm/contracts_test.go index 1a03f93fc223..22fe467c3f97 100644 --- a/core/vm/contracts_test.go +++ b/core/vm/contracts_test.go @@ -618,7 +618,7 @@ func benchmarkPrecompiled(addr string, test precompiledTest, bench *testing.B) { return } if common.Bytes2Hex(res) != test.expected { - bench.Error(fmt.Sprintf("Expected %v, got %v", test.expected, common.Bytes2Hex(res))) + bench.Errorf("Expected %v, got %v", test.expected, common.Bytes2Hex(res)) return } }) From a4f98fc0350577dbcdd350fb9d2d5219a1d3c17c Mon Sep 17 00:00:00 2001 From: Daniel Liu Date: Tue, 24 Sep 2024 18:05:53 +0800 Subject: [PATCH 04/12] core/vm: for tracing, do not report post-op memory (#24867) --- core/vm/interpreter.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index 1e48dbd0979a..c4b4b4143b80 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -255,11 +255,15 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) ( if err != nil || !contract.UseGas(dynamicCost) { return nil, ErrOutOfGas } + // Do tracing before memory expansion + if in.cfg.Debug { + in.cfg.Tracer.CaptureState(in.evm, pc, op, gasCopy, cost, callContext, in.returnData, in.evm.depth, err) + logged = true + } if memorySize > 0 { mem.Resize(memorySize) } - } - if in.cfg.Debug { + } else if in.cfg.Debug { in.cfg.Tracer.CaptureState(in.evm, pc, op, gasCopy, cost, callContext, in.returnData, in.evm.depth, err) logged = true } From 4927464129bb074eda978bb2dcdfc8d77c9b45f2 Mon Sep 17 00:00:00 2001 From: Daniel Liu Date: Tue, 24 Sep 2024 18:56:33 +0800 Subject: [PATCH 05/12] core/vm: reduce overhead in instructions-benchmark (#24860) --- core/vm/instructions_test.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/core/vm/instructions_test.go b/core/vm/instructions_test.go index 000a22b2d8eb..c095e8b9f3e6 100644 --- a/core/vm/instructions_test.go +++ b/core/vm/instructions_test.go @@ -291,26 +291,33 @@ func opBenchmark(bench *testing.B, op executionFunc, args ...string) { var ( env = NewEVM(BlockContext{}, TxContext{}, nil, nil, params.TestChainConfig, Config{}) stack = newstack() + scope = &ScopeContext{nil, stack, nil} evmInterpreter = NewEVMInterpreter(env, env.vmConfig) ) env.interpreter = evmInterpreter // convert args - byteArgs := make([][]byte, len(args)) + intArgs := make([]*uint256.Int, len(args)) for i, arg := range args { - byteArgs[i] = common.Hex2Bytes(arg) + intArgs[i] = new(uint256.Int).SetBytes(common.Hex2Bytes(arg)) } pc := uint64(0) bench.ResetTimer() for i := 0; i < bench.N; i++ { - for _, arg := range byteArgs { - a := new(uint256.Int) - a.SetBytes(arg) - stack.push(a) + for _, arg := range intArgs { + stack.push(arg) } - op(&pc, evmInterpreter, &ScopeContext{nil, stack, nil}) + op(&pc, evmInterpreter, scope) stack.pop() } + bench.StopTimer() + + for i, arg := range args { + want := new(uint256.Int).SetBytes(common.Hex2Bytes(arg)) + if have := intArgs[i]; !want.Eq(have) { + bench.Fatalf("input #%d mutated, have %x want %x", i, have, want) + } + } } func BenchmarkOpAdd64(b *testing.B) { From 506778cc036b5c7011de69d144bb29c6296c5d2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Fri, 3 Jun 2022 10:40:14 +0200 Subject: [PATCH 06/12] core/vm: optimize jumpdest analysis (#23500) core/vm: optimize PUSH opcode discrimination --- core/vm/analysis.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/vm/analysis.go b/core/vm/analysis.go index 3733bab6a7c0..4aa8cfe70f11 100644 --- a/core/vm/analysis.go +++ b/core/vm/analysis.go @@ -76,7 +76,7 @@ func codeBitmapInternal(code, bits bitvec) bitvec { for pc := uint64(0); pc < uint64(len(code)); { op := OpCode(code[pc]) pc++ - if op < PUSH1 || op > PUSH32 { + if int8(op) < int8(PUSH1) { // If not PUSH (the int8(op) > int(PUSH32) is always false). continue } numbits := op - PUSH1 + 1 From 2f3f5d7a31e97741ebb429f6294c11ed44e4a057 Mon Sep 17 00:00:00 2001 From: Daniel Liu Date: Wed, 25 Sep 2024 10:15:14 +0800 Subject: [PATCH 07/12] core/vm: more linters (#24783) --- core/vm/contracts.go | 2 -- core/vm/instructions_test.go | 48 ++++++++++++++++++------------------ 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/core/vm/contracts.go b/core/vm/contracts.go index b06d0590c7f6..f7e10794c2b5 100644 --- a/core/vm/contracts.go +++ b/core/vm/contracts.go @@ -29,8 +29,6 @@ import ( "github.com/XinFinOrg/XDPoSChain/crypto/blake2b" "github.com/XinFinOrg/XDPoSChain/crypto/bn256" "github.com/XinFinOrg/XDPoSChain/params" - - //lint:ignore SA1019 Needed for precompile "golang.org/x/crypto/ripemd160" ) diff --git a/core/vm/instructions_test.go b/core/vm/instructions_test.go index c095e8b9f3e6..25dceabd5a40 100644 --- a/core/vm/instructions_test.go +++ b/core/vm/instructions_test.go @@ -230,35 +230,35 @@ func TestAddMod(t *testing.T) { } } -// getResult is a convenience function to generate the expected values -func getResult(args []*twoOperandParams, opFn executionFunc) []TwoOperandTestcase { - var ( - env = NewEVM(BlockContext{}, TxContext{}, nil, nil, params.TestChainConfig, Config{}) - stack = newstack() - pc = uint64(0) - interpreter = env.interpreter.(*EVMInterpreter) - ) - result := make([]TwoOperandTestcase, len(args)) - for i, param := range args { - x := new(uint256.Int).SetBytes(common.Hex2Bytes(param.x)) - y := new(uint256.Int).SetBytes(common.Hex2Bytes(param.y)) - stack.push(x) - stack.push(y) - _, err := opFn(&pc, interpreter, &ScopeContext{nil, stack, nil}) - if err != nil { - log.Fatalln(err) - } - actual := stack.pop() - result[i] = TwoOperandTestcase{param.x, param.y, fmt.Sprintf("%064x", actual)} - } - return result -} - // utility function to fill the json-file with testcases // Enable this test to generate the 'testcases_xx.json' files func TestWriteExpectedValues(t *testing.T) { t.Skip("Enable this test to create json test cases.") + // getResult is a convenience function to generate the expected values + getResult := func(args []*twoOperandParams, opFn executionFunc) []TwoOperandTestcase { + var ( + env = NewEVM(BlockContext{}, TxContext{}, nil, nil, params.TestChainConfig, Config{}) + stack = newstack() + pc = uint64(0) + interpreter = env.interpreter.(*EVMInterpreter) + ) + result := make([]TwoOperandTestcase, len(args)) + for i, param := range args { + x := new(uint256.Int).SetBytes(common.Hex2Bytes(param.x)) + y := new(uint256.Int).SetBytes(common.Hex2Bytes(param.y)) + stack.push(x) + stack.push(y) + _, err := opFn(&pc, interpreter, &ScopeContext{nil, stack, nil}) + if err != nil { + log.Fatalln(err) + } + actual := stack.pop() + result[i] = TwoOperandTestcase{param.x, param.y, fmt.Sprintf("%064x", actual)} + } + return result + } + for name, method := range twoOpMethods { data, err := json.Marshal(getResult(commonParams, method)) if err != nil { From 2ab87f99e9fb071677bcc73fa64b1f7987dfe0bb Mon Sep 17 00:00:00 2001 From: Daniel Liu Date: Wed, 25 Sep 2024 17:14:42 +0800 Subject: [PATCH 08/12] core/vm: remove empty lines --- core/vm/instructions_test.go | 3 --- core/vm/interpreter.go | 1 - core/vm/runtime/runtime_test.go | 1 - 3 files changed, 5 deletions(-) diff --git a/core/vm/instructions_test.go b/core/vm/instructions_test.go index 25dceabd5a40..a9ab947c2293 100644 --- a/core/vm/instructions_test.go +++ b/core/vm/instructions_test.go @@ -47,7 +47,6 @@ var commonParams []*twoOperandParams var twoOpMethods map[string]executionFunc func init() { - // Params is a list of common edgecases that should be used for some common tests params := []string{ "0000000000000000000000000000000000000000000000000000000000000000", // 0 @@ -93,7 +92,6 @@ func init() { } func testTwoOperandOp(t *testing.T, tests []TwoOperandTestcase, opFn executionFunc, name string) { - var ( env = NewEVM(BlockContext{}, TxContext{}, nil, nil, params.TestChainConfig, Config{}) stack = newstack() @@ -648,7 +646,6 @@ func TestCreate2Addreses(t *testing.T) { expected: "0xE33C0C7F7df4809055C3ebA6c09CFe4BaF1BD9e0", }, } { - origin := common.BytesToAddress(common.FromHex(tt.origin)) salt := common.BytesToHash(common.FromHex(tt.salt)) code := common.FromHex(tt.code) diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index c4b4b4143b80..ebfaa6387ca5 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -145,7 +145,6 @@ func NewEVMInterpreter(evm *EVM, cfg Config) *EVMInterpreter { // considered a revert-and-consume-all-gas operation except for // ErrExecutionReverted which means revert-and-keep-gas-left. func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) (ret []byte, err error) { - // Increment the call depth which is restricted to 1024 in.evm.depth++ defer func() { in.evm.depth-- }() diff --git a/core/vm/runtime/runtime_test.go b/core/vm/runtime/runtime_test.go index 34031a95e996..fc986c3a7628 100644 --- a/core/vm/runtime/runtime_test.go +++ b/core/vm/runtime/runtime_test.go @@ -374,7 +374,6 @@ func benchmarkNonModifyingCode(gas uint64, code []byte, name string, b *testing. // BenchmarkSimpleLoop test a pretty simple loop which loops until OOG // 55 ms func BenchmarkSimpleLoop(b *testing.B) { - staticCallIdentity := []byte{ byte(vm.JUMPDEST), // [ count ] // push args for the call From 8a8abffb983fdf8bd0ba62d2eeab33a41d67cce3 Mon Sep 17 00:00:00 2001 From: Daniel Liu Date: Wed, 25 Sep 2024 17:40:04 +0800 Subject: [PATCH 09/12] core/vm: not deep copy return data slice upon call completion (#25183) --- core/vm/instructions.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/vm/instructions.go b/core/vm/instructions.go index eaabc1a76ec6..36401455109d 100644 --- a/core/vm/instructions.go +++ b/core/vm/instructions.go @@ -710,7 +710,6 @@ func opCall(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byt } stack.push(&temp) if err == nil || err == ErrExecutionReverted { - ret = common.CopyBytes(ret) scope.Memory.Set(retOffset.Uint64(), retSize.Uint64(), ret) } scope.Contract.Gas += returnGas @@ -746,7 +745,6 @@ func opCallCode(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([ } stack.push(&temp) if err == nil || err == ErrExecutionReverted { - ret = common.CopyBytes(ret) scope.Memory.Set(retOffset.Uint64(), retSize.Uint64(), ret) } scope.Contract.Gas += returnGas @@ -775,7 +773,6 @@ func opDelegateCall(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext } stack.push(&temp) if err == nil || err == ErrExecutionReverted { - ret = common.CopyBytes(ret) scope.Memory.Set(retOffset.Uint64(), retSize.Uint64(), ret) } scope.Contract.Gas += returnGas @@ -804,7 +801,6 @@ func opStaticCall(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) } stack.push(&temp) if err == nil || err == ErrExecutionReverted { - ret = common.CopyBytes(ret) scope.Memory.Set(retOffset.Uint64(), retSize.Uint64(), ret) } scope.Contract.Gas += returnGas From 87cee1aff90ac56a1593a33632a9e5c9019610fa Mon Sep 17 00:00:00 2001 From: Daniel Liu Date: Wed, 25 Sep 2024 17:53:44 +0800 Subject: [PATCH 10/12] core/vm: minor trivial clean up (#25880) --- core/vm/instructions.go | 4 ++-- core/vm/interpreter.go | 15 +++------------ 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/core/vm/instructions.go b/core/vm/instructions.go index 36401455109d..c9f3fc380d03 100644 --- a/core/vm/instructions.go +++ b/core/vm/instructions.go @@ -22,8 +22,8 @@ import ( "github.com/XinFinOrg/XDPoSChain/common" "github.com/XinFinOrg/XDPoSChain/core/types" "github.com/XinFinOrg/XDPoSChain/params" + "github.com/XinFinOrg/XDPoSChain/crypto" "github.com/holiman/uint256" - "golang.org/x/crypto/sha3" ) func opAdd(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) { @@ -238,7 +238,7 @@ func opKeccak256(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ( data := scope.Memory.GetPtr(int64(offset.Uint64()), int64(size.Uint64())) if interpreter.hasher == nil { - interpreter.hasher = sha3.NewLegacyKeccak256().(keccakState) + interpreter.hasher = crypto.NewKeccakState() } else { interpreter.hasher.Reset() } diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index ebfaa6387ca5..ada646fdb00f 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -17,10 +17,9 @@ package vm import ( - "hash" - "github.com/XinFinOrg/XDPoSChain/common" "github.com/XinFinOrg/XDPoSChain/common/math" + "github.com/XinFinOrg/XDPoSChain/crypto" "github.com/XinFinOrg/XDPoSChain/log" ) @@ -68,21 +67,13 @@ type ScopeContext struct { Contract *Contract } -// keccakState wraps sha3.state. In addition to the usual hash methods, it also supports -// Read to get a variable amount of data from the hash state. Read is faster than Sum -// because it doesn't copy the internal state, but also modifies the internal state. -type keccakState interface { - hash.Hash - Read([]byte) (int, error) -} - // EVMInterpreter represents an EVM interpreter type EVMInterpreter struct { evm *EVM cfg Config - hasher keccakState // Keccak256 hasher instance shared across opcodes - hasherBuf common.Hash // Keccak256 hasher result array shared aross opcodes + hasher crypto.KeccakState // Keccak256 hasher instance shared across opcodes + hasherBuf common.Hash // Keccak256 hasher result array shared aross opcodes readOnly bool // Whether to throw on stateful modifications returnData []byte // Last CALL's return data for subsequent reuse From 1ad774fea84e67d74b2ca259c096fd6a7e1a885c Mon Sep 17 00:00:00 2001 From: Daniel Liu Date: Wed, 25 Sep 2024 18:07:10 +0800 Subject: [PATCH 11/12] core/vm: fix docstrings --- core/vm/interface.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/vm/interface.go b/core/vm/interface.go index 903a957a2017..5c695ffed703 100644 --- a/core/vm/interface.go +++ b/core/vm/interface.go @@ -79,12 +79,12 @@ type StateDB interface { // CallContext provides a basic interface for the EVM calling conventions. The EVM // depends on this context being implemented for doing subcalls and initialising new EVM contracts. type CallContext interface { - // Call another contract + // Call calls another contract. Call(env *EVM, me ContractRef, addr common.Address, data []byte, gas, value *big.Int) ([]byte, error) - // Take another's contract code and execute within our own context + // CallCode takes another contracts code and execute within our own context CallCode(env *EVM, me ContractRef, addr common.Address, data []byte, gas, value *big.Int) ([]byte, error) - // Same as CallCode except sender and value is propagated from parent to child scope + // DelegateCall is same as CallCode except sender and value is propagated from parent to child scope DelegateCall(env *EVM, me ContractRef, addr common.Address, data []byte, gas *big.Int) ([]byte, error) - // Create a new contract + // Create creates a new contract Create(env *EVM, me ContractRef, data []byte, gas, value *big.Int) ([]byte, common.Address, error) } From 29c72fbdad821b8d575655118f9d6ea872c531f2 Mon Sep 17 00:00:00 2001 From: Daniel Liu Date: Thu, 26 Sep 2024 15:51:24 +0800 Subject: [PATCH 12/12] core/vm: deepcopy jumptable when enabling extra eips (#26137) --- core/vm/interpreter.go | 8 +++++--- core/vm/jump_table.go | 11 +++++++++++ core/vm/jump_table_test.go | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 core/vm/jump_table_test.go diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index ada646fdb00f..9983b02290da 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -110,15 +110,17 @@ func NewEVMInterpreter(evm *EVM, cfg Config) *EVMInterpreter { cfg.JumpTable = &frontierInstructionSet } var extraEips []int + if len(cfg.ExtraEips) > 0 { + // Deep-copy jumptable to prevent modification of opcodes in other tables + cfg.JumpTable = copyJumpTable(cfg.JumpTable) + } for _, eip := range cfg.ExtraEips { - copy := *cfg.JumpTable - if err := EnableEIP(eip, ©); err != nil { + if err := EnableEIP(eip, cfg.JumpTable); err != nil { // Disable it, so caller can check if it's activated or not log.Error("EIP activation failed", "eip", eip, "error", err) } else { extraEips = append(extraEips, eip) } - cfg.JumpTable = © } cfg.ExtraEips = extraEips } diff --git a/core/vm/jump_table.go b/core/vm/jump_table.go index ca46f9166aad..46f3dc959d39 100644 --- a/core/vm/jump_table.go +++ b/core/vm/jump_table.go @@ -1058,3 +1058,14 @@ func newFrontierInstructionSet() JumpTable { return validate(tbl) } + +func copyJumpTable(source *JumpTable) *JumpTable { + dest := *source + for i, op := range source { + if op != nil { + opCopy := *op + dest[i] = &opCopy + } + } + return &dest +} diff --git a/core/vm/jump_table_test.go b/core/vm/jump_table_test.go new file mode 100644 index 000000000000..d7c9408bca2c --- /dev/null +++ b/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 . + +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 := newEip1559InstructionSet() + 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) +}