SLOAD/SSTORE opcodes

[leonardoalt]

@barryWhiteHat suggested I worked on this, so here's my attempt. I used the MLOAD/MSTORE specs as base, and I'm not sure about several things in this spec cus I don't know a lot about that stuff in general, especially in the circuit and constraints sections.

I'll start working on the Rust zkevm-circuit side of this once this spec is stable.

Here are my questions for this spec:

• Does the Access Set need to be taken into account somehow? Both SLOAD and STORE read from the set and potentially update it with the pair (context_address, key). Currently I just used a Boolean flag is_touched, which I guess would somehow come from the busmapping? Would we need two versions of that flag here, one pre and one post update?
• For the gas computation we need to know the storage[key] value at the beginning of the transaction and the current one. Would both also just come from the busmapping?
• In the constraints part, for memory 32 lookups were needed, one for each byte. I wrote 1 lookup here because storage slots are always 32 bytes, but does that mean we also need 32 lookups here?

Happy to get any direction on those items and fix them!

[han0110]

Thanks for exploring this! BTW there is another WIP PR for SLOAD/SSTORE here.

Let me try to answer the questions:

Does the Access Set need to be taken into account somehow? Both SLOAD and STORE read from the set and potentially update it with the pair (context_address, key).

Yes, we maintain everything random accessible data in rw_table (read-write-able part of bus-mapping), which could be retrieved by lookup with current rw_counter as timestamp for consistency check (by State circuit).

Also currently we assume we would be able to verify multiple transactions in a proof, so the tuple will be like (tx_id, context_address, key) for separation.

Currently I just used a Boolean flag is_touched, which I guess would somehow come from the busmapping? Would we need two versions of that flag here, one pre and one post update?

Yes, every access record from rw_table contains current and previous value, so when we touch the access list, we always write the current value to 1, and infer is_cold_access = current - previous.

For the gas computation we need to know the storage[key] value at the beginning of the transaction and the current one. Would both also just come from the busmapping?

Yes, we get storage[key] also by lookup to rw_table. But I totally forgot the gas cost calculation of SSTORE needs the initial value from previous state trie, I think in State circuit we should track the initial value and provide it in rw_table for gas cost calculation.

In the constraints part, for memory 32 lookups were needed, one for each byte. I wrote 1 lookup here because storage slots are always 32 bytes, but does that mean we also need 32 lookups here?

I don't think we need, because value on stack and storage in rw_table all live as random linear combination of 32 bytes (the approach we try to have 32 bytes value in a 253-bit field), the job for SLOAD/SSTORE is to move them around. As for memory, the de-combination of stack value is required because values in memory live as bytes and we need to maintain them by every memory address.

[0xmountaintop]

I am also working on this (but happy to have more people to work together&discuss), and have had a few discussions with @han0110, so may be able to answer a bit here.

here's my attempt: scroll-tech#11

Does the Access Set need to be taken into account?

yes. and this will affect the gas calculation. I think currently at this stage we could just assume it will be provided in the lookup and no need to calc by ourselves (like in begin_tx.py)?
This also defers the need to "know to know the storage[key] value at the beginning of the transaction and the current one". We can think about how to deal with it in a good way in the future.

Would we need two versions of that flag here, one pre and one post update?

we need to look it up in access_list, and mark it after operations.

In the constraints part, for memory 32 lookups were needed, one for each byte. I wrote 1 lookup here because storage slots are always 32 bytes, but does that mean we also need 32 lookups here?

yes. we only need 1 lookup, for 1 word.

---

BTW, as pointed out by @han0110, we need to deal with state revert in sstore.

[0xmountaintop]

we could just use type annotation

def write(self, key: U256, value: U256):

so that we don;t need these asserts?

[0xmountaintop]

you mention in the markdown spec that you use 1 word instead of 32bytes. (and you are right about that).

I think there's no need to convert U256 to and from [U8], which seems redundant.

[leonardoalt]

Thanks both for the answers/comments/reviews!

Ah ok, wasn't aware of the other repo. For info, what's the difference between the two spec repos?

Agree with all the comments.
One thing: I briefly talked to @barryWhiteHat today, and he had the opinion that the access list and its influence on the gas cost shouldn't be taken into account now, that is, the current spec should be legacy only and this would be taken care of later on.
What's your take on this?

Please feel free to close this PR! We can continue the discussion on the other PR, if needed.

[han0110]

Ah, I thought only EIP2930 (tx level access list) is postponed by only supporting legacy tx format, so I still implemented EIP2929 in BeginTx and CALL.

@barryWhiteHat Do you think we should remove features of access list EIPs (can do quickly) and add them back in the future, or should we go with EIP2929 directly (I found it's a relatively straightforward feature to implement)?

[barryWhiteHat]

Yeah i think EIP2929 should be okay. IIUC its just reprice for some opocdes and that also gets applied to legacy txs.

[0xmountaintop]

Ah, I thought only EIP2930 (tx level access list) is postponed by only supporting legacy tx format, so I still implemented EIP2929 in BeginTx and CALL.

@barryWhiteHat Do you think we should remove features of access list EIPs (can do quickly) and add them back in the future, or should we go with EIP2929 directly (I found it's a relatively straightforward feature to implement)?

btw should I wait for #78?

[han0110]

btw should I wait for #78?

No, I think scroll-tech#11 can go without that.

[ed255]

I noticed that #106 was recently merged which added the SLOAD and SSTORE opcode specs, and thus superseded this PR. Sot seems there will be no further progress with this PR; maybe we can close it?

[leonardoalt]

Yep!