Update EIP-4762: reworked gas schedule from interop

[gballet]

For reference: https://notes.ethereum.org/EHXrhQ8-QdeqnqLH3-KBuw?view

[eth-bot]

✅ All reviewers have approved.

[gabrocheleau]

Should we update CODE_KECCAK_LEAF_KEY to 1 instead of leaving 1 and 2 empty?

[g11tech]

this implies that stateless clients need to maintain system contracts . Also there is an unresolved issue of stateless clients not knowing if the contract has been actually deployed or not.

we should simply declare that stateless clients should assume activated system contracts deployed.

[gballet]

this implies that stateless clients need to maintain system contracts

it does not: because there is no obligation to call the contract's EVM, you are free to write the storage slots directly without having a local copy of the code.

Also there is an unresolved issue of stateless clients not knowing if the contract has been actually deployed or not.

I also don't see why this would be true. System contract activation is part of the pectra fork, so it should assume that.

we should simply declare that stateless clients should assume activated system contracts deployed.

indeed. Or do you mean that it should be in the spec itself?

[g11tech]

because there is no obligation to call the contract's EVM, you are free to write the storage slots directly without having a local copy of the code

for system contracts like 7002, even stateless focused clients like ethereumjs just does the EVM call to get the triggered exits contract to get the exits. But this isn't a deal breaker just something clients need to bundle in verkle.

System contract activation is part of the pectra fork

deployment is a manual exercise and not really part of client hardfork (that clients deploy this contract on forkblock which imo should have been a better mechanism to deploy the tx if not already deployed). I mean I already had done the mistake of not adding a contract in a local testnet and then debugging why things weren't working. So I would like things foolproof if possible. The problem could become acute as the number of such contracts grow.

indeed. Or do you mean that it should be in the spec itself?

yes a simple mention of this would clearup the "ruleset" for the stateless client.

[g11tech]

whole account = basic data + code hash? and 2929 warm cost if accessed in a tx?

[gballet]

yes

[gballet]

this is because the coinbase might not exist, so to avoid a lot of issues, we decide to touch the whole account so that it's created if it doesn't exist already.

[g11tech]

what if there is no tx that puts any value to it, and lets say there is no tips that get added to a non existent coinbase, then it should be 158 cleaned up in the end of block?

[jsign]

is warmed before the block

"before the block" is maybe confusing?

What about:

The coinbase account basic data and code hash is accessed before transaction execution, and considered warm for all txs.

[tanishqjasoria]

i think this is covered below?
https://github.com/ethereum/EIPs/pull/8550/files#diff-cd2aaa222faeb6081a41605386a509b3c4839285cd15f1b0ebe009318707914eR126

[tanishqjasoria]

i think this is also duplicated - https://github.com/ethereum/EIPs/pull/8550/files#diff-cd2aaa222faeb6081a41605386a509b3c4839285cd15f1b0ebe009318707914eL143

[tanishqjasoria]

we also clarify that the contract creation complete gas cost should be charged before adding the 1/64 gas after the creation is completed.

[tanishqjasoria]

should we add that the code chunks during system calls are not to be warmed? or it might cause additional confusion?

[g11tech]

system calls totally ignore code chunks imo both for accesses and gas compute and we can clearly mention it to remove confusion

[tanishqjasoria]

we should add the clarification about storage cells here.
If in a trasnaction, storage value is set and then reset to zero, we should insert zero to the tree and not leave it empty because the user paid the gas cost.
Basically if we charge CHUNK_FILL_COST, fill the chunk value.

[g11tech]

lateral question/observation to the above:

this is just because withdrawals are processed at the end of the block, and there is no actual cost charged to the coinbase for processing them?

[jsign]

I raised a similar confusion yesterday to Guillaume about this line.

but are added to the witness at no cost ...

seems to imply there're a case where they're added at a cost; but who is paying that cost?

If the idea is that withdrawals never pay a cost, from my perspective clarifying they're "cold" (as mentioned in the first words in the sentence) is confusing.

[jsign]

@gballet, I wonder if we're missing an extra clarification here.

The *CALL variants that can be value-bearing, should also respect this rule?

Today in geth, we have this order (e.g: makeCallVariantGasEIP4762(gasCall)):

• Charge general *CALL costs. (inside makeCallVariantGasEIP4762(...))
• Do the 1/64 stuff (inside gasCall(...))
• Charge BALANCE if transfersValue (inside gasCall(...))

So I'm wondering if this requires a clarification, or was intentional. If was intentional it means that today in kaustinenv6-geth we need a change.

[matkt]

do we have after the gas rework some logic to charge warm storage when stateless gas cost is zero ? for example ethereum/go-ethereum@kaustinen-with-shapella...gballet:go-ethereum:kaustinen-with-shapella#diff-a6893c63a589eae0dd0ec9d5282aa504592c3024189cba62504b75c43a019658R57
if yes I think it will be nice to have that in the doc

[jsign]

Shouldn't we also touch the VERSION thus BASIC_DATA_LEAF_KEY?

[gballet]

TODO: add version

[jsign]

[Edit: comment quoting code is not correct, I meant L173 (tx.target, 0, BASIC_DATA_LEAF_KEY)]

If we have a tx (non-value-bearing) targeting a precompile or system contract, we don't need to touch BASIC_DATA. (i.e: stateless client knows the code-size)

We can still argue that this rule should be avoided to avoid a border case. But considering we didn't do any similar simplification for precompiles/system contracts in other places in the EIP (i.e. we avoid adding in the witness for them as much as possible), then maybe not doing it here is actually adding a border case (we have to remember this is an exception), not removing it.

No strong opinions, just surfacing this.
cc @gballet @g11tech @gabrocheleau @tanishqjasoria @matkt

[morph-dev]

Since we are cleaning up the spec, I believe we can simplify pedersen_hash function as well.

def pedersen_hash(inp: bytes) -> bytes32:
    assert len(inp) <= 255 * 16
    # Interpret input as list of 128 bit (16 byte) integers
    ext_input = inp + b"\0" * (255 * 16 - len(inp))
    ints = [2 + 256 * len(inp)] + \
           [int.from_bytes(ext_input[16 * i:16 * (i + 1)], 'little') for i in range(255)]
    return compute_commitment_root(ints).hash_point_to_bytes()

To my understanding, it's only used with input that is exactly 64 bytes long: 32 bytes for address + 32 bytes for tree_index. I understand the idea to be more generic, but does it have to be?

I propose to simplify it to something like this (if we want to keep identical functionality):

def pedersen_hash(address: Address32, tree_index: int) -> bytes32:
    marker = 16386 # 2 + 256 * 64
    ints = [marker,
            int.from_bytes(address[:16], 'little'),
            int.from_bytes(address[16:], 'little'),
            tree_index % (2 ** 128),
            tree_index / (2 ** 128)] + \
           [0] * 251
    return compute_commitment_root(ints).hash_point_to_bytes()

I believe that we can simplify it further and just use 2 as the marker.

[gballet]

To my understanding, it's only used with input that is exactly 64 bytes long: 32 bytes for address + 32 bytes for tree_index. I understand the idea to be more generic, but does it have to be?

What you describe is already implemented in e.g. geth but is an optimization. Simplifying it would make it more specific, when this describes a hash function that could be used for more things (and it was in previous versions of this spec, so presumably it can be used even more in the future).

I propose to simplify it to something like this (if we want to keep identical functionality):

def pedersen_hash(address: Address32, tree_index: int) -> bytes32:
    marker = 16386 # 2 + 256 * 64
    ints = [marker,
            int.from_bytes(address[:16], 'little'),
            int.from_bytes(address[16:], 'little'),
            tree_index % (2 ** 128),
            tree_index / (2 ** 128)] + \
           [0] * 251
    return compute_commitment_root(ints).hash_point_to_bytes()

this is assuming tree_index is little-endian

I believe that we can simplify it further and just use 2 as the marker.

if you assume a fixed input, sure. But if not, [a b c d 0 0 0 0 ... ] and [a b c d] would have the same hash.

[eth-bot]

All Reviewers Have Approved; Performing Automatic Merge...