-
Notifications
You must be signed in to change notification settings - Fork 997
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.
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
TheVerge: spec draft #3230
TheVerge: spec draft #3230
Conversation
e4fac79
to
55c7841
Compare
Should this be rebased on top of deneb? CC @terencechain |
Co-Authored-By: Dankrad Feist <[email protected]>
Co-authored-by: terencechain <[email protected]>
Co-authored-by: Mikhail Kalinin <[email protected]>
Co-authored-by: Mikhail Kalinin <[email protected]>
db74090
to
d0c7bbc
Compare
Co-Authored-By: dapplion <[email protected]>
Let's swap this feature name/dir to reflect the EIP number -- https://eips.ethereum.org/EIPS/eip-6800 |
Co-authored-by: g11tech <[email protected]>
Fix the lint errors. Remove custom type `StateDiff` and then use `List[StemStateDiff, MAX_STEMS]` directly in `ExecutionWitness`.
# Null means not currently present | ||
current_value: Optional[Bytes32] | ||
# Null means value not updated | ||
new_value: Optional[Bytes32] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should either be updated for EIP-7495 StableContainer
/ Profile
(on SuffixStateDiff
), or unwrapped so that e.g. a zero value is used instead of None
. Would like to withdraw EIP-6475 style Optional
as the functionality is covered by the more advanced EIP-7495 as well.
SSZ library implementation progress here: https://stabilitynow.box
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right.
@gballet
The current Optional
in pyspec is the Python typing.Optional. Our SSZ lib Remerkleable only implemented SSZ Union
, so technically we can use Union[None, Bytes32]
here in the specs. However, other SSZ libs didn't implement Union
and may go implement the newer SSZ types as @etan-status suggested.
Is it possible to use Bytes32()
, the empty 0x00...00
, to represent null in this case so that we don't introduce new SSZ typing here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no it is not possible to use Bytes32()
and I was initially using Union[None, Byte32]
and was asked to replace it with Optional
. Precisely because Union
didn't exist (except in the wonderful gballet/ssz.zig, of course 😇).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to give StableContainer a shot, this is how it would look there:
class StableSuffixStateDiff(StableContainer[8]): # or [4] if this is not planned to grow in the future
suffix: Optional[Bytes1]
# Null means not currently present
current_value: Optional[Bytes32]
# Null means value not updated
new_value: Optional[Bytes32]
class SuffixStateDiff(Profile[StableSuffixStateDiff]):
suffix: Bytes1
# Null means not currently present
current_value: Optional[Bytes32]
# Null means value not updated
new_value: Optional[Bytes32]
Serialization of the Profile
looks like this:
- 1 byte optional_fields, 0b1/0 for current_value presence/absence, 0b1/0 for new_value presence/absence
- 1 byte suffix
- 32 bytes current_value if present, or nothing otherwise
- 32 bytes new_value if present, or nothing otherwise
Notably, no variable length offsets, this is 7-9 bytes more compact than the current EIP-6475 based impl, and requires one fewer hash as well.
Merkleization would look like this, with htr == hash_tree_root
and based on 8 capacity. can lower to 4 if sufficient, should reflect theoretical design space, not actual space used today, for future extensibility without breaking merkleization of existing EIP-4788 based verifiers:
- htr
- htr
- htr
- htr
- htr(suffix)
- htr(current_value) if current_value is not None else htr(0)
- htr
- htr(new_value) if new_value is not None else htr(0)
- htr(0)
- htr
- htr
- htr
- htr(0)
- htr(0)
- htr
- htr(0)
- htr(0)
- htr
- htr
- active_fields (suffix always 0b1, then 0b0/0b1 for current_value, then 0b0/0b1 for new_value, rest 0)
- htr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs custom remerkleable from here:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm as the initial version. 👍
It now passes the linter but we haven't enabled the tests yet.
This PR adds the execution witness to the
ExecutionPayload
container, and also describes the fork.TODO:
FIXME
infork.md
forhistorical_summaries
Co-Authored-By: Dankrad Feist [email protected]