Skip to content

add SLOAD & SSTORE spec#11

Closed
0xmountaintop wants to merge 90 commits into
masterfrom
storage
Closed

add SLOAD & SSTORE spec#11
0xmountaintop wants to merge 90 commits into
masterfrom
storage

Conversation

@0xmountaintop
Copy link
Copy Markdown

No description provided.

Comment thread specs/opcode/54SLOAD_55SSTORE.md Outdated
# SLOAD & SSTORE op code

Disclaimer: this version of the sload/sstore sepc only corresponds to [EIP-2929](https://eips.ethereum.org/EIPS/eip-2929).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

but does not implement EIP 2930 yet? (if yes, add this into the spec)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

none of 2028, 2930, 1559 is implemented yet. should I add them all?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

in fact. 2930 breaks (improves on) 2029. if 2930 then it can't exactly be 2929

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed in b1db976

@lispc
Copy link
Copy Markdown

lispc commented Jan 11, 2022

great job, i did not realize sstore/sload are so complex... warm code reset refund...

One question: there are many branches in the sstore gas cost, while there are only 4 test cases. Will it be better to improve test coverage?

@0xmountaintop
Copy link
Copy Markdown
Author

One question: there are many branches in the sstore gas cost, while there are only 4 test cases. Will it be better to improve test coverage?

ok will do

@0xmountaintop
Copy link
Copy Markdown
Author

after merging with master, need to fix some conflicts again

0xmountaintop and others added 5 commits January 11, 2022 16:03
* minor

* refactor

* so fk

* update

* some cleaning
@0xmountaintop 0xmountaintop requested a review from lispc January 11, 2022 10:02
@0xmountaintop
Copy link
Copy Markdown
Author

@lispc updated

- `SSTORE`: +2
- gas:
- `SLOAD`:
+ the accessed address is warm: gas + WARM_STORAGE_READ_COST
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
+ the accessed address is warm: gas + WARM_STORAGE_READ_COST
+ the accessed address is warm: + WARM_STORAGE_READ_COST

I think we only need to mark how much gas needs to add for each step.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I prefer keeping them as "gas + XXX", so that it's consistent with other markdown files.

from ..opcode import Opcode
from ..table import CallContextFieldTag, TxContextFieldTag

COLD_SLOAD_COST = 2100
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

move all the gas constants into a single file?

Copy link
Copy Markdown
Author

@0xmountaintop 0xmountaintop Jan 12, 2022

Choose a reason for hiding this comment

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

I try moving to "src/zkevm_specs/params.py" and use

from ......params import (
    COLD_SLOAD_COST,
    WARM_STORAGE_READ_COST,
    SLOAD_GAS,
    SSTORE_SET_GAS,
    SSTORE_RESET_GAS,
    SSTORE_CLEARS_SCHEDULE,
)

but didn't make it.

encounter an error with ImportError: attempted relative import beyond top-level package

so I just put it as "src/zkevm_specs/evm/execution/params.py". see 9d7c63b

@0xmountaintop 0xmountaintop deleted the storage branch February 21, 2022 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants