Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filling fixes #516

Merged
merged 8 commits into from
Oct 23, 2024
Merged

Filling fixes #516

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0c7fc0b
fix makeCallVariantGasEIP4762
jsign Oct 22, 2024
553047e
fix
jsign Oct 22, 2024
66ec2be
fix
jsign Oct 22, 2024
ab5cb6d
fix
jsign Oct 22, 2024
6ed5997
ci: target v0.0.6
jsign Oct 22, 2024
e6de39b
improve fix
jsign Oct 23, 2024
30bc0a0
add witness gas to "old" gas upon error
gballet Oct 23, 2024
700da70
workflow: remove target branch from list of branches
gballet Oct 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 6 additions & 51 deletions core/vm/operations_verkle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,18 @@ func gasExtCodeHash4762(evm *EVM, contract *Contract, stack *Stack, mem *Memory,
return evm.Accesses.TouchCodeHash(address[:], false, contract.Gas, true), nil
}

func makeCallVariantGasEIP4762(oldCalculator gasFunc) gasFunc {
func makeCallVariantGasEIP4762(oldCalculator gasFunc, withTransferCosts bool) gasFunc {
return func(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize uint64) (uint64, error) {
var (
target = common.Address(stack.Back(1).Bytes20())
transfersValue = !stack.Back(2).IsZero()
witnessGas uint64
_, isPrecompile = evm.precompile(target)
isSystemContract = evm.isSystemContract(target)
)

// If value is transferred, it is charged before 1/64th
// is subtracted from the available gas pool.
if transfersValue {
if withTransferCosts && !stack.Back(2).IsZero() {
Copy link
Collaborator Author

@jsign jsign Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I honestly would like to have the transfersValue to make this if more clear, but breaking up the if without the && creates a "branching" complexity for the if-elseif-else.

I feel safer being sure we remove L60 since not doing that adds some extra assumptions on what is on the stack for withTransferCosts=false situations -- so with my proposed way I access Back(2) only in cases that matter. In other cases that Back(2) would lead to the wrong semantics of the variable name.

This might feel pedantic, but I prefer not to have confusions or hacky names that only make sense in some situations.

wantedValueTransferWitnessGas := evm.Accesses.TouchAndChargeValueTransfer(contract.Address().Bytes()[:], target[:], contract.Gas)
if wantedValueTransferWitnessGas > contract.Gas {
return wantedValueTransferWitnessGas, nil
Expand Down Expand Up @@ -103,55 +102,11 @@ func makeCallVariantGasEIP4762(oldCalculator gasFunc) gasFunc {
}
}

// TODO(verkle): clean up... maybe we can add another parameter in the other wrapper, but can get messy.
func makeCallVariantGasEIP4762WithoutTransfer(oldCalculator gasFunc) gasFunc {
return func(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize uint64) (uint64, error) {
var (
target = common.Address(stack.Back(1).Bytes20())
witnessGas uint64
_, isPrecompile = evm.precompile(target)
isSystemContract = evm.isSystemContract(target)
)

// If value is transferred, it is charged before 1/64th
// is subtracted from the available gas pool.
if isPrecompile || isSystemContract {
witnessGas = params.WarmStorageReadCostEIP2929
} else {
// The charging for the value transfer is done BEFORE subtracting
// the 1/64th gas, as this is considered part of the CALL instruction.
// (so before we get to this point)
// But the message call is part of the subcall, for which only 63/64th
// of the gas should be available.
wantedMessageCallWitnessGas := evm.Accesses.TouchAndChargeMessageCall(target.Bytes(), contract.Gas-witnessGas)
var overflow bool
if witnessGas, overflow = math.SafeAdd(witnessGas, wantedMessageCallWitnessGas); overflow {
return 0, ErrGasUintOverflow
}
if witnessGas > contract.Gas {
return witnessGas, nil
}
}

contract.Gas -= witnessGas
gas, err := oldCalculator(evm, contract, stack, mem, memorySize)
if err != nil {
return 0, err
}
contract.Gas += witnessGas
var overflow bool
if gas, overflow = math.SafeAdd(gas, witnessGas); overflow {
return 0, ErrGasUintOverflow
}
return gas, nil
}
}

var (
gasCallEIP4762 = makeCallVariantGasEIP4762(gasCall)
gasCallCodeEIP4762 = makeCallVariantGasEIP4762WithoutTransfer(gasCallCode)
gasStaticCallEIP4762 = makeCallVariantGasEIP4762WithoutTransfer(gasStaticCall)
gasDelegateCallEIP4762 = makeCallVariantGasEIP4762WithoutTransfer(gasDelegateCall)
gasCallEIP4762 = makeCallVariantGasEIP4762(gasCall, true)
gasCallCodeEIP4762 = makeCallVariantGasEIP4762(gasCallCode, false)
gasStaticCallEIP4762 = makeCallVariantGasEIP4762(gasStaticCall, false)
gasDelegateCallEIP4762 = makeCallVariantGasEIP4762(gasDelegateCall, false)
)

func gasSelfdestructEIP4762(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize uint64) (uint64, error) {
Expand Down