Skip to content

Commit

Permalink
core/vm: make gas cost reporting to tracers correct (ethereum#22702)
Browse files Browse the repository at this point in the history
Previously, the makeCallVariantGasCallEIP2929 charged the cold account access cost directly, leading to an incorrect gas cost passed to the tracer from the main execution loop.
This change still temporarily charges the cost (to allow for an accurate calculation of the available gas for the call), but then afterwards refunds it and instead returns the correct total gas cost to be then properly charged in the main loop.
  • Loading branch information
holiman authored Apr 27, 2021
1 parent a0a99e6 commit 65a1c2d
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 4 deletions.
22 changes: 18 additions & 4 deletions core/vm/operations_acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,15 @@ 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)
// 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
if !contract.UseGas(ColdAccountAccessCostEIP2929 - WarmStorageReadCostEIP2929) {
// Charge the remaining difference here already, to correctly calculate available
// gas for call
if !contract.UseGas(coldCost) {
return 0, ErrOutOfGas
}
}
Expand All @@ -189,7 +194,16 @@ 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, 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
}
}

Expand Down
80 changes: 80 additions & 0 deletions core/vm/runtime/runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}

0 comments on commit 65a1c2d

Please sign in to comment.