From cc2aa7d3f08ddaafa9a5a87cf24906b26009169e Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 20 Apr 2021 09:58:16 +0200 Subject: [PATCH 1/3] core/vm/runtime: testcase for call variants cost --- core/vm/runtime/runtime_test.go | 80 +++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/core/vm/runtime/runtime_test.go b/core/vm/runtime/runtime_test.go index 26927553245b..dcf2d0d4478d 100644 --- a/core/vm/runtime/runtime_test.go +++ b/core/vm/runtime/runtime_test.go @@ -608,3 +608,83 @@ func TestEip2929Cases(t *testing.T) { "account (cheap)", code) } } + +// TestColdAccountAccessCost test that the cold account access cost is reported +// correctly +// see: https://github.com/ethereum/go-ethereum/issues/22649 +func TestColdAccountAccessCost(t *testing.T) { + for i, tc := range []struct { + code []byte + step int + want uint64 + }{ + { // EXTCODEHASH(0xff) + code: []byte{byte(vm.PUSH1), 0xFF, byte(vm.EXTCODEHASH), byte(vm.POP)}, + step: 1, + want: 2600, + }, + { // BALANCE(0xff) + code: []byte{byte(vm.PUSH1), 0xFF, byte(vm.BALANCE), byte(vm.POP)}, + step: 1, + want: 2600, + }, + { // CALL(0xff) + code: []byte{ + byte(vm.PUSH1), 0x0, + byte(vm.DUP1), byte(vm.DUP1), byte(vm.DUP1), byte(vm.DUP1), + byte(vm.PUSH1), 0xff, byte(vm.DUP1), byte(vm.CALL), byte(vm.POP), + }, + step: 7, + want: 2855, + }, + { // CALLCODE(0xff) + code: []byte{ + byte(vm.PUSH1), 0x0, + byte(vm.DUP1), byte(vm.DUP1), byte(vm.DUP1), byte(vm.DUP1), + byte(vm.PUSH1), 0xff, byte(vm.DUP1), byte(vm.CALLCODE), byte(vm.POP), + }, + step: 7, + want: 2855, + }, + { // DELEGATECALL(0xff) + code: []byte{ + byte(vm.PUSH1), 0x0, + byte(vm.DUP1), byte(vm.DUP1), byte(vm.DUP1), + byte(vm.PUSH1), 0xff, byte(vm.DUP1), byte(vm.DELEGATECALL), byte(vm.POP), + }, + step: 6, + want: 2855, + }, + { // STATICCALL(0xff) + code: []byte{ + byte(vm.PUSH1), 0x0, + byte(vm.DUP1), byte(vm.DUP1), byte(vm.DUP1), + byte(vm.PUSH1), 0xff, byte(vm.DUP1), byte(vm.STATICCALL), byte(vm.POP), + }, + step: 6, + want: 2855, + }, + { // SELFDESTRUCT(0xff) + code: []byte{ + byte(vm.PUSH1), 0xff, byte(vm.SELFDESTRUCT), + }, + step: 1, + want: 7600, + }, + } { + tracer := vm.NewStructLogger(nil) + Execute(tc.code, nil, &Config{ + EVMConfig: vm.Config{ + Debug: true, + Tracer: tracer, + }, + }) + have := tracer.StructLogs()[tc.step].GasCost + if want := tc.want; have != want { + for ii, op := range tracer.StructLogs() { + t.Logf("%d: %v %d", ii, op.OpName(), op.GasCost) + } + t.Fatalf("tescase %d, gas report wrong, step %d, have %d want %d", i, tc.step, have, want) + } + } +} From 767dd93102dd3409e86671c306c2850470ffc413 Mon Sep 17 00:00:00 2001 From: Ansgar Dietrichs Date: Fri, 16 Apr 2021 00:14:13 +0200 Subject: [PATCH 2/3] core/vm: fix EIP-2929 call gas tracing --- core/vm/operations_acl.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/core/vm/operations_acl.go b/core/vm/operations_acl.go index 191953ce5e4a..57ac0799063a 100644 --- a/core/vm/operations_acl.go +++ b/core/vm/operations_acl.go @@ -177,9 +177,12 @@ func makeCallVariantGasCallEIP2929(oldCalculator gasFunc) gasFunc { return func(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize uint64) (uint64, error) { addr := common.Address(stack.Back(1).Bytes20()) // Check slot presence in the access list - if !evm.StateDB.AddressInAccessList(addr) { + warmAccess := evm.StateDB.AddressInAccessList(addr) + if !warmAccess { evm.StateDB.AddAddressToAccessList(addr) // The WarmStorageReadCostEIP2929 (100) is already deducted in the form of a constant cost + // Temporarily charge the remaining difference here already, to correctly calculate available + // gas for call if !contract.UseGas(ColdAccountAccessCostEIP2929 - WarmStorageReadCostEIP2929) { return 0, ErrOutOfGas } @@ -189,7 +192,18 @@ func makeCallVariantGasCallEIP2929(oldCalculator gasFunc) gasFunc { // - transfer value // - memory expansion // - 63/64ths rule - return oldCalculator(evm, contract, stack, mem, memorySize) + gas, err := oldCalculator(evm, contract, stack, mem, memorySize) + if warmAccess || err != nil { + return gas, err + } + // In case of a cold access, add the already charged gas back and return the total sum + // as dynamic gas to be properly accounted for and charged on the outside + if gas, overflow := math.SafeAdd(gas, ColdAccountAccessCostEIP2929-WarmStorageReadCostEIP2929); overflow { + return 0, ErrGasUintOverflow + } else { + contract.Gas += ColdAccountAccessCostEIP2929 - WarmStorageReadCostEIP2929 + return gas, nil + } } } From 70a80281f7b14ec13cdc2b00a366184f21927249 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 20 Apr 2021 10:12:26 +0200 Subject: [PATCH 3/3] core/vm: minor changes --- core/vm/operations_acl.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/core/vm/operations_acl.go b/core/vm/operations_acl.go index 57ac0799063a..45b51d80cdf9 100644 --- a/core/vm/operations_acl.go +++ b/core/vm/operations_acl.go @@ -178,12 +178,14 @@ func makeCallVariantGasCallEIP2929(oldCalculator gasFunc) gasFunc { addr := common.Address(stack.Back(1).Bytes20()) // Check slot presence in the access list warmAccess := evm.StateDB.AddressInAccessList(addr) + // The WarmStorageReadCostEIP2929 (100) is already deducted in the form of a constant cost, so + // the cost to charge for cold access, if any, is Cold - Warm + coldCost := ColdAccountAccessCostEIP2929 - WarmStorageReadCostEIP2929 if !warmAccess { evm.StateDB.AddAddressToAccessList(addr) - // The WarmStorageReadCostEIP2929 (100) is already deducted in the form of a constant cost - // Temporarily charge the remaining difference here already, to correctly calculate available + // Charge the remaining difference here already, to correctly calculate available // gas for call - if !contract.UseGas(ColdAccountAccessCostEIP2929 - WarmStorageReadCostEIP2929) { + if !contract.UseGas(coldCost) { return 0, ErrOutOfGas } } @@ -196,14 +198,12 @@ func makeCallVariantGasCallEIP2929(oldCalculator gasFunc) gasFunc { if warmAccess || err != nil { return gas, err } - // In case of a cold access, add the already charged gas back and return the total sum - // as dynamic gas to be properly accounted for and charged on the outside - if gas, overflow := math.SafeAdd(gas, ColdAccountAccessCostEIP2929-WarmStorageReadCostEIP2929); overflow { - return 0, ErrGasUintOverflow - } else { - contract.Gas += ColdAccountAccessCostEIP2929 - WarmStorageReadCostEIP2929 - return gas, nil - } + // In case of a cold access, we temporarily add the cold charge back, and also + // add it to the returned gas. By adding it to the return, it will be charged + // outside of this function, as part of the dynamic gas, and that will make it + // also become correctly reported to tracers. + contract.Gas += coldCost + return gas + coldCost, nil } }